Re: [Qemu-devel] [PATCH] target-tilegx: Execute _start and reach to __libc_start_main successfully

[Chris Metcalf <cmetcalf@ezchip.com>]

On 2/24/2015 2:53 AM, Chen Gang S wrote:
>   typedef struct CPUTLState {
> +    uint64_t regs[TILEGX_R_COUNT]; /* Common used registers by outside */
> +    uint64_t zero;                 /* Zero register */
> +    uint64_t pc;                   /* Current pc */
>       CPU_COMMON
>   } CPUTLState;

I skimmed through this and my only comment is that I was surprised to see "zero" as part of the state, since it's always 0.  :-)  No doubt there is some reason that this makes sense.

> +#define TILEGX_GEN_CHK_BEGIN(x) \
> +    if ((x) == TILEGX_R_ZERO) {
> +#define TILEGX_GEN_CHK_END(x) \
> +        return 0; \
> +    } \
> +    if ((x) >= TILEGX_R_COUNT) { \
> +        return -1; \
> +    }

This macro pattern seems potentially a little confusing and I do wonder if there is some way to avoid having to explicitly check the zero register every time; for example, maybe you make it a legitimate part of the state and declare that there are 64 registers, and then just always finish any register-update phase by re-zeroing that register?  It might yield a smaller code footprint and probably be just as fast, as long as it was easy to know where registers were updated.

Also, note that you should model accesses to registers 56..62 as causing an interrupt to be raised, rather than returning -1.  But you might choose to just put this on your TODO list and continue making forward progress for the time being.  But a FIXME comment here would be appropriate.

> +    case 0x3800000000000000ULL:

There are a lot of constants defined in the tile <arch/opcode.h> header, and presumably you could synthesize these constant values from them.  Perhaps it would make sense to import that header into qemu and then use symbolic values for all of this kind of thing?

I can't really comment on most of the rest of the code, and I only skimmed it quickly, but generally: good work getting as far as you have!

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com