Re: [Qemu-devel] [PATCH 12/17] s390x: Prepare cpu.h for emulation

[Alexander Graf]

On 28.03.2011, at 16:54, Peter Maydell wrote:

> On 24 March 2011 15:58, Alexander Graf <address@hidden> wrote:
>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> 
> Minor nits only.
> 
>> -    FPReg fregs[16]; /* FP registers */
>> +    CPU_DoubleU fregs[16]; /* FP registers */
> 
> These changes mean that the FPReg typedef in this file is no longer
> used, so you might as well delete it.

Good point :)

> 
> Personally I prefer the way target-arm handles float regs,
> ie it just has 'float64 regs[32]' and relies on them being
> the right representation to pass in registers. This is
> less likely to work with float128s though, and anyway I suspect
> Nathan would disagree with me, so this isn't a request to change
> this code.
> 
>> +#define EXCP_EXT  1
>> +
>> +#define EXCP_SVC 2 /* supervisor call (syscall) */
>> +#define EXCP_PGM 3 /* program interruption */
>> +/* XXX */
>> +#define EXCP_EXECUTE_SVC 0xff00000 /* supervisor call via execute insn */
> 
> This comment ought to have an explanation of what the issue is
> that means it's 'XXX'...

It means "this exception shouldn't be referenced in any code, but was at the 
point of putting the XXX in there". I merely forgot to remove it again. Thanks 
for the reminder :).

> 
>> +    CC_OP_ADD_64,               /* */
>> +    CC_OP_ADDU_64,              /* */
>> +    CC_OP_SUB_64,               /* */
>> +    CC_OP_SUBU_64,              /* */
>> +    CC_OP_ABS_64,               /* */
>> +    CC_OP_NABS_64,              /* */
> 
> Why the empty comments?

Uh - yeah :).

> 
>> +static inline uint64_t time2tod(uint64_t time) {
>> +    return (time << 9) / 125;
>> +}
> 
> Could maybe use a comment about what units we're converting
> to and from here.

Yup :)


Alex