[PATCH 05/33] target/mips: Have check_msa_access() return a boolean

Philippe Mathieu-Daudé posted 33 patches 4 years, 1 month ago
Maintainers: Aurelien Jarno <aurelien@aurel32.net>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>
There is a newer version of this series
[PATCH 05/33] target/mips: Have check_msa_access() return a boolean
Posted by Philippe Mathieu-Daudé 4 years, 1 month ago
Have check_msa_access() return a boolean value so we can
return early if MSA is not enabled.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/tcg/msa_translate.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
index 3ef912da6b8..9e0a08fe335 100644
--- a/target/mips/tcg/msa_translate.c
+++ b/target/mips/tcg/msa_translate.c
@@ -293,19 +293,19 @@ void msa_translate_init(void)
     }
 }
 
-static inline int check_msa_access(DisasContext *ctx)
+static inline bool check_msa_access(DisasContext *ctx)
 {
     if (unlikely((ctx->hflags & MIPS_HFLAG_FPU) &&
                  !(ctx->hflags & MIPS_HFLAG_F64))) {
         gen_reserved_instruction(ctx);
-        return 0;
+        return false;
     }
 
     if (unlikely(!(ctx->hflags & MIPS_HFLAG_MSA))) {
         generate_exception_end(ctx, EXCP_MSADIS);
-        return 0;
+        return false;
     }
-    return 1;
+    return true;
 }
 
 static void gen_check_zero_element(TCGv tresult, uint8_t df, uint8_t wt,
@@ -354,7 +354,9 @@ static bool gen_msa_BxZ_V(DisasContext *ctx, int wt, int s16, TCGCond cond)
 {
     TCGv_i64 t0;
 
-    check_msa_access(ctx);
+    if (!check_msa_access(ctx)) {
+        return false;
+    }
 
     if (ctx->hflags & MIPS_HFLAG_BMASK) {
         gen_reserved_instruction(ctx);
@@ -386,7 +388,9 @@ static bool trans_BNZ_V(DisasContext *ctx, arg_msa_bz *a)
 
 static bool gen_msa_BxZ(DisasContext *ctx, int df, int wt, int s16, bool if_not)
 {
-    check_msa_access(ctx);
+    if (!check_msa_access(ctx)) {
+        return false;
+    }
 
     if (ctx->hflags & MIPS_HFLAG_BMASK) {
         gen_reserved_instruction(ctx);
@@ -2158,7 +2162,9 @@ static bool trans_MSA(DisasContext *ctx, arg_MSA *a)
 {
     uint32_t opcode = ctx->opcode;
 
-    check_msa_access(ctx);
+    if (!check_msa_access(ctx)) {
+        return false;
+    }
 
     switch (MASK_MSA_MINOR(opcode)) {
     case OPC_MSA_I8_00:
-- 
2.31.1

Re: [PATCH 05/33] target/mips: Have check_msa_access() return a boolean
Posted by Richard Henderson 4 years, 1 month ago
On 10/23/21 2:47 PM, Philippe Mathieu-Daudé wrote:
> Have check_msa_access() return a boolean value so we can
> return early if MSA is not enabled.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   target/mips/tcg/msa_translate.c | 20 +++++++++++++-------
>   1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
> index 3ef912da6b8..9e0a08fe335 100644
> --- a/target/mips/tcg/msa_translate.c
> +++ b/target/mips/tcg/msa_translate.c
> @@ -293,19 +293,19 @@ void msa_translate_init(void)
>       }
>   }
>   
> -static inline int check_msa_access(DisasContext *ctx)
> +static inline bool check_msa_access(DisasContext *ctx)
>   {
>       if (unlikely((ctx->hflags & MIPS_HFLAG_FPU) &&
>                    !(ctx->hflags & MIPS_HFLAG_F64))) {
>           gen_reserved_instruction(ctx);
> -        return 0;
> +        return false;
>       }
>   
>       if (unlikely(!(ctx->hflags & MIPS_HFLAG_MSA))) {
>           generate_exception_end(ctx, EXCP_MSADIS);
> -        return 0;
> +        return false;
>       }

When we return false, we have raised an exception.

> @@ -354,7 +354,9 @@ static bool gen_msa_BxZ_V(DisasContext *ctx, int wt, int s16, TCGCond cond)
>   {
>       TCGv_i64 t0;
>   
> -    check_msa_access(ctx);
> +    if (!check_msa_access(ctx)) {
> +        return false;
> +    }

... which means that here we should return true, meaning that we have recognized the 
instruction and emitted some code for it.  In this case: we have recognized that the 
instruction is valid but not enabled.

Otherwise, we will return to decode_opc and (eventually) emit another 
gen_reserved_instruction.


r~

Re: [PATCH 05/33] target/mips: Have check_msa_access() return a boolean
Posted by Philippe Mathieu-Daudé 4 years, 1 month ago
On 10/24/21 03:02, Richard Henderson wrote:
> On 10/23/21 2:47 PM, Philippe Mathieu-Daudé wrote:
>> Have check_msa_access() return a boolean value so we can
>> return early if MSA is not enabled.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   target/mips/tcg/msa_translate.c | 20 +++++++++++++-------
>>   1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/target/mips/tcg/msa_translate.c
>> b/target/mips/tcg/msa_translate.c
>> index 3ef912da6b8..9e0a08fe335 100644
>> --- a/target/mips/tcg/msa_translate.c
>> +++ b/target/mips/tcg/msa_translate.c
>> @@ -293,19 +293,19 @@ void msa_translate_init(void)
>>       }
>>   }
>>   -static inline int check_msa_access(DisasContext *ctx)
>> +static inline bool check_msa_access(DisasContext *ctx)
>>   {
>>       if (unlikely((ctx->hflags & MIPS_HFLAG_FPU) &&
>>                    !(ctx->hflags & MIPS_HFLAG_F64))) {
>>           gen_reserved_instruction(ctx);
>> -        return 0;
>> +        return false;
>>       }
>>         if (unlikely(!(ctx->hflags & MIPS_HFLAG_MSA))) {
>>           generate_exception_end(ctx, EXCP_MSADIS);
>> -        return 0;
>> +        return false;
>>       }
> 
> When we return false, we have raised an exception.
> 
>> @@ -354,7 +354,9 @@ static bool gen_msa_BxZ_V(DisasContext *ctx, int
>> wt, int s16, TCGCond cond)
>>   {
>>       TCGv_i64 t0;
>>   -    check_msa_access(ctx);
>> +    if (!check_msa_access(ctx)) {
>> +        return false;
>> +    }
> 
> ... which means that here we should return true, meaning that we have
> recognized the instruction and emitted some code for it.  In this case:
> we have recognized that the instruction is valid but not enabled.
> 
> Otherwise, we will return to decode_opc and (eventually) emit another
> gen_reserved_instruction.

Yes, this is what I intended to do. I incorrectly copied/pasted 'false'
then it spread all over the file. Thanks for catching this.