[llvm-commits] [llvm-gcc-4.2] r46726 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

[Dale Johannesen]

On Feb 7, 2008, at 9:54 AM, Devang Patel wrote:

> It is possible that I'm not solving the real problem here.
>
> struct {
> 	char a;
> 	char b;
> 	int c:8;
> 	int d:20;
> } S;
>
> When this struct is packed and S is aligned at 4 byte, what alignment
> should be specified on load instruction to load 'd' ?

The struct is only 6 bytes long in this case (try it with gcc), so  
representing d with a 32-bit load is probably not a good idea; there  
are machines where such misaligned loads are valid (e.g. x86) and a 32- 
bit load will read off the end of the structure.

In general I prefer the IR to be target-independent where possible,  
but I don't see a good alternative to having the FE break up the load  
of d into 3 pieces here (or perhaps char + short).  Having the IR  
include a load off the end of the object, that wasn't there in the  
source, and requiring the BE to figure this out seems very wrong.  So  
to answer your question, there needs to be more than one load.

However, if we modify the packed struct thus (it's now 7 bytes)

> struct {
> 	char a;
> 	char b;
> 	int c:8;
> 	int d:32;
> } S;


referring to d with misaligned 32-bit loads and stores is sensible.   
That's valid code on x86, and representing it as byte operations and  
expecting the BE to stick them back together is a lot of trouble for  
no benefit.  Of course, on many other targets, that load must be  
broken into pieces.   That could be done in either end, but since  
we've got to break up 20-bit fields in the FE anyway, I guess we might  
as well do so for 32-bit fields when the target requires that.  I  
could probably be convinced to do this one in the BE though.

> Devang
> On Feb 7, 2008, at 9:35 AM, Duncan Sands wrote:
>
>> Hi Devang,
>>
>>> Use appropriate alignment while handling  packed struct member.
>>
>> the reason I asked for a testcase is that I thought all objects
>> created by the IR we generate (whether locals, globals or on the
>> heap) were supposed to be aligned at least as much as the gcc
>> alignment.  If so, the fact that we create a packed struct type
>> (alignment 1) shouldn't make the gcc object alignment info in
>> expr_align wrong, in fact it should be irrelevant.  My fear is
>> that your change might only be papering over the problem.
>>
>>>   bool isVolatile = TREE_THIS_VOLATILE(exp);
>>>   const Type *Ty = ConvertType(TREE_TYPE(exp));
>>>   unsigned Alignment = expr_align(exp) / 8;
>>> +  if (TREE_CODE(exp) == COMPONENT_REF)
>>> +    if (const StructType *STy =
>>> +         
>>> dyn_cast<StructType>(ConvertType(TREE_TYPE(TREE_OPERAND(exp,
>>> 0))))) +      if (STy->isPacked())
>>> +        // Packed struct members use 1 byte alignment
>>> +        Alignment = 1;
>>> +
>>>
>>
>> Best wishes,
>>
>> Duncan.
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits