[PATCH][RFC] target/hppa: Avoid nullification for uaddcmt instruction

[PATCH][RFC] target/hppa: Avoid nullification for uaddcmt instruction

[Helge Deller]

The uaddcmt (UNIT ADD COMPLEMENT AND TRAP ON CONDITION) instruction
triggers a trap if the condition is true, and stores the result of the
addition in the target register otherwise.
It does not use the condition to nullify the following instruction, so
drop the calculated condition and do not install it as null_cond.

This patch is not tested and as such sent as RFC.
I just stumbled over the apparently wrong behaviour while debugging the
uaddcm instruction.

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

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 46b2d6508d..6088e9bbf3 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -1363,7 +1363,11 @@ static void do_unit(DisasContext *ctx, unsigned rt, TCGv_i64 in1,
         save_gpr(ctx, rt, dest);
 
         cond_free(&ctx->null_cond);
-        ctx->null_cond = cond;
+        if (is_tc) {
+            cond_free(&cond);
+        } else {
+            ctx->null_cond = cond;
+        }
     }
 }
 

Re: [PATCH][RFC] target/hppa: Avoid nullification for uaddcmt instruction

[Richard Henderson]

On 3/23/24 11:15, Helge Deller wrote:
> The uaddcmt (UNIT ADD COMPLEMENT AND TRAP ON CONDITION) instruction
> triggers a trap if the condition is true, and stores the result of the
> addition in the target register otherwise.
> It does not use the condition to nullify the following instruction, so
> drop the calculated condition and do not install it as null_cond.

That's not what the manual says:

if (trapc == TC && cond_satisfied)
     conditional_trap;
else {
     GR[t] ← res;
     if (cond_satisfied) PSW[N] ← 1;
}

We have implemented this like so:

if (trapc == TC)
     conditional_trap(cond_satisfied);
GR[t] ← res;
if (cond_satisfied) PSW[N] ← 1;

because the conditional trap step does not return if it traps.


> This patch is not tested and as such sent as RFC.
> I just stumbled over the apparently wrong behaviour while debugging the
> uaddcm instruction.

Under separate cover you said the condition might be computed incorrectly.  I think that 
is more likely the root cause of the wrong behaviour.


r~

Re: [PATCH][RFC] target/hppa: Avoid nullification for uaddcmt instruction

[Helge Deller]

On 3/24/24 18:13, Richard Henderson wrote:
> On 3/23/24 11:15, Helge Deller wrote:
>> The uaddcmt (UNIT ADD COMPLEMENT AND TRAP ON CONDITION) instruction
>> triggers a trap if the condition is true, and stores the result of the
>> addition in the target register otherwise.
>> It does not use the condition to nullify the following instruction, so
>> drop the calculated condition and do not install it as null_cond.
>
> That's not what the manual says:
>
> if (trapc == TC && cond_satisfied)
>      conditional_trap;
> else {
>      GR[t] ← res;
>      if (cond_satisfied) PSW[N] ← 1;
> }

The above is from the 64-bit manual.
My mail was based on the info from the 32-bit manual, which says:
	res ← GR[r1] + ∼GR[r2];
	if (cond_satisfied)
		conditional_trap;
	else
		GR[t] ← res

But basically it doesn't matter, since as you explain below,
if it traps, it won't return anyway and as such PSW[N] will
not be modified.

> We have implemented this like so:
>
> if (trapc == TC)
>      conditional_trap(cond_satisfied);
> GR[t] ← res;
> if (cond_satisfied) PSW[N] ← 1;
>
> because the conditional trap step does not return if it traps.

Yes.
Thanks for review anyway!

Helge

>> This patch is not tested and as such sent as RFC.
>> I just stumbled over the apparently wrong behaviour while debugging the
>> uaddcm instruction.
>
> Under separate cover you said the condition might be computed incorrectly.  I think that is more likely the root cause of the wrong behaviour.
>
>
> r~
>