Pierrick Bouvier <pierrick.bouv...@linaro.org> writes:
[...]
> At this point, I would like to focus on having a first version of TargetInfo
> API, and not reviewing any other changes, as things may be modified, and they
> would need to be reviewed again. It's hard to follow the same abstraction
> done multiple times in multiple series.
>
> Regarding your proposal for target_system_arch(), I understand that you tried
> to reuse the existing SysEmuTarget, which was a good intention.
> However, I don't think we should use any string compare for this (which
> qemu_api_parse does). It has several flaws:
qemu_api_parse()? Do you mean qapi_enum_parse()?
> - The most important one: it can fail (what if -1 is returned?). Enums can be
> guaranteed and exhaustive at compile time.
> - It's slower than having the current arch directly known at compile time.
> As well, since SysEmuTarget is a generated enum, it makes it much harder to
> follow code IMHO.
> QAPI requires those things to be defined from a json file for external usage,
> but it's not a good reason for being forced to use it in all the codebase as
> the only possible abstraction.
>
> To have something fast and infallible, we can adopt this solution:
>
> In target_info.h:
>
> /* Named TargetArch to not clash with poisoned TARGET_X */
> typedef enum TargetArch {
> TARGET_ARCH_AARCH64,
> TARGET_ARCH_ALPHA,
> TARGET_ARCH_ARM,
> TARGET_ARCH_AVR,
> TARGET_ARCH_HPPA,
> TARGET_ARCH_I386,
> TARGET_ARCH_LOONGARCH64,
> TARGET_ARCH_M68K,
> TARGET_ARCH_MICROBLAZE,
> TARGET_ARCH_MICROBLAZEEL,
> TARGET_ARCH_MIPS,
> TARGET_ARCH_MIPS64,
> TARGET_ARCH_MIPS64EL,
> TARGET_ARCH_MIPSEL,
> TARGET_ARCH_OR1K,
> TARGET_ARCH_PPC,
> TARGET_ARCH_PPC64,
> TARGET_ARCH_RISCV32,
> TARGET_ARCH_RISCV64,
> TARGET_ARCH_RX,
> TARGET_ARCH_S390X,
> TARGET_ARCH_SH4,
> TARGET_ARCH_SH4EB,
> TARGET_ARCH_SPARC,
> TARGET_ARCH_SPARC64,
> TARGET_ARCH_TRICORE,
> TARGET_ARCH_X86_64,
> TARGET_ARCH_XTENSA,
> TARGET_ARCH_XTENSAEB,
> } TargetArch;
Effectively duplicates generated enum SysEmuTarget. Can you explain why
that's useful?
>
> typedef struct TargetInfo {
> ...
> TargetArch target_arch;
> ...
> }
>
> static inline target_arch() {
> return target_info()->target_arch;
> }
>
> static inline target_aarch64() {
> return target_arch() == TARGET_ARCH_AARCH64;
> }
>
>
> In target_info-stub.c:
>
> #ifdef TARGET_AARCH64
> # define TARGET_ARCH TARGET_ARCH_AARCH64
> #elif TARGET_ARCH_ALPHA
> # define TARGET_ARCH TARGET_ARCH_ALPHA
> ...
> #endif
>
> static const TargetInfo target_info_stub = {
> ...
> .target_arch = TARGET_ARCH;
> ...
> }
