Skip to content

Commit 56f6b6a

Browse files
Introduce th IRAfterPass enum. Replace the usage of bool to indicate IR modification with this enum.
This is done in order to make IR modification more explicit and introduce it in the type system. Signed-off-by: Tomas Fabrizio Orsi <[email protected]>
1 parent ec456fc commit 56f6b6a

File tree

10 files changed

+89
-50
lines changed

10 files changed

+89
-50
lines changed

dialects/hir/src/transforms/spill.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use alloc::rc::Rc;
33
use midenc_hir::{
44
adt::SmallDenseMap,
55
dialects::builtin::{Function, FunctionRef, LocalVariable},
6-
pass::{Pass, PassExecutionState},
6+
pass::{IRAfterPass, Pass, PassExecutionState},
77
BlockRef, BuilderExt, EntityMut, Op, OpBuilder, OperationName, OperationRef, Report, Rewriter,
88
SourceSpan, Spanned, Symbol, ValueRef,
99
};
@@ -31,22 +31,22 @@ impl Pass for TransformSpills {
3131
&mut self,
3232
op: EntityMut<'_, Self::Target>,
3333
state: &mut PassExecutionState,
34-
) -> Result<bool, Report> {
34+
) -> Result<IRAfterPass, Report> {
3535
let function = op.into_entity_ref();
3636
log::debug!(target: "insert-spills", "computing and inserting spills for {}", function.as_operation());
3737

3838
if function.is_declaration() {
3939
log::debug!(target: "insert-spills", "function has no body, no spills needed!");
4040
state.preserved_analyses_mut().preserve_all();
41-
return Ok(false);
41+
return Ok(IRAfterPass::Unchanged);
4242
}
4343
let mut analysis =
4444
state.analysis_manager().get_analysis_for::<SpillAnalysis, Function>()?;
4545

4646
if !analysis.has_spills() {
4747
log::debug!(target: "insert-spills", "no spills needed!");
4848
state.preserved_analyses_mut().preserve_all();
49-
return Ok(false);
49+
return Ok(IRAfterPass::Unchanged);
5050
}
5151

5252
// Take ownership of the spills analysis so that we can mutate the analysis state during

dialects/scf/src/transforms/cfg_to_scf.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use midenc_hir::{
77
diagnostics::Severity,
88
dialects::builtin,
99
dominance::DominanceInfo,
10-
pass::{Pass, PassExecutionState},
10+
pass::{IRAfterPass, Pass, PassExecutionState},
1111
Builder, EntityMut, Forward, Op, Operation, OperationName, OperationRef, RawWalk, Report,
1212
SmallVec, Spanned, Type, ValueRange, ValueRef, WalkResult,
1313
};
@@ -51,7 +51,7 @@ impl Pass for LiftControlFlowToSCF {
5151
&mut self,
5252
op: EntityMut<'_, Self::Target>,
5353
state: &mut PassExecutionState,
54-
) -> Result<bool, Report> {
54+
) -> Result<IRAfterPass, Report> {
5555
let mut transformation = ControlFlowToSCFTransformation;
5656
let mut changed = false;
5757

@@ -131,7 +131,7 @@ impl Pass for LiftControlFlowToSCF {
131131

132132
if result.was_interrupted() {
133133
// TODO: Maybe Change into_result implementation
134-
return result.into_result().map(|_| false);
134+
return result.into_result().map(|_| IRAfterPass::Unchanged);
135135
}
136136

137137
log::debug!(
@@ -142,7 +142,7 @@ impl Pass for LiftControlFlowToSCF {
142142
state.preserved_analyses_mut().preserve_all();
143143
}
144144

145-
Ok(changed)
145+
Ok(changed.into())
146146
}
147147
}
148148

hir-transform/src/canonicalization.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use alloc::{boxed::Box, format, rc::Rc};
22

33
use midenc_hir::{
4-
pass::{OperationPass, Pass, PassExecutionState},
4+
pass::{IRAfterPass, OperationPass, Pass, PassExecutionState},
55
patterns::{self, FrozenRewritePatternSet, GreedyRewriteConfig, RewritePatternSet},
66
Context, EntityMut, Operation, OperationName, Report, Spanned,
77
};
@@ -93,10 +93,10 @@ impl Pass for Canonicalizer {
9393
&mut self,
9494
op: EntityMut<'_, Self::Target>,
9595
state: &mut PassExecutionState,
96-
) -> Result<bool, Report> {
96+
) -> Result<IRAfterPass, Report> {
9797
let Some(rewrites) = self.rewrites.as_ref() else {
9898
log::debug!("skipping canonicalization as there are no rewrite patterns to apply");
99-
return Ok(false);
99+
return Ok(IRAfterPass::Unchanged);
100100
};
101101
let op = {
102102
let ptr = op.as_operation_ref();
@@ -143,6 +143,6 @@ impl Pass for Canonicalizer {
143143
}
144144
};
145145

146-
Ok(changed)
146+
Ok(changed.into())
147147
}
148148
}

hir-transform/src/sccp.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use midenc_hir::{
2-
pass::{Pass, PassExecutionState},
2+
pass::{IRAfterPass, Pass, PassExecutionState},
33
patterns::NoopRewriterListener,
44
BlockRef, Builder, EntityMut, OpBuilder, Operation, OperationFolder, OperationName, RegionList,
55
Report, SmallVec, ValueRef,
@@ -38,7 +38,7 @@ impl Pass for SparseConditionalConstantPropagation {
3838
&mut self,
3939
mut op: EntityMut<'_, Self::Target>,
4040
state: &mut PassExecutionState,
41-
) -> Result<bool, Report> {
41+
) -> Result<IRAfterPass, Report> {
4242
// Run sparse constant propagation + dead code analysis
4343
let mut solver = DataFlowSolver::default();
4444
solver.load::<DeadCodeAnalysis>();
@@ -58,7 +58,7 @@ impl SparseConditionalConstantPropagation {
5858
op: &mut Operation,
5959
_state: &mut PassExecutionState,
6060
solver: &DataFlowSolver,
61-
) -> Result<bool, Report> {
61+
) -> Result<IRAfterPass, Report> {
6262
let mut worklist = SmallVec::<[BlockRef; 8]>::default();
6363

6464
let add_to_worklist = |regions: &RegionList, worklist: &mut SmallVec<[BlockRef; 8]>| {
@@ -124,7 +124,7 @@ impl SparseConditionalConstantPropagation {
124124
}
125125
}
126126

127-
Ok(replaced_any)
127+
Ok(replaced_any.into())
128128
}
129129
}
130130

hir-transform/src/sink.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use midenc_hir::{
44
adt::SmallDenseMap,
55
dominance::DominanceInfo,
66
matchers::{self, Matcher},
7-
pass::{Pass, PassExecutionState},
7+
pass::{IRAfterPass, Pass, PassExecutionState},
88
traits::{ConstantLike, Terminator},
99
Backward, Builder, EntityMut, Forward, FxHashSet, OpBuilder, Operation, OperationName,
1010
OperationRef, ProgramPoint, RawWalk, Region, RegionBranchOpInterface,
@@ -96,7 +96,7 @@ impl Pass for ControlFlowSink {
9696
&mut self,
9797
op: EntityMut<'_, Self::Target>,
9898
state: &mut PassExecutionState,
99-
) -> Result<bool, Report> {
99+
) -> Result<IRAfterPass, Report> {
100100
let op = op.into_entity_ref();
101101
log::debug!(target: "control-flow-sink", "sinking operations in {op}");
102102

@@ -105,7 +105,7 @@ impl Pass for ControlFlowSink {
105105

106106
let dominfo = state.analysis_manager().get_analysis::<DominanceInfo>()?;
107107

108-
let mut sunk = false;
108+
let mut sunk = IRAfterPass::Unchanged;
109109
operation.raw_prewalk_all::<Forward, _>(|op: OperationRef| {
110110
let regions_to_sink = {
111111
let op = op.borrow();
@@ -173,7 +173,7 @@ impl Pass for SinkOperandDefs {
173173
&mut self,
174174
op: EntityMut<'_, Self::Target>,
175175
_state: &mut PassExecutionState,
176-
) -> Result<bool, Report> {
176+
) -> Result<IRAfterPass, Report> {
177177
let operation = op.as_operation_ref();
178178
drop(op);
179179

@@ -185,7 +185,7 @@ impl Pass for SinkOperandDefs {
185185
// then process the worklist, moving everything into position.
186186
let mut worklist = alloc::collections::VecDeque::default();
187187

188-
let mut changed = false;
188+
let mut changed = IRAfterPass::Unchanged;
189189
// Visit ops in "true" post-order (i.e. block bodies are visited bottom-up).
190190
operation.raw_postwalk_all::<Backward, _>(|operation: OperationRef| {
191191
// Determine if any of this operation's operands represent one of the following:
@@ -310,7 +310,7 @@ impl Pass for SinkOperandDefs {
310310
log::trace!(target: "sink-operand-defs", " rewriting operand {operand_value} as {replacement}");
311311
operand.borrow_mut().set(replacement);
312312

313-
changed = true;
313+
changed = IRAfterPass::Changed;
314314
// If no other uses of this value remain, then remove the original
315315
// operation, as it is now dead.
316316
if !operand_value.borrow().is_used() {
@@ -357,7 +357,7 @@ impl Pass for SinkOperandDefs {
357357
log::trace!(target: "sink-operand-defs", " rewriting operand {operand_value} as {replacement}");
358358
sink_state.replacements.insert(operand_value, replacement);
359359
operand.borrow_mut().set(replacement);
360-
changed = true;
360+
changed = IRAfterPass::Changed;
361361
} else {
362362
log::trace!(target: "sink-operand-defs", " defining op is a constant with no other uses, moving into place");
363363
// The original op can be moved
@@ -552,14 +552,14 @@ pub fn control_flow_sink<P, F>(
552552
dominfo: &DominanceInfo,
553553
should_move_into_region: P,
554554
move_into_region: F,
555-
) -> bool
555+
) -> IRAfterPass
556556
where
557557
P: Fn(&Operation, &Region) -> bool,
558558
F: Fn(OperationRef, RegionRef),
559559
{
560560
let sinker = Sinker::new(dominfo, should_move_into_region, move_into_region);
561561
let sunk_regions = sinker.sink_regions(regions);
562-
sunk_regions > 0
562+
(sunk_regions > 0).into()
563563
}
564564

565565
/// Populates `regions` with regions of the provided region branch op that are executed at most once

hir-transform/src/spill.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use midenc_hir::{
44
adt::{SmallDenseMap, SmallSet},
55
cfg::Graph,
66
dominance::{DomTreeNode, DominanceFrontier, DominanceInfo},
7-
pass::AnalysisManager,
7+
pass::{AnalysisManager, IRAfterPass},
88
traits::SingleRegion,
99
BlockRef, Builder, Context, FxHashMap, OpBuilder, OpOperand, Operation, OperationRef,
1010
ProgramPoint, Region, RegionBranchOpInterface, RegionBranchPoint, RegionRef, Report, Rewriter,
@@ -113,7 +113,7 @@ pub fn transform_spills(
113113
analysis: &mut SpillAnalysis,
114114
interface: &mut dyn TransformSpillsInterface,
115115
analysis_manager: AnalysisManager,
116-
) -> Result<bool, Report> {
116+
) -> Result<IRAfterPass, Report> {
117117
assert!(
118118
op.borrow().implements::<dyn SingleRegion>(),
119119
"the spills transformation is not supported when the root op is multi-region"
@@ -253,7 +253,7 @@ pub fn transform_spills(
253253
}
254254

255255
// QUESTION: If the code has reached this point, then, in theory, it must've gotten changed. Right? Requires double check.
256-
Ok(true)
256+
Ok(IRAfterPass::Changed)
257257
}
258258

259259
fn rewrite_single_block_spills(

hir/src/pass.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ pub use self::{
1111
analysis::{Analysis, AnalysisManager, OperationAnalysis, PreservedAnalyses},
1212
instrumentation::{PassInstrumentation, PassInstrumentor, PipelineParentInfo},
1313
manager::{IRPrintingConfig, Nesting, OpPassManager, PassDisplayMode, PassManager},
14-
pass::{OperationPass, Pass, PassExecutionState},
14+
pass::{IRAfterPass, OperationPass, Pass, PassExecutionState},
1515
registry::{PassInfo, PassPipelineInfo},
1616
specialization::PassTarget,
1717
statistics::{PassStatistic, Statistic, StatisticValue},
@@ -168,26 +168,31 @@ impl Pass for Print {
168168
&mut self,
169169
op: crate::EntityMut<'_, Self::Target>,
170170
_state: &mut PassExecutionState,
171-
) -> Result<bool, crate::Report> {
171+
) -> Result<IRAfterPass, crate::Report> {
172172
let op = op.into_entity_ref();
173173
self.print_ir(op);
174-
Ok(false)
174+
Ok(IRAfterPass::Unchanged)
175175
}
176176
}
177177

178178
impl PassInstrumentation for Print {
179-
fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef, changed: bool) {
179+
fn run_after_pass(
180+
&mut self,
181+
pass: &dyn OperationPass,
182+
op: &OperationRef,
183+
changed: IRAfterPass,
184+
) {
180185
std::println!(
181186
"
182187
------------------------------------DESPUES:\
183188
{}-------------------------------------------------------
184189
",
185190
pass.name()
186191
);
187-
std::println!("RESULTADO DE CHANGED: {}", changed);
192+
// std::println!("RESULTADO DE CHANGED: {}", changed);
188193
#[allow(clippy::needless_bool)]
189194
// Always print, unless "only_when_modified" has been set and there have not been changes.
190-
let print_when_changed = if self.only_when_modified && !changed {
195+
let print_when_changed = if self.only_when_modified && changed == IRAfterPass::Unchanged {
191196
false
192197
} else {
193198
true

hir/src/pass/instrumentation.rs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use compact_str::CompactString;
55
use smallvec::SmallVec;
66

77
use super::OperationPass;
8-
use crate::{OperationName, OperationRef};
8+
use crate::{pass::IRAfterPass, OperationName, OperationRef};
99

1010
#[allow(unused_variables)]
1111
pub trait PassInstrumentation {
@@ -22,7 +22,13 @@ pub trait PassInstrumentation {
2222
) {
2323
}
2424
fn run_before_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) {}
25-
fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef, changed: bool) {}
25+
fn run_after_pass(
26+
&mut self,
27+
pass: &dyn OperationPass,
28+
op: &OperationRef,
29+
changed: IRAfterPass,
30+
) {
31+
}
2632
fn run_after_pass_failed(&mut self, pass: &dyn OperationPass, op: &OperationRef) {}
2733
fn run_before_analysis(&mut self, name: &str, id: &TypeId, op: &OperationRef) {}
2834
fn run_after_analysis(&mut self, name: &str, id: &TypeId, op: &OperationRef) {}
@@ -54,7 +60,12 @@ impl<P: ?Sized + PassInstrumentation> PassInstrumentation for Box<P> {
5460
(**self).run_before_pass(pass, op);
5561
}
5662

57-
fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef, changed: bool) {
63+
fn run_after_pass(
64+
&mut self,
65+
pass: &dyn OperationPass,
66+
op: &OperationRef,
67+
changed: IRAfterPass,
68+
) {
5869
(**self).run_after_pass(pass, op, changed);
5970
}
6071

@@ -97,7 +108,12 @@ impl PassInstrumentor {
97108
self.instrument(|pi| pi.run_before_pass(pass, op));
98109
}
99110

100-
pub fn run_after_pass(&self, pass: &dyn OperationPass, op: &OperationRef, changed: bool) {
111+
pub fn run_after_pass(
112+
&self,
113+
pass: &dyn OperationPass,
114+
op: &OperationRef,
115+
changed: IRAfterPass,
116+
) {
101117
self.instrument(|pi| pi.run_after_pass(pass, op, changed));
102118
}
103119

0 commit comments

Comments
 (0)