[cfe-commits] r51673 - in /cfe/trunk: lib/CodeGen/CodeGenTypes.cpp test/CodeGen/struct-x86-darwin.c test/CodeGen/struct.c

[Devang Patel]

On May 29, 2008, at 2:29 PM, Eli Friedman wrote:

> On Thu, May 29, 2008 at 1:24 PM, Devang Patel <dpatel at apple.com>  
> wrote:
>>
>> On May 29, 2008, at 3:58 AM, Eli Friedman wrote:
>>
>>> Author: efriedma
>>> Date: Thu May 29 05:58:49 2008
>>> New Revision: 51673
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=51673&view=rev
>>> Log:
>>> Always use packed structs.  This isn't really very nice, but there's
>>> nothing fundamentally wrong with it. Emitting unpacked structs where
>>> possible is more work for almost no practical benefit.
>>
>> It may cause performance degradation in real world.
>
> I don't follow; this doesn't affect the alignment of loads.

Consider

	struct { char c; int i; }

At llvm level you use either

        { i8, i32 }

or

       <{ i8, i8, i8, i8, i32 }>

The packed version involves 5 copies during  struct element by element  
copy. And it also raises the issue of dealing with undefined bytes.

Plus, such layout change causes changes in the arithmetic involved to  
access bit fields.  I have myself once added two line patch in llvm- 
gcc to aggressively (but not all the time) use packed LLVM struct.  
This simplified code in llvm-gcc but I had to back out that patch  
after few weeks. Because our customer reported 25% performance  
regression and we tracked it down to this change in struct layout at  
llvm level, which introduced significant penalty while accessing union  
bitfields.

You can argue that 1) the there is no guaranteed solution if the code  
performance relies on such behavior  and 2) the code that emits   
bitfield access in llvm-gcc can be made smarter.
But the point is, such changes have performance impact, so do not take  
it lightly.

>>> We'll probably
>>> want to fix it at some point anyway, but it's low priority.
>>
>> Please add FIXME in the code.
>
> Okay, I'll do that.

Thanks!

>>> The issue with long double in particular is that LLVM thinks an  
>>> X86 long
>>> double is 10 bytes, while clang considers it for all purposes to be
>>> either 12 or 16 bytes, depending on the platform, even in a packed
>>> struct.
>>
>> I expect clang and LLVM to use same target data. Is it not true ?
>
> When LLVM packs a struct, it appears to use the tightest packing
> possible, which is in fact 10 bytes for an x86 long double.  The issue
> is that gcc-compatible C struct layout doesn't allow packing them that
> tightly (might be considered a bug, but not really something that can
> be fixed at this point).  Not sure how to deal with it (it doesn't
> show up with types with power-of-two width).

I am at lost. When you say "LLVM", you mean llvm-gcc ?
-
Devang