[PATCH] D42521: [CodeGen] Use the non-virtual alignment when emitting the base class subobject constructor

2018-01-26 Thread Akira Hatanaka via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC323578: [CodeGen] Use the non-virtual alignment when 
emitting the base (authored by ahatanak, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D42521

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenCXX/virtual-bases.cpp


Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -615,7 +615,14 @@
 
   llvm::Value *ThisPtr = CGF.LoadCXXThis();
   QualType RecordTy = CGF.getContext().getTypeDeclType(ClassDecl);
-  LValue LHS = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy);
+  LValue LHS;
+
+  // If a base constructor is being emitted, create an LValue that has the
+  // non-virtual alignment.
+  if (CGF.CurGD.getCtorType() == Ctor_Base)
+LHS = CGF.MakeNaturalAlignPointeeAddrLValue(ThisPtr, RecordTy);
+  else
+LHS = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy);
 
   EmitLValueForAnyFieldInitialization(CGF, MemberInit, LHS);
 
Index: test/CodeGenCXX/virtual-bases.cpp
===
--- test/CodeGenCXX/virtual-bases.cpp
+++ test/CodeGenCXX/virtual-bases.cpp
@@ -46,3 +46,37 @@
 D::D() { }
 
 }
+
+namespace virtualBaseAlignment {
+
+// Check that the store to B::x in the base constructor has an 8-byte 
alignment.
+
+// CHECK: define linkonce_odr void 
@_ZN20virtualBaseAlignment1BC1Ev(%[[STRUCT_B:.*]]* %[[THIS:.*]])
+// CHECK: %[[THIS_ADDR:.*]] = alloca %[[STRUCT_B]]*, align 8
+// CHECK: store %[[STRUCT_B]]* %[[THIS]], %[[STRUCT_B]]** %[[THIS_ADDR]], 
align 8
+// CHECK: %[[THIS1:.*]] = load %[[STRUCT_B]]*, %[[STRUCT_B]]** %[[THIS_ADDR]], 
align 8
+// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_B]], %[[STRUCT_B]]* 
%[[THIS1]], i32 0, i32 2
+// CHECK: store i32 123, i32* %[[X]], align 16
+
+// CHECK: define linkonce_odr void 
@_ZN20virtualBaseAlignment1BC2Ev(%[[STRUCT_B]]* %[[THIS:.*]], i8** %{{.*}})
+// CHECK: %[[THIS_ADDR:.*]] = alloca %[[STRUCT_B]]*, align 8
+// CHECK: store %[[STRUCT_B]]* %[[THIS]], %[[STRUCT_B]]** %[[THIS_ADDR]], 
align 8
+// CHECK: %[[THIS1:.*]] = load %[[STRUCT_B]]*, %[[STRUCT_B]]** %[[THIS_ADDR]], 
align 8
+// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_B]], %[[STRUCT_B]]* 
%[[THIS1]], i32 0, i32 2
+// CHECK: store i32 123, i32* %[[X]], align 8
+
+struct A {
+  __attribute__((aligned(16))) double data1;
+};
+
+struct B : public virtual A {
+  B() : x(123) {}
+  double a;
+  int x;
+};
+
+struct C : public virtual B {};
+
+void test() { B b; C c; }
+
+}


Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -615,7 +615,14 @@
 
   llvm::Value *ThisPtr = CGF.LoadCXXThis();
   QualType RecordTy = CGF.getContext().getTypeDeclType(ClassDecl);
-  LValue LHS = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy);
+  LValue LHS;
+
+  // If a base constructor is being emitted, create an LValue that has the
+  // non-virtual alignment.
+  if (CGF.CurGD.getCtorType() == Ctor_Base)
+LHS = CGF.MakeNaturalAlignPointeeAddrLValue(ThisPtr, RecordTy);
+  else
+LHS = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy);
 
   EmitLValueForAnyFieldInitialization(CGF, MemberInit, LHS);
 
Index: test/CodeGenCXX/virtual-bases.cpp
===
--- test/CodeGenCXX/virtual-bases.cpp
+++ test/CodeGenCXX/virtual-bases.cpp
@@ -46,3 +46,37 @@
 D::D() { }
 
 }
+
+namespace virtualBaseAlignment {
+
+// Check that the store to B::x in the base constructor has an 8-byte alignment.
+
+// CHECK: define linkonce_odr void @_ZN20virtualBaseAlignment1BC1Ev(%[[STRUCT_B:.*]]* %[[THIS:.*]])
+// CHECK: %[[THIS_ADDR:.*]] = alloca %[[STRUCT_B]]*, align 8
+// CHECK: store %[[STRUCT_B]]* %[[THIS]], %[[STRUCT_B]]** %[[THIS_ADDR]], align 8
+// CHECK: %[[THIS1:.*]] = load %[[STRUCT_B]]*, %[[STRUCT_B]]** %[[THIS_ADDR]], align 8
+// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_B]], %[[STRUCT_B]]* %[[THIS1]], i32 0, i32 2
+// CHECK: store i32 123, i32* %[[X]], align 16
+
+// CHECK: define linkonce_odr void @_ZN20virtualBaseAlignment1BC2Ev(%[[STRUCT_B]]* %[[THIS:.*]], i8** %{{.*}})
+// CHECK: %[[THIS_ADDR:.*]] = alloca %[[STRUCT_B]]*, align 8
+// CHECK: store %[[STRUCT_B]]* %[[THIS]], %[[STRUCT_B]]** %[[THIS_ADDR]], align 8
+// CHECK: %[[THIS1:.*]] = load %[[STRUCT_B]]*, %[[STRUCT_B]]** %[[THIS_ADDR]], align 8
+// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_B]], %[[STRUCT_B]]* %[[THIS1]], i32 0, i32 2
+// CHECK: store i32 123, i32* %[[X]], align 8
+
+struct A {
+  __attribute__((aligned(16))) double data1;
+};
+
+struct B : public virtual A {
+  B() : x(123) {}
+  double a;
+  int x;
+};
+
+struct C : public virtual B {};
+
+void test() { B b; C c; }
+
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D42521: [CodeGen] Use the non-virtual alignment when emitting the base class subobject constructor

2018-01-25 Thread John McCall via Phabricator via cfe-commits 
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM.


https://reviews.llvm.org/D42521



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42521: [CodeGen] Use the non-virtual alignment when emitting the base class subobject constructor

2018-01-25 Thread Akira Hatanaka via Phabricator via cfe-commits 
ahatanak created this revision.
ahatanak added reviewers: rjmccall, vsk.

This patch fixes what looks like a bug in IRGen's member initialization where 
it uses the alignment of a complete object instead of its non-virtual alignment 
when emitting the base constructor.

For example, when emitting the base constructor for B, IRGen uses a 16-byte 
alignment to initialize B::x because B's alignment is 16-byte (which is because 
A is 16-byte aligned). This is not correct since A is a virtual base of B and 
therefore we cannot guarantee that the pointer to B that is passed to the base 
constructor is always 16-byte aligned.

  struct A {
__attribute__((aligned(16))) double data1;
  };
  
  struct B : public virtual A {
B() : x(123) {}
double a;
int x;
  };
  
  struct C : public virtual B {};
  
  void test() { B b; C c; }

To fix the bug, this patch calls CGF.MakeNaturalAlignPointeeAddrLValue in 
EmitMemberInitializer to create an LValue that uses the non-virtual alignment 
of the class.

rdar://problem/36382481


https://reviews.llvm.org/D42521

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenCXX/virtual-bases.cpp


Index: test/CodeGenCXX/virtual-bases.cpp
===
--- test/CodeGenCXX/virtual-bases.cpp
+++ test/CodeGenCXX/virtual-bases.cpp
@@ -46,3 +46,37 @@
 D::D() { }
 
 }
+
+namespace virtualBaseAlignment {
+
+// Check that the store to B::x in the base constructor has an 8-byte 
alignment.
+
+// CHECK: define linkonce_odr void 
@_ZN20virtualBaseAlignment1BC1Ev(%[[STRUCT_B:.*]]* %[[THIS:.*]])
+// CHECK: %[[THIS_ADDR:.*]] = alloca %[[STRUCT_B]]*, align 8
+// CHECK: store %[[STRUCT_B]]* %[[THIS]], %[[STRUCT_B]]** %[[THIS_ADDR]], 
align 8
+// CHECK: %[[THIS1:.*]] = load %[[STRUCT_B]]*, %[[STRUCT_B]]** %[[THIS_ADDR]], 
align 8
+// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_B]], %[[STRUCT_B]]* 
%[[THIS1]], i32 0, i32 2
+// CHECK: store i32 123, i32* %[[X]], align 16
+
+// CHECK: define linkonce_odr void 
@_ZN20virtualBaseAlignment1BC2Ev(%[[STRUCT_B]]* %[[THIS:.*]], i8** %{{.*}})
+// CHECK: %[[THIS_ADDR:.*]] = alloca %[[STRUCT_B]]*, align 8
+// CHECK: store %[[STRUCT_B]]* %[[THIS]], %[[STRUCT_B]]** %[[THIS_ADDR]], 
align 8
+// CHECK: %[[THIS1:.*]] = load %[[STRUCT_B]]*, %[[STRUCT_B]]** %[[THIS_ADDR]], 
align 8
+// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_B]], %[[STRUCT_B]]* 
%[[THIS1]], i32 0, i32 2
+// CHECK: store i32 123, i32* %[[X]], align 8
+
+struct A {
+  __attribute__((aligned(16))) double data1;
+};
+
+struct B : public virtual A {
+  B() : x(123) {}
+  double a;
+  int x;
+};
+
+struct C : public virtual B {};
+
+void test() { B b; C c; }
+
+}
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -615,7 +615,14 @@
 
   llvm::Value *ThisPtr = CGF.LoadCXXThis();
   QualType RecordTy = CGF.getContext().getTypeDeclType(ClassDecl);
-  LValue LHS = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy);
+  LValue LHS;
+
+  // If a base constructor is being emitted, create an LValue that has the
+  // non-virtual alignment.
+  if (CGF.CurGD.getCtorType() == Ctor_Base)
+LHS = CGF.MakeNaturalAlignPointeeAddrLValue(ThisPtr, RecordTy);
+  else
+LHS = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy);
 
   EmitLValueForAnyFieldInitialization(CGF, MemberInit, LHS);
 


Index: test/CodeGenCXX/virtual-bases.cpp
===
--- test/CodeGenCXX/virtual-bases.cpp
+++ test/CodeGenCXX/virtual-bases.cpp
@@ -46,3 +46,37 @@
 D::D() { }
 
 }
+
+namespace virtualBaseAlignment {
+
+// Check that the store to B::x in the base constructor has an 8-byte alignment.
+
+// CHECK: define linkonce_odr void @_ZN20virtualBaseAlignment1BC1Ev(%[[STRUCT_B:.*]]* %[[THIS:.*]])
+// CHECK: %[[THIS_ADDR:.*]] = alloca %[[STRUCT_B]]*, align 8
+// CHECK: store %[[STRUCT_B]]* %[[THIS]], %[[STRUCT_B]]** %[[THIS_ADDR]], align 8
+// CHECK: %[[THIS1:.*]] = load %[[STRUCT_B]]*, %[[STRUCT_B]]** %[[THIS_ADDR]], align 8
+// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_B]], %[[STRUCT_B]]* %[[THIS1]], i32 0, i32 2
+// CHECK: store i32 123, i32* %[[X]], align 16
+
+// CHECK: define linkonce_odr void @_ZN20virtualBaseAlignment1BC2Ev(%[[STRUCT_B]]* %[[THIS:.*]], i8** %{{.*}})
+// CHECK: %[[THIS_ADDR:.*]] = alloca %[[STRUCT_B]]*, align 8
+// CHECK: store %[[STRUCT_B]]* %[[THIS]], %[[STRUCT_B]]** %[[THIS_ADDR]], align 8
+// CHECK: %[[THIS1:.*]] = load %[[STRUCT_B]]*, %[[STRUCT_B]]** %[[THIS_ADDR]], align 8
+// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_B]], %[[STRUCT_B]]* %[[THIS1]], i32 0, i32 2
+// CHECK: store i32 123, i32* %[[X]], align 8
+
+struct A {
+  __attribute__((aligned(16))) double data1;
+};
+
+struct B : public virtual A {
+  B() : x(123) {}
+  double a;
+  int x;
+};
+
+struct C : public virtual B {};
+
+void test() { B b; C c; }
+
+}
Index: lib/CodeGen/CGClass.cpp