r208786 - Don't copy objects with trivial, deleted copy ctors
Richard Smith
richard at metafoo.co.uk
Wed May 14 16:15:28 PDT 2014
On 14 May 2014 09:11, "Reid Kleckner" <reid at kleckner.net> wrote:
>
> Author: rnk
> Date: Wed May 14 11:02:09 2014
> New Revision: 208786
>
> URL: http://llvm.org/viewvc/llvm-project?rev=208786&view=rev
> Log:
> Don't copy objects with trivial, deleted copy ctors
>
> This affects both the Itanium and Microsoft C++ ABIs.
>
> This is in anticipation of a change to the Itanium C++ ABI, and should
> match GCC's current behavior. The new text will likely be:
>
> """
> Pass an object of class type by value if every copy constructor and
> move constructor is deleted or trivial and at least one of them is not
> deleted, and the destructor is trivial.
> """
> http://sourcerytools.com/pipermail/cxx-abi-dev/2014-May/002728.html
>
> 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.
>
> Reviewers: rsmith
>
> Differential Revision: http://reviews.llvm.org/D3660
>
> Added:
> cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
> Modified:
> cfe/trunk/lib/CodeGen/CGCXXABI.cpp
> cfe/trunk/lib/CodeGen/CGCXXABI.h
> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
> cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGCXXABI.cpp
> URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.cpp?rev=208786&r1=208785&r2=208786&view=diff
>
==============================================================================
> --- cfe/trunk/lib/CodeGen/CGCXXABI.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGCXXABI.cpp Wed May 14 11:02:09 2014
> @@ -28,6 +28,37 @@ void CGCXXABI::ErrorUnsupportedABI(CodeG
> << 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;
> +
> + // If RD has a non-trivial destructor, we cannot copy the argument.
> + if (RD->hasNonTrivialDestructor())
> + return false;
> +
> + // We can only copy the argument if there exists at least one trivial,
> + // 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;
> + for (const CXXConstructorDecl *CD : RD->ctors()) {
> + if (CD->isCopyConstructor() || CD->isMoveConstructor()) {
> + assert(CD->isTrivial());
> + // We had at least one undeleted trivial copy or move ctor. Return
> + // directly.
> + if (!CD->isDeleted())
> + return true;
> + CopyOrMoveDeleted = true;
> + }
> + }
> +
> + // If all trivial copy and move constructors are deleted, we cannot
copy the
> + // argument.
> + return !CopyOrMoveDeleted;
This returns false if either trivial copy or move is deleted. Shouldn't we
only return false if both of them are?
> +}
> +
> llvm::Constant *CGCXXABI::GetBogusMemberPointer(QualType T) {
> return llvm::Constant::getNullValue(CGM.getTypes().ConvertType(T));
> }
>
> Modified: cfe/trunk/lib/CodeGen/CGCXXABI.h
> URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.h?rev=208786&r1=208785&r2=208786&view=diff
>
==============================================================================
> --- cfe/trunk/lib/CodeGen/CGCXXABI.h (original)
> +++ cfe/trunk/lib/CodeGen/CGCXXABI.h Wed May 14 11:02:09 2014
> @@ -112,6 +112,10 @@ public:
> 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;
>
>
> Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
> URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=208786&r1=208785&r2=208786&view=diff
>
==============================================================================
> --- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
> +++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Wed May 14 11:02:09 2014
> @@ -55,9 +55,8 @@ public:
> bool classifyReturnType(CGFunctionInfo &FI) const override;
>
> 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;
> return RAA_Default;
> }
> @@ -762,13 +761,11 @@ bool ItaniumCXXABI::classifyReturnType(C
> if (!RD)
> return false;
>
> - // Return indirectly if we have a non-trivial copy ctor or non-trivial
dtor.
> - if (RD->hasNonTrivialDestructor() ||
RD->hasNonTrivialCopyConstructor()) {
> + // If C++ prohibits us from making a copy, return by address.
> + if (!canCopyArgument(RD)) {
> FI.getReturnInfo() = ABIArgInfo::getIndirect(0, /*ByVal=*/false);
> return true;
> }
> -
> - // Otherwise, use the normal C ABI rules.
> return false;
> }
>
>
> Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
> URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=208786&r1=208785&r2=208786&view=diff
>
==============================================================================
> --- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)
> +++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Wed May 14 11:02:09 2014
> @@ -404,19 +404,46 @@ MicrosoftCXXABI::getRecordArgABI(const C
> 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;
> +
> + // 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;
> +
> + // We have a trivial copy constructor or no copy constructors, but
we have
> + // to make sure it isn't deleted.
> + bool CopyDeleted = false;
> + for (const CXXConstructorDecl *CD : RD->ctors()) {
> + if (CD->isCopyConstructor()) {
> + assert(CD->isTrivial());
> + // We had at least one undeleted trivial copy ctor. Return
directly.
> + if (!CD->isDeleted())
> + return RAA_Default;
> + CopyDeleted = true;
> + }
> + }
> +
> + // The trivial copy constructor was deleted. Return indirectly.
> + if (CopyDeleted)
> + return RAA_Indirect;
> +
> + // There were no copy ctors. Return in RAX.
> return RAA_Default;
> }
>
>
> Added: cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
> URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp?rev=208786&view=auto
>
==============================================================================
> --- cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp (added)
> +++ cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp Wed May 14 11:02:09 2014
> @@ -0,0 +1,200 @@
> +// 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 {
> + void *p;
> +};
> +void foo(A);
> +void bar() {
> + foo({});
> +}
> +// CHECK-LABEL: define void @_ZN7trivial3barEv()
> +// CHECK: alloca %"struct.trivial::A"
> +// CHECK: load i8**
> +// CHECK: call void @_ZN7trivial3fooENS_1AE(i8* %{{.*}})
> +// CHECK-LABEL: declare void @_ZN7trivial3fooENS_1AE(i8*)
> +
> +// WIN64-LABEL: declare void @"\01?foo at trivial@@YAXUA at 1@@Z"(i64)
> +}
> +
> +namespace default_ctor {
> +struct A {
> + A();
> + void *p;
> +};
> +void foo(A);
> +void bar() {
> + // Core issue 1590. We can pass this type in registers, even though
C++
> + // normally doesn't permit copies when using braced initialization.
> + foo({});
> +}
> +// CHECK-LABEL: define void @_ZN12default_ctor3barEv()
> +// CHECK: alloca %"struct.default_ctor::A"
> +// CHECK: call void @_Z{{.*}}C1Ev(
> +// CHECK: load i8**
> +// CHECK: call void @_ZN12default_ctor3fooENS_1AE(i8* %{{.*}})
> +// CHECK-LABEL: declare void @_ZN12default_ctor3fooENS_1AE(i8*)
> +
> +// WIN64-LABEL: declare void @"\01?foo at default_ctor@@YAXUA at 1@@Z"(i64)
> +}
> +
> +namespace move_ctor {
> +// The presence of a move constructor implicitly deletes the trivial
copy ctor
> +// and means that 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 at move_ctor@@YAXUA at 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 at all_deleted@@YAXUA at 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 at implicitly_deleted@@YAXUA at 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 at one_deleted@@YAXUA at 1
@@Z"(%"struct.one_deleted::A"*)
> +}
> +
> +namespace copy_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 @_ZN14copy_defaulted3barEv()
> +// CHECK: call void @_Z{{.*}}C1Ev(
> +// CHECK: load i8**
> +// CHECK: call void @_ZN14copy_defaulted3fooENS_1AE(i8* %{{.*}})
> +// CHECK-LABEL: declare void @_ZN14copy_defaulted3fooENS_1AE(i8*)
> +
> +// WIN64-LABEL: declare void @"\01?foo at copy_defaulted@@YAXUA at 1@@Z"(i64)
> +}
> +
> +namespace move_defaulted {
> +struct A {
> + A();
> + A(const A &o) = delete;
> + A(A &&o) = default;
> + void *p;
> +};
> +void foo(A);
> +void bar() {
> + foo({});
> +}
> +// CHECK-LABEL: define void @_ZN14move_defaulted3barEv()
> +// CHECK: call void @_Z{{.*}}C1Ev(
> +// CHECK: load i8**
> +// CHECK: call void @_ZN14move_defaulted3fooENS_1AE(i8* %{{.*}})
> +// CHECK-LABEL: declare void @_ZN14move_defaulted3fooENS_1AE(i8*)
> +
> +// WIN64-LABEL: declare void @"\01?foo at move_defaulted@@YAXUA at 1
@@Z"(%"struct.move_defaulted::A"*)
> +}
> +
> +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 at trivial_defaulted@@YAXUA at 1
@@Z"(i64)
> +}
> +
> +namespace two_copy_ctors {
> +struct A {
> + A();
> + A(const A &) = default;
> + A(const A &, int = 0);
> + void *p;
> +};
> +struct B : A {};
> +
> +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"*)
> +
> +// WIN64-LABEL: declare void @"\01?foo at two_copy_ctors@@YAXUB at 1
@@Z"(%"struct.two_copy_ctors::B"*)
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140514/6815e07f/attachment.html>
More information about the cfe-commits
mailing list