Skip to content

Commit cf4f47b

Browse files
committed
[ome] Explicitly set to OwnershipKind::None the ownership forwarded by all instructions that inherit from OwnershipForwardingMixin.
I thought it would be enough to change SILValue::getOwnershipKind() to return None when we are not in OSSA, but I found that is not sufficient. In some cases, users will call getOwnershipKind() directly on these classes and will get the stored value (which has not been changed) instead of the correct ownership, OwnershipKind::None. I ran into this when one of the RAUW utilities when done a path that wanted Owned ownership when we were in a non-ossa function and then (happily I may add) hit an assert.
1 parent ee17f5e commit cf4f47b

File tree

1 file changed

+47
-2
lines changed

1 file changed

+47
-2
lines changed

lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,14 @@
2222
///
2323
//===----------------------------------------------------------------------===//
2424

25+
#include "llvm/Support/ErrorHandling.h"
2526
#define DEBUG_TYPE "sil-ownership-model-eliminator"
2627

2728
#include "swift/Basic/BlotSetVector.h"
2829
#include "swift/SIL/Projection.h"
2930
#include "swift/SIL/SILBuilder.h"
3031
#include "swift/SIL/SILFunction.h"
32+
#include "swift/SIL/SILInstruction.h"
3133
#include "swift/SIL/SILVisitor.h"
3234
#include "swift/SILOptimizer/Analysis/SimplifyInstruction.h"
3335
#include "swift/SILOptimizer/PassManager/Transforms.h"
@@ -119,7 +121,16 @@ struct OwnershipModelEliminatorVisitor
119121
eraseInstruction(i);
120122
}
121123

122-
bool visitSILInstruction(SILInstruction *) { return false; }
124+
bool visitSILInstruction(SILInstruction *inst) {
125+
// Make sure this wasn't a forwarding instruction in case someone adds a new
126+
// forwarding instruction but does not update this code.
127+
if (OwnershipForwardingMixin::isa(inst)) {
128+
llvm::errs() << "Found unhandled forwarding inst: " << *inst;
129+
llvm_unreachable("standard error handler");
130+
}
131+
return false;
132+
}
133+
123134
bool visitLoadInst(LoadInst *li);
124135
bool visitStoreInst(StoreInst *si);
125136
bool visitStoreBorrowInst(StoreBorrowInst *si);
@@ -164,6 +175,37 @@ struct OwnershipModelEliminatorVisitor
164175

165176
void splitDestructure(SILInstruction *destructure,
166177
SILValue destructureOperand);
178+
179+
#define HANDLE_FORWARDING_INST(Cls) \
180+
bool visit##Cls##Inst(Cls##Inst *i) { \
181+
OwnershipForwardingMixin::get(i)->setOwnershipKind(OwnershipKind::None); \
182+
return true; \
183+
}
184+
HANDLE_FORWARDING_INST(ConvertFunction)
185+
HANDLE_FORWARDING_INST(Upcast)
186+
HANDLE_FORWARDING_INST(UncheckedRefCast)
187+
HANDLE_FORWARDING_INST(RefToBridgeObject)
188+
HANDLE_FORWARDING_INST(BridgeObjectToRef)
189+
HANDLE_FORWARDING_INST(ThinToThickFunction)
190+
HANDLE_FORWARDING_INST(UnconditionalCheckedCast)
191+
HANDLE_FORWARDING_INST(Struct)
192+
HANDLE_FORWARDING_INST(Object)
193+
HANDLE_FORWARDING_INST(Tuple)
194+
HANDLE_FORWARDING_INST(Enum)
195+
HANDLE_FORWARDING_INST(UncheckedEnumData)
196+
HANDLE_FORWARDING_INST(SelectEnum)
197+
HANDLE_FORWARDING_INST(SelectValue)
198+
HANDLE_FORWARDING_INST(OpenExistentialRef)
199+
HANDLE_FORWARDING_INST(InitExistentialRef)
200+
HANDLE_FORWARDING_INST(MarkDependence)
201+
HANDLE_FORWARDING_INST(DifferentiableFunction)
202+
HANDLE_FORWARDING_INST(LinearFunction)
203+
HANDLE_FORWARDING_INST(StructExtract)
204+
HANDLE_FORWARDING_INST(TupleExtract)
205+
HANDLE_FORWARDING_INST(LinearFunctionExtract)
206+
HANDLE_FORWARDING_INST(DifferentiableFunctionExtract)
207+
HANDLE_FORWARDING_INST(MarkUninitialized)
208+
#undef HANDLE_FORWARDING_INST
167209
};
168210

169211
} // end anonymous namespace
@@ -295,6 +337,8 @@ bool OwnershipModelEliminatorVisitor::visitDestroyValueInst(
295337

296338
bool OwnershipModelEliminatorVisitor::visitCheckedCastBranchInst(
297339
CheckedCastBranchInst *cbi) {
340+
cbi->setOwnershipKind(OwnershipKind::None);
341+
298342
// In ownership qualified SIL, checked_cast_br must pass its argument to the
299343
// fail case so we can clean it up. In non-ownership qualified SIL, we expect
300344
// no argument from the checked_cast_br in the default case. The way that we
@@ -313,6 +357,8 @@ bool OwnershipModelEliminatorVisitor::visitCheckedCastBranchInst(
313357

314358
bool OwnershipModelEliminatorVisitor::visitSwitchEnumInst(
315359
SwitchEnumInst *swei) {
360+
swei->setOwnershipKind(OwnershipKind::None);
361+
316362
// In ownership qualified SIL, switch_enum must pass its argument to the fail
317363
// case so we can clean it up. In non-ownership qualified SIL, we expect no
318364
// argument from the switch_enum in the default case. The way that we handle
@@ -340,7 +386,6 @@ void OwnershipModelEliminatorVisitor::splitDestructure(
340386

341387
// First before we destructure anything, see if we can simplify any of our
342388
// instruction operands.
343-
344389
SILModule &M = destructureInst->getModule();
345390
SILType opType = destructureOperand->getType();
346391

0 commit comments

Comments
 (0)