qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Chen Gang S
Subject: Re: [Qemu-devel] [PATCH] target-tilegx: Execute _start and reach to __libc_start_main successfully
Date: Wed, 25 Feb 2015 00:31:02 +0800
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.4.0

On 2/24/15 22:21, Chris Metcalf 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.
> 

Originally, I did not add zero register, but after think of, for gen_st
operation, tcg_gen_st*() only support from tcg_target_long to register,
so I add zero register for "offsetof(CPUTLState, zero)".

Welcome any better ways for it.
 
>> +#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.
> 

Originally, I really used 64 registers, but after tried, I found that I
still had to process TILEGX_R_ZERO specially, e.g.

 - When src is zero, we can use mov operation instead of add operation.

 - When dst is zero, in most cases, we can just skip the insn, but in
   some cases, it may cause exception in user mode (e.g st zero r0).

Originally, I also tried a wrap function for zero register, but I found
for different operands, when they meet zero register, they would need
different operations:

 - For add/addi operation, it will change to mov/movi operation.

 - For mov operation, it will change to movi operation.

 - For shl3add, if srcb is zero register, it will change to shli
   operation.

 - For ld/st operation, it still be ld/st operation, but they need
   "offsetof(CPUTLState, zero)", and in some cases, it should be cause
   exception.

So after think of, for me, I still prefer to use 56 registers with
individual zero register, and use macros for it.

> 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.
>

Yeah, thanks. I shall add it when I send patch v2 for it.

Also I forgot to use "offsetof(CPUTLState, zero)" for ld zero register
case, and still "return 0" for "st zero r1" or "ld r1 zero". I shall
fix it in the patch v2.

 
>> +    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?
>

OK, thanks.  originally I wanted to reuse them, but after think of, I
prefer the 64-bit immediate number instead of.

 - The decoding function may be very long, but it is still a simple
   function, I guess, it is still simple enough for readers.

 - 64-bit immediate number has better performance under 64-bit machine
   (e.g x86-64).

But I guess, there are still quite a few of inline functions (e.g. get
src/dst) in arch/opcode_tilegx.h may be reused by me in the future. :-)


> 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!
> 

Thank you for your work and your encouragement.


Thanks.
-- 
Chen Gang

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]