Skip to content

Commit 3671202

Browse files
hyphyp
authored
Merge pull request #67442 from hyp/eng/5.9/windows-almighty
[cxx-interop][5.9] windows ABI fixes
2 parents 713a434 + 9b721b2 commit 3671202

22 files changed

+183
-75
lines changed

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 27 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2472,39 +2472,33 @@ namespace {
24722472
}
24732473

24742474
if (cxxRecordDecl) {
2475-
// FIXME: Swift right now uses AddressOnly type layout
2476-
// in a way that conflates C++ types
2477-
// that need to be destroyed or copied explicitly with C++
2478-
// types that have to be passed indirectly, because
2479-
// only AddressOnly types can be copied or destroyed using C++
2480-
// semantics. However, in actuality these two concepts are
2481-
// separate and don't map to one notion of AddressOnly type
2482-
// layout cleanly. We should reserve the use of AddressOnly
2483-
// type layout when types have to use C++ copy/move/destroy
2484-
// operations, but allow AddressOnly types to be passed
2485-
// directly as well. This will help unify the MSVC and
2486-
// Itanium difference here, and will allow us to support
2487-
// trivial_abi C++ types as well.
2488-
auto isNonTrivialForPurposeOfCalls =
2489-
[](const clang::CXXRecordDecl *decl) -> bool {
2490-
return decl->hasNonTrivialCopyConstructor() ||
2491-
decl->hasNonTrivialMoveConstructor() ||
2492-
!decl->hasTrivialDestructor();
2493-
};
2494-
auto isAddressOnlySwiftStruct =
2495-
[&](const clang::CXXRecordDecl *decl) -> bool {
2496-
// MSVC ABI allows non-trivially destroyed C++ types
2497-
// to be passed in register. This is not supported, as such
2498-
// type wouldn't be destroyed in Swift correctly. Therefore,
2499-
// force AddressOnly type layout using the old heuristic.
2500-
// FIXME: Support can pass in registers for MSVC correctly.
2501-
if (Impl.SwiftContext.LangOpts.Target.isWindowsMSVCEnvironment())
2502-
return isNonTrivialForPurposeOfCalls(decl);
2503-
return !decl->canPassInRegisters();
2504-
};
2505-
if (auto structResult = dyn_cast<StructDecl>(result))
2506-
structResult->setIsCxxNonTrivial(
2507-
isAddressOnlySwiftStruct(cxxRecordDecl));
2475+
if (auto structResult = dyn_cast<StructDecl>(result)) {
2476+
// Address-only type is a type that can't be passed in registers.
2477+
// Address-only types are typically non-trivial, however some
2478+
// non-trivial types can be loadable as well (although such types
2479+
// are not yet available in Swift).
2480+
bool isAddressOnly = !cxxRecordDecl->canPassInRegisters();
2481+
// Check if the given type is non-trivial to ensure we can
2482+
// still perform the right copy/move/destroy even if it's
2483+
// not an address-only type.
2484+
auto isNonTrivial = [](const clang::CXXRecordDecl *decl) -> bool {
2485+
return decl->hasNonTrivialCopyConstructor() ||
2486+
decl->hasNonTrivialMoveConstructor() ||
2487+
!decl->hasTrivialDestructor();
2488+
};
2489+
if (!isAddressOnly &&
2490+
Impl.SwiftContext.LangOpts.Target.isWindowsMSVCEnvironment() &&
2491+
isNonTrivial(cxxRecordDecl)) {
2492+
// MSVC ABI allows non-trivially destroyed C++ types
2493+
// to be passed in register. This is not supported, as such
2494+
// type wouldn't be destroyed in Swift correctly. Therefore,
2495+
// mark this type as unavailable.
2496+
// FIXME: Support can pass in registers for MSVC correctly.
2497+
Impl.markUnavailable(result, "non-trivial C++ class with trivial "
2498+
"ABI is not yet available in Swift");
2499+
}
2500+
structResult->setIsCxxNonTrivial(isAddressOnly);
2501+
}
25082502

25092503
for (auto &getterAndSetter : Impl.GetterSetterMap[result]) {
25102504
auto getter = getterAndSetter.second.first;

‎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/Inputs/destructors.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,21 @@ struct DummyStruct {};
66
struct __attribute__((swift_attr("import_unsafe")))
77
HasUserProvidedDestructorAndDummy {
88
DummyStruct dummy;
9+
HasUserProvidedDestructorAndDummy(DummyStruct dummy) : dummy(dummy) {}
10+
#if __is_target_os(windows)
11+
// On windows, force this type to be address-only.
12+
HasUserProvidedDestructorAndDummy(const HasUserProvidedDestructorAndDummy &) {}
13+
#endif
914
~HasUserProvidedDestructorAndDummy() {}
1015
};
1116

1217
struct __attribute__((swift_attr("import_unsafe"))) HasUserProvidedDestructor {
1318
int *value;
19+
#if __is_target_os(windows)
20+
// On windows, force this type to be address-only.
21+
HasUserProvidedDestructor() {}
22+
HasUserProvidedDestructor(const HasUserProvidedDestructor &other) {}
23+
#endif
1424
~HasUserProvidedDestructor() { *value = 42; }
1525
};
1626

‎test/Interop/Cxx/class/Inputs/nodiscard.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
class NoDiscardMultiply {
77
public:
88
NoDiscardMultiply() {}
9+
NoDiscardMultiply(const NoDiscardMultiply &) { }
910
~NoDiscardMultiply() {}
1011

1112
[[nodiscard]] int Multiply(int x, int y) { return x * y; }

‎test/Interop/Cxx/class/Inputs/protocol-conformance.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ struct DoesNotConformToProtocol {
1212
struct DummyStruct {};
1313

1414
struct __attribute__((swift_attr("import_unsafe"))) NonTrivial {
15+
NonTrivial(const NonTrivial &other) {}
1516
~NonTrivial() {}
1617
NonTrivial(DummyStruct) {}
1718
NonTrivial() {}
@@ -67,4 +68,4 @@ struct HasOperatorPlusEqual {
6768

6869
using HasOperatorPlusEqualInt = HasOperatorPlusEqual<int>;
6970

70-
#endif // TEST_INTEROP_CXX_CLASS_INPUTS_PROTOCOL_CONFORMANCE_H
71+
#endif // TEST_INTEROP_CXX_CLASS_INPUTS_PROTOCOL_CONFORMANCE_H

‎test/Interop/Cxx/class/Inputs/type-classification.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ struct StructWithSubobjectMoveAssignment {
8080
};
8181

8282
struct __attribute__((swift_attr("import_unsafe"))) StructWithDestructor {
83+
#if __is_target_os(windows)
84+
// On windows, force this type to be address-only.
85+
StructWithDestructor() {}
86+
StructWithDestructor(const StructWithDestructor &other) {}
87+
#endif
88+
8389
~StructWithDestructor() {}
8490
};
8591

‎test/Interop/Cxx/class/delete-operator-destructor-ref-irgen.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ public:
2424
class Container {
2525
public:
2626
Container() : pointer(new BaseClass) {}
27+
Container(const Container &other) : pointer(new BaseClass) {}
2728
~Container() { delete pointer; }
2829

2930
inline int method() const {

‎test/Interop/Cxx/class/destructors-correct-abi-irgen.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ import Destructors
44

55
// CHECK-LABEL: define {{.*}}void @"$s4main4testyyF"
66
// CHECK: [[H:%.*]] = alloca %TSo33HasUserProvidedDestructorAndDummyV
7+
// CHECK: [[CXX_THIS_PRE:%.*]] = bitcast %TSo33HasUserProvidedDestructorAndDummyV* [[H]] to %struct.HasUserProvidedDestructorAndDummy*
78
// CHECK: [[CXX_THIS:%.*]] = bitcast %TSo33HasUserProvidedDestructorAndDummyV* [[H]] to %struct.HasUserProvidedDestructorAndDummy*
89
// CHECK: call {{.*}}@{{_ZN33HasUserProvidedDestructorAndDummyD(1|2)Ev|"\?\?1HasUserProvidedDestructorAndDummy@@QEAA@XZ"}}(%struct.HasUserProvidedDestructorAndDummy* [[CXX_THIS]])
10+
911
// CHECK: ret void
1012
public func test() {
1113
let d = DummyStruct()
12-
let h = HasUserProvidedDestructorAndDummy(dummy: d)
14+
let h = HasUserProvidedDestructorAndDummy(d)
1315
}

‎test/Interop/Cxx/class/inheritance/Inputs/fields.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ struct DerivedFromOneField : OneField {};
3030

3131
struct __attribute__((swift_attr("import_unsafe"))) NonTrivial {
3232
NonTrivial() {}
33+
NonTrivial(const NonTrivial &) {}
3334
~NonTrivial() {}
3435
};
3536

@@ -45,6 +46,7 @@ struct NonTrivialDerivedWithOneField : NonTrivialHasThreeFields {
4546

4647
struct __attribute__((swift_attr("import_unsafe"))) NonTrivialHasOneField {
4748
NonTrivialHasOneField() {}
49+
NonTrivialHasOneField(const NonTrivialHasOneField &other) : e(other.e) {}
4850
~NonTrivialHasOneField() {}
4951

5052
int e = 5;

‎test/Interop/Cxx/class/inheritance/Inputs/functions.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
struct __attribute__((swift_attr("import_unsafe"))) NonTrivial {
22
NonTrivial() {}
3+
NonTrivial(const NonTrivial &) {}
34
~NonTrivial() {}
45

56
inline const char *inNonTrivial() const

‎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: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
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: %[[CXX_THIS_PRE:.*]] = bitcast %TSo10HasMethodsV* %[[instance]] to %struct.HasMethods*
16+
// CHECK: %[[CXX_THIS:.*]] = bitcast %TSo10HasMethodsV* %[[instance]] to %struct.HasMethods*
17+
// CHECK: call void @_ZN10HasMethods28nonConstPassThroughAsWrapperEi(%struct.NonTrivialInWrapper* sret(%struct.NonTrivialInWrapper) %{{.*}}, %struct.HasMethods* %[[CXX_THIS]], i32 42)
18+
19+
// CHECK: define {{.*}} void @_ZN10HasMethods28nonConstPassThroughAsWrapperEi(%struct.NonTrivialInWrapper* noalias sret(%struct.NonTrivialInWrapper) {{.*}} %{{.*}}, %struct.HasMethods* {{.*}} %{{.*}}, i32 noundef %{{.*}})

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
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: %[[CXX_THIS_PRE:.*]] = bitcast %TSo10HasMethodsV* %[[instance]] to %struct.HasMethods*
16+
// CHECK: %[[CXX_THIS:.*]] = bitcast %TSo10HasMethodsV* %[[instance]] to %struct.HasMethods*
17+
// CHECK: call void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(%struct.HasMethods* %[[CXX_THIS]], %struct.NonTrivialInWrapper* sret(%struct.NonTrivialInWrapper) %{{.*}}, i32 42)
18+
19+
// CHECK: define {{.*}} void @"?nonConstPassThroughAsWrapper@HasMethods@@QEAA?AUNonTrivialInWrapper@@H@Z"(%struct.HasMethods* {{.*}} %{{.*}}, %struct.NonTrivialInWrapper* 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/exceptions/trap-on-exception-irgen-itanium.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ public:
9292
class ClassWithDestructor {
9393
int m = 0;
9494
public:
95+
#if __is_target_os(windows)
96+
// On windows, force this type to be address-only.
97+
inline ClassWithDestructor() noexcept {}
98+
inline ClassWithDestructor(const ClassWithDestructor &other) noexcept : m(other.m) {}
99+
#endif
95100
inline ~ClassWithDestructor() {
96101
(void)freeFunctionNoThrow(0);
97102
}
@@ -100,6 +105,11 @@ public:
100105
class ClassWithThrowingDestructor {
101106
int m = 0;
102107
public:
108+
#if __is_target_os(windows)
109+
// On windows, force this type to be address-only.
110+
inline ClassWithThrowingDestructor() noexcept {}
111+
inline ClassWithThrowingDestructor(const ClassWithThrowingDestructor &other) noexcept : m(other.m) {}
112+
#endif
103113
inline ~ClassWithThrowingDestructor() noexcept(false) {
104114
throw 2;
105115
}
@@ -139,6 +149,8 @@ struct StructWithDefaultConstructor {
139149

140150

141151
struct NonTrivial {
152+
NonTrivial() noexcept;
153+
NonTrivial(const NonTrivial &other) noexcept;
142154
~NonTrivial() {}
143155
};
144156

0 commit comments

Comments
 (0)