Hi rsmith, majnemer,

This affects both the Itanium and Microsoft C++ ABIs.

Our C++ ABI code was checking if the object had a non-trivial copy ctor
or non-trivial dtor.  Now we check if there are any trivial, non-deleted
copy or move ctors before passing an argument in registers.

On x86 Windows, we can mostly use the same logic, where we use inalloca
instead of passing by address.  However, on Win64, there are register
parameters, and we have to do what MSVC does.  MSVC ignores the presence
of non-trivial move constructors and only considers the presence of
non-trivial or deleted copy constructors.  If a non-trivial or deleted
copy ctor is present, it passes the argument indirectly.

This change fixes bugs and makes us more ABI compatible with both GCC
and MSVC.

Fixes PR19668.

http://reviews.llvm.org/D3660

Files:
  lib/CodeGen/CGCXXABI.cpp
  lib/CodeGen/CGCXXABI.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  test/CodeGenCXX/uncopyable-args.cpp
Index: lib/CodeGen/CGCXXABI.cpp
===================================================================
--- lib/CodeGen/CGCXXABI.cpp
+++ lib/CodeGen/CGCXXABI.cpp
@@ -28,6 +28,29 @@
     << S;
 }
 
+bool CGCXXABI::canCopyArgument(const CXXRecordDecl *RD) const {
+  // If RD has a non-trivial move or copy constructor, we cannot copy the
+  // argument.
+  if (RD->hasNonTrivialCopyConstructor() || RD->hasNonTrivialMoveConstructor())
+    return false;
+
+  // We can only copy the argument if there exists at least one trivial,
+  // non-deleted copy or move constructor.
+  bool AnyDeleted = false;
+  bool AllDeleted = true;
+  for (const CXXConstructorDecl *CD : RD->ctors()) {
+    if (CD->isCopyConstructor() || CD->isMoveConstructor()) {
+      assert(CD->isTrivial());
+      AnyDeleted |= CD->isDeleted();
+      AllDeleted &= CD->isDeleted();
+    }
+  }
+
+  // If all trivial copy and move constructors are deleted, we cannot copy the
+  // argument.
+  return !(AnyDeleted && AllDeleted);
+}
+
 llvm::Constant *CGCXXABI::GetBogusMemberPointer(QualType T) {
   return llvm::Constant::getNullValue(CGM.getTypes().ConvertType(T));
 }
Index: lib/CodeGen/CGCXXABI.h
===================================================================
--- lib/CodeGen/CGCXXABI.h
+++ lib/CodeGen/CGCXXABI.h
@@ -111,6 +111,10 @@
     RAA_Indirect
   };
 
+  /// Returns true if C++ allows us to copy the memory of an object of type RD
+  /// when it is passed as an argument.
+  bool canCopyArgument(const CXXRecordDecl *RD) const;
+
   /// Returns how an argument of the given record type should be passed.
   virtual RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const = 0;
 
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -59,9 +59,11 @@
   }
 
   RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const override {
-    // Structures with either a non-trivial destructor or a non-trivial
-    // copy constructor are always indirect.
-    if (!RD->hasTrivialDestructor() || RD->hasNonTrivialCopyConstructor())
+    // If C++ prohibits us from making a copy, pass by address.
+    if (!canCopyArgument(RD))
+      return RAA_Indirect;
+    // If the class has a destructor, the ABI requires us to pass by address.
+    if (RD->hasNonTrivialDestructor())
       return RAA_Indirect;
     return RAA_Default;
   }
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===================================================================
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -405,20 +405,37 @@
     return RAA_Default;
 
   case llvm::Triple::x86:
-    // 32-bit x86 constructs non-trivial objects directly in outgoing argument
-    // slots.  LLVM uses the inalloca attribute to implement this.
-    if (RD->hasNonTrivialCopyConstructor() || RD->hasNonTrivialDestructor())
+    // All record arguments are passed in memory on x86.  Decide whether to
+    // construct the object directly in argument memory, or to construct the
+    // argument elsewhere and copy the bytes during the call.
+
+    // If C++ prohibits us from making a copy, construct the arguments directly
+    // into argument memory.
+    if (!canCopyArgument(RD))
+      return RAA_DirectInMemory;
+
+    // If the class has a destructor, construct the argument directly in memory
+    // anyway to eliminate an extra destructor call.
+    if (RD->hasNonTrivialDestructor())
       return RAA_DirectInMemory;
+
+    // Otherwise, construct the argument into a temporary and copy the bytes
+    // into the outgoing argument memory.
     return RAA_Default;
 
   case llvm::Triple::x86_64:
-    // Win64 passes objects with non-trivial copy ctors indirectly.
-    if (RD->hasNonTrivialCopyConstructor())
-      return RAA_Indirect;
     // Win64 passes objects larger than 8 bytes indirectly.
     if (getContext().getTypeSize(RD->getTypeForDecl()) > 64)
       return RAA_Indirect;
-    return RAA_Default;
+
+    // MSVC passes objects with a trivial, non-deleted copy ctor directly.
+    for (const CXXConstructorDecl *CD : RD->ctors()) {
+      if (CD->isCopyConstructor() && CD->isTrivial() && !CD->isDeleted())
+        return RAA_Default;
+    }
+
+    // Otherwise, it has an interesting copy ctor, so pass it indirectly.
+    return RAA_Indirect;
   }
 
   llvm_unreachable("invalid enum");
Index: test/CodeGenCXX/uncopyable-args.cpp
===================================================================
--- /dev/null
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -0,0 +1,140 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s | FileCheck %s -check-prefix=WIN64
+
+namespace trivial {
+// Trivial structs should be passed directly.
+struct A {
+  A();
+  void *p;
+};
+void foo(A);
+void bar() {
+  foo({});
+}
+// CHECK-LABEL: define void @_ZN7trivial3barEv()
+// CHECK: alloca %"struct.trivial::A"
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK: load i8**
+// CHECK: call void @_ZN7trivial3fooENS_1AE(i8* %{{.*}})
+// CHECK-LABEL: declare void @_ZN7trivial3fooENS_1AE(i8*)
+
+// WIN64-LABEL: declare void @"\01?foo@trivial@@YAXUA@1@@Z"(i64)
+}
+
+namespace move_ctor {
+// The presence of a move constructor means we have to pass this struct by
+// address.
+struct A {
+  A();
+  A(A &&o);
+  void *p;
+};
+void foo(A);
+void bar() {
+  foo({});
+}
+// CHECK-LABEL: define void @_ZN9move_ctor3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+
+// WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
+}
+
+namespace all_deleted {
+struct A {
+  A();
+  A(const A &o) = delete;
+  A(A &&o) = delete;
+  void *p;
+};
+void foo(A);
+void bar() {
+  foo({});
+}
+// CHECK-LABEL: define void @_ZN11all_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT call
+// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+
+// WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
+}
+
+namespace implicitly_deleted {
+struct A {
+  A();
+  A &operator=(A &&o);
+  void *p;
+};
+void foo(A);
+void bar() {
+  foo({});
+}
+// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT call
+// CHECK: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
+
+// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
+}
+
+namespace one_deleted {
+struct A {
+  A();
+  A(A &&o) = delete;
+  void *p;
+};
+void foo(A);
+void bar() {
+  foo({});
+}
+// CHECK-LABEL: define void @_ZN11one_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT call
+// CHECK: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
+
+// WIN64-LABEL: declare void @"\01?foo@one_deleted@@YAXUA@1@@Z"(%"struct.one_deleted::A"*)
+}
+
+namespace one_defaulted {
+struct A {
+  A();
+  A(const A &o) = default;
+  A(A &&o) = delete;
+  void *p;
+};
+void foo(A);
+void bar() {
+  foo({});
+}
+// CHECK-LABEL: define void @_ZN13one_defaulted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK: load i8**
+// CHECK: call void @_ZN13one_defaulted3fooENS_1AE(i8* %{{.*}})
+// CHECK-LABEL: declare void @_ZN13one_defaulted3fooENS_1AE(i8*)
+
+// WIN64-LABEL: declare void @"\01?foo@one_defaulted@@YAXUA@1@@Z"(i64)
+}
+
+namespace trivial_defaulted {
+struct A {
+  A();
+  A(const A &o) = default;
+  void *p;
+};
+void foo(A);
+void bar() {
+  foo({});
+}
+// CHECK-LABEL: define void @_ZN17trivial_defaulted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK: load i8**
+// CHECK: call void @_ZN17trivial_defaulted3fooENS_1AE(i8* %{{.*}})
+// CHECK-LABEL: declare void @_ZN17trivial_defaulted3fooENS_1AE(i8*)
+
+// WIN64-LABEL: declare void @"\01?foo@trivial_defaulted@@YAXUA@1@@Z"(i64)
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to