| Message ID | 20240317221431.251515-6-svens@stackframe.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | few fixes for hppa target | expand |
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? r~
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.
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();
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(+)