Fix copy-to-borrow optimization's end_borrow insertion #79810
Conversation
|
@swift-ci test |
| // FIXME: Use an InstructionSetVector | ||
| var allLifetimeEndingInstructions : InstructionWorklist { | ||
| var worklist = InstructionWorklist(context) | ||
| worklist.pushIfNotVisited(contentsOf: Array(destroys.lazy.map { $0 }) + Array(nonDestroyingLiverangeExits)) |
There was a problem hiding this comment.
You don't need to "convert" the lazy map sequence to an array, because pushIfNotVisited accepts any kind of sequence.
Instead you can call pushIfNotVisited twice:
worklist.pushIfNotVisited(contentsOf: destroys.lazy.map { $0 })
worklist.pushIfNotVisited(contentsOf: nonDestroyingLiverangeExits)
| @@ -124,8 +124,11 @@ private struct Uses { | |||
| // E.g. the none-case of a switch_enum of an Optional. | |||
| private(set) var nonDestroyingLiverangeExits: Stack<Instruction> | |||
|
|
|||
| var allLifetimeEndingInstructions: [Instruction] { | |||
| Array(destroys.lazy.map { $0 }) + Array(nonDestroyingLiverangeExits) | |||
| // FIXME: Use an InstructionSetVector | |||
There was a problem hiding this comment.
An InstructionWorklist is basically an instruction set-vector. We will probably not add an InstructionSetVector in swift. So this comment doesn't make sense.
There was a problem hiding this comment.
It worked out here with inlining the worklist construction at the use-site. InstructionSetVector may still be valuable in some cases. To use a InstructionWorklist in this PR I had to originally copy it to a var and then iterate by popping the elements.
| @@ -323,7 +326,12 @@ private func createEndBorrows(for beginBorrow: Value, atEndOf liverange: Instruc | |||
| // destroy_value %2 | |||
| // destroy_value %3 // The final destroy. Here we need to create the `end_borrow`(s) | |||
| // | |||
| for endInst in collectedUses.allLifetimeEndingInstructions { | |||
| var worklist = collectedUses.allLifetimeEndingInstructions | |||
There was a problem hiding this comment.
Given that the return value of allLifetimeEndingInstructions is not a simple Array anymore, but a worklist which has to be deinitialized, it's probably simpler to remove the var allLifetimeEndingInstructions and inline the worklist construction directly here.
meg-gupta
force-pushed
the
fixctb
branch
2 times, most recently
from
4b49e19 to
c130be1
Compare
|
@swift-ci test |
|
@swift-ci test source compatibility |
allLifetimeEndingInstructions collects insertion points for end_borrows, and can end up having duplicate entries in the case of enums with non payloaded cases. Unique the entries so we don't end up with multiple end_borrows Fixes rdar://146212574
|
@swift-ci test |
|
@swift-ci test source compatibility |
|
@swift-ci please test Linux |
allLifetimeEndingInstructions collects insertion points for end_borrows, and can end up having duplicate entries in the case of enums with non payloaded cases.
Unique the entries so we don't end up with multiple end_borrows
Fixes rdar://146212574