From 12bbc11b7408491b5792534babec65f2d4bd7179 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Wed, 31 Jan 2024 17:28:12 -0800 Subject: [PATCH 1/7] [IRGen] Fix a bug where an argument wasn't annotated with sret This fixes a bug in expandExternalSignatureTypes where it wasn't annotating a function call parameter type with sret when the result was being returned indirectly. The bug was causing calls to ObjC methods that return their results indirectly to crash. Resolves rdar://121618707 --- lib/IRGen/GenCall.cpp | 70 ++++++++++++------ lib/IRGen/GenDecl.cpp | 3 +- lib/IRGen/GenFunc.cpp | 30 +++++++- lib/IRGen/IRGenModule.h | 7 +- lib/IRGen/IRGenSIL.cpp | 7 +- lib/IRGen/Signature.h | 3 +- ...is-and-indirect-return-irgen-itanium.swift | 2 +- test/Interop/Cxx/objc-correctness/sret.swift | 72 +++++++++++++++++++ 8 files changed, 161 insertions(+), 33 deletions(-) create mode 100644 test/Interop/Cxx/objc-correctness/sret.swift diff --git a/lib/IRGen/GenCall.cpp b/lib/IRGen/GenCall.cpp index 82eac3ea0cfea..4d5cc50839811 100644 --- a/lib/IRGen/GenCall.cpp +++ b/lib/IRGen/GenCall.cpp @@ -428,6 +428,10 @@ namespace { IRGenModule &IGM; CanSILFunctionType FnType; bool forStaticCall = false; // Used for objc_method (direct call or not). + + // Indicates this is a c++ constructor call. + bool forCXXConstructorCall = false; + public: SmallVector ParamIRTypes; llvm::Type *ResultIRType = nullptr; @@ -442,8 +446,10 @@ namespace { FunctionPointerKind FnKind; SignatureExpansion(IRGenModule &IGM, CanSILFunctionType fnType, - FunctionPointerKind fnKind, bool forStaticCall = false) - : IGM(IGM), FnType(fnType), forStaticCall(forStaticCall), FnKind(fnKind) {} + FunctionPointerKind fnKind, bool forStaticCall = false, + bool forCXXConstructorCall = false) + : IGM(IGM), FnType(fnType), forStaticCall(forStaticCall), + forCXXConstructorCall(forCXXConstructorCall), FnKind(fnKind) {} /// Expand the components of the primary entrypoint of the function type. void expandFunctionType( @@ -1361,9 +1367,21 @@ static bool doesClangExpansionMatchSchema(IRGenModule &IGM, void SignatureExpansion::expandExternalSignatureTypes() { assert(FnType->getLanguage() == SILFunctionLanguage::C); - // Convert the SIL result type to a Clang type. - auto clangResultTy = - IGM.getClangType(FnType->getFormalCSemanticResult(IGM.getSILModule())); + auto SILResultTy = [&]() { + if (FnType->getNumResults() == 0) + return SILType::getPrimitiveObjectType(IGM.Context.TheEmptyTupleType); + + return SILType::getPrimitiveObjectType( + FnType->getSingleResult().getReturnValueType( + IGM.getSILModule(), FnType, TypeExpansionContext::minimal())); + }(); + + // Convert the SIL result type to a Clang type. If this is for a c++ + // constructor, use 'void' as the return type. + auto clangResultTy = IGM.getClangType( + forCXXConstructorCall + ? SILType::getPrimitiveObjectType(IGM.Context.TheEmptyTupleType) + : SILResultTy); // Now convert the parameters to Clang types. auto params = FnType->getParameters(); @@ -1371,16 +1389,6 @@ void SignatureExpansion::expandExternalSignatureTypes() { SmallVector paramTys; auto const &clangCtx = IGM.getClangASTContext(); - bool formalIndirectResult = FnType->getNumResults() > 0 && - FnType->getSingleResult().isFormalIndirect(); - if (formalIndirectResult) { - auto resultType = getSILFuncConventions().getSingleSILResultType( - IGM.getMaximalTypeExpansionContext()); - auto clangTy = - IGM.getClangASTContext().getPointerType(IGM.getClangType(resultType)); - paramTys.push_back(clangTy); - } - switch (FnType->getRepresentation()) { case SILFunctionTypeRepresentation::ObjCMethod: { // ObjC methods take their 'self' argument first, followed by an @@ -1409,7 +1417,11 @@ void SignatureExpansion::expandExternalSignatureTypes() { } case SILFunctionTypeRepresentation::CFunctionPointer: - // No implicit arguments. + if (forCXXConstructorCall) { + auto clangTy = IGM.getClangASTContext().getPointerType( + IGM.getClangType(SILResultTy)); + paramTys.push_back(clangTy); + } break; case SILFunctionTypeRepresentation::Thin: @@ -1447,6 +1459,15 @@ void SignatureExpansion::expandExternalSignatureTypes() { auto &returnInfo = FI.getReturnInfo(); +#ifndef NDEBUG + bool formalIndirectResult = FnType->getNumResults() > 0 && + FnType->getSingleResult().isFormalIndirect(); + assert( + (forCXXConstructorCall || !formalIndirectResult || + returnInfo.isIndirect()) && + "swift and clang disagree on whether the result is returned indirectly"); +#endif + // Does the result need an extension attribute? if (returnInfo.isExtend()) { bool signExt = clangResultTy->hasSignedIntegerRepresentation(); @@ -2153,10 +2174,11 @@ Signature SignatureExpansion::getSignature() { Signature Signature::getUncached(IRGenModule &IGM, CanSILFunctionType formalType, - FunctionPointerKind fpKind, - bool forStaticCall) { + FunctionPointerKind fpKind, bool forStaticCall, + bool forCXXConstructorCall) { GenericContextScope scope(IGM, formalType->getInvocationGenericSignature()); - SignatureExpansion expansion(IGM, formalType, fpKind, forStaticCall); + SignatureExpansion expansion(IGM, formalType, fpKind, forStaticCall, + forCXXConstructorCall); expansion.expandFunctionType(); return expansion.getSignature(); } @@ -3950,11 +3972,13 @@ static void externalizeArguments(IRGenFunction &IGF, const Callee &callee, params = params.drop_back(); } - if (fnType->getNumResults() > 0 && - fnType->getSingleResult().isFormalIndirect()) { - // Ignore the indirect result parameter. + bool formalIndirectResult = fnType->getNumResults() > 0 && + fnType->getSingleResult().isFormalIndirect(); + + // If clang returns directly and swift returns indirectly, this must be a c++ + // constructor call. In that case, skip the "self" param. + if (!FI.getReturnInfo().isIndirect() && formalIndirectResult) firstParam += 1; - } for (unsigned i = firstParam; i != paramEnd; ++i) { auto clangParamTy = FI.arg_begin()[i].type; diff --git a/lib/IRGen/GenDecl.cpp b/lib/IRGen/GenDecl.cpp index fa8a67cc53217..9cabe99bc5ca5 100644 --- a/lib/IRGen/GenDecl.cpp +++ b/lib/IRGen/GenDecl.cpp @@ -3583,7 +3583,8 @@ llvm::Function *IRGenModule::getAddrOfSILFunction( } if (cxxCtor) { - Signature signature = getSignature(f->getLoweredFunctionType()); + Signature signature = getSignature(f->getLoweredFunctionType(), + /*isCXXConstructorCall*/ true); // The thunk has private linkage, so it doesn't need to have a predictable // mangled name -- we just need to make sure the name is unique. diff --git a/lib/IRGen/GenFunc.cpp b/lib/IRGen/GenFunc.cpp index b97022157197c..3f3b66d819c35 100644 --- a/lib/IRGen/GenFunc.cpp +++ b/lib/IRGen/GenFunc.cpp @@ -125,11 +125,13 @@ namespace { const CanSILFunctionType FormalType; mutable Signature TheSignature; + mutable Signature TheCXXConstructorSignature; public: FuncSignatureInfo(CanSILFunctionType formalType) : FormalType(formalType) {} + Signature getCXXConstructorSignature(IRGenModule &IGM) const; Signature getSignature(IRGenModule &IGM) const; }; @@ -674,6 +676,21 @@ Signature FuncSignatureInfo::getSignature(IRGenModule &IGM) const { return TheSignature; } +Signature +FuncSignatureInfo::getCXXConstructorSignature(IRGenModule &IGM) const { + // If it's already been filled in, we're done. + if (TheCXXConstructorSignature.isValid()) + return TheCXXConstructorSignature; + + // Update the cache and return. + TheCXXConstructorSignature = + Signature::getUncached(IGM, FormalType, FunctionPointerKind(FormalType), + /*forStaticCall*/ false, + /*forCXXConstructorCall*/ true); + assert(TheCXXConstructorSignature.isValid()); + return TheCXXConstructorSignature; +} + Signature ObjCFuncSignatureInfo::getDirectSignature(IRGenModule &IGM) const { // If it's already been filled in, we're done. if (TheDirectSignature.isValid()) @@ -712,13 +729,16 @@ getFuncSignatureInfoForLowered(IRGenModule &IGM, CanSILFunctionType type) { llvm_unreachable("bad function type representation"); } -Signature IRGenModule::getSignature(CanSILFunctionType type) { - return getSignature(type, FunctionPointerKind(type)); +Signature IRGenModule::getSignature(CanSILFunctionType type, + bool isCXXConstructorCall) { + return getSignature(type, FunctionPointerKind(type), /*forStaticCall*/ false, + isCXXConstructorCall); } Signature IRGenModule::getSignature(CanSILFunctionType type, FunctionPointerKind kind, - bool forStaticCall) { + bool forStaticCall, + bool isCXXConstructorCall) { // Don't bother caching if we're working with a special kind. if (kind.isSpecial()) return Signature::getUncached(*this, type, kind); @@ -730,6 +750,10 @@ Signature IRGenModule::getSignature(CanSILFunctionType type, auto &objcSigInfo = static_cast(sigInfo); return objcSigInfo.getDirectSignature(*this); } + + if (isCXXConstructorCall) + return sigInfo.getCXXConstructorSignature(*this); + return sigInfo.getSignature(*this); } diff --git a/lib/IRGen/IRGenModule.h b/lib/IRGen/IRGenModule.h index b4ced620f610a..bd25757a470b7 100644 --- a/lib/IRGen/IRGenModule.h +++ b/lib/IRGen/IRGenModule.h @@ -1574,10 +1574,11 @@ private: \ void finalizeClangCodeGen(); void finishEmitAfterTopLevel(); - Signature getSignature(CanSILFunctionType fnType); Signature getSignature(CanSILFunctionType fnType, - FunctionPointerKind kind, - bool forStaticCall = false); + bool isCXXConstructorCall = false); + Signature getSignature(CanSILFunctionType fnType, FunctionPointerKind kind, + bool forStaticCall = false, + bool isCXXConstructorCall = false); llvm::FunctionType *getFunctionType(CanSILFunctionType type, llvm::AttributeList &attrs, ForeignFunctionInfo *foreignInfo=nullptr); diff --git a/lib/IRGen/IRGenSIL.cpp b/lib/IRGen/IRGenSIL.cpp index 63e5501d09741..a307d22ce2ca1 100644 --- a/lib/IRGen/IRGenSIL.cpp +++ b/lib/IRGen/IRGenSIL.cpp @@ -2969,8 +2969,13 @@ void IRGenSILFunction::visitFunctionRefBaseInst(FunctionRefBaseInst *i) { auto fnType = fn->getLoweredFunctionType(); auto fpKind = irgen::classifyFunctionPointerKind(fn); + bool isCXXConstructor = false; - auto sig = IGM.getSignature(fnType, fpKind, true /*forStaticCall*/); + if (auto *clangFnDecl = fn->getClangDecl()) + isCXXConstructor = isa(clangFnDecl); + + auto sig = IGM.getSignature(fnType, fpKind, true /*forStaticCall*/, + isCXXConstructor); // Note that the pointer value returned by getAddrOfSILFunction doesn't // necessarily have element type sig.getType(), e.g. if it's imported. diff --git a/lib/IRGen/Signature.h b/lib/IRGen/Signature.h index 32c6d5e83d6a0..d1c5739f13180 100644 --- a/lib/IRGen/Signature.h +++ b/lib/IRGen/Signature.h @@ -204,7 +204,8 @@ class Signature { /// clients should generally be using. static Signature getUncached(IRGenModule &IGM, CanSILFunctionType formalType, FunctionPointerKind kind, - bool forStaticCall = false); + bool forStaticCall = false, + bool forCXXConstructorCall = false); static SignatureExpansionABIDetails getUncachedABIDetails(IRGenModule &IGM, CanSILFunctionType formalType, diff --git a/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-itanium.swift b/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-itanium.swift index c77654b8c7aec..ac477b555e0a4 100644 --- a/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-itanium.swift +++ b/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-itanium.swift @@ -12,6 +12,6 @@ public func use() -> CInt { // CHECK: %[[instance:.*]] = alloca %TSo10HasMethodsV // CHECK: %[[result:.*]] = alloca %TSo19NonTrivialInWrapperV -// CHECK: call void @_ZN10HasMethods28nonConstPassThroughAsWrapperEi(ptr sret(%struct.NonTrivialInWrapper) %[[result]], ptr %[[instance]], i32 42) +// CHECK: call void @_ZN10HasMethods28nonConstPassThroughAsWrapperEi(ptr noalias nocapture sret(%struct.NonTrivialInWrapper) %[[result]], ptr %[[instance]], i32 42) // CHECK: define {{.*}} void @_ZN10HasMethods28nonConstPassThroughAsWrapperEi(ptr noalias sret(%struct.NonTrivialInWrapper) {{.*}} %{{.*}}, ptr {{.*}} %{{.*}}, i32 noundef %{{.*}}) diff --git a/test/Interop/Cxx/objc-correctness/sret.swift b/test/Interop/Cxx/objc-correctness/sret.swift new file mode 100644 index 0000000000000..cb2a36e71a370 --- /dev/null +++ b/test/Interop/Cxx/objc-correctness/sret.swift @@ -0,0 +1,72 @@ +// RUN: %empty-directory(%t) +// RUN: split-file %s %t + +// RUN: %target-swift-emit-ir -I %t/Inputs -cxx-interoperability-mode=default %t/test.swift | %FileCheck %s + +// REQUIRES: objc_interop + +//--- Inputs/header.h + +class S { +public: + int a; + ~S(); + S getS(int) const; + static S getSStatic(int); +}; + +S getS(int); + +@interface C ++(S)getS:(int)a; +-(S)getS:(int)a; +@end + +//--- Inputs/module.modulemap + +module SRet { + header "header.h" + requires cplusplus + export * +} + +//--- test.swift + +import SRet + +func test(c : C, s : S) { + let _ : S = c.getS(1) + let _ : S = C.getS(1) + let _ : S = S() + let _ : S = getS(1) + let _ : S = s.getS(1) + let _ : S = S.getSStatic(1) +} + +// CHECK: %[[TSO1SV:.*]] = type <{ %[[TS5INT32V:.*]] }> +// CHECK: %[[TS5INT32V]] = type <{ i32 }> +// CHECK: %[[SWIFT_OPAQUE:.*]] = type opaque +// CHECK: %[[CLASS_S:.*]] = type { i32 } + +// CHECK: define hidden swiftcc void @"$s4testAA1c1sySo1CC_So1SVtF"(ptr %[[V0:.*]], ptr {{.*}}%[[V1:.*]]) +// CHECK: %[[V2:.*]] = alloca %[[TSO1SV]], align 4 +// CHECK: %[[V3:.*]] = alloca %[[TSO1SV]], align 4 +// CHECK: %[[V4:.*]] = alloca %[[TSO1SV]], align 4 +// CHECK: %[[V5:.*]] = alloca %[[TSO1SV]], align 4 +// CHECK: %[[V6:.*]] = alloca %[[TSO1SV]], align 4 +// CHECK: %[[V7:.*]] = alloca %[[TSO1SV]], align 4 +// CHECK: call void @llvm.lifetime.start.p0(i64 4, ptr %[[V2]]) +// CHECK: %[[V8:.*]] = load ptr, ptr @"\01L_selector(getS:)", align 8 +// CHECK: invoke void @objc_msgSend(ptr noalias nocapture sret(%[[SWIFT_OPAQUE]]) %[[V2]], ptr %[[V0]], ptr %[[V8]], i32 1) + +// CHECK: %[[V10:.*]] = load ptr, ptr @"OBJC_CLASS_REF_$_C", align 8 +// CHECK: %[[V11:.*]] = call ptr @objc_opt_self(ptr %[[V10]]) +// CHECK: %[[V12:.*]] = load ptr, ptr @"\01L_selector(getS:)", align 8 +// CHECK: invoke void @objc_msgSend(ptr noalias nocapture sret(%[[SWIFT_OPAQUE]]) %[[V3]], ptr %[[V11]], ptr %[[V12]], i32 1) + +// CHECK: %[[V14:.*]] = call ptr @_ZN1SC1Ev(ptr %[[V4]]) +// CHECK: invoke void @_Z4getSi(ptr noalias nocapture sret(%[[CLASS_S]]) %[[V5]], i32 1) + +// CHECK: invoke void @_ZNK1S4getSEi(ptr noalias nocapture sret(%[[CLASS_S]]) %[[V6]], ptr %[[V1]], i32 1) + +// CHECK: invoke void @_ZN1S10getSStaticEi(ptr noalias nocapture sret(%[[CLASS_S]]) %[[V7]], i32 1) From e2081b9adc76cf04fd9e742f97f08e169a3cd308 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Wed, 7 Feb 2024 20:24:08 -0800 Subject: [PATCH 2/7] Pass target triple --- test/Interop/Cxx/objc-correctness/sret.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Interop/Cxx/objc-correctness/sret.swift b/test/Interop/Cxx/objc-correctness/sret.swift index cb2a36e71a370..9e61186034c1a 100644 --- a/test/Interop/Cxx/objc-correctness/sret.swift +++ b/test/Interop/Cxx/objc-correctness/sret.swift @@ -1,7 +1,7 @@ // RUN: %empty-directory(%t) // RUN: split-file %s %t -// RUN: %target-swift-emit-ir -I %t/Inputs -cxx-interoperability-mode=default %t/test.swift | %FileCheck %s +// RUN: %target-swift-emit-ir -I %t/Inputs -cxx-interoperability-mode=default %t/test.swift -target arm64-apple-macos12 | %FileCheck %s // REQUIRES: objc_interop From b65aaf8eb665449198959eb825fd4c4e80d36083 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Thu, 8 Feb 2024 02:26:19 -0800 Subject: [PATCH 3/7] Make sure the sret type matches the type of the pointee type. --- lib/IRGen/GenCall.cpp | 24 ++++++++++++------- ...is-and-indirect-return-irgen-itanium.swift | 2 +- test/Interop/Cxx/objc-correctness/sret.swift | 12 ++++------ 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/lib/IRGen/GenCall.cpp b/lib/IRGen/GenCall.cpp index 4d5cc50839811..e38b41bfc2056 100644 --- a/lib/IRGen/GenCall.cpp +++ b/lib/IRGen/GenCall.cpp @@ -474,7 +474,7 @@ namespace { private: const TypeInfo &expand(SILParameterInfo param); - llvm::Type *addIndirectResult(); + llvm::Type *addIndirectResult(SILType resultType); SILFunctionConventions getSILFuncConventions() const { return SILFunctionConventions(FnType, IGM.getSILModule()); @@ -532,9 +532,7 @@ namespace { } // end namespace irgen } // end namespace swift -llvm::Type *SignatureExpansion::addIndirectResult() { - auto resultType = getSILFuncConventions().getSILResultType( - IGM.getMaximalTypeExpansionContext()); +llvm::Type *SignatureExpansion::addIndirectResult(SILType resultType) { const TypeInfo &resultTI = IGM.getTypeInfo(resultType); auto storageTy = resultTI.getStorageType(); addIndirectResultAttributes(IGM, Attrs, ParamIRTypes.size(), claimSRet(), @@ -931,7 +929,7 @@ SignatureExpansion::expandDirectResult() { auto &ti = IGM.getTypeInfo(resultType); auto &native = ti.nativeReturnValueSchema(IGM); if (native.requiresIndirect()) - return std::make_pair(addIndirectResult(), nullptr); + return std::make_pair(addIndirectResult(resultType), nullptr); // Disable the use of sret if we have a non-trivial direct result. if (!native.empty()) CanUseSRet = false; @@ -1572,6 +1570,8 @@ void SignatureExpansion::expandExternalSignatureTypes() { // If we return indirectly, that is the first parameter type. if (returnInfo.isIndirect()) { + auto resultType = getSILFuncConventions().getSingleSILResultType( + IGM.getMaximalTypeExpansionContext()); if (IGM.Triple.isWindowsMSVCEnvironment() && FnType->getRepresentation() == SILFunctionTypeRepresentation::CXXMethod) { @@ -1579,9 +1579,9 @@ void SignatureExpansion::expandExternalSignatureTypes() { // returned indirect values. emitArg(0); firstParamToLowerNormally = 1; - addIndirectResult(); + addIndirectResult(resultType); } else - addIndirectResult(); + addIndirectResult(resultType); } // Use a special IR type for passing block pointers. @@ -2007,7 +2007,7 @@ void SignatureExpansion::expandAsyncEntryType() { auto &ti = IGM.getTypeInfo(resultType); auto &native = ti.nativeReturnValueSchema(IGM); if (native.requiresIndirect()) - addIndirectResult(); + addIndirectResult(resultType); // Add the indirect result types. expandIndirectResults(); @@ -3257,7 +3257,13 @@ llvm::CallBase *IRBuilder::CreateCallOrInvoke( for (unsigned argIndex = 0; argIndex < func->arg_size(); ++argIndex) { if (func->hasParamAttribute(argIndex, llvm::Attribute::StructRet)) { llvm::AttrBuilder builder(func->getContext()); - builder.addStructRetAttr(func->getParamStructRetType(argIndex)); + // See if there is a sret parameter in the signature. There are cases + // where the called function has a sret parameter, but the signature + // doesn't (e.g., noreturn functions). + llvm::Type *ty = attrs.getParamStructRetType(argIndex); + if (!ty) + ty = func->getParamStructRetType(argIndex); + builder.addStructRetAttr(ty); attrs = attrs.addParamAttributes(func->getContext(), argIndex, builder); } if (func->hasParamAttribute(argIndex, llvm::Attribute::ByVal)) { diff --git a/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-itanium.swift b/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-itanium.swift index ac477b555e0a4..cc722ffb5e9d9 100644 --- a/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-itanium.swift +++ b/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-itanium.swift @@ -12,6 +12,6 @@ public func use() -> CInt { // CHECK: %[[instance:.*]] = alloca %TSo10HasMethodsV // CHECK: %[[result:.*]] = alloca %TSo19NonTrivialInWrapperV -// CHECK: call void @_ZN10HasMethods28nonConstPassThroughAsWrapperEi(ptr noalias nocapture sret(%struct.NonTrivialInWrapper) %[[result]], ptr %[[instance]], i32 42) +// CHECK: call void @_ZN10HasMethods28nonConstPassThroughAsWrapperEi(ptr noalias sret(%TSo19NonTrivialInWrapperV) %[[result]], ptr %[[instance]], i32 42) // CHECK: define {{.*}} void @_ZN10HasMethods28nonConstPassThroughAsWrapperEi(ptr noalias sret(%struct.NonTrivialInWrapper) {{.*}} %{{.*}}, ptr {{.*}} %{{.*}}, i32 noundef %{{.*}}) diff --git a/test/Interop/Cxx/objc-correctness/sret.swift b/test/Interop/Cxx/objc-correctness/sret.swift index 9e61186034c1a..bda042a6b2adf 100644 --- a/test/Interop/Cxx/objc-correctness/sret.swift +++ b/test/Interop/Cxx/objc-correctness/sret.swift @@ -45,8 +45,6 @@ func test(c : C, s : S) { // CHECK: %[[TSO1SV:.*]] = type <{ %[[TS5INT32V:.*]] }> // CHECK: %[[TS5INT32V]] = type <{ i32 }> -// CHECK: %[[SWIFT_OPAQUE:.*]] = type opaque -// CHECK: %[[CLASS_S:.*]] = type { i32 } // CHECK: define hidden swiftcc void @"$s4testAA1c1sySo1CC_So1SVtF"(ptr %[[V0:.*]], ptr {{.*}}%[[V1:.*]]) // CHECK: %[[V2:.*]] = alloca %[[TSO1SV]], align 4 @@ -57,16 +55,16 @@ func test(c : C, s : S) { // CHECK: %[[V7:.*]] = alloca %[[TSO1SV]], align 4 // CHECK: call void @llvm.lifetime.start.p0(i64 4, ptr %[[V2]]) // CHECK: %[[V8:.*]] = load ptr, ptr @"\01L_selector(getS:)", align 8 -// CHECK: invoke void @objc_msgSend(ptr noalias nocapture sret(%[[SWIFT_OPAQUE]]) %[[V2]], ptr %[[V0]], ptr %[[V8]], i32 1) +// CHECK: invoke void @objc_msgSend(ptr noalias sret(%[[TSO1SV]]) %[[V2]], ptr %[[V0]], ptr %[[V8]], i32 1) // CHECK: %[[V10:.*]] = load ptr, ptr @"OBJC_CLASS_REF_$_C", align 8 // CHECK: %[[V11:.*]] = call ptr @objc_opt_self(ptr %[[V10]]) // CHECK: %[[V12:.*]] = load ptr, ptr @"\01L_selector(getS:)", align 8 -// CHECK: invoke void @objc_msgSend(ptr noalias nocapture sret(%[[SWIFT_OPAQUE]]) %[[V3]], ptr %[[V11]], ptr %[[V12]], i32 1) +// CHECK: invoke void @objc_msgSend(ptr noalias sret(%[[TSO1SV]]) %[[V3]], ptr %[[V11]], ptr %[[V12]], i32 1) // CHECK: %[[V14:.*]] = call ptr @_ZN1SC1Ev(ptr %[[V4]]) -// CHECK: invoke void @_Z4getSi(ptr noalias nocapture sret(%[[CLASS_S]]) %[[V5]], i32 1) +// CHECK: invoke void @_Z4getSi(ptr noalias sret(%[[TSO1SV]]) %[[V5]], i32 1) -// CHECK: invoke void @_ZNK1S4getSEi(ptr noalias nocapture sret(%[[CLASS_S]]) %[[V6]], ptr %[[V1]], i32 1) +// CHECK: invoke void @_ZNK1S4getSEi(ptr noalias sret(%[[TSO1SV]]) %[[V6]], ptr %[[V1]], i32 1) -// CHECK: invoke void @_ZN1S10getSStaticEi(ptr noalias nocapture sret(%[[CLASS_S]]) %[[V7]], i32 1) +// CHECK: invoke void @_ZN1S10getSStaticEi(ptr noalias sret(%[[TSO1SV]]) %[[V7]], i32 1) From 6a8336371e3c2f9babbeb06ab970e1466128bc7c Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Thu, 8 Feb 2024 21:00:30 -0800 Subject: [PATCH 4/7] Fix tests --- test/IRGen/extern_c_abitypes.swift | 6 +++--- .../methods-this-and-indirect-return-irgen-msvc.swift | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/IRGen/extern_c_abitypes.swift b/test/IRGen/extern_c_abitypes.swift index 42c5c6e0e2e00..1621648220f14 100644 --- a/test/IRGen/extern_c_abitypes.swift +++ b/test/IRGen/extern_c_abitypes.swift @@ -99,11 +99,11 @@ func test() { // assume %struct.c_struct and %TSo8c_structV have compatible layout // - // CHECK-x86_64: call void @c_roundtrip_c_struct(ptr noalias nocapture sret(%struct.c_struct) {{.*}}, ptr{{( byval\(%struct.c_struct\))?}}[[ALIGN:(align [0-9]+)?]] {{.*}}) + // CHECK-x86_64: call void @c_roundtrip_c_struct(ptr noalias nocapture sret(%TSo8c_structV) {{.*}}, ptr{{( byval\(%struct.c_struct\))?}}[[ALIGN:(align [0-9]+)?]] {{.*}}) // CHECK-x86_64: call void @swift_roundtrip_c_struct(ptr noalias nocapture sret(%TSo8c_structV) {{.*}}, ptr{{( byval\(%TSo8c_structV\))?}}[[ALIGN]] {{.*}}) - // CHECK-arm64: call void @c_roundtrip_c_struct(ptr noalias nocapture sret(%struct.c_struct) {{.*}}, ptr{{( byval\(%struct.c_struct\))?}}[[ALIGN:(align [0-9]+)?]] {{.*}}) + // CHECK-arm64: call void @c_roundtrip_c_struct(ptr noalias nocapture sret(%TSo8c_structV) {{.*}}, ptr{{( byval\(%struct.c_struct\))?}}[[ALIGN:(align [0-9]+)?]] {{.*}}) // CHECK-arm64: call void @swift_roundtrip_c_struct(ptr noalias nocapture sret(%TSo8c_structV) {{.*}}, ptr{{( byval\(%TSo8c_structV\))?}}[[ALIGN]] {{.*}}) - // CHECK-wasm32: call void @c_roundtrip_c_struct(ptr noalias nocapture sret(%struct.c_struct) {{.*}}, ptr{{( byval\(%struct.c_struct\))?}}[[ALIGN:(align [0-9]+)?]] {{.*}}) + // CHECK-wasm32: call void @c_roundtrip_c_struct(ptr noalias nocapture sret(%TSo8c_structV) {{.*}}, ptr{{( byval\(%struct.c_struct\))?}}[[ALIGN:(align [0-9]+)?]] {{.*}}) // CHECK-wasm32: call void @swift_roundtrip_c_struct(ptr noalias nocapture sret(%TSo8c_structV) {{.*}}, ptr{{( byval\(%TSo8c_structV\))?}}[[ALIGN]] {{.*}}) // CHECK-armv7k: call [3 x i32] @c_roundtrip_c_struct([3 x i32] {{.*}}) // CHECK-armv7k: call [3 x i32] @swift_roundtrip_c_struct([3 x i32] {{.*}}) diff --git a/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-msvc.swift b/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-msvc.swift index 354469360f2a4..71da6c5f936ab 100644 --- a/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-msvc.swift +++ b/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-msvc.swift @@ -12,7 +12,7 @@ public func use() -> CInt { // CHECK: %[[instance:.*]] = alloca %TSo10HasMethodsV // CHECK: %[[result:.*]] = alloca %TSo19NonTrivialInWrapperV -// CHECK: call void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(ptr %[[instance]], ptr sret(%struct.NonTrivialInWrapper) %[[result]], i32 42) +// CHECK: call void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(ptr %[[instance]], ptr noalias sret(%struct.NonTrivialInWrapper) %[[result]], i32 42) // CHECK-x86_64: define {{.*}} void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(ptr {{.*}} %{{.*}}, ptr noalias sret(%struct.NonTrivialInWrapper) {{.*}} %{{.*}}, i32 noundef %{{.*}}) // CHECK-aarch64: define {{.*}} void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(ptr {{.*}} %{{.*}}, ptr inreg noalias sret(%struct.NonTrivialInWrapper) {{.*}} %{{.*}}, i32 noundef %{{.*}}) From 9ddee0f2c8ccf440b62db88ffe7aa08cd0ee47c1 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Thu, 8 Feb 2024 16:27:15 -0800 Subject: [PATCH 5/7] Call arrangeCXXStructorDeclaration to arrange constructor signatures --- lib/IRGen/GenCall.cpp | 52 +++++++++++++++++++++++++++-------------- lib/IRGen/GenDecl.cpp | 3 +-- lib/IRGen/GenFunc.cpp | 30 +++++++++++++----------- lib/IRGen/IRGenModule.h | 12 ++++++---- lib/IRGen/IRGenSIL.cpp | 8 +++---- lib/IRGen/Signature.h | 9 +++---- 6 files changed, 67 insertions(+), 47 deletions(-) diff --git a/lib/IRGen/GenCall.cpp b/lib/IRGen/GenCall.cpp index e38b41bfc2056..1c8e3d7f4d74a 100644 --- a/lib/IRGen/GenCall.cpp +++ b/lib/IRGen/GenCall.cpp @@ -25,6 +25,7 @@ #include "swift/SIL/SILModule.h" #include "swift/SIL/SILType.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/GlobalDecl.h" #include "clang/AST/RecordLayout.h" #include "clang/Basic/TargetInfo.h" #include "clang/CodeGen/CodeGenABITypes.h" @@ -430,7 +431,7 @@ namespace { bool forStaticCall = false; // Used for objc_method (direct call or not). // Indicates this is a c++ constructor call. - bool forCXXConstructorCall = false; + const clang::CXXConstructorDecl *cxxCtorDecl = nullptr; public: SmallVector ParamIRTypes; @@ -447,9 +448,9 @@ namespace { SignatureExpansion(IRGenModule &IGM, CanSILFunctionType fnType, FunctionPointerKind fnKind, bool forStaticCall = false, - bool forCXXConstructorCall = false) + const clang::CXXConstructorDecl *cxxCtorDecl = nullptr) : IGM(IGM), FnType(fnType), forStaticCall(forStaticCall), - forCXXConstructorCall(forCXXConstructorCall), FnKind(fnKind) {} + cxxCtorDecl(cxxCtorDecl), FnKind(fnKind) {} /// Expand the components of the primary entrypoint of the function type. void expandFunctionType( @@ -1374,12 +1375,8 @@ void SignatureExpansion::expandExternalSignatureTypes() { IGM.getSILModule(), FnType, TypeExpansionContext::minimal())); }(); - // Convert the SIL result type to a Clang type. If this is for a c++ - // constructor, use 'void' as the return type. - auto clangResultTy = IGM.getClangType( - forCXXConstructorCall - ? SILType::getPrimitiveObjectType(IGM.Context.TheEmptyTupleType) - : SILResultTy); + // Convert the SIL result type to a Clang type. + auto clangResultTy = IGM.getClangType(SILResultTy); // Now convert the parameters to Clang types. auto params = FnType->getParameters(); @@ -1415,7 +1412,7 @@ void SignatureExpansion::expandExternalSignatureTypes() { } case SILFunctionTypeRepresentation::CFunctionPointer: - if (forCXXConstructorCall) { + if (cxxCtorDecl) { auto clangTy = IGM.getClangASTContext().getPointerType( IGM.getClangType(SILResultTy)); paramTys.push_back(clangTy); @@ -1447,10 +1444,15 @@ void SignatureExpansion::expandExternalSignatureTypes() { // Generate function info for this signature. auto extInfo = clang::FunctionType::ExtInfo(); - auto &FI = clang::CodeGen::arrangeFreeFunctionCall(IGM.ClangCodeGen->CGM(), - clangResultTy, paramTys, extInfo, - clang::CodeGen::RequiredArgs::All); - ForeignInfo.ClangInfo = &FI; + + if (cxxCtorDecl) + ForeignInfo.ClangInfo = &clang::CodeGen::arrangeCXXStructorDeclaration( + IGM.ClangCodeGen->CGM(), {cxxCtorDecl, clang::Ctor_Complete}); + else + ForeignInfo.ClangInfo = &clang::CodeGen::arrangeFreeFunctionCall( + IGM.ClangCodeGen->CGM(), clangResultTy, paramTys, extInfo, + clang::CodeGen::RequiredArgs::All); + auto &FI = *ForeignInfo.ClangInfo; assert(FI.arg_size() == paramTys.size() && "Expected one ArgInfo for each parameter type!"); @@ -1461,8 +1463,7 @@ void SignatureExpansion::expandExternalSignatureTypes() { bool formalIndirectResult = FnType->getNumResults() > 0 && FnType->getSingleResult().isFormalIndirect(); assert( - (forCXXConstructorCall || !formalIndirectResult || - returnInfo.isIndirect()) && + (cxxCtorDecl || !formalIndirectResult || returnInfo.isIndirect()) && "swift and clang disagree on whether the result is returned indirectly"); #endif @@ -2175,10 +2176,10 @@ Signature SignatureExpansion::getSignature() { Signature Signature::getUncached(IRGenModule &IGM, CanSILFunctionType formalType, FunctionPointerKind fpKind, bool forStaticCall, - bool forCXXConstructorCall) { + const clang::CXXConstructorDecl *cxxCtorDecl) { GenericContextScope scope(IGM, formalType->getInvocationGenericSignature()); SignatureExpansion expansion(IGM, formalType, fpKind, forStaticCall, - forCXXConstructorCall); + cxxCtorDecl); expansion.expandFunctionType(); return expansion.getSignature(); } @@ -3995,6 +3996,12 @@ static void externalizeArguments(IRGenFunction &IGF, const Callee &callee, // swiftcall function pointers through SIL as C functions anyway. assert(FI.getExtParameterInfo(i).getABI() == clang::ParameterABI::Ordinary); + assert((!silConv.isSILIndirect(params[i - firstParam]) || + AI.getKind() == clang::CodeGen::ABIArgInfo::Direct || + AI.getKind() == clang::CodeGen::ABIArgInfo::Indirect) && + "indirect SIL types passed indirectly should be classified as " + "either Direct or Indirect"); + // Add a padding argument if required. if (auto *padType = AI.getPaddingType()) out.add(llvm::UndefValue::get(padType)); @@ -4046,6 +4053,15 @@ static void externalizeArguments(IRGenFunction &IGF, const Callee &callee, case clang::CodeGen::ABIArgInfo::IndirectAliased: llvm_unreachable("not implemented"); case clang::CodeGen::ABIArgInfo::Indirect: { + // If this is a SIL type passed indirectly, avoid emitting a redundant + // initializing copy. + if (silConv.isSILIndirect(params[i - firstParam])) { + assert(paramType.isAddress() && "SIL type is not an address?"); + auto addr = in.claimNext(); + out.add(addr); + break; + } + auto &ti = cast(IGF.getTypeInfo(paramType)); auto temp = ti.allocateStack(IGF, paramType, "indirect-temporary"); diff --git a/lib/IRGen/GenDecl.cpp b/lib/IRGen/GenDecl.cpp index 9cabe99bc5ca5..34f3f0a648daa 100644 --- a/lib/IRGen/GenDecl.cpp +++ b/lib/IRGen/GenDecl.cpp @@ -3583,8 +3583,7 @@ llvm::Function *IRGenModule::getAddrOfSILFunction( } if (cxxCtor) { - Signature signature = getSignature(f->getLoweredFunctionType(), - /*isCXXConstructorCall*/ true); + Signature signature = getSignature(f->getLoweredFunctionType(), cxxCtor); // The thunk has private linkage, so it doesn't need to have a predictable // mangled name -- we just need to make sure the name is unique. diff --git a/lib/IRGen/GenFunc.cpp b/lib/IRGen/GenFunc.cpp index 3f3b66d819c35..b91cae426794e 100644 --- a/lib/IRGen/GenFunc.cpp +++ b/lib/IRGen/GenFunc.cpp @@ -131,7 +131,9 @@ namespace { FuncSignatureInfo(CanSILFunctionType formalType) : FormalType(formalType) {} - Signature getCXXConstructorSignature(IRGenModule &IGM) const; + Signature + getCXXConstructorSignature(const clang::CXXConstructorDecl *cxxCtorDecl, + IRGenModule &IGM) const; Signature getSignature(IRGenModule &IGM) const; }; @@ -676,8 +678,8 @@ Signature FuncSignatureInfo::getSignature(IRGenModule &IGM) const { return TheSignature; } -Signature -FuncSignatureInfo::getCXXConstructorSignature(IRGenModule &IGM) const { +Signature FuncSignatureInfo::getCXXConstructorSignature( + const clang::CXXConstructorDecl *cxxCtorDecl, IRGenModule &IGM) const { // If it's already been filled in, we're done. if (TheCXXConstructorSignature.isValid()) return TheCXXConstructorSignature; @@ -685,8 +687,7 @@ FuncSignatureInfo::getCXXConstructorSignature(IRGenModule &IGM) const { // Update the cache and return. TheCXXConstructorSignature = Signature::getUncached(IGM, FormalType, FunctionPointerKind(FormalType), - /*forStaticCall*/ false, - /*forCXXConstructorCall*/ true); + /*forStaticCall*/ false, cxxCtorDecl); assert(TheCXXConstructorSignature.isValid()); return TheCXXConstructorSignature; } @@ -729,16 +730,17 @@ getFuncSignatureInfoForLowered(IRGenModule &IGM, CanSILFunctionType type) { llvm_unreachable("bad function type representation"); } -Signature IRGenModule::getSignature(CanSILFunctionType type, - bool isCXXConstructorCall) { +Signature +IRGenModule::getSignature(CanSILFunctionType type, + const clang::CXXConstructorDecl *cxxCtorDecl) { return getSignature(type, FunctionPointerKind(type), /*forStaticCall*/ false, - isCXXConstructorCall); + cxxCtorDecl); } -Signature IRGenModule::getSignature(CanSILFunctionType type, - FunctionPointerKind kind, - bool forStaticCall, - bool isCXXConstructorCall) { +Signature +IRGenModule::getSignature(CanSILFunctionType type, FunctionPointerKind kind, + bool forStaticCall, + const clang::CXXConstructorDecl *cxxCtorDecl) { // Don't bother caching if we're working with a special kind. if (kind.isSpecial()) return Signature::getUncached(*this, type, kind); @@ -751,8 +753,8 @@ Signature IRGenModule::getSignature(CanSILFunctionType type, return objcSigInfo.getDirectSignature(*this); } - if (isCXXConstructorCall) - return sigInfo.getCXXConstructorSignature(*this); + if (cxxCtorDecl) + return sigInfo.getCXXConstructorSignature(cxxCtorDecl, *this); return sigInfo.getSignature(*this); } diff --git a/lib/IRGen/IRGenModule.h b/lib/IRGen/IRGenModule.h index bd25757a470b7..20774e07b40ff 100644 --- a/lib/IRGen/IRGenModule.h +++ b/lib/IRGen/IRGenModule.h @@ -1574,11 +1574,13 @@ private: \ void finalizeClangCodeGen(); void finishEmitAfterTopLevel(); - Signature getSignature(CanSILFunctionType fnType, - bool isCXXConstructorCall = false); - Signature getSignature(CanSILFunctionType fnType, FunctionPointerKind kind, - bool forStaticCall = false, - bool isCXXConstructorCall = false); + Signature + getSignature(CanSILFunctionType fnType, + const clang::CXXConstructorDecl *cxxCtorDecl = nullptr); + Signature + getSignature(CanSILFunctionType fnType, FunctionPointerKind kind, + bool forStaticCall = false, + const clang::CXXConstructorDecl *cxxCtorDecl = nullptr); llvm::FunctionType *getFunctionType(CanSILFunctionType type, llvm::AttributeList &attrs, ForeignFunctionInfo *foreignInfo=nullptr); diff --git a/lib/IRGen/IRGenSIL.cpp b/lib/IRGen/IRGenSIL.cpp index a307d22ce2ca1..e24b5ad589302 100644 --- a/lib/IRGen/IRGenSIL.cpp +++ b/lib/IRGen/IRGenSIL.cpp @@ -2969,13 +2969,13 @@ void IRGenSILFunction::visitFunctionRefBaseInst(FunctionRefBaseInst *i) { auto fnType = fn->getLoweredFunctionType(); auto fpKind = irgen::classifyFunctionPointerKind(fn); - bool isCXXConstructor = false; + const clang::CXXConstructorDecl *cxxCtorDecl = nullptr; if (auto *clangFnDecl = fn->getClangDecl()) - isCXXConstructor = isa(clangFnDecl); + cxxCtorDecl = dyn_cast(clangFnDecl); - auto sig = IGM.getSignature(fnType, fpKind, true /*forStaticCall*/, - isCXXConstructor); + auto sig = + IGM.getSignature(fnType, fpKind, true /*forStaticCall*/, cxxCtorDecl); // Note that the pointer value returned by getAddrOfSILFunction doesn't // necessarily have element type sig.getType(), e.g. if it's imported. diff --git a/lib/IRGen/Signature.h b/lib/IRGen/Signature.h index d1c5739f13180..53c4fa1416a05 100644 --- a/lib/IRGen/Signature.h +++ b/lib/IRGen/Signature.h @@ -31,6 +31,7 @@ namespace llvm { } namespace clang { + class CXXConstructorDecl; namespace CodeGen { class CGFunctionInfo; } @@ -202,10 +203,10 @@ class Signature { /// This is a private detail of the implementation of /// IRGenModule::getSignature(CanSILFunctionType), which is what /// clients should generally be using. - static Signature getUncached(IRGenModule &IGM, CanSILFunctionType formalType, - FunctionPointerKind kind, - bool forStaticCall = false, - bool forCXXConstructorCall = false); + static Signature + getUncached(IRGenModule &IGM, CanSILFunctionType formalType, + FunctionPointerKind kind, bool forStaticCall = false, + const clang::CXXConstructorDecl *cxxCtorDecl = nullptr); static SignatureExpansionABIDetails getUncachedABIDetails(IRGenModule &IGM, CanSILFunctionType formalType, From d444a754493b140d160ff7183cc4bec11dd33083 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Wed, 14 Feb 2024 13:48:33 -0800 Subject: [PATCH 6/7] Compute the return type of c++ constructors without calling arrangeCXXStructorDeclaration --- lib/IRGen/GenCall.cpp | 42 ++++++++++++++++------------------------- lib/IRGen/GenDecl.cpp | 5 ++++- lib/IRGen/GenStruct.cpp | 2 +- 3 files changed, 21 insertions(+), 28 deletions(-) diff --git a/lib/IRGen/GenCall.cpp b/lib/IRGen/GenCall.cpp index 1c8e3d7f4d74a..42581c7629f87 100644 --- a/lib/IRGen/GenCall.cpp +++ b/lib/IRGen/GenCall.cpp @@ -1375,8 +1375,12 @@ void SignatureExpansion::expandExternalSignatureTypes() { IGM.getSILModule(), FnType, TypeExpansionContext::minimal())); }(); - // Convert the SIL result type to a Clang type. - auto clangResultTy = IGM.getClangType(SILResultTy); + // Convert the SIL result type to a Clang type. If this is for a c++ + // constructor, use 'void' as the return type to arrange the function type. + auto clangResultTy = IGM.getClangType( + cxxCtorDecl + ? SILType::getPrimitiveObjectType(IGM.Context.TheEmptyTupleType) + : SILResultTy); // Now convert the parameters to Clang types. auto params = FnType->getParameters(); @@ -1445,14 +1449,10 @@ void SignatureExpansion::expandExternalSignatureTypes() { // Generate function info for this signature. auto extInfo = clang::FunctionType::ExtInfo(); - if (cxxCtorDecl) - ForeignInfo.ClangInfo = &clang::CodeGen::arrangeCXXStructorDeclaration( - IGM.ClangCodeGen->CGM(), {cxxCtorDecl, clang::Ctor_Complete}); - else - ForeignInfo.ClangInfo = &clang::CodeGen::arrangeFreeFunctionCall( - IGM.ClangCodeGen->CGM(), clangResultTy, paramTys, extInfo, - clang::CodeGen::RequiredArgs::All); - auto &FI = *ForeignInfo.ClangInfo; + auto &FI = clang::CodeGen::arrangeFreeFunctionCall(IGM.ClangCodeGen->CGM(), + clangResultTy, paramTys, extInfo, + clang::CodeGen::RequiredArgs::All); + ForeignInfo.ClangInfo = &FI; assert(FI.arg_size() == paramTys.size() && "Expected one ArgInfo for each parameter type!"); @@ -1596,7 +1596,12 @@ void SignatureExpansion::expandExternalSignatureTypes() { for (auto i : indices(paramTys).slice(firstParamToLowerNormally)) emitArg(i); - if (returnInfo.isIndirect() || returnInfo.isIgnore()) { + if (cxxCtorDecl) { + ResultIRType = cast(IGM.getAddrOfClangGlobalDecl( + {cxxCtorDecl, clang::Ctor_Complete}, + (ForDefinition_t) false)) + ->getReturnType(); + } else if (returnInfo.isIndirect() || returnInfo.isIgnore()) { ResultIRType = IGM.VoidTy; } else { ResultIRType = returnInfo.getCoerceToType(); @@ -3996,12 +4001,6 @@ static void externalizeArguments(IRGenFunction &IGF, const Callee &callee, // swiftcall function pointers through SIL as C functions anyway. assert(FI.getExtParameterInfo(i).getABI() == clang::ParameterABI::Ordinary); - assert((!silConv.isSILIndirect(params[i - firstParam]) || - AI.getKind() == clang::CodeGen::ABIArgInfo::Direct || - AI.getKind() == clang::CodeGen::ABIArgInfo::Indirect) && - "indirect SIL types passed indirectly should be classified as " - "either Direct or Indirect"); - // Add a padding argument if required. if (auto *padType = AI.getPaddingType()) out.add(llvm::UndefValue::get(padType)); @@ -4053,15 +4052,6 @@ static void externalizeArguments(IRGenFunction &IGF, const Callee &callee, case clang::CodeGen::ABIArgInfo::IndirectAliased: llvm_unreachable("not implemented"); case clang::CodeGen::ABIArgInfo::Indirect: { - // If this is a SIL type passed indirectly, avoid emitting a redundant - // initializing copy. - if (silConv.isSILIndirect(params[i - firstParam])) { - assert(paramType.isAddress() && "SIL type is not an address?"); - auto addr = in.claimNext(); - out.add(addr); - break; - } - auto &ti = cast(IGF.getTypeInfo(paramType)); auto temp = ti.allocateStack(IGF, paramType, "indirect-temporary"); diff --git a/lib/IRGen/GenDecl.cpp b/lib/IRGen/GenDecl.cpp index 34f3f0a648daa..3811e49d0647e 100644 --- a/lib/IRGen/GenDecl.cpp +++ b/lib/IRGen/GenDecl.cpp @@ -3511,7 +3511,10 @@ llvm::Constant *swift::irgen::emitCXXConstructorThunkIfNeeded( emitCXXConstructorCall(subIGF, ctor, ctorFnType, ctorAddress, Args); if (isa(call)) IGM.emittedForeignFunctionThunksWithExceptionTraps.insert(thunk); - subIGF.Builder.CreateRetVoid(); + if (ctorFnType->getReturnType()->isVoidTy()) + subIGF.Builder.CreateRetVoid(); + else + subIGF.Builder.CreateRet(call); return thunk; } diff --git a/lib/IRGen/GenStruct.cpp b/lib/IRGen/GenStruct.cpp index 9f03f36814362..e7e105d058d31 100644 --- a/lib/IRGen/GenStruct.cpp +++ b/lib/IRGen/GenStruct.cpp @@ -620,7 +620,7 @@ namespace { auto clangFnAddr = IGF.IGM.getAddrOfClangGlobalDecl(globalDecl, NotForDefinition); auto callee = cast(clangFnAddr->stripPointerCasts()); - Signature signature = IGF.IGM.getSignature(fnType); + Signature signature = IGF.IGM.getSignature(fnType, copyConstructor); std::string name = "__swift_cxx_copy_ctor" + callee->getName().str(); auto *origClangFnAddr = clangFnAddr; clangFnAddr = emitCXXConstructorThunkIfNeeded( From a87806ba7aaedd767a799d5e36c943afc2de076c Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Thu, 22 Feb 2024 10:11:47 -0800 Subject: [PATCH 7/7] Fix sret type in test case --- .../method/methods-this-and-indirect-return-irgen-msvc.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-msvc.swift b/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-msvc.swift index 71da6c5f936ab..333f94e30ded1 100644 --- a/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-msvc.swift +++ b/test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-msvc.swift @@ -12,7 +12,7 @@ public func use() -> CInt { // CHECK: %[[instance:.*]] = alloca %TSo10HasMethodsV // CHECK: %[[result:.*]] = alloca %TSo19NonTrivialInWrapperV -// CHECK: call void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(ptr %[[instance]], ptr noalias sret(%struct.NonTrivialInWrapper) %[[result]], i32 42) +// CHECK: call void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(ptr %[[instance]], ptr noalias sret(%TSo19NonTrivialInWrapperV) %[[result]], i32 42) // CHECK-x86_64: define {{.*}} void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(ptr {{.*}} %{{.*}}, ptr noalias sret(%struct.NonTrivialInWrapper) {{.*}} %{{.*}}, i32 noundef %{{.*}}) // CHECK-aarch64: define {{.*}} void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(ptr {{.*}} %{{.*}}, ptr inreg noalias sret(%struct.NonTrivialInWrapper) {{.*}} %{{.*}}, i32 noundef %{{.*}})