On 20.11.14 15:32, Tom Musta wrote:
> On 11/20/2014 8:14 AM, Alexander Graf wrote:
>>
>>
>> On 12.11.14 22:46, Tom Musta wrote:
>>> The Floating Point Move instructions (fmr., fabs., fnabs., fneg.,
>>> and fcpsgn.) incorrectly copy FPSCR[FPCC] instead of [FX,FEX,VX,OX].
>>> Furthermore, the current code does this via a call to gen_compute_fprf,
>>> which is awkward since these instructions do not actually set FPRF.
>>>
>>> Change the code to use the gen_set_cr1_from_fpscr utility.
>>>
>>> Signed-off-by: Tom Musta <tommu...@gmail.com>
>>> ---
>>> target-ppc/translate.c | 50
>>> ++++++++++++++++++++++++++++-------------------
>>> 1 files changed, 30 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>>> index 910ce56..2d79e39 100644
>>> --- a/target-ppc/translate.c
>>> +++ b/target-ppc/translate.c
>>> @@ -2077,6 +2077,21 @@ static void gen_srd(DisasContext *ctx)
>>> }
>>> #endif
>>>
>>> +#if defined(TARGET_PPC64)
>>> +static void gen_set_cr1_from_fpscr(DisasContext *ctx)
>>> +{
>>> + TCGv_i32 tmp = tcg_temp_new_i32();
>>> + tcg_gen_trunc_tl_i32(tmp, cpu_fpscr);
>>> + tcg_gen_shri_i32(cpu_crf[1], tmp, 28);
>>> + tcg_temp_free_i32(tmp);
>>> +}
>>> +#else
>>> +static void gen_set_cr1_from_fpscr(DisasContext *ctx)
>>> +{
>>> + tcg_gen_shri_tl(cpu_crf[1], cpu_fpscr, 28);
>>> +}
>>> +#endif
>>> +
>>> /*** Floating-Point arithmetic
>>> ***/
>>> #define _GEN_FLOAT_ACB(name, op, op1, op2, isfloat, set_fprf, type)
>>> \
>>> static void gen_f##name(DisasContext *ctx)
>>> \
>>> @@ -2370,7 +2385,9 @@ static void gen_fabs(DisasContext *ctx)
>>> }
>>> tcg_gen_andi_i64(cpu_fpr[rD(ctx->opcode)], cpu_fpr[rB(ctx->opcode)],
>>> ~(1ULL << 63));
>>> - gen_compute_fprf(cpu_fpr[rD(ctx->opcode)], 0, Rc(ctx->opcode) != 0);
>>> + if (unlikely(Rc(ctx->opcode))) {
>>> + gen_set_cr1_from_fpscr(ctx);
>>> + }
>>
>> I don't quite understand this. We set cr1 based on fpscr, but we don't
>> recalculate the respective fpscr bits?
>>
>> Wouldn't we get outdated comparison data?
>>
>>
>> Alex
>>
>
> Nope.
>
> The floating point move instructions don't actually even alter the FPSCR.
> From the ISA (see the last sentence):
>
> 4.6.5 Floating-Point Move Instructions
> These instructions copy data from one floating-point
> register to another, altering the sign bit (bit 0) as
> described below for fneg, fabs, fnabs, and fcpsgn.
> These instructions treat NaNs just like any other kind of
> value (e.g., the sign bit of a NaN may be altered by
> fneg, fabs, fnabs, and fcpsgn). These instructions do
> not alter the FPSCR.
Thanks, applied with the following squashed in:
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 7aef089..35c3a16 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2088,7 +2088,7 @@ static void gen_set_cr1_from_fpscr(DisasContext *ctx)
#else
static void gen_set_cr1_from_fpscr(DisasContext *ctx)
{
- tcg_gen_shri_tl(cpu_crf[1], cpu_fpscr, 28);
+ tcg_gen_shri_tl(cpu_crf[1], cpu_fpscr, 28);
}
#endif
Alex
