[LLVMdev] RFC: llvm-convert.cpp Patch

[Bill Wendling]

On Nov 6, 2007, at 8:25 PM, Chris Lattner wrote:
> On Nov 6, 2007, at 5:45 PM, Bill Wendling wrote:
>
>> Hi all,
>>
>> This patch is to fix a problem on PPC64 where an unaligned memcpy is
>> generated. The testcase is this:
>>
>> $ cat testcase.c
>> void Qux() {
>>  char Bar[11] = {0};
>> }
>>
>> What happens is that we produce LLVM code like this:
>>
>> 	call void @llvm.memcpy.i64( i8* %event_list2, i8* getelementptr ([11
>> x i8]* @C.103.30698, i32 0, i32 0), i64 11, i32 8 )
>>
>> Notice that it has an 8-byte alignment. However, the Bar variable has
>> 1-byte alignment, because it's being placed into the "redzone". The
>> generic code for the function is:
>>
>> $ more testcase.c.t03.generic
>> Qux ()
>> {
>>  static char C.0[11] = {0};
>>  char Bar[11];
>>
>>  Bar = C.0;
>> }
>>
>> Anyway, it turns out that the gimplifier was generating the correct
>> alignment, but it was being overridden in assemble_variable():
>>
>>  /* On some machines, it is good to increase alignment sometimes.  */
>>  if (! DECL_USER_ALIGN (decl))
>>    {
>> #ifdef DATA_ALIGNMENT
>>      align = DATA_ALIGNMENT (TREE_TYPE (decl), align);
>> #endif
>> #ifdef CONSTANT_ALIGNMENT
>> =>    if (DECL_INITIAL (decl) != 0 && DECL_INITIAL (decl) !=
>> error_mark_node)
>>        align = CONSTANT_ALIGNMENT (DECL_INITIAL (decl), align);
>> #endif
>>    }
>>
>> By setting the DECL_USER_ALIGN field, we can get the correct
>> alignment.
>
> I don't get it Bill.  llvm should be making memcpy with the right
> alignment regardless of how Bar is formed.  Changing the alignment of
> bar papers over this problem, but we will still get improper alignment
> for other cases.  Also, DECL_USER_ALIGN isn't appropriate to tweak
> here... the user isn't playing around with __attribute__((aligned))
>
Bar isn't the one that's getting realigned with the above code, it's  
the constructor. The patch only sets the DECL_USER_ALIGN field for  
the constructor (not Bar) when the constructor's created. My thoughts  
were that if the constructor was initially given the alignment of the  
variable (Bar, in this case), then it shouldn't be changed in the  
assemble_variable() function, unless it's also changing the alignment  
of Bar (which it's not). I'm actually not sure why it doesn't change  
Bar's alignment when it's changing Bar's constructor's alignment.  
That seems like a GCC bug, IMHO, though they probably don't get hit  
with it or "fix" it later on or something.

My guess is that you want something akin to  
TreeToLLVM::EmitBuiltinMemCopy where it gets the min of the alignments?

-bw