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

[Akira Hatanaka]

I filed a bug report.

http://llvm.org/bugs/show_bug.cgi?id=11290

Please take a look at it when you have time.

On Tue, Nov 1, 2011 at 6:10 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> 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
>>>
>>
>