Re: [Qemu-devel] [PATCH 1/4] target-tilegx: Add fpu header file
On 11/13/15 00:10, Peter Maydell wrote: > On 12 November 2015 at 16:04, Chen Gang wrote: >> 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. > >> 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. > > Please don't use 'pragma pack' or bitfields. If you need to pack > and unpack things from a target-CPU defined field use bit operations > and/or extract32/deposit32/extract64/deposit64. > OK, thanks, although it means I have to rewrite most of code, and test them again. I shall try to send patch v2 within the next week. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH 1/4] target-tilegx: Add fpu header file
On 12 November 2015 at 16:04, Chen Gang wrote: > 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. > 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. Please don't use 'pragma pack' or bitfields. If you need to pack and unpack things from a target-CPU defined field use bit operations and/or extract32/deposit32/extract64/deposit64. thanks -- PMM
Re: [Qemu-devel] [PATCH 1/4] target-tilegx: Add fpu header file
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 76 5 4 3 2 1 0 >> + * >> + *1 0 0 0 0 1 1 0 1 1 00 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
Re: [Qemu-devel] [PATCH 1/4] target-tilegx: Add fpu header file
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. +#endif +} TileGXFPSFmt; +/* Watch your spacing. + * Double exp analyzing: (0x21b00 << 1) - 0x36(54) = 0x400 + * + * 17 16 15 14 13 12 11 10 9 8 76 5 4 3 2 1 0 + * + *1 0 0 0 0 1 1 0 1 1 00 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? +#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. +#pragma pack(pop) Huh? What are you attempting to do here? r~
