On 12/9/21 17:04, Peter Maydell wrote:
> On Tue, 17 Mar 2020 at 16:43, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
>>
>> From: Yoshinori Sato <ys...@users.sourceforge.jp>
>>
>> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
>> Tested-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>> Signed-off-by: Yoshinori Sato <ys...@users.sourceforge.jp>
>> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
>> [PMD: Removed tlb_fill, extracted from patch of Yoshinori Sato
>> 'Convert to CPUClass::tlb_fill']
>> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>> Message-Id: <20200224141923.82118-6-ys...@users.sourceforge.jp>
>> Acked-by: Richard Henderson <richard.hender...@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>
> Somewhat late, but I've just noticed a bug in the helper_set_fpsw()
> function introduced in this patch. The function has changed a little
> since but the bug is still there in the version in git:
>
>> +void helper_set_fpsw(CPURXState *env, uint32_t val)
>> +{
>> + static const int roundmode[] = {
>> + float_round_nearest_even,
>> + float_round_to_zero,
>> + float_round_up,
>> + float_round_down,
>> + };
>> + uint32_t fpsw = env->fpsw;
>> + fpsw |= 0x7fffff03;
>> + val &= ~0x80000000;
>> + fpsw &= val;
>> + FIELD_DP32(fpsw, FPSW, FS, FIELD_EX32(fpsw, FPSW, FLAGS) != 0);
>
> FIELD_DP32() does not update its first argument, it merely reads
> it. It returns the new value with the field change applied, so
I tried adding __attribute__((warn_unused_result)) to depositXX()
but it doesn't trigger, apparently due to how FIELD_DP32() macro
is written.
> you need to use it like this:
>
> fpsw = FIELD_DP32(fpsw, ....);
>
> Would somebody like to write a patch ?
Will do, thanks for the report.
> (I noticed this because I just made the same mistake in some new
> code I was writing, so I did a quick grep of the codebase to see
> if there were any instances of it already present. I think the macro
> magic used in the definitions of FIELD_DP* to provide a compile error
> if you pass a value that's bigger than the target field has the
> unfortunate side effect of suppressing the compiler warning that the
> whole statement has no effect.)
>
> thanks
> -- PMM
>
