Re: [Qemu-devel] [PATCH v11 23/29] target/arm: [tcg] Port to translate_insn

[Lluís Vilanova]

Richard Henderson writes:

> On 07/07/2017 07:18 AM, Lluís Vilanova wrote:
>> There was no code being generated after this specific case, but I haven't
>> checked if DISAS_EXC is set in any other place that is not immediately 
>> followed
>> by a "goto done_generating".

> Typically we haven't actually done a goto, but simply exit the loop and emit
> nothing within the final cleanup (tb_stop?).

The case handled by DISAS_SKIP ignores tb_stop() (the target code can simply
return when DISAS_EXC is found instead of DISAS_SKIP) *and* gen_io_end(); this
last one is never omitted when DISAS_EXC is found now, and theoretically
DISAS_EXC can be set by any target-specific hook. Thus my question if the
generic call to gen_io_end() should check for DISAS_EXC too (I have no idea if
it would be an error to call it with DISAS_EXC set, or whether it makes sense to
for a target to set it so that gen_io_start() is called but gen_io_end() is then
skipped by a DISAS_EXC set in ops->translate_insn()).


>> Does this mean DISAS_EXC should be on the generic code and do a "goto
>> done_generating" whenever it is found? And if so, what are the correct 
>> places to
>> check for this? After ops->insn_start, ops->translate_insn, ops->tb_stop?

> Yes, this should be handled generically, since all targets need it.

> That said, I would prefer a better name like DISAS_NORETURN, which does not
> imply that an actual exception has been raised, but explicitly says that all
> following code is dead.

I can use that name.

And in fact, generalizing DISAS_NORETURN will allow dropping the enum result of
breakpoint_check(), and instead simply return a bool (whether a breakpoint did
hit). Targets can then set DISAS_NORETURN and return true instead of returning
BC_HIT_TB (simply returning true is equivalent to the previous BC_HIT_INSN).


Cheers,
  Lluis