Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/hppa/translate.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 47f4b23d1b..d8973a63df 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3515,18 +3515,12 @@ static bool trans_bb_sar(DisasContext *ctx, arg_bb_sar *a)
 
 static bool trans_bb_imm(DisasContext *ctx, arg_bb_imm *a)
 {
-    TCGv_i64 tmp, tcg_r;
     DisasCond cond;
-    int p;
+    int p = a->p | (a->d ? 0 : 32);
 
     nullify_over(ctx);
-
-    tmp = tcg_temp_new_i64();
-    tcg_r = load_gpr(ctx, a->r);
-    p = a->p | (a->d ? 0 : 32);
-    tcg_gen_shli_i64(tmp, tcg_r, p);
-
-    cond = cond_make_ti(a->c ? TCG_COND_GE : TCG_COND_LT, tmp, 0);
+    cond = cond_make_vi(a->c ? TCG_COND_TSTEQ : TCG_COND_TSTNE,
+                        load_gpr(ctx, a->r), 1ull << (63 - p));
     return do_cbranch(ctx, a->disp, a->n, &cond);
 }
 
-- 
2.34.1

* Richard Henderson <richard.henderson@linaro.org>:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Helge Deller <deller@gmx.de>

> ---
>  target/hppa/translate.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 47f4b23d1b..d8973a63df 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -3515,18 +3515,12 @@ static bool trans_bb_sar(DisasContext *ctx, arg_bb_sar *a)
>  
>  static bool trans_bb_imm(DisasContext *ctx, arg_bb_imm *a)
>  {
> -    TCGv_i64 tmp, tcg_r;
>      DisasCond cond;
> -    int p;
> +    int p = a->p | (a->d ? 0 : 32);
>  
>      nullify_over(ctx);
> -
> -    tmp = tcg_temp_new_i64();
> -    tcg_r = load_gpr(ctx, a->r);
> -    p = a->p | (a->d ? 0 : 32);
> -    tcg_gen_shli_i64(tmp, tcg_r, p);
> -
> -    cond = cond_make_ti(a->c ? TCG_COND_GE : TCG_COND_LT, tmp, 0);
> +    cond = cond_make_vi(a->c ? TCG_COND_TSTEQ : TCG_COND_TSTNE,
> +                        load_gpr(ctx, a->r), 1ull << (63 - p));

I wonder if this actually fixes a bug...
Before it tested against all values >= tmp (even for which the bit
wasn't set), and now it correctly just checks the bit.


>      return do_cbranch(ctx, a->disp, a->n, &cond);
>  }
>  
> -- 
> 2.34.1
>

On 5/14/24 18:36, Helge Deller wrote:
>> -    tcg_gen_shli_i64(tmp, tcg_r, p);
>> -
>> -    cond = cond_make_ti(a->c ? TCG_COND_GE : TCG_COND_LT, tmp, 0);
>> +    cond = cond_make_vi(a->c ? TCG_COND_TSTEQ : TCG_COND_TSTNE,
>> +                        load_gpr(ctx, a->r), 1ull << (63 - p));
> 
> I wonder if this actually fixes a bug...
> Before it tested against all values >= tmp (even for which the bit
> wasn't set), and now it correctly just checks the bit.

No, the shli moved the bit under test up to the sign bit.
The comparison was always against 0.


r~

On 13/5/24 09:46, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/hppa/translate.c | 12 +++---------
>   1 file changed, 3 insertions(+), 9 deletions(-)

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