Skip to content

Commit 77b5409

Browse files
hyphyp
committed
[cxx-interop] Fix the windows ABI for returning indirect values out of methods
Fixes #66326 This allows us to reneable Windows method tests. Note that Windows still has a broken convention for non-trivial record with non-trivial destructor but trivial copy-constructor, so classes in the methods.swift test need an explicit copy constructor. Fixes rdar://88391102
1 parent 68c464f commit 77b5409

File tree

6 files changed

+73
-27
lines changed

6 files changed

+73
-27
lines changed

‎lib/IRGen/GenCall.cpp

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2301,7 +2301,14 @@ class SyncCallEmission final : public CallEmission {
23012301
SILFunctionConventions fnConv(origCalleeType, IGF.getSILModule());
23022302

23032303
// Pass along the indirect result pointers.
2304-
original.transferInto(adjusted, fnConv.getNumIndirectSILResults());
2304+
auto passIndirectResults = [&]() {
2305+
original.transferInto(adjusted, fnConv.getNumIndirectSILResults());
2306+
};
2307+
// Indirect results for C++ methods can come
2308+
// after `this`.
2309+
if (getCallee().getRepresentation() !=
2310+
SILFunctionTypeRepresentation::CXXMethod)
2311+
passIndirectResults();
23052312

23062313
// Pass along the coroutine buffer.
23072314
switch (origCalleeType->getCoroutineKind()) {
@@ -2341,7 +2348,14 @@ class SyncCallEmission final : public CallEmission {
23412348
IGF.IGM.getPointerAlignment());
23422349
}
23432350

2351+
// Windows ABI places `this` before the
2352+
// returned indirect values.
2353+
bool isThisFirst = IGF.IGM.Triple.isWindowsMSVCEnvironment();
2354+
if (!isThisFirst)
2355+
passIndirectResults();
23442356
adjusted.add(arg);
2357+
if (isThisFirst)
2358+
passIndirectResults();
23452359
}
23462360

23472361
LLVM_FALLTHROUGH;

‎test/Interop/Cxx/class/method/Inputs/methods.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@
44
struct __attribute__((swift_attr("import_unsafe"))) NonTrivialInWrapper {
55
int value;
66

7+
NonTrivialInWrapper(int value) : value(value) {}
8+
9+
// explicit copy constructor is needed, as on Windows a destructor
10+
// still makes this a type that's passed in registers.
11+
NonTrivialInWrapper(const NonTrivialInWrapper &other) : value(other.value) {}
712
~NonTrivialInWrapper() { }
813
};
914

‎test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-itanium.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// RUN: %target-swift-emit-irgen -I %S/Inputs -enable-experimental-cxx-interop %s -Xcc -fignore-exceptions | %FileCheck %s
2+
3+
// UNSUPPORTED: OS=windows-msvc
4+
5+
import Methods
6+
7+
public func use() -> CInt {
8+
var instance = HasMethods()
9+
let result = instance.nonConstPassThroughAsWrapper(42)
10+
return result.value
11+
}
12+
13+
// CHECK: %[[instance:.*]] = alloca %TSo10HasMethodsV
14+
// CHECK: %[[result:.*]] = alloca %TSo19NonTrivialInWrapperV
15+
// CHECK: call void @_ZN10HasMethods28nonConstPassThroughAsWrapperEi(ptr sret(%struct.NonTrivialInWrapper) %[[result]], ptr %[[instance]], i32 42)
16+
17+
// CHECK: define {{.*}} void @_ZN10HasMethods28nonConstPassThroughAsWrapperEi(ptr noalias sret(%struct.NonTrivialInWrapper) {{.*}} %{{.*}}, ptr {{.*}} %{{.*}}, i32 noundef %{{.*}})

‎test/Interop/Cxx/class/method/methods-this-and-indirect-return-irgen-msvc.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// RUN: %target-swift-emit-irgen -I %S/Inputs -enable-experimental-cxx-interop %s | %FileCheck %s
2+
3+
// REQUIRES: OS=windows-msvc
4+
5+
import Methods
6+
7+
public func use() -> CInt {
8+
var instance = HasMethods()
9+
let result = instance.nonConstPassThroughAsWrapper(42)
10+
return result.value
11+
}
12+
13+
// CHECK: %[[instance:.*]] = alloca %TSo10HasMethodsV
14+
// CHECK: %[[result:.*]] = alloca %TSo19NonTrivialInWrapperV
15+
// CHECK: call void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(ptr %[[instance]], ptr sret(%struct.NonTrivialInWrapper) %[[result]], i32 42)
16+
17+
// CHECK: define {{.*}} void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(ptr {{.*}} %{{.*}}, ptr noalias sret(%struct.NonTrivialInWrapper) {{.*}} %{{.*}}, i32 noundef %{{.*}})

‎test/Interop/Cxx/class/method/methods.swift

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop)
22
//
33
// REQUIRES: executable_test
4-
//
5-
// Crash when running on windows: rdar://88391102
6-
// XFAIL: OS=windows-msvc
74

85
import StdlibUnittest
96
import Methods
@@ -34,15 +31,15 @@ CxxMethodTestSuite.test("(Int, Int) -> Int") {
3431
CxxMethodTestSuite.test("(NonTrivialInWrapper, NonTrivialInWrapper) -> Int") {
3532
var instance = HasMethods()
3633

37-
expectEqual(42, instance.nonConstSum(NonTrivialInWrapper(value: 40), NonTrivialInWrapper(value: 2)))
38-
expectEqual(42, instance.constSum(NonTrivialInWrapper(value: 40), NonTrivialInWrapper(value: 2)))
34+
expectEqual(42, instance.nonConstSum(NonTrivialInWrapper(40), NonTrivialInWrapper(2)))
35+
expectEqual(42, instance.constSum(NonTrivialInWrapper(40), NonTrivialInWrapper(2)))
3936
}
4037

4138
CxxMethodTestSuite.test("(NonTrivialInWrapper, NonTrivialInWrapper) -> NonTrivialInWrapper") {
4239
var instance = HasMethods()
4340

44-
expectEqual(42, instance.nonConstSumAsWrapper(NonTrivialInWrapper(value: 40), NonTrivialInWrapper(value: 2)).value)
45-
expectEqual(42, instance.constSumAsWrapper(NonTrivialInWrapper(value: 40), NonTrivialInWrapper(value: 2)).value)
41+
expectEqual(42, instance.nonConstSumAsWrapper(NonTrivialInWrapper(40), NonTrivialInWrapper(2)).value)
42+
expectEqual(42, instance.constSumAsWrapper(NonTrivialInWrapper(40), NonTrivialInWrapper(2)).value)
4643
}
4744

4845
CxxMethodTestSuite.test("(Int) -> NonTrivialInWrapper") {

‎test/Interop/Cxx/templates/member-templates-silgen.swift

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,19 @@
11
// RUN: %target-swift-emit-sil %s -I %S/Inputs -enable-experimental-cxx-interop | %FileCheck %s
22

3-
// We can't yet call member functions correctly on Windows
4-
// (https://github.com/apple/swift/issues/55575).
5-
// XFAIL: OS=windows-msvc
6-
73
import MemberTemplates
84

95
// CHECK-LABEL: sil hidden @$s4main9basicTestyyF : $@convention(thin) () -> ()
106

11-
// CHECK: [[ADD:%.*]] = function_ref @_ZN18HasMemberTemplates17addSameTypeParamsIiEET_S1_S1_ : $@convention(cxx_method) (Int32, Int32, @inout HasMemberTemplates) -> Int32
7+
// CHECK: [[ADD:%.*]] = function_ref @{{_ZN18HasMemberTemplates17addSameTypeParamsIiEET_S1_S1_|\?\?\$addSameTypeParams@H@HasMemberTemplates@@QEAAHHH@Z}} : $@convention(cxx_method) (Int32, Int32, @inout HasMemberTemplates) -> Int32
128
// CHECK: apply [[ADD]]({{.*}}) : $@convention(cxx_method) (Int32, Int32, @inout HasMemberTemplates) -> Int32
139

14-
// CHECK: [[ADD_TWO_TEMPLATES:%.*]] = function_ref @_ZN18HasMemberTemplates18addMixedTypeParamsIiiEET_S1_T0_ : $@convention(cxx_method) (Int32, Int32, @inout HasMemberTemplates) -> Int32
10+
// CHECK: [[ADD_TWO_TEMPLATES:%.*]] = function_ref @{{_ZN18HasMemberTemplates18addMixedTypeParamsIiiEET_S1_T0_|\?\?\$addMixedTypeParams@HH@HasMemberTemplates@@QEAAHHH@Z}} : $@convention(cxx_method) (Int32, Int32, @inout HasMemberTemplates) -> Int32
1511
// CHECK: apply [[ADD_TWO_TEMPLATES]]({{.*}}) : $@convention(cxx_method) (Int32, Int32, @inout HasMemberTemplates) -> Int32
1612

17-
// CHECK: [[ADD_ALL:%.*]] = function_ref @_ZN18HasMemberTemplates6addAllIiiEEiiT_T0_ : $@convention(cxx_method) (Int32, Int32, Int32, @inout HasMemberTemplates) -> Int32
13+
// CHECK: [[ADD_ALL:%.*]] = function_ref @{{_ZN18HasMemberTemplates6addAllIiiEEiiT_T0_|\?\?\$addAll@HH@HasMemberTemplates@@QEAAHHHH@Z}} : $@convention(cxx_method) (Int32, Int32, Int32, @inout HasMemberTemplates) -> Int32
1814
// CHECK: apply [[ADD_ALL]]({{.*}}) : $@convention(cxx_method) (Int32, Int32, Int32, @inout HasMemberTemplates) -> Int32
1915

20-
// CHECK: [[DO_NOTHING:%.*]] = function_ref @_ZN18HasMemberTemplates17doNothingConstRefIiEEvRKT_ : $@convention(cxx_method) (@in_guaranteed Int32, @inout HasMemberTemplates) -> ()
16+
// CHECK: [[DO_NOTHING:%.*]] = function_ref @{{_ZN18HasMemberTemplates17doNothingConstRefIiEEvRKT_|\?\?\$doNothingConstRef@H@HasMemberTemplates@@QEAAXAEBH@Z}} : $@convention(cxx_method) (@in_guaranteed Int32, @inout HasMemberTemplates) -> ()
2117
// CHECK: apply [[DO_NOTHING]]({{.*}}) : $@convention(cxx_method) (@in_guaranteed Int32, @inout HasMemberTemplates) -> ()
2218

2319
// CHECK-LABEL: end sil function '$s4main9basicTestyyF'
@@ -30,17 +26,17 @@ func basicTest() {
3026
obj.doNothingConstRef(i)
3127
}
3228

33-
// CHECK-LABEL: sil [clang HasMemberTemplates.addSameTypeParams] @_ZN18HasMemberTemplates17addSameTypeParamsIiEET_S1_S1_ : $@convention(cxx_method) (Int32, Int32, @inout HasMemberTemplates) -> Int32
29+
// CHECK-LABEL: sil [clang HasMemberTemplates.addSameTypeParams] @{{_ZN18HasMemberTemplates17addSameTypeParamsIiEET_S1_S1_|\?\?\$addSameTypeParams@H@HasMemberTemplates@@QEAAHHH@Z}} : $@convention(cxx_method) (Int32, Int32, @inout HasMemberTemplates) -> Int32
3430

35-
// CHECK-LABEL: sil [clang HasMemberTemplates.addMixedTypeParams] @_ZN18HasMemberTemplates18addMixedTypeParamsIiiEET_S1_T0_ : $@convention(cxx_method) (Int32, Int32, @inout HasMemberTemplates) -> Int32
31+
// CHECK-LABEL: sil [clang HasMemberTemplates.addMixedTypeParams] @{{_ZN18HasMemberTemplates18addMixedTypeParamsIiiEET_S1_T0_|\?\?\$addMixedTypeParams@HH@HasMemberTemplates@@QEAAHHH@Z}} : $@convention(cxx_method) (Int32, Int32, @inout HasMemberTemplates) -> Int32
3632

37-
// CHECK-LABEL: sil [clang HasMemberTemplates.addAll] @_ZN18HasMemberTemplates6addAllIiiEEiiT_T0_ : $@convention(cxx_method) (Int32, Int32, Int32, @inout HasMemberTemplates) -> Int32
33+
// CHECK-LABEL: sil [clang HasMemberTemplates.addAll] @{{_ZN18HasMemberTemplates6addAllIiiEEiiT_T0_|\?\?\$addAll@HH@HasMemberTemplates@@QEAAHHHH@Z}} : $@convention(cxx_method) (Int32, Int32, Int32, @inout HasMemberTemplates) -> Int32
3834

39-
// CHECK-LABEL: sil [clang HasMemberTemplates.doNothingConstRef] @_ZN18HasMemberTemplates17doNothingConstRefIiEEvRKT_ : $@convention(cxx_method) (@in_guaranteed Int32, @inout HasMemberTemplates) -> ()
35+
// CHECK-LABEL: sil [clang HasMemberTemplates.doNothingConstRef] @{{_ZN18HasMemberTemplates17doNothingConstRefIiEEvRKT_|\?\?\$doNothingConstRef@H@HasMemberTemplates@@QEAAXAEBH@Z}} : $@convention(cxx_method) (@in_guaranteed Int32, @inout HasMemberTemplates) -> ()
4036

4137
// CHECK-LABEL: sil hidden @$s4main12testSetValueyyF : $@convention(thin) () -> ()
4238

43-
// CHECK: [[SET_VALUE:%.*]] = function_ref @_ZN32TemplateClassWithMemberTemplatesIiE8setValueIlEEvT_ : $@convention(cxx_method) (Int, @inout TemplateClassWithMemberTemplates<Int32>) -> ()
39+
// CHECK: [[SET_VALUE:%.*]] = function_ref @{{_ZN32TemplateClassWithMemberTemplatesIiE8setValueIlEEvT_|\?\?\$setValue@_J@\?\$TemplateClassWithMemberTemplates@H@@QEAAX_J@Z}} : $@convention(cxx_method) (Int, @inout TemplateClassWithMemberTemplates<Int32>) -> ()
4440
// CHECK: apply [[SET_VALUE]]({{.*}}) : $@convention(cxx_method) (Int, @inout TemplateClassWithMemberTemplates<Int32>) -> ()
4541

4642
// CHECK-LABEL: end sil function '$s4main12testSetValueyyF'
@@ -51,13 +47,13 @@ func testSetValue() {
5147

5248
// CHECK-LABEL: sil hidden @$s4main17testStaticMembersyyF : $@convention(thin) () -> ()
5349

54-
// CHECK: [[ADD_FN:%.*]] = function_ref @_ZN24HasStaticMemberTemplates3addIlEET_S1_S1_ : $@convention(c) (Int, Int) -> Int
50+
// CHECK: [[ADD_FN:%.*]] = function_ref @{{_ZN24HasStaticMemberTemplates3addIlEET_S1_S1_|\?\?\$add@_J@HasStaticMemberTemplates@@SA_J_J0@Z}} : $@convention(c) (Int, Int) -> Int
5551
// CHECK: apply [[ADD_FN]]({{.*}}) : $@convention(c) (Int, Int) -> Int
5652

57-
// CHECK: [[ADD_TWO_TEMPLATES_FN:%.*]] = function_ref @_ZN24HasStaticMemberTemplates15addTwoTemplatesIlcEET_S1_T0_ : $@convention(c) (Int, Int8) -> Int
53+
// CHECK: [[ADD_TWO_TEMPLATES_FN:%.*]] = function_ref @{{_ZN24HasStaticMemberTemplates15addTwoTemplatesIlcEET_S1_T0_|\?\?\$addTwoTemplates@_JD@HasStaticMemberTemplates@@SA_J_JD@Z}} : $@convention(c) (Int, Int8) -> Int
5854
// CHECK: apply [[ADD_TWO_TEMPLATES_FN]]({{.*}}) : $@convention(c) (Int, Int8) -> Int
5955

60-
// CHECK: [[REMOVE_REFERENCE_FN:%.*]] = function_ref @_ZN24HasStaticMemberTemplates15removeReferenceIlEET_RS1_ : $@convention(c) (@inout Int) -> Int
56+
// CHECK: [[REMOVE_REFERENCE_FN:%.*]] = function_ref @{{_ZN24HasStaticMemberTemplates15removeReferenceIlEET_RS1_|\?\?\$removeReference@_J@HasStaticMemberTemplates@@SA_JAEA_J@Z}} : $@convention(c) (@inout Int) -> Int
6157
// CHECK: apply [[REMOVE_REFERENCE_FN]]({{.*}}) : $@convention(c) (@inout Int) -> Int
6258

6359
// CHECK-LABEL: end sil function '$s4main17testStaticMembersyyF'
@@ -69,8 +65,8 @@ func testStaticMembers() {
6965
HasStaticMemberTemplates.removeReference(&x)
7066
}
7167

72-
// CHECK: sil hidden_external [clang HasStaticMemberTemplates.add] @_ZN24HasStaticMemberTemplates3addIlEET_S1_S1_ : $@convention(c) (Int, Int) -> Int
68+
// CHECK: sil hidden_external [clang HasStaticMemberTemplates.add] @{{_ZN24HasStaticMemberTemplates3addIlEET_S1_S1_|\?\?\$add@_J@HasStaticMemberTemplates@@SA_J_J0@Z}} : $@convention(c) (Int, Int) -> Int
7369

74-
// CHECK: sil hidden_external [clang HasStaticMemberTemplates.addTwoTemplates] @_ZN24HasStaticMemberTemplates15addTwoTemplatesIlcEET_S1_T0_ : $@convention(c) (Int, Int8) -> Int
70+
// CHECK: sil hidden_external [clang HasStaticMemberTemplates.addTwoTemplates] @{{_ZN24HasStaticMemberTemplates15addTwoTemplatesIlcEET_S1_T0_|\?\?\$addTwoTemplates@_JD@HasStaticMemberTemplates@@SA_J_JD@Z}} : $@convention(c) (Int, Int8) -> Int
7571

76-
// CHECK: sil hidden_external [clang HasStaticMemberTemplates.removeReference] @_ZN24HasStaticMemberTemplates15removeReferenceIlEET_RS1_ : $@convention(c) (@inout Int) -> Int
72+
// CHECK: sil hidden_external [clang HasStaticMemberTemplates.removeReference] @{{_ZN24HasStaticMemberTemplates15removeReferenceIlEET_RS1_|\?\?\$removeReference@_J@HasStaticMemberTemplates@@SA_JAEA_J@Z}} : $@convention(c) (@inout Int) -> Int

0 commit comments

Comments
 (0)