Re: [Qemu-devel] [PATCH v9 04/26] target: [tcg] Add generic translation framework

["Lluís Vilanova" <vilanova@ac.upc.edu>]

Emilio G Cota writes:

> On Sun, Jun 25, 2017 at 11:59:54 +0300, Lluís Vilanova wrote:
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> ---
>> Makefile.target                |    1 
>> include/exec/gen-icount.h      |    2 
>> include/exec/translate-block.h |  125 +++++++++++++++++++++++++++
>> include/qom/cpu.h              |   22 +++++
>> translate-block.c              |  185 ++++++++++++++++++++++++++++++++++++++++
>> 5 files changed, 334 insertions(+), 1 deletion(-)
>> create mode 100644 include/exec/translate-block.h
>> create mode 100644 translate-block.c
> (snip)
>> diff --git a/include/exec/translate-block.h b/include/exec/translate-block.h
>> new file mode 100644
>> index 0000000000..d14d23f2cb
>> --- /dev/null
>> +++ b/include/exec/translate-block.h
> (snip)
>> +/**
>> + * DisasJumpType:
>> + * @DJ_NEXT: Next instruction in program order.
>> + * @DJ_TOO_MANY: Too many instructions translated.
>> + * @DJ_TARGET: Start of target-specific conditions.
>> + *
>> + * What instruction to disassemble next.
>> + */
>> +typedef enum DisasJumpType {
>> +    DJ_NEXT,
>> +    DJ_TOO_MANY,
>> +    DJ_TARGET,
>> +} DisasJumpType;

> I'd give up on the enum to avoid unnecessary casts. Just define DJ_TARGET
> (or rather, DISAS_TARGET :>) and let the architecture code add more define's
> using it.

I'm all for restoring the original name (haven't checked if it will produce any
redefine errors).

But using an enum makes the API more explicit about the intended values. Still,
if the churn of casting outweighs the API clarity, I can revert this.

Another option previously suggested on the list is defining DISAS_TARGET_[0..N]
on the enum, and letting targets simply define their own name when mapped to
those. I'll try that one before completely dropping the enum. That is, unless
someone is strongly for going back to defines.


Cheers,
  Lluis