[v2] target/hppa: Misc improvements

[PATCH v2 15/45] target/hppa: Use umax in do_ibranch_priv

Posted by Richard Henderson 1 year, 9 months ago

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/hppa/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index ae66068123..22935f4645 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -1981,7 +1981,7 @@ static TCGv_i64 do_ibranch_priv(DisasContext *ctx, TCGv_i64 offset)
         dest = tcg_temp_new_i64();
         tcg_gen_andi_i64(dest, offset, -4);
         tcg_gen_ori_i64(dest, dest, ctx->privilege);
-        tcg_gen_movcond_i64(TCG_COND_GTU, dest, dest, offset, dest, offset);
+        tcg_gen_umax_i64(dest, dest, offset);
         break;
     }
     return dest;
-- 
2.34.1

Re: [PATCH v2 15/45] target/hppa: Use umax in do_ibranch_priv

Posted by Philippe Mathieu-Daudé 1 year, 9 months ago

Hi Richard,

On 13/5/24 09:46, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/hppa/translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index ae66068123..22935f4645 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -1981,7 +1981,7 @@ static TCGv_i64 do_ibranch_priv(DisasContext *ctx, TCGv_i64 offset)
>           dest = tcg_temp_new_i64();
>           tcg_gen_andi_i64(dest, offset, -4);
>           tcg_gen_ori_i64(dest, dest, ctx->privilege);
> -        tcg_gen_movcond_i64(TCG_COND_GTU, dest, dest, offset, dest, offset);
> +        tcg_gen_umax_i64(dest, dest, offset);

Isn't tcg_gen_umax_i64(dest, dest, offset) equal to:

     tcg_gen_movcond_i64(TCG_COND_GEU, dest, dest, offset, dest, offset);

?

>           break;
>       }
>       return dest;

Re: [PATCH v2 15/45] target/hppa: Use umax in do_ibranch_priv

Posted by Richard Henderson 1 year, 9 months ago

On 5/13/24 13:18, Philippe Mathieu-Daudé wrote:
> Hi Richard,
> 
> On 13/5/24 09:46, Richard Henderson wrote:
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/hppa/translate.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
>> index ae66068123..22935f4645 100644
>> --- a/target/hppa/translate.c
>> +++ b/target/hppa/translate.c
>> @@ -1981,7 +1981,7 @@ static TCGv_i64 do_ibranch_priv(DisasContext *ctx, TCGv_i64 offset)
>>           dest = tcg_temp_new_i64();
>>           tcg_gen_andi_i64(dest, offset, -4);
>>           tcg_gen_ori_i64(dest, dest, ctx->privilege);
>> -        tcg_gen_movcond_i64(TCG_COND_GTU, dest, dest, offset, dest, offset);
>> +        tcg_gen_umax_i64(dest, dest, offset);
> 
> Isn't tcg_gen_umax_i64(dest, dest, offset) equal to:
> 
>      tcg_gen_movcond_i64(TCG_COND_GEU, dest, dest, offset, dest, offset);
> 
> ?

Yes, but I think it is clearer to use max.
At some point we might add min/max opcodes to tcg too.


r~

Re: [PATCH v2 15/45] target/hppa: Use umax in do_ibranch_priv

Posted by Philippe Mathieu-Daudé 1 year, 9 months ago

On 13/5/24 15:23, Richard Henderson wrote:
> On 5/13/24 13:18, Philippe Mathieu-Daudé wrote:
>> Hi Richard,
>>
>> On 13/5/24 09:46, Richard Henderson wrote:
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>   target/hppa/translate.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
>>> index ae66068123..22935f4645 100644
>>> --- a/target/hppa/translate.c
>>> +++ b/target/hppa/translate.c
>>> @@ -1981,7 +1981,7 @@ static TCGv_i64 do_ibranch_priv(DisasContext 
>>> *ctx, TCGv_i64 offset)
>>>           dest = tcg_temp_new_i64();
>>>           tcg_gen_andi_i64(dest, offset, -4);
>>>           tcg_gen_ori_i64(dest, dest, ctx->privilege);
>>> -        tcg_gen_movcond_i64(TCG_COND_GTU, dest, dest, offset, dest, 
>>> offset);
>>> +        tcg_gen_umax_i64(dest, dest, offset);
>>
>> Isn't tcg_gen_umax_i64(dest, dest, offset) equal to:
>>
>>      tcg_gen_movcond_i64(TCG_COND_GEU, dest, dest, offset, dest, offset);
>>
>> ?
> 
> Yes, but I think it is clearer to use max.

OK, maybe mention it in commit description to clear doubts?

> At some point we might add min/max opcodes to tcg too.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>