From c5c164154328f56796916ea6828ab15811ebac97 Mon Sep 17 00:00:00 2001
From: James Y Knight <jyknight@google.com>
Date: Fri, 21 Aug 2015 18:19:06 +0000
Subject: [PATCH] Properly provide alignment of 'byval' arguments down to llvm.

This is important in the case that the LLVM-inferred llvm-struct
alignment is not the same as the clang-known C-struct alignment.

Differential Revision: http://reviews.llvm.org/D12243

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@245719 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/CodeGen/CGCall.cpp         | 21 ++++++++++++++++++---
 test/CodeGen/le32-arguments.c  |  6 +++---
 test/CodeGen/nvptx-abi.c       | 10 +++++-----
 test/CodeGen/sparc-arguments.c | 27 +++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 11 deletions(-)
 create mode 100644 test/CodeGen/sparc-arguments.c

diff --git a/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp
index a5802346d4..f40dd08648 100644
--- a/lib/CodeGen/CGCall.cpp
+++ b/lib/CodeGen/CGCall.cpp
@@ -1663,20 +1663,35 @@ void CodeGenModule::ConstructAttributeList(const CGFunctionInfo &FI,
         Attrs.addAttribute(llvm::Attribute::InReg);
       break;
 
-    case ABIArgInfo::Indirect:
+    case ABIArgInfo::Indirect: {
       if (AI.getInReg())
         Attrs.addAttribute(llvm::Attribute::InReg);
 
       if (AI.getIndirectByVal())
         Attrs.addAttribute(llvm::Attribute::ByVal);
 
-      Attrs.addAlignmentAttr(AI.getIndirectAlign());
+      unsigned Align = AI.getIndirectAlign();
+
+      // In a byval argument, it is important that the required
+      // alignment of the type is honored, as LLVM might be creating a
+      // *new* stack object, and needs to know what alignment to give
+      // it. (Sometimes it can deduce a sensible alignment on its own,
+      // but not if clang decides it must emit a packed struct, or the
+      // user specifies increased alignment requirements.)
+      //
+      // This is different from indirect *not* byval, where the object
+      // exists already, and the align attribute is purely
+      // informative.
+      if (Align == 0 && AI.getIndirectByVal())
+        Align = getContext().getTypeAlignInChars(ParamType).getQuantity();
+
+      Attrs.addAlignmentAttr(Align);
 
       // byval disables readnone and readonly.
       FuncAttrs.removeAttribute(llvm::Attribute::ReadOnly)
         .removeAttribute(llvm::Attribute::ReadNone);
       break;
-
+    }
     case ABIArgInfo::Ignore:
     case ABIArgInfo::Expand:
       continue;
diff --git a/test/CodeGen/le32-arguments.c b/test/CodeGen/le32-arguments.c
index d26640e692..e81d84387d 100644
--- a/test/CodeGen/le32-arguments.c
+++ b/test/CodeGen/le32-arguments.c
@@ -10,7 +10,7 @@ typedef struct {
   int bb;
 } s1;
 // Structs should be passed byval and not split up
-// CHECK-LABEL: define void @f1(%struct.s1* byval %i)
+// CHECK-LABEL: define void @f1(%struct.s1* byval align 4 %i)
 void f1(s1 i) {}
 
 typedef struct {
@@ -48,7 +48,7 @@ union simple_union {
   char b;
 };
 // Unions should be passed as byval structs
-// CHECK-LABEL: define void @f7(%union.simple_union* byval %s)
+// CHECK-LABEL: define void @f7(%union.simple_union* byval align 4 %s)
 void f7(union simple_union s) {}
 
 typedef struct {
@@ -57,5 +57,5 @@ typedef struct {
   int b8 : 8;
 } bitfield1;
 // Bitfields should be passed as byval structs
-// CHECK-LABEL: define void @f8(%struct.bitfield1* byval %bf1)
+// CHECK-LABEL: define void @f8(%struct.bitfield1* byval align 4 %bf1)
 void f8(bitfield1 bf1) {}
diff --git a/test/CodeGen/nvptx-abi.c b/test/CodeGen/nvptx-abi.c
index 58ad6a17f2..7973bf0653 100644
--- a/test/CodeGen/nvptx-abi.c
+++ b/test/CodeGen/nvptx-abi.c
@@ -21,14 +21,14 @@ float bar(void) {
 
 void foo(float4_t x) {
 // CHECK-LABEL: @foo
-// CHECK: %struct.float4_s* byval %x
+// CHECK: %struct.float4_s* byval align 4 %x
 }
 
 void fooN(float4_t x, float4_t y, float4_t z) {
 // CHECK-LABEL: @fooN
-// CHECK: %struct.float4_s* byval %x
-// CHECK: %struct.float4_s* byval %y
-// CHECK: %struct.float4_s* byval %z
+// CHECK: %struct.float4_s* byval align 4 %x
+// CHECK: %struct.float4_s* byval align 4 %y
+// CHECK: %struct.float4_s* byval align 4 %z
 }
 
 typedef struct nested_s {
@@ -39,5 +39,5 @@ typedef struct nested_s {
 
 void baz(nested_t x) {
 // CHECK-LABEL: @baz
-// CHECK: %struct.nested_s* byval %x)
+// CHECK: %struct.nested_s* byval align 8 %x)
 }
diff --git a/test/CodeGen/sparc-arguments.c b/test/CodeGen/sparc-arguments.c
new file mode 100644
index 0000000000..c86b40b11f
--- /dev/null
+++ b/test/CodeGen/sparc-arguments.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple sparc-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+
+// Ensure that we pass proper alignment to llvm in the call
+// instruction. The proper alignment for the type is sometimes known
+// only by clang, and is not manifest in the LLVM-type. So, it must be
+// explicitly passed through. (Besides the case of the user specifying
+// alignment, as here, this situation also occurrs for non-POD C++
+// structs with tail-padding: clang emits these as packed llvm-structs
+// for ABI reasons.)
+
+struct s1 {
+  int x;
+} __attribute__((aligned(8)));
+
+struct s1 x1;
+
+
+// Ensure the align 8 is passed through:
+// CHECK-LABEL: define void @f1()
+// CHECK: call void @f1_helper(%struct.s1* byval align 8 @x1)
+// Also ensure the declaration of f1_helper includes it
+// CHECK: declare void @f1_helper(%struct.s1* byval align 8)
+
+void f1_helper(struct s1);
+void f1() {
+  f1_helper(x1);
+}
-- 
2.40.0