r208836 - Revert Itanium parts of "Don't copy objects with trivial, deleted copy ctors"

Reid Kleckner reid at kleckner.net
Wed May 14 18:26:32 PDT 2014


Author: rnk
Date: Wed May 14 20:26:32 2014
New Revision: 208836

URL: http://llvm.org/viewvc/llvm-project?rev=208836&view=rev
Log:
Revert Itanium parts of "Don't copy objects with trivial, deleted copy ctors"

This undoes half of r208786.

It had problems with lazily declared special members in cases like this:
  struct A {
    A();
    A &operator=(A &&o);
    void *p;
  };
  void foo(A);
  void bar() {
    foo({});
  }

In this case, the copy and move constructors are implicitly deleted.
However, Clang doesn't eagerly declare the copy ctor in the AST, so we
pass the struct in registers.  Furthermore, GCC passes this in registers
even though this class should be uncopyable.

Revert this for now until the dust settles.

Modified:
    cfe/trunk/lib/CodeGen/CGCXXABI.cpp
    cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
    cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp

Modified: cfe/trunk/lib/CodeGen/CGCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.cpp?rev=208836&r1=208835&r2=208836&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCXXABI.cpp Wed May 14 20:26:32 2014
@@ -42,7 +42,8 @@ bool CGCXXABI::canCopyArgument(const CXX
   // non-deleted copy or move constructor.
   // FIXME: This assumes that all lazily declared copy and move constructors are
   // not deleted.  This assumption might not be true in some corner cases.
-  bool CopyOrMoveDeleted = false;
+  bool CopyDeleted = false;
+  bool MoveDeleted = false;
   for (const CXXConstructorDecl *CD : RD->ctors()) {
     if (CD->isCopyConstructor() || CD->isMoveConstructor()) {
       assert(CD->isTrivial());
@@ -50,13 +51,16 @@ bool CGCXXABI::canCopyArgument(const CXX
       // directly.
       if (!CD->isDeleted())
         return true;
-      CopyOrMoveDeleted = true;
+      if (CD->isCopyConstructor())
+        CopyDeleted = true;
+      else
+        MoveDeleted = true;
     }
   }
 
   // If all trivial copy and move constructors are deleted, we cannot copy the
   // argument.
-  return !CopyOrMoveDeleted;
+  return !(CopyDeleted && MoveDeleted);
 }
 
 llvm::Constant *CGCXXABI::GetBogusMemberPointer(QualType T) {

Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=208836&r1=208835&r2=208836&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Wed May 14 20:26:32 2014
@@ -55,8 +55,11 @@ public:
   bool classifyReturnType(CGFunctionInfo &FI) const override;
 
   RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const override {
-    // If C++ prohibits us from making a copy, pass by address.
-    if (!canCopyArgument(RD))
+    // Structures with either a non-trivial destructor or a non-trivial
+    // copy constructor are always indirect.
+    // FIXME: Use canCopyArgument() when it is fixed to handle lazily declared
+    // special members.
+    if (RD->hasNonTrivialDestructor() || RD->hasNonTrivialCopyConstructor())
       return RAA_Indirect;
     return RAA_Default;
   }
@@ -761,8 +764,10 @@ bool ItaniumCXXABI::classifyReturnType(C
   if (!RD)
     return false;
 
-  // If C++ prohibits us from making a copy, return by address.
-  if (!canCopyArgument(RD)) {
+  // Return indirectly if we have a non-trivial copy ctor or non-trivial dtor.
+  // FIXME: Use canCopyArgument() when it is fixed to handle lazily declared
+  // special members.
+  if (RD->hasNonTrivialDestructor() || RD->hasNonTrivialCopyConstructor()) {
     FI.getReturnInfo() = ABIArgInfo::getIndirect(0, /*ByVal=*/false);
     return true;
   }

Modified: cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp?rev=208836&r1=208835&r2=208836&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp Wed May 14 20:26:32 2014
@@ -52,11 +52,12 @@ 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"*)
+// FIXME: The copy ctor is implicitly deleted.
+// CHECK-DISABLED-LABEL: define void @_ZN9move_ctor3barEv()
+// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
+// CHECK-DISABLED-NOT: call
+// CHECK-DISABLED: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// CHECK-DISABLED-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
 
 // WIN64-LABEL: declare void @"\01?foo at move_ctor@@YAXUA at 1@@Z"(%"struct.move_ctor::A"*)
 }
@@ -72,11 +73,12 @@ 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"*)
+// FIXME: The copy ctor is deleted.
+// CHECK-DISABLED-LABEL: define void @_ZN11all_deleted3barEv()
+// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
+// CHECK-DISABLED-NOT call
+// CHECK-DISABLED: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// CHECK-DISABLED-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
 
 // WIN64-LABEL: declare void @"\01?foo at all_deleted@@YAXUA at 1@@Z"(%"struct.all_deleted::A"*)
 }
@@ -91,11 +93,12 @@ 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"*)
+// FIXME: The copy and move ctors are implicitly deleted.
+// CHECK-DISABLED-LABEL: define void @_ZN18implicitly_deleted3barEv()
+// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
+// CHECK-DISABLED-NOT call
+// CHECK-DISABLED: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
+// CHECK-DISABLED-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
 
 // WIN64-LABEL: declare void @"\01?foo at implicitly_deleted@@YAXUA at 1@@Z"(%"struct.implicitly_deleted::A"*)
 }
@@ -110,11 +113,12 @@ 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"*)
+// FIXME: The copy constructor is implicitly deleted.
+// CHECK-DISABLED-LABEL: define void @_ZN11one_deleted3barEv()
+// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
+// CHECK-DISABLED-NOT call
+// CHECK-DISABLED: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
+// CHECK-DISABLED-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
 
 // WIN64-LABEL: declare void @"\01?foo at one_deleted@@YAXUA at 1@@Z"(%"struct.one_deleted::A"*)
 }
@@ -191,10 +195,12 @@ void foo(B);
 void bar() {
   foo({});
 }
-// CHECK-LABEL: define void @_ZN14two_copy_ctors3barEv()
-// CHECK: call void @_Z{{.*}}C1Ev(
-// CHECK: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
-// CHECK-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
+// FIXME: This class has a non-trivial copy ctor and a trivial copy ctor.  It's
+// not clear whether we should pass by address or in registers.
+// CHECK-DISABLED-LABEL: define void @_ZN14two_copy_ctors3barEv()
+// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
+// CHECK-DISABLED: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
+// CHECK-DISABLED-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
 
 // WIN64-LABEL: declare void @"\01?foo at two_copy_ctors@@YAXUB at 1@@Z"(%"struct.two_copy_ctors::B"*)
 }





More information about the cfe-commits mailing list