diff mbox series

target/loongarch: Fix "movfcsr2gr" gets wrong value

Message ID 20220806133426.542314-1-huqi@loongson.cn (mailing list archive)
State New, archived
Headers show
Series target/loongarch: Fix "movfcsr2gr" gets wrong value | expand

Commit Message

Qi Hu Aug. 6, 2022, 1:34 p.m. UTC 
In follow sequence:
  movgr2fcsr $r0,$r0
  movgr2fr.d $f0,$r0
  fdiv.d     $f0,$f0,$f0
  movfcsr2gr $r12,$r0

The value of $r12 is "0x00000000", but it should be "0x10100000"(V flag).

These fp helpers are definded with wrong flag "TCG_CALL_NO_WG" (but the
helpers write the globle "fcsr"). This causes the "fdiv" helper updates
the "fcsr" into mem, but "movfcsr2gr" does not load it from mem.

Reported-by: Feiyang Chen <chenfeiyang@loongson.cn>
Signed-off-by: Qi Hu <huqi@loongson.cn>
---
 target/loongarch/helper.h | 64 +++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

Comments

Richard Henderson Aug. 6, 2022, 4:15 p.m. UTC | #1 
On 8/6/22 06:34, Qi Hu wrote:
> In follow sequence:
>    movgr2fcsr $r0,$r0
>    movgr2fr.d $f0,$r0
>    fdiv.d     $f0,$f0,$f0
>    movfcsr2gr $r12,$r0
> 
> The value of $r12 is "0x00000000", but it should be "0x10100000"(V flag).
> 
> These fp helpers are definded with wrong flag "TCG_CALL_NO_WG" (but the
> helpers write the globle "fcsr"). This causes the "fdiv" helper updates
> the "fcsr" into mem, but "movfcsr2gr" does not load it from mem.
> 
> Reported-by: Feiyang Chen<chenfeiyang@loongson.cn>
> Signed-off-by: Qi Hu<huqi@loongson.cn>
> ---
>   target/loongarch/helper.h | 64 +++++++++++++++++++--------------------
>   1 file changed, 32 insertions(+), 32 deletions(-)

That's one solution, though it may perform less well.

The other is to remove cpu_fcsr0, and instead access that value with explicit loads and 
stores in the two (!) instructions that use that tcg temporary.


r~
Qi Hu Aug. 7, 2022, 3:22 a.m. UTC | #2 
> On Aug 7, 2022, at 00:18, Richard Henderson <richard.henderson@linaro.org> wrote:
> 
> On 8/6/22 06:34, Qi Hu wrote:
>> In follow sequence:
>>   movgr2fcsr $r0,$r0
>>   movgr2fr.d $f0,$r0
>>   fdiv.d     $f0,$f0,$f0
>>   movfcsr2gr $r12,$r0
>> The value of $r12 is "0x00000000", but it should be "0x10100000"(V flag).
>> These fp helpers are definded with wrong flag "TCG_CALL_NO_WG" (but the
>> helpers write the globle "fcsr"). This causes the "fdiv" helper updates
>> the "fcsr" into mem, but "movfcsr2gr" does not load it from mem.
>> Reported-by: Feiyang Chen<chenfeiyang@loongson.cn>
>> Signed-off-by: Qi Hu<huqi@loongson.cn>
>> ---
>>  target/loongarch/helper.h | 64 +++++++++++++++++++--------------------
>>  1 file changed, 32 insertions(+), 32 deletions(-)
> 
> That's one solution, though it may perform less well.
> 
> The other is to remove cpu_fcsr0, and instead access that value with explicit loads and stores in the two (!) instructions that use that tcg temporary.
> 
This solution is better. Thanks for your advice. :)

Qi
> 
> r~
diff mbox series

Patch

diff --git a/target/loongarch/helper.h b/target/loongarch/helper.h
index cbbe008f32..8bd6b36f53 100644
--- a/target/loongarch/helper.h
+++ b/target/loongarch/helper.h
@@ -17,38 +17,38 @@  DEF_HELPER_FLAGS_3(crc32c, TCG_CALL_NO_RWG_SE, tl, tl, tl, tl)
 DEF_HELPER_FLAGS_2(cpucfg, TCG_CALL_NO_RWG_SE, tl, env, tl)
 
 /* Floating-point helper */
-DEF_HELPER_FLAGS_3(fadd_s, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fadd_d, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fsub_s, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fsub_d, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fmul_s, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fmul_d, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fdiv_s, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fdiv_d, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fmax_s, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fmax_d, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fmin_s, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fmin_d, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fmaxa_s, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fmaxa_d, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fmina_s, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fmina_d, TCG_CALL_NO_WG, i64, env, i64, i64)
-
-DEF_HELPER_FLAGS_5(fmuladd_s, TCG_CALL_NO_WG, i64, env, i64, i64, i64, i32)
-DEF_HELPER_FLAGS_5(fmuladd_d, TCG_CALL_NO_WG, i64, env, i64, i64, i64, i32)
-
-DEF_HELPER_FLAGS_3(fscaleb_s, TCG_CALL_NO_WG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(fscaleb_d, TCG_CALL_NO_WG, i64, env, i64, i64)
-
-DEF_HELPER_FLAGS_2(flogb_s, TCG_CALL_NO_WG, i64, env, i64)
-DEF_HELPER_FLAGS_2(flogb_d, TCG_CALL_NO_WG, i64, env, i64)
-
-DEF_HELPER_FLAGS_2(fsqrt_s, TCG_CALL_NO_WG, i64, env, i64)
-DEF_HELPER_FLAGS_2(fsqrt_d, TCG_CALL_NO_WG, i64, env, i64)
-DEF_HELPER_FLAGS_2(frsqrt_s, TCG_CALL_NO_WG, i64, env, i64)
-DEF_HELPER_FLAGS_2(frsqrt_d, TCG_CALL_NO_WG, i64, env, i64)
-DEF_HELPER_FLAGS_2(frecip_s, TCG_CALL_NO_WG, i64, env, i64)
-DEF_HELPER_FLAGS_2(frecip_d, TCG_CALL_NO_WG, i64, env, i64)
+DEF_HELPER_3(fadd_s, i64, env, i64, i64)
+DEF_HELPER_3(fadd_d, i64, env, i64, i64)
+DEF_HELPER_3(fsub_s, i64, env, i64, i64)
+DEF_HELPER_3(fsub_d, i64, env, i64, i64)
+DEF_HELPER_3(fmul_s, i64, env, i64, i64)
+DEF_HELPER_3(fmul_d, i64, env, i64, i64)
+DEF_HELPER_3(fdiv_s, i64, env, i64, i64)
+DEF_HELPER_3(fdiv_d, i64, env, i64, i64)
+DEF_HELPER_3(fmax_s, i64, env, i64, i64)
+DEF_HELPER_3(fmax_d, i64, env, i64, i64)
+DEF_HELPER_3(fmin_s, i64, env, i64, i64)
+DEF_HELPER_3(fmin_d, i64, env, i64, i64)
+DEF_HELPER_3(fmaxa_s, i64, env, i64, i64)
+DEF_HELPER_3(fmaxa_d, i64, env, i64, i64)
+DEF_HELPER_3(fmina_s, i64, env, i64, i64)
+DEF_HELPER_3(fmina_d, i64, env, i64, i64)
+
+DEF_HELPER_5(fmuladd_s, i64, env, i64, i64, i64, i32)
+DEF_HELPER_5(fmuladd_d, i64, env, i64, i64, i64, i32)
+
+DEF_HELPER_3(fscaleb_s, i64, env, i64, i64)
+DEF_HELPER_3(fscaleb_d, i64, env, i64, i64)
+
+DEF_HELPER_2(flogb_s, i64, env, i64)
+DEF_HELPER_2(flogb_d, i64, env, i64)
+
+DEF_HELPER_2(fsqrt_s, i64, env, i64)
+DEF_HELPER_2(fsqrt_d, i64, env, i64)
+DEF_HELPER_2(frsqrt_s, i64, env, i64)
+DEF_HELPER_2(frsqrt_d, i64, env, i64)
+DEF_HELPER_2(frecip_s, i64, env, i64)
+DEF_HELPER_2(frecip_d, i64, env, i64)
 
 DEF_HELPER_FLAGS_2(fclass_s, TCG_CALL_NO_RWG_SE, i64, env, i64)
 DEF_HELPER_FLAGS_2(fclass_d, TCG_CALL_NO_RWG_SE, i64, env, i64)