From 46eb3e1fedd017b03b5d34c0da32be846e397c9d Mon Sep 17 00:00:00 2001 From: Hiroshi Yamauchi Date: Mon, 28 Oct 2024 14:25:02 -0700 Subject: [PATCH 1/4] Add the inreg attribute to sreg when present. On Windows/AArch64, a different register is used between when an arugment is both inreg and sret (X0 or X1) and when it is just sret (X8) as the following comment indicates: https://github.com/llvm/llvm-project/blob/46fe36a4295f05d5d3731762e31fc4e6e99863e9/llvm/lib/Target/AArch64/AArch64CallingConvention.td#L42 ``` // In AAPCS, an SRet is passed in X8, not X0 like a normal pointer parameter. // However, on windows, in some circumstances, the SRet is passed in X0 or X1 // instead. The presence of the inreg attribute indicates that SRet is // passed in the alternative register (X0 or X1), not X8: // - X0 for non-instance methods. // - X1 for instance methods. // The "sret" attribute identifies indirect returns. // The "inreg" attribute identifies non-aggregate types. // The position of the "sret" attribute identifies instance/non-instance // methods. // "sret" on argument 0 means non-instance methods. // "sret" on argument 1 means instance methods. CCIfInReg>>>>, CCIfSRet>>, ``` So missing/dropping inreg can cause a codegen bug. This is a partial fix for #74866 Cherrypick https://github.com/swiftlang/swift/pull/76159 --- lib/IRGen/GenCall.cpp | 16 ++++++++++------ ...ods-this-and-indirect-return-irgen-msvc.swift | 3 ++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/IRGen/GenCall.cpp b/lib/IRGen/GenCall.cpp index 4dceadb3c14cf..6bda5db6e73ac 100644 --- a/lib/IRGen/GenCall.cpp +++ b/lib/IRGen/GenCall.cpp @@ -358,7 +358,8 @@ static void addIndirectResultAttributes(IRGenModule &IGM, llvm::AttributeList &attrs, unsigned paramIndex, bool allowSRet, llvm::Type *storageType, - const TypeInfo &typeInfo) { + const TypeInfo &typeInfo, + bool useInReg = false) { llvm::AttrBuilder b(IGM.getLLVMContext()); b.addAttribute(llvm::Attribute::NoAlias); // Bitwise takable value types are guaranteed not to capture @@ -368,6 +369,8 @@ static void addIndirectResultAttributes(IRGenModule &IGM, if (allowSRet) { assert(storageType); b.addStructRetAttr(storageType); + if (useInReg) + b.addAttribute(llvm::Attribute::InReg); } attrs = attrs.addParamAttributes(IGM.getLLVMContext(), paramIndex, b); } @@ -475,7 +478,7 @@ namespace { private: const TypeInfo &expand(SILParameterInfo param); - llvm::Type *addIndirectResult(SILType resultType); + llvm::Type *addIndirectResult(SILType resultType, bool useInReg = false); SILFunctionConventions getSILFuncConventions() const { return SILFunctionConventions(FnType, IGM.getSILModule()); @@ -533,11 +536,12 @@ namespace { } // end namespace irgen } // end namespace swift -llvm::Type *SignatureExpansion::addIndirectResult(SILType resultType) { +llvm::Type *SignatureExpansion::addIndirectResult(SILType resultType, + bool useInReg) { const TypeInfo &resultTI = IGM.getTypeInfo(resultType); auto storageTy = resultTI.getStorageType(); addIndirectResultAttributes(IGM, Attrs, ParamIRTypes.size(), claimSRet(), - storageTy, resultTI); + storageTy, resultTI, useInReg); addPointerParameter(storageTy); return IGM.VoidTy; } @@ -1580,9 +1584,9 @@ void SignatureExpansion::expandExternalSignatureTypes() { // returned indirect values. emitArg(0); firstParamToLowerNormally = 1; - addIndirectResult(resultType); + addIndirectResult(resultType, returnInfo.getInReg()); } else - addIndirectResult(resultType); + addIndirectResult(resultType, returnInfo.getInReg()); } // Use a special IR type for passing block pointers. 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 333f94e30ded1..29c8bf3528c75 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,8 @@ 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(%TSo19NonTrivialInWrapperV) %[[result]], i32 42) +// CHECK-x86_64: call void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(ptr %[[instance]], ptr noalias sret(%TSo19NonTrivialInWrapperV) %[[result]], i32 42) +// CHECK-aarch64: call void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(ptr %[[instance]], ptr inreg 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 %{{.*}}) From b7dad2a35d536df7b01f52f5ae5c08d7f9bdc1cb Mon Sep 17 00:00:00 2001 From: Hiroshi Yamauchi Date: Mon, 28 Oct 2024 14:29:05 -0700 Subject: [PATCH 2/4] Fix the IR gen for C++ method calls and refactor around CGFunctionInfo In GenCall, fix the IR gen for C++ method calls as under MSVC as the calling conventions for free functions and C++ methods can be different. This also fixes the missing inreg (on sret arguments) issues on Windows ARM64. Also refactor to use CGFunctionInfo returnInfo isSretAfterThis to detect when to reorder the sret and the this arguments under MSVC. In ClagImporter, don't drop the return type for the compound assignment operators such as operator+= when the return value is a reference so that the CGFunctionInfo will be correctly indicate an indirect return for the compound assignment operators. Cherrypick https://github.com/swiftlang/swift/pull/76324 --- lib/ClangImporter/ImportType.cpp | 9 +++-- lib/IRGen/GenCall.cpp | 38 +++++++++++-------- .../Cxx/class/method/Inputs/inreg-sret.h | 11 ++++++ .../Cxx/class/method/Inputs/module.modulemap | 5 +++ .../Interop/Cxx/class/method/inreg-sret.swift | 28 ++++++++++++++ ...c-abi-return-indirect-trivial-record.swift | 10 ++--- .../Cxx/operators/member-inline-irgen.swift | 2 +- .../operators/member-out-of-line-irgen.swift | 2 +- 8 files changed, 78 insertions(+), 27 deletions(-) create mode 100644 test/Interop/Cxx/class/method/Inputs/inreg-sret.h create mode 100644 test/Interop/Cxx/class/method/inreg-sret.swift diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp index 7083371b818da..a603fe9d577e5 100644 --- a/lib/ClangImporter/ImportType.cpp +++ b/lib/ClangImporter/ImportType.cpp @@ -2204,10 +2204,11 @@ ImportedType ClangImporter::Implementation::importFunctionReturnType( // C++ operators +=, -=, *=, /= may return a reference to self. This is not // idiomatic in Swift, let's drop these return values. clang::OverloadedOperatorKind op = clangDecl->getOverloadedOperator(); - if (op == clang::OverloadedOperatorKind::OO_PlusEqual || - op == clang::OverloadedOperatorKind::OO_MinusEqual || - op == clang::OverloadedOperatorKind::OO_StarEqual || - op == clang::OverloadedOperatorKind::OO_SlashEqual) + if ((op == clang::OverloadedOperatorKind::OO_PlusEqual || + op == clang::OverloadedOperatorKind::OO_MinusEqual || + op == clang::OverloadedOperatorKind::OO_StarEqual || + op == clang::OverloadedOperatorKind::OO_SlashEqual) && + clangDecl->getReturnType()->isReferenceType()) return {SwiftContext.getVoidType(), false}; // Fix up optionality. diff --git a/lib/IRGen/GenCall.cpp b/lib/IRGen/GenCall.cpp index 6bda5db6e73ac..d3ccb6e8e9e0e 100644 --- a/lib/IRGen/GenCall.cpp +++ b/lib/IRGen/GenCall.cpp @@ -1453,9 +1453,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); + bool isCXXMethod = + FnType->getRepresentation() == SILFunctionTypeRepresentation::CXXMethod; + auto &FI = isCXXMethod ? + clang::CodeGen::arrangeCXXMethodCall(IGM.ClangCodeGen->CGM(), + clangResultTy, paramTys, extInfo, {}, + clang::CodeGen::RequiredArgs::All) : + clang::CodeGen::arrangeFreeFunctionCall(IGM.ClangCodeGen->CGM(), + clangResultTy, paramTys, extInfo, {}, + clang::CodeGen::RequiredArgs::All); ForeignInfo.ClangInfo = &FI; assert(FI.arg_size() == paramTys.size() && @@ -1577,9 +1583,7 @@ void SignatureExpansion::expandExternalSignatureTypes() { if (returnInfo.isIndirect()) { auto resultType = getSILFuncConventions().getSingleSILResultType( IGM.getMaximalTypeExpansionContext()); - if (IGM.Triple.isWindowsMSVCEnvironment() && - FnType->getRepresentation() == - SILFunctionTypeRepresentation::CXXMethod) { + if (returnInfo.isSRetAfterThis()) { // Windows ABI places `this` before the // returned indirect values. emitArg(0); @@ -2538,11 +2542,12 @@ class SyncCallEmission final : public CallEmission { // Windows ABI places `this` before the // returned indirect values. - bool isThisFirst = IGF.IGM.Triple.isWindowsMSVCEnvironment(); - if (!isThisFirst) + auto &returnInfo = + getCallee().getForeignInfo().ClangInfo->getReturnInfo(); + if (returnInfo.isIndirect() && !returnInfo.isSRetAfterThis()) passIndirectResults(); adjusted.add(arg); - if (isThisFirst) + if (returnInfo.isIndirect() && returnInfo.isSRetAfterThis()) passIndirectResults(); } @@ -3119,9 +3124,10 @@ void CallEmission::emitToUnmappedMemory(Address result) { assert(LastArgWritten == 1 && "emitting unnaturally to indirect result"); Args[0] = result.getAddress(); - if (IGF.IGM.Triple.isWindowsMSVCEnvironment() && - getCallee().getRepresentation() == - SILFunctionTypeRepresentation::CXXMethod && + + auto *FI = getCallee().getForeignInfo().ClangInfo; + if (FI && FI->getReturnInfo().isIndirect() && + FI->getReturnInfo().isSRetAfterThis() && Args[1] == getCallee().getCXXMethodSelf()) { // C++ methods in MSVC ABI pass `this` before the // indirectly returned value. @@ -3486,10 +3492,10 @@ void CallEmission::emitToExplosion(Explosion &out, bool isOutlined) { emitToMemory(temp, substResultTI, isOutlined); return; } - if (IGF.IGM.Triple.isWindowsMSVCEnvironment() && - getCallee().getRepresentation() == - SILFunctionTypeRepresentation::CXXMethod && - substResultType.isVoid()) { + + auto *FI = getCallee().getForeignInfo().ClangInfo; + if (FI && FI->getReturnInfo().isIndirect() && + FI->getReturnInfo().isSRetAfterThis() && substResultType.isVoid()) { // Some C++ methods return a value but are imported as // returning `Void` (e.g. `operator +=`). In this case // we should allocate the correct temp indirect return diff --git a/test/Interop/Cxx/class/method/Inputs/inreg-sret.h b/test/Interop/Cxx/class/method/Inputs/inreg-sret.h new file mode 100644 index 0000000000000..45111faa42f8d --- /dev/null +++ b/test/Interop/Cxx/class/method/Inputs/inreg-sret.h @@ -0,0 +1,11 @@ +#ifndef TEST_INTEROP_CXX_CLASS_METHOD_INREG_SRET_H +#define TEST_INTEROP_CXX_CLASS_METHOD_INREG_SRET_H + +struct OptionalBridgedBasicBlock { +}; + +struct BridgedFunction { + OptionalBridgedBasicBlock getFirstBlock() const { return {}; } +}; + +#endif // TEST_INTEROP_CXX_CLASS_METHOD_INREG_SRET_H diff --git a/test/Interop/Cxx/class/method/Inputs/module.modulemap b/test/Interop/Cxx/class/method/Inputs/module.modulemap index 20e5779e5a331..4d4f4b7bead1d 100644 --- a/test/Interop/Cxx/class/method/Inputs/module.modulemap +++ b/test/Interop/Cxx/class/method/Inputs/module.modulemap @@ -14,3 +14,8 @@ module UnsafeProjections { export * } + +module InRegSRet { + header "inreg-sret.h" + requires cplusplus +} diff --git a/test/Interop/Cxx/class/method/inreg-sret.swift b/test/Interop/Cxx/class/method/inreg-sret.swift new file mode 100644 index 0000000000000..1188f2f358d8a --- /dev/null +++ b/test/Interop/Cxx/class/method/inreg-sret.swift @@ -0,0 +1,28 @@ +// RUN: %target-swift-emit-irgen -I %S/Inputs -cxx-interoperability-mode=default %s | %FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-%target-cpu + +// REQUIRES: OS=windows-msvc + +import InRegSRet + +final public class BasicBlock { +} + +extension OptionalBridgedBasicBlock { + public var block: BasicBlock? { nil } +} + +final public class Function { + public var bridged: BridgedFunction { + BridgedFunction() + } + + public var firstBlock : BasicBlock? { bridged.getFirstBlock().block } +} + +// Check that inreg on the sret isn't missing + +// CHECK-x86_64: call void @"?getFirstBlock@BridgedFunction@@QEBA?AUOptionalBridgedBasicBlock@@XZ"(ptr {{.*}}, ptr noalias nocapture sret(%TSo25OptionalBridgedBasicBlockV) {{.*}}) +// CHECK-aarch64: call void @"?getFirstBlock@BridgedFunction@@QEBA?AUOptionalBridgedBasicBlock@@XZ"(ptr {{.*}}, ptr inreg noalias nocapture sret(%TSo25OptionalBridgedBasicBlockV) {{.*}}) + +// CHECK-x86_64: define {{.*}} void @"?getFirstBlock@BridgedFunction@@QEBA?AUOptionalBridgedBasicBlock@@XZ"(ptr {{.*}} %{{.*}}, ptr noalias sret(%struct.OptionalBridgedBasicBlock) {{.*}} %{{.*}}) +// CHECK-aarch64: define {{.*}} void @"?getFirstBlock@BridgedFunction@@QEBA?AUOptionalBridgedBasicBlock@@XZ"(ptr {{.*}} %{{.*}}, ptr inreg noalias sret(%struct.OptionalBridgedBasicBlock) {{.*}} %{{.*}}) diff --git a/test/Interop/Cxx/class/method/msvc-abi-return-indirect-trivial-record.swift b/test/Interop/Cxx/class/method/msvc-abi-return-indirect-trivial-record.swift index 3a0e8ba51aa60..7587abb3d1dfe 100644 --- a/test/Interop/Cxx/class/method/msvc-abi-return-indirect-trivial-record.swift +++ b/test/Interop/Cxx/class/method/msvc-abi-return-indirect-trivial-record.swift @@ -57,7 +57,7 @@ public func test(_ result: VecStr) -> CInt { } // CHECK: swiftcc i32 @"$s4Test4testys5Int32VSo0014vecstr_yuJCataVF"({{.*}} %[[RESULT:.*]]) -// CHECK: call void @"?begin@?$vec@Vstr@@@@QEBA?AVIt@@XZ"(ptr %[[RESULT]], ptr sret{{.*}} +// CHECK: call void @"?begin@?$vec@Vstr@@@@QEBA?AVIt@@XZ"(ptr %[[RESULT]], ptr {{.*}} sret{{.*}} public func passTempForIndirectRetToVoidCall() { var lhs = LoadableIntWrapper(value: 2) @@ -65,7 +65,7 @@ public func passTempForIndirectRetToVoidCall() { lhs += rhs } -// CHECK: void @"$sSo18LoadableIntWrapperV2peoiyyABz_ABtFZ"(ptr -// CHECK: %[[OPRESULT:.*]] = alloca %struct.LoadableIntWrapper, align 16 -// CHECK-x86_64: call void @"??YLoadableIntWrapper@@QEAA?AU0@U0@@Z"(ptr {{.*}}, ptr sret(%struct.LoadableIntWrapper) %[[OPRESULT]], i32 -// CHECK-aarch64: call void @"??YLoadableIntWrapper@@QEAA?AU0@U0@@Z"(ptr {{.*}}, ptr sret(%struct.LoadableIntWrapper) %[[OPRESULT]], i64 +// CHECK: i32 @"$sSo18LoadableIntWrapperV2peoiyA2Bz_ABtFZ"(ptr +// CHECK: %[[OPRESULT:.*]] = alloca %TSo18LoadableIntWrapperV, align 4 +// CHECK-x86_64: call void @"??YLoadableIntWrapper@@QEAA?AU0@U0@@Z"(ptr {{.*}}, ptr {{.*}} sret(%TSo18LoadableIntWrapperV) %[[OPRESULT]], i32 +// CHECK-aarch64: call void @"??YLoadableIntWrapper@@QEAA?AU0@U0@@Z"(ptr {{.*}}, ptr {{.*}} sret(%TSo18LoadableIntWrapperV) %[[OPRESULT]], i64 diff --git a/test/Interop/Cxx/operators/member-inline-irgen.swift b/test/Interop/Cxx/operators/member-inline-irgen.swift index af442111061dd..c3f5472ab4ba7 100644 --- a/test/Interop/Cxx/operators/member-inline-irgen.swift +++ b/test/Interop/Cxx/operators/member-inline-irgen.swift @@ -5,7 +5,7 @@ import MemberInline public func sub(_ lhs: inout LoadableIntWrapper, _ rhs: LoadableIntWrapper) -> LoadableIntWrapper { lhs - rhs } // CHECK-SYSV: call [[RESA:i32|i64]] [[NAMEA:@_ZN18LoadableIntWrappermiES_]](ptr {{%[0-9]+}}, {{i32|\[1 x i32\]|i64|%struct.LoadableIntWrapper\* byval\(.*\) align 4}} {{%[0-9]+}}) -// CHECK-WIN: call [[RESA:void]] [[NAMEA:@"\?\?GLoadableIntWrapper@@QEAA\?AU0@U0@@Z"]](ptr {{%[0-9]+}}, ptr sret(%struct.LoadableIntWrapper) {{.*}}, i32 {{%[0-9]+}}) +// CHECK-WIN: call [[RESA:void]] [[NAMEA:@"\?\?GLoadableIntWrapper@@QEAA\?AU0@U0@@Z"]](ptr {{%[0-9]+}}, ptr {{.*}} sret(%TSo18LoadableIntWrapperV) {{.*}}, i32 {{%[0-9]+}}) public func call(_ wrapper: inout LoadableIntWrapper, _ arg: Int32) -> Int32 { wrapper(arg) } diff --git a/test/Interop/Cxx/operators/member-out-of-line-irgen.swift b/test/Interop/Cxx/operators/member-out-of-line-irgen.swift index a283f348f783a..0dbb058f2a29f 100644 --- a/test/Interop/Cxx/operators/member-out-of-line-irgen.swift +++ b/test/Interop/Cxx/operators/member-out-of-line-irgen.swift @@ -7,5 +7,5 @@ public func add(_ lhs: inout LoadableIntWrapper, _ rhs: LoadableIntWrapper) -> L // CHECK-SYSV: call {{i32|i64}} [[NAME:@_ZNK18LoadableIntWrapperplES_]](ptr %{{[0-9]+}}, {{i32|\[1 x i32\]|i64|%struct.LoadableIntWrapper\* byval\(.*\)}}{{.*}}) // CHECK-SYSV: declare {{.*}}{{i32|i64}} [[NAME]](ptr {{.*}}, {{i32|\[1 x i32\]|i64|%struct.LoadableIntWrapper\* .*byval\(%struct.LoadableIntWrapper\)}}{{.*}}) -// CHECK-WIN: call void [[NAME:@"\?\?HLoadableIntWrapper@@QEBA\?AU0@U0@@Z"]](ptr %{{[0-9]+}}, ptr sret(%struct.LoadableIntWrapper) {{.*}}, i32 %{{[0-9]+}}) +// CHECK-WIN: call void [[NAME:@"\?\?HLoadableIntWrapper@@QEBA\?AU0@U0@@Z"]](ptr %{{[0-9]+}}, ptr {{.*}} sret(%TSo18LoadableIntWrapperV) {{.*}}, i32 %{{[0-9]+}}) // CHECK-WIN: declare dso_local void [[NAME]](ptr {{.*}}, ptr sret(%struct.LoadableIntWrapper) {{.*}}, i32) From 6f552e00f22cc807f4d3be942433c7ebe331bd75 Mon Sep 17 00:00:00 2001 From: Hiroshi Yamauchi Date: Mon, 28 Oct 2024 14:31:08 -0700 Subject: [PATCH 3/4] Ensure that BridgedTypeArray is indirectly returned On Windows ARM64, how a struct value type is returned is sensitive to conditions including whether a user-defined constructor exists, etc. See https://learn.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=msvc-170#return-values That caused a calling convention mismatch between the non-USED_IN_CPP_SOURCE (Swift) side and the USE_IN_CPP_SOURCE (C++) side and a crash. Add this constructor so that the calling convention matches. This is a fix for the OnoneSimplification crash in and is a partial fix for Cherrypick https://github.com/swiftlang/swift/pull/76433 --- include/swift/SIL/SILBridging.h | 4 + .../Cxx/class/method/Inputs/module.modulemap | 5 ++ .../Cxx/class/method/Inputs/sret-win-arm64.h | 84 +++++++++++++++++++ .../Cxx/class/method/sret-win-arm64.swift | 31 +++++++ 4 files changed, 124 insertions(+) create mode 100644 test/Interop/Cxx/class/method/Inputs/sret-win-arm64.h create mode 100644 test/Interop/Cxx/class/method/sret-win-arm64.swift diff --git a/include/swift/SIL/SILBridging.h b/include/swift/SIL/SILBridging.h index 22d5756522df2..4a3d0e7759230 100644 --- a/include/swift/SIL/SILBridging.h +++ b/include/swift/SIL/SILBridging.h @@ -674,6 +674,10 @@ struct BridgedSubstitutionMap { struct BridgedTypeArray { BridgedArrayRef typeArray; + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64, + BridgedTypeArray() : typeArray() {} + #ifdef USED_IN_CPP_SOURCE BridgedTypeArray(llvm::ArrayRef types) : typeArray(types) {} diff --git a/test/Interop/Cxx/class/method/Inputs/module.modulemap b/test/Interop/Cxx/class/method/Inputs/module.modulemap index 4d4f4b7bead1d..0959de2a1895f 100644 --- a/test/Interop/Cxx/class/method/Inputs/module.modulemap +++ b/test/Interop/Cxx/class/method/Inputs/module.modulemap @@ -19,3 +19,8 @@ module InRegSRet { header "inreg-sret.h" requires cplusplus } + +module SRetWinARM64 { + header "sret-win-arm64.h" + requires cplusplus +} diff --git a/test/Interop/Cxx/class/method/Inputs/sret-win-arm64.h b/test/Interop/Cxx/class/method/Inputs/sret-win-arm64.h new file mode 100644 index 0000000000000..c9abcc04d6168 --- /dev/null +++ b/test/Interop/Cxx/class/method/Inputs/sret-win-arm64.h @@ -0,0 +1,84 @@ +#ifndef TEST_INTEROP_CXX_CLASS_METHOD_SRET_WIN_ARM64_H +#define TEST_INTEROP_CXX_CLASS_METHOD_SRET_WIN_ARM64_H + +#include + +namespace llvm { + +template +class ArrayRef { +public: + const T *Data = nullptr; + size_t Length = 0; +}; + +} // namespace llvm + +namespace swift { + +class Type { +}; + +class SubstitutionMap { +private: + void *storage = nullptr; + +public: + llvm::ArrayRef getReplacementTypes() const; +}; + +} // namespace swift + +class BridgedArrayRef { +public: + const void * Data; + size_t Length; + + BridgedArrayRef() : Data(nullptr), Length(0) {} + +#ifdef USED_IN_CPP_SOURCE + template + BridgedArrayRef(llvm::ArrayRef arr) + : Data(arr.Data), Length(arr.Length) {} + + template + llvm::ArrayRef unbridged() const { + return {static_cast(Data), Length}; + } +#endif +}; + +struct BridgedSubstitutionMap { + uint64_t storage[1]; + +#ifdef USED_IN_CPP_SOURCE + BridgedSubstitutionMap(swift::SubstitutionMap map) { + *reinterpret_cast(&storage) = map; + } + swift::SubstitutionMap unbridged() const { + return *reinterpret_cast(&storage); + } +#endif + + BridgedSubstitutionMap() {} +}; + +struct BridgedTypeArray { + BridgedArrayRef typeArray; + +#ifdef AFTER_FIX + BridgedTypeArray() : typeArray() {} +#endif + +#ifdef USED_IN_CPP_SOURCE + BridgedTypeArray(llvm::ArrayRef types) : typeArray(types) {} + + llvm::ArrayRef unbridged() const { + return typeArray.unbridged(); + } +#endif + + static BridgedTypeArray fromReplacementTypes(BridgedSubstitutionMap substMap); +}; + +#endif // TEST_INTEROP_CXX_CLASS_METHOD_SRET_WIN_ARM64_H diff --git a/test/Interop/Cxx/class/method/sret-win-arm64.swift b/test/Interop/Cxx/class/method/sret-win-arm64.swift new file mode 100644 index 0000000000000..bd33cd34af01f --- /dev/null +++ b/test/Interop/Cxx/class/method/sret-win-arm64.swift @@ -0,0 +1,31 @@ +// RUN: %target-swift-emit-irgen -I %S/Inputs -cxx-interoperability-mode=default %s | %FileCheck %s -check-prefix=CHECK-before-fix +// RUN: %target-swift-emit-irgen -I %S/Inputs -cxx-interoperability-mode=default %s -Xcc -DAFTER_FIX | %FileCheck %s -check-prefix=CHECK-after-fix + +// REQUIRES: OS=windows-msvc && CPU=aarch64 + +import SRetWinARM64 + +public struct OptionalTypeArray { + private let bridged: BridgedTypeArray + public init(bridged: BridgedTypeArray) { + self.bridged = bridged + } +} + +public struct SubstitutionMap { + public let bridged: BridgedSubstitutionMap + + public var replacementTypes: OptionalTypeArray { + let types = BridgedTypeArray.fromReplacementTypes(bridged) + return OptionalTypeArray(bridged: types) + } +} + +public func test(sm: SubstitutionMap) -> OptionalTypeArray { + return sm.replacementTypes +} + +// Check that BridgedTypeArray is indirectly returned via sret after the fix + +// CHECK-before-fix: declare {{.*}} [2 x i64] @"?fromReplacementTypes@BridgedTypeArray@@SA?AU1@UBridgedSubstitutionMap@@@Z"(i64) +// CHECK-after-fix: declare {{.*}} void @"?fromReplacementTypes@BridgedTypeArray@@SA?AU1@UBridgedSubstitutionMap@@@Z"(ptr inreg sret(%struct.BridgedTypeArray) align 8, i64) From 36f450c2311d36e39263b6e56531c49423ab0b7f Mon Sep 17 00:00:00 2001 From: Hiroshi Yamauchi Date: Mon, 28 Oct 2024 15:37:11 -0700 Subject: [PATCH 4/4] Ensure that bridged types are indirectly returned on Windows ARM64 On Windows ARM64, how a struct value type is returned is sensitive to conditions including whether a user-defined constructor exists, etc. See https://learn.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=msvc-170#return-values That caused a calling convention mismatch between the non-USED_IN_CPP_SOURCE (Swift) side and the USE_IN_CPP_SOURCE (C++) side and a crash. Following #76433 add constructors to several bridged C++ struct/class types so that the calling convention matches. This is a partial fix for #74866 Cherrypick https://github.com/swiftlang/swift/pull/76589 --- include/swift/AST/ASTBridging.h | 24 +++++++++++ include/swift/Basic/BasicBridging.h | 13 ++++++ include/swift/Parse/ParseBridging.h | 4 ++ include/swift/SIL/SILBridging.h | 40 +++++++++++++++++++ .../swift/SILOptimizer/OptimizerBridging.h | 4 ++ 5 files changed, 85 insertions(+) diff --git a/include/swift/AST/ASTBridging.h b/include/swift/AST/ASTBridging.h index 4f50c5dce5143..34df8de4f5a27 100644 --- a/include/swift/AST/ASTBridging.h +++ b/include/swift/AST/ASTBridging.h @@ -80,6 +80,10 @@ class BridgedDeclBaseName { BridgedIdentifier Ident; public: + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedDeclBaseName() : Ident() {} + #ifdef USED_IN_CPP_SOURCE BridgedDeclBaseName(swift::DeclBaseName baseName) : Ident(baseName.Ident) {} @@ -106,6 +110,10 @@ class BridgedDeclNameRef { void *_Nonnull opaque; public: + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedDeclNameRef() : opaque() {} + #ifdef USED_IN_CPP_SOURCE BridgedDeclNameRef(swift::DeclNameRef name) : opaque(name.getOpaqueValue()) {} @@ -162,6 +170,10 @@ class BridgedASTContext { swift::ASTContext * _Nonnull Ctx; public: + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedASTContext() : Ctx() {} + #ifdef USED_IN_CPP_SOURCE SWIFT_UNAVAILABLE("Use init(raw:) instead") BridgedASTContext(swift::ASTContext &ctx) : Ctx(&ctx) {} @@ -317,6 +329,10 @@ class BridgedDiagnosticArgument { int64_t storage[3]; public: + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedDiagnosticArgument() {} + #ifdef USED_IN_CPP_SOURCE BridgedDiagnosticArgument(const swift::DiagnosticArgument &arg) { *reinterpret_cast(&storage) = arg; @@ -334,6 +350,10 @@ class BridgedDiagnosticFixIt { int64_t storage[7]; public: + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedDiagnosticFixIt() {} + #ifdef USED_IN_CPP_SOURCE BridgedDiagnosticFixIt(const swift::DiagnosticInfo::FixIt &fixit){ *reinterpret_cast(&storage) = fixit; @@ -1333,6 +1353,10 @@ class BridgedStmtConditionElement { void *_Nonnull Raw; public: + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedStmtConditionElement() {} + #ifdef USED_IN_CPP_SOURCE BridgedStmtConditionElement(swift::StmtConditionElement elem) : Raw(elem.getOpaqueValue()) {} diff --git a/include/swift/Basic/BasicBridging.h b/include/swift/Basic/BasicBridging.h index 2b6849b6aadae..3197e89e190bf 100644 --- a/include/swift/Basic/BasicBridging.h +++ b/include/swift/Basic/BasicBridging.h @@ -23,6 +23,15 @@ // Pure bridging mode does not permit including any C++/llvm/swift headers. // See also the comments for `BRIDGING_MODE` in the top-level CMakeLists.txt file. // +// +// Note: On Windows ARM64, how a C++ struct/class value type is +// returned is sensitive to conditions including whether a +// user-defined constructor exists, etc. See +// https://learn.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=msvc-170#return-values +// So, if a C++ struct/class type is returned as a value between Swift +// and C++, we need to be careful to match the return convention +// matches between the non-USED_IN_CPP_SOURCE (Swift) side and the +// USE_IN_CPP_SOURCE (C++) side. #include "swift/Basic/BridgedSwiftObject.h" #include "swift/Basic/Compiler.h" @@ -228,6 +237,10 @@ class BridgedOwnedString { size_t Length; public: + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedOwnedString() {} + #ifdef USED_IN_CPP_SOURCE BridgedOwnedString(const std::string &stringToCopy); diff --git a/include/swift/Parse/ParseBridging.h b/include/swift/Parse/ParseBridging.h index fb48fc8be49b5..e18fcf2a780ca 100644 --- a/include/swift/Parse/ParseBridging.h +++ b/include/swift/Parse/ParseBridging.h @@ -30,6 +30,10 @@ class BridgedLegacyParser { swift::Parser *_Nonnull const handle; public: + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedLegacyParser() : handle(nullptr) {} + #ifdef USED_IN_CPP_SOURCE BridgedLegacyParser(swift::Parser &P) : handle(&P) {} diff --git a/include/swift/SIL/SILBridging.h b/include/swift/SIL/SILBridging.h index 4a3d0e7759230..a8c4ee14741b8 100644 --- a/include/swift/SIL/SILBridging.h +++ b/include/swift/SIL/SILBridging.h @@ -82,6 +82,10 @@ struct BridgedResultInfo { swift::TypeBase * _Nonnull type; BridgedResultConvention convention; + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedResultInfo() {} + #ifdef USED_IN_CPP_SOURCE inline static BridgedResultConvention castToResultConvention(swift::ResultConvention convention) { @@ -99,6 +103,10 @@ struct OptionalBridgedResultInfo { swift::TypeBase * _Nullable type = nullptr; BridgedResultConvention convention = BridgedResultConvention::Indirect; + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + OptionalBridgedResultInfo() {} + #ifdef USED_IN_CPP_SOURCE OptionalBridgedResultInfo(std::optional resultInfo) { if (resultInfo) { @@ -113,6 +121,10 @@ struct OptionalBridgedResultInfo { struct BridgedResultInfoArray { BridgedArrayRef resultInfoArray; + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedResultInfoArray() {} + #ifdef USED_IN_CPP_SOURCE BridgedResultInfoArray(llvm::ArrayRef results) : resultInfoArray(results) {} @@ -195,6 +207,10 @@ struct BridgedParameterInfo { struct BridgedParameterInfoArray { BridgedArrayRef parameterInfoArray; + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedParameterInfoArray() {} + #ifdef USED_IN_CPP_SOURCE BridgedParameterInfoArray(llvm::ArrayRef parameters) : parameterInfoArray(parameters) {} @@ -213,6 +229,10 @@ struct BridgedParameterInfoArray { struct BridgedYieldInfoArray { BridgedArrayRef yieldInfoArray; + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedYieldInfoArray() {} + #ifdef USED_IN_CPP_SOURCE BridgedYieldInfoArray(llvm::ArrayRef yields) : yieldInfoArray(yields) {} @@ -303,6 +323,10 @@ struct BridgedType { struct EnumElementIterator { uint64_t storage[4]; + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + EnumElementIterator() {} + #ifdef USED_IN_CPP_SOURCE EnumElementIterator(swift::EnumDecl::ElementRange::iterator i) { static_assert(sizeof(EnumElementIterator) >= sizeof(swift::EnumDecl::ElementRange::iterator)); @@ -316,6 +340,10 @@ struct BridgedType { SWIFT_IMPORT_UNSAFE BRIDGED_INLINE EnumElementIterator getNext() const; }; + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedType() {} + #ifdef USED_IN_CPP_SOURCE BridgedType(swift::SILType t) : opaqueValue(t.getOpaqueValue()) {} @@ -698,6 +726,10 @@ struct BridgedTypeArray { struct BridgedSILTypeArray { BridgedArrayRef typeArray; + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedSILTypeArray() {} + #ifdef USED_IN_CPP_SOURCE BridgedSILTypeArray(llvm::ArrayRef silTypes) : typeArray(silTypes) {} @@ -716,6 +748,10 @@ struct BridgedSILTypeArray { struct BridgedLocation { uint64_t storage[3]; + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + BridgedLocation() {} + #ifdef USED_IN_CPP_SOURCE BridgedLocation(const swift::SILDebugLocation &loc) { *reinterpret_cast(&storage) = loc; @@ -742,6 +778,10 @@ struct BridgedGenericSpecializationInformation { struct OptionalBridgedSILDebugVariable { uint64_t storage[16]; + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + OptionalBridgedSILDebugVariable() {} + #ifdef USED_IN_CPP_SOURCE using OptionalSILDebugVariable = std::optional; diff --git a/include/swift/SILOptimizer/OptimizerBridging.h b/include/swift/SILOptimizer/OptimizerBridging.h index f6e4d28cc8aa0..e88a8888c3f96 100644 --- a/include/swift/SILOptimizer/OptimizerBridging.h +++ b/include/swift/SILOptimizer/OptimizerBridging.h @@ -85,6 +85,10 @@ struct BridgedCalleeAnalysis { struct CalleeList { uint64_t storage[3]; + // Ensure that this struct value type will be indirectly returned on + // Windows ARM64 + CalleeList() {} + #ifdef USED_IN_CPP_SOURCE CalleeList(swift::CalleeList list) { *reinterpret_cast(&storage) = list;