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
