From 57ee29a2a1fc33add3e1736614e6e666c432a840 Mon Sep 17 00:00:00 2001 From: Henrich Lauko Date: Thu, 2 Apr 2026 12:31:53 +0200 Subject: [PATCH] [CIR] Implement isMemcpyEquivalentSpecialMember for trivial copy/move ctors (#186700) Implements isMemcpyEquivalentSpecialMember in CIR codegen so that trivial copy/move constructors and defaulted union copy/move ops emit a cir.copy directly instead of making a real constructor call. The logic is shared with OG codegen by moving the implementation into ASTContext, where it also gains the pointer field protection (PFP) check that was previously missing in CIR. --- clang/include/clang/AST/DeclCXX.h | 13 ++++++ clang/include/clang/CIR/MissingFeatures.h | 1 - clang/lib/AST/DeclCXX.cpp | 34 +++++++++++++++ clang/lib/CIR/CodeGen/CIRGenClass.cpp | 23 +++++++---- clang/lib/CodeGen/CGClass.cpp | 40 ++++-------------- .../CIR/CodeGen/copy-constructor-memcpy.cpp | 41 +++++++++++++++++++ clang/test/CIR/CodeGen/coro-task.cpp | 1 - .../CIR/CodeGen/cxx-special-member-attr.cpp | 11 ++--- clang/test/CIR/CodeGen/nrvo.cpp | 2 +- .../combined-firstprivate-clause.cpp | 6 ++- .../compute-firstprivate-clause-templates.cpp | 2 + .../compute-firstprivate-clause.cpp | 6 ++- 12 files changed, 130 insertions(+), 50 deletions(-) create mode 100644 clang/test/CIR/CodeGen/copy-constructor-memcpy.cpp diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 48fd12efdcaf..2af396f025c9 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -2229,6 +2229,19 @@ public: /// Determine whether this is a move assignment operator. bool isMoveAssignmentOperator() const; + /// Determine whether this is a copy or move constructor or a copy or move + /// assignment operator. + bool isCopyOrMoveConstructorOrAssignment() const; + + /// Determine whether this is a copy or move constructor. Always returns + /// false for non-constructor methods; see also + /// CXXConstructorDecl::isCopyOrMoveConstructor(). + bool isCopyOrMoveConstructor() const; + + /// Returns whether this is a copy/move constructor or assignment operator + /// that can be implemented as a memcpy of the object representation. + bool isMemcpyEquivalentSpecialMember(const ASTContext &Ctx) const; + CXXMethodDecl *getCanonicalDecl() override { return cast(FunctionDecl::getCanonicalDecl()); } diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h index 9110ed7cd82f..b9a6b83daa13 100644 --- a/clang/include/clang/CIR/MissingFeatures.h +++ b/clang/include/clang/CIR/MissingFeatures.h @@ -288,7 +288,6 @@ struct MissingFeatures { static bool instrumentation() { return false; } static bool intrinsicElementTypeSupport() { return false; } static bool intrinsics() { return false; } - static bool isMemcpyEquivalentSpecialMember() { return false; } static bool isTrivialCtorOrDtor() { return false; } static bool lambdaCaptures() { return false; } static bool loopInfoStack() { return false; } diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index 083c53e28cb9..3b9d888bb2c0 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -2770,6 +2770,40 @@ bool CXXMethodDecl::isMoveAssignmentOperator() const { return Context.hasSameUnqualifiedType(ClassType, ParamType); } +bool CXXMethodDecl::isCopyOrMoveConstructor() const { + if (const auto *Ctor = dyn_cast(this)) + return Ctor->isCopyOrMoveConstructor(); + return false; +} + +bool CXXMethodDecl::isCopyOrMoveConstructorOrAssignment() const { + return isCopyOrMoveConstructor() || isCopyAssignmentOperator() || + isMoveAssignmentOperator(); +} + +bool CXXMethodDecl::isMemcpyEquivalentSpecialMember( + const ASTContext &Ctx) const { + if (!isCopyOrMoveConstructorOrAssignment()) + return false; + + // Non-trivially-copyable fields with pointer field protection need to be + // copied one by one. + const CXXRecordDecl *Parent = getParent(); + if (!Ctx.arePFPFieldsTriviallyCopyable(Parent) && + Ctx.hasPFPFields(Ctx.getCanonicalTagType(Parent))) + return false; + + // We can emit a memcpy for a trivial copy or move constructor/assignment. + if (isTrivial() && !Parent->mayInsertExtraPadding()) + return true; + + // We *must* emit a memcpy for a defaulted union copy or move op. + if (Parent->isUnion() && isDefaulted()) + return true; + + return false; +} + void CXXMethodDecl::addOverriddenMethod(const CXXMethodDecl *MD) { assert(MD->isCanonicalDecl() && "Method is not canonical!"); assert(MD->isVirtual() && "Method is not virtual!"); diff --git a/clang/lib/CIR/CodeGen/CIRGenClass.cpp b/clang/lib/CIR/CodeGen/CIRGenClass.cpp index 3eead9a62317..e54514950e00 100644 --- a/clang/lib/CIR/CodeGen/CIRGenClass.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenClass.cpp @@ -1306,19 +1306,27 @@ void CIRGenFunction::emitCXXConstructorCall(const clang::CXXConstructorDecl *d, bool delegating, AggValueSlot thisAVS, const clang::CXXConstructExpr *e) { - CallArgList args; Address thisAddr = thisAVS.getAddress(); QualType thisType = d->getThisType(); mlir::Value thisPtr = thisAddr.getPointer(); assert(!cir::MissingFeatures::addressSpace()); - args.add(RValue::get(thisPtr), thisType); + // If this is a trivial constructor, just emit what's needed. If this is a + // union copy constructor, we must emit a memcpy, because the AST does not + // model that copy. + if (d->isMemcpyEquivalentSpecialMember(getContext())) { + assert(e->getNumArgs() == 1 && "unexpected argcount for trivial ctor"); + const Expr *arg = e->getArg(0); + LValue src = emitLValue(arg); + CanQualType destTy = getContext().getCanonicalTagType(d->getParent()); + LValue dest = makeAddrLValue(thisAddr, destTy); + emitAggregateCopy(dest, src, src.getType(), thisAVS.mayOverlap()); + return; + } - // In LLVM Codegen: If this is a trivial constructor, just emit what's needed. - // If this is a union copy constructor, we must emit a memcpy, because the AST - // does not model that copy. - assert(!cir::MissingFeatures::isMemcpyEquivalentSpecialMember()); + CallArgList args; + args.add(RValue::get(thisPtr), thisType); const FunctionProtoType *fpt = d->getType()->castAs(); @@ -1344,7 +1352,8 @@ void CIRGenFunction::emitCXXConstructorCall( // ctor call into trivial initialization. assert(!cir::MissingFeatures::isTrivialCtorOrDtor()); - assert(!cir::MissingFeatures::isMemcpyEquivalentSpecialMember()); + // Note: memcpy-equivalent special members are handled in the + // emitCXXConstructorCall overload that takes a CXXConstructExpr. bool passPrototypeArgs = true; diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp index 6572009fc600..8cb524772d31 100644 --- a/clang/lib/CodeGen/CGClass.cpp +++ b/clang/lib/CodeGen/CGClass.cpp @@ -570,32 +570,6 @@ static void EmitBaseInitializer(CodeGenFunction &CGF, isBaseVirtual); } -static bool isMemcpyEquivalentSpecialMember(CodeGenModule &CGM, - const CXXMethodDecl *D) { - auto *CD = dyn_cast(D); - if (!(CD && CD->isCopyOrMoveConstructor()) && - !D->isCopyAssignmentOperator() && !D->isMoveAssignmentOperator()) - return false; - - // Non-trivially-copyable fields with pointer field protection need to be - // copied one by one. - ASTContext &Ctx = CGM.getContext(); - const CXXRecordDecl *Parent = D->getParent(); - if (!Ctx.arePFPFieldsTriviallyCopyable(Parent) && - Ctx.hasPFPFields(Ctx.getCanonicalTagType(Parent))) - return false; - - // We can emit a memcpy for a trivial copy or move constructor/assignment. - if (D->isTrivial() && !D->getParent()->mayInsertExtraPadding()) - return true; - - // We *must* emit a memcpy for a defaulted union copy or move op. - if (D->getParent()->isUnion() && D->isDefaulted()) - return true; - - return false; -} - static void EmitLValueForAnyFieldInitialization(CodeGenFunction &CGF, CXXCtorInitializer *MemberInit, LValue &LHS) { @@ -650,8 +624,8 @@ static void EmitMemberInitializer(CodeGenFunction &CGF, QualType BaseElementTy = CGF.getContext().getBaseElementType(Array); CXXConstructExpr *CE = dyn_cast(MemberInit->getInit()); if (BaseElementTy.isPODType(CGF.getContext()) || - (CE && - isMemcpyEquivalentSpecialMember(CGF.CGM, CE->getConstructor()))) { + (CE && CE->getConstructor()->isMemcpyEquivalentSpecialMember( + CGF.getContext()))) { unsigned SrcArgIndex = CGF.CGM.getCXXABI().getSrcArgforCopyCtor(Constructor, Args); llvm::Value *SrcPtr = @@ -1059,8 +1033,8 @@ private: CXXConstructExpr *CE = dyn_cast(MemberInit->getInit()); // Bail out on non-memcpyable, not-trivially-copyable members. - if (!(CE && - isMemcpyEquivalentSpecialMember(CGF.CGM, CE->getConstructor())) && + if (!(CE && CE->getConstructor()->isMemcpyEquivalentSpecialMember( + CGF.getContext())) && !(FieldType.isTriviallyCopyableType(CGF.getContext()) || FieldType->isReferenceType())) return false; @@ -1169,7 +1143,7 @@ private: return nullptr; } else if (CXXMemberCallExpr *MCE = dyn_cast(S)) { CXXMethodDecl *MD = dyn_cast(MCE->getCalleeDecl()); - if (!(MD && isMemcpyEquivalentSpecialMember(CGF.CGM, MD))) + if (!(MD && MD->isMemcpyEquivalentSpecialMember(CGF.getContext()))) return nullptr; MemberExpr *IOA = dyn_cast(MCE->getImplicitObjectArgument()); if (!IOA) @@ -2304,7 +2278,7 @@ void CodeGenFunction::EmitCXXConstructorCall( // If this is a trivial constructor, emit a memcpy now before we lose // the alignment information on the argument. // FIXME: It would be better to preserve alignment information into CallArg. - if (isMemcpyEquivalentSpecialMember(CGM, D)) { + if (D->isMemcpyEquivalentSpecialMember(getContext())) { assert(E->getNumArgs() == 1 && "unexpected argcount for trivial ctor"); const Expr *Arg = E->getArg(0); @@ -2372,7 +2346,7 @@ void CodeGenFunction::EmitCXXConstructorCall( // If this is a trivial constructor, just emit what's needed. If this is a // union copy constructor, we must emit a memcpy, because the AST does not // model that copy. - if (isMemcpyEquivalentSpecialMember(CGM, D)) { + if (D->isMemcpyEquivalentSpecialMember(getContext())) { assert(Args.size() == 2 && "unexpected argcount for trivial ctor"); QualType SrcTy = D->getParamDecl(0)->getType().getNonReferenceType(); Address Src = makeNaturalAddressForPointer( diff --git a/clang/test/CIR/CodeGen/copy-constructor-memcpy.cpp b/clang/test/CIR/CodeGen/copy-constructor-memcpy.cpp new file mode 100644 index 000000000000..d7b141cd91bc --- /dev/null +++ b/clang/test/CIR/CodeGen/copy-constructor-memcpy.cpp @@ -0,0 +1,41 @@ +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu \ +// RUN: -fclangir -emit-cir %s -o %t.cir +// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu \ +// RUN: -fclangir -emit-llvm %s -o %t.ll +// RUN: FileCheck --check-prefix=LLVM --input-file=%t.ll %s +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu \ +// RUN: -emit-llvm %s -o %t.og.ll +// RUN: FileCheck --check-prefix=OGCG --input-file=%t.og.ll %s + +// Test that trivial copy constructors are inlined as aggregate copies +// (memcpy-equivalent special members) rather than emitted as function calls. + +struct S { + int a; + int b; +}; + +struct W { + S s; + W(const S &src) : s(src) {} +}; + +void test(const S &src) { + W w(src); +} + +// The copy of S in W's constructor should be inlined as cir.copy, +// not a call to S's copy constructor. + +// CIR-LABEL: cir.func{{.*}} @_ZN1WC2ERK1S +// CIR-NOT: cir.call @_ZN1SC +// CIR: cir.copy %{{.+}} to %{{.+}} : !cir.ptr + +// Both CIR-lowered LLVM and OG produce memcpy for the inlined copy. + +// LLVM-LABEL: define{{.*}} void @_ZN1WC2ERK1S +// LLVM: call void @llvm.memcpy.p0.p0.i64({{.*}}i64 8 + +// OGCG-LABEL: define{{.*}} void @_ZN1WC2ERK1S +// OGCG: call void @llvm.memcpy.p0.p0.i64({{.*}}i64 8 diff --git a/clang/test/CIR/CodeGen/coro-task.cpp b/clang/test/CIR/CodeGen/coro-task.cpp index b52f0f187107..568dedf2c792 100644 --- a/clang/test/CIR/CodeGen/coro-task.cpp +++ b/clang/test/CIR/CodeGen/coro-task.cpp @@ -430,7 +430,6 @@ folly::coro::Task yield1() { // yield_value + await(yield) // CIR: %[[YIELD_TASK:.*]] = cir.call @_Z5yieldv(){{.*}} // CIR: cir.store{{.*}} %[[YIELD_TASK]], %[[T_ADDR]] -// CIR: cir.copy %[[T_ADDR]] to %[[AWAITER_COPY_ADDR]] // CIR: %[[AWAITER:.*]] = cir.load{{.*}} %[[AWAITER_COPY_ADDR]] // CIR: %[[YIELD_SUSP:.*]] = cir.call @_ZN5folly4coro4TaskIvE12promise_type11yield_valueES2_(%[[PROMISE]], %[[AWAITER]]){{.*}} // CIR: cir.store{{.*}} %[[YIELD_SUSP]], %[[SUSP1]] diff --git a/clang/test/CIR/CodeGen/cxx-special-member-attr.cpp b/clang/test/CIR/CodeGen/cxx-special-member-attr.cpp index a68d8a5d4820..bfca59db44fd 100644 --- a/clang/test/CIR/CodeGen/cxx-special-member-attr.cpp +++ b/clang/test/CIR/CodeGen/cxx-special-member-attr.cpp @@ -30,21 +30,22 @@ struct Foo { ~Foo(); }; +// Trivial copy/move assignment operator definitions appear at module level. +// CIR: @_ZN4FlubaSERKS_(%arg0: !cir.ptr {{[{][^}]*[}]}} loc({{.*}}), %arg1: !cir.ptr {{[{][^}]*[}]}} loc({{.*}})) -> (!cir.ptr{{.*}}) special_member<#cir.cxx_assign> +// CIR: @_ZN4FlubaSEOS_(%arg0: !cir.ptr {{[{][^}]*[}]}} loc({{.*}}), %arg1: !cir.ptr {{[{][^}]*[}]}} loc({{.*}})) -> (!cir.ptr{{.*}}) special_member<#cir.cxx_assign> + void trivial_func() { Flub f1{}; Flub f2 = f1; - // Trivial copy constructors/assignments are replaced with cir.copy + // Trivial copy/move constructors are inlined as cir.copy // CIR: cir.copy {{.*}} : !cir.ptr Flub f3 = static_cast(f1); - // CIR: @_ZN4FlubC1EOS_(%arg0: !cir.ptr {{[{][^}]*[}]}} loc({{.*}}), %arg1: !cir.ptr {{[{][^}]*[}]}} loc({{.*}})) special_member<#cir.cxx_ctor + // CIR: cir.copy {{.*}} : !cir.ptr f2 = f1; - // CIR: @_ZN4FlubaSERKS_(%arg0: !cir.ptr {{[{][^}]*[}]}} loc({{.*}}), %arg1: !cir.ptr {{[{][^}]*[}]}} loc({{.*}})) -> (!cir.ptr{{.*}}) special_member<#cir.cxx_assign> - f1 = static_cast(f3); - // CIR: @_ZN4FlubaSEOS_(%arg0: !cir.ptr {{[{][^}]*[}]}} loc({{.*}}), %arg1: !cir.ptr {{[{][^}]*[}]}} loc({{.*}})) -> (!cir.ptr{{.*}}) special_member<#cir.cxx_assign> } void non_trivial_func() { diff --git a/clang/test/CIR/CodeGen/nrvo.cpp b/clang/test/CIR/CodeGen/nrvo.cpp index a3af2d1ff032..08aad0723d33 100644 --- a/clang/test/CIR/CodeGen/nrvo.cpp +++ b/clang/test/CIR/CodeGen/nrvo.cpp @@ -32,7 +32,7 @@ struct S f1() { // CIR-NOELIDE-NEXT: %[[RETVAL:.*]] = cir.alloca !rec_S, !cir.ptr, ["__retval"] // CIR-NOELIDE-NEXT: %[[S:.*]] = cir.alloca !rec_S, !cir.ptr, ["s", init] // CIR-NOELIDE-NEXT: cir.call @_ZN1SC1Ev(%[[S]]) : (!cir.ptr {{.*}}) -> () -// CIR-NOELIDE-NEXT: cir.call @_ZN1SC1EOS_(%[[RETVAL]], %[[S]]){{.*}} : (!cir.ptr {{.*}}, !cir.ptr {{.*}}) -> () +// CIR-NOELIDE-NEXT: cir.copy %[[S]] to %[[RETVAL]] : !cir.ptr // CIR-NOELIDE-NEXT: %[[RET:.*]] = cir.load %[[RETVAL]] : !cir.ptr, !rec_S // CIR-NOELIDE-NEXT: cir.return %[[RET]] diff --git a/clang/test/CIR/CodeGenOpenACC/combined-firstprivate-clause.cpp b/clang/test/CIR/CodeGenOpenACC/combined-firstprivate-clause.cpp index 0a67314a91b0..a75c2fcab14e 100644 --- a/clang/test/CIR/CodeGenOpenACC/combined-firstprivate-clause.cpp +++ b/clang/test/CIR/CodeGenOpenACC/combined-firstprivate-clause.cpp @@ -1,6 +1,8 @@ // RUN: %clang_cc1 -fopenacc -triple x86_64-linux-gnu -Wno-openacc-self-if-potential-conflict -emit-cir -fclangir -triple x86_64-linux-pc %s -o - | FileCheck %s -struct NoCopyConstruct {}; +struct NoCopyConstruct { + int x; +}; struct CopyConstruct { CopyConstruct() = default; @@ -9,10 +11,12 @@ struct CopyConstruct { struct NonDefaultCtor { NonDefaultCtor(); + int x; }; struct HasDtor { ~HasDtor(); + int x; }; // CHECK: acc.firstprivate.recipe @firstprivatization__ZTSi : !cir.ptr init { diff --git a/clang/test/CIR/CodeGenOpenACC/compute-firstprivate-clause-templates.cpp b/clang/test/CIR/CodeGenOpenACC/compute-firstprivate-clause-templates.cpp index 4592d42d3763..853acad55f33 100644 --- a/clang/test/CIR/CodeGenOpenACC/compute-firstprivate-clause-templates.cpp +++ b/clang/test/CIR/CodeGenOpenACC/compute-firstprivate-clause-templates.cpp @@ -7,10 +7,12 @@ struct CopyConstruct { struct NonDefaultCtor { NonDefaultCtor(); + int x; }; struct HasDtor { ~HasDtor(); + int x; }; diff --git a/clang/test/CIR/CodeGenOpenACC/compute-firstprivate-clause.cpp b/clang/test/CIR/CodeGenOpenACC/compute-firstprivate-clause.cpp index c4cec3023e04..79966a39b339 100644 --- a/clang/test/CIR/CodeGenOpenACC/compute-firstprivate-clause.cpp +++ b/clang/test/CIR/CodeGenOpenACC/compute-firstprivate-clause.cpp @@ -1,7 +1,9 @@ // RUN: %clang_cc1 -fopenacc -triple x86_64-linux-gnu -Wno-openacc-self-if-potential-conflict -emit-cir -fclangir -triple x86_64-linux-pc %s -o %t.ll // RUN: FileCheck --input-file=%t.ll %s -struct NoCopyConstruct {}; +struct NoCopyConstruct { + int x; +}; struct CopyConstruct { CopyConstruct() = default; @@ -10,10 +12,12 @@ struct CopyConstruct { struct NonDefaultCtor { NonDefaultCtor(); + int x; }; struct HasDtor { ~HasDtor(); + int x; }; // CHECK: acc.firstprivate.recipe @firstprivatization__ZTSi : !cir.ptr init {