1. Patchew
  2. QEMU
  3. View series

[PATCH] target/mips: Remove unreachable 32-bit code on 64-bit Loongson Ext

Philippe Mathieu-Daudé posted 1 patch 3 weeks, 6 days ago 
target/mips/tcg/loong_translate.c | 43 +++----------------------------
target/mips/tcg/translate.c       |  2 +-
target/mips/tcg/meson.build       |  2 +-
3 files changed, 6 insertions(+), 41 deletions(-)
[PATCH] target/mips: Remove unreachable 32-bit code on 64-bit Loongson Ext
Posted by Philippe Mathieu-Daudé 3 weeks, 6 days ago 
Loongson fixed-point multiplies and divisions opcodes are
specific to 64-bit cores (Loongson-2 and Loongson-3 families).
Simplify by removing the 32-bit checks.

Reported-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Based-on: <20230831203024.87300-1-philmd@linaro.org>
---
 target/mips/tcg/loong_translate.c | 43 +++----------------------------
 target/mips/tcg/translate.c       |  2 +-
 target/mips/tcg/meson.build       |  2 +-
 3 files changed, 6 insertions(+), 41 deletions(-)

diff --git a/target/mips/tcg/loong_translate.c b/target/mips/tcg/loong_translate.c
index c896e64b9e6..91711b8e052 100644
--- a/target/mips/tcg/loong_translate.c
+++ b/target/mips/tcg/loong_translate.c
@@ -31,13 +31,6 @@ static bool gen_lext_DIV_G(DisasContext *s, int rd, int rs, int rt,
     TCGv t0, t1;
     TCGLabel *l1, *l2, *l3;
 
-    if (is_double) {
-        if (TARGET_LONG_BITS != 64) {
-            return false;
-        }
-        check_mips_64(s);
-    }
-
     if (rd == 0) {
         /* Treat as NOP. */
         return true;
@@ -61,8 +54,7 @@ static bool gen_lext_DIV_G(DisasContext *s, int rd, int rs, int rt,
     tcg_gen_br(l3);
     gen_set_label(l1);
 
-    tcg_gen_brcondi_tl(TCG_COND_NE, t0, is_double && TARGET_LONG_BITS == 64
-                                        ? LLONG_MIN : INT_MIN, l2);
+    tcg_gen_brcondi_tl(TCG_COND_NE, t0, is_double ? LLONG_MIN : INT_MIN, l2);
     tcg_gen_brcondi_tl(TCG_COND_NE, t1, -1LL, l2);
     tcg_gen_mov_tl(cpu_gpr[rd], t0);
 
@@ -93,13 +85,6 @@ static bool gen_lext_DIVU_G(DisasContext *s, int rd, int rs, int rt,
     TCGv t0, t1;
     TCGLabel *l1, *l2;
 
-    if (is_double) {
-        if (TARGET_LONG_BITS != 64) {
-            return false;
-        }
-        check_mips_64(s);
-    }
-
     if (rd == 0) {
         /* Treat as NOP. */
         return true;
@@ -147,13 +132,6 @@ static bool gen_lext_MOD_G(DisasContext *s, int rd, int rs, int rt,
     TCGv t0, t1;
     TCGLabel *l1, *l2, *l3;
 
-    if (is_double) {
-        if (TARGET_LONG_BITS != 64) {
-            return false;
-        }
-        check_mips_64(s);
-    }
-
     if (rd == 0) {
         /* Treat as NOP. */
         return true;
@@ -173,8 +151,7 @@ static bool gen_lext_MOD_G(DisasContext *s, int rd, int rs, int rt,
         tcg_gen_ext32u_tl(t1, t1);
     }
     tcg_gen_brcondi_tl(TCG_COND_EQ, t1, 0, l1);
-    tcg_gen_brcondi_tl(TCG_COND_NE, t0, is_double && TARGET_LONG_BITS == 64
-                                        ? LLONG_MIN : INT_MIN, l2);
+    tcg_gen_brcondi_tl(TCG_COND_NE, t0, is_double ? LLONG_MIN : INT_MIN, l2);
     tcg_gen_brcondi_tl(TCG_COND_NE, t1, -1LL, l2);
     gen_set_label(l1);
     tcg_gen_movi_tl(cpu_gpr[rd], 0);
@@ -205,13 +182,6 @@ static bool gen_lext_MODU_G(DisasContext *s, int rd, int rs, int rt,
     TCGv t0, t1;
     TCGLabel *l1, *l2;
 
-    if (is_double) {
-        if (TARGET_LONG_BITS != 64) {
-            return false;
-        }
-        check_mips_64(s);
-    }
-
     if (rd == 0) {
         /* Treat as NOP. */
         return true;
@@ -257,13 +227,6 @@ static bool gen_lext_MULT_G(DisasContext *s, int rd, int rs, int rt,
 {
     TCGv t0, t1;
 
-    if (is_double) {
-        if (TARGET_LONG_BITS != 64) {
-            return false;
-        }
-        check_mips_64(s);
-    }
-
     if (rd == 0) {
         /* Treat as NOP. */
         return true;
@@ -295,6 +258,8 @@ static bool trans_DMULTu_G(DisasContext *s, arg_muldiv *a)
 
 bool decode_ext_loongson(DisasContext *ctx, uint32_t insn)
 {
+    assert(ctx->hflags & MIPS_HFLAG_64);
+
     if ((ctx->insn_flags & INSN_LOONGSON2E)
             && decode_godson2(ctx, ctx->opcode)) {
         return true;
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 9839575247e..68a5c21bb2d 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -15020,7 +15020,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
     if (cpu_supports_isa(env, INSN_VR54XX) && decode_ext_vr54xx(ctx, ctx->opcode)) {
         return;
     }
-    if (decode_ext_loongson(ctx, ctx->opcode)) {
+    if (TARGET_LONG_BITS == 64 && decode_ext_loongson(ctx, ctx->opcode)) {
         return;
     }
 #if defined(TARGET_MIPS64)
diff --git a/target/mips/tcg/meson.build b/target/mips/tcg/meson.build
index fbb6d6eb407..fd91148df74 100644
--- a/target/mips/tcg/meson.build
+++ b/target/mips/tcg/meson.build
@@ -16,7 +16,6 @@ mips_ss.add(files(
   'fpu_helper.c',
   'ldst_helper.c',
   'lmmi_helper.c',
-  'loong_translate.c',
   'msa_helper.c',
   'msa_translate.c',
   'op_helper.c',
@@ -31,6 +30,7 @@ mips_ss.add(when: 'TARGET_MIPS64', if_true: files(
   'tx79_translate.c',
   'octeon_translate.c',
   'lcsr_translate.c',
+  'loong_translate.c',
 ), if_false: files(
   'mxu_translate.c',
 ))
-- 
2.45.2
Re: [PATCH] target/mips: Remove unreachable 32-bit code on 64-bit Loongson Ext
Posted by Philippe Mathieu-Daudé 3 weeks, 6 days ago 
On 26/10/24 12:45, Philippe Mathieu-Daudé wrote:
> Loongson fixed-point multiplies and divisions opcodes are
> specific to 64-bit cores (Loongson-2 and Loongson-3 families).
> Simplify by removing the 32-bit checks.
> 
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Based-on: <20230831203024.87300-1-philmd@linaro.org>
> ---
>   target/mips/tcg/loong_translate.c | 43 +++----------------------------
>   target/mips/tcg/translate.c       |  2 +-
>   target/mips/tcg/meson.build       |  2 +-
>   3 files changed, 6 insertions(+), 41 deletions(-)


> @@ -295,6 +258,8 @@ static bool trans_DMULTu_G(DisasContext *s, arg_muldiv *a)
>   
>   bool decode_ext_loongson(DisasContext *ctx, uint32_t insn)
>   {
> +    assert(ctx->hflags & MIPS_HFLAG_64);
> +
>       if ((ctx->insn_flags & INSN_LOONGSON2E)
>               && decode_godson2(ctx, ctx->opcode)) {
>           return true;
> diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
> index 9839575247e..68a5c21bb2d 100644
> --- a/target/mips/tcg/translate.c
> +++ b/target/mips/tcg/translate.c
> @@ -15020,7 +15020,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
>       if (cpu_supports_isa(env, INSN_VR54XX) && decode_ext_vr54xx(ctx, ctx->opcode)) {
>           return;
>       }
> -    if (decode_ext_loongson(ctx, ctx->opcode)) {
> +    if (TARGET_LONG_BITS == 64 && decode_ext_loongson(ctx, ctx->opcode)) {
>           return;
>       }

Thinking of single binary, better is to extract decode_running_64bit()
and check as runtime; IOW squash:

-- >8 --
diff --git a/target/mips/tcg/translate.h b/target/mips/tcg/translate.h
index 36f3396477a..aa70f27fbba 100644
--- a/target/mips/tcg/translate.h
+++ b/target/mips/tcg/translate.h
@@ -219,0 +220,2 @@ bool decode_ase_mxu(DisasContext *ctx, uint32_t insn);
+bool decode_running_64bit(DisasContext *ctx);
+
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 68a5c21bb2d..67480106226 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -1617,0 +1618,5 @@ static inline void check_ps(DisasContext *ctx)
+bool decode_running_64bit(DisasContext *ctx)
+{
+    return ctx->hflags & MIPS_HFLAG_64;
+}
+
@@ -1624 +1629 @@ void check_mips_64(DisasContext *ctx)
-    if (unlikely((TARGET_LONG_BITS != 64) || !(ctx->hflags & 
MIPS_HFLAG_64))) {
+    if (unlikely((TARGET_LONG_BITS != 64) || !decode_running_64bit(ctx))) {
@@ -15023 +15028 @@ static void decode_opc(CPUMIPSState *env, 
DisasContext *ctx)
-    if (TARGET_LONG_BITS == 64 && decode_ext_loongson(ctx, ctx->opcode)) {
+    if (decode_running_64bit(ctx) && decode_ext_loongson(ctx, 
ctx->opcode)) {
diff --git a/target/mips/tcg/meson.build b/target/mips/tcg/meson.build
index fd91148df74..fbb6d6eb407 100644
--- a/target/mips/tcg/meson.build
+++ b/target/mips/tcg/meson.build
@@ -18,0 +19 @@ mips_ss.add(files(
+  'loong_translate.c',
@@ -33 +33,0 @@ mips_ss.add(when: 'TARGET_MIPS64', if_true: files(
-  'loong_translate.c',
---
Re: [PATCH] target/mips: Remove unreachable 32-bit code on 64-bit Loongson Ext
Posted by Philippe Mathieu-Daudé 3 weeks, 6 days ago 
On 26/10/24 14:20, Philippe Mathieu-Daudé wrote:
> On 26/10/24 12:45, Philippe Mathieu-Daudé wrote:
>> Loongson fixed-point multiplies and divisions opcodes are
>> specific to 64-bit cores (Loongson-2 and Loongson-3 families).
>> Simplify by removing the 32-bit checks.
>>
>> Reported-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> Based-on: <20230831203024.87300-1-philmd@linaro.org>
>> ---
>>   target/mips/tcg/loong_translate.c | 43 +++----------------------------
>>   target/mips/tcg/translate.c       |  2 +-
>>   target/mips/tcg/meson.build       |  2 +-
>>   3 files changed, 6 insertions(+), 41 deletions(-)
> 
> 
>> @@ -295,6 +258,8 @@ static bool trans_DMULTu_G(DisasContext *s, 
>> arg_muldiv *a)
>>   bool decode_ext_loongson(DisasContext *ctx, uint32_t insn)
>>   {
>> +    assert(ctx->hflags & MIPS_HFLAG_64);
>> +
>>       if ((ctx->insn_flags & INSN_LOONGSON2E)
>>               && decode_godson2(ctx, ctx->opcode)) {
>>           return true;
>> diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
>> index 9839575247e..68a5c21bb2d 100644
>> --- a/target/mips/tcg/translate.c
>> +++ b/target/mips/tcg/translate.c
>> @@ -15020,7 +15020,7 @@ static void decode_opc(CPUMIPSState *env, 
>> DisasContext *ctx)
>>       if (cpu_supports_isa(env, INSN_VR54XX) && decode_ext_vr54xx(ctx, 
>> ctx->opcode)) {
>>           return;
>>       }
>> -    if (decode_ext_loongson(ctx, ctx->opcode)) {
>> +    if (TARGET_LONG_BITS == 64 && decode_ext_loongson(ctx, 
>> ctx->opcode)) {
>>           return;
>>       }
> 
> Thinking of single binary, better is to extract decode_running_64bit()
> and check as runtime; IOW squash:
> 
> -- >8 --
> diff --git a/target/mips/tcg/translate.h b/target/mips/tcg/translate.h
> index 36f3396477a..aa70f27fbba 100644
> --- a/target/mips/tcg/translate.h
> +++ b/target/mips/tcg/translate.h
> @@ -219,0 +220,2 @@ bool decode_ase_mxu(DisasContext *ctx, uint32_t insn);
> +bool decode_running_64bit(DisasContext *ctx);
> +
> diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
> index 68a5c21bb2d..67480106226 100644
> --- a/target/mips/tcg/translate.c
> +++ b/target/mips/tcg/translate.c
> @@ -1617,0 +1618,5 @@ static inline void check_ps(DisasContext *ctx)
> +bool decode_running_64bit(DisasContext *ctx)
> +{
> +    return ctx->hflags & MIPS_HFLAG_64;
> +}
> +
> @@ -1624 +1629 @@ void check_mips_64(DisasContext *ctx)
> -    if (unlikely((TARGET_LONG_BITS != 64) || !(ctx->hflags & 
> MIPS_HFLAG_64))) {
> +    if (unlikely((TARGET_LONG_BITS != 64) || 
> !decode_running_64bit(ctx))) {
> @@ -15023 +15028 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
> -    if (TARGET_LONG_BITS == 64 && decode_ext_loongson(ctx, ctx->opcode)) {
> +    if (decode_running_64bit(ctx) && decode_ext_loongson(ctx, 
> ctx->opcode)) {
> diff --git a/target/mips/tcg/meson.build b/target/mips/tcg/meson.build
> index fd91148df74..fbb6d6eb407 100644
> --- a/target/mips/tcg/meson.build
> +++ b/target/mips/tcg/meson.build
> @@ -18,0 +19 @@ mips_ss.add(files(
> +  'loong_translate.c',
> @@ -33 +33,0 @@ mips_ss.add(when: 'TARGET_MIPS64', if_true: files(
> -  'loong_translate.c',
> ---

Eh not that fast... Still work to do before getting there:

target/mips/tcg/loong_translate.c:57:53: warning: implicit conversion 
from 'long long' to 'int32_t' (aka 'int') changes value from 
-9223372036854775808 to 0 [-Wconstant-conversion]
    57 |     tcg_gen_brcondi_tl(TCG_COND_NE, t0, is_double ? LLONG_MIN : 
INT_MIN, l2);
       |     ~~~~~~~~~~~~~~~~~~                              ^~~~~~~~~

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.