Re: [Qemu-devel] [PATCH 1/4] target-tilegx: Add fpu header file

[Chen Gang]

On 11/12/15 22:34, Richard Henderson wrote:
> On 11/08/2015 06:43 AM, Chen Gang wrote:
> 
>> +#if !defined(HOST_WORDS_BIGENDIAN)
>> +    /* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */
>> +    uint64_t exp : 8;             /* exp, 0x9e: 31 + TILEGX_F_EXP_FZERO */
>> +    uint64_t uiknown0 : 2;        /* unknown */
> 
> I would really rather you didn't use bitfields, because of exactly this sort 
> of endianness problem.  Because, really, you can't trust this layout.  But I 
> won't press this point, because it is complicated enough already.
> 

Because of endianess issues, for me, I don't like bit fields either. But
I can not find any other simpler ways than current.

>> +#endif
>> +} TileGXFPSFmt;
>> +/*
> 
> Watch your spacing.
> 

OK, thanks. And I also shall let the related comments above the structure.


>> + * Double exp analyzing: (0x21b00 << 1) - 0x36(54) = 0x400

Oh, it should be (0x21b00 << 1) - 0x37(55) = 0x3ff

>> + *
>> + *   17  16  15  14  13  12  11  10   9   8   7    6   5   4   3   2   1   0
>> + *
>> + *    1   0   0   0   0   1   1   0   1   1   0    0   0   0   0   0   0   0
>> + *
>> + *    0   0   0   0   0   1   1   0   1   1   1    => 0x37(55)
>> + *
>> + *    0   1   1   1   1   1   1   1   1   1   1    => 0x3ff
> 
> What is this table supposed to mean?
> 

I want to use this table to explain my guess: at first, we don't know
the internal exp position, neither know the internal mantasa bits. We
have to guess both of them:

 - the real exp should not be less than 11 bits (IEEE double exp is 11
   bits).

 - Since IEEE double exp 0 is 0x3ff, probobly, for tilegx internally,
   0x3ff shoul be '0', too (don't move mantissa).

 - After analyzing the data from floatunssidf2 in gcc tilegx.md (the
   table above is part for it), I found that "(0x21b00 << 1) - 0x37 =
   0x3ff" is the best to match "all things".

So, I guess: the double exp is from bit 7 to bit 17, then the mantissa
is 60 bits (55 + 1 + 4), it from bit 0 to bit 59.

We can use 7 - 19 bits for normal exp calculation, then can get the real
exp (7 - 17 bits) with the overflow and underflow (so, I guess 18 bit is
for overflow, and 19 bit is for underflow).

>> +#if 0
>> +        uint64_t exp : 11;        /* exp, 0x21b << 1: 55 + 
>> TILEGX_F_EXP_DZERO */
>> +        uint64_t ov : 1;          /* overflow for mul, low priority */
>> +        uint64_t uv : 1;          /* underflow for mul, high priority */
>> +#endif
> 
> 
> No if 0.
> 

OK, I shall remove them.

>> +#pragma pack(pop)
> 
> Huh?  What are you attempting to do here?
> 

It is for matching "#pragma pack(push, 1)" which is above all related
struct/unions in this header file.

For the bit fields, we use uint64_t, but gcc still treate it as two
uint32_t, so for safety reason, I use pragma pack for our structures.

Thanks.
-- 
Chen Gang (陈刚)

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