On Thu, Aug 27, 2026 at 7:13 PM Richard Henderson
<[email protected]> wrote:
>
> On 8/26/26 22:02, Matt Turner wrote:
> > tcg_gen_lookup_and_goto_ptr() takes no arguments and emits a call to
> > helper_lookup_tb_ptr(), which recovers the destination PC from env by
> > calling back into the target through TCGCPUOps::get_tb_cpu_state(). At
> > translation time the caller already has the destination PC in a temp, and
> > knows the flags, cflags and cs_base any destination it may reach has to
> > match, because they are the ones the block being generated was translated
> > with.
> >
> > Pass both, so that a later patch can use them to look the destination up
> > inline. Nothing reads them yet and the generated code does not change.
> >
> > The contract on @pc is the whole of the interface: it must hold exactly
> > what get_tb_cpu_state() reports as the pc for the destination block. Five
> > targets keep their PC in a temp whose value is that pc by construction and
> > so can pass it: alpha, loongarch, mips, ppc and s390x. Everything else
> > passes NULL and keeps today's behavior.
> >
> > For six of those the TB pc is derived and passing the PC temp would be
> > wrong: avr's TB pc is the word address doubled, i386's is eip before
> > segmentation, riscv masks it to 32 bits when xl is MXL_RV32, hppa derives
> > it from the IAQ, hexagon adjusts it inside a hardware loop, and sparc puts
> > npc in cs_base. The remaining seven -- arm, m68k, microblaze, or1k, rx, sh4
> > and tricore -- look like they could pass it, but I have not convinced
> > myself of the contract for them and have nothing to test them with. Each is
> > a one-line change for whoever wants it.
> >
> > The common entry point takes a TCGTemp rather than a TCGv and reads the
> > width from it, because the translators that are built for both values of
> > TARGET_LONG_BITS -- arm, s390x, microblaze -- cannot include tcg-op.h.
> > tcg-op.h wraps it for everyone else. This is the same split as
> > tcg_gen_qemu_ld_*_chk().
> >
> > v4: Split out of "tcg: probe the TB jump cache inline instead of calling a
> >      helper", which did the API change and the inline probe in one patch.
> >      Requested by Richard Henderson.
> >
> > Signed-off-by: Matt Turner<[email protected]>
> > ---
>
> Ok, I was a bit surprised at your claim that only 5 targets qualify, as
> there are plenty that have a simple PC.
>
> However! The subtlety of the interface, that we are asserting that the
> current TranslationBlock flags are still valid, now leads me to think
> that it's a mistake to adjust the current interface.
>
> We need to introduce a new interface to which targets may be migrated.
> This won't be difficult, but it's not entirely trivial.
>
> For instance, target/arm/ has
>
>          case DISAS_UPDATE_NOCHAIN:
>              gen_update_pc(dc, curr_insn_len(dc));
>              /* fall through */
>          case DISAS_JUMP:
>              gen_goto_ptr();
>              break;
>
> where DISAS_UPDATE_NOCHAIN requires the helper because of state change
> and DISAS_JUMP does not.
>
> This is subtle enough that we probably want to verify that the flags are
> unchanged with --enable-debug-tcg.

Agreed, and done.  tcg_gen_lookup_and_goto_ptr(void) is unchanged in
v5. Nothing that isn't migrated sees any difference.

> Perhaps tcg_gen_goto_jc_{i32,i64,tl)?

Sounds good. Done.

Replying to bits out of order:

> The simplest solution, IMO is to define functions for _i32 and _i64, as
> for most everything else, and to have a _tl alias in tcg-op.h.

Also done -- the _tmp()/TCGTemp form is gone.  tcg_gen_goto_jc_i32()
and tcg_gen_goto_jc_i64() are declared in tcg-op-common.h, and
tcg-op.h has the _tl alias in each TARGET_LONG_BITS arm alongside the
qemu_ld/st ones. The _i32 entry point zero-extends into a temp and
shares the body, so there is one implementation.

s390x is the one migrated target that cannot include tcg-op.h, so it
calls tcg_gen_goto_jc_i64(psw_addr) directly; that reads fine.

> the subtlety is asserting that the current TB flags are still valid ...
> target/arm: DISAS_UPDATE_NOCHAIN requires the helper because of the
> state change, DISAS_JUMP does not.

Right, and that distinction is exactly what the new interface makes
the caller state.  The contract is now written dow the goto_jc runs,
the CPU state must already be the destination's -- @pc must be what
get_tb_cpu_state() would report, it would report must be the ones this
block was translated with.  A translator that hasn't finished updating
state lookup key (avr's word address, i386's eip before segmentation),
keeps using tcg_gen_lookup_and_goto_ptr().

> we probably want to verify that the flags ar
> --enable-debug-tcg

Done.  Under CONFIG_DEBUG_TCG the goto_jc emits a call to a new
helper_goto_jc_check(env, pc, flags, cs_base), get_tb_cpu_state() and
asserts all three against what the block was translated with.  It
costs nothing in a normal contract being stated rather than assumed
while I migrated the targets.

Five targets are migrated in v5: alpha, loongarch, mips, ppc and s390x
-- the ones whose lookup_and_goto_ptr sites are u already the
destination's, and the pc register is the key".  arm, i386 and the
rest are untouched and can be migrated point of it being a separate
interface.

All five build and run their indirect-dispatch paths (computed goto,
function pointers, returns, and a longjmp out completion under
--enable-debug-tcg, so the assert has actually been exercised on each.

Reply via email to