Skip to content

Commit 257a4bf

Browse files
committed
Add CFG post-dominator computation and continue construct validation
Add post-dominator computation to ControlFlowGraph by running the standard dominator algorithm on the reversed CFG with a pseudo-exit node. Post-dominators are used to validate that in each loop's continue construct, the back-edge block structurally post-dominates the continue target, ensuring the continue construct is well-structured. Changes: - cfg_analysis.rs: Add post_dominators field, compute_post_dominators() using reversed CFG with pseudo-exit, and post_dominates() query method - error.rs: Add ContinueConstructNotPostDominated error variant - rules/cfg.rs: Add ContinueConstructPostDominanceRule that validates back-edge blocks post-dominate their continue targets - tests: 3 unit tests for post-dominator computation (linear, diamond, multiple exits) and 3 integration tests (single-block continue, multi-block continue, negative case with split back edges)
1 parent 430f567 commit 257a4bf

File tree

4 files changed

+526
-0
lines changed

4 files changed

+526
-0
lines changed

rust/spirv-tools-core/src/validation/cfg_analysis.rs

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ pub struct ControlFlowGraph {
2424
pub successors: HashMap<Id, HashSet<Id>>,
2525
/// Dominator sets: for each block, the set of blocks that dominate it.
2626
pub dominators: HashMap<Id, HashSet<Id>>,
27+
/// Post-dominator sets: for each block, the set of blocks that post-dominate it.
28+
/// A block P post-dominates block B if every path from B to any exit goes through P.
29+
pub post_dominators: HashMap<Id, HashSet<Id>>,
2730
/// Blocks that are reachable from the entry block.
2831
pub reachable: HashSet<Id>,
2932
}
@@ -141,12 +144,16 @@ impl ControlFlowGraph {
141144
// Compute dominators
142145
let dominators = Self::compute_dominators(entry, &blocks, &predecessors);
143146

147+
// Compute post-dominators (dominators on the reversed CFG)
148+
let post_dominators = Self::compute_post_dominators(&blocks, &successors);
149+
144150
Some(Self {
145151
entry,
146152
blocks,
147153
predecessors,
148154
successors,
149155
dominators,
156+
post_dominators,
150157
reachable,
151158
})
152159
}
@@ -229,6 +236,76 @@ impl ControlFlowGraph {
229236
dominators
230237
}
231238

239+
/// Compute post-dominators using a pseudo-exit node on the reversed CFG.
240+
///
241+
/// A block P post-dominates block B if every path from B to any exit
242+
/// goes through P. Computed by running the dominator algorithm on the
243+
/// reversed graph with a pseudo-exit node as entry.
244+
fn compute_post_dominators(
245+
blocks: &HashSet<Id>,
246+
successors: &HashMap<Id, HashSet<Id>>,
247+
) -> HashMap<Id, HashSet<Id>> {
248+
// Find exit blocks (blocks with no successors within the function)
249+
let exit_blocks: Vec<Id> = blocks
250+
.iter()
251+
.filter(|b| {
252+
successors
253+
.get(b)
254+
.map_or(true, |s| s.is_empty() || s.iter().all(|t| !blocks.contains(t)))
255+
})
256+
.copied()
257+
.collect();
258+
259+
if exit_blocks.is_empty() {
260+
// All blocks are in cycles with no exit — can't compute post-dominators
261+
return HashMap::new();
262+
}
263+
264+
// Create pseudo-exit node (u32::MAX won't conflict with real SPIR-V IDs)
265+
let pseudo_exit = Id::try_from(u32::MAX).unwrap();
266+
267+
// Build augmented block set including pseudo-exit
268+
let mut aug_blocks = blocks.clone();
269+
aug_blocks.insert(pseudo_exit);
270+
271+
// Build predecessor map for the reversed graph.
272+
// In the reversed graph: predecessors_reversed[B] = successors_original[B]
273+
// Additionally, pseudo_exit is a predecessor of each exit block
274+
// (from the added exit→pseudo_exit edge in the augmented original graph).
275+
let mut reversed_preds: HashMap<Id, HashSet<Id>> = HashMap::new();
276+
277+
// pseudo_exit is the entry of the reversed graph — no predecessors
278+
reversed_preds.insert(pseudo_exit, HashSet::new());
279+
280+
for block in blocks {
281+
let mut preds = successors.get(block).cloned().unwrap_or_default();
282+
// Only keep edges to blocks within this function
283+
preds.retain(|p| blocks.contains(p));
284+
285+
// Exit blocks get pseudo_exit as a predecessor
286+
if exit_blocks.contains(block) {
287+
preds.insert(pseudo_exit);
288+
}
289+
290+
reversed_preds.insert(*block, preds);
291+
}
292+
293+
// Run standard dominator computation on the reversed graph
294+
let post_doms = Self::compute_dominators(pseudo_exit, &aug_blocks, &reversed_preds);
295+
296+
// Remove pseudo_exit from results and from dominator sets
297+
let mut result: HashMap<Id, HashSet<Id>> = HashMap::new();
298+
for (block, mut doms) in post_doms {
299+
if block == pseudo_exit {
300+
continue;
301+
}
302+
doms.remove(&pseudo_exit);
303+
result.insert(block, doms);
304+
}
305+
306+
result
307+
}
308+
232309
/// Check if block `dominator` dominates block `block`.
233310
pub fn dominates(&self, dominator: Id, block: Id) -> bool {
234311
if dominator == block {
@@ -240,6 +317,20 @@ impl ControlFlowGraph {
240317
.unwrap_or(false)
241318
}
242319

320+
/// Check if block `post_dominator` post-dominates block `block`.
321+
///
322+
/// A block P post-dominates B if every path from B to any function exit
323+
/// goes through P.
324+
pub fn post_dominates(&self, post_dominator: Id, block: Id) -> bool {
325+
if post_dominator == block {
326+
return true;
327+
}
328+
self.post_dominators
329+
.get(&block)
330+
.map(|pdoms| pdoms.contains(&post_dominator))
331+
.unwrap_or(false)
332+
}
333+
243334
/// Check if a block is reachable from entry.
244335
pub fn is_reachable(&self, block: Id) -> bool {
245336
self.reachable.contains(&block)
@@ -416,4 +507,105 @@ mod tests {
416507
// b2 does NOT dominate b3 (can reach b3 through b1)
417508
assert!(!dominators.get(&b3).unwrap().contains(&b2));
418509
}
510+
511+
#[test]
512+
fn test_post_dominator_computation_linear() {
513+
// Linear CFG: entry → B1 → B2 (exit)
514+
// B2 post-dominates B1 and entry
515+
// B1 post-dominates entry
516+
let entry = Id::try_from(1u32).unwrap();
517+
let b1 = Id::try_from(2u32).unwrap();
518+
let b2 = Id::try_from(3u32).unwrap();
519+
520+
let blocks: HashSet<Id> = [entry, b1, b2].into_iter().collect();
521+
let mut successors: HashMap<Id, HashSet<Id>> = HashMap::new();
522+
successors.insert(entry, [b1].into_iter().collect());
523+
successors.insert(b1, [b2].into_iter().collect());
524+
successors.insert(b2, HashSet::new()); // exit block
525+
526+
let post_doms = ControlFlowGraph::compute_post_dominators(&blocks, &successors);
527+
528+
// B2 post-dominates itself, B1, and entry
529+
assert!(post_doms.get(&entry).unwrap().contains(&b2));
530+
assert!(post_doms.get(&b1).unwrap().contains(&b2));
531+
assert!(post_doms.get(&b2).unwrap().contains(&b2));
532+
533+
// B1 post-dominates entry
534+
assert!(post_doms.get(&entry).unwrap().contains(&b1));
535+
536+
// entry does NOT post-dominate B1 or B2
537+
assert!(!post_doms.get(&b1).unwrap().contains(&entry));
538+
assert!(!post_doms.get(&b2).unwrap().contains(&entry));
539+
}
540+
541+
#[test]
542+
fn test_post_dominator_computation_diamond() {
543+
// Diamond CFG:
544+
// entry
545+
// / \
546+
// B1 B2
547+
// \ /
548+
// B3 (exit)
549+
//
550+
// B3 post-dominates everything
551+
// B1 does NOT post-dominate entry (can reach B3 through B2)
552+
let entry = Id::try_from(1u32).unwrap();
553+
let b1 = Id::try_from(2u32).unwrap();
554+
let b2 = Id::try_from(3u32).unwrap();
555+
let b3 = Id::try_from(4u32).unwrap();
556+
557+
let blocks: HashSet<Id> = [entry, b1, b2, b3].into_iter().collect();
558+
let mut successors: HashMap<Id, HashSet<Id>> = HashMap::new();
559+
successors.insert(entry, [b1, b2].into_iter().collect());
560+
successors.insert(b1, [b3].into_iter().collect());
561+
successors.insert(b2, [b3].into_iter().collect());
562+
successors.insert(b3, HashSet::new()); // exit block
563+
564+
let post_doms = ControlFlowGraph::compute_post_dominators(&blocks, &successors);
565+
566+
// B3 post-dominates all blocks
567+
assert!(post_doms.get(&entry).unwrap().contains(&b3));
568+
assert!(post_doms.get(&b1).unwrap().contains(&b3));
569+
assert!(post_doms.get(&b2).unwrap().contains(&b3));
570+
571+
// B1 does NOT post-dominate entry
572+
assert!(!post_doms.get(&entry).unwrap().contains(&b1));
573+
// B2 does NOT post-dominate entry
574+
assert!(!post_doms.get(&entry).unwrap().contains(&b2));
575+
}
576+
577+
#[test]
578+
fn test_post_dominator_computation_multiple_exits() {
579+
// CFG with two exits:
580+
// entry
581+
// / \
582+
// B1 B2 (exit)
583+
// |
584+
// B3 (exit)
585+
//
586+
// Neither B1 nor B2 post-dominates entry (two paths to exit)
587+
let entry = Id::try_from(1u32).unwrap();
588+
let b1 = Id::try_from(2u32).unwrap();
589+
let b2 = Id::try_from(3u32).unwrap();
590+
let b3 = Id::try_from(4u32).unwrap();
591+
592+
let blocks: HashSet<Id> = [entry, b1, b2, b3].into_iter().collect();
593+
let mut successors: HashMap<Id, HashSet<Id>> = HashMap::new();
594+
successors.insert(entry, [b1, b2].into_iter().collect());
595+
successors.insert(b1, [b3].into_iter().collect());
596+
successors.insert(b2, HashSet::new()); // exit
597+
successors.insert(b3, HashSet::new()); // exit
598+
599+
let post_doms = ControlFlowGraph::compute_post_dominators(&blocks, &successors);
600+
601+
// No block post-dominates entry except itself (two independent exit paths)
602+
let entry_pdoms = post_doms.get(&entry).unwrap();
603+
assert!(entry_pdoms.contains(&entry));
604+
assert!(!entry_pdoms.contains(&b1));
605+
assert!(!entry_pdoms.contains(&b2));
606+
assert!(!entry_pdoms.contains(&b3));
607+
608+
// B3 post-dominates B1
609+
assert!(post_doms.get(&b1).unwrap().contains(&b3));
610+
}
419611
}

rust/spirv-tools-core/src/validation/error.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2747,6 +2747,18 @@ pub enum ValidationError {
27472747
/// The continue target block.
27482748
continue_target: Id,
27492749
},
2750+
/// The back-edge block does not structurally post-dominate the continue target.
2751+
#[error("back-edge block {back_edge_block:?} does not post-dominate continue target {continue_target:?} of loop header {header:?} in function {function:?}")]
2752+
ContinueConstructNotPostDominated {
2753+
/// The function containing the loop.
2754+
function: Id,
2755+
/// The loop header block.
2756+
header: Id,
2757+
/// The continue target block.
2758+
continue_target: Id,
2759+
/// The back-edge block.
2760+
back_edge_block: Id,
2761+
},
27502762
/// Unroll and DontUnroll loop controls are both specified.
27512763
#[error("Unroll and DontUnroll loop controls must not both be specified")]
27522764
LoopControlUnrollAndDontUnroll {

rust/spirv-tools-core/src/validation/rules/cfg.rs

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1816,6 +1816,100 @@ impl ValidationRule for SwitchSelectorTypeRule {
18161816
}
18171817
}
18181818

1819+
// ============================================================================
1820+
// Continue Construct Post-Dominance Rule
1821+
// ============================================================================
1822+
1823+
/// Validates that in each loop's continue construct, the back-edge block
1824+
/// structurally post-dominates the continue target.
1825+
///
1826+
/// The back-edge block is the block within the continue construct that branches
1827+
/// back to the loop header. For the continue construct to be well-structured,
1828+
/// every path from the continue target must eventually reach the back-edge block.
1829+
pub struct ContinueConstructPostDominanceRule;
1830+
1831+
impl ValidationRule for ContinueConstructPostDominanceRule {
1832+
fn name(&self) -> &'static str {
1833+
"continue-construct-post-dominance"
1834+
}
1835+
1836+
fn validate(&self, ctx: &ValidationContext<'_>) -> ValidationResult {
1837+
let module = ctx.module();
1838+
1839+
for func in &module.functions {
1840+
let func_id = func
1841+
.def
1842+
.as_ref()
1843+
.and_then(|d| d.result_id)
1844+
.map(to_id)
1845+
.unwrap_or_else(|| to_id(0));
1846+
1847+
if func.blocks.is_empty() {
1848+
continue;
1849+
}
1850+
1851+
let Some(cfg) = ControlFlowGraph::build(func) else {
1852+
continue;
1853+
};
1854+
1855+
// Find all loop headers with their merge info
1856+
for block in &func.blocks {
1857+
let Some(header_id) = get_block_label(block) else {
1858+
continue;
1859+
};
1860+
1861+
for inst in &block.instructions {
1862+
if inst.class.opcode != Op::LoopMerge {
1863+
continue;
1864+
}
1865+
1866+
// Get continue target (second operand of LoopMerge)
1867+
let Some(Operand::IdRef(raw_continue)) = inst.operands.get(1) else {
1868+
continue;
1869+
};
1870+
let continue_target = to_id(*raw_continue);
1871+
1872+
// Skip if continue target is unreachable
1873+
if !cfg.is_reachable(continue_target) {
1874+
continue;
1875+
}
1876+
1877+
// Find the back-edge block: a predecessor of the loop header
1878+
// that is dominated by the continue target.
1879+
let Some(preds) = cfg.get_predecessors(header_id) else {
1880+
continue;
1881+
};
1882+
1883+
for back_edge_candidate in preds {
1884+
// The back-edge block must be dominated by the continue target
1885+
// (it's within the continue construct) and must not be the entry
1886+
// block branching to the header for the first time.
1887+
if !cfg.dominates(continue_target, *back_edge_candidate) {
1888+
continue;
1889+
}
1890+
1891+
// This is a back-edge block. Check that it post-dominates
1892+
// the continue target.
1893+
if !cfg.post_dominates(*back_edge_candidate, continue_target) {
1894+
return Err(
1895+
ValidationError::ContinueConstructNotPostDominated {
1896+
function: func_id,
1897+
header: header_id,
1898+
continue_target,
1899+
back_edge_block: *back_edge_candidate,
1900+
}
1901+
.into(),
1902+
);
1903+
}
1904+
}
1905+
}
1906+
}
1907+
}
1908+
1909+
Ok(())
1910+
}
1911+
}
1912+
18191913
/// Static rule instances
18201914
static BLOCK_STRUCTURE_RULE: BlockStructureRule = BlockStructureRule;
18211915
static MERGE_INSTRUCTION_RULE: MergeInstructionRule = MergeInstructionRule;
@@ -1832,6 +1926,8 @@ static MAXIMAL_RECONVERGENCE_PREDECESSORS_RULE: MaximalReconvergencePredecessors
18321926
static LIFETIME_RULE: LifetimeRule = LifetimeRule;
18331927
static SWITCH_CASE_UNIQUENESS_RULE: SwitchCaseUniquenessRule = SwitchCaseUniquenessRule;
18341928
static SWITCH_SELECTOR_TYPE_RULE: SwitchSelectorTypeRule = SwitchSelectorTypeRule;
1929+
static CONTINUE_CONSTRUCT_POST_DOMINANCE_RULE: ContinueConstructPostDominanceRule =
1930+
ContinueConstructPostDominanceRule;
18351931

18361932
/// Returns all CFG validation rules.
18371933
pub fn all_cfg_rules() -> Vec<&'static dyn ValidationRule> {
@@ -1850,5 +1946,6 @@ pub fn all_cfg_rules() -> Vec<&'static dyn ValidationRule> {
18501946
&LIFETIME_RULE,
18511947
&SWITCH_CASE_UNIQUENESS_RULE,
18521948
&SWITCH_SELECTOR_TYPE_RULE,
1949+
&CONTINUE_CONSTRUCT_POST_DOMINANCE_RULE,
18531950
]
18541951
}

0 commit comments

Comments
 (0)