Skip to content

Commit 62d5546

Browse files
gottesmmgottesmm
committed
[ownership] Rename OwnershipForwardingMixin::{get,set}OwnershipKind() -> {get,set}ForwardingOwnershipKind().
TLDR: This is just an NFC rename in preparation for changing SILValue::getOwnershipKind() of any forwarding instructions to return OwnershipKind::None if they have a trivial result despite forwarding ownership that isn't OwnershipKind::None (consider an unchecked_enum_data of a trivial payload from a non-trivial enum). This ensures that one does not by mistake use this routine instead of SILValue::getOwnershipKind(). The reason why these two things must be distinguished is that the forwarding ownership kind of an instruction that inherits from OwnershipForwardingMixin is explicitly not the ValueOwnershipKind of the result of the instruction. Instead it is a separate piece of state that: 1. For certain forwarding instructions, defines the OwnershipConstraint of the forwarding instruction. 2. Defines the ownership kind of the result of the value. If the result of the value is non-trivial then it is exactly the set ownership kind. If the result is trivial, we use OwnershipKind::None instead. As an example of this, consider an unchecked_enum_data that extracts from a non-trivial enum a trivial payload: ``` enum Either { case int(Int) case obj(Klass) } %1 = load_borrow %0 : $*Either %2 = unchecked_enum_data %1 : $Either, #Either.int!enumelt.1 // Int type end_borrow %1 : $Either ``` If we were to identify the forwarding ownership kind (guaranteed) of unchecked_enum_data with the value ownership kind of its result, we would violate ownership since we would be passing a guaranteed value to the operand of the unchecked_enum_data that will only accept values with OwnershipKind::None. =><=.
1 parent edcc3a2 commit 62d5546

File tree

9 files changed

+56
-61
lines changed

9 files changed

+56
-61
lines changed

‎include/swift/SIL/SILInstruction.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,9 +1025,12 @@ class OwnershipForwardingMixin {
10251025
}
10261026

10271027
public:
1028-
ValueOwnershipKind getOwnershipKind() const { return ownershipKind; }
1029-
1030-
void setOwnershipKind(ValueOwnershipKind newKind) { ownershipKind = newKind; }
1028+
ValueOwnershipKind getForwardingOwnershipKind() const {
1029+
return ownershipKind;
1030+
}
1031+
void setForwardingOwnershipKind(ValueOwnershipKind newKind) {
1032+
ownershipKind = newKind;
1033+
}
10311034

10321035
/// Defined inline below due to forward declaration issues.
10331036
static OwnershipForwardingMixin *get(SILInstruction *inst);

‎lib/SIL/IR/OperandOwnership.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,8 @@ OPERAND_OWNERSHIP(EndBorrow, AbortApply)
299299
#define FORWARDING_OWNERSHIP(INST) \
300300
OperandOwnership OperandOwnershipClassifier::visit##INST##Inst( \
301301
INST##Inst *i) { \
302-
return i->getOwnershipKind().getForwardingOperandOwnership( \
303-
/*allowUnowned*/false); \
302+
return i->getForwardingOwnershipKind().getForwardingOperandOwnership( \
303+
/*allowUnowned*/ false); \
304304
}
305305
FORWARDING_OWNERSHIP(Object)
306306
FORWARDING_OWNERSHIP(OpenExistentialRef)
@@ -318,8 +318,8 @@ FORWARDING_OWNERSHIP(LinearFunction)
318318
#define FORWARDING_ANY_OWNERSHIP(INST) \
319319
OperandOwnership OperandOwnershipClassifier::visit##INST##Inst( \
320320
INST##Inst *i) { \
321-
return i->getOwnershipKind().getForwardingOperandOwnership( \
322-
/*allowUnowned*/true); \
321+
return i->getForwardingOwnershipKind().getForwardingOperandOwnership( \
322+
/*allowUnowned*/ true); \
323323
}
324324
FORWARDING_ANY_OWNERSHIP(Upcast)
325325
FORWARDING_ANY_OWNERSHIP(UncheckedRefCast)
@@ -338,8 +338,8 @@ FORWARDING_ANY_OWNERSHIP(CheckedCastBranch)
338338
#define AGGREGATE_OWNERSHIP(INST) \
339339
OperandOwnership OperandOwnershipClassifier::visit##INST##Inst( \
340340
INST##Inst *i) { \
341-
return i->getOwnershipKind().getForwardingOperandOwnership( \
342-
/*allowUnowned*/true); \
341+
return i->getForwardingOwnershipKind().getForwardingOperandOwnership( \
342+
/*allowUnowned*/ true); \
343343
}
344344
AGGREGATE_OWNERSHIP(Tuple)
345345
AGGREGATE_OWNERSHIP(Struct)

‎lib/SIL/IR/ValueOwnership.cpp

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ ValueOwnershipKindClassifier::visitForwardingInst(SILInstruction *i,
241241
#define FORWARDING_OWNERSHIP_INST(INST) \
242242
ValueOwnershipKind ValueOwnershipKindClassifier::visit##INST##Inst( \
243243
INST##Inst *I) { \
244-
return I->getOwnershipKind(); \
244+
return I->getForwardingOwnershipKind(); \
245245
}
246246
FORWARDING_OWNERSHIP_INST(BridgeObjectToRef)
247247
FORWARDING_OWNERSHIP_INST(ConvertFunction)
@@ -258,6 +258,7 @@ FORWARDING_OWNERSHIP_INST(UncheckedValueCast)
258258
FORWARDING_OWNERSHIP_INST(UncheckedEnumData)
259259
FORWARDING_OWNERSHIP_INST(SelectEnum)
260260
FORWARDING_OWNERSHIP_INST(Enum)
261+
FORWARDING_OWNERSHIP_INST(MarkDependence)
261262
// NOTE: init_existential_ref from a reference counting perspective is not
262263
// considered to be "owned" since it doesn't affect reference counts. That being
263264
// said in the past, we wanted to conceptually treat it as an owned value that
@@ -312,12 +313,6 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitSILFunctionArgument(
312313
return Arg->getOwnershipKind();
313314
}
314315

315-
// This is a forwarding instruction through only one of its arguments.
316-
ValueOwnershipKind
317-
ValueOwnershipKindClassifier::visitMarkDependenceInst(MarkDependenceInst *MDI) {
318-
return MDI->getOwnershipKind();
319-
}
320-
321316
ValueOwnershipKind ValueOwnershipKindClassifier::visitApplyInst(ApplyInst *ai) {
322317
auto *f = ai->getFunction();
323318
bool isTrivial = ai->getType().isTrivial(*f);

‎lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 24 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,26 +1081,26 @@ ValueOwnershipKind ForwardingOperand::getOwnershipKind() const {
10811081
// in each if itself since we have an unreachable at the bottom to ensure if a
10821082
// new subclass of OwnershipForwardingInst is added
10831083
if (auto *ofsvi = dyn_cast<AllArgOwnershipForwardingSingleValueInst>(user))
1084-
return ofsvi->getOwnershipKind();
1084+
return ofsvi->getForwardingOwnershipKind();
10851085

10861086
if (auto *ofsvi = dyn_cast<FirstArgOwnershipForwardingSingleValueInst>(user))
1087-
return ofsvi->getOwnershipKind();
1087+
return ofsvi->getForwardingOwnershipKind();
10881088

10891089
if (auto *ofci = dyn_cast<OwnershipForwardingConversionInst>(user))
1090-
return ofci->getOwnershipKind();
1090+
return ofci->getForwardingOwnershipKind();
10911091

10921092
if (auto *ofseib = dyn_cast<OwnershipForwardingSelectEnumInstBase>(user))
1093-
return ofseib->getOwnershipKind();
1093+
return ofseib->getForwardingOwnershipKind();
10941094

10951095
if (auto *ofmvi =
10961096
dyn_cast<OwnershipForwardingMultipleValueInstruction>(user)) {
10971097
assert(ofmvi->getNumOperands() == 1);
1098-
return ofmvi->getOwnershipKind();
1098+
return ofmvi->getForwardingOwnershipKind();
10991099
}
11001100

11011101
if (auto *ofti = dyn_cast<OwnershipForwardingTermInst>(user)) {
11021102
assert(ofti->getNumOperands() == 1);
1103-
return ofti->getOwnershipKind();
1103+
return ofti->getForwardingOwnershipKind();
11041104
}
11051105

11061106
llvm_unreachable("Unhandled forwarding inst?!");
@@ -1112,21 +1112,17 @@ void ForwardingOperand::setOwnershipKind(ValueOwnershipKind newKind) const {
11121112
// in each if itself since we have an unreachable at the bottom to ensure if a
11131113
// new subclass of OwnershipForwardingInst is added
11141114
if (auto *ofsvi = dyn_cast<AllArgOwnershipForwardingSingleValueInst>(user))
1115-
if (!ofsvi->getType().isTrivial(*ofsvi->getFunction()))
1116-
return ofsvi->setOwnershipKind(newKind);
1115+
return ofsvi->setForwardingOwnershipKind(newKind);
11171116
if (auto *ofsvi = dyn_cast<FirstArgOwnershipForwardingSingleValueInst>(user))
1118-
if (!ofsvi->getType().isTrivial(*ofsvi->getFunction()))
1119-
return ofsvi->setOwnershipKind(newKind);
1117+
return ofsvi->setForwardingOwnershipKind(newKind);
11201118
if (auto *ofci = dyn_cast<OwnershipForwardingConversionInst>(user))
1121-
if (!ofci->getType().isTrivial(*ofci->getFunction()))
1122-
return ofci->setOwnershipKind(newKind);
1119+
return ofci->setForwardingOwnershipKind(newKind);
11231120
if (auto *ofseib = dyn_cast<OwnershipForwardingSelectEnumInstBase>(user))
1124-
if (!ofseib->getType().isTrivial(*ofseib->getFunction()))
1125-
return ofseib->setOwnershipKind(newKind);
1121+
return ofseib->setForwardingOwnershipKind(newKind);
11261122
if (auto *ofmvi = dyn_cast<OwnershipForwardingMultipleValueInstruction>(user)) {
11271123
assert(ofmvi->getNumOperands() == 1);
11281124
if (!ofmvi->getOperand(0)->getType().isTrivial(*ofmvi->getFunction())) {
1129-
ofmvi->setOwnershipKind(newKind);
1125+
ofmvi->setForwardingOwnershipKind(newKind);
11301126
// TODO: Refactor this better.
11311127
if (auto *dsi = dyn_cast<DestructureStructInst>(ofmvi)) {
11321128
for (auto &result : dsi->getAllResultsBuffer()) {
@@ -1149,7 +1145,7 @@ void ForwardingOperand::setOwnershipKind(ValueOwnershipKind newKind) const {
11491145
if (auto *ofti = dyn_cast<OwnershipForwardingTermInst>(user)) {
11501146
assert(ofti->getNumOperands() == 1);
11511147
if (!ofti->getOperand(0)->getType().isTrivial(*ofti->getFunction())) {
1152-
ofti->setOwnershipKind(newKind);
1148+
ofti->setForwardingOwnershipKind(newKind);
11531149

11541150
// Then convert all of its incoming values that are owned to be guaranteed.
11551151
for (auto &succ : ofti->getSuccessors()) {
@@ -1178,24 +1174,24 @@ void ForwardingOperand::replaceOwnershipKind(ValueOwnershipKind oldKind,
11781174
auto *user = use->getUser();
11791175

11801176
if (auto *fInst = dyn_cast<AllArgOwnershipForwardingSingleValueInst>(user))
1181-
if (fInst->getOwnershipKind() == oldKind)
1182-
return fInst->setOwnershipKind(newKind);
1177+
if (fInst->getForwardingOwnershipKind() == oldKind)
1178+
return fInst->setForwardingOwnershipKind(newKind);
11831179

11841180
if (auto *fInst = dyn_cast<FirstArgOwnershipForwardingSingleValueInst>(user))
1185-
if (fInst->getOwnershipKind() == oldKind)
1186-
return fInst->setOwnershipKind(newKind);
1181+
if (fInst->getForwardingOwnershipKind() == oldKind)
1182+
return fInst->setForwardingOwnershipKind(newKind);
11871183

11881184
if (auto *ofci = dyn_cast<OwnershipForwardingConversionInst>(user))
1189-
if (ofci->getOwnershipKind() == oldKind)
1190-
return ofci->setOwnershipKind(newKind);
1185+
if (ofci->getForwardingOwnershipKind() == oldKind)
1186+
return ofci->setForwardingOwnershipKind(newKind);
11911187

11921188
if (auto *ofseib = dyn_cast<OwnershipForwardingSelectEnumInstBase>(user))
1193-
if (ofseib->getOwnershipKind() == oldKind)
1194-
return ofseib->setOwnershipKind(newKind);
1189+
if (ofseib->getForwardingOwnershipKind() == oldKind)
1190+
return ofseib->setForwardingOwnershipKind(newKind);
11951191

11961192
if (auto *ofmvi = dyn_cast<OwnershipForwardingMultipleValueInstruction>(user)) {
1197-
if (ofmvi->getOwnershipKind() == oldKind) {
1198-
ofmvi->setOwnershipKind(newKind);
1193+
if (ofmvi->getForwardingOwnershipKind() == oldKind) {
1194+
ofmvi->setForwardingOwnershipKind(newKind);
11991195
}
12001196
// TODO: Refactor this better.
12011197
if (auto *dsi = dyn_cast<DestructureStructInst>(ofmvi)) {
@@ -1216,8 +1212,8 @@ void ForwardingOperand::replaceOwnershipKind(ValueOwnershipKind oldKind,
12161212
}
12171213

12181214
if (auto *ofti = dyn_cast<OwnershipForwardingTermInst>(user)) {
1219-
if (ofti->getOwnershipKind() == oldKind) {
1220-
ofti->setOwnershipKind(newKind);
1215+
if (ofti->getForwardingOwnershipKind() == oldKind) {
1216+
ofti->setForwardingOwnershipKind(newKind);
12211217
// Then convert all of its incoming values that are owned to be guaranteed.
12221218
for (auto &succ : ofti->getSuccessors()) {
12231219
auto *succBlock = succ.getBB();

‎lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,14 +1161,14 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
11611161
void checkOwnershipForwardingInst(SILInstruction *i) {
11621162
if (auto *o = dyn_cast<OwnedFirstArgForwardingSingleValueInst>(i)) {
11631163
ValueOwnershipKind kind = OwnershipKind::Owned;
1164-
require(kind.isCompatibleWith(o->getOwnershipKind()),
1164+
require(kind.isCompatibleWith(o->getForwardingOwnershipKind()),
11651165
"OwnedFirstArgForwardingSingleValueInst's ownership kind must be "
11661166
"compatible with owned");
11671167
}
11681168

11691169
if (auto *o = dyn_cast<GuaranteedFirstArgForwardingSingleValueInst>(i)) {
11701170
ValueOwnershipKind kind = OwnershipKind::Guaranteed;
1171-
require(kind.isCompatibleWith(o->getOwnershipKind()),
1171+
require(kind.isCompatibleWith(o->getForwardingOwnershipKind()),
11721172
"GuaranteedFirstArgForwardingSingleValueInst's ownership kind "
11731173
"must be compatible with guaranteed");
11741174
}
@@ -2997,7 +2997,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
29972997
// compatible with our destructure_struct's ownership kind /and/ that if
29982998
// our destructure ownership kind is non-trivial then all non-trivial
29992999
// results must have the same ownership kind as our operand.
3000-
auto parentKind = DSI->getOwnershipKind();
3000+
auto parentKind = DSI->getForwardingOwnershipKind();
30013001
for (const DestructureStructResult &result : DSI->getAllResultsBuffer()) {
30023002
require(parentKind.isCompatibleWith(result.getOwnershipKind()),
30033003
"destructure result with ownership that is incompatible with "
@@ -3016,7 +3016,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
30163016
// compatible with our destructure_struct's ownership kind /and/ that if
30173017
// our destructure ownership kind is non-trivial then all non-trivial
30183018
// results must have the same ownership kind as our operand.
3019-
auto parentKind = dti->getOwnershipKind();
3019+
auto parentKind = dti->getForwardingOwnershipKind();
30203020
for (const auto &result : dti->getAllResultsBuffer()) {
30213021
require(parentKind.isCompatibleWith(result.getOwnershipKind()),
30223022
"destructure result with ownership that is incompatible with "
@@ -4407,7 +4407,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
44074407
if (!dest->getArgument(0)->getType().isTrivial(*SOI->getFunction())) {
44084408
require(
44094409
dest->getArgument(0)->getOwnershipKind().isCompatibleWith(
4410-
SOI->getOwnershipKind()),
4410+
SOI->getForwardingOwnershipKind()),
44114411
"Switch enum non-trivial destination arg must have ownership "
44124412
"kind that is compatible with the switch_enum's operand");
44134413
}

‎lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,8 @@ struct OwnershipModelEliminatorVisitor
178178

179179
#define HANDLE_FORWARDING_INST(Cls) \
180180
bool visit##Cls##Inst(Cls##Inst *i) { \
181-
OwnershipForwardingMixin::get(i)->setOwnershipKind(OwnershipKind::None); \
181+
OwnershipForwardingMixin::get(i)->setForwardingOwnershipKind( \
182+
OwnershipKind::None); \
182183
return true; \
183184
}
184185
HANDLE_FORWARDING_INST(ConvertFunction)
@@ -337,7 +338,7 @@ bool OwnershipModelEliminatorVisitor::visitDestroyValueInst(
337338

338339
bool OwnershipModelEliminatorVisitor::visitCheckedCastBranchInst(
339340
CheckedCastBranchInst *cbi) {
340-
cbi->setOwnershipKind(OwnershipKind::None);
341+
cbi->setForwardingOwnershipKind(OwnershipKind::None);
341342

342343
// In ownership qualified SIL, checked_cast_br must pass its argument to the
343344
// fail case so we can clean it up. In non-ownership qualified SIL, we expect
@@ -357,7 +358,7 @@ bool OwnershipModelEliminatorVisitor::visitCheckedCastBranchInst(
357358

358359
bool OwnershipModelEliminatorVisitor::visitSwitchEnumInst(
359360
SwitchEnumInst *swei) {
360-
swei->setOwnershipKind(OwnershipKind::None);
361+
swei->setForwardingOwnershipKind(OwnershipKind::None);
361362

362363
// In ownership qualified SIL, switch_enum must pass its argument to the fail
363364
// case so we can clean it up. In non-ownership qualified SIL, we expect no

‎lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -741,7 +741,7 @@ SILCombiner::buildConcreteOpenedExistentialInfoFromSoleConformingType(
741741
// If we have an owned open_existential_ref, we only optimize for now if our
742742
// open_existential_ref has a single non-debug consuming use that is a
743743
// destroy_value.
744-
if (OER->getOwnershipKind() != OwnershipKind::Owned) {
744+
if (OER->getForwardingOwnershipKind() != OwnershipKind::Owned) {
745745
// We use OER as the insertion point so that
746746
SILBuilderWithScope b(std::next(OER->getIterator()), Builder);
747747
auto loc = RegularLocation::getAutoGeneratedLocation();

‎lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ SILCombiner::visitUncheckedRefCastInst(UncheckedRefCastInst *urci) {
471471
// can remove both the open_existential_ref and the init_existential_ref.
472472
if (auto *oer = dyn_cast<OpenExistentialRefInst>(urci->getOperand())) {
473473
if (auto *ier = dyn_cast<InitExistentialRefInst>(oer->getOperand())) {
474-
if (ier->getOwnershipKind() != OwnershipKind::Owned) {
474+
if (ier->getForwardingOwnershipKind() != OwnershipKind::Owned) {
475475
return Builder.createUncheckedRefCast(urci->getLoc(), ier->getOperand(),
476476
urci->getType());
477477
}
@@ -735,8 +735,8 @@ SILCombiner::visitRawPointerToRefInst(RawPointerToRefInst *rawToRef) {
735735
// contrast, for guaranteed, we are replacing a BitwiseEscape use
736736
// (ref_to_rawpointer) with a ForwardedBorrowingUse (unchecked_ref_cast)
737737
// which is safe.
738-
if (newInst->getOwnershipKind() == OwnershipKind::Owned) {
739-
newInst->setOwnershipKind(OwnershipKind::Unowned);
738+
if (newInst->getForwardingOwnershipKind() == OwnershipKind::Owned) {
739+
newInst->setForwardingOwnershipKind(OwnershipKind::Unowned);
740740
}
741741
helper.perform(newInst);
742742
return nullptr;
@@ -825,8 +825,8 @@ visitUncheckedBitwiseCastInst(UncheckedBitwiseCastInst *UBCI) {
825825
// creating breaking OSSA. In contrast, if we have a guaranteed value, we
826826
// are going to be replacing an UnownedInstantaneousUse with an
827827
// InstantaneousUse which is always safe for a guaranteed value.
828-
if (newInst->getOwnershipKind() == OwnershipKind::Owned) {
829-
newInst->setOwnershipKind(OwnershipKind::Unowned);
828+
if (newInst->getForwardingOwnershipKind() == OwnershipKind::Owned) {
829+
newInst->setForwardingOwnershipKind(OwnershipKind::Unowned);
830830
}
831831
helper.perform(newInst);
832832
return nullptr;
@@ -1086,7 +1086,7 @@ SILCombiner::visitConvertFunctionInst(ConvertFunctionInst *cfi) {
10861086
// We handle the case of an identity conversion in inst simplify, so if we
10871087
// see this pattern then we know that we don't have a round trip and thus
10881088
// should just bypass the intermediate conversion.
1089-
if (cfi->getOwnershipKind() != OwnershipKind::Owned) {
1089+
if (cfi->getForwardingOwnershipKind() != OwnershipKind::Owned) {
10901090
cfi->getOperandRef().set(subCFI->getConverted());
10911091
// Return cfi to show we changed it.
10921092
return cfi;

‎lib/SILOptimizer/SemanticARC/OwnershipLiveRange.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,12 +263,12 @@ static SILValue convertIntroducerToGuaranteed(OwnedValueIntroducer introducer) {
263263
}
264264
case OwnedValueIntroducerKind::Struct: {
265265
auto *si = cast<StructInst>(introducer.value);
266-
si->setOwnershipKind(OwnershipKind::Guaranteed);
266+
si->setForwardingOwnershipKind(OwnershipKind::Guaranteed);
267267
return si;
268268
}
269269
case OwnedValueIntroducerKind::Tuple: {
270270
auto *ti = cast<TupleInst>(introducer.value);
271-
ti->setOwnershipKind(OwnershipKind::Guaranteed);
271+
ti->setForwardingOwnershipKind(OwnershipKind::Guaranteed);
272272
return ti;
273273
}
274274
case OwnedValueIntroducerKind::Copy:

0 commit comments

Comments
 (0)