On 24 February 2015 at 23:21, Chris Metcalf <cmetc...@ezchip.com> wrote:
> 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.
I think that is definitely an error...
>> +#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.
See target-arm/translate-a64.c for one way to handle an
always-reads-as-zero register.
-- PMM
