Skip to content

Commit 4154cdb

Browse files
nate-chandlernate-chandler
authored
Merge pull request #71799 from nate-chandler/noncopyable-bugs/20240221/2
[IRGen] Outlined value functions that destroy move-only-with-deinit types take and forward those types' metadata.
2 parents 1d44e2e + 36b473d commit 4154cdb

File tree

8 files changed

+295
-53
lines changed

8 files changed

+295
-53
lines changed

‎lib/IRGen/GenExistential.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,8 @@ namespace {
253253
asDerived().emitValueAssignWithCopy(IGF, destValue, srcValue);
254254
emitCopyOfTables(IGF, dest, src);
255255
} else {
256-
OutliningMetadataCollector collector(IGF);
256+
OutliningMetadataCollector collector(IGF, LayoutIsNeeded,
257+
DeinitIsNotNeeded);
257258
collector.emitCallToOutlinedCopy(dest, src, T, *this,
258259
IsNotInitialization, IsNotTake);
259260
}
@@ -267,7 +268,8 @@ namespace {
267268
asDerived().emitValueInitializeWithCopy(IGF, destValue, srcValue);
268269
emitCopyOfTables(IGF, dest, src);
269270
} else {
270-
OutliningMetadataCollector collector(IGF);
271+
OutliningMetadataCollector collector(IGF, LayoutIsNeeded,
272+
DeinitIsNotNeeded);
271273
collector.emitCallToOutlinedCopy(dest, src, T, *this,
272274
IsInitialization, IsNotTake);
273275
}
@@ -281,7 +283,8 @@ namespace {
281283
asDerived().emitValueAssignWithTake(IGF, destValue, srcValue);
282284
emitCopyOfTables(IGF, dest, src);
283285
} else {
284-
OutliningMetadataCollector collector(IGF);
286+
OutliningMetadataCollector collector(IGF, LayoutIsNeeded,
287+
DeinitIsNotNeeded);
285288
collector.emitCallToOutlinedCopy(dest, src, T, *this,
286289
IsNotInitialization, IsTake);
287290
}
@@ -295,7 +298,8 @@ namespace {
295298
asDerived().emitValueInitializeWithTake(IGF, destValue, srcValue);
296299
emitCopyOfTables(IGF, dest, src);
297300
} else {
298-
OutliningMetadataCollector collector(IGF);
301+
OutliningMetadataCollector collector(IGF, LayoutIsNeeded,
302+
DeinitIsNotNeeded);
299303
collector.emitCallToOutlinedCopy(dest, src, T, *this,
300304
IsInitialization, IsTake);
301305
}
@@ -307,7 +311,8 @@ namespace {
307311
Address valueAddr = projectValue(IGF, existential);
308312
asDerived().emitValueDestroy(IGF, valueAddr);
309313
} else {
310-
OutliningMetadataCollector collector(IGF);
314+
OutliningMetadataCollector collector(IGF, LayoutIsNeeded,
315+
DeinitIsNeeded);
311316
collector.emitCallToOutlinedDestroy(existential, T, *this);
312317
}
313318
}
@@ -963,7 +968,8 @@ class OpaqueExistentialTypeInfo final :
963968
srcBuffer);
964969
} else {
965970
// Create an outlined function to avoid explosion
966-
OutliningMetadataCollector collector(IGF);
971+
OutliningMetadataCollector collector(IGF, LayoutIsNeeded,
972+
DeinitIsNotNeeded);
967973
collector.emitCallToOutlinedCopy(dest, src, T, *this,
968974
IsInitialization, IsNotTake);
969975
}
@@ -979,7 +985,8 @@ class OpaqueExistentialTypeInfo final :
979985
IGF.emitMemCpy(dest, src, getLayout().getSize(IGF.IGM));
980986
} else {
981987
// Create an outlined function to avoid explosion
982-
OutliningMetadataCollector collector(IGF);
988+
OutliningMetadataCollector collector(IGF, LayoutIsNeeded,
989+
DeinitIsNotNeeded);
983990
collector.emitCallToOutlinedCopy(dest, src, T, *this,
984991
IsInitialization, IsTake);
985992
}

‎lib/IRGen/GenType.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ void LoadableTypeInfo::initializeWithCopy(IRGenFunction &IGF, Address destAddr,
184184
loadAsCopy(IGF, srcAddr, copy);
185185
initialize(IGF, copy, destAddr, true);
186186
} else {
187-
OutliningMetadataCollector collector(IGF);
187+
OutliningMetadataCollector collector(IGF, LayoutIsNeeded,
188+
DeinitIsNotNeeded);
188189
// No need to collect anything because we assume loadable types can be
189190
// loaded without enums.
190191
collector.emitCallToOutlinedCopy(

‎lib/IRGen/IRGenModule.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,8 +1189,10 @@ class IRGenModule {
11891189
llvm::Constant *getOrCreateRetainFunction(const TypeInfo &objectTI, SILType t,
11901190
llvm::Type *llvmType, Atomicity atomicity);
11911191

1192-
llvm::Constant *getOrCreateReleaseFunction(const TypeInfo &objectTI, SILType t,
1193-
llvm::Type *llvmType, Atomicity atomicity);
1192+
llvm::Constant *
1193+
getOrCreateReleaseFunction(const TypeInfo &objectTI, SILType t,
1194+
llvm::Type *llvmType, Atomicity atomicity,
1195+
const OutliningMetadataCollector &collector);
11941196

11951197
llvm::Constant *getOrCreateOutlinedInitializeWithTakeFunction(
11961198
SILType objectType, const TypeInfo &objectTI,

‎lib/IRGen/IRGenSIL.cpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5069,15 +5069,9 @@ void IRGenSILFunction::visitReleaseValueAddrInst(
50695069
if (tryEmitDestroyUsingDeinit(*this, addr, addrTy)) {
50705070
return;
50715071
}
5072-
llvm::Type *llvmType = addr.getAddress()->getType();
50735072
const TypeInfo &addrTI = getTypeInfo(addrTy);
50745073
auto atomicity = i->isAtomic() ? Atomicity::Atomic : Atomicity::NonAtomic;
5075-
auto *outlinedF = cast<llvm::Function>(
5076-
IGM.getOrCreateReleaseFunction(addrTI, objectT, llvmType, atomicity));
5077-
llvm::Value *args[] = {addr.getAddress()};
5078-
llvm::CallInst *call =
5079-
Builder.CreateCall(outlinedF->getFunctionType(), outlinedF, args);
5080-
call->setCallingConv(IGM.DefaultCC);
5074+
addrTI.callOutlinedRelease(*this, addr, objectT, atomicity);
50815075
}
50825076

50835077
void IRGenSILFunction::visitDestroyValueInst(swift::DestroyValueInst *i) {

‎lib/IRGen/Outlining.cpp

Lines changed: 77 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,29 @@
3232
using namespace swift;
3333
using namespace irgen;
3434

35-
void OutliningMetadataCollector::collectTypeMetadataForLayout(SILType type) {
35+
void OutliningMetadataCollector::collectTypeMetadataForLayout(SILType ty) {
3636
// If the type has no archetypes, we can emit it from scratch in the callee.
37-
if (!type.hasArchetype()) {
37+
if (!ty.hasArchetype()) {
3838
return;
3939
}
4040

4141
// Substitute opaque types if allowed.
42-
type =
43-
IGF.IGM.substOpaqueTypesWithUnderlyingTypes(type, CanGenericSignature());
42+
ty = IGF.IGM.substOpaqueTypesWithUnderlyingTypes(ty, CanGenericSignature());
4443

45-
auto formalType = type.getASTType();
46-
auto &ti = IGF.IGM.getTypeInfoForLowered(formalType);
44+
auto astType = ty.getASTType();
45+
auto &ti = IGF.IGM.getTypeInfoForLowered(astType);
46+
47+
if (needsDeinit) {
48+
auto *nominal = ty.getASTType()->getAnyNominal();
49+
if (nominal && nominal->getValueTypeDestructor()) {
50+
assert(ty.isMoveOnly());
51+
collectFormalTypeMetadata(ty.getASTType());
52+
}
53+
}
54+
55+
if (!needsLayout) {
56+
return;
57+
}
4758

4859
// We don't need the metadata for fixed size types or types that are not ABI
4960
// accessible. Outlining will call the value witness of the enclosing type of
@@ -55,16 +66,11 @@ void OutliningMetadataCollector::collectTypeMetadataForLayout(SILType type) {
5566
// If the type is a legal formal type, add it as a formal type.
5667
// FIXME: does this force us to emit a more expensive metadata than we need
5768
// to?
58-
if (formalType->isLegalFormalType()) {
59-
return collectFormalTypeMetadata(formalType);
69+
if (astType->isLegalFormalType()) {
70+
return collectFormalTypeMetadata(astType);
6071
}
6172

62-
auto key = LocalTypeDataKey(type.getASTType(),
63-
LocalTypeDataKind::forRepresentationTypeMetadata());
64-
if (Values.count(key)) return;
65-
66-
auto metadata = IGF.emitTypeMetadataRefForLayout(type);
67-
Values.insert({key, metadata});
73+
collectRepresentationTypeMetadata(ty);
6874
}
6975

7076
void OutliningMetadataCollector::collectFormalTypeMetadata(CanType type) {
@@ -78,6 +84,15 @@ void OutliningMetadataCollector::collectFormalTypeMetadata(CanType type) {
7884
Values.insert({key, metadata});
7985
}
8086

87+
void OutliningMetadataCollector::collectRepresentationTypeMetadata(SILType ty) {
88+
auto key = LocalTypeDataKey(
89+
ty.getASTType(), LocalTypeDataKind::forRepresentationTypeMetadata());
90+
if (Values.count(key))
91+
return;
92+
93+
auto metadata = IGF.emitTypeMetadataRefForLayout(ty);
94+
Values.insert({key, metadata});
95+
}
8196

8297
void OutliningMetadataCollector::addMetadataArguments(
8398
SmallVectorImpl<llvm::Value*> &args) const {
@@ -136,11 +151,12 @@ irgen::getTypeAndGenericSignatureForManglingOutlineFunction(SILType type) {
136151
}
137152

138153
bool TypeInfo::withMetadataCollector(
139-
IRGenFunction &IGF, SILType T,
154+
IRGenFunction &IGF, SILType T, LayoutIsNeeded_t needsLayout,
155+
DeinitIsNeeded_t needsDeinit,
140156
llvm::function_ref<void(OutliningMetadataCollector &)> invocation) const {
141157
if (!T.hasLocalArchetype() &&
142158
!IGF.outliningCanCallValueWitnesses()) {
143-
OutliningMetadataCollector collector(IGF);
159+
OutliningMetadataCollector collector(IGF, needsLayout, needsDeinit);
144160
if (T.hasArchetype()) {
145161
collectMetadataForOutlining(collector, T);
146162
}
@@ -150,7 +166,7 @@ bool TypeInfo::withMetadataCollector(
150166

151167
if (!T.hasArchetype()) {
152168
// The implementation will call vwt in this case.
153-
OutliningMetadataCollector collector(IGF);
169+
OutliningMetadataCollector collector(IGF, needsLayout, needsDeinit);
154170
invocation(collector);
155171
return true;
156172
}
@@ -161,9 +177,11 @@ bool TypeInfo::withMetadataCollector(
161177
void TypeInfo::callOutlinedCopy(IRGenFunction &IGF, Address dest, Address src,
162178
SILType T, IsInitialization_t isInit,
163179
IsTake_t isTake) const {
164-
if (withMetadataCollector(IGF, T, [&](auto collector) {
165-
collector.emitCallToOutlinedCopy(dest, src, T, *this, isInit, isTake);
166-
})) {
180+
if (withMetadataCollector(IGF, T, LayoutIsNeeded, DeinitIsNotNeeded,
181+
[&](auto collector) {
182+
collector.emitCallToOutlinedCopy(
183+
dest, src, T, *this, isInit, isTake);
184+
})) {
167185
return;
168186
}
169187

@@ -183,6 +201,8 @@ void OutliningMetadataCollector::emitCallToOutlinedCopy(
183201
Address dest, Address src,
184202
SILType T, const TypeInfo &ti,
185203
IsInitialization_t isInit, IsTake_t isTake) const {
204+
assert(needsLayout);
205+
assert(!needsDeinit);
186206
llvm::SmallVector<llvm::Value *, 4> args;
187207
args.push_back(IGF.Builder.CreateElementBitCast(src, ti.getStorageType())
188208
.getAddress());
@@ -356,9 +376,10 @@ void TypeInfo::callOutlinedDestroy(IRGenFunction &IGF,
356376
if (IGF.IGM.getTypeLowering(T).isTrivial())
357377
return;
358378

359-
if (withMetadataCollector(IGF, T, [&](auto collector) {
360-
collector.emitCallToOutlinedDestroy(addr, T, *this);
361-
})) {
379+
if (withMetadataCollector(
380+
IGF, T, LayoutIsNeeded, DeinitIsNeeded, [&](auto collector) {
381+
collector.emitCallToOutlinedDestroy(addr, T, *this);
382+
})) {
362383
return;
363384
}
364385

@@ -367,6 +388,8 @@ void TypeInfo::callOutlinedDestroy(IRGenFunction &IGF,
367388

368389
void OutliningMetadataCollector::emitCallToOutlinedDestroy(
369390
Address addr, SILType T, const TypeInfo &ti) const {
391+
assert(needsLayout);
392+
assert(needsDeinit);
370393
llvm::SmallVector<llvm::Value *, 4> args;
371394
args.push_back(IGF.Builder.CreateElementBitCast(addr, ti.getStorageType())
372395
.getAddress());
@@ -439,24 +462,46 @@ llvm::Constant *IRGenModule::getOrCreateRetainFunction(const TypeInfo &ti,
439462
true /*setIsNoInline*/);
440463
}
441464

442-
llvm::Constant *
443-
IRGenModule::getOrCreateReleaseFunction(const TypeInfo &ti,
444-
SILType t,
445-
llvm::Type *llvmType,
446-
Atomicity atomicity) {
465+
void TypeInfo::callOutlinedRelease(IRGenFunction &IGF, Address addr, SILType T,
466+
Atomicity atomicity) const {
467+
OutliningMetadataCollector collector(IGF, LayoutIsNotNeeded, DeinitIsNeeded);
468+
collectMetadataForOutlining(collector, T);
469+
collector.emitCallToOutlinedRelease(addr, T, *this, atomicity);
470+
}
471+
472+
void OutliningMetadataCollector::emitCallToOutlinedRelease(
473+
Address addr, SILType T, const TypeInfo &ti, Atomicity atomicity) const {
474+
assert(!needsLayout);
475+
assert(needsDeinit);
476+
llvm::SmallVector<llvm::Value *, 4> args;
477+
args.push_back(addr.getAddress());
478+
addMetadataArguments(args);
479+
auto *outlinedF = cast<llvm::Function>(IGF.IGM.getOrCreateReleaseFunction(
480+
ti, T, addr.getAddress()->getType(), atomicity, *this));
481+
llvm::CallInst *call =
482+
IGF.Builder.CreateCall(outlinedF->getFunctionType(), outlinedF, args);
483+
call->setCallingConv(IGF.IGM.DefaultCC);
484+
}
485+
486+
llvm::Constant *IRGenModule::getOrCreateReleaseFunction(
487+
const TypeInfo &ti, SILType t, llvm::Type *ptrTy, Atomicity atomicity,
488+
const OutliningMetadataCollector &collector) {
447489
auto *loadableTI = cast<LoadableTypeInfo>(&ti);
448490
IRGenMangler mangler;
449491
auto manglingBits =
450492
getTypeAndGenericSignatureForManglingOutlineFunction(t);
451493
auto funcName = mangler.mangleOutlinedReleaseFunction(manglingBits.first,
452494
manglingBits.second);
453-
llvm::Type *argTys[] = {llvmType};
495+
llvm::SmallVector<llvm::Type *, 4> argTys;
496+
argTys.push_back(ptrTy);
497+
collector.addMetadataParameterTypes(argTys);
454498
return getOrCreateHelperFunction(
455-
funcName, llvmType, argTys,
499+
funcName, ptrTy, argTys,
456500
[&](IRGenFunction &IGF) {
457-
auto it = IGF.CurFn->arg_begin();
458-
Address addr(&*it++, loadableTI->getStorageType(),
501+
Explosion params = IGF.collectParameters();
502+
Address addr(params.claimNext(), loadableTI->getStorageType(),
459503
loadableTI->getFixedAlignment());
504+
collector.bindMetadataParameters(IGF, params);
460505
Explosion loaded;
461506
loadableTI->loadAsTake(IGF, addr, loaded);
462507
loadableTI->consume(IGF, loaded, atomicity, t);

‎lib/IRGen/Outlining.h

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717
#ifndef SWIFT_IRGEN_OUTLINING_H
1818
#define SWIFT_IRGEN_OUTLINING_H
1919

20+
#include "IRGen.h"
2021
#include "LocalTypeDataKind.h"
2122
#include "swift/Basic/LLVM.h"
2223
#include "llvm/ADT/MapVector.h"
23-
#include "LocalTypeDataKind.h"
2424

2525
namespace llvm {
2626
class Value;
@@ -41,6 +41,16 @@ class IRGenFunction;
4141
class IRGenModule;
4242
class TypeInfo;
4343

44+
enum LayoutIsNeeded_t : bool {
45+
LayoutIsNotNeeded = false,
46+
LayoutIsNeeded = true
47+
};
48+
49+
enum DeinitIsNeeded_t : bool {
50+
DeinitIsNotNeeded = false,
51+
DeinitIsNeeded = true
52+
};
53+
4454
/// A helper class for emitting outlined value operations.
4555
///
4656
/// The use-pattern for this class is:
@@ -51,23 +61,32 @@ class TypeInfo;
5161
class OutliningMetadataCollector {
5262
public:
5363
IRGenFunction &IGF;
64+
const unsigned needsLayout : 1;
65+
const unsigned needsDeinit : 1;
66+
5467
private:
5568
llvm::MapVector<LocalTypeDataKey, llvm::Value *> Values;
5669
friend class IRGenModule;
5770

5871
public:
59-
OutliningMetadataCollector(IRGenFunction &IGF) : IGF(IGF) {}
72+
OutliningMetadataCollector(IRGenFunction &IGF, LayoutIsNeeded_t needsLayout,
73+
DeinitIsNeeded_t needsDeinitTypes)
74+
: IGF(IGF), needsLayout(needsLayout), needsDeinit(needsDeinitTypes) {}
6075

61-
void collectFormalTypeMetadata(CanType type);
6276
void collectTypeMetadataForLayout(SILType type);
6377

6478
void emitCallToOutlinedCopy(Address dest, Address src,
6579
SILType T, const TypeInfo &ti,
6680
IsInitialization_t isInit, IsTake_t isTake) const;
6781
void emitCallToOutlinedDestroy(Address addr, SILType T,
6882
const TypeInfo &ti) const;
83+
void emitCallToOutlinedRelease(Address addr, SILType T, const TypeInfo &ti,
84+
Atomicity atomicity) const;
6985

7086
private:
87+
void collectFormalTypeMetadata(CanType type);
88+
void collectRepresentationTypeMetadata(SILType ty);
89+
7190
void addMetadataArguments(SmallVectorImpl<llvm::Value *> &args) const ;
7291
void addMetadataParameterTypes(SmallVectorImpl<llvm::Type *> &paramTys) const;
7392
void bindMetadataParameters(IRGenFunction &helperIGF,

0 commit comments

Comments
 (0)