Skip to content

Commit ae6cd35

Browse files
gottesmmgottesmm
committed
[ownership] Add helper method Operand::isConsumingUse() const.
This just provides a higher level of abstraction around determining if an operand consumes its associated value needing to touch the ownership kind map/etc. I also refactored parts of the code base where it made sense to use this instead of messing with the ownership kind map itself.
1 parent 885dda1 commit ae6cd35

File tree

3 files changed

+61
-59
lines changed

3 files changed

+61
-59
lines changed

‎include/swift/SIL/SILValue.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,14 @@ class Operand {
644644
OperandOwnershipKindMap
645645
getOwnershipKindMap(bool isForwardingSubValue = false) const;
646646

647+
/// Returns true if this operand acts as a use that consumes its associated
648+
/// value.
649+
bool isConsumingUse() const {
650+
auto map = getOwnershipKindMap();
651+
auto constraint = map.getLifetimeConstraint(get().getOwnershipKind());
652+
return constraint == UseLifetimeConstraint::MustBeInvalidated;
653+
}
654+
647655
private:
648656
void removeFromCurrent() {
649657
if (!Back) return;

‎lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp

Lines changed: 51 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -79,75 +79,72 @@ class LiveRange {
7979

8080
LiveRange::LiveRange(SILValue value)
8181
: destroys(), generalForwardingInsts(), unknownConsumingUsers() {
82-
SmallVector<Operand *, 32> worklist(value->getUses());
82+
assert(value.getOwnershipKind() == ValueOwnershipKind::Owned);
8383

84+
// We know that our silvalue produces an @owned value. Look through all of our
85+
// uses and classify them as either consuming or not.
86+
SmallVector<Operand *, 32> worklist(value->getUses());
8487
while (!worklist.empty()) {
8588
auto *op = worklist.pop_back_val();
8689

8790
// Skip type dependent operands.
8891
if (op->isTypeDependent())
8992
continue;
9093

91-
auto *user = op->getUser();
94+
// Do a quick check that we did not add ValueOwnershipKind that are not
95+
// owned to the worklist.
96+
assert(op->get().getOwnershipKind() == ValueOwnershipKind::Owned &&
97+
"Added non-owned value to worklist?!");
9298

93-
// We know that a copy_value produces an @owned value. Look through all of
94-
// our uses and classify them as either invalidating or not
95-
// invalidating. Make sure that all of the invalidating ones are
96-
// destroy_value since otherwise the live_range is not complete.
97-
auto map = op->getOwnershipKindMap();
98-
auto constraint = map.getLifetimeConstraint(ValueOwnershipKind::Owned);
99-
switch (constraint) {
100-
case UseLifetimeConstraint::MustBeInvalidated: {
101-
// See if we have a destroy value. If we don't we have an
102-
// unknown consumer. Return false, we need this live range.
103-
if (auto *dvi = dyn_cast<DestroyValueInst>(user)) {
104-
destroys.push_back(dvi);
105-
continue;
106-
}
99+
auto *user = op->getUser();
107100

108-
// Otherwise, see if we have a forwarding value that has a single
109-
// non-trivial operand that can accept a guaranteed value. If not, we can
110-
// not recursively process it, so be conservative and assume that we /may
111-
// consume/ the value, so the live range must not be eliminated.
112-
//
113-
// DISCUSSION: For now we do not support forwarding instructions with
114-
// multiple non-trivial arguments since we would need to optimize all of
115-
// the non-trivial arguments at the same time.
116-
//
117-
// NOTE: Today we do not support TermInsts for simplicity... we /could/
118-
// support it though if we need to.
119-
if (isa<TermInst>(user) || !isGuaranteedForwardingInst(user) ||
120-
1 != count_if(user->getOperandValues(
121-
true /*ignore type dependent operands*/),
122-
[&](SILValue v) {
123-
return v.getOwnershipKind() ==
124-
ValueOwnershipKind::Owned;
125-
})) {
126-
unknownConsumingUsers.push_back(user);
127-
continue;
128-
}
101+
// Ok, this constraint can take something owned as live. Assert that it
102+
// can also accept something that is guaranteed. Any non-consuming use of
103+
// an owned value should be able to take a guaranteed parameter as well
104+
// (modulo bugs). We assert to catch these.
105+
if (!op->isConsumingUse()) {
106+
continue;
107+
}
129108

130-
// Ok, this is a forwarding instruction whose ownership we can flip from
131-
// owned -> guaranteed. Visit its users recursively to see if the the
132-
// users force the live range to be alive.
133-
generalForwardingInsts.push_back(user);
134-
for (SILValue v : user->getResults()) {
135-
if (v.getOwnershipKind() != ValueOwnershipKind::Owned)
136-
continue;
137-
llvm::copy(v->getUses(), std::back_inserter(worklist));
138-
}
109+
// Ok, we know now that we have a consuming use. See if we have a destroy
110+
// value, quickly up front. If we do have one, stash it and continue.
111+
if (auto *dvi = dyn_cast<DestroyValueInst>(user)) {
112+
destroys.push_back(dvi);
139113
continue;
140114
}
141-
case UseLifetimeConstraint::MustBeLive:
142-
// Ok, this constraint can take something owned as live. Assert that it
143-
// can also accept something that is guaranteed. Any non-consuming use of
144-
// an owned value should be able to take a guaranteed parameter as well
145-
// (modulo bugs). We assert to catch these.
146-
assert(map.canAcceptKind(ValueOwnershipKind::Guaranteed) &&
147-
"Any non-consuming use of an owned value should be able to take a "
148-
"guaranteed value");
115+
116+
// Otherwise, we have some form of consuming use that either forwards or
117+
// that we do not understand. See if we have a forwarding value that has a
118+
// single non-trivial operand that can accept a guaranteed value. If not, we
119+
// can not recursively process it, so be conservative and assume that we
120+
// /may consume/ the value, so the live range must not be eliminated.
121+
//
122+
// DISCUSSION: For now we do not support forwarding instructions with
123+
// multiple non-trivial arguments since we would need to optimize all of
124+
// the non-trivial arguments at the same time.
125+
//
126+
// NOTE: Today we do not support TermInsts for simplicity... we /could/
127+
// support it though if we need to.
128+
if (isa<TermInst>(user) || !isGuaranteedForwardingInst(user) ||
129+
1 != count_if(user->getOperandValues(
130+
true /*ignore type dependent operands*/),
131+
[&](SILValue v) {
132+
return v.getOwnershipKind() ==
133+
ValueOwnershipKind::Owned;
134+
})) {
135+
unknownConsumingUsers.push_back(user);
149136
continue;
150137
}
138+
139+
// Ok, this is a forwarding instruction whose ownership we can flip from
140+
// owned -> guaranteed. Visit its users recursively to see if the the
141+
// users force the live range to be alive.
142+
generalForwardingInsts.push_back(user);
143+
for (SILValue v : user->getResults()) {
144+
if (v.getOwnershipKind() != ValueOwnershipKind::Owned)
145+
continue;
146+
llvm::copy(v->getUses(), std::back_inserter(worklist));
147+
}
151148
}
152149
}
153150

‎lib/SILOptimizer/Utils/InstOptUtils.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -362,13 +362,10 @@ static void destroyConsumedOperandOfDeadInst(Operand &operand) {
362362
assert(!isEndOfScopeMarker(deadInst) && !isa<DestroyValueInst>(deadInst) &&
363363
!isa<DestroyAddrInst>(deadInst) &&
364364
"lifetime ending instruction is deleted without its operand");
365-
ValueOwnershipKind operandOwnershipKind = operandValue.getOwnershipKind();
366-
UseLifetimeConstraint lifetimeConstraint =
367-
operand.getOwnershipKindMap().getLifetimeConstraint(operandOwnershipKind);
368-
if (lifetimeConstraint == UseLifetimeConstraint::MustBeInvalidated) {
365+
if (operand.isConsumingUse()) {
369366
// Since deadInst cannot be an end-of-scope instruction (asserted above),
370367
// this must be a consuming use of an owned value.
371-
assert(operandOwnershipKind == ValueOwnershipKind::Owned);
368+
assert(operandValue.getOwnershipKind() == ValueOwnershipKind::Owned);
372369
SILBuilderWithScope builder(deadInst);
373370
builder.emitDestroyValueOperation(deadInst->getLoc(), operandValue);
374371
}

0 commit comments

Comments
 (0)