Skip to content

Commit 11f0ff6

Browse files
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

Lines changed: 2 additions & 1 deletion
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

Lines changed: 2 additions & 1 deletion
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

Lines changed: 4 additions & 2 deletions
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

Lines changed: 5 additions & 1 deletion
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 5 additions & 0 deletions
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

Lines changed: 0 additions & 5 deletions
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

Lines changed: 5 additions & 4 deletions
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

Lines changed: 15 additions & 7 deletions
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

Lines changed: 3 additions & 1 deletion
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;

0 commit comments

Comments
 (0)