Re: [Qemu-ppc] [2.3 V2 PATCH 2/6] target-ppc: Fix Floating Point Move Instructions That Set CR1

[Alexander Graf]

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 <address@hidden>
> ---
>  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