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 you need to use it like this: fpsw = FIELD_DP32(fpsw, ....); Would somebody like to write a patch ? (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