Skip to content

Commit 11f0ff6

Browse files
gottesmmgottesmm
committed
[sil] Ensure that all SILValues have a parent function by making it so that SILUndef is uniqued at the function instead of module level.
For years, optimizer engineers have been hitting a common bug caused by passes assuming all SILValues have a parent function only to be surprised by SILUndef. Generally we see SILUndef not that often so we see this come up later in testing. This patch eliminates that problem by making SILUndef uniqued at the function level instead of the module level. This ensures that it makes sense for SILUndef to have a parent function, eliminating this possibility since we can define an API to get its parent function. rdar://123484595
1 parent 34dd632 commit 11f0ff6

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

59 files changed

+493
-379
lines changed

‎SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LetPropertyLowering.swift

+2-1
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,8 @@ private func insertEndInitInstructions(
9999
atEndOf initRegion: InstructionRange,
100100
_ context: FunctionPassContext
101101
) {
102-
var ssaUpdater = SSAUpdater(type: markUninitialized.type, ownership: .owned, context)
102+
var ssaUpdater = SSAUpdater(function: markUninitialized.parentFunction,
103+
type: markUninitialized.type, ownership: .owned, context)
103104
ssaUpdater.addAvailableValue(markUninitialized, in: markUninitialized.parentBlock)
104105

105106
for endInst in initRegion.ends {

‎SwiftCompilerSources/Sources/Optimizer/FunctionPasses/RedundantLoadElimination.swift

+2-1
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,8 @@ private extension LoadInst {
236236
}
237237

238238
private func replace(load: LoadInst, with availableValues: [AvailableValue], _ context: FunctionPassContext) {
239-
var ssaUpdater = SSAUpdater(type: load.type, ownership: load.ownership, context)
239+
var ssaUpdater = SSAUpdater(function: load.parentFunction,
240+
type: load.type, ownership: load.ownership, context)
240241

241242
for availableValue in availableValues {
242243
let block = availableValue.instruction.parentBlock

‎SwiftCompilerSources/Sources/Optimizer/Utilities/SSAUpdater.swift

+4-2
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@ import OptimizerBridging
1717
struct SSAUpdater<Context: MutatingContext> {
1818
let context: Context
1919

20-
init(type: Type, ownership: Ownership, _ context: Context) {
20+
init(function: Function, type: Type, ownership: Ownership,
21+
_ context: Context) {
2122
self.context = context
22-
context._bridged.SSAUpdater_initialize(type.bridged, ownership._bridged)
23+
context._bridged.SSAUpdater_initialize(function.bridged, type.bridged,
24+
ownership._bridged)
2325
}
2426

2527
mutating func addAvailableValue(_ value: Value, in block: BasicBlock) {

‎include/swift/SIL/SILBuilder.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,11 @@ class SILBuilder {
200200
assert(F && "cannot create this instruction without a function context");
201201
return *F;
202202
}
203-
203+
204+
/// If this SILBuilder is inserting into a function, return that function. If
205+
/// we are inserting into a global, this returns nullptr.
206+
SILFunction *maybeGetFunction() const { return F; }
207+
204208
bool isInsertingIntoGlobal() const { return F == nullptr; }
205209

206210
TypeExpansionContext getTypeExpansionContext() const {

‎include/swift/SIL/SILCloner.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ SILCloner<ImplClass>::getMappedValue(SILValue Value) {
593593
if (auto *U = dyn_cast<SILUndef>(Value)) {
594594
auto type = getOpType(U->getType());
595595
ValueBase *undef =
596-
(type == U->getType() ? U : SILUndef::get(type, Builder.getFunction()));
596+
(type == U->getType() ? U : SILUndef::get(Builder.getFunction(), type));
597597
return SILValue(undef);
598598
}
599599

‎include/swift/SIL/SILFunction.h

+5
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class BasicBlockBitfield;
4040
class NodeBitfield;
4141
class OperandBitfield;
4242
class CalleeCache;
43+
class SILUndef;
4344

4445
namespace Lowering {
4546
class TypeLowering;
@@ -211,6 +212,7 @@ class SILFunction
211212
friend class BasicBlockBitfield;
212213
friend class NodeBitfield;
213214
friend class OperandBitfield;
215+
friend SILUndef;
214216

215217
/// Module - The SIL module that the function belongs to.
216218
SILModule &Module;
@@ -329,6 +331,9 @@ class SILFunction
329331

330332
PerformanceConstraints perfConstraints = PerformanceConstraints::None;
331333

334+
/// This is the set of undef values we've created, for uniquing purposes.
335+
llvm::DenseMap<SILType, SILUndef *> undefValues;
336+
332337
/// This is the number of uses of this SILFunction inside the SIL.
333338
/// It does not include references from debug scopes.
334339
unsigned RefCount = 0;

‎include/swift/SIL/SILModule.h

-5
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ class FuncDecl;
108108
class IRGenOptions;
109109
class KeyPathPattern;
110110
class ModuleDecl;
111-
class SILUndef;
112111
class SourceFile;
113112
class SerializedSILLoader;
114113
class SILFunctionBuilder;
@@ -192,7 +191,6 @@ class SILModule {
192191
friend SILType;
193192
friend SILVTable;
194193
friend SILProperty;
195-
friend SILUndef;
196194
friend SILWitnessTable;
197195
friend SILMoveOnlyDeinit;
198196
friend Lowering::SILGenModule;
@@ -315,9 +313,6 @@ class SILModule {
315313
/// This is a cache of builtin Function declarations to numeric ID mappings.
316314
llvm::DenseMap<Identifier, BuiltinInfo> BuiltinIDCache;
317315

318-
/// This is the set of undef values we've created, for uniquing purposes.
319-
llvm::DenseMap<SILType, SILUndef *> UndefValues;
320-
321316
llvm::DenseMap<std::pair<Decl *, VarDecl *>, unsigned> fieldIndices;
322317
llvm::DenseMap<EnumElementDecl *, unsigned> enumCaseIndices;
323318

‎include/swift/SIL/SILNode.h

+5-4
Original file line numberDiff line numberDiff line change
@@ -409,12 +409,13 @@ class alignas(8) SILNode :
409409
/// otherwise return null.
410410
SILBasicBlock *getParentBlock() const;
411411

412-
/// If this is a SILArgument or a SILInstruction get its parent function,
413-
/// otherwise return null.
412+
/// Returns the parent function of this value.
413+
///
414+
/// Only returns nullptr if the given value's parent is a sil global variable
415+
/// initializer.
414416
SILFunction *getFunction() const;
415417

416-
/// If this is a SILArgument or a SILInstruction get its parent module,
417-
/// otherwise return null.
418+
/// Return the parent module of this value.
418419
SILModule *getModule() const;
419420

420421
/// Pretty-print the node. If the node is an instruction, the output

‎include/swift/SIL/SILUndef.h

+15-7
Original file line numberDiff line numberDiff line change
@@ -23,28 +23,36 @@ class SILInstruction;
2323
class SILModule;
2424

2525
class SILUndef : public ValueBase {
26-
SILUndef(SILType type);
26+
/// A back pointer to the function that this SILUndef is uniqued by.
27+
SILFunction *parent;
28+
29+
SILUndef(SILFunction *parent, SILType type);
2730

2831
public:
2932
void operator=(const SILArgument &) = delete;
3033
void operator delete(void *, size_t) = delete;
3134

32-
static SILUndef *get(SILType ty, SILModule &m);
33-
3435
/// Return a SILUndef with the same type as the passed in value.
3536
static SILUndef *get(SILValue value) {
36-
return SILUndef::get(value->getType(), *value->getModule());
37+
return SILUndef::get(value->getFunction(), value->getType());
3738
}
3839

39-
static SILUndef *get(SILType ty, const SILFunction &f);
40+
static SILUndef *get(SILFunction *f, SILType ty);
41+
static SILUndef *get(SILFunction &f, SILType ty) {
42+
return SILUndef::get(&f, ty);
43+
}
4044

45+
/// This is an API only used by SILSSAUpdater... please do not use it anywhere
46+
/// else.
4147
template <class OwnerTy>
42-
static SILUndef *getSentinelValue(SILType type, OwnerTy owner) {
48+
static SILUndef *getSentinelValue(SILFunction *fn, OwnerTy owner,
49+
SILType type) {
4350
// Ownership kind isn't used here, the value just needs to have a unique
4451
// address.
45-
return new (*owner) SILUndef(type);
52+
return new (*owner) SILUndef(fn, type);
4653
}
4754

55+
SILFunction *getParent() const { return parent; }
4856
ValueOwnershipKind getOwnershipKind() const { return OwnershipKind::None; }
4957

5058
static bool classof(const SILArgument *) = delete;

‎include/swift/SILOptimizer/OptimizerBridging.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,9 @@ struct BridgedPassContext {
307307

308308
// SSAUpdater
309309

310-
BRIDGED_INLINE void SSAUpdater_initialize(BridgedType type, BridgedValue::Ownership ownership) const;
310+
BRIDGED_INLINE void
311+
SSAUpdater_initialize(BridgedFunction function, BridgedType type,
312+
BridgedValue::Ownership ownership) const;
311313
BRIDGED_INLINE void SSAUpdater_addAvailableValue(BridgedBasicBlock block, BridgedValue value) const;
312314
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedValue SSAUpdater_getValueAtEndOfBlock(BridgedBasicBlock block) const;
313315
SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedValue SSAUpdater_getValueInMiddleOfBlock(BridgedBasicBlock block) const;

‎include/swift/SILOptimizer/OptimizerBridgingImpl.h

+5-3
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ void BridgedPassContext::moveInstructionBefore(BridgedInstruction inst, BridgedI
229229
}
230230

231231
BridgedValue BridgedPassContext::getSILUndef(BridgedType type) const {
232-
return {swift::SILUndef::get(type.unbridged(), *invocation->getFunction())};
232+
return {swift::SILUndef::get(invocation->getFunction(), type.unbridged())};
233233
}
234234

235235
bool BridgedPassContext::optimizeMemoryAccesses(BridgedFunction f) const {
@@ -410,8 +410,10 @@ bool BridgedPassContext::continueWithNextSubpassRun(OptionalBridgedInstruction i
410410
inst.unbridged(), invocation->getFunction(), invocation->getTransform());
411411
}
412412

413-
void BridgedPassContext::SSAUpdater_initialize(BridgedType type, BridgedValue::Ownership ownership) const {
414-
invocation->initializeSSAUpdater(type.unbridged(),
413+
void BridgedPassContext::SSAUpdater_initialize(
414+
BridgedFunction function, BridgedType type,
415+
BridgedValue::Ownership ownership) const {
416+
invocation->initializeSSAUpdater(function.getFunction(), type.unbridged(),
415417
BridgedValue::castToOwnership(ownership));
416418
}
417419

‎include/swift/SILOptimizer/PassManager/PassManager.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,11 @@ class SwiftPassInvocation {
160160
void setNeedFixStackNesting(bool newValue) { needFixStackNesting = newValue; }
161161
bool getNeedFixStackNesting() const { return needFixStackNesting; }
162162

163-
void initializeSSAUpdater(SILType type, ValueOwnershipKind ownership) {
163+
void initializeSSAUpdater(SILFunction *fn, SILType type,
164+
ValueOwnershipKind ownership) {
164165
if (!ssaUpdater)
165166
ssaUpdater = new SILSSAUpdater;
166-
ssaUpdater->initialize(type, ownership);
167+
ssaUpdater->initialize(fn, type, ownership);
167168
}
168169

169170
SILSSAUpdater *getSSAUpdater() const {

‎include/swift/SILOptimizer/Utils/SILSSAUpdater.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,8 @@ class SILSSAUpdater {
7979
}
8080

8181
/// Initialize for a use of a value of type and ownershipKind
82-
void initialize(SILType type, ValueOwnershipKind ownershipKind);
82+
void initialize(SILFunction *fn, SILType type,
83+
ValueOwnershipKind ownershipKind);
8384

8485
bool hasValueForBlock(SILBasicBlock *block) const;
8586
void addAvailableValue(SILBasicBlock *block, SILValue value);

‎lib/IRGen/IRGenSIL.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -2809,7 +2809,7 @@ void IRGenSILFunction::visitDifferentiableFunctionInst(
28092809
i->getModule().Types,
28102810
LookUpConformanceInModule(i->getModule().getSwiftModule()));
28112811
auto *undef = SILUndef::get(
2812-
SILType::getPrimitiveObjectType(derivativeFnType), *i->getFunction());
2812+
i->getFunction(), SILType::getPrimitiveObjectType(derivativeFnType));
28132813
return getLoweredExplosion(undef);
28142814
};
28152815
auto jvpExp = getDerivativeExplosion(AutoDiffDerivativeFunctionKind::JVP);

‎lib/IRGen/LoadableByAddress.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -1383,7 +1383,7 @@ void LoadableStorageAllocation::allocateLoadableStorage() {
13831383
SILArgument *LoadableStorageAllocation::replaceArgType(SILBuilder &argBuilder,
13841384
SILArgument *arg,
13851385
SILType newSILType) {
1386-
SILValue undef = SILUndef::get(newSILType, *pass.F);
1386+
SILValue undef = SILUndef::get(pass.F, newSILType);
13871387
SmallVector<Operand *, 8> useList(arg->use_begin(), arg->use_end());
13881388
for (auto *use : useList) {
13891389
use->set(undef);
@@ -1459,7 +1459,7 @@ void LoadableStorageAllocation::convertIndirectFunctionArgs() {
14591459

14601460
static void convertBBArgType(SILBuilder &argBuilder, SILType newSILType,
14611461
SILArgument *arg) {
1462-
SILValue undef = SILUndef::get(newSILType, argBuilder.getFunction());
1462+
SILValue undef = SILUndef::get(argBuilder.getFunction(), newSILType);
14631463
SmallVector<Operand *, 8> useList(arg->use_begin(), arg->use_end());
14641464
for (auto *use : useList) {
14651465
use->set(undef);

‎lib/SIL/IR/SILBasicBlock.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ SILPhiArgument *SILBasicBlock::replacePhiArgumentAndReplaceAllUses(
232232
// replacePhiArgument() expects the replaced argument to not have
233233
// any uses.
234234
SmallVector<Operand *, 16> operands;
235-
SILValue undef = SILUndef::get(ty, *getParent());
235+
SILValue undef = SILUndef::get(getParent(), ty);
236236
SILArgument *arg = getArgument(i);
237237
while (!arg->use_empty()) {
238238
Operand *use = *arg->use_begin();

‎lib/SIL/IR/SILUndef.cpp

+5-9
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,12 @@
1515

1616
using namespace swift;
1717

18-
SILUndef::SILUndef(SILType type)
19-
: ValueBase(ValueKind::SILUndef, type) {}
18+
SILUndef::SILUndef(SILFunction *parent, SILType type)
19+
: ValueBase(ValueKind::SILUndef, type), parent(parent) {}
2020

21-
SILUndef *SILUndef::get(SILType ty, SILModule &m) {
22-
SILUndef *&entry = m.UndefValues[ty];
21+
SILUndef *SILUndef::get(SILFunction *fn, SILType ty) {
22+
SILUndef *&entry = fn->undefValues[ty];
2323
if (entry == nullptr)
24-
entry = new (m) SILUndef(ty);
24+
entry = new (fn->getModule()) SILUndef(fn, ty);
2525
return entry;
2626
}
27-
28-
SILUndef *SILUndef::get(SILType ty, const SILFunction &f) {
29-
return SILUndef::get(ty, f.getModule());
30-
}

‎lib/SIL/IR/SILValue.cpp

+13-8
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ void ValueBase::replaceAllUsesWithUndef() {
6262
}
6363
while (!use_empty()) {
6464
Operand *Op = *use_begin();
65-
Op->set(SILUndef::get(Op->get()->getType(), *F));
65+
Op->set(SILUndef::get(F, Op->get()->getType()));
6666
}
6767
}
6868

@@ -213,17 +213,22 @@ SILBasicBlock *SILNode::getParentBlock() const {
213213
}
214214

215215
SILFunction *SILNode::getFunction() const {
216-
if (auto *parentBlock = getParentBlock())
217-
return parentBlock->getParent();
218-
return nullptr;
219-
}
216+
if (auto *parentBlock = getParentBlock()) {
217+
// This can return nullptr if the block's parent is a global variable
218+
// initializer.
219+
if (auto *parentFunction = parentBlock->getParent()) {
220+
return parentFunction;
221+
}
222+
}
223+
224+
if (auto *undef = dyn_cast<SILUndef>(this))
225+
return undef->getParent();
220226

221-
SILModule *SILNode::getModule() const {
222-
if (SILFunction *func = getFunction())
223-
return &func->getModule();
224227
return nullptr;
225228
}
226229

230+
SILModule *SILNode::getModule() const { return &getFunction()->getModule(); }
231+
227232
/// Get a location for this value.
228233
SILLocation SILValue::getLoc() const {
229234
if (auto *instr = Value->getDefiningInstruction())

‎lib/SIL/Parser/ParseSIL.cpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,8 @@ bool SILParser::parseVerbatim(StringRef name) {
188188
SILParser::~SILParser() {
189189
for (auto &Entry : ForwardRefLocalValues) {
190190
if (ValueBase *dummyVal = LocalValues[Entry.first()]) {
191-
dummyVal->replaceAllUsesWith(SILUndef::get(dummyVal->getType(), SILMod));
191+
dummyVal->replaceAllUsesWith(
192+
SILUndef::get(dummyVal->getFunction(), dummyVal->getType()));
192193
::delete cast<PlaceholderValue>(dummyVal);
193194
}
194195
}
@@ -351,7 +352,7 @@ bool SILParser::parseGlobalName(Identifier &Name) {
351352
SILValue SILParser::getLocalValue(UnresolvedValueName Name, SILType Type,
352353
SILLocation Loc, SILBuilder &B) {
353354
if (Name.isUndef())
354-
return SILUndef::get(Type, B.getFunction());
355+
return SILUndef::get(B.getFunction(), Type);
355356

356357
// Check to see if this is already defined.
357358
ValueBase *&Entry = LocalValues[Name.Name];
@@ -365,7 +366,7 @@ SILValue SILParser::getLocalValue(UnresolvedValueName Name, SILType Type,
365366
P.diagnose(Name.NameLoc, diag::sil_value_use_type_mismatch, Name.Name,
366367
EntryTy.getRawASTType(), Type.getRawASTType());
367368
// Make sure to return something of the requested type.
368-
return SILUndef::get(Type, B.getFunction());
369+
return SILUndef::get(B.getFunction(), Type);
369370
}
370371

371372
return SILValue(Entry);

‎lib/SIL/Utils/Projection.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1244,7 +1244,7 @@ ProjectionTree::computeExplodedArgumentValueInner(SILBuilder &Builder,
12441244
if (Iter != LeafValues.end())
12451245
return Iter->second;
12461246
// Return undef for dead node.
1247-
return SILUndef::get(Node->getType(), Builder.getFunction());
1247+
return SILUndef::get(Builder.getFunction(), Node->getType());
12481248
}
12491249

12501250
// This is an aggregate node, construct its value from its children

‎lib/SILGen/SILGenBridging.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -1552,8 +1552,8 @@ SILFunction *SILGenFunction::emitNativeAsyncToForeignThunk(SILDeclRef thunk) {
15521552
scope.pop();
15531553

15541554
// Return void to the immediate caller.
1555-
B.createReturn(loc, SILUndef::get(SGM.Types.getEmptyTupleType(), F));
1556-
1555+
B.createReturn(loc, SILUndef::get(&F, SGM.Types.getEmptyTupleType()));
1556+
15571557
return closure;
15581558
}
15591559

@@ -1855,7 +1855,7 @@ void SILGenFunction::emitNativeToForeignThunk(SILDeclRef thunk) {
18551855
}
18561856

18571857
// The immediate function result is an empty tuple.
1858-
return SILUndef::get(SGM.Types.getEmptyTupleType(), F);
1858+
return SILUndef::get(&F, SGM.Types.getEmptyTupleType());
18591859
};
18601860

18611861
if (!substTy->hasErrorResult()) {
@@ -1966,7 +1966,7 @@ void SILGenFunction::emitNativeToForeignThunk(SILDeclRef thunk) {
19661966
auto paramTy = param.getSILStorageInterfaceType();
19671967
if (paramTy.isTrivial(F)) {
19681968
// If it's trivial, the value passed doesn't matter.
1969-
completionHandlerArgs.push_back(SILUndef::get(paramTy, F.getModule()));
1969+
completionHandlerArgs.push_back(SILUndef::get(&F, paramTy));
19701970
} else {
19711971
// If it's not trivial, it must be a nullable class type. Pass
19721972
// nil.
@@ -2002,7 +2002,7 @@ void SILGenFunction::emitNativeToForeignThunk(SILDeclRef thunk) {
20022002
if (foreignAsync) {
20032003
// After invoking the completion handler, our immediate return value is
20042004
// void.
2005-
result = SILUndef::get(SGM.Types.getEmptyTupleType(), F);
2005+
result = SILUndef::get(&F, SGM.Types.getEmptyTupleType());
20062006
} else {
20072007
result = contBB->createPhiArgument(objcResultTy, OwnershipKind::Owned);
20082008
}

0 commit comments

Comments
 (0)