Re: [Qemu-devel] [PATCH 09/10 v11] target-tilegx: Generate tcg instructions to finish "Hello world"

[Chen Gang]

On 6/3/15 00:32, Richard Henderson wrote:
> On 06/01/2015 01:54 PM, Chen Gang wrote:
>>> Further, the < TILEGX_R_COUNT restriction is also incorrect.  True, you 
>>> don't
>>> actually implement the top 7 special registers, but that doesn't matter, you
>>> should still be incrementing them.
>>>
>>
>> We did not implement them, so can not increment them, either.
>>
>> They are hidden to outside, or we have to define and implement them.
>>
>> So for me, the current code is correct.
> 
> It isn't correct, it's simply functional.  These registers may eventually be
> implemented, and at that point this code will fail.  You'll note that your
> store_add functions don't have the same problem, because they don't have this
> R_COUNT check.  It would be better to increase the number of buffer slots and
> do the right thing here in load_add.
> 

For me, it is about 2 discussions:

 - Whether need implement additional 7 registers.

   I guess not. But if we will really implement them in future, we need
   only let TILEGX_R_COUNT = TILEGX_R_ZERO, and all things should still
   be OK.

 - Whether need 2 or more tmp variables for one pipe.

   It is not necessary, but it will let the code simplier.


> My suggestion is to expand tmp_regs to 4, drop tmp_regcur, and have dest_gr
> manage all of the indexing.  I.e.
> 
> static TCGv dest_gr(DisasContext *dc, uint8_t rdst)
> {
>     int n = dc->n_tmp_regs++;
>     assert(n < ARRAY_SIZE(dc->tmp_regs));
>     dc->tmp_regs[n].idx = rdst;
>     return dc->tmp_regs[n].val = tcg_temp_new_i64();
> }
> 
> In this way you can in fact call dest_gr twice within load_add and everything
> will Just Work.
> 

For me, the code is fine (and reset dc->n_tmp_regs for each bundle).

Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed