[DCE] Keep outer borrows alive when reborrowing. #39939
Conversation
The new utility looks through ownership forwarding instructions to find the original values with guaranteed ownership.
|
@swift-ci please test |
|
Build failed |
Previously, when encountering a borrow of a guaranteed value, the end_borrows of that reborrow were marked alive. Only doing that enables end_borrows of the outer borrow scope can be marked as dead. The result is that uses of the reborrowed value (including its end_borrow) can outstrip the outer borrow scope, which is illegal. Here, the outer borrow scope's end_borrows are marked alive. To do that, the originally borrowed values have to be identified via findGuaranteedReferenceRoots.
nate-chandler
force-pushed
the
dce/keep-outer-borrow-scope-alive-when-reborrowing
branch
from
eebe1d7 to
0a18c62
Compare
|
@swift-ci please test |
| @@ -204,6 +204,11 @@ SILValue findReferenceRoot(SILValue ref); | |||
| /// Find the first owned root of the reference. | |||
| SILValue findOwnershipReferenceRoot(SILValue ref); | |||
|
|
|||
| /// Look through all ownership forwarding instructions to find the values which | |||
| /// were originally borrowed. | |||
| void findGuaranteedReferenceRoots(SILValue value, | |||
There was a problem hiding this comment.
This already exists, why are you inventing a new form of this? I don't know why we have the one on line 205 as well.
There was a problem hiding this comment.
They do different things.
There was a problem hiding this comment.
@nate-chandler you want
bool getAllBorrowIntroducingValues(SILValue value,
SmallVectorImpl &out);
that's in OwnershipUtils.h
There was a problem hiding this comment.
I didn't know of that function. It looks like the behavior is slightly different, but I'll see if it does what is needed here.
There was a problem hiding this comment.
Nate and I spoke about this. I have some stuff reliant on this. He is going to in a subsequent commit look into eliminating this and using the already written API.
There was a problem hiding this comment.
To clarify, I am going to determine whether these should be combined or not.
nate-chandler
deleted the
dce/keep-outer-borrow-scope-alive-when-reborrowing
branch
|
@nate-chandler this looks good. As we discussed, the logic for "find borrow introducers" should be centralized in SIL/Utils/OwnershipUtils because it models a structural requirement of SIL independent of optimization. It needs to be complete and verified. So we also need to call it from the SILVerifier. |
Previously, when encountering a borrow of a guaranteed value, the end_borrows of that reborrow were marked alive. Only doing that enables end_borrows of the outer borrow scope can be marked as dead. The result is that uses of the reborrowed value (including its end_borrow) can outstrip the outer borrow scope, which is illegal.
Here, the outer borrow scope's end_borrows are marked alive. To do that, the originally borrowed values have to be identified via findGuaranteedReferenceRoots.