Re: [Qemu-devel] [PATCH v2 13/27] target/arm: convert conversion helpers to fpst/ahp_flag

[Richard Henderson]

On 05/14/2018 06:41 AM, Peter Maydell wrote:
> On 12 May 2018 at 01:42, Richard Henderson <address@hidden> wrote:
>> From: Alex Bennée <address@hidden>
>>
>> Instead of passing env and leaving it up to the helper to get the
>> right fpstatus we pass it explicitly. There was already a get_fpstatus
>> helper for neon for the 32 bit code. We also add an get_ahp_flag() for
>> passing the state of the alternative FP16 format flag. This leaves
>> scope for later tracking the AHP state in translation flags.
>>
>> Signed-off-by: Alex Bennée <address@hidden>
>> Signed-off-by: Richard Henderson <address@hidden>
> 
>> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
>> index a0b0c43d12..570791e7b5 100644
>> --- a/target/arm/translate-a64.c
>> +++ b/target/arm/translate-a64.c
>> @@ -5147,10 +5147,15 @@ static void handle_fp_fcvt(DisasContext *s, int 
>> opcode,
>>          } else {
>>              /* Single to half */
>>              TCGv_i32 tcg_rd = tcg_temp_new_i32();
>> -            gen_helper_vfp_fcvt_f32_to_f16(tcg_rd, tcg_rn, cpu_env);
>> +            TCGv_i32 ahp = get_ahp_flag();
>> +            TCGv_ptr fpst = get_fpstatus_ptr(true);
> 
> This should be passing in 'false' to get_fpstatus_ptr(), as should
> all the other uses where we get an fpst to pass to an f*_to_f16 or
> f16_to_f* conversion helper. This is because conversions between
> f16 and other sizes ignore FPCR.FZ16 for the 16-bit end, but they
> still honour FPCR.FZ for the other end. So you need to pass in the
> fpstatus pointer which has the flush-denormals status flags set
> to match FPCR.FZ, not the fp16-specific one.
> 
> Otherwise you'll generate the wrong exception flags for cases like
>  FPCR.FZ = 1; FPCR.FZ16 = 0; s0 = 0x00000001; fcvt h1, s0
> (which should give h1 = 0x0000 and set InputDenormal).

Good catch, thanks.


r~