<p dir="ltr"><br>
On 14 May 2014 09:11, "Reid Kleckner" <<a href="mailto:reid@kleckner.net">reid@kleckner.net</a>> wrote:<br>
><br>
> Author: rnk<br>
> Date: Wed May 14 11:02:09 2014<br>
> New Revision: 208786<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=208786&view=rev">http://llvm.org/viewvc/llvm-project?rev=208786&view=rev</a><br>
> Log:<br>
> Don't copy objects with trivial, deleted copy ctors<br>
><br>
> This affects both the Itanium and Microsoft C++ ABIs.<br>
><br>
> This is in anticipation of a change to the Itanium C++ ABI, and should<br>
> match GCC's current behavior.  The new text will likely be:<br>
><br>
> """<br>
> Pass an object of class type by value if every copy constructor and<br>
> move constructor is deleted or trivial and at least one of them is not<br>
> deleted, and the destructor is trivial.<br>
> """<br>
> <a href="http://sourcerytools.com/pipermail/cxx-abi-dev/2014-May/002728.html">http://sourcerytools.com/pipermail/cxx-abi-dev/2014-May/002728.html</a><br>
><br>
> On x86 Windows, we can mostly use the same logic, where we use inalloca<br>
> instead of passing by address.  However, on Win64, there are register<br>
> parameters, and we have to do what MSVC does.  MSVC ignores the presence<br>
> of non-trivial move constructors and only considers the presence of<br>
> non-trivial or deleted copy constructors.  If a non-trivial or deleted<br>
> copy ctor is present, it passes the argument indirectly.<br>
><br>
> This change fixes bugs and makes us more ABI compatible with both GCC<br>
> and MSVC.<br>
><br>
> Fixes PR19668.<br>
><br>
> Reviewers: rsmith<br>
><br>
> Differential Revision: <a href="http://reviews.llvm.org/D3660">http://reviews.llvm.org/D3660</a><br>
><br>
> Added:<br>
>     cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp<br>
> Modified:<br>
>     cfe/trunk/lib/CodeGen/CGCXXABI.cpp<br>
>     cfe/trunk/lib/CodeGen/CGCXXABI.h<br>
>     cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp<br>
>     cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp<br>
><br>
> Modified: cfe/trunk/lib/CodeGen/CGCXXABI.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.cpp?rev=208786&r1=208785&r2=208786&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.cpp?rev=208786&r1=208785&r2=208786&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/CodeGen/CGCXXABI.cpp (original)<br>
> +++ cfe/trunk/lib/CodeGen/CGCXXABI.cpp Wed May 14 11:02:09 2014<br>
> @@ -28,6 +28,37 @@ void CGCXXABI::ErrorUnsupportedABI(CodeG<br>
>      << S;<br>
>  }<br>
><br>
> +bool CGCXXABI::canCopyArgument(const CXXRecordDecl *RD) const {<br>
> +  // If RD has a non-trivial move or copy constructor, we cannot copy the<br>
> +  // argument.<br>
> +  if (RD->hasNonTrivialCopyConstructor() || RD->hasNonTrivialMoveConstructor())<br>
> +    return false;<br>
> +<br>
> +  // If RD has a non-trivial destructor, we cannot copy the argument.<br>
> +  if (RD->hasNonTrivialDestructor())<br>
> +    return false;<br>
> +<br>
> +  // We can only copy the argument if there exists at least one trivial,<br>
> +  // non-deleted copy or move constructor.<br>
> +  // FIXME: This assumes that all lazily declared copy and move constructors are<br>
> +  // not deleted.  This assumption might not be true in some corner cases.<br>
> +  bool CopyOrMoveDeleted = false;<br>
> +  for (const CXXConstructorDecl *CD : RD->ctors()) {<br>
> +    if (CD->isCopyConstructor() || CD->isMoveConstructor()) {<br>
> +      assert(CD->isTrivial());<br>
> +      // We had at least one undeleted trivial copy or move ctor.  Return<br>
> +      // directly.<br>
> +      if (!CD->isDeleted())<br>
> +        return true;<br>
> +      CopyOrMoveDeleted = true;<br>
> +    }<br>
> +  }<br>
> +<br>
> +  // If all trivial copy and move constructors are deleted, we cannot copy the<br>
> +  // argument.<br>
> +  return !CopyOrMoveDeleted;</p>
<p dir="ltr">This returns false if either trivial copy or move is deleted. Shouldn't we only return false if both of them are?</p>
<p dir="ltr">> +}<br>
> +<br>
>  llvm::Constant *CGCXXABI::GetBogusMemberPointer(QualType T) {<br>
>    return llvm::Constant::getNullValue(CGM.getTypes().ConvertType(T));<br>
>  }<br>
><br>
> Modified: cfe/trunk/lib/CodeGen/CGCXXABI.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.h?rev=208786&r1=208785&r2=208786&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.h?rev=208786&r1=208785&r2=208786&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/CodeGen/CGCXXABI.h (original)<br>
> +++ cfe/trunk/lib/CodeGen/CGCXXABI.h Wed May 14 11:02:09 2014<br>
> @@ -112,6 +112,10 @@ public:<br>
>      RAA_Indirect<br>
>    };<br>
><br>
> +  /// Returns true if C++ allows us to copy the memory of an object of type RD<br>
> +  /// when it is passed as an argument.<br>
> +  bool canCopyArgument(const CXXRecordDecl *RD) const;<br>
> +<br>
>    /// Returns how an argument of the given record type should be passed.<br>
>    virtual RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const = 0;<br>
><br>
><br>
> Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=208786&r1=208785&r2=208786&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=208786&r1=208785&r2=208786&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)<br>
> +++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Wed May 14 11:02:09 2014<br>
> @@ -55,9 +55,8 @@ public:<br>
>    bool classifyReturnType(CGFunctionInfo &FI) const override;<br>
><br>
>    RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const override {<br>
> -    // Structures with either a non-trivial destructor or a non-trivial<br>
> -    // copy constructor are always indirect.<br>
> -    if (!RD->hasTrivialDestructor() || RD->hasNonTrivialCopyConstructor())<br>
> +    // If C++ prohibits us from making a copy, pass by address.<br>
> +    if (!canCopyArgument(RD))<br>
>        return RAA_Indirect;<br>
>      return RAA_Default;<br>
>    }<br>
> @@ -762,13 +761,11 @@ bool ItaniumCXXABI::classifyReturnType(C<br>
>    if (!RD)<br>
>      return false;<br>
><br>
> -  // Return indirectly if we have a non-trivial copy ctor or non-trivial dtor.<br>
> -  if (RD->hasNonTrivialDestructor() || RD->hasNonTrivialCopyConstructor()) {<br>
> +  // If C++ prohibits us from making a copy, return by address.<br>
> +  if (!canCopyArgument(RD)) {<br>
>      FI.getReturnInfo() = ABIArgInfo::getIndirect(0, /*ByVal=*/false);<br>
>      return true;<br>
>    }<br>
> -<br>
> -  // Otherwise, use the normal C ABI rules.<br>
>    return false;<br>
>  }<br>
><br>
><br>
> Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=208786&r1=208785&r2=208786&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=208786&r1=208785&r2=208786&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)<br>
> +++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Wed May 14 11:02:09 2014<br>
> @@ -404,19 +404,46 @@ MicrosoftCXXABI::getRecordArgABI(const C<br>
>      return RAA_Default;<br>
><br>
>    case llvm::Triple::x86:<br>
> -    // 32-bit x86 constructs non-trivial objects directly in outgoing argument<br>
> -    // slots.  LLVM uses the inalloca attribute to implement this.<br>
> -    if (RD->hasNonTrivialCopyConstructor() || RD->hasNonTrivialDestructor())<br>
> +    // All record arguments are passed in memory on x86.  Decide whether to<br>
> +    // construct the object directly in argument memory, or to construct the<br>
> +    // argument elsewhere and copy the bytes during the call.<br>
> +<br>
> +    // If C++ prohibits us from making a copy, construct the arguments directly<br>
> +    // into argument memory.<br>
> +    if (!canCopyArgument(RD))<br>
>        return RAA_DirectInMemory;<br>
> +<br>
> +    // Otherwise, construct the argument into a temporary and copy the bytes<br>
> +    // into the outgoing argument memory.<br>
>      return RAA_Default;<br>
><br>
>    case llvm::Triple::x86_64:<br>
>      // Win64 passes objects with non-trivial copy ctors indirectly.<br>
>      if (RD->hasNonTrivialCopyConstructor())<br>
>        return RAA_Indirect;<br>
> +<br>
>      // Win64 passes objects larger than 8 bytes indirectly.<br>
>      if (getContext().getTypeSize(RD->getTypeForDecl()) > 64)<br>
>        return RAA_Indirect;<br>
> +<br>
> +    // We have a trivial copy constructor or no copy constructors, but we have<br>
> +    // to make sure it isn't deleted.<br>
> +    bool CopyDeleted = false;<br>
> +    for (const CXXConstructorDecl *CD : RD->ctors()) {<br>
> +      if (CD->isCopyConstructor()) {<br>
> +        assert(CD->isTrivial());<br>
> +        // We had at least one undeleted trivial copy ctor.  Return directly.<br>
> +        if (!CD->isDeleted())<br>
> +          return RAA_Default;<br>
> +        CopyDeleted = true;<br>
> +      }<br>
> +    }<br>
> +<br>
> +    // The trivial copy constructor was deleted.  Return indirectly.<br>
> +    if (CopyDeleted)<br>
> +      return RAA_Indirect;<br>
> +<br>
> +    // There were no copy ctors.  Return in RAX.<br>
>      return RAA_Default;<br>
>    }<br>
><br>
><br>
> Added: cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp?rev=208786&view=auto">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp?rev=208786&view=auto</a><br>

> ==============================================================================<br>
> --- cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp (added)<br>
> +++ cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp Wed May 14 11:02:09 2014<br>
> @@ -0,0 +1,200 @@<br>
> +// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s<br>
> +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -emit-llvm -o - %s | FileCheck %s -check-prefix=WIN64<br>
> +<br>
> +namespace trivial {<br>
> +// Trivial structs should be passed directly.<br>
> +struct A {<br>
> +  void *p;<br>
> +};<br>
> +void foo(A);<br>
> +void bar() {<br>
> +  foo({});<br>
> +}<br>
> +// CHECK-LABEL: define void @_ZN7trivial3barEv()<br>
> +// CHECK: alloca %"struct.trivial::A"<br>
> +// CHECK: load i8**<br>
> +// CHECK: call void @_ZN7trivial3fooENS_1AE(i8* %{{.*}})<br>
> +// CHECK-LABEL: declare void @_ZN7trivial3fooENS_1AE(i8*)<br>
> +<br>
> +// WIN64-LABEL: declare void @"\01?foo@trivial@@YAXUA@1@@Z"(i64)<br>
> +}<br>
> +<br>
> +namespace default_ctor {<br>
> +struct A {<br>
> +  A();<br>
> +  void *p;<br>
> +};<br>
> +void foo(A);<br>
> +void bar() {<br>
> +  // Core issue 1590.  We can pass this type in registers, even though C++<br>
> +  // normally doesn't permit copies when using braced initialization.<br>
> +  foo({});<br>
> +}<br>
> +// CHECK-LABEL: define void @_ZN12default_ctor3barEv()<br>
> +// CHECK: alloca %"struct.default_ctor::A"<br>
> +// CHECK: call void @_Z{{.*}}C1Ev(<br>
> +// CHECK: load i8**<br>
> +// CHECK: call void @_ZN12default_ctor3fooENS_1AE(i8* %{{.*}})<br>
> +// CHECK-LABEL: declare void @_ZN12default_ctor3fooENS_1AE(i8*)<br>
> +<br>
> +// WIN64-LABEL: declare void @"\01?foo@default_ctor@@YAXUA@1@@Z"(i64)<br>
> +}<br>
> +<br>
> +namespace move_ctor {<br>
> +// The presence of a move constructor implicitly deletes the trivial copy ctor<br>
> +// and means that we have to pass this struct by address.<br>
> +struct A {<br>
> +  A();<br>
> +  A(A &&o);<br>
> +  void *p;<br>
> +};<br>
> +void foo(A);<br>
> +void bar() {<br>
> +  foo({});<br>
> +}<br>
> +// CHECK-LABEL: define void @_ZN9move_ctor3barEv()<br>
> +// CHECK: call void @_Z{{.*}}C1Ev(<br>
> +// CHECK-NOT: call<br>
> +// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})<br>
> +// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)<br>
> +<br>
> +// WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)<br>
> +}<br>
> +<br>
> +namespace all_deleted {<br>
> +struct A {<br>
> +  A();<br>
> +  A(const A &o) = delete;<br>
> +  A(A &&o) = delete;<br>
> +  void *p;<br>
> +};<br>
> +void foo(A);<br>
> +void bar() {<br>
> +  foo({});<br>
> +}<br>
> +// CHECK-LABEL: define void @_ZN11all_deleted3barEv()<br>
> +// CHECK: call void @_Z{{.*}}C1Ev(<br>
> +// CHECK-NOT call<br>
> +// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})<br>
> +// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)<br>
> +<br>
> +// WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)<br>
> +}<br>
> +<br>
> +namespace implicitly_deleted {<br>
> +struct A {<br>
> +  A();<br>
> +  A &operator=(A &&o);<br>
> +  void *p;<br>
> +};<br>
> +void foo(A);<br>
> +void bar() {<br>
> +  foo({});<br>
> +}<br>
> +// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()<br>
> +// CHECK: call void @_Z{{.*}}C1Ev(<br>
> +// CHECK-NOT call<br>
> +// CHECK: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})<br>
> +// CHECK-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)<br>
> +<br>
> +// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)<br>
> +}<br>
> +<br>
> +namespace one_deleted {<br>
> +struct A {<br>
> +  A();<br>
> +  A(A &&o) = delete;<br>
> +  void *p;<br>
> +};<br>
> +void foo(A);<br>
> +void bar() {<br>
> +  foo({});<br>
> +}<br>
> +// CHECK-LABEL: define void @_ZN11one_deleted3barEv()<br>
> +// CHECK: call void @_Z{{.*}}C1Ev(<br>
> +// CHECK-NOT call<br>
> +// CHECK: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})<br>
> +// CHECK-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)<br>
> +<br>
> +// WIN64-LABEL: declare void @"\01?foo@one_deleted@@YAXUA@1@@Z"(%"struct.one_deleted::A"*)<br>
> +}<br>
> +<br>
> +namespace copy_defaulted {<br>
> +struct A {<br>
> +  A();<br>
> +  A(const A &o) = default;<br>
> +  A(A &&o) = delete;<br>
> +  void *p;<br>
> +};<br>
> +void foo(A);<br>
> +void bar() {<br>
> +  foo({});<br>
> +}<br>
> +// CHECK-LABEL: define void @_ZN14copy_defaulted3barEv()<br>
> +// CHECK: call void @_Z{{.*}}C1Ev(<br>
> +// CHECK: load i8**<br>
> +// CHECK: call void @_ZN14copy_defaulted3fooENS_1AE(i8* %{{.*}})<br>
> +// CHECK-LABEL: declare void @_ZN14copy_defaulted3fooENS_1AE(i8*)<br>
> +<br>
> +// WIN64-LABEL: declare void @"\01?foo@copy_defaulted@@YAXUA@1@@Z"(i64)<br>
> +}<br>
> +<br>
> +namespace move_defaulted {<br>
> +struct A {<br>
> +  A();<br>
> +  A(const A &o) = delete;<br>
> +  A(A &&o) = default;<br>
> +  void *p;<br>
> +};<br>
> +void foo(A);<br>
> +void bar() {<br>
> +  foo({});<br>
> +}<br>
> +// CHECK-LABEL: define void @_ZN14move_defaulted3barEv()<br>
> +// CHECK: call void @_Z{{.*}}C1Ev(<br>
> +// CHECK: load i8**<br>
> +// CHECK: call void @_ZN14move_defaulted3fooENS_1AE(i8* %{{.*}})<br>
> +// CHECK-LABEL: declare void @_ZN14move_defaulted3fooENS_1AE(i8*)<br>
> +<br>
> +// WIN64-LABEL: declare void @"\01?foo@move_defaulted@@YAXUA@1@@Z"(%"struct.move_defaulted::A"*)<br>
> +}<br>
> +<br>
> +namespace trivial_defaulted {<br>
> +struct A {<br>
> +  A();<br>
> +  A(const A &o) = default;<br>
> +  void *p;<br>
> +};<br>
> +void foo(A);<br>
> +void bar() {<br>
> +  foo({});<br>
> +}<br>
> +// CHECK-LABEL: define void @_ZN17trivial_defaulted3barEv()<br>
> +// CHECK: call void @_Z{{.*}}C1Ev(<br>
> +// CHECK: load i8**<br>
> +// CHECK: call void @_ZN17trivial_defaulted3fooENS_1AE(i8* %{{.*}})<br>
> +// CHECK-LABEL: declare void @_ZN17trivial_defaulted3fooENS_1AE(i8*)<br>
> +<br>
> +// WIN64-LABEL: declare void @"\01?foo@trivial_defaulted@@YAXUA@1@@Z"(i64)<br>
> +}<br>
> +<br>
> +namespace two_copy_ctors {<br>
> +struct A {<br>
> +  A();<br>
> +  A(const A &) = default;<br>
> +  A(const A &, int = 0);<br>
> +  void *p;<br>
> +};<br>
> +struct B : A {};<br>
> +<br>
> +void foo(B);<br>
> +void bar() {<br>
> +  foo({});<br>
> +}<br>
> +// CHECK-LABEL: define void @_ZN14two_copy_ctors3barEv()<br>
> +// CHECK: call void @_Z{{.*}}C1Ev(<br>
> +// CHECK: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})<br>
> +// CHECK-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)<br>
> +<br>
> +// WIN64-LABEL: declare void @"\01?foo@two_copy_ctors@@YAXUB@1@@Z"(%"struct.two_copy_ctors::B"*)<br>
> +}<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</p>