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

[Eli Friedman]

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