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

[Akira Hatanaka]

Okay, should I file a bug report for this?

> 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
>