bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #106612 - JakobDegen:cleanup-wf, r=tmiasko

Browse source 
Document wf constraints on control flow in cleanup blocks

Was recently made aware of [this code](a377893da2/compiler/rustc_codegen_ssa/src/mir/analyze.rs (L247-L368)), which has this potential ICE: a377893da2/compiler/rustc_codegen_ssa/src/mir/analyze.rs (L308-L314)

Roughly speaking, the code there is attempting to partition the cleanup blocks into funclets that satisfy a "unique successor" property, and the ICE is set off if that's not possible. This PR documents the well-formedness constraints that MIR must satisfy to avoid setting off that ICE.

The constraints documented are slightly stronger than the cases in which the ICE would have been set off in that code. This is necessary though, since whether or not that ICE gets set off can depend on iteration order in some graphs.

This sort of constraint is kind of ugly, but I don't know a better alternative at the moment. It's worth knowing that two important optimizations are still correct:
 - Removing edges in the cfg: Fewer edges => fewer paths => stronger dominance relations => more contractions, and more contractions can't turn a forest into not-a-forest.
 - Contracting an edge u -> v when u only has one successor and v only has one predecessor: u already dominated v, so this contraction was going to happen anyway.

There is definitely a MIR opt somewhere that can run afoul of this, but I don't know where it is. `@saethlin` was able to set it off though, so maybe he'll be able to shed some light on it.

r? `@RalfJung` I suppose, and cc `@tmiasko` who might have insight/opinions on this
This commit is contained in:
bors 2023-01-17 11:34:35 +00:00
commit f34cc658eb
3 changed files with 110 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

10
compiler/rustc_middle/src/mir/syntax.rs
View file

@ -512,6 +512,16 @@ pub struct CopyNonOverlapping<'tcx> {
/// must also be `cleanup`. This is a part of the type system and checked statically, so it is
/// still an error to have such an edge in the CFG even if it's known that it won't be taken at
/// runtime.
/// 4. The control flow between cleanup blocks must look like an upside down tree. Roughly
/// speaking, this means that control flow that looks like a V is allowed, while control flow
/// that looks like a W is not. This is necessary to ensure that landing pad information can be
/// correctly codegened on MSVC. More precisely:
///
/// Begin with the standard control flow graph `G`. Modify `G` as follows: for any two cleanup
/// vertices `u` and `v` such that `u` dominates `v`, contract `u` and `v` into a single vertex,
/// deleting self edges and duplicate edges in the process. Now remove all vertices from `G`
/// that are not cleanup vertices or are not reachable. The resulting graph must be an inverted
/// tree, that is each vertex may have at most one successor and there may be no cycles.
#[derive(Clone, TyEncodable, TyDecodable, Hash, HashStable, PartialEq, TypeFoldable, TypeVisitable)]
pub enum TerminatorKind<'tcx> {
/// Block has one successor; we continue execution there.