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

0 commit comments

Comments
 (0)