Analyse storage liveness and preserve it during generator transformation #44480
Conversation
|
|
||
| for i in 0..(self.mir_local_count) { | ||
| let l = Local::new(i); | ||
| if liveness.contains(&l) && !self.remap.contains_key(&l) { |
There was a problem hiding this comment.
A function return automatically does a StorageDead of all locals, so there is no need to insert anything here.
|
|
||
| let mut ignored = StorageIgnored(IdxSetBuf::new_filled(mir.basic_blocks().len())); | ||
| ignored.visit_mir(mir); | ||
|
|
||
| let mut set = liveness::LocalSet::new_empty(mir.local_decls.len()); | ||
| let result = liveness::liveness_of_locals(mir); |
There was a problem hiding this comment.
nit: I would rename this to liveness
| let mut storage_liveness = state_for_location(loc, &analysis, &storage_live); | ||
|
|
||
| storage_liveness_map.insert(block, storage_liveness.clone()); | ||
|
|
There was a problem hiding this comment.
... and also have a let live_locals = storage_liveness; here. It's free to use better names.
| }).collect(); | ||
|
|
||
| cases.insert(0, (0, drop_clean)); | ||
|
|
||
| // The poisoned state 1 falls through to the default case which is just to return | ||
| // The returned state 1 and the poisoned state 2 falls through to |
There was a problem hiding this comment.
nit: double space the poisoned state
| }).collect(); | ||
|
|
||
| cases.insert(0, (0, drop_clean)); | ||
|
|
||
| // The poisoned state 1 falls through to the default case which is just to return | ||
| // The returned state 1 and the poisoned state 2 falls through to |
There was a problem hiding this comment.
nit: double space the poisoned state
|
Nice! r=me modulo nits. |
| state: u32, | ||
| resume: BasicBlock, | ||
| drop: Option<BasicBlock>, | ||
| storage_liveness: liveness::LocalSet, |
There was a problem hiding this comment.
I find this rather ugly. Can you have a
fn make_storage_live(..., point: &SuspensionPoint, terminates_to: BasicBlock) {
}And call it from both collects to avoid the ugly mutation? I think you can.
| @@ -781,6 +814,30 @@ impl MirPass for StateTransform { | |||
|
|
|||
| dump_mir(tcx, None, "generator_post-transform", &0, source, mir); | |||
|
|
|||
| // Create StorageLive instruction blocks for suspension points | |||
| for point in &mut transform.suspension_points { | |||
There was a problem hiding this comment.
(this is the part that could be extracted to a function)
carols10cents
added
the
S-waiting-on-author
|
@bors r+ |
|
📌 Commit 23d3d9f has been approved by |
|
I think it's possible to merge the 3 special case states into SuspensionPoint and clean up quite a bit of code, but I can leave that to a further PR. |
|
☔ The latest upstream changes (presumably #44275) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@bors r+ |
|
📌 Commit b6bbd4f has been approved by |
|
⌛ Testing commit b6bbd4f0929d8308c340fe0c71dbf1dfbc9ff226 with merge 81fa6e7db64256e6d7049627669ff64e99176959... |
|
💔 Test failed - status-travis |
|
I think you need to build the error index yourself to find out what is happening at line 11731 😓 |
|
@bors r+ |
|
📌 Commit 0e8e659 has been approved by |
Analyse storage liveness and preserve it during generator transformation This uses a dataflow analysis on `StorageLive` and `StorageDead` statements to infer where the storage of locals are live. The result of this analysis is intersected with the regular liveness analysis such that a local is can only be live when its storage is. This fixes #44184. If the storage of a local is live across a suspension point, we'll insert a `StorageLive` statement for it after the suspension point so storage liveness is preserved. This fixes #44179. r? @arielb1
|
☀️ Test successful - status-appveyor, status-travis |
This uses a dataflow analysis on
StorageLiveandStorageDeadstatements to infer where the storage of locals are live. The result of this analysis is intersected with the regular liveness analysis such that a local is can only be live when its storage is. This fixes #44184. If the storage of a local is live across a suspension point, we'll insert aStorageLivestatement for it after the suspension point so storage liveness is preserved. This fixes #44179.r? @arielb1