From fcc1f6b65e63b514916cfead96e11de4e498c701 Mon Sep 17 00:00:00 2001 From: Hiroshi Yamauchi Date: Thu, 24 Oct 2024 12:33:58 -0700 Subject: [PATCH] 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. --- 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 82311a91db10d..3fbde6b08e92c 100644 --- a/lib/ClangImporter/ImportType.cpp +++ b/lib/ClangImporter/ImportType.cpp @@ -2203,10 +2203,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 5669339fb2c55..f48dd4a7c1d49 100644 --- a/lib/IRGen/GenCall.cpp +++ b/lib/IRGen/GenCall.cpp @@ -1471,9 +1471,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() && @@ -1595,9 +1601,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); @@ -2577,11 +2581,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(); } @@ -3171,9 +3176,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. @@ -3566,10 +3572,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 ceb26fb064b91..9a27e5c97da9e 100644 --- a/test/Interop/Cxx/class/method/Inputs/module.modulemap +++ b/test/Interop/Cxx/class/method/Inputs/module.modulemap @@ -19,3 +19,8 @@ module SRetWinARM64 { header "sret-win-arm64.h" requires cplusplus } + +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 7dacd0835f103..ebc02d41c1ecf 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)