[llvm-commits] [llvm-gcc-4.2] r47655 - /llvm-gcc-4.2/trunk/gcc/llvm-types.cpp

[Dale Johannesen]

On Feb 26, 2008, at 9:20 PM, Chris Lattner wrote:

> On Feb 26, 2008, at 5:32 PM, Dale Johannesen wrote:
>> URL: http://llvm.org/viewvc/llvm-project?rev=47655&view=rev
>> Log:
>> Hack to handle darwin ppc32's weird choice of
>> representation for bool.  Fixes
>> gcc.dg-struct-layout-1/t009

Sorry, I assumed some familiarity with the testsuite structure, and  
have the pathname wrong to boot.  The file struct-layout-1.exp builds  
tests dynamically; it lives in <llvm-gcc.src>/gcc/testsuite/gcc.dg/ 
compat.  The dynamically built tests consist of 3 source files each of  
whose names contains a string of the form t0<2-digit number>.  The  
easiest way to run them is

cd <llvm-gcc.obj>
make -k check RUNTESTFLAGS=struct-layout-1.exp

and look at the log file in <llvm-gcc.obj>/gcc/testsuite/gcc/gcc.log  
to see what got executed.  There is also a C++ variant.
These tests exercise self-consistency in struct/union layout and  
calling convention very thoroughly (they are not ABI tests against the  
installed cc), and are well worth looking at.
Currently, this passes cleanly on Darwin x86-32 (finally!) but there  
are lots of other failures on ppc32, which I'll be looking at.
Anton said there are also a few failures left on Linux x86-32, but  
that thread seems to have died...

>> Nice!

Really?

>
>>    tree TheGccTy = TREE_TYPE(Field);
>> +#ifdef TARGET_POWERPC
>
> Eww. :)  Is this information encoded in the trees somehow?  Duncan, do
> you have any idea?  Dale, can you include a tree dump of the type in
> this case?

That's more like it.   This is the source (remember _Bool is 4 bytes):

typedef _Bool Tal2bool __attribute__((aligned (2)));
union {Tal2bool g:1;} u;

and this is the tree:

<union_type 0x104bbd0 type_0 HI
     size <integer_cst 0x42804420 type <integer_type 0x4280d070  
bit_size_type> constant invariant 16>
     unit size <integer_cst 0x42804450 type <integer_type 0x4280d000  
long unsigned int> constant invariant 2>
     user align 16 symtab 0 alias set -1
     fields <field_decl 0x104bc40 g
         type <boolean_type 0xe89b60 Tal2bool public unsigned SI
             size <integer_cst 0x42804600 constant invariant 32>
             unit size <integer_cst 0x42804120 constant invariant 4>
             user align 16 symtab 0 alias set -1 precision 1 min  
<integer_cst 0x428049f0 0> max <integer_cst 0x42804a50 1>>
         unsigned external asm-frame-size 0 bit-field nonaddressable  
decl_4 SI file /Volumes/MacOS9/gcc/llvm-gcc-4.2.leo.obj/gcc/testsuite/ 
gcc/gcc.dg-struct-layout-1/t009_test.h line 47
         size <integer_cst 0x42804f00 constant invariant 1>
         unit size <integer_cst 0x42804240 constant invariant 1>
         align 1 offset_align 128
         offset <integer_cst 0x42804150 constant invariant 0>
         bit offset <integer_cst 0x42804d20 constant invariant 0>  
bit_field_type <boolean_type 0xe89b60 Tal2bool> context <union_type  
0x104bbd0>>
     pointer_to_this <pointer_type 0x104bd90> chain <type_decl  
0x1203080 D.5522>>

On Feb 26, 2008, at 10:35 PM, Anton Korobeynikov wrote:
>> +#ifdef TARGET_POWERPC
> Also, maybe it will be better to move this chunk of code into
> config/rs6000 directory? The same way as TARGET_ADJUST_LLVM_CC and
> friends are working?

Sure, I can do that.  Personally I've never like this part of gcc  
style, which seems to me to obscure what's going on, but it is  
traditional.

> +    // Normally gcc reduces the size of bitfields to the size  
> necessary
> +    // to hold the bits, e.g. a 1-bit field becomes QI.

On Feb 27, 2008, at 12:43 AM, Duncan Sands wrote:
>> +    // Normally gcc reduces the size of bitfields to the size  
>> necessary
>> +    // to hold the bits, e.g. a 1-bit field becomes QI.
>
> So this is a union with a bitfield member?  If so, can you not just  
> do:
>
> (1) use isBitfield to see if it is a bitfield.
> (2) rather than converting the type (which should be completely  
> ignored in
> the case of a bitfield), generate i8, i16 or whatever based on  
> DECL_SIZE.
>
> And don't condition this on PPC or other tests.  If the problem is  
> what I'm
> imagining it is, this is needed in general: though we usually get  
> away with
> it by using the type generated by gcc's struct layout, we don't in  
> fact want
> to use the types produced by gcc's struct layout for bitfields (we  
> don't in
> the rest of the struct conversion stuff).


ConvertUnion does in fact look at the type of bitfields; I didn't  
introduce this.  In all other cases I've seen, gcc has already done  
the reduction to i8 etc. that you're talking about, so in general this  
works.  The problem here is that the type of the bitfield is bigger  
than the type of the union; that is due to Darwin ppc32's bizarre  
choice of representation for bool, and I don't think it can happen  
otherwise.