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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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);
       |     ~~~~~~~~~~~~~~~~~~                              ^~~~~~~~~