[cfe-dev] alignment of fields in byval aggregate arguments

[Eli Friedman]

On Tue, Nov 1, 2011 at 5:48 PM, Akira Hatanaka <ahatanak at gmail.com> wrote:
> Okay, should I file a bug report for this?

Sure.  I'll try and take a look sometime soon.

-Eli

>> The alignment computation here is simply overconservative.  The
>> natural alignment of the element type isn't necessarily correct, and 1
>> is conservatively correct.  It should be easy to fix; in this context,
>> the alignment of the underlying object is TypeAlign.
>
>
> On Tue, Nov 1, 2011 at 5:05 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>> On Tue, Nov 1, 2011 at 4:36 PM, Akira Hatanaka <ahatanak at gmail.com> wrote:
>>> I am trying to make changes in the way clang handles Mips' N64 byval
>>> arguments based on the recommendation in the link below. This ABI
>>> requires double arguments in byval structures be passed in 64-bit
>>> double precision registers and everything else in 64-bit integer
>>> registers (or on the stack, if the argument registers are already in
>>> use).
>>>
>>> http://thread.gmane.org/gmane.comp.compilers.llvm.devel/44509
>>>
>>> This is the example source program I am going to use to explain my approach:
>>>
>>> ### source code
>>> typedef struct {
>>>  double d1;
>>>  double d2;
>>>  int i1[4];
>>>  double d3;
>>> } S1;
>>>
>>> void foo2(int, S1 s1);
>>>
>>> void foo1(S1 *s1) {
>>>  foo2(13, *s1);
>>> }
>>> ### end of source code
>>>
>>> The changes I made are in MipsABIInfo::classifyArgumentType in
>>> lib/CodeGen/TargetInfo.cpp. Instead of returning
>>> ABIArgInfo::getIndirect when a byval argument is passed, it returns
>>> ABIArgInfo::getDirect(structure {f64, f64, i64, i64, f64}).
>>>
>>> This is the bit code clang generates after I apply the changes:
>>>
>>> ### bit code
>>> %struct.S1 = type { double, double, [4 x i32], double }
>>>
>>> define void @foo1(%struct.S1* nocapture %s1) nounwind {
>>> entry:
>>>  %0 = getelementptr inbounds %struct.S1* %s1, i64 0, i32 0
>>>  %1 = load double* %0, align 1
>>>  %2 = getelementptr %struct.S1* %s1, i64 0, i32 1
>>>  %3 = load double* %2, align 1
>>>  %4 = getelementptr %struct.S1* %s1, i64 0, i32 2
>>>  %5 = bitcast [4 x i32]* %4 to i64*
>>>  %6 = load i64* %5, align 1
>>>  %7 = getelementptr %struct.S1* %s1, i64 0, i32 2, i64 2
>>>  %8 = bitcast i32* %7 to i64*
>>>  %9 = load i64* %8, align 1
>>>  %10 = getelementptr %struct.S1* %s1, i64 0, i32 3
>>>  %11 = load double* %10, align 1
>>>  tail call void @foo2(i32 13, double %1, double %3, i64 %6, i64 %9,
>>> double %11) nounwind
>>>  ret void
>>> }
>>>
>>> declare void @foo2(i32, double, double, i64, i64, double)
>>> ### end of bit code
>>>
>>> The generated code above is fine except that the loads' alignments are
>>> all 1, which results in the backend generating unaligned loads. I
>>> tracked down what is causing this to happen and discovered the
>>> following piece of code in CGCall.cpp is determining the alignment:
>>>
>>> 01703           // We don't know what we're loading from.
>>> 01704           LI->setAlignment(1);
>>>
>>> What is the reason the alignment here always has to be 1 and are there
>>> any ways to have it emit a different alignment?
>>
>> The alignment computation here is simply overconservative.  The
>> natural alignment of the element type isn't necessarily correct, and 1
>> is conservatively correct.  It should be easy to fix; in this context,
>> the alignment of the underlying object is TypeAlign.
>>
>> -Eli
>>
>