Re: [PATCH 5/7] target/hppa: copy new_spc to iasq_f on be,n instruction

[Sven Schnelle]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 3/17/24 12:14, Sven Schnelle wrote:
>> Otherwise the first instruction at the new location gets executed from
>> the old space.
>> Signed-off-by: Sven Schnelle <svens@stackframe.org>
>> ---
>>   target/hppa/translate.c | 3 +++
>>   1 file changed, 3 insertions(+)
>> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
>> index 58d7ec1ade..a09112e4ae 100644
>> --- a/target/hppa/translate.c
>> +++ b/target/hppa/translate.c
>> @@ -3777,6 +3777,9 @@ static bool trans_be(DisasContext *ctx, arg_be *a)
>>           }
>>           copy_iaoq_entry(ctx, cpu_iaoq_b, -1, tmp);
>>           tcg_gen_mov_i64(cpu_iasq_b, new_spc);
>> +        if (a->n) {
>> +            tcg_gen_mov_i64(cpu_iasq_f, new_spc);
>> +        }
>>           nullify_set(ctx, a->n);
>>       }
>>       tcg_gen_lookup_and_goto_ptr();
>
>
> Without use_nullify_skip(), we're going to execute the next
> instruction even if we know it is nullified (a->n).  This is usually
> because there's a page crossing or breakpoint, and we need to take the
> exception that might be raised there.
>
> So, we advance the queue:
>
>         copy_iaoq_entry(ctx, cpu_iaoq_f, ctx->iaoq_b, cpu_iaoq_b);
>         if (ctx->iaoq_b == -1) {
>             tcg_gen_mov_i64(cpu_iasq_f, cpu_iasq_b);
>         }
>
> then put the branch destination at the back of the queue:
>
>         copy_iaoq_entry(ctx, cpu_iaoq_b, -1, tmp);
>         tcg_gen_mov_i64(cpu_iasq_b, new_spc);
>
> Note that iaoq_b is always -1 on a space change.
>
> So your change does not look correct.
> What is the issue that you saw?

I was running the CPU instruction tests from HP Offline Diagnostics, and
some be instruction was ending up in the wrong space. I don't have the
details anymore. I'd say we drop the patch for now and I test it again
later and provide more details.