From a0b54f19bd0bb4e1437073e4f648ee063418e8b1 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Tue, 15 Apr 2025 17:27:46 -0300 Subject: [PATCH 01/56] WIP: First draft of adding IR printing functionality of the PassManager So far, a `Print` pass has been added every time the OpPassManager::nest() is called. The `Print` pass presented a good starting point for this issue, since it had implemented the basic building blocks for IR printing. What is currently missing is: - Configuring the IRPrintingConfig struct. At the moment, the IRPrintingConfig This, in turn, means that regardless of if there have been modifications to the IR or not the IR is *always* printed. The issue states that this should be configurable. I assume that these configurations should come from the Session/Context struct (although I don't think there's a 1 to 1 correspondance). - Realted to this, making printing granular is also missing. At the moment, the Print Pass is applied irregardless of the type. The issue also states that it should be able to *only print IR when a Pass modified the IR*. - One interesting note is that the PassManager has some functionality related to IR printing, although after some initial attempts I found it simpler to add that logic in the OpPassManager itself. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/ir/print.rs | 1 + hir/src/pass.rs | 8 ++++---- hir/src/pass/manager.rs | 22 +++++++++++++++++----- midenc-compile/src/stages/rewrite.rs | 1 + midenc-session/src/lib.rs | 3 +++ 5 files changed, 26 insertions(+), 9 deletions(-) diff --git a/hir/src/ir/print.rs b/hir/src/ir/print.rs index 86381db07..ca3bfb3ea 100644 --- a/hir/src/ir/print.rs +++ b/hir/src/ir/print.rs @@ -9,6 +9,7 @@ use crate::{ AttributeValue, EntityWithId, SuccessorOperands, Value, }; +#[derive(Debug)] pub struct OpPrintingFlags { pub print_entry_block_headers: bool, pub print_intrinsic_attributes: bool, diff --git a/hir/src/pass.rs b/hir/src/pass.rs index d87999128..460a3c021 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -97,7 +97,7 @@ impl Pass for Print { match self.filter { OpFilter::All => { let target = self.target.as_deref().unwrap_or("printer"); - log::trace!(target: target, "{op}"); + log::error!(target: target, "{op}"); } OpFilter::Type { dialect, @@ -106,21 +106,21 @@ impl Pass for Print { let name = op.name(); if name.dialect() == dialect && name.name() == op_name { let target = self.target.as_deref().unwrap_or("printer"); - log::trace!(target: target, "{op}"); + log::error!(target: target, "{op}"); } } OpFilter::Symbol(None) => { if let Some(sym) = op.as_symbol() { let name = sym.name().as_str(); let target = self.target.as_deref().unwrap_or(name); - log::trace!(target: target, "{}", sym.as_symbol_operation()); + log::error!(target: target, "{}", sym.as_symbol_operation()); } } OpFilter::Symbol(Some(filter)) => { if let Some(sym) = op.as_symbol().filter(|sym| sym.name().as_str().contains(filter)) { let target = self.target.as_deref().unwrap_or(filter); - log::trace!(target: target, "{}", sym.as_symbol_operation()); + log::error!(target: target, "{}", sym.as_symbol_operation()); } } } diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index a9e4cc23d..d8c2d46e1 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -9,8 +9,8 @@ use super::{ PassInstrumentor, PipelineParentInfo, Statistic, }; use crate::{ - traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, Operation, - OperationName, OperationRef, Report, + pass::Print, traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, + Operation, OperationName, OperationRef, Report, }; #[derive(Debug, Default, Copy, Clone, PartialEq, Eq)] @@ -29,6 +29,7 @@ pub enum PassDisplayMode { // TODO(pauls) #[allow(unused)] +#[derive(Default, Debug)] pub struct IRPrintingConfig { print_module_scope: bool, print_after_only_on_change: bool, @@ -40,7 +41,7 @@ pub struct IRPrintingConfig { pub struct PassManager { context: Rc, /// The underlying pass manager - pm: OpPassManager, + pub pm: OpPassManager, /// A manager for pass instrumentation instrumentor: Rc, /// An optional crash reproducer generator, if this pass manager is setup to @@ -169,8 +170,11 @@ impl PassManager { self } - pub fn enable_ir_printing(&mut self, _config: IRPrintingConfig) { - todo!() + // pub fn enable_ir_printing(&mut self, _config: IRPrintingConfig) { + pub fn enable_ir_printing(&mut self) -> &mut Self { + self.pm.printer = Some(IRPrintingConfig::default()); + // self.add_pass(Box::new(Print::any())); + self } pub fn enable_timing(&mut self, yes: bool) -> &mut Self { @@ -232,6 +236,9 @@ pub struct OpPassManager { passes: SmallVec<[Box; 8]>, /// Control the implicit nesting of passes that mismatch the name set for this manager nesting: Nesting, + + /// Indicates wheter or not to print IR after passes + pub printer: Option, } impl OpPassManager { @@ -246,6 +253,7 @@ impl OpPassManager { name: None, passes: Default::default(), nesting, + printer: None, } } @@ -275,6 +283,7 @@ impl OpPassManager { name: Some(name), passes: Default::default(), nesting, + printer: None, } } @@ -298,6 +307,7 @@ impl OpPassManager { name: Some(name), passes: Default::default(), nesting, + printer: None, } } @@ -307,6 +317,7 @@ impl OpPassManager { name: Some(name), passes: Default::default(), nesting, + printer: None, } } @@ -519,6 +530,7 @@ impl OpPassManager { pub fn nest_pass_manager(&mut self, nested: Self) -> NestedOpPassManager<'_> { let adaptor = Box::new(OpToOpPassAdaptor::new(nested)); + self.add_pass(Box::new(Print::any())); NestedOpPassManager { parent: self, nested: Some(adaptor), diff --git a/midenc-compile/src/stages/rewrite.rs b/midenc-compile/src/stages/rewrite.rs index 5fa1448c9..6a5d30cef 100644 --- a/midenc-compile/src/stages/rewrite.rs +++ b/midenc-compile/src/stages/rewrite.rs @@ -49,6 +49,7 @@ impl Stage for ApplyRewritesStage { // Construct a pass manager with the default pass pipeline let mut pm = PassManager::on::(context.clone(), Nesting::Implicit); + pm.enable_ir_printing(); let mut rewrite_config = GreedyRewriteConfig::default(); rewrite_config.with_region_simplification_level(RegionSimplificationLevel::Normal); diff --git a/midenc-session/src/lib.rs b/midenc-session/src/lib.rs index 07fbc933b..5d0c93683 100644 --- a/midenc-session/src/lib.rs +++ b/midenc-session/src/lib.rs @@ -128,6 +128,9 @@ impl Session { where I: IntoIterator, { + // TODO: Remove this. I'm only hardcoding this for debugging purposes. + let mut options = options; + options.print_ir_after_all = true; let inputs = inputs.into_iter().collect::>(); Self::make(inputs, output_dir, output_file, target_dir, options, emitter, source_manager) From 17c50659632365ea0f93dea7394b7f3e9053d092 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Wed, 16 Apr 2025 12:01:53 -0300 Subject: [PATCH 02/56] Revert "WIP: First draft" (See commit message for more details). After exploring the codebase a bit further, I managed to find the PassInstrumentor vector, which holds a series of PassInstrumentations. These PassInstrumentations apply passes before and after other passes. This structure, I think, fits the IR printing issue better. So I'll move the logic to this struct. This reverts commit a0b54f19bd0bb4e1437073e4f648ee063418e8b1. --- hir/src/ir/print.rs | 1 - hir/src/pass.rs | 8 ++++---- hir/src/pass/manager.rs | 22 +++++----------------- midenc-compile/src/stages/rewrite.rs | 1 - midenc-session/src/lib.rs | 3 --- 5 files changed, 9 insertions(+), 26 deletions(-) diff --git a/hir/src/ir/print.rs b/hir/src/ir/print.rs index ca3bfb3ea..86381db07 100644 --- a/hir/src/ir/print.rs +++ b/hir/src/ir/print.rs @@ -9,7 +9,6 @@ use crate::{ AttributeValue, EntityWithId, SuccessorOperands, Value, }; -#[derive(Debug)] pub struct OpPrintingFlags { pub print_entry_block_headers: bool, pub print_intrinsic_attributes: bool, diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 460a3c021..d87999128 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -97,7 +97,7 @@ impl Pass for Print { match self.filter { OpFilter::All => { let target = self.target.as_deref().unwrap_or("printer"); - log::error!(target: target, "{op}"); + log::trace!(target: target, "{op}"); } OpFilter::Type { dialect, @@ -106,21 +106,21 @@ impl Pass for Print { let name = op.name(); if name.dialect() == dialect && name.name() == op_name { let target = self.target.as_deref().unwrap_or("printer"); - log::error!(target: target, "{op}"); + log::trace!(target: target, "{op}"); } } OpFilter::Symbol(None) => { if let Some(sym) = op.as_symbol() { let name = sym.name().as_str(); let target = self.target.as_deref().unwrap_or(name); - log::error!(target: target, "{}", sym.as_symbol_operation()); + log::trace!(target: target, "{}", sym.as_symbol_operation()); } } OpFilter::Symbol(Some(filter)) => { if let Some(sym) = op.as_symbol().filter(|sym| sym.name().as_str().contains(filter)) { let target = self.target.as_deref().unwrap_or(filter); - log::error!(target: target, "{}", sym.as_symbol_operation()); + log::trace!(target: target, "{}", sym.as_symbol_operation()); } } } diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index d8c2d46e1..a9e4cc23d 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -9,8 +9,8 @@ use super::{ PassInstrumentor, PipelineParentInfo, Statistic, }; use crate::{ - pass::Print, traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, - Operation, OperationName, OperationRef, Report, + traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, Operation, + OperationName, OperationRef, Report, }; #[derive(Debug, Default, Copy, Clone, PartialEq, Eq)] @@ -29,7 +29,6 @@ pub enum PassDisplayMode { // TODO(pauls) #[allow(unused)] -#[derive(Default, Debug)] pub struct IRPrintingConfig { print_module_scope: bool, print_after_only_on_change: bool, @@ -41,7 +40,7 @@ pub struct IRPrintingConfig { pub struct PassManager { context: Rc, /// The underlying pass manager - pub pm: OpPassManager, + pm: OpPassManager, /// A manager for pass instrumentation instrumentor: Rc, /// An optional crash reproducer generator, if this pass manager is setup to @@ -170,11 +169,8 @@ impl PassManager { self } - // pub fn enable_ir_printing(&mut self, _config: IRPrintingConfig) { - pub fn enable_ir_printing(&mut self) -> &mut Self { - self.pm.printer = Some(IRPrintingConfig::default()); - // self.add_pass(Box::new(Print::any())); - self + pub fn enable_ir_printing(&mut self, _config: IRPrintingConfig) { + todo!() } pub fn enable_timing(&mut self, yes: bool) -> &mut Self { @@ -236,9 +232,6 @@ pub struct OpPassManager { passes: SmallVec<[Box; 8]>, /// Control the implicit nesting of passes that mismatch the name set for this manager nesting: Nesting, - - /// Indicates wheter or not to print IR after passes - pub printer: Option, } impl OpPassManager { @@ -253,7 +246,6 @@ impl OpPassManager { name: None, passes: Default::default(), nesting, - printer: None, } } @@ -283,7 +275,6 @@ impl OpPassManager { name: Some(name), passes: Default::default(), nesting, - printer: None, } } @@ -307,7 +298,6 @@ impl OpPassManager { name: Some(name), passes: Default::default(), nesting, - printer: None, } } @@ -317,7 +307,6 @@ impl OpPassManager { name: Some(name), passes: Default::default(), nesting, - printer: None, } } @@ -530,7 +519,6 @@ impl OpPassManager { pub fn nest_pass_manager(&mut self, nested: Self) -> NestedOpPassManager<'_> { let adaptor = Box::new(OpToOpPassAdaptor::new(nested)); - self.add_pass(Box::new(Print::any())); NestedOpPassManager { parent: self, nested: Some(adaptor), diff --git a/midenc-compile/src/stages/rewrite.rs b/midenc-compile/src/stages/rewrite.rs index 6a5d30cef..5fa1448c9 100644 --- a/midenc-compile/src/stages/rewrite.rs +++ b/midenc-compile/src/stages/rewrite.rs @@ -49,7 +49,6 @@ impl Stage for ApplyRewritesStage { // Construct a pass manager with the default pass pipeline let mut pm = PassManager::on::(context.clone(), Nesting::Implicit); - pm.enable_ir_printing(); let mut rewrite_config = GreedyRewriteConfig::default(); rewrite_config.with_region_simplification_level(RegionSimplificationLevel::Normal); diff --git a/midenc-session/src/lib.rs b/midenc-session/src/lib.rs index 5d0c93683..07fbc933b 100644 --- a/midenc-session/src/lib.rs +++ b/midenc-session/src/lib.rs @@ -128,9 +128,6 @@ impl Session { where I: IntoIterator, { - // TODO: Remove this. I'm only hardcoding this for debugging purposes. - let mut options = options; - options.print_ir_after_all = true; let inputs = inputs.into_iter().collect::>(); Self::make(inputs, output_dir, output_file, target_dir, options, emitter, source_manager) From e9220bfad6d522bcdfee44975563f3ced33a8a8e Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Wed, 16 Apr 2025 16:49:11 -0300 Subject: [PATCH 03/56] Add PassExecutionState as a parameter to the run_after_pass funtion. This is because both the Pass and OperationPass traits requiere it to execute. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 13 +++++++++++++ hir/src/pass/instrumentation.rs | 28 ++++++++++++++++++++++------ hir/src/pass/manager.rs | 6 +++--- midenc-compile/src/stages/rewrite.rs | 1 + 4 files changed, 39 insertions(+), 9 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index d87999128..7b2e861c7 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -16,6 +16,7 @@ pub use self::{ specialization::PassTarget, statistics::{PassStatistic, Statistic, StatisticValue}, }; +use crate::OperationRef; /// A `Pass` which prints IR it is run on, based on provided configuration. #[derive(Default)] @@ -127,3 +128,15 @@ impl Pass for Print { Ok(()) } } + +impl PassInstrumentation for Print { + fn run_after_pass( + &mut self, + _pass: &dyn OperationPass, + _op: &OperationRef, + _state: &PassExecutionState, + ) { + // Self::run_on_operation(&mut self, op, state).unwrap(); + todo!(); + } +} diff --git a/hir/src/pass/instrumentation.rs b/hir/src/pass/instrumentation.rs index c9d3f15ff..cdcb61c0e 100644 --- a/hir/src/pass/instrumentation.rs +++ b/hir/src/pass/instrumentation.rs @@ -5,7 +5,7 @@ use compact_str::CompactString; use smallvec::SmallVec; use super::OperationPass; -use crate::{OperationName, OperationRef}; +use crate::{pass::PassExecutionState, OperationName, OperationRef}; #[allow(unused_variables)] pub trait PassInstrumentation { @@ -22,7 +22,13 @@ pub trait PassInstrumentation { ) { } fn run_before_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) {} - fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) {} + fn run_after_pass( + &mut self, + pass: &dyn OperationPass, + op: &OperationRef, + state: &PassExecutionState, + ) { + } fn run_after_pass_failed(&mut self, pass: &dyn OperationPass, op: &OperationRef) {} fn run_before_analysis(&mut self, name: &str, id: &TypeId, op: &OperationRef) {} fn run_after_analysis(&mut self, name: &str, id: &TypeId, op: &OperationRef) {} @@ -54,8 +60,13 @@ impl PassInstrumentation for Box

{ (**self).run_before_pass(pass, op); } - fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { - (**self).run_after_pass(pass, op); + fn run_after_pass( + &mut self, + pass: &dyn OperationPass, + op: &OperationRef, + state: &PassExecutionState, + ) { + (**self).run_after_pass(pass, op, state); } fn run_after_pass_failed(&mut self, pass: &dyn OperationPass, op: &OperationRef) { @@ -97,8 +108,13 @@ impl PassInstrumentor { self.instrument(|pi| pi.run_before_pass(pass, op)); } - pub fn run_after_pass(&self, pass: &dyn OperationPass, op: &OperationRef) { - self.instrument(|pi| pi.run_after_pass(pass, op)); + pub fn run_after_pass( + &self, + pass: &dyn OperationPass, + op: &OperationRef, + state: &PassExecutionState, + ) { + self.instrument(|pi| pi.run_after_pass(pass, op, state)); } pub fn run_after_pass_failed(&self, pass: &dyn OperationPass, op: &OperationRef) { diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index a9e4cc23d..3c2ebfa00 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -9,8 +9,8 @@ use super::{ PassInstrumentor, PipelineParentInfo, Statistic, }; use crate::{ - traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, Operation, - OperationName, OperationRef, Report, + pass::Print, traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, + Operation, OperationName, OperationRef, Report, }; #[derive(Debug, Default, Copy, Clone, PartialEq, Eq)] @@ -952,7 +952,7 @@ impl OpToOpPassAdaptor { if result.is_err() { instrumentor.run_after_pass_failed(pass, &op); } else { - instrumentor.run_after_pass(pass, &op); + instrumentor.run_after_pass(pass, &op, &execution_state); } } diff --git a/midenc-compile/src/stages/rewrite.rs b/midenc-compile/src/stages/rewrite.rs index 5fa1448c9..6a5d30cef 100644 --- a/midenc-compile/src/stages/rewrite.rs +++ b/midenc-compile/src/stages/rewrite.rs @@ -49,6 +49,7 @@ impl Stage for ApplyRewritesStage { // Construct a pass manager with the default pass pipeline let mut pm = PassManager::on::(context.clone(), Nesting::Implicit); + pm.enable_ir_printing(); let mut rewrite_config = GreedyRewriteConfig::default(); rewrite_config.with_region_simplification_level(RegionSimplificationLevel::Normal); From 42ddc952760e263d870fa4f01ddc5ced37b70a9a Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Wed, 16 Apr 2025 17:42:00 -0300 Subject: [PATCH 04/56] Revert "Add PassExecutionState as a parameter to the run_after_pass funtion." This reverts commit e9220bfad6d522bcdfee44975563f3ced33a8a8e. --- hir/src/pass.rs | 13 ------------- hir/src/pass/instrumentation.rs | 28 ++++++---------------------- hir/src/pass/manager.rs | 6 +++--- midenc-compile/src/stages/rewrite.rs | 1 - 4 files changed, 9 insertions(+), 39 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 7b2e861c7..d87999128 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -16,7 +16,6 @@ pub use self::{ specialization::PassTarget, statistics::{PassStatistic, Statistic, StatisticValue}, }; -use crate::OperationRef; /// A `Pass` which prints IR it is run on, based on provided configuration. #[derive(Default)] @@ -128,15 +127,3 @@ impl Pass for Print { Ok(()) } } - -impl PassInstrumentation for Print { - fn run_after_pass( - &mut self, - _pass: &dyn OperationPass, - _op: &OperationRef, - _state: &PassExecutionState, - ) { - // Self::run_on_operation(&mut self, op, state).unwrap(); - todo!(); - } -} diff --git a/hir/src/pass/instrumentation.rs b/hir/src/pass/instrumentation.rs index cdcb61c0e..c9d3f15ff 100644 --- a/hir/src/pass/instrumentation.rs +++ b/hir/src/pass/instrumentation.rs @@ -5,7 +5,7 @@ use compact_str::CompactString; use smallvec::SmallVec; use super::OperationPass; -use crate::{pass::PassExecutionState, OperationName, OperationRef}; +use crate::{OperationName, OperationRef}; #[allow(unused_variables)] pub trait PassInstrumentation { @@ -22,13 +22,7 @@ pub trait PassInstrumentation { ) { } fn run_before_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) {} - fn run_after_pass( - &mut self, - pass: &dyn OperationPass, - op: &OperationRef, - state: &PassExecutionState, - ) { - } + fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) {} fn run_after_pass_failed(&mut self, pass: &dyn OperationPass, op: &OperationRef) {} fn run_before_analysis(&mut self, name: &str, id: &TypeId, op: &OperationRef) {} fn run_after_analysis(&mut self, name: &str, id: &TypeId, op: &OperationRef) {} @@ -60,13 +54,8 @@ impl PassInstrumentation for Box

{ (**self).run_before_pass(pass, op); } - fn run_after_pass( - &mut self, - pass: &dyn OperationPass, - op: &OperationRef, - state: &PassExecutionState, - ) { - (**self).run_after_pass(pass, op, state); + fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { + (**self).run_after_pass(pass, op); } fn run_after_pass_failed(&mut self, pass: &dyn OperationPass, op: &OperationRef) { @@ -108,13 +97,8 @@ impl PassInstrumentor { self.instrument(|pi| pi.run_before_pass(pass, op)); } - pub fn run_after_pass( - &self, - pass: &dyn OperationPass, - op: &OperationRef, - state: &PassExecutionState, - ) { - self.instrument(|pi| pi.run_after_pass(pass, op, state)); + pub fn run_after_pass(&self, pass: &dyn OperationPass, op: &OperationRef) { + self.instrument(|pi| pi.run_after_pass(pass, op)); } pub fn run_after_pass_failed(&self, pass: &dyn OperationPass, op: &OperationRef) { diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 3c2ebfa00..a9e4cc23d 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -9,8 +9,8 @@ use super::{ PassInstrumentor, PipelineParentInfo, Statistic, }; use crate::{ - pass::Print, traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, - Operation, OperationName, OperationRef, Report, + traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, Operation, + OperationName, OperationRef, Report, }; #[derive(Debug, Default, Copy, Clone, PartialEq, Eq)] @@ -952,7 +952,7 @@ impl OpToOpPassAdaptor { if result.is_err() { instrumentor.run_after_pass_failed(pass, &op); } else { - instrumentor.run_after_pass(pass, &op, &execution_state); + instrumentor.run_after_pass(pass, &op); } } diff --git a/midenc-compile/src/stages/rewrite.rs b/midenc-compile/src/stages/rewrite.rs index 6a5d30cef..5fa1448c9 100644 --- a/midenc-compile/src/stages/rewrite.rs +++ b/midenc-compile/src/stages/rewrite.rs @@ -49,7 +49,6 @@ impl Stage for ApplyRewritesStage { // Construct a pass manager with the default pass pipeline let mut pm = PassManager::on::(context.clone(), Nesting::Implicit); - pm.enable_ir_printing(); let mut rewrite_config = GreedyRewriteConfig::default(); rewrite_config.with_region_simplification_level(RegionSimplificationLevel::Normal); From 76309ab4400f61836caf6c800582fe3dcdb8b5c4 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Wed, 16 Apr 2025 17:54:34 -0300 Subject: [PATCH 05/56] WIP: Move printing logic into PassInstrumentator. Signed-off-by: Tomas Fabrizio Orsi --- .vscode/launch.json | 16 +++++++++++ hir/src/pass.rs | 41 +++++++++++++++++++++++++++- hir/src/pass/manager.rs | 12 +++++--- midenc-compile/src/stages/rewrite.rs | 1 + 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index fb2af45c7..9473b3c0b 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -4,6 +4,22 @@ // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 "version": "0.2.0", "configurations": [ + { + "type": "lldb", + "request": "launch", + "name": "Debug compilador", + "cargo": { + "args": ["build", "--bin=midenc", "--package=midenc"], + "filter": { + "name": "midenc", + "kind": "bin" + } + }, + "args": ["compile", "ejemplo_wasm/wasm_sum_sub_extra.wasm"], + // "args": ["compile", "root_ns:root@1.0.masm"], + "cwd": "${workspaceFolder}", + "sourceLanguages": ["rust"] + }, { "type": "lldb", "request": "launch", diff --git a/hir/src/pass.rs b/hir/src/pass.rs index d87999128..d5e528ab1 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -16,6 +16,7 @@ pub use self::{ specialization::PassTarget, statistics::{PassStatistic, Statistic, StatisticValue}, }; +use crate::OperationRef; /// A `Pass` which prints IR it is run on, based on provided configuration. #[derive(Default)] @@ -24,7 +25,7 @@ pub struct Print { target: Option, } -#[derive(Default)] +#[derive(Default, Debug)] enum OpFilter { /// Print all operations #[default] @@ -127,3 +128,41 @@ impl Pass for Print { Ok(()) } } + +impl PassInstrumentation for Print { + fn run_after_pass(&mut self, _pass: &dyn OperationPass, op: &OperationRef) { + // ::run_on_operation(&mut self, op, state).unwrap(); + std::dbg!(&self.filter); + + match self.filter { + OpFilter::All => { + let target = self.target.as_deref().unwrap_or("printer"); + log::error!(target: target, "{op}"); + } + OpFilter::Type { + dialect, + op: op_name, + } => { + let name = op.name(); + if name.dialect() == dialect && name.name() == op_name { + let target = self.target.as_deref().unwrap_or("printer"); + log::error!(target: target, "{op}"); + } + } + _ => todo!(), // OpFilter::Symbol(None) => { + // if let Some(sym) = op.as_symbol() { + // let name = sym.name().as_str(); + // let target = self.target.as_deref().unwrap_or(name); + // log::trace!(target: target, "{}", sym.as_symbol_operation()); + // } + // } + // OpFilter::Symbol(Some(filter)) => { + // if let Some(sym) = op.as_symbol().filter(|sym| sym.name().as_str().contains(filter)) + // { + // let target = self.target.as_deref().unwrap_or(filter); + // log::trace!(target: target, "{}", sym.as_symbol_operation()); + // } + // } + } + } +} diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index a9e4cc23d..c0bf93b71 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -9,8 +9,8 @@ use super::{ PassInstrumentor, PipelineParentInfo, Statistic, }; use crate::{ - traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, Operation, - OperationName, OperationRef, Report, + pass::Print, traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, + Operation, OperationName, OperationRef, Report, }; #[derive(Debug, Default, Copy, Clone, PartialEq, Eq)] @@ -169,8 +169,12 @@ impl PassManager { self } - pub fn enable_ir_printing(&mut self, _config: IRPrintingConfig) { - todo!() + // pub fn enable_ir_printing(&mut self, _config: IRPrintingConfig) { + pub fn enable_ir_printing(&mut self) { + // self.add_instrumentation(pi) + let p = Box::new(Print::any()); + self.add_instrumentation(p); + // todo!() } pub fn enable_timing(&mut self, yes: bool) -> &mut Self { diff --git a/midenc-compile/src/stages/rewrite.rs b/midenc-compile/src/stages/rewrite.rs index 5fa1448c9..6a5d30cef 100644 --- a/midenc-compile/src/stages/rewrite.rs +++ b/midenc-compile/src/stages/rewrite.rs @@ -49,6 +49,7 @@ impl Stage for ApplyRewritesStage { // Construct a pass manager with the default pass pipeline let mut pm = PassManager::on::(context.clone(), Nesting::Implicit); + pm.enable_ir_printing(); let mut rewrite_config = GreedyRewriteConfig::default(); rewrite_config.with_region_simplification_level(RegionSimplificationLevel::Normal); From 1b36899b34cb28308cf4cd7c96aadf64b2f2bfd0 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 21 Apr 2025 10:07:58 -0300 Subject: [PATCH 06/56] Cast the OperationRef into an EntityRef inside the InstrumentationPass for Print. which allows for all the cases to be executed and for the full structure of the IR to be printer. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index d5e528ab1..f9c91ec4f 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -134,6 +134,7 @@ impl PassInstrumentation for Print { // ::run_on_operation(&mut self, op, state).unwrap(); std::dbg!(&self.filter); + let op = op.borrow(); match self.filter { OpFilter::All => { let target = self.target.as_deref().unwrap_or("printer"); @@ -149,20 +150,20 @@ impl PassInstrumentation for Print { log::error!(target: target, "{op}"); } } - _ => todo!(), // OpFilter::Symbol(None) => { - // if let Some(sym) = op.as_symbol() { - // let name = sym.name().as_str(); - // let target = self.target.as_deref().unwrap_or(name); - // log::trace!(target: target, "{}", sym.as_symbol_operation()); - // } - // } - // OpFilter::Symbol(Some(filter)) => { - // if let Some(sym) = op.as_symbol().filter(|sym| sym.name().as_str().contains(filter)) - // { - // let target = self.target.as_deref().unwrap_or(filter); - // log::trace!(target: target, "{}", sym.as_symbol_operation()); - // } - // } + OpFilter::Symbol(None) => { + if let Some(sym) = op.as_symbol() { + let name = sym.name().as_str(); + let target = self.target.as_deref().unwrap_or(name); + log::trace!(target: target, "{}", sym.as_symbol_operation()); + } + } + OpFilter::Symbol(Some(filter)) => { + if let Some(sym) = op.as_symbol().filter(|sym| sym.name().as_str().contains(filter)) + { + let target = self.target.as_deref().unwrap_or(filter); + log::trace!(target: target, "{}", sym.as_symbol_operation()); + } + } } } } From c007ef7880879c116217f8aef9e8052d69195f55 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 21 Apr 2025 11:00:10 -0300 Subject: [PATCH 07/56] Move operation printing to auxiliary print_ir function Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 82 +++++++++++++++++-------------------------------- 1 file changed, 28 insertions(+), 54 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index f9c91ec4f..f246b2803 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -16,7 +16,7 @@ pub use self::{ specialization::PassTarget, statistics::{PassStatistic, Statistic, StatisticValue}, }; -use crate::OperationRef; +use crate::{EntityRef, Operation, OperationRef}; /// A `Pass` which prints IR it is run on, based on provided configuration. #[derive(Default)] @@ -76,29 +76,12 @@ impl Print { self.target = Some(target); self } -} -impl Pass for Print { - type Target = crate::Operation; - - fn name(&self) -> &'static str { - "print" - } - - fn can_schedule_on(&self, _name: &crate::OperationName) -> bool { - true - } - - fn run_on_operation( - &mut self, - op: crate::EntityMut<'_, Self::Target>, - _state: &mut PassExecutionState, - ) -> Result<(), crate::Report> { - let op = op.into_entity_ref(); + fn print_ir(&self, op: EntityRef<'_, Operation>) { match self.filter { OpFilter::All => { let target = self.target.as_deref().unwrap_or("printer"); - log::trace!(target: target, "{op}"); + log::error!(target: target, "{op}"); } OpFilter::Type { dialect, @@ -107,63 +90,54 @@ impl Pass for Print { let name = op.name(); if name.dialect() == dialect && name.name() == op_name { let target = self.target.as_deref().unwrap_or("printer"); - log::trace!(target: target, "{op}"); + log::error!(target: target, "{op}"); } } OpFilter::Symbol(None) => { if let Some(sym) = op.as_symbol() { let name = sym.name().as_str(); let target = self.target.as_deref().unwrap_or(name); - log::trace!(target: target, "{}", sym.as_symbol_operation()); + log::error!(target: target, "{}", sym.as_symbol_operation()); } } OpFilter::Symbol(Some(filter)) => { if let Some(sym) = op.as_symbol().filter(|sym| sym.name().as_str().contains(filter)) { let target = self.target.as_deref().unwrap_or(filter); - log::trace!(target: target, "{}", sym.as_symbol_operation()); + log::error!(target: target, "{}", sym.as_symbol_operation()); } } } + } +} + +impl Pass for Print { + type Target = crate::Operation; + + fn name(&self) -> &'static str { + "print" + } + + fn can_schedule_on(&self, _name: &crate::OperationName) -> bool { + true + } + + fn run_on_operation( + &mut self, + op: crate::EntityMut<'_, Self::Target>, + _state: &mut PassExecutionState, + ) -> Result<(), crate::Report> { + let op = op.into_entity_ref(); + self.print_ir(op); Ok(()) } } impl PassInstrumentation for Print { fn run_after_pass(&mut self, _pass: &dyn OperationPass, op: &OperationRef) { - // ::run_on_operation(&mut self, op, state).unwrap(); std::dbg!(&self.filter); let op = op.borrow(); - match self.filter { - OpFilter::All => { - let target = self.target.as_deref().unwrap_or("printer"); - log::error!(target: target, "{op}"); - } - OpFilter::Type { - dialect, - op: op_name, - } => { - let name = op.name(); - if name.dialect() == dialect && name.name() == op_name { - let target = self.target.as_deref().unwrap_or("printer"); - log::error!(target: target, "{op}"); - } - } - OpFilter::Symbol(None) => { - if let Some(sym) = op.as_symbol() { - let name = sym.name().as_str(); - let target = self.target.as_deref().unwrap_or(name); - log::trace!(target: target, "{}", sym.as_symbol_operation()); - } - } - OpFilter::Symbol(Some(filter)) => { - if let Some(sym) = op.as_symbol().filter(|sym| sym.name().as_str().contains(filter)) - { - let target = self.target.as_deref().unwrap_or(filter); - log::trace!(target: target, "{}", sym.as_symbol_operation()); - } - } - } + self.print_ir(op); } } From 511dac2d3d6d32ffd559f5a85b639a3af01944b7 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 21 Apr 2025 12:46:42 -0300 Subject: [PATCH 08/56] Add IRPrintConfig to the the enable_ir_print function. Not yet used, but will determine how the Pass print is created. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 2 +- hir/src/pass/manager.rs | 31 ++++++++++++++++++++++------ midenc-compile/src/compiler.rs | 4 +++- midenc-compile/src/stages/rewrite.rs | 5 +++-- 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index f246b2803..a7d9fa929 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -10,7 +10,7 @@ pub mod statistics; pub use self::{ analysis::{Analysis, AnalysisManager, OperationAnalysis, PreservedAnalyses}, instrumentation::{PassInstrumentation, PassInstrumentor, PipelineParentInfo}, - manager::{Nesting, OpPassManager, PassDisplayMode, PassManager}, + manager::{IRPrintingConfig, Nesting, OpPassManager, PassDisplayMode, PassManager}, pass::{OperationPass, Pass, PassExecutionState}, registry::{PassInfo, PassPipelineInfo}, specialization::PassTarget, diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index c0bf93b71..745eb7a13 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -1,7 +1,14 @@ -use alloc::{boxed::Box, collections::BTreeMap, format, rc::Rc, string::ToString}; +use alloc::{ + boxed::Box, + collections::BTreeMap, + format, + rc::Rc, + string::{String, ToString}, + vec::Vec, +}; use compact_str::{CompactString, ToCompactString}; -use midenc_session::diagnostics::Severity; +use midenc_session::{diagnostics::Severity, Options}; use smallvec::{smallvec, SmallVec}; use super::{ @@ -29,13 +36,27 @@ pub enum PassDisplayMode { // TODO(pauls) #[allow(unused)] +#[derive(Default)] pub struct IRPrintingConfig { print_module_scope: bool, print_after_only_on_change: bool, print_after_only_on_failure: bool, + // NOTE: Taken from the Options struct + print_ir_after_all: bool, + print_ir_after_pass: Vec, flags: OpPrintingFlags, } +impl From<&Options> for IRPrintingConfig { + fn from(options: &Options) -> Self { + let mut irprintconfig = IRPrintingConfig::default(); + irprintconfig.print_ir_after_all = options.print_ir_after_all; + irprintconfig.print_ir_after_pass = options.print_ir_after_pass.clone(); + + irprintconfig + } +} + /// The main pass manager and pipeline builder pub struct PassManager { context: Rc, @@ -73,6 +94,7 @@ impl PassManager { /// /// The created pass manager can schedule operations that match type `T`. pub fn on(context: Rc, nesting: Nesting) -> Self { + // let a = context.session().options; Self::new(context, ::full_name(), nesting) } @@ -169,12 +191,9 @@ impl PassManager { self } - // pub fn enable_ir_printing(&mut self, _config: IRPrintingConfig) { - pub fn enable_ir_printing(&mut self) { - // self.add_instrumentation(pi) + pub fn enable_ir_printing(&mut self, _config: IRPrintingConfig) { let p = Box::new(Print::any()); self.add_instrumentation(p); - // todo!() } pub fn enable_timing(&mut self, yes: bool) -> &mut Self { diff --git a/midenc-compile/src/compiler.rs b/midenc-compile/src/compiler.rs index 1a6f27ecd..8f978569e 100644 --- a/midenc-compile/src/compiler.rs +++ b/midenc-compile/src/compiler.rs @@ -503,8 +503,10 @@ impl Compiler { options.no_link = codegen.no_link; options.print_cfg_after_all = unstable.print_cfg_after_all; options.print_cfg_after_pass = unstable.print_cfg_after_pass; - options.print_ir_after_all = unstable.print_ir_after_all; + // options.print_ir_after_all = unstable.print_ir_after_all; + options.print_ir_after_all = true; options.print_ir_after_pass = unstable.print_ir_after_pass; + std::dbg!(&options); // Establish --target-dir let target_dir = if self.target_dir.is_absolute() { diff --git a/midenc-compile/src/stages/rewrite.rs b/midenc-compile/src/stages/rewrite.rs index 6a5d30cef..9a0b44566 100644 --- a/midenc-compile/src/stages/rewrite.rs +++ b/midenc-compile/src/stages/rewrite.rs @@ -3,7 +3,7 @@ use alloc::boxed::Box; use midenc_dialect_hir::transforms::TransformSpills; use midenc_dialect_scf::transforms::LiftControlFlowToSCF; use midenc_hir::{ - pass::{Nesting, PassManager}, + pass::{IRPrintingConfig, Nesting, PassManager}, patterns::{GreedyRewriteConfig, RegionSimplificationLevel}, Op, }; @@ -22,6 +22,7 @@ impl Stage for ApplyRewritesStage { } fn run(&mut self, input: Self::Input, context: Rc) -> CompilerResult { + let ir_print_config: IRPrintingConfig = (&context.as_ref().session().options).into(); log::debug!(target: "driver", "applying rewrite passes"); // TODO(pauls): Set up pass registration for new pass infra /* @@ -49,7 +50,7 @@ impl Stage for ApplyRewritesStage { // Construct a pass manager with the default pass pipeline let mut pm = PassManager::on::(context.clone(), Nesting::Implicit); - pm.enable_ir_printing(); + pm.enable_ir_printing(ir_print_config); let mut rewrite_config = GreedyRewriteConfig::default(); rewrite_config.with_region_simplification_level(RegionSimplificationLevel::Normal); From d4537075015c5862698649924415f2c042a0c007 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 21 Apr 2025 16:01:22 -0300 Subject: [PATCH 09/56] Enable Print to only print ir after a specific pass has passed. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/ir/print.rs | 1 + hir/src/pass.rs | 39 ++++++++++++++++++++++++++++------ hir/src/pass/manager.rs | 14 +++++++++--- midenc-compile/src/compiler.rs | 7 +++--- 4 files changed, 49 insertions(+), 12 deletions(-) diff --git a/hir/src/ir/print.rs b/hir/src/ir/print.rs index 86381db07..ca3bfb3ea 100644 --- a/hir/src/ir/print.rs +++ b/hir/src/ir/print.rs @@ -9,6 +9,7 @@ use crate::{ AttributeValue, EntityWithId, SuccessorOperands, Value, }; +#[derive(Debug)] pub struct OpPrintingFlags { pub print_entry_block_headers: bool, pub print_intrinsic_attributes: bool, diff --git a/hir/src/pass.rs b/hir/src/pass.rs index a7d9fa929..757004779 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -16,15 +16,27 @@ pub use self::{ specialization::PassTarget, statistics::{PassStatistic, Statistic, StatisticValue}, }; -use crate::{EntityRef, Operation, OperationRef}; +use crate::{ + alloc::{string::String, vec::Vec}, + EntityRef, Operation, OperationRef, +}; /// A `Pass` which prints IR it is run on, based on provided configuration. #[derive(Default)] pub struct Print { filter: OpFilter, + pass_filter: PassFilter, target: Option, } +#[allow(dead_code)] +#[derive(Default, Debug)] +enum PassFilter { + #[default] + All, + Certain(Vec), +} + #[derive(Default, Debug)] enum OpFilter { /// Print all operations @@ -45,6 +57,7 @@ impl Print { pub fn any() -> Self { Self { filter: OpFilter::All, + pass_filter: PassFilter::All, target: None, } } @@ -55,14 +68,21 @@ impl Print { let op = ::name(); Self { filter: OpFilter::Type { dialect, op }, + pass_filter: PassFilter::All, target: None, } } + pub fn with_pass_filter(mut self, passes: Vec) -> Self { + self.pass_filter = PassFilter::Certain(passes); + self + } + /// Create a printer that only prints `Symbol` operations containing `name` pub fn symbol_matching(name: &'static str) -> Self { Self { filter: OpFilter::Symbol(Some(name)), + pass_filter: PassFilter::All, target: None, } } @@ -109,6 +129,13 @@ impl Print { } } } + + fn should_print(&self, pass: &dyn OperationPass) -> bool { + match &self.pass_filter { + PassFilter::All => true, + PassFilter::Certain(passes) => passes.iter().any(|p| p == pass.name()), + } + } } impl Pass for Print { @@ -134,10 +161,10 @@ impl Pass for Print { } impl PassInstrumentation for Print { - fn run_after_pass(&mut self, _pass: &dyn OperationPass, op: &OperationRef) { - std::dbg!(&self.filter); - - let op = op.borrow(); - self.print_ir(op); + fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { + if self.should_print(pass) { + let op = op.borrow(); + self.print_ir(op); + } } } diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 745eb7a13..85a037c04 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -36,7 +36,7 @@ pub enum PassDisplayMode { // TODO(pauls) #[allow(unused)] -#[derive(Default)] +#[derive(Default, Debug)] pub struct IRPrintingConfig { print_module_scope: bool, print_after_only_on_change: bool, @@ -191,8 +191,16 @@ impl PassManager { self } - pub fn enable_ir_printing(&mut self, _config: IRPrintingConfig) { - let p = Box::new(Print::any()); + pub fn enable_ir_printing(&mut self, config: IRPrintingConfig) { + std::dbg!(&config); + let print = if config.print_ir_after_all { + Print::any() + } else if !config.print_ir_after_pass.is_empty() { + Print::any().with_pass_filter(config.print_ir_after_pass) + } else { + Print::any() + }; + let p = Box::new(print); self.add_instrumentation(p); } diff --git a/midenc-compile/src/compiler.rs b/midenc-compile/src/compiler.rs index 8f978569e..2a0cd8e0f 100644 --- a/midenc-compile/src/compiler.rs +++ b/midenc-compile/src/compiler.rs @@ -503,9 +503,10 @@ impl Compiler { options.no_link = codegen.no_link; options.print_cfg_after_all = unstable.print_cfg_after_all; options.print_cfg_after_pass = unstable.print_cfg_after_pass; - // options.print_ir_after_all = unstable.print_ir_after_all; - options.print_ir_after_all = true; - options.print_ir_after_pass = unstable.print_ir_after_pass; + options.print_ir_after_all = unstable.print_ir_after_all; + // options.print_ir_after_all = true; + // options.print_ir_after_pass = unstable.print_ir_after_pass; + options.print_ir_after_pass = vec!["canonicalizer".to_string()]; std::dbg!(&options); // Establish --target-dir From be814de3444814d49e1c5b35ef0d8c14b5ee1042 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 21 Apr 2025 16:12:30 -0300 Subject: [PATCH 10/56] Add run_before_pass implementation for Print struct Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 757004779..0a203aad7 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -163,6 +163,15 @@ impl Pass for Print { impl PassInstrumentation for Print { fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { if self.should_print(pass) { + std::dbg!("RUN AFTER PASS"); + let op = op.borrow(); + self.print_ir(op); + } + } + + fn run_before_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { + if self.should_print(pass) { + std::dbg!("RUN BEFORE PASS"); let op = op.borrow(); self.print_ir(op); } From 129df1c7b05701bc1fdc6fba7cd0405cd1f7a4d4 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 21 Apr 2025 16:27:07 -0300 Subject: [PATCH 11/56] Add some initial documentation regarding PassFilters. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 0a203aad7..cbb1f06e2 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -32,8 +32,10 @@ pub struct Print { #[allow(dead_code)] #[derive(Default, Debug)] enum PassFilter { + /// Print IR regardless of pass #[default] All, + /// Only print IR if the pass's name matches one of these matches. Certain(Vec), } From da00c03d43905adc9d1757c369f24a74b0c19bda Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Tue, 22 Apr 2025 11:05:28 -0300 Subject: [PATCH 12/56] Remove debug config from .vscode directory Signed-off-by: Tomas Fabrizio Orsi --- .vscode/launch.json | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 9473b3c0b..fb2af45c7 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -4,22 +4,6 @@ // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 "version": "0.2.0", "configurations": [ - { - "type": "lldb", - "request": "launch", - "name": "Debug compilador", - "cargo": { - "args": ["build", "--bin=midenc", "--package=midenc"], - "filter": { - "name": "midenc", - "kind": "bin" - } - }, - "args": ["compile", "ejemplo_wasm/wasm_sum_sub_extra.wasm"], - // "args": ["compile", "root_ns:root@1.0.masm"], - "cwd": "${workspaceFolder}", - "sourceLanguages": ["rust"] - }, { "type": "lldb", "request": "launch", From 3cd1f1560da50323c3de029c81cdd1ceaef058d8 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Tue, 22 Apr 2025 12:10:31 -0300 Subject: [PATCH 13/56] Clean code up: Add documentation, remove prints, slight refactors. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 18 +++++++++--------- hir/src/pass/manager.rs | 15 ++++++++------- midenc-compile/src/compiler.rs | 5 +---- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index cbb1f06e2..6a6a7192a 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -29,13 +29,13 @@ pub struct Print { target: Option, } -#[allow(dead_code)] +/// Filter for the different passes. #[derive(Default, Debug)] enum PassFilter { - /// Print IR regardless of pass + /// Print IR regardless of which pass is executed. #[default] All, - /// Only print IR if the pass's name matches one of these matches. + /// Only print IR if the pass's name is present in the vector. Certain(Vec), } @@ -75,6 +75,8 @@ impl Print { } } + /// Adds a PassFilter to Print. IR will only be printed before and after those passes are + /// executed. pub fn with_pass_filter(mut self, passes: Vec) -> Self { self.pass_filter = PassFilter::Certain(passes); self @@ -103,7 +105,7 @@ impl Print { match self.filter { OpFilter::All => { let target = self.target.as_deref().unwrap_or("printer"); - log::error!(target: target, "{op}"); + log::trace!(target: target, "{op}"); } OpFilter::Type { dialect, @@ -112,21 +114,21 @@ impl Print { let name = op.name(); if name.dialect() == dialect && name.name() == op_name { let target = self.target.as_deref().unwrap_or("printer"); - log::error!(target: target, "{op}"); + log::trace!(target: target, "{op}"); } } OpFilter::Symbol(None) => { if let Some(sym) = op.as_symbol() { let name = sym.name().as_str(); let target = self.target.as_deref().unwrap_or(name); - log::error!(target: target, "{}", sym.as_symbol_operation()); + log::trace!(target: target, "{}", sym.as_symbol_operation()); } } OpFilter::Symbol(Some(filter)) => { if let Some(sym) = op.as_symbol().filter(|sym| sym.name().as_str().contains(filter)) { let target = self.target.as_deref().unwrap_or(filter); - log::error!(target: target, "{}", sym.as_symbol_operation()); + log::trace!(target: target, "{}", sym.as_symbol_operation()); } } } @@ -165,7 +167,6 @@ impl Pass for Print { impl PassInstrumentation for Print { fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { if self.should_print(pass) { - std::dbg!("RUN AFTER PASS"); let op = op.borrow(); self.print_ir(op); } @@ -173,7 +174,6 @@ impl PassInstrumentation for Print { fn run_before_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { if self.should_print(pass) { - std::dbg!("RUN BEFORE PASS"); let op = op.borrow(); self.print_ir(op); } diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 85a037c04..d14d42989 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -94,7 +94,6 @@ impl PassManager { /// /// The created pass manager can schedule operations that match type `T`. pub fn on(context: Rc, nesting: Nesting) -> Self { - // let a = context.session().options; Self::new(context, ::full_name(), nesting) } @@ -192,16 +191,18 @@ impl PassManager { } pub fn enable_ir_printing(&mut self, config: IRPrintingConfig) { - std::dbg!(&config); let print = if config.print_ir_after_all { - Print::any() + Some(Print::any()) } else if !config.print_ir_after_pass.is_empty() { - Print::any().with_pass_filter(config.print_ir_after_pass) + Some(Print::any().with_pass_filter(config.print_ir_after_pass)) } else { - Print::any() + None }; - let p = Box::new(print); - self.add_instrumentation(p); + + if let Some(print) = print { + let print = Box::new(print); + self.add_instrumentation(print); + } } pub fn enable_timing(&mut self, yes: bool) -> &mut Self { diff --git a/midenc-compile/src/compiler.rs b/midenc-compile/src/compiler.rs index 2a0cd8e0f..1a6f27ecd 100644 --- a/midenc-compile/src/compiler.rs +++ b/midenc-compile/src/compiler.rs @@ -504,10 +504,7 @@ impl Compiler { options.print_cfg_after_all = unstable.print_cfg_after_all; options.print_cfg_after_pass = unstable.print_cfg_after_pass; options.print_ir_after_all = unstable.print_ir_after_all; - // options.print_ir_after_all = true; - // options.print_ir_after_pass = unstable.print_ir_after_pass; - options.print_ir_after_pass = vec!["canonicalizer".to_string()]; - std::dbg!(&options); + options.print_ir_after_pass = unstable.print_ir_after_pass; // Establish --target-dir let target_dir = if self.target_dir.is_absolute() { From 300ecbcd2df84938835b40752802b3ba8b3c68ee Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Tue, 22 Apr 2025 17:40:06 -0300 Subject: [PATCH 14/56] Make `run_on_operation` return a boolean indicating that the IR has been changed. Signed-off-by: Tomas Fabrizio Orsi --- dialects/hir/src/transforms/spill.rs | 6 +++--- dialects/scf/src/transforms/cfg_to_scf.rs | 7 ++++--- hir-transform/src/canonicalization.rs | 24 +++++++++++++---------- hir-transform/src/sccp.rs | 15 ++++++++------ hir-transform/src/sink.rs | 20 ++++++++++++------- hir-transform/src/spill.rs | 5 +++-- hir/src/pass.rs | 4 ++-- hir/src/pass/manager.rs | 10 +++++----- hir/src/pass/pass.rs | 8 ++++---- 9 files changed, 57 insertions(+), 42 deletions(-) diff --git a/dialects/hir/src/transforms/spill.rs b/dialects/hir/src/transforms/spill.rs index a981d5f57..879c6874a 100644 --- a/dialects/hir/src/transforms/spill.rs +++ b/dialects/hir/src/transforms/spill.rs @@ -31,14 +31,14 @@ impl Pass for TransformSpills { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result<(), Report> { + ) -> Result { let function = op.into_entity_ref(); log::debug!(target: "insert-spills", "computing and inserting spills for {}", function.as_operation()); if function.is_declaration() { log::debug!(target: "insert-spills", "function has no body, no spills needed!"); state.preserved_analyses_mut().preserve_all(); - return Ok(()); + return Ok(false); } let mut analysis = state.analysis_manager().get_analysis_for::()?; @@ -46,7 +46,7 @@ impl Pass for TransformSpills { if !analysis.has_spills() { log::debug!(target: "insert-spills", "no spills needed!"); state.preserved_analyses_mut().preserve_all(); - return Ok(()); + return Ok(false); } // Take ownership of the spills analysis so that we can mutate the analysis state during diff --git a/dialects/scf/src/transforms/cfg_to_scf.rs b/dialects/scf/src/transforms/cfg_to_scf.rs index 0ca813aea..fd092d847 100644 --- a/dialects/scf/src/transforms/cfg_to_scf.rs +++ b/dialects/scf/src/transforms/cfg_to_scf.rs @@ -51,7 +51,7 @@ impl Pass for LiftControlFlowToSCF { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result<(), Report> { + ) -> Result { let mut transformation = ControlFlowToSCFTransformation; let mut changed = false; @@ -130,7 +130,8 @@ impl Pass for LiftControlFlowToSCF { }); if result.was_interrupted() { - return result.into_result(); + // TODO: Maybe Change into_result implementation + return result.into_result().map(|_| false); } log::debug!( @@ -141,7 +142,7 @@ impl Pass for LiftControlFlowToSCF { state.preserved_analyses_mut().preserve_all(); } - Ok(()) + Ok(changed) } } diff --git a/hir-transform/src/canonicalization.rs b/hir-transform/src/canonicalization.rs index 5e600a774..d033e4d6a 100644 --- a/hir-transform/src/canonicalization.rs +++ b/hir-transform/src/canonicalization.rs @@ -93,10 +93,10 @@ impl Pass for Canonicalizer { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result<(), Report> { + ) -> Result { let Some(rewrites) = self.rewrites.as_ref() else { log::debug!("skipping canonicalization as there are no rewrite patterns to apply"); - return Ok(()); + return Ok(false); }; let op = { let ptr = op.as_operation_ref(); @@ -129,16 +129,20 @@ impl Pass for Canonicalizer { } let op = op.borrow(); - match converged { + let changed = match converged { Ok(changed) => { - log::debug!("canonicalization converged for '{}', changed={changed}", op.name()) + log::debug!("canonicalization converged for '{}', changed={changed}", op.name()); + changed } - Err(changed) => log::warn!( - "canonicalization failed to converge for '{}', changed={changed}", - op.name() - ), - } + Err(changed) => { + log::warn!( + "canonicalization failed to converge for '{}', changed={changed}", + op.name() + ); + changed + } + }; - Ok(()) + Ok(changed) } } diff --git a/hir-transform/src/sccp.rs b/hir-transform/src/sccp.rs index f12c85685..d7e7c9548 100644 --- a/hir-transform/src/sccp.rs +++ b/hir-transform/src/sccp.rs @@ -38,7 +38,7 @@ impl Pass for SparseConditionalConstantPropagation { &mut self, mut op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result<(), Report> { + ) -> Result { // Run sparse constant propagation + dead code analysis let mut solver = DataFlowSolver::default(); solver.load::(); @@ -58,7 +58,7 @@ impl SparseConditionalConstantPropagation { op: &mut Operation, _state: &mut PassExecutionState, solver: &DataFlowSolver, - ) -> Result<(), Report> { + ) -> Result { let mut worklist = SmallVec::<[BlockRef; 8]>::default(); let add_to_worklist = |regions: &RegionList, worklist: &mut SmallVec<[BlockRef; 8]>| { @@ -76,6 +76,7 @@ impl SparseConditionalConstantPropagation { add_to_worklist(op.regions(), &mut worklist); + let mut replaced_any = false; while let Some(mut block) = worklist.pop() { let mut block = block.borrow_mut(); let body = block.body_mut(); @@ -91,8 +92,10 @@ impl SparseConditionalConstantPropagation { let mut replaced_all = num_results != 0; for index in 0..num_results { let result = { op.borrow().get_result(index).borrow().as_value_ref() }; - replaced_all &= - replace_with_constant(solver, &mut builder, &mut folder, result); + let replaced = replace_with_constant(solver, &mut builder, &mut folder, result); + + replaced_any |= replaced; + replaced_all &= replaced; } // If all of the results of the operation were replaced, try to erase the operation @@ -112,7 +115,7 @@ impl SparseConditionalConstantPropagation { builder.set_insertion_point_to_start(block.as_block_ref()); for arg in block.arguments() { - replace_with_constant( + replaced_any |= replace_with_constant( solver, &mut builder, &mut folder, @@ -121,7 +124,7 @@ impl SparseConditionalConstantPropagation { } } - Ok(()) + Ok(replaced_any) } } diff --git a/hir-transform/src/sink.rs b/hir-transform/src/sink.rs index 3504c1398..defaa91dc 100644 --- a/hir-transform/src/sink.rs +++ b/hir-transform/src/sink.rs @@ -96,7 +96,7 @@ impl Pass for ControlFlowSink { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result<(), Report> { + ) -> Result { let op = op.into_entity_ref(); log::debug!(target: "control-flow-sink", "sinking operations in {op}"); @@ -105,6 +105,7 @@ impl Pass for ControlFlowSink { let dominfo = state.analysis_manager().get_analysis::()?; + let mut sunk = false; operation.raw_prewalk_all::(|op: OperationRef| { let regions_to_sink = { let op = op.borrow(); @@ -118,7 +119,7 @@ impl Pass for ControlFlowSink { }; // Sink side-effect free operations. - control_flow_sink( + sunk = control_flow_sink( ®ions_to_sink, &dominfo, |op: &Operation, _region: &Region| op.is_memory_effect_free(), @@ -132,7 +133,7 @@ impl Pass for ControlFlowSink { ); }); - Ok(()) + Ok(sunk) } } @@ -172,7 +173,7 @@ impl Pass for SinkOperandDefs { &mut self, op: EntityMut<'_, Self::Target>, _state: &mut PassExecutionState, - ) -> Result<(), Report> { + ) -> Result { let operation = op.as_operation_ref(); drop(op); @@ -184,6 +185,7 @@ impl Pass for SinkOperandDefs { // then process the worklist, moving everything into position. let mut worklist = alloc::collections::VecDeque::default(); + let mut changed = false; // Visit ops in "true" post-order (i.e. block bodies are visited bottom-up). operation.raw_postwalk_all::(|operation: OperationRef| { // Determine if any of this operation's operands represent one of the following: @@ -308,6 +310,7 @@ impl Pass for SinkOperandDefs { log::trace!(target: "sink-operand-defs", " rewriting operand {operand_value} as {replacement}"); operand.borrow_mut().set(replacement); + changed = true; // If no other uses of this value remain, then remove the original // operation, as it is now dead. if !operand_value.borrow().is_used() { @@ -354,6 +357,7 @@ impl Pass for SinkOperandDefs { log::trace!(target: "sink-operand-defs", " rewriting operand {operand_value} as {replacement}"); sink_state.replacements.insert(operand_value, replacement); operand.borrow_mut().set(replacement); + changed = true; } else { log::trace!(target: "sink-operand-defs", " defining op is a constant with no other uses, moving into place"); // The original op can be moved @@ -397,7 +401,7 @@ impl Pass for SinkOperandDefs { } } - Ok(()) + Ok(changed) } } @@ -548,12 +552,14 @@ pub fn control_flow_sink( dominfo: &DominanceInfo, should_move_into_region: P, move_into_region: F, -) where +) -> bool +where P: Fn(&Operation, &Region) -> bool, F: Fn(OperationRef, RegionRef), { let sinker = Sinker::new(dominfo, should_move_into_region, move_into_region); - sinker.sink_regions(regions); + let sunk_regions = sinker.sink_regions(regions); + sunk_regions > 0 } /// Populates `regions` with regions of the provided region branch op that are executed at most once diff --git a/hir-transform/src/spill.rs b/hir-transform/src/spill.rs index d758ede48..579ead8dd 100644 --- a/hir-transform/src/spill.rs +++ b/hir-transform/src/spill.rs @@ -113,7 +113,7 @@ pub fn transform_spills( analysis: &mut SpillAnalysis, interface: &mut dyn TransformSpillsInterface, analysis_manager: AnalysisManager, -) -> Result<(), Report> { +) -> Result { assert!( op.borrow().implements::(), "the spills transformation is not supported when the root op is multi-region" @@ -252,7 +252,8 @@ pub fn transform_spills( )?; } - Ok(()) + // QUESTION: If the code has reached this point, then, in theory, it must've gotten changed. Right? Requires double check. + Ok(true) } fn rewrite_single_block_spills( diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 6a6a7192a..c3475abde 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -157,10 +157,10 @@ impl Pass for Print { &mut self, op: crate::EntityMut<'_, Self::Target>, _state: &mut PassExecutionState, - ) -> Result<(), crate::Report> { + ) -> Result { let op = op.into_entity_ref(); self.print_ir(op); - Ok(()) + Ok(false) } } diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index d14d42989..9c0059826 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -956,9 +956,9 @@ impl OpToOpPassAdaptor { let mut result = if let Some(adaptor) = pass.as_any_mut().downcast_mut::() { - adaptor.run_on_operation(op, &mut execution_state, verify) + adaptor.run_on_operation(op, &mut execution_state, verify).map(|_| ()) } else { - pass.run_on_operation(op, &mut execution_state) + pass.run_on_operation(op, &mut execution_state).map(|_| ()) }; // Invalidate any non-preserved analyses @@ -1006,7 +1006,7 @@ impl OpToOpPassAdaptor { op: OperationRef, state: &mut PassExecutionState, verify: bool, - ) -> Result<(), Report> { + ) -> Result { let analysis_manager = state.analysis_manager(); let instrumentor = analysis_manager.pass_instrumentor(); let parent_info = PipelineParentInfo { @@ -1041,7 +1041,7 @@ impl OpToOpPassAdaptor { } } - Ok(()) + Ok(false) } } @@ -1078,7 +1078,7 @@ impl Pass for OpToOpPassAdaptor { &mut self, _op: EntityMut<'_, Operation>, _state: &mut PassExecutionState, - ) -> Result<(), Report> { + ) -> Result { unreachable!("unexpected call to `Pass::run_on_operation` for OpToOpPassAdaptor") } } diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index 231ea5c09..880e2040c 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -44,7 +44,7 @@ pub trait OperationPass { &mut self, op: OperationRef, state: &mut PassExecutionState, - ) -> Result<(), Report>; + ) -> Result; fn run_pipeline( &mut self, pipeline: &mut OpPassManager, @@ -121,7 +121,7 @@ where &mut self, mut op: OperationRef, state: &mut PassExecutionState, - ) -> Result<(), Report> { + ) -> Result { let op = <

::Target as PassTarget>::into_target_mut(&mut op);

::run_on_operation(self, op, state) } @@ -231,7 +231,7 @@ pub trait Pass: Sized + Any { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result<(), Report>; + ) -> Result; /// Schedule an arbitrary pass pipeline on the provided operation. /// /// This can be invoke any time in a pass to dynamic schedule more passes. The provided @@ -330,7 +330,7 @@ where &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result<(), Report> { + ) -> Result { (**self).run_on_operation(op, state) } From 9dc31b999d63a2607f383660296182ca83295174 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Tue, 22 Apr 2025 18:04:22 -0300 Subject: [PATCH 15/56] Add changed parameter to run_after_pass function Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 2 +- hir/src/pass/instrumentation.rs | 10 +++++----- hir/src/pass/manager.rs | 15 ++++++++++----- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index c3475abde..adaf203c0 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -165,7 +165,7 @@ impl Pass for Print { } impl PassInstrumentation for Print { - fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { + fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef, _changed: bool) { if self.should_print(pass) { let op = op.borrow(); self.print_ir(op); diff --git a/hir/src/pass/instrumentation.rs b/hir/src/pass/instrumentation.rs index c9d3f15ff..73affd42e 100644 --- a/hir/src/pass/instrumentation.rs +++ b/hir/src/pass/instrumentation.rs @@ -22,7 +22,7 @@ pub trait PassInstrumentation { ) { } fn run_before_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) {} - fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) {} + fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef, changed: bool) {} fn run_after_pass_failed(&mut self, pass: &dyn OperationPass, op: &OperationRef) {} fn run_before_analysis(&mut self, name: &str, id: &TypeId, op: &OperationRef) {} fn run_after_analysis(&mut self, name: &str, id: &TypeId, op: &OperationRef) {} @@ -54,8 +54,8 @@ impl PassInstrumentation for Box

{ (**self).run_before_pass(pass, op); } - fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { - (**self).run_after_pass(pass, op); + fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef, changed: bool) { + (**self).run_after_pass(pass, op, changed); } fn run_after_pass_failed(&mut self, pass: &dyn OperationPass, op: &OperationRef) { @@ -97,8 +97,8 @@ impl PassInstrumentor { self.instrument(|pi| pi.run_before_pass(pass, op)); } - pub fn run_after_pass(&self, pass: &dyn OperationPass, op: &OperationRef) { - self.instrument(|pi| pi.run_after_pass(pass, op)); + pub fn run_after_pass(&self, pass: &dyn OperationPass, op: &OperationRef, changed: bool) { + self.instrument(|pi| pi.run_after_pass(pass, op, changed)); } pub fn run_after_pass_failed(&self, pass: &dyn OperationPass, op: &OperationRef) { diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 9c0059826..c8f3ce969 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -956,9 +956,9 @@ impl OpToOpPassAdaptor { let mut result = if let Some(adaptor) = pass.as_any_mut().downcast_mut::() { - adaptor.run_on_operation(op, &mut execution_state, verify).map(|_| ()) + adaptor.run_on_operation(op, &mut execution_state, verify) } else { - pass.run_on_operation(op, &mut execution_state).map(|_| ()) + pass.run_on_operation(op, &mut execution_state) }; // Invalidate any non-preserved analyses @@ -976,20 +976,25 @@ impl OpToOpPassAdaptor { // * If the pass said that it preserved all analyses then it can't have permuted the IR let run_verifier_now = !execution_state.preserved_analyses().is_all(); if run_verifier_now { - result = Self::verify(&op, run_verifier_recursively); + result = Self::verify(&op, run_verifier_recursively).map(|_| true); } } if let Some(instrumentor) = pi.as_deref() { + let in_result = if let Ok(result) = result { + result + } else { + false + }; if result.is_err() { instrumentor.run_after_pass_failed(pass, &op); } else { - instrumentor.run_after_pass(pass, &op); + instrumentor.run_after_pass(pass, &op, in_result); } } // Return the pass result - result + result.map(|_| ()) } fn verify(op: &OperationRef, verify_recursively: bool) -> Result<(), Report> { From ec456fc990393290c558986d1026dafc811e78c0 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Wed, 23 Apr 2025 15:53:35 -0300 Subject: [PATCH 16/56] Receive "only print when ir has been modified" as a CLI parameter only_when_modified Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 44 ++++++++++++++++++++++++++----- hir/src/pass/manager.rs | 10 ++++++- midenc-compile/src/compiler.rs | 7 +++++ midenc-session/src/options/mod.rs | 3 +++ 4 files changed, 57 insertions(+), 7 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index adaf203c0..d17091f86 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -27,6 +27,7 @@ pub struct Print { filter: OpFilter, pass_filter: PassFilter, target: Option, + only_when_modified: bool, } /// Filter for the different passes. @@ -61,6 +62,7 @@ impl Print { filter: OpFilter::All, pass_filter: PassFilter::All, target: None, + only_when_modified: false, } } @@ -72,9 +74,13 @@ impl Print { filter: OpFilter::Type { dialect, op }, pass_filter: PassFilter::All, target: None, + only_when_modified: false, } } + // pub fn from_config(config: IRPrintingConfig) { + // if config.print + // } /// Adds a PassFilter to Print. IR will only be printed before and after those passes are /// executed. pub fn with_pass_filter(mut self, passes: Vec) -> Self { @@ -82,12 +88,17 @@ impl Print { self } + pub fn with_only_print_when_modified(&mut self) { + self.only_when_modified = true; + } + /// Create a printer that only prints `Symbol` operations containing `name` pub fn symbol_matching(name: &'static str) -> Self { Self { filter: OpFilter::Symbol(Some(name)), pass_filter: PassFilter::All, target: None, + only_when_modified: false, } } @@ -105,7 +116,7 @@ impl Print { match self.filter { OpFilter::All => { let target = self.target.as_deref().unwrap_or("printer"); - log::trace!(target: target, "{op}"); + log::error!(target: target, "{op}"); } OpFilter::Type { dialect, @@ -114,21 +125,21 @@ impl Print { let name = op.name(); if name.dialect() == dialect && name.name() == op_name { let target = self.target.as_deref().unwrap_or("printer"); - log::trace!(target: target, "{op}"); + log::error!(target: target, "{op}"); } } OpFilter::Symbol(None) => { if let Some(sym) = op.as_symbol() { let name = sym.name().as_str(); let target = self.target.as_deref().unwrap_or(name); - log::trace!(target: target, "{}", sym.as_symbol_operation()); + log::error!(target: target, "{}", sym.as_symbol_operation()); } } OpFilter::Symbol(Some(filter)) => { if let Some(sym) = op.as_symbol().filter(|sym| sym.name().as_str().contains(filter)) { let target = self.target.as_deref().unwrap_or(filter); - log::trace!(target: target, "{}", sym.as_symbol_operation()); + log::error!(target: target, "{}", sym.as_symbol_operation()); } } } @@ -165,14 +176,35 @@ impl Pass for Print { } impl PassInstrumentation for Print { - fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef, _changed: bool) { - if self.should_print(pass) { + fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef, changed: bool) { + std::println!( + " +------------------------------------DESPUES:\ + {}------------------------------------------------------- +", + pass.name() + ); + std::println!("RESULTADO DE CHANGED: {}", changed); + #[allow(clippy::needless_bool)] + // Always print, unless "only_when_modified" has been set and there have not been changes. + let print_when_changed = if self.only_when_modified && !changed { + false + } else { + true + }; + if self.should_print(pass) && print_when_changed { let op = op.borrow(); self.print_ir(op); } } fn run_before_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { + std::println!( + " +------------------------------------ANTES:{}------------------------------------------------------- +", + pass.name() + ); if self.should_print(pass) { let op = op.borrow(); self.print_ir(op); diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index c8f3ce969..7ba8eea1c 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -44,14 +44,17 @@ pub struct IRPrintingConfig { // NOTE: Taken from the Options struct print_ir_after_all: bool, print_ir_after_pass: Vec, + print_ir_after_modified: bool, flags: OpPrintingFlags, } impl From<&Options> for IRPrintingConfig { fn from(options: &Options) -> Self { let mut irprintconfig = IRPrintingConfig::default(); + irprintconfig.print_ir_after_all = options.print_ir_after_all; irprintconfig.print_ir_after_pass = options.print_ir_after_pass.clone(); + irprintconfig.print_ir_after_modified = options.print_ir_after_modified; irprintconfig } @@ -199,7 +202,12 @@ impl PassManager { None }; - if let Some(print) = print { + //TODO: Refactor this + if let Some(mut print) = print { + if config.print_ir_after_modified { + std::dbg!("ONLY WHEN MODIFIED"); + print.with_only_print_when_modified(); + } let print = Box::new(print); self.add_instrumentation(print); } diff --git a/midenc-compile/src/compiler.rs b/midenc-compile/src/compiler.rs index 1a6f27ecd..63a980b2b 100644 --- a/midenc-compile/src/compiler.rs +++ b/midenc-compile/src/compiler.rs @@ -339,6 +339,12 @@ pub struct UnstableOptions { ) )] pub print_ir_after_pass: Vec, + /// Only print the IR if the pass modified the IR structure. + #[cfg_attr( + feature = "std", + arg(long, default_value_t = false, help_heading = "Passes") + )] + pub print_ir_after_modified: bool, } impl CodegenOptions { @@ -505,6 +511,7 @@ impl Compiler { options.print_cfg_after_pass = unstable.print_cfg_after_pass; options.print_ir_after_all = unstable.print_ir_after_all; options.print_ir_after_pass = unstable.print_ir_after_pass; + options.print_ir_after_modified = unstable.print_ir_after_modified; // Establish --target-dir let target_dir = if self.target_dir.is_absolute() { diff --git a/midenc-session/src/options/mod.rs b/midenc-session/src/options/mod.rs index af547aa60..359e21ee7 100644 --- a/midenc-session/src/options/mod.rs +++ b/midenc-session/src/options/mod.rs @@ -52,6 +52,8 @@ pub struct Options { pub print_ir_after_all: bool, /// Print IR to stdout each time the named passes are applied pub print_ir_after_pass: Vec, + /// Only print the IR if the pass modified the IR structure. + pub print_ir_after_modified: bool, /// Save intermediate artifacts in memory during compilation pub save_temps: bool, /// We store any leftover argument matches in the session options for use @@ -126,6 +128,7 @@ impl Options { print_cfg_after_pass: vec![], print_ir_after_all: false, print_ir_after_pass: vec![], + print_ir_after_modified: false, flags: CompileFlags::default(), } } From 56f6b6aadca267a64d6b0c9af0acadbfece69800 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 24 Apr 2025 10:07:18 -0300 Subject: [PATCH 17/56] 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 --- dialects/hir/src/transforms/spill.rs | 8 ++++---- dialects/scf/src/transforms/cfg_to_scf.rs | 8 ++++---- hir-transform/src/canonicalization.rs | 8 ++++---- hir-transform/src/sccp.rs | 8 ++++---- hir-transform/src/sink.rs | 18 ++++++++--------- hir-transform/src/spill.rs | 6 +++--- hir/src/pass.rs | 17 ++++++++++------ hir/src/pass/instrumentation.rs | 24 +++++++++++++++++++---- hir/src/pass/manager.rs | 18 +++++++++-------- hir/src/pass/pass.rs | 24 +++++++++++++++++++---- 10 files changed, 89 insertions(+), 50 deletions(-) diff --git a/dialects/hir/src/transforms/spill.rs b/dialects/hir/src/transforms/spill.rs index 879c6874a..73480e748 100644 --- a/dialects/hir/src/transforms/spill.rs +++ b/dialects/hir/src/transforms/spill.rs @@ -3,7 +3,7 @@ use alloc::rc::Rc; use midenc_hir::{ adt::SmallDenseMap, dialects::builtin::{Function, FunctionRef, LocalVariable}, - pass::{Pass, PassExecutionState}, + pass::{IRAfterPass, Pass, PassExecutionState}, BlockRef, BuilderExt, EntityMut, Op, OpBuilder, OperationName, OperationRef, Report, Rewriter, SourceSpan, Spanned, Symbol, ValueRef, }; @@ -31,14 +31,14 @@ impl Pass for TransformSpills { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { let function = op.into_entity_ref(); log::debug!(target: "insert-spills", "computing and inserting spills for {}", function.as_operation()); if function.is_declaration() { log::debug!(target: "insert-spills", "function has no body, no spills needed!"); state.preserved_analyses_mut().preserve_all(); - return Ok(false); + return Ok(IRAfterPass::Unchanged); } let mut analysis = state.analysis_manager().get_analysis_for::()?; @@ -46,7 +46,7 @@ impl Pass for TransformSpills { if !analysis.has_spills() { log::debug!(target: "insert-spills", "no spills needed!"); state.preserved_analyses_mut().preserve_all(); - return Ok(false); + return Ok(IRAfterPass::Unchanged); } // Take ownership of the spills analysis so that we can mutate the analysis state during diff --git a/dialects/scf/src/transforms/cfg_to_scf.rs b/dialects/scf/src/transforms/cfg_to_scf.rs index fd092d847..5b70b80dc 100644 --- a/dialects/scf/src/transforms/cfg_to_scf.rs +++ b/dialects/scf/src/transforms/cfg_to_scf.rs @@ -7,7 +7,7 @@ use midenc_hir::{ diagnostics::Severity, dialects::builtin, dominance::DominanceInfo, - pass::{Pass, PassExecutionState}, + pass::{IRAfterPass, Pass, PassExecutionState}, Builder, EntityMut, Forward, Op, Operation, OperationName, OperationRef, RawWalk, Report, SmallVec, Spanned, Type, ValueRange, ValueRef, WalkResult, }; @@ -51,7 +51,7 @@ impl Pass for LiftControlFlowToSCF { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { let mut transformation = ControlFlowToSCFTransformation; let mut changed = false; @@ -131,7 +131,7 @@ impl Pass for LiftControlFlowToSCF { if result.was_interrupted() { // TODO: Maybe Change into_result implementation - return result.into_result().map(|_| false); + return result.into_result().map(|_| IRAfterPass::Unchanged); } log::debug!( @@ -142,7 +142,7 @@ impl Pass for LiftControlFlowToSCF { state.preserved_analyses_mut().preserve_all(); } - Ok(changed) + Ok(changed.into()) } } diff --git a/hir-transform/src/canonicalization.rs b/hir-transform/src/canonicalization.rs index d033e4d6a..2da0dc308 100644 --- a/hir-transform/src/canonicalization.rs +++ b/hir-transform/src/canonicalization.rs @@ -1,7 +1,7 @@ use alloc::{boxed::Box, format, rc::Rc}; use midenc_hir::{ - pass::{OperationPass, Pass, PassExecutionState}, + pass::{IRAfterPass, OperationPass, Pass, PassExecutionState}, patterns::{self, FrozenRewritePatternSet, GreedyRewriteConfig, RewritePatternSet}, Context, EntityMut, Operation, OperationName, Report, Spanned, }; @@ -93,10 +93,10 @@ impl Pass for Canonicalizer { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { let Some(rewrites) = self.rewrites.as_ref() else { log::debug!("skipping canonicalization as there are no rewrite patterns to apply"); - return Ok(false); + return Ok(IRAfterPass::Unchanged); }; let op = { let ptr = op.as_operation_ref(); @@ -143,6 +143,6 @@ impl Pass for Canonicalizer { } }; - Ok(changed) + Ok(changed.into()) } } diff --git a/hir-transform/src/sccp.rs b/hir-transform/src/sccp.rs index d7e7c9548..545c23cf6 100644 --- a/hir-transform/src/sccp.rs +++ b/hir-transform/src/sccp.rs @@ -1,5 +1,5 @@ use midenc_hir::{ - pass::{Pass, PassExecutionState}, + pass::{IRAfterPass, Pass, PassExecutionState}, patterns::NoopRewriterListener, BlockRef, Builder, EntityMut, OpBuilder, Operation, OperationFolder, OperationName, RegionList, Report, SmallVec, ValueRef, @@ -38,7 +38,7 @@ impl Pass for SparseConditionalConstantPropagation { &mut self, mut op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { // Run sparse constant propagation + dead code analysis let mut solver = DataFlowSolver::default(); solver.load::(); @@ -58,7 +58,7 @@ impl SparseConditionalConstantPropagation { op: &mut Operation, _state: &mut PassExecutionState, solver: &DataFlowSolver, - ) -> Result { + ) -> Result { let mut worklist = SmallVec::<[BlockRef; 8]>::default(); let add_to_worklist = |regions: &RegionList, worklist: &mut SmallVec<[BlockRef; 8]>| { @@ -124,7 +124,7 @@ impl SparseConditionalConstantPropagation { } } - Ok(replaced_any) + Ok(replaced_any.into()) } } diff --git a/hir-transform/src/sink.rs b/hir-transform/src/sink.rs index defaa91dc..155af49d9 100644 --- a/hir-transform/src/sink.rs +++ b/hir-transform/src/sink.rs @@ -4,7 +4,7 @@ use midenc_hir::{ adt::SmallDenseMap, dominance::DominanceInfo, matchers::{self, Matcher}, - pass::{Pass, PassExecutionState}, + pass::{IRAfterPass, Pass, PassExecutionState}, traits::{ConstantLike, Terminator}, Backward, Builder, EntityMut, Forward, FxHashSet, OpBuilder, Operation, OperationName, OperationRef, ProgramPoint, RawWalk, Region, RegionBranchOpInterface, @@ -96,7 +96,7 @@ impl Pass for ControlFlowSink { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { let op = op.into_entity_ref(); log::debug!(target: "control-flow-sink", "sinking operations in {op}"); @@ -105,7 +105,7 @@ impl Pass for ControlFlowSink { let dominfo = state.analysis_manager().get_analysis::()?; - let mut sunk = false; + let mut sunk = IRAfterPass::Unchanged; operation.raw_prewalk_all::(|op: OperationRef| { let regions_to_sink = { let op = op.borrow(); @@ -173,7 +173,7 @@ impl Pass for SinkOperandDefs { &mut self, op: EntityMut<'_, Self::Target>, _state: &mut PassExecutionState, - ) -> Result { + ) -> Result { let operation = op.as_operation_ref(); drop(op); @@ -185,7 +185,7 @@ impl Pass for SinkOperandDefs { // then process the worklist, moving everything into position. let mut worklist = alloc::collections::VecDeque::default(); - let mut changed = false; + let mut changed = IRAfterPass::Unchanged; // Visit ops in "true" post-order (i.e. block bodies are visited bottom-up). operation.raw_postwalk_all::(|operation: OperationRef| { // Determine if any of this operation's operands represent one of the following: @@ -310,7 +310,7 @@ impl Pass for SinkOperandDefs { log::trace!(target: "sink-operand-defs", " rewriting operand {operand_value} as {replacement}"); operand.borrow_mut().set(replacement); - changed = true; + changed = IRAfterPass::Changed; // If no other uses of this value remain, then remove the original // operation, as it is now dead. if !operand_value.borrow().is_used() { @@ -357,7 +357,7 @@ impl Pass for SinkOperandDefs { log::trace!(target: "sink-operand-defs", " rewriting operand {operand_value} as {replacement}"); sink_state.replacements.insert(operand_value, replacement); operand.borrow_mut().set(replacement); - changed = true; + changed = IRAfterPass::Changed; } else { log::trace!(target: "sink-operand-defs", " defining op is a constant with no other uses, moving into place"); // The original op can be moved @@ -552,14 +552,14 @@ pub fn control_flow_sink( dominfo: &DominanceInfo, should_move_into_region: P, move_into_region: F, -) -> bool +) -> IRAfterPass where P: Fn(&Operation, &Region) -> bool, F: Fn(OperationRef, RegionRef), { let sinker = Sinker::new(dominfo, should_move_into_region, move_into_region); let sunk_regions = sinker.sink_regions(regions); - sunk_regions > 0 + (sunk_regions > 0).into() } /// Populates `regions` with regions of the provided region branch op that are executed at most once diff --git a/hir-transform/src/spill.rs b/hir-transform/src/spill.rs index 579ead8dd..9e6398ca2 100644 --- a/hir-transform/src/spill.rs +++ b/hir-transform/src/spill.rs @@ -4,7 +4,7 @@ use midenc_hir::{ adt::{SmallDenseMap, SmallSet}, cfg::Graph, dominance::{DomTreeNode, DominanceFrontier, DominanceInfo}, - pass::AnalysisManager, + pass::{AnalysisManager, IRAfterPass}, traits::SingleRegion, BlockRef, Builder, Context, FxHashMap, OpBuilder, OpOperand, Operation, OperationRef, ProgramPoint, Region, RegionBranchOpInterface, RegionBranchPoint, RegionRef, Report, Rewriter, @@ -113,7 +113,7 @@ pub fn transform_spills( analysis: &mut SpillAnalysis, interface: &mut dyn TransformSpillsInterface, analysis_manager: AnalysisManager, -) -> Result { +) -> Result { assert!( op.borrow().implements::(), "the spills transformation is not supported when the root op is multi-region" @@ -253,7 +253,7 @@ pub fn transform_spills( } // QUESTION: If the code has reached this point, then, in theory, it must've gotten changed. Right? Requires double check. - Ok(true) + Ok(IRAfterPass::Changed) } fn rewrite_single_block_spills( diff --git a/hir/src/pass.rs b/hir/src/pass.rs index d17091f86..9b4cdcda6 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -11,7 +11,7 @@ pub use self::{ analysis::{Analysis, AnalysisManager, OperationAnalysis, PreservedAnalyses}, instrumentation::{PassInstrumentation, PassInstrumentor, PipelineParentInfo}, manager::{IRPrintingConfig, Nesting, OpPassManager, PassDisplayMode, PassManager}, - pass::{OperationPass, Pass, PassExecutionState}, + pass::{IRAfterPass, OperationPass, Pass, PassExecutionState}, registry::{PassInfo, PassPipelineInfo}, specialization::PassTarget, statistics::{PassStatistic, Statistic, StatisticValue}, @@ -168,15 +168,20 @@ impl Pass for Print { &mut self, op: crate::EntityMut<'_, Self::Target>, _state: &mut PassExecutionState, - ) -> Result { + ) -> Result { let op = op.into_entity_ref(); self.print_ir(op); - Ok(false) + Ok(IRAfterPass::Unchanged) } } impl PassInstrumentation for Print { - fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef, changed: bool) { + fn run_after_pass( + &mut self, + pass: &dyn OperationPass, + op: &OperationRef, + changed: IRAfterPass, + ) { std::println!( " ------------------------------------DESPUES:\ @@ -184,10 +189,10 @@ impl PassInstrumentation for Print { ", pass.name() ); - std::println!("RESULTADO DE CHANGED: {}", changed); + // std::println!("RESULTADO DE CHANGED: {}", changed); #[allow(clippy::needless_bool)] // Always print, unless "only_when_modified" has been set and there have not been changes. - let print_when_changed = if self.only_when_modified && !changed { + let print_when_changed = if self.only_when_modified && changed == IRAfterPass::Unchanged { false } else { true diff --git a/hir/src/pass/instrumentation.rs b/hir/src/pass/instrumentation.rs index 73affd42e..09d33e2bc 100644 --- a/hir/src/pass/instrumentation.rs +++ b/hir/src/pass/instrumentation.rs @@ -5,7 +5,7 @@ use compact_str::CompactString; use smallvec::SmallVec; use super::OperationPass; -use crate::{OperationName, OperationRef}; +use crate::{pass::IRAfterPass, OperationName, OperationRef}; #[allow(unused_variables)] pub trait PassInstrumentation { @@ -22,7 +22,13 @@ pub trait PassInstrumentation { ) { } fn run_before_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) {} - fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef, changed: bool) {} + fn run_after_pass( + &mut self, + pass: &dyn OperationPass, + op: &OperationRef, + changed: IRAfterPass, + ) { + } fn run_after_pass_failed(&mut self, pass: &dyn OperationPass, op: &OperationRef) {} fn run_before_analysis(&mut self, name: &str, id: &TypeId, op: &OperationRef) {} fn run_after_analysis(&mut self, name: &str, id: &TypeId, op: &OperationRef) {} @@ -54,7 +60,12 @@ impl PassInstrumentation for Box

{ (**self).run_before_pass(pass, op); } - fn run_after_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef, changed: bool) { + fn run_after_pass( + &mut self, + pass: &dyn OperationPass, + op: &OperationRef, + changed: IRAfterPass, + ) { (**self).run_after_pass(pass, op, changed); } @@ -97,7 +108,12 @@ impl PassInstrumentor { self.instrument(|pi| pi.run_before_pass(pass, op)); } - pub fn run_after_pass(&self, pass: &dyn OperationPass, op: &OperationRef, changed: bool) { + pub fn run_after_pass( + &self, + pass: &dyn OperationPass, + op: &OperationRef, + changed: IRAfterPass, + ) { self.instrument(|pi| pi.run_after_pass(pass, op, changed)); } diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 7ba8eea1c..df5f0e43f 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -16,8 +16,10 @@ use super::{ PassInstrumentor, PipelineParentInfo, Statistic, }; use crate::{ - pass::Print, traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, - Operation, OperationName, OperationRef, Report, + pass::{IRAfterPass, Print}, + traits::IsolatedFromAbove, + Context, EntityMut, OpPrintingFlags, OpRegistration, Operation, OperationName, OperationRef, + Report, }; #[derive(Debug, Default, Copy, Clone, PartialEq, Eq)] @@ -205,7 +207,6 @@ impl PassManager { //TODO: Refactor this if let Some(mut print) = print { if config.print_ir_after_modified { - std::dbg!("ONLY WHEN MODIFIED"); print.with_only_print_when_modified(); } let print = Box::new(print); @@ -984,7 +985,8 @@ impl OpToOpPassAdaptor { // * If the pass said that it preserved all analyses then it can't have permuted the IR let run_verifier_now = !execution_state.preserved_analyses().is_all(); if run_verifier_now { - result = Self::verify(&op, run_verifier_recursively).map(|_| true); + result = + Self::verify(&op, run_verifier_recursively).map(|_| IRAfterPass::Unchanged); } } @@ -992,7 +994,7 @@ impl OpToOpPassAdaptor { let in_result = if let Ok(result) = result { result } else { - false + IRAfterPass::Unchanged }; if result.is_err() { instrumentor.run_after_pass_failed(pass, &op); @@ -1019,7 +1021,7 @@ impl OpToOpPassAdaptor { op: OperationRef, state: &mut PassExecutionState, verify: bool, - ) -> Result { + ) -> Result { let analysis_manager = state.analysis_manager(); let instrumentor = analysis_manager.pass_instrumentor(); let parent_info = PipelineParentInfo { @@ -1054,7 +1056,7 @@ impl OpToOpPassAdaptor { } } - Ok(false) + Ok(IRAfterPass::Unchanged) } } @@ -1091,7 +1093,7 @@ impl Pass for OpToOpPassAdaptor { &mut self, _op: EntityMut<'_, Operation>, _state: &mut PassExecutionState, - ) -> Result { + ) -> Result { unreachable!("unexpected call to `Pass::run_on_operation` for OpToOpPassAdaptor") } } diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index 880e2040c..ea83a8e7a 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -44,7 +44,7 @@ pub trait OperationPass { &mut self, op: OperationRef, state: &mut PassExecutionState, - ) -> Result; + ) -> Result; fn run_pipeline( &mut self, pipeline: &mut OpPassManager, @@ -121,7 +121,7 @@ where &mut self, mut op: OperationRef, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { let op = <

::Target as PassTarget>::into_target_mut(&mut op);

::run_on_operation(self, op, state) } @@ -136,6 +136,22 @@ where } } +#[derive(Debug, PartialEq, Clone, Copy)] +pub enum IRAfterPass { + Unchanged, + Changed, +} + +// NOTE: I am not sure if using a From impl +impl From for IRAfterPass { + fn from(ir_was_changed: bool) -> Self { + match ir_was_changed { + true => IRAfterPass::Changed, + false => IRAfterPass::Unchanged, + } + } +} + /// A compiler pass which operates on an [Operation] of some kind. #[allow(unused_variables)] pub trait Pass: Sized + Any { @@ -231,7 +247,7 @@ pub trait Pass: Sized + Any { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result; + ) -> Result; /// Schedule an arbitrary pass pipeline on the provided operation. /// /// This can be invoke any time in a pass to dynamic schedule more passes. The provided @@ -330,7 +346,7 @@ where &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { (**self).run_on_operation(op, state) } From 877da4a67cf0f6ab451b1efeeb3d4e73f5d82c26 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 24 Apr 2025 11:32:55 -0300 Subject: [PATCH 18/56] Remove the Pass implementation of the Print pass. Functionality moved to the PassInstrumentation implementation Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 9b4cdcda6..9f41db952 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -153,28 +153,6 @@ impl Print { } } -impl Pass for Print { - type Target = crate::Operation; - - fn name(&self) -> &'static str { - "print" - } - - fn can_schedule_on(&self, _name: &crate::OperationName) -> bool { - true - } - - fn run_on_operation( - &mut self, - op: crate::EntityMut<'_, Self::Target>, - _state: &mut PassExecutionState, - ) -> Result { - let op = op.into_entity_ref(); - self.print_ir(op); - Ok(IRAfterPass::Unchanged) - } -} - impl PassInstrumentation for Print { fn run_after_pass( &mut self, From 242d6db904558c958559dd5cbbc0a826a32b4252 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 24 Apr 2025 11:54:32 -0300 Subject: [PATCH 19/56] Remove debug prints. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 9f41db952..0bf52b8b0 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -160,15 +160,6 @@ impl PassInstrumentation for Print { op: &OperationRef, changed: IRAfterPass, ) { - std::println!( - " -------------------------------------DESPUES:\ - {}------------------------------------------------------- -", - pass.name() - ); - // std::println!("RESULTADO DE CHANGED: {}", changed); - #[allow(clippy::needless_bool)] // Always print, unless "only_when_modified" has been set and there have not been changes. let print_when_changed = if self.only_when_modified && changed == IRAfterPass::Unchanged { false @@ -182,12 +173,6 @@ impl PassInstrumentation for Print { } fn run_before_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { - std::println!( - " -------------------------------------ANTES:{}------------------------------------------------------- -", - pass.name() - ); if self.should_print(pass) { let op = op.borrow(); self.print_ir(op); From 06464cf5e85b2018887972c617203975ecd0633e Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 24 Apr 2025 12:08:36 -0300 Subject: [PATCH 20/56] Remove duplicate struct member from IRPrintConfig and clean up from Option print_after_only_on_change is the same configuration as print_ir_after_modification. I removed the former simply because the latter has the same format as the other print_ir flags. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass/manager.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index df5f0e43f..2c8914e39 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -41,7 +41,6 @@ pub enum PassDisplayMode { #[derive(Default, Debug)] pub struct IRPrintingConfig { print_module_scope: bool, - print_after_only_on_change: bool, print_after_only_on_failure: bool, // NOTE: Taken from the Options struct print_ir_after_all: bool, @@ -52,13 +51,12 @@ pub struct IRPrintingConfig { impl From<&Options> for IRPrintingConfig { fn from(options: &Options) -> Self { - let mut irprintconfig = IRPrintingConfig::default(); - - irprintconfig.print_ir_after_all = options.print_ir_after_all; - irprintconfig.print_ir_after_pass = options.print_ir_after_pass.clone(); - irprintconfig.print_ir_after_modified = options.print_ir_after_modified; - - irprintconfig + IRPrintingConfig { + print_ir_after_all: options.print_ir_after_all, + print_ir_after_pass: options.print_ir_after_pass.clone(), + print_ir_after_modified: options.print_ir_after_modified, + ..Default::default() + } } } From df7f506b3b1b954887308f83d628fd18aec6ef35 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 24 Apr 2025 12:23:25 -0300 Subject: [PATCH 21/56] Clean up should_print functionality in Print::run_after_pass function call Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 0bf52b8b0..4f78a6127 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -145,12 +145,22 @@ impl Print { } } - fn should_print(&self, pass: &dyn OperationPass) -> bool { + fn pass_filter(&self, pass: &dyn OperationPass) -> bool { match &self.pass_filter { PassFilter::All => true, PassFilter::Certain(passes) => passes.iter().any(|p| p == pass.name()), } } + + fn should_print(&self, pass: &dyn OperationPass, ir_changed: IRAfterPass) -> bool { + let pass_filter = self.pass_filter(pass); + + // Always print, unless "only_when_modified" has been set and there have not been changes. + let modification_filter = + !matches!((ir_changed, self.only_when_modified), (IRAfterPass::Unchanged, true)); + + pass_filter && modification_filter + } } impl PassInstrumentation for Print { @@ -160,20 +170,14 @@ impl PassInstrumentation for Print { op: &OperationRef, changed: IRAfterPass, ) { - // Always print, unless "only_when_modified" has been set and there have not been changes. - let print_when_changed = if self.only_when_modified && changed == IRAfterPass::Unchanged { - false - } else { - true - }; - if self.should_print(pass) && print_when_changed { + if self.should_print(pass, changed) { let op = op.borrow(); self.print_ir(op); } } fn run_before_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { - if self.should_print(pass) { + if self.pass_filter(pass) { let op = op.borrow(); self.print_ir(op); } From 06527ecf12c50f345984d4c5b7061c3e087ab7b7 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 24 Apr 2025 14:18:36 -0300 Subject: [PATCH 22/56] Add an OperationRef to `PassInstrumentation::run_before_pipeline` Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 13 ++++++++++++- hir/src/pass/instrumentation.rs | 7 +++++-- hir/src/pass/manager.rs | 2 +- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 4f78a6127..a20676861 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -18,7 +18,7 @@ pub use self::{ }; use crate::{ alloc::{string::String, vec::Vec}, - EntityRef, Operation, OperationRef, + EntityRef, Operation, OperationName, OperationRef, }; /// A `Pass` which prints IR it is run on, based on provided configuration. @@ -182,4 +182,15 @@ impl PassInstrumentation for Print { self.print_ir(op); } } + + fn run_before_pipeline( + &mut self, + _name: Option<&OperationName>, + _parent_info: &PipelineParentInfo, + op: OperationRef, + ) { + std::dbg!("BEFORE THE PIPELINE"); + let op = op.borrow(); + self.print_ir(op); + } } diff --git a/hir/src/pass/instrumentation.rs b/hir/src/pass/instrumentation.rs index 09d33e2bc..c612bf655 100644 --- a/hir/src/pass/instrumentation.rs +++ b/hir/src/pass/instrumentation.rs @@ -13,6 +13,7 @@ pub trait PassInstrumentation { &mut self, name: Option<&OperationName>, parent_info: &PipelineParentInfo, + op: OperationRef, ) { } fn run_after_pipeline( @@ -44,8 +45,9 @@ impl PassInstrumentation for Box

{ &mut self, name: Option<&OperationName>, parent_info: &PipelineParentInfo, + op: OperationRef, ) { - (**self).run_before_pipeline(name, parent_info); + (**self).run_before_pipeline(name, parent_info, op); } fn run_after_pipeline( @@ -92,8 +94,9 @@ impl PassInstrumentor { &self, name: Option<&OperationName>, parent_info: &PipelineParentInfo, + op: OperationRef, ) { - self.instrument(|pi| pi.run_before_pipeline(name, parent_info)); + self.instrument(|pi| pi.run_before_pipeline(name, parent_info, op)); } pub fn run_after_pipeline( diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 2c8914e39..026683cc6 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -859,7 +859,7 @@ impl OpToOpPassAdaptor { let mut op_name = None; if let Some(instrumentor) = instrumentor.as_deref() { op_name = pm.name().cloned(); - instrumentor.run_before_pipeline(op_name.as_ref(), parent_info.as_ref().unwrap()); + instrumentor.run_before_pipeline(op_name.as_ref(), parent_info.as_ref().unwrap(), op); } for pass in pm.passes_mut() { From b758cb1d6ab1cd9469292dc765e8ac31bb07c403 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 24 Apr 2025 14:45:27 -0300 Subject: [PATCH 23/56] Print the name of the pass before the IR when executing run_*_pass Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index a20676861..b2c33b9f8 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -164,31 +164,39 @@ impl Print { } impl PassInstrumentation for Print { + fn run_before_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { + if self.only_when_modified { + return; + } + log::error!("Before the {} pass", pass.name()); + if self.pass_filter(pass) { + let op = op.borrow(); + self.print_ir(op); + } + } + fn run_after_pass( &mut self, pass: &dyn OperationPass, op: &OperationRef, changed: IRAfterPass, ) { + log::error!("After the {} pass", pass.name()); if self.should_print(pass, changed) { let op = op.borrow(); self.print_ir(op); } } - fn run_before_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { - if self.pass_filter(pass) { - let op = op.borrow(); - self.print_ir(op); - } - } - fn run_before_pipeline( &mut self, _name: Option<&OperationName>, _parent_info: &PipelineParentInfo, op: OperationRef, ) { + if !self.only_when_modified { + return; + } std::dbg!("BEFORE THE PIPELINE"); let op = op.borrow(); self.print_ir(op); From cfa6fb0cc17bf2a7fdee28ca7b9f4b6285ae7a15 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 24 Apr 2025 15:42:21 -0300 Subject: [PATCH 24/56] Created PassType enum. One variant for each pass. Signed-off-by: Tomas Fabrizio Orsi --- dialects/hir/src/transforms/spill.rs | 6 +++++- dialects/scf/src/transforms/cfg_to_scf.rs | 6 +++++- hir-transform/src/canonicalization.rs | 6 +++++- hir-transform/src/sccp.rs | 6 +++++- hir-transform/src/sink.rs | 10 +++++++++- hir/src/pass.rs | 3 ++- hir/src/pass/manager.rs | 11 ++++++++++- hir/src/pass/pass.rs | 16 ++++++++++++++++ 8 files changed, 57 insertions(+), 7 deletions(-) diff --git a/dialects/hir/src/transforms/spill.rs b/dialects/hir/src/transforms/spill.rs index 73480e748..1c364c759 100644 --- a/dialects/hir/src/transforms/spill.rs +++ b/dialects/hir/src/transforms/spill.rs @@ -3,7 +3,7 @@ use alloc::rc::Rc; use midenc_hir::{ adt::SmallDenseMap, dialects::builtin::{Function, FunctionRef, LocalVariable}, - pass::{IRAfterPass, Pass, PassExecutionState}, + pass::{pass::PassType, IRAfterPass, Pass, PassExecutionState}, BlockRef, BuilderExt, EntityMut, Op, OpBuilder, OperationName, OperationRef, Report, Rewriter, SourceSpan, Spanned, Symbol, ValueRef, }; @@ -19,6 +19,10 @@ impl Pass for TransformSpills { "transform-spills" } + fn pass_type(&self) -> Option { + Some(PassType::TransformSpills) + } + fn argument(&self) -> &'static str { "transform-spills" } diff --git a/dialects/scf/src/transforms/cfg_to_scf.rs b/dialects/scf/src/transforms/cfg_to_scf.rs index 5b70b80dc..c501cc71f 100644 --- a/dialects/scf/src/transforms/cfg_to_scf.rs +++ b/dialects/scf/src/transforms/cfg_to_scf.rs @@ -7,7 +7,7 @@ use midenc_hir::{ diagnostics::Severity, dialects::builtin, dominance::DominanceInfo, - pass::{IRAfterPass, Pass, PassExecutionState}, + pass::{pass::PassType, IRAfterPass, Pass, PassExecutionState}, Builder, EntityMut, Forward, Op, Operation, OperationName, OperationRef, RawWalk, Report, SmallVec, Spanned, Type, ValueRange, ValueRef, WalkResult, }; @@ -35,6 +35,10 @@ impl Pass for LiftControlFlowToSCF { "lift-control-flow" } + fn pass_type(&self) -> Option { + Some(PassType::LiftControlFlowToSCF) + } + fn argument(&self) -> &'static str { "lift-control-flow" } diff --git a/hir-transform/src/canonicalization.rs b/hir-transform/src/canonicalization.rs index 2da0dc308..6044aa261 100644 --- a/hir-transform/src/canonicalization.rs +++ b/hir-transform/src/canonicalization.rs @@ -1,7 +1,7 @@ use alloc::{boxed::Box, format, rc::Rc}; use midenc_hir::{ - pass::{IRAfterPass, OperationPass, Pass, PassExecutionState}, + pass::{pass::PassType, IRAfterPass, OperationPass, Pass, PassExecutionState}, patterns::{self, FrozenRewritePatternSet, GreedyRewriteConfig, RewritePatternSet}, Context, EntityMut, Operation, OperationName, Report, Spanned, }; @@ -62,6 +62,10 @@ impl Pass for Canonicalizer { "canonicalizer" } + fn pass_type(&self) -> Option { + Some(PassType::Canonicalizer) + } + fn argument(&self) -> &'static str { "canonicalizer" } diff --git a/hir-transform/src/sccp.rs b/hir-transform/src/sccp.rs index 545c23cf6..1fb0efac1 100644 --- a/hir-transform/src/sccp.rs +++ b/hir-transform/src/sccp.rs @@ -1,5 +1,5 @@ use midenc_hir::{ - pass::{IRAfterPass, Pass, PassExecutionState}, + pass::{pass::PassType, IRAfterPass, Pass, PassExecutionState}, patterns::NoopRewriterListener, BlockRef, Builder, EntityMut, OpBuilder, Operation, OperationFolder, OperationName, RegionList, Report, SmallVec, ValueRef, @@ -26,6 +26,10 @@ impl Pass for SparseConditionalConstantPropagation { "sparse-conditional-constant-propagation" } + fn pass_type(&self) -> Option { + Some(PassType::SparseConditionalConstantPropagation) + } + fn argument(&self) -> &'static str { "sparse-conditional-constant-propagation" } diff --git a/hir-transform/src/sink.rs b/hir-transform/src/sink.rs index 155af49d9..c4d6fec55 100644 --- a/hir-transform/src/sink.rs +++ b/hir-transform/src/sink.rs @@ -4,7 +4,7 @@ use midenc_hir::{ adt::SmallDenseMap, dominance::DominanceInfo, matchers::{self, Matcher}, - pass::{IRAfterPass, Pass, PassExecutionState}, + pass::{pass::PassType, IRAfterPass, Pass, PassExecutionState}, traits::{ConstantLike, Terminator}, Backward, Builder, EntityMut, Forward, FxHashSet, OpBuilder, Operation, OperationName, OperationRef, ProgramPoint, RawWalk, Region, RegionBranchOpInterface, @@ -84,6 +84,10 @@ impl Pass for ControlFlowSink { "control-flow-sink" } + fn pass_type(&self) -> Option { + Some(PassType::ControlFlowSink) + } + fn argument(&self) -> &'static str { "control-flow-sink" } @@ -161,6 +165,10 @@ impl Pass for SinkOperandDefs { "sink-operand-defs" } + fn pass_type(&self) -> Option { + Some(PassType::SinkOperandDefs) + } + fn argument(&self) -> &'static str { "sink-operand-defs" } diff --git a/hir/src/pass.rs b/hir/src/pass.rs index b2c33b9f8..c9807ab41 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -1,8 +1,9 @@ mod analysis; mod instrumentation; mod manager; +/// Made public momentarily. #[allow(clippy::module_inception)] -mod pass; +pub mod pass; pub mod registry; mod specialization; pub mod statistics; diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 026683cc6..57f172ac6 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -16,7 +16,7 @@ use super::{ PassInstrumentor, PipelineParentInfo, Statistic, }; use crate::{ - pass::{IRAfterPass, Print}, + pass::{pass::PassType, IRAfterPass, Print}, traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, Operation, OperationName, OperationRef, Report, @@ -746,6 +746,11 @@ impl OpToOpPassAdaptor { Self { pms: smallvec![pm] } } + #[allow(dead_code)] + fn pass_type(&self) -> Option { + None + } + pub fn name(&self) -> CompactString { use core::fmt::Write; @@ -1065,6 +1070,10 @@ impl Pass for OpToOpPassAdaptor { crate::interner::Symbol::intern(self.name()).as_str() } + fn pass_type(&self) -> Option { + None + } + #[inline(always)] fn target_name(&self, _context: &Context) -> Option { None diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index ea83a8e7a..3345192f9 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -136,6 +136,16 @@ where } } +pub enum PassType { + Canonicalizer, + ControlFlowSink, + LiftControlFlowToSCF, + OpToOpPassAdaptor, + SinkOperandDefs, + SparseConditionalConstantPropagation, + TransformSpills, +} + #[derive(Debug, PartialEq, Clone, Copy)] pub enum IRAfterPass { Unchanged, @@ -260,6 +270,8 @@ pub trait Pass: Sized + Any { ) -> Result<(), Report> { state.run_pipeline(pipeline, op) } + + fn pass_type(&self) -> Option; } impl

Pass for Box

@@ -286,6 +298,10 @@ where (**self).name() } + fn pass_type(&self) -> Option { + (**self).pass_type() + } + #[inline] fn argument(&self) -> &'static str { (**self).argument() From 59550949b86beec2ea0bb07937570bdaf081128d Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 24 Apr 2025 17:18:08 -0300 Subject: [PATCH 25/56] Make the PassFilter store PassTypes instead of strings, use that for pass comparison Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 27 ++++++++++++++++++--------- hir/src/pass/manager.rs | 21 +++++++++------------ hir/src/pass/pass.rs | 24 ++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 21 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index c9807ab41..6386ecb92 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -12,7 +12,7 @@ pub use self::{ analysis::{Analysis, AnalysisManager, OperationAnalysis, PreservedAnalyses}, instrumentation::{PassInstrumentation, PassInstrumentor, PipelineParentInfo}, manager::{IRPrintingConfig, Nesting, OpPassManager, PassDisplayMode, PassManager}, - pass::{IRAfterPass, OperationPass, Pass, PassExecutionState}, + pass::{IRAfterPass, OperationPass, Pass, PassExecutionState, PassType}, registry::{PassInfo, PassPipelineInfo}, specialization::PassTarget, statistics::{PassStatistic, Statistic, StatisticValue}, @@ -38,7 +38,7 @@ enum PassFilter { #[default] All, /// Only print IR if the pass's name is present in the vector. - Certain(Vec), + Certain(Vec), } #[derive(Default, Debug)] @@ -84,7 +84,7 @@ impl Print { // } /// Adds a PassFilter to Print. IR will only be printed before and after those passes are /// executed. - pub fn with_pass_filter(mut self, passes: Vec) -> Self { + pub fn with_pass_filter(mut self, passes: Vec) -> Self { self.pass_filter = PassFilter::Certain(passes); self } @@ -149,7 +149,13 @@ impl Print { fn pass_filter(&self, pass: &dyn OperationPass) -> bool { match &self.pass_filter { PassFilter::All => true, - PassFilter::Certain(passes) => passes.iter().any(|p| p == pass.name()), + PassFilter::Certain(passes) => passes.iter().any(|p| { + if let Some(p_type) = pass.pass_type() { + *p == p_type + } else { + false + } + }), } } @@ -157,8 +163,10 @@ impl Print { let pass_filter = self.pass_filter(pass); // Always print, unless "only_when_modified" has been set and there have not been changes. - let modification_filter = - !matches!((ir_changed, self.only_when_modified), (IRAfterPass::Unchanged, true)); + let modification_filter = match (self.only_when_modified, ir_changed) { + (true, IRAfterPass::Unchanged) => false, + _ => true, + }; pass_filter && modification_filter } @@ -169,8 +177,8 @@ impl PassInstrumentation for Print { if self.only_when_modified { return; } - log::error!("Before the {} pass", pass.name()); if self.pass_filter(pass) { + log::error!("Before the {} pass", pass.name()); let op = op.borrow(); self.print_ir(op); } @@ -182,8 +190,8 @@ impl PassInstrumentation for Print { op: &OperationRef, changed: IRAfterPass, ) { - log::error!("After the {} pass", pass.name()); if self.should_print(pass, changed) { + log::error!("After the {} pass", pass.name()); let op = op.borrow(); self.print_ir(op); } @@ -198,7 +206,8 @@ impl PassInstrumentation for Print { if !self.only_when_modified { return; } - std::dbg!("BEFORE THE PIPELINE"); + + log::error!("IR before the pass pipeline"); let op = op.borrow(); self.print_ir(op); } diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 57f172ac6..2faded75d 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -1,11 +1,4 @@ -use alloc::{ - boxed::Box, - collections::BTreeMap, - format, - rc::Rc, - string::{String, ToString}, - vec::Vec, -}; +use alloc::{boxed::Box, collections::BTreeMap, format, rc::Rc, string::ToString, vec::Vec}; use compact_str::{CompactString, ToCompactString}; use midenc_session::{diagnostics::Severity, Options}; @@ -44,16 +37,18 @@ pub struct IRPrintingConfig { print_after_only_on_failure: bool, // NOTE: Taken from the Options struct print_ir_after_all: bool, - print_ir_after_pass: Vec, + print_ir_after_pass: Vec, print_ir_after_modified: bool, flags: OpPrintingFlags, } impl From<&Options> for IRPrintingConfig { fn from(options: &Options) -> Self { + let pass_filters: Vec = + options.print_ir_after_pass.iter().map(|a| a.into()).collect(); IRPrintingConfig { print_ir_after_all: options.print_ir_after_all, - print_ir_after_pass: options.print_ir_after_pass.clone(), + print_ir_after_pass: pass_filters, print_ir_after_modified: options.print_ir_after_modified, ..Default::default() } @@ -987,9 +982,11 @@ impl OpToOpPassAdaptor { // // * If the pass said that it preserved all analyses then it can't have permuted the IR let run_verifier_now = !execution_state.preserved_analyses().is_all(); + if run_verifier_now { - result = - Self::verify(&op, run_verifier_recursively).map(|_| IRAfterPass::Unchanged); + if let Err(verification_result) = Self::verify(&op, run_verifier_recursively) { + result = result.map_err(|_| verification_result); + } } } diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index 3345192f9..b9e3632d1 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -15,6 +15,9 @@ pub trait OperationPass { fn as_any_mut(&mut self) -> &mut dyn Any; fn into_any(self: Box) -> Box; fn name(&self) -> &'static str; + + fn pass_type(&self) -> Option; + fn argument(&self) -> &'static str { // NOTE: Could we compute an argument string from the type name? "" @@ -61,6 +64,10 @@ where

::as_any(self) } + fn pass_type(&self) -> Option { +

::pass_type(self) + } + fn as_any_mut(&mut self) -> &mut dyn Any {

::as_any_mut(self) } @@ -136,6 +143,7 @@ where } } +#[derive(Debug, PartialEq, Clone, Copy)] pub enum PassType { Canonicalizer, ControlFlowSink, @@ -146,6 +154,22 @@ pub enum PassType { TransformSpills, } +impl From<&String> for PassType { + fn from(pass_name: &String) -> Self { + match pass_name.as_str() { + "canonicalizer" => PassType::Canonicalizer, + "control-flow-sink" => PassType::ControlFlowSink, + "lift-control-flow" => PassType::LiftControlFlowToSCF, + "sink-operand-defs" => PassType::SinkOperandDefs, + "sparse-conditional-constant-propagation" => { + PassType::SparseConditionalConstantPropagation + } + "transform-spills" => PassType::TransformSpills, + _ => panic!("ERROR: '{pass_name}' unrecognized pass."), + } + } +} + #[derive(Debug, PartialEq, Clone, Copy)] pub enum IRAfterPass { Unchanged, From 6a7e684b04286cc0fceeb3204151863ee264fad4 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Fri, 25 Apr 2025 11:28:54 -0300 Subject: [PATCH 26/56] First draft of Print creation refactor. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 95 +++++++++++++++++++++-------------------- hir/src/pass/manager.rs | 31 +++++--------- 2 files changed, 58 insertions(+), 68 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 6386ecb92..295b89341 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -25,8 +25,8 @@ use crate::{ /// A `Pass` which prints IR it is run on, based on provided configuration. #[derive(Default)] pub struct Print { - filter: OpFilter, - pass_filter: PassFilter, + filter: Option, + pass_filter: Option, target: Option, only_when_modified: bool, } @@ -57,50 +57,50 @@ enum OpFilter { } impl Print { - /// Create a printer that prints any operation - pub fn any() -> Self { - Self { - filter: OpFilter::All, - pass_filter: PassFilter::All, - target: None, - only_when_modified: false, - } - } - - /// Create a printer that only prints operations of type `T` - pub fn only() -> Self { + // /// Create a printer that prints any operation + // pub fn any() -> Self { + // Self { + // filter: OpFilter::All, + // pass_filter: PassFilter::All, + // target: None, + // only_when_modified: false, + // } + // } + pub fn with_type_filter(mut self) -> Self { let dialect = ::dialect_name(); let op = ::name(); - Self { - filter: OpFilter::Type { dialect, op }, - pass_filter: PassFilter::All, - target: None, - only_when_modified: false, - } + self.filter = Some(OpFilter::Type { dialect, op }); + self } - // pub fn from_config(config: IRPrintingConfig) { - // if config.print - // } - /// Adds a PassFilter to Print. IR will only be printed before and after those passes are - /// executed. - pub fn with_pass_filter(mut self, passes: Vec) -> Self { - self.pass_filter = PassFilter::Certain(passes); + /// Create a printer that only prints `Symbol` operations containing `name` + pub fn with_symbol_matching(mut self, name: &'static str) -> Self { + self.filter = Some(OpFilter::Symbol(Some(name))); + self + } + + pub fn with_all_symbols(mut self) -> Self { + self.filter = Some(OpFilter::All); self } - pub fn with_only_print_when_modified(&mut self) { - self.only_when_modified = true; + pub fn with_no_pass_filter(mut self) -> Self { + self.pass_filter = Some(PassFilter::All); + self } - /// Create a printer that only prints `Symbol` operations containing `name` - pub fn symbol_matching(name: &'static str) -> Self { - Self { - filter: OpFilter::Symbol(Some(name)), - pass_filter: PassFilter::All, - target: None, - only_when_modified: false, + pub fn with_pass_filter(mut self, config: IRPrintingConfig) -> Self { + if config.print_ir_after_all { + self.pass_filter = Some(PassFilter::All); + } else if !config.print_ir_after_pass.is_empty() { + self.pass_filter = Some(PassFilter::Certain(config.print_ir_after_pass)); } + + if config.print_ir_after_modified { + self.only_when_modified = true; + }; + + self } /// Specify the `log` target to write the IR output to. @@ -115,47 +115,49 @@ impl Print { fn print_ir(&self, op: EntityRef<'_, Operation>) { match self.filter { - OpFilter::All => { + Some(OpFilter::All) => { let target = self.target.as_deref().unwrap_or("printer"); log::error!(target: target, "{op}"); } - OpFilter::Type { + Some(OpFilter::Type { dialect, op: op_name, - } => { + }) => { let name = op.name(); if name.dialect() == dialect && name.name() == op_name { let target = self.target.as_deref().unwrap_or("printer"); log::error!(target: target, "{op}"); } } - OpFilter::Symbol(None) => { + Some(OpFilter::Symbol(None)) => { if let Some(sym) = op.as_symbol() { let name = sym.name().as_str(); let target = self.target.as_deref().unwrap_or(name); log::error!(target: target, "{}", sym.as_symbol_operation()); } } - OpFilter::Symbol(Some(filter)) => { + Some(OpFilter::Symbol(Some(filter))) => { if let Some(sym) = op.as_symbol().filter(|sym| sym.name().as_str().contains(filter)) { let target = self.target.as_deref().unwrap_or(filter); log::error!(target: target, "{}", sym.as_symbol_operation()); } } + None => (), } } fn pass_filter(&self, pass: &dyn OperationPass) -> bool { match &self.pass_filter { - PassFilter::All => true, - PassFilter::Certain(passes) => passes.iter().any(|p| { + Some(PassFilter::All) => true, + Some(PassFilter::Certain(passes)) => passes.iter().any(|p| { if let Some(p_type) = pass.pass_type() { *p == p_type } else { false } }), + None => true, } } @@ -163,10 +165,8 @@ impl Print { let pass_filter = self.pass_filter(pass); // Always print, unless "only_when_modified" has been set and there have not been changes. - let modification_filter = match (self.only_when_modified, ir_changed) { - (true, IRAfterPass::Unchanged) => false, - _ => true, - }; + let modification_filter = + !matches!((self.only_when_modified, ir_changed), (true, IRAfterPass::Unchanged)); pass_filter && modification_filter } @@ -190,6 +190,7 @@ impl PassInstrumentation for Print { op: &OperationRef, changed: IRAfterPass, ) { + std::dbg!(changed); if self.should_print(pass, changed) { log::error!("After the {} pass", pass.name()); let op = op.borrow(); diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 2faded75d..2b0ad4edd 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -33,13 +33,13 @@ pub enum PassDisplayMode { #[allow(unused)] #[derive(Default, Debug)] pub struct IRPrintingConfig { - print_module_scope: bool, - print_after_only_on_failure: bool, + pub print_module_scope: bool, + pub print_after_only_on_failure: bool, // NOTE: Taken from the Options struct - print_ir_after_all: bool, - print_ir_after_pass: Vec, - print_ir_after_modified: bool, - flags: OpPrintingFlags, + pub print_ir_after_all: bool, + pub print_ir_after_pass: Vec, + pub print_ir_after_modified: bool, + pub flags: OpPrintingFlags, } impl From<&Options> for IRPrintingConfig { @@ -189,22 +189,11 @@ impl PassManager { } pub fn enable_ir_printing(&mut self, config: IRPrintingConfig) { - let print = if config.print_ir_after_all { - Some(Print::any()) - } else if !config.print_ir_after_pass.is_empty() { - Some(Print::any().with_pass_filter(config.print_ir_after_pass)) - } else { - None - }; + // NOTE: This, I think, is cleaner BUT means that a Print struct is always created and malloced, even if no printing configuration was passed. + let print = Print::default().with_all_symbols().with_pass_filter(config); - //TODO: Refactor this - if let Some(mut print) = print { - if config.print_ir_after_modified { - print.with_only_print_when_modified(); - } - let print = Box::new(print); - self.add_instrumentation(print); - } + let print = Box::new(print); + self.add_instrumentation(print); } pub fn enable_timing(&mut self, yes: bool) -> &mut Self { From fc5fe30c93ae5e67d3ccdbc90f941d408ddd0ebe Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Fri, 25 Apr 2025 11:38:43 -0300 Subject: [PATCH 27/56] Make Print::new return an Option. This avoids creating a Box everytime. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 12 ++++++++++++ hir/src/pass/manager.rs | 11 +++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 295b89341..441b286df 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -57,6 +57,17 @@ enum OpFilter { } impl Print { + pub fn new(config: &IRPrintingConfig) -> Option { + if config.print_ir_after_all + || !config.print_ir_after_pass.is_empty() + || config.print_ir_after_modified + { + Some(Self::default()) + } else { + None + } + } + // /// Create a printer that prints any operation // pub fn any() -> Self { // Self { @@ -96,6 +107,7 @@ impl Print { self.pass_filter = Some(PassFilter::Certain(config.print_ir_after_pass)); } + // QUESTION: What should if the user only specifies "print after modification" but hasn't specified pass? if config.print_ir_after_modified { self.only_when_modified = true; }; diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 2b0ad4edd..ba0b3d4cc 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -189,11 +189,14 @@ impl PassManager { } pub fn enable_ir_printing(&mut self, config: IRPrintingConfig) { - // NOTE: This, I think, is cleaner BUT means that a Print struct is always created and malloced, even if no printing configuration was passed. - let print = Print::default().with_all_symbols().with_pass_filter(config); + let print = Print::new(&config) + .map(|p| p.with_pass_filter(config)) + .map(|p| p.with_all_symbols()); - let print = Box::new(print); - self.add_instrumentation(print); + if let Some(print) = print { + let print = Box::new(print); + self.add_instrumentation(print); + } } pub fn enable_timing(&mut self, yes: bool) -> &mut Self { From b5b5295a11a337ac44a276b193b65b25ffdf1e60 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Fri, 25 Apr 2025 14:38:16 -0300 Subject: [PATCH 28/56] Changed with_all_symbols call to generic with_symbol_filter call Enables expansion later on Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 22 ++++++++++------------ hir/src/pass/manager.rs | 4 ++-- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 441b286df..5e851523a 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -68,15 +68,6 @@ impl Print { } } - // /// Create a printer that prints any operation - // pub fn any() -> Self { - // Self { - // filter: OpFilter::All, - // pass_filter: PassFilter::All, - // target: None, - // only_when_modified: false, - // } - // } pub fn with_type_filter(mut self) -> Self { let dialect = ::dialect_name(); let op = ::name(); @@ -90,7 +81,14 @@ impl Print { self } - pub fn with_all_symbols(mut self) -> Self { + #[allow(unused_mut)] + pub fn with_symbol_filter(mut self, _config: &IRPrintingConfig) -> Self { + // NOTE: At the moment, symbol filtering is not processed by the CLI. However, were it to be + // added, it could be done inside this function + self.with_all_symbols() + } + + fn with_all_symbols(mut self) -> Self { self.filter = Some(OpFilter::All); self } @@ -100,11 +98,11 @@ impl Print { self } - pub fn with_pass_filter(mut self, config: IRPrintingConfig) -> Self { + pub fn with_pass_filter(mut self, config: &IRPrintingConfig) -> Self { if config.print_ir_after_all { self.pass_filter = Some(PassFilter::All); } else if !config.print_ir_after_pass.is_empty() { - self.pass_filter = Some(PassFilter::Certain(config.print_ir_after_pass)); + self.pass_filter = Some(PassFilter::Certain(config.print_ir_after_pass.clone())); } // QUESTION: What should if the user only specifies "print after modification" but hasn't specified pass? diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index ba0b3d4cc..c6762b425 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -190,8 +190,8 @@ impl PassManager { pub fn enable_ir_printing(&mut self, config: IRPrintingConfig) { let print = Print::new(&config) - .map(|p| p.with_pass_filter(config)) - .map(|p| p.with_all_symbols()); + .map(|p| p.with_pass_filter(&config)) + .map(|p| p.with_symbol_filter(&config)); if let Some(print) = print { let print = Box::new(print); From 1c7b9b97d2a2fc41b8e23b0d1bc883d0b4dc3e3b Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Fri, 25 Apr 2025 14:59:42 -0300 Subject: [PATCH 29/56] Remove todos Signed-off-by: Tomas Fabrizio Orsi --- dialects/scf/src/transforms/cfg_to_scf.rs | 1 - hir-transform/src/spill.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/dialects/scf/src/transforms/cfg_to_scf.rs b/dialects/scf/src/transforms/cfg_to_scf.rs index c501cc71f..ca672f0bc 100644 --- a/dialects/scf/src/transforms/cfg_to_scf.rs +++ b/dialects/scf/src/transforms/cfg_to_scf.rs @@ -134,7 +134,6 @@ impl Pass for LiftControlFlowToSCF { }); if result.was_interrupted() { - // TODO: Maybe Change into_result implementation return result.into_result().map(|_| IRAfterPass::Unchanged); } diff --git a/hir-transform/src/spill.rs b/hir-transform/src/spill.rs index 9e6398ca2..6e88e1ca4 100644 --- a/hir-transform/src/spill.rs +++ b/hir-transform/src/spill.rs @@ -252,7 +252,6 @@ pub fn transform_spills( )?; } - // QUESTION: If the code has reached this point, then, in theory, it must've gotten changed. Right? Requires double check. Ok(IRAfterPass::Changed) } From b84386ce6e1e440e833adc11db4a5620776bd33a Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Fri, 25 Apr 2025 15:09:02 -0300 Subject: [PATCH 30/56] Move `Print::run_before_pipeline` up Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 5e851523a..a274afb57 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -183,6 +183,21 @@ impl Print { } impl PassInstrumentation for Print { + fn run_before_pipeline( + &mut self, + _name: Option<&OperationName>, + _parent_info: &PipelineParentInfo, + op: OperationRef, + ) { + if !self.only_when_modified { + return; + } + + log::error!("IR before the pass pipeline"); + let op = op.borrow(); + self.print_ir(op); + } + fn run_before_pass(&mut self, pass: &dyn OperationPass, op: &OperationRef) { if self.only_when_modified { return; @@ -200,26 +215,10 @@ impl PassInstrumentation for Print { op: &OperationRef, changed: IRAfterPass, ) { - std::dbg!(changed); if self.should_print(pass, changed) { log::error!("After the {} pass", pass.name()); let op = op.borrow(); self.print_ir(op); } } - - fn run_before_pipeline( - &mut self, - _name: Option<&OperationName>, - _parent_info: &PipelineParentInfo, - op: OperationRef, - ) { - if !self.only_when_modified { - return; - } - - log::error!("IR before the pass pipeline"); - let op = op.borrow(); - self.print_ir(op); - } } From f51e6c16fd01867f743cdbfb8a014a1b1303630d Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Fri, 25 Apr 2025 16:20:55 -0300 Subject: [PATCH 31/56] Remove the Option from the pass filter. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index a274afb57..4e22bfb6e 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -26,7 +26,7 @@ use crate::{ #[derive(Default)] pub struct Print { filter: Option, - pass_filter: Option, + pass_filter: PassFilter, target: Option, only_when_modified: bool, } @@ -93,16 +93,11 @@ impl Print { self } - pub fn with_no_pass_filter(mut self) -> Self { - self.pass_filter = Some(PassFilter::All); - self - } - pub fn with_pass_filter(mut self, config: &IRPrintingConfig) -> Self { if config.print_ir_after_all { - self.pass_filter = Some(PassFilter::All); + self.pass_filter = PassFilter::All; } else if !config.print_ir_after_pass.is_empty() { - self.pass_filter = Some(PassFilter::Certain(config.print_ir_after_pass.clone())); + self.pass_filter = PassFilter::Certain(config.print_ir_after_pass.clone()); } // QUESTION: What should if the user only specifies "print after modification" but hasn't specified pass? @@ -159,15 +154,14 @@ impl Print { fn pass_filter(&self, pass: &dyn OperationPass) -> bool { match &self.pass_filter { - Some(PassFilter::All) => true, - Some(PassFilter::Certain(passes)) => passes.iter().any(|p| { + PassFilter::All => true, + PassFilter::Certain(passes) => passes.iter().any(|p| { if let Some(p_type) = pass.pass_type() { *p == p_type } else { false } }), - None => true, } } From 591458bcdee2d2d07cf3c0908b53374c2fe01bb9 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Fri, 25 Apr 2025 16:31:09 -0300 Subject: [PATCH 32/56] Make the enable_ir_printing function consume and return the PassManager Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass/manager.rs | 3 ++- midenc-compile/src/stages/rewrite.rs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index c6762b425..a2a79db5f 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -188,7 +188,7 @@ impl PassManager { self } - pub fn enable_ir_printing(&mut self, config: IRPrintingConfig) { + pub fn enable_ir_printing(mut self, config: IRPrintingConfig) -> Self { let print = Print::new(&config) .map(|p| p.with_pass_filter(&config)) .map(|p| p.with_symbol_filter(&config)); @@ -197,6 +197,7 @@ impl PassManager { let print = Box::new(print); self.add_instrumentation(print); } + self } pub fn enable_timing(&mut self, yes: bool) -> &mut Self { diff --git a/midenc-compile/src/stages/rewrite.rs b/midenc-compile/src/stages/rewrite.rs index 9a0b44566..eef8e8d2b 100644 --- a/midenc-compile/src/stages/rewrite.rs +++ b/midenc-compile/src/stages/rewrite.rs @@ -49,8 +49,8 @@ impl Stage for ApplyRewritesStage { */ // Construct a pass manager with the default pass pipeline - let mut pm = PassManager::on::(context.clone(), Nesting::Implicit); - pm.enable_ir_printing(ir_print_config); + let mut pm = PassManager::on::(context.clone(), Nesting::Implicit) + .enable_ir_printing(ir_print_config); let mut rewrite_config = GreedyRewriteConfig::default(); rewrite_config.with_region_simplification_level(RegionSimplificationLevel::Normal); From 8137b9eb60074e08d9cce8568679d9dcc3d21c84 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Fri, 25 Apr 2025 16:35:42 -0300 Subject: [PATCH 33/56] Change log::error! for log::trace! Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 4e22bfb6e..ff4626a50 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -122,7 +122,7 @@ impl Print { match self.filter { Some(OpFilter::All) => { let target = self.target.as_deref().unwrap_or("printer"); - log::error!(target: target, "{op}"); + log::trace!(target: target, "{op}"); } Some(OpFilter::Type { dialect, @@ -131,21 +131,21 @@ impl Print { let name = op.name(); if name.dialect() == dialect && name.name() == op_name { let target = self.target.as_deref().unwrap_or("printer"); - log::error!(target: target, "{op}"); + log::trace!(target: target, "{op}"); } } Some(OpFilter::Symbol(None)) => { if let Some(sym) = op.as_symbol() { let name = sym.name().as_str(); let target = self.target.as_deref().unwrap_or(name); - log::error!(target: target, "{}", sym.as_symbol_operation()); + log::trace!(target: target, "{}", sym.as_symbol_operation()); } } Some(OpFilter::Symbol(Some(filter))) => { if let Some(sym) = op.as_symbol().filter(|sym| sym.name().as_str().contains(filter)) { let target = self.target.as_deref().unwrap_or(filter); - log::error!(target: target, "{}", sym.as_symbol_operation()); + log::trace!(target: target, "{}", sym.as_symbol_operation()); } } None => (), @@ -187,7 +187,7 @@ impl PassInstrumentation for Print { return; } - log::error!("IR before the pass pipeline"); + log::trace!("IR before the pass pipeline"); let op = op.borrow(); self.print_ir(op); } @@ -197,7 +197,7 @@ impl PassInstrumentation for Print { return; } if self.pass_filter(pass) { - log::error!("Before the {} pass", pass.name()); + log::trace!("Before the {} pass", pass.name()); let op = op.borrow(); self.print_ir(op); } @@ -210,7 +210,7 @@ impl PassInstrumentation for Print { changed: IRAfterPass, ) { if self.should_print(pass, changed) { - log::error!("After the {} pass", pass.name()); + log::trace!("After the {} pass", pass.name()); let op = op.borrow(); self.print_ir(op); } From 52a162ac221b84e7c20d9ee03780d896528edd9f Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 28 Apr 2025 09:31:03 -0300 Subject: [PATCH 34/56] If the user set the "print after modify" flag but didn't set any ir filter flags, set the "every IR filter" flag. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 15 ++++++++++++--- midenc-compile/src/compiler.rs | 3 ++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index ff4626a50..5f0b08f8f 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -94,15 +94,24 @@ impl Print { } pub fn with_pass_filter(mut self, config: &IRPrintingConfig) -> Self { - if config.print_ir_after_all { + let is_ir_filter_set = if config.print_ir_after_all { self.pass_filter = PassFilter::All; + true } else if !config.print_ir_after_pass.is_empty() { self.pass_filter = PassFilter::Certain(config.print_ir_after_pass.clone()); - } + true + } else { + false + }; - // QUESTION: What should if the user only specifies "print after modification" but hasn't specified pass? if config.print_ir_after_modified { self.only_when_modified = true; + // NOTE: If the user specified the "print after modification" flag, but didn't specify + // any IR pass filter flag; then we assume that the desired behavior is to set the "all + // pass" filter. + if !is_ir_filter_set { + self.pass_filter = PassFilter::All; + } }; self diff --git a/midenc-compile/src/compiler.rs b/midenc-compile/src/compiler.rs index 63a980b2b..e9e377d23 100644 --- a/midenc-compile/src/compiler.rs +++ b/midenc-compile/src/compiler.rs @@ -339,7 +339,8 @@ pub struct UnstableOptions { ) )] pub print_ir_after_pass: Vec, - /// Only print the IR if the pass modified the IR structure. + /// Only print the IR if the pass modified the IR structure. If this flag is set, and no IR + /// filter flag is; then the default behavior is to print the IR after every pass. #[cfg_attr( feature = "std", arg(long, default_value_t = false, help_heading = "Passes") From ed579fbe6fcdf46d8d0dc9e96125f403e84a7fa3 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 28 Apr 2025 09:47:20 -0300 Subject: [PATCH 35/56] Remove default macro from PassFilter enum. Removed in favor of more explicit logic. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 5f0b08f8f..3a7691f98 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -26,16 +26,15 @@ use crate::{ #[derive(Default)] pub struct Print { filter: Option, - pass_filter: PassFilter, + pass_filter: Option, target: Option, only_when_modified: bool, } /// Filter for the different passes. -#[derive(Default, Debug)] +#[derive(Debug)] enum PassFilter { /// Print IR regardless of which pass is executed. - #[default] All, /// Only print IR if the pass's name is present in the vector. Certain(Vec), @@ -95,10 +94,10 @@ impl Print { pub fn with_pass_filter(mut self, config: &IRPrintingConfig) -> Self { let is_ir_filter_set = if config.print_ir_after_all { - self.pass_filter = PassFilter::All; + self.pass_filter = Some(PassFilter::All); true } else if !config.print_ir_after_pass.is_empty() { - self.pass_filter = PassFilter::Certain(config.print_ir_after_pass.clone()); + self.pass_filter = Some(PassFilter::Certain(config.print_ir_after_pass.clone())); true } else { false @@ -110,7 +109,7 @@ impl Print { // any IR pass filter flag; then we assume that the desired behavior is to set the "all // pass" filter. if !is_ir_filter_set { - self.pass_filter = PassFilter::All; + self.pass_filter = Some(PassFilter::All); } }; @@ -163,14 +162,15 @@ impl Print { fn pass_filter(&self, pass: &dyn OperationPass) -> bool { match &self.pass_filter { - PassFilter::All => true, - PassFilter::Certain(passes) => passes.iter().any(|p| { + Some(PassFilter::All) => true, + Some(PassFilter::Certain(passes)) => passes.iter().any(|p| { if let Some(p_type) = pass.pass_type() { *p == p_type } else { false } }), + None => false, } } From 56df599f613f46e98239511e342fa69eb06c793a Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 28 Apr 2025 10:53:02 -0300 Subject: [PATCH 36/56] Rename: PassType -> PassIdentifier; pass_type() -> pass_id() Signed-off-by: Tomas Fabrizio Orsi --- dialects/hir/src/transforms/spill.rs | 6 ++--- dialects/scf/src/transforms/cfg_to_scf.rs | 6 ++--- hir-transform/src/canonicalization.rs | 6 ++--- hir-transform/src/sccp.rs | 6 ++--- hir-transform/src/sink.rs | 10 ++++---- hir/src/pass.rs | 6 ++--- hir/src/pass/manager.rs | 10 ++++---- hir/src/pass/pass.rs | 28 +++++++++++------------ 8 files changed, 39 insertions(+), 39 deletions(-) diff --git a/dialects/hir/src/transforms/spill.rs b/dialects/hir/src/transforms/spill.rs index 1c364c759..868dca109 100644 --- a/dialects/hir/src/transforms/spill.rs +++ b/dialects/hir/src/transforms/spill.rs @@ -3,7 +3,7 @@ use alloc::rc::Rc; use midenc_hir::{ adt::SmallDenseMap, dialects::builtin::{Function, FunctionRef, LocalVariable}, - pass::{pass::PassType, IRAfterPass, Pass, PassExecutionState}, + pass::{pass::PassIdentifier, IRAfterPass, Pass, PassExecutionState}, BlockRef, BuilderExt, EntityMut, Op, OpBuilder, OperationName, OperationRef, Report, Rewriter, SourceSpan, Spanned, Symbol, ValueRef, }; @@ -19,8 +19,8 @@ impl Pass for TransformSpills { "transform-spills" } - fn pass_type(&self) -> Option { - Some(PassType::TransformSpills) + fn pass_id(&self) -> Option { + Some(PassIdentifier::TransformSpills) } fn argument(&self) -> &'static str { diff --git a/dialects/scf/src/transforms/cfg_to_scf.rs b/dialects/scf/src/transforms/cfg_to_scf.rs index ca672f0bc..cb0b411ce 100644 --- a/dialects/scf/src/transforms/cfg_to_scf.rs +++ b/dialects/scf/src/transforms/cfg_to_scf.rs @@ -7,7 +7,7 @@ use midenc_hir::{ diagnostics::Severity, dialects::builtin, dominance::DominanceInfo, - pass::{pass::PassType, IRAfterPass, Pass, PassExecutionState}, + pass::{pass::PassIdentifier, IRAfterPass, Pass, PassExecutionState}, Builder, EntityMut, Forward, Op, Operation, OperationName, OperationRef, RawWalk, Report, SmallVec, Spanned, Type, ValueRange, ValueRef, WalkResult, }; @@ -35,8 +35,8 @@ impl Pass for LiftControlFlowToSCF { "lift-control-flow" } - fn pass_type(&self) -> Option { - Some(PassType::LiftControlFlowToSCF) + fn pass_id(&self) -> Option { + Some(PassIdentifier::LiftControlFlowToSCF) } fn argument(&self) -> &'static str { diff --git a/hir-transform/src/canonicalization.rs b/hir-transform/src/canonicalization.rs index 6044aa261..cf94ebbf8 100644 --- a/hir-transform/src/canonicalization.rs +++ b/hir-transform/src/canonicalization.rs @@ -1,7 +1,7 @@ use alloc::{boxed::Box, format, rc::Rc}; use midenc_hir::{ - pass::{pass::PassType, IRAfterPass, OperationPass, Pass, PassExecutionState}, + pass::{pass::PassIdentifier, IRAfterPass, OperationPass, Pass, PassExecutionState}, patterns::{self, FrozenRewritePatternSet, GreedyRewriteConfig, RewritePatternSet}, Context, EntityMut, Operation, OperationName, Report, Spanned, }; @@ -62,8 +62,8 @@ impl Pass for Canonicalizer { "canonicalizer" } - fn pass_type(&self) -> Option { - Some(PassType::Canonicalizer) + fn pass_id(&self) -> Option { + Some(PassIdentifier::Canonicalizer) } fn argument(&self) -> &'static str { diff --git a/hir-transform/src/sccp.rs b/hir-transform/src/sccp.rs index 1fb0efac1..1183f69f7 100644 --- a/hir-transform/src/sccp.rs +++ b/hir-transform/src/sccp.rs @@ -1,5 +1,5 @@ use midenc_hir::{ - pass::{pass::PassType, IRAfterPass, Pass, PassExecutionState}, + pass::{pass::PassIdentifier, IRAfterPass, Pass, PassExecutionState}, patterns::NoopRewriterListener, BlockRef, Builder, EntityMut, OpBuilder, Operation, OperationFolder, OperationName, RegionList, Report, SmallVec, ValueRef, @@ -26,8 +26,8 @@ impl Pass for SparseConditionalConstantPropagation { "sparse-conditional-constant-propagation" } - fn pass_type(&self) -> Option { - Some(PassType::SparseConditionalConstantPropagation) + fn pass_id(&self) -> Option { + Some(PassIdentifier::SparseConditionalConstantPropagation) } fn argument(&self) -> &'static str { diff --git a/hir-transform/src/sink.rs b/hir-transform/src/sink.rs index c4d6fec55..92ccef0fc 100644 --- a/hir-transform/src/sink.rs +++ b/hir-transform/src/sink.rs @@ -4,7 +4,7 @@ use midenc_hir::{ adt::SmallDenseMap, dominance::DominanceInfo, matchers::{self, Matcher}, - pass::{pass::PassType, IRAfterPass, Pass, PassExecutionState}, + pass::{pass::PassIdentifier, IRAfterPass, Pass, PassExecutionState}, traits::{ConstantLike, Terminator}, Backward, Builder, EntityMut, Forward, FxHashSet, OpBuilder, Operation, OperationName, OperationRef, ProgramPoint, RawWalk, Region, RegionBranchOpInterface, @@ -84,8 +84,8 @@ impl Pass for ControlFlowSink { "control-flow-sink" } - fn pass_type(&self) -> Option { - Some(PassType::ControlFlowSink) + fn pass_id(&self) -> Option { + Some(PassIdentifier::ControlFlowSink) } fn argument(&self) -> &'static str { @@ -165,8 +165,8 @@ impl Pass for SinkOperandDefs { "sink-operand-defs" } - fn pass_type(&self) -> Option { - Some(PassType::SinkOperandDefs) + fn pass_id(&self) -> Option { + Some(PassIdentifier::SinkOperandDefs) } fn argument(&self) -> &'static str { diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 3a7691f98..44e870927 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -12,7 +12,7 @@ pub use self::{ analysis::{Analysis, AnalysisManager, OperationAnalysis, PreservedAnalyses}, instrumentation::{PassInstrumentation, PassInstrumentor, PipelineParentInfo}, manager::{IRPrintingConfig, Nesting, OpPassManager, PassDisplayMode, PassManager}, - pass::{IRAfterPass, OperationPass, Pass, PassExecutionState, PassType}, + pass::{IRAfterPass, OperationPass, Pass, PassExecutionState, PassIdentifier}, registry::{PassInfo, PassPipelineInfo}, specialization::PassTarget, statistics::{PassStatistic, Statistic, StatisticValue}, @@ -37,7 +37,7 @@ enum PassFilter { /// Print IR regardless of which pass is executed. All, /// Only print IR if the pass's name is present in the vector. - Certain(Vec), + Certain(Vec), } #[derive(Default, Debug)] @@ -164,7 +164,7 @@ impl Print { match &self.pass_filter { Some(PassFilter::All) => true, Some(PassFilter::Certain(passes)) => passes.iter().any(|p| { - if let Some(p_type) = pass.pass_type() { + if let Some(p_type) = pass.pass_id() { *p == p_type } else { false diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index a2a79db5f..f2f132256 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -9,7 +9,7 @@ use super::{ PassInstrumentor, PipelineParentInfo, Statistic, }; use crate::{ - pass::{pass::PassType, IRAfterPass, Print}, + pass::{pass::PassIdentifier, IRAfterPass, Print}, traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, Operation, OperationName, OperationRef, Report, @@ -37,14 +37,14 @@ pub struct IRPrintingConfig { pub print_after_only_on_failure: bool, // NOTE: Taken from the Options struct pub print_ir_after_all: bool, - pub print_ir_after_pass: Vec, + pub print_ir_after_pass: Vec, pub print_ir_after_modified: bool, pub flags: OpPrintingFlags, } impl From<&Options> for IRPrintingConfig { fn from(options: &Options) -> Self { - let pass_filters: Vec = + let pass_filters: Vec = options.print_ir_after_pass.iter().map(|a| a.into()).collect(); IRPrintingConfig { print_ir_after_all: options.print_ir_after_all, @@ -735,7 +735,7 @@ impl OpToOpPassAdaptor { } #[allow(dead_code)] - fn pass_type(&self) -> Option { + fn pass_id(&self) -> Option { None } @@ -1060,7 +1060,7 @@ impl Pass for OpToOpPassAdaptor { crate::interner::Symbol::intern(self.name()).as_str() } - fn pass_type(&self) -> Option { + fn pass_id(&self) -> Option { None } diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index b9e3632d1..48b201921 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -16,7 +16,7 @@ pub trait OperationPass { fn into_any(self: Box) -> Box; fn name(&self) -> &'static str; - fn pass_type(&self) -> Option; + fn pass_id(&self) -> Option; fn argument(&self) -> &'static str { // NOTE: Could we compute an argument string from the type name? @@ -64,8 +64,8 @@ where

::as_any(self) } - fn pass_type(&self) -> Option { -

::pass_type(self) + fn pass_id(&self) -> Option { +

::pass_id(self) } fn as_any_mut(&mut self) -> &mut dyn Any { @@ -144,7 +144,7 @@ where } #[derive(Debug, PartialEq, Clone, Copy)] -pub enum PassType { +pub enum PassIdentifier { Canonicalizer, ControlFlowSink, LiftControlFlowToSCF, @@ -154,17 +154,17 @@ pub enum PassType { TransformSpills, } -impl From<&String> for PassType { +impl From<&String> for PassIdentifier { fn from(pass_name: &String) -> Self { match pass_name.as_str() { - "canonicalizer" => PassType::Canonicalizer, - "control-flow-sink" => PassType::ControlFlowSink, - "lift-control-flow" => PassType::LiftControlFlowToSCF, - "sink-operand-defs" => PassType::SinkOperandDefs, + "canonicalizer" => PassIdentifier::Canonicalizer, + "control-flow-sink" => PassIdentifier::ControlFlowSink, + "lift-control-flow" => PassIdentifier::LiftControlFlowToSCF, + "sink-operand-defs" => PassIdentifier::SinkOperandDefs, "sparse-conditional-constant-propagation" => { - PassType::SparseConditionalConstantPropagation + PassIdentifier::SparseConditionalConstantPropagation } - "transform-spills" => PassType::TransformSpills, + "transform-spills" => PassIdentifier::TransformSpills, _ => panic!("ERROR: '{pass_name}' unrecognized pass."), } } @@ -295,7 +295,7 @@ pub trait Pass: Sized + Any { state.run_pipeline(pipeline, op) } - fn pass_type(&self) -> Option; + fn pass_id(&self) -> Option; } impl

Pass for Box

@@ -322,8 +322,8 @@ where (**self).name() } - fn pass_type(&self) -> Option { - (**self).pass_type() + fn pass_id(&self) -> Option { + (**self).pass_id() } #[inline] From 00d617b543f851e5fbb16a18e62bd793c3491c16 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 28 Apr 2025 10:58:07 -0300 Subject: [PATCH 37/56] Rename: IRAfterPass -> PostPassStatus Also rename variants accordingly: IRAfterPass::Changed -> PostPassStatus::IRChanged IRAfterPass::Unchanged -> PostPassStatus::IRUnchanged Signed-off-by: Tomas Fabrizio Orsi --- dialects/hir/src/transforms/spill.rs | 8 ++++---- dialects/scf/src/transforms/cfg_to_scf.rs | 6 +++--- hir-transform/src/canonicalization.rs | 6 +++--- hir-transform/src/sccp.rs | 6 +++--- hir-transform/src/sink.rs | 16 ++++++++-------- hir-transform/src/spill.rs | 6 +++--- hir/src/pass.rs | 8 ++++---- hir/src/pass/instrumentation.rs | 8 ++++---- hir/src/pass/manager.rs | 10 +++++----- hir/src/pass/pass.rs | 20 ++++++++++---------- 10 files changed, 47 insertions(+), 47 deletions(-) diff --git a/dialects/hir/src/transforms/spill.rs b/dialects/hir/src/transforms/spill.rs index 868dca109..5e778728b 100644 --- a/dialects/hir/src/transforms/spill.rs +++ b/dialects/hir/src/transforms/spill.rs @@ -3,7 +3,7 @@ use alloc::rc::Rc; use midenc_hir::{ adt::SmallDenseMap, dialects::builtin::{Function, FunctionRef, LocalVariable}, - pass::{pass::PassIdentifier, IRAfterPass, Pass, PassExecutionState}, + pass::{pass::PassIdentifier, Pass, PassExecutionState, PostPassStatus}, BlockRef, BuilderExt, EntityMut, Op, OpBuilder, OperationName, OperationRef, Report, Rewriter, SourceSpan, Spanned, Symbol, ValueRef, }; @@ -35,14 +35,14 @@ impl Pass for TransformSpills { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { let function = op.into_entity_ref(); log::debug!(target: "insert-spills", "computing and inserting spills for {}", function.as_operation()); if function.is_declaration() { log::debug!(target: "insert-spills", "function has no body, no spills needed!"); state.preserved_analyses_mut().preserve_all(); - return Ok(IRAfterPass::Unchanged); + return Ok(PostPassStatus::IRUnchanged); } let mut analysis = state.analysis_manager().get_analysis_for::()?; @@ -50,7 +50,7 @@ impl Pass for TransformSpills { if !analysis.has_spills() { log::debug!(target: "insert-spills", "no spills needed!"); state.preserved_analyses_mut().preserve_all(); - return Ok(IRAfterPass::Unchanged); + return Ok(PostPassStatus::IRUnchanged); } // Take ownership of the spills analysis so that we can mutate the analysis state during diff --git a/dialects/scf/src/transforms/cfg_to_scf.rs b/dialects/scf/src/transforms/cfg_to_scf.rs index cb0b411ce..7f36d6b6e 100644 --- a/dialects/scf/src/transforms/cfg_to_scf.rs +++ b/dialects/scf/src/transforms/cfg_to_scf.rs @@ -7,7 +7,7 @@ use midenc_hir::{ diagnostics::Severity, dialects::builtin, dominance::DominanceInfo, - pass::{pass::PassIdentifier, IRAfterPass, Pass, PassExecutionState}, + pass::{pass::PassIdentifier, Pass, PassExecutionState, PostPassStatus}, Builder, EntityMut, Forward, Op, Operation, OperationName, OperationRef, RawWalk, Report, SmallVec, Spanned, Type, ValueRange, ValueRef, WalkResult, }; @@ -55,7 +55,7 @@ impl Pass for LiftControlFlowToSCF { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { let mut transformation = ControlFlowToSCFTransformation; let mut changed = false; @@ -134,7 +134,7 @@ impl Pass for LiftControlFlowToSCF { }); if result.was_interrupted() { - return result.into_result().map(|_| IRAfterPass::Unchanged); + return result.into_result().map(|_| PostPassStatus::IRUnchanged); } log::debug!( diff --git a/hir-transform/src/canonicalization.rs b/hir-transform/src/canonicalization.rs index cf94ebbf8..112a48e5c 100644 --- a/hir-transform/src/canonicalization.rs +++ b/hir-transform/src/canonicalization.rs @@ -1,7 +1,7 @@ use alloc::{boxed::Box, format, rc::Rc}; use midenc_hir::{ - pass::{pass::PassIdentifier, IRAfterPass, OperationPass, Pass, PassExecutionState}, + pass::{pass::PassIdentifier, OperationPass, Pass, PassExecutionState, PostPassStatus}, patterns::{self, FrozenRewritePatternSet, GreedyRewriteConfig, RewritePatternSet}, Context, EntityMut, Operation, OperationName, Report, Spanned, }; @@ -97,10 +97,10 @@ impl Pass for Canonicalizer { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { let Some(rewrites) = self.rewrites.as_ref() else { log::debug!("skipping canonicalization as there are no rewrite patterns to apply"); - return Ok(IRAfterPass::Unchanged); + return Ok(PostPassStatus::IRUnchanged); }; let op = { let ptr = op.as_operation_ref(); diff --git a/hir-transform/src/sccp.rs b/hir-transform/src/sccp.rs index 1183f69f7..06177b6db 100644 --- a/hir-transform/src/sccp.rs +++ b/hir-transform/src/sccp.rs @@ -1,5 +1,5 @@ use midenc_hir::{ - pass::{pass::PassIdentifier, IRAfterPass, Pass, PassExecutionState}, + pass::{pass::PassIdentifier, Pass, PassExecutionState, PostPassStatus}, patterns::NoopRewriterListener, BlockRef, Builder, EntityMut, OpBuilder, Operation, OperationFolder, OperationName, RegionList, Report, SmallVec, ValueRef, @@ -42,7 +42,7 @@ impl Pass for SparseConditionalConstantPropagation { &mut self, mut op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { // Run sparse constant propagation + dead code analysis let mut solver = DataFlowSolver::default(); solver.load::(); @@ -62,7 +62,7 @@ impl SparseConditionalConstantPropagation { op: &mut Operation, _state: &mut PassExecutionState, solver: &DataFlowSolver, - ) -> Result { + ) -> Result { let mut worklist = SmallVec::<[BlockRef; 8]>::default(); let add_to_worklist = |regions: &RegionList, worklist: &mut SmallVec<[BlockRef; 8]>| { diff --git a/hir-transform/src/sink.rs b/hir-transform/src/sink.rs index 92ccef0fc..e812535f8 100644 --- a/hir-transform/src/sink.rs +++ b/hir-transform/src/sink.rs @@ -4,7 +4,7 @@ use midenc_hir::{ adt::SmallDenseMap, dominance::DominanceInfo, matchers::{self, Matcher}, - pass::{pass::PassIdentifier, IRAfterPass, Pass, PassExecutionState}, + pass::{pass::PassIdentifier, Pass, PassExecutionState, PostPassStatus}, traits::{ConstantLike, Terminator}, Backward, Builder, EntityMut, Forward, FxHashSet, OpBuilder, Operation, OperationName, OperationRef, ProgramPoint, RawWalk, Region, RegionBranchOpInterface, @@ -100,7 +100,7 @@ impl Pass for ControlFlowSink { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { let op = op.into_entity_ref(); log::debug!(target: "control-flow-sink", "sinking operations in {op}"); @@ -109,7 +109,7 @@ impl Pass for ControlFlowSink { let dominfo = state.analysis_manager().get_analysis::()?; - let mut sunk = IRAfterPass::Unchanged; + let mut sunk = PostPassStatus::IRUnchanged; operation.raw_prewalk_all::(|op: OperationRef| { let regions_to_sink = { let op = op.borrow(); @@ -181,7 +181,7 @@ impl Pass for SinkOperandDefs { &mut self, op: EntityMut<'_, Self::Target>, _state: &mut PassExecutionState, - ) -> Result { + ) -> Result { let operation = op.as_operation_ref(); drop(op); @@ -193,7 +193,7 @@ impl Pass for SinkOperandDefs { // then process the worklist, moving everything into position. let mut worklist = alloc::collections::VecDeque::default(); - let mut changed = IRAfterPass::Unchanged; + let mut changed = PostPassStatus::IRUnchanged; // Visit ops in "true" post-order (i.e. block bodies are visited bottom-up). operation.raw_postwalk_all::(|operation: OperationRef| { // Determine if any of this operation's operands represent one of the following: @@ -318,7 +318,7 @@ impl Pass for SinkOperandDefs { log::trace!(target: "sink-operand-defs", " rewriting operand {operand_value} as {replacement}"); operand.borrow_mut().set(replacement); - changed = IRAfterPass::Changed; + changed = PostPassStatus::IRChanged; // If no other uses of this value remain, then remove the original // operation, as it is now dead. if !operand_value.borrow().is_used() { @@ -365,7 +365,7 @@ impl Pass for SinkOperandDefs { log::trace!(target: "sink-operand-defs", " rewriting operand {operand_value} as {replacement}"); sink_state.replacements.insert(operand_value, replacement); operand.borrow_mut().set(replacement); - changed = IRAfterPass::Changed; + changed = PostPassStatus::IRChanged; } else { log::trace!(target: "sink-operand-defs", " defining op is a constant with no other uses, moving into place"); // The original op can be moved @@ -560,7 +560,7 @@ pub fn control_flow_sink( dominfo: &DominanceInfo, should_move_into_region: P, move_into_region: F, -) -> IRAfterPass +) -> PostPassStatus where P: Fn(&Operation, &Region) -> bool, F: Fn(OperationRef, RegionRef), diff --git a/hir-transform/src/spill.rs b/hir-transform/src/spill.rs index 6e88e1ca4..58c5ba978 100644 --- a/hir-transform/src/spill.rs +++ b/hir-transform/src/spill.rs @@ -4,7 +4,7 @@ use midenc_hir::{ adt::{SmallDenseMap, SmallSet}, cfg::Graph, dominance::{DomTreeNode, DominanceFrontier, DominanceInfo}, - pass::{AnalysisManager, IRAfterPass}, + pass::{AnalysisManager, PostPassStatus}, traits::SingleRegion, BlockRef, Builder, Context, FxHashMap, OpBuilder, OpOperand, Operation, OperationRef, ProgramPoint, Region, RegionBranchOpInterface, RegionBranchPoint, RegionRef, Report, Rewriter, @@ -113,7 +113,7 @@ pub fn transform_spills( analysis: &mut SpillAnalysis, interface: &mut dyn TransformSpillsInterface, analysis_manager: AnalysisManager, -) -> Result { +) -> Result { assert!( op.borrow().implements::(), "the spills transformation is not supported when the root op is multi-region" @@ -252,7 +252,7 @@ pub fn transform_spills( )?; } - Ok(IRAfterPass::Changed) + Ok(PostPassStatus::IRChanged) } fn rewrite_single_block_spills( diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 44e870927..6aa848126 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -12,7 +12,7 @@ pub use self::{ analysis::{Analysis, AnalysisManager, OperationAnalysis, PreservedAnalyses}, instrumentation::{PassInstrumentation, PassInstrumentor, PipelineParentInfo}, manager::{IRPrintingConfig, Nesting, OpPassManager, PassDisplayMode, PassManager}, - pass::{IRAfterPass, OperationPass, Pass, PassExecutionState, PassIdentifier}, + pass::{OperationPass, Pass, PassExecutionState, PassIdentifier, PostPassStatus}, registry::{PassInfo, PassPipelineInfo}, specialization::PassTarget, statistics::{PassStatistic, Statistic, StatisticValue}, @@ -174,12 +174,12 @@ impl Print { } } - fn should_print(&self, pass: &dyn OperationPass, ir_changed: IRAfterPass) -> bool { + fn should_print(&self, pass: &dyn OperationPass, ir_changed: PostPassStatus) -> bool { let pass_filter = self.pass_filter(pass); // Always print, unless "only_when_modified" has been set and there have not been changes. let modification_filter = - !matches!((self.only_when_modified, ir_changed), (true, IRAfterPass::Unchanged)); + !matches!((self.only_when_modified, ir_changed), (true, PostPassStatus::IRUnchanged)); pass_filter && modification_filter } @@ -216,7 +216,7 @@ impl PassInstrumentation for Print { &mut self, pass: &dyn OperationPass, op: &OperationRef, - changed: IRAfterPass, + changed: PostPassStatus, ) { if self.should_print(pass, changed) { log::trace!("After the {} pass", pass.name()); diff --git a/hir/src/pass/instrumentation.rs b/hir/src/pass/instrumentation.rs index c612bf655..7db417f25 100644 --- a/hir/src/pass/instrumentation.rs +++ b/hir/src/pass/instrumentation.rs @@ -5,7 +5,7 @@ use compact_str::CompactString; use smallvec::SmallVec; use super::OperationPass; -use crate::{pass::IRAfterPass, OperationName, OperationRef}; +use crate::{pass::PostPassStatus, OperationName, OperationRef}; #[allow(unused_variables)] pub trait PassInstrumentation { @@ -27,7 +27,7 @@ pub trait PassInstrumentation { &mut self, pass: &dyn OperationPass, op: &OperationRef, - changed: IRAfterPass, + changed: PostPassStatus, ) { } fn run_after_pass_failed(&mut self, pass: &dyn OperationPass, op: &OperationRef) {} @@ -66,7 +66,7 @@ impl PassInstrumentation for Box

{ &mut self, pass: &dyn OperationPass, op: &OperationRef, - changed: IRAfterPass, + changed: PostPassStatus, ) { (**self).run_after_pass(pass, op, changed); } @@ -115,7 +115,7 @@ impl PassInstrumentor { &self, pass: &dyn OperationPass, op: &OperationRef, - changed: IRAfterPass, + changed: PostPassStatus, ) { self.instrument(|pi| pi.run_after_pass(pass, op, changed)); } diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index f2f132256..28ec428ba 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -9,7 +9,7 @@ use super::{ PassInstrumentor, PipelineParentInfo, Statistic, }; use crate::{ - pass::{pass::PassIdentifier, IRAfterPass, Print}, + pass::{pass::PassIdentifier, PostPassStatus, Print}, traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, Operation, OperationName, OperationRef, Report, @@ -987,7 +987,7 @@ impl OpToOpPassAdaptor { let in_result = if let Ok(result) = result { result } else { - IRAfterPass::Unchanged + PostPassStatus::IRUnchanged }; if result.is_err() { instrumentor.run_after_pass_failed(pass, &op); @@ -1014,7 +1014,7 @@ impl OpToOpPassAdaptor { op: OperationRef, state: &mut PassExecutionState, verify: bool, - ) -> Result { + ) -> Result { let analysis_manager = state.analysis_manager(); let instrumentor = analysis_manager.pass_instrumentor(); let parent_info = PipelineParentInfo { @@ -1049,7 +1049,7 @@ impl OpToOpPassAdaptor { } } - Ok(IRAfterPass::Unchanged) + Ok(PostPassStatus::IRUnchanged) } } @@ -1090,7 +1090,7 @@ impl Pass for OpToOpPassAdaptor { &mut self, _op: EntityMut<'_, Operation>, _state: &mut PassExecutionState, - ) -> Result { + ) -> Result { unreachable!("unexpected call to `Pass::run_on_operation` for OpToOpPassAdaptor") } } diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index 48b201921..4e69fffd9 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -47,7 +47,7 @@ pub trait OperationPass { &mut self, op: OperationRef, state: &mut PassExecutionState, - ) -> Result; + ) -> Result; fn run_pipeline( &mut self, pipeline: &mut OpPassManager, @@ -128,7 +128,7 @@ where &mut self, mut op: OperationRef, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { let op = <

::Target as PassTarget>::into_target_mut(&mut op);

::run_on_operation(self, op, state) } @@ -171,17 +171,17 @@ impl From<&String> for PassIdentifier { } #[derive(Debug, PartialEq, Clone, Copy)] -pub enum IRAfterPass { - Unchanged, - Changed, +pub enum PostPassStatus { + IRUnchanged, + IRChanged, } // NOTE: I am not sure if using a From impl -impl From for IRAfterPass { +impl From for PostPassStatus { fn from(ir_was_changed: bool) -> Self { match ir_was_changed { - true => IRAfterPass::Changed, - false => IRAfterPass::Unchanged, + true => PostPassStatus::IRChanged, + false => PostPassStatus::IRUnchanged, } } } @@ -281,7 +281,7 @@ pub trait Pass: Sized + Any { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result; + ) -> Result; /// Schedule an arbitrary pass pipeline on the provided operation. /// /// This can be invoke any time in a pass to dynamic schedule more passes. The provided @@ -386,7 +386,7 @@ where &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result { (**self).run_on_operation(op, state) } From bd560c0c6005542cdc12eb8716f7d666379d6a13 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 28 Apr 2025 11:03:03 -0300 Subject: [PATCH 38/56] Remove unused OpToOpPassAdaptor::pass_id() Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass/manager.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 28ec428ba..b54650bae 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -734,11 +734,6 @@ impl OpToOpPassAdaptor { Self { pms: smallvec![pm] } } - #[allow(dead_code)] - fn pass_id(&self) -> Option { - None - } - pub fn name(&self) -> CompactString { use core::fmt::Write; From e53b3744c1b389667e26f740fdf4376d4886cf3b Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 28 Apr 2025 11:24:39 -0300 Subject: [PATCH 39/56] Remove outdated comment in From for PostPassStatus Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass/pass.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index 4e69fffd9..9412d7475 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -176,7 +176,6 @@ pub enum PostPassStatus { IRChanged, } -// NOTE: I am not sure if using a From impl impl From for PostPassStatus { fn from(ir_was_changed: bool) -> Self { match ir_was_changed { From 4333d3513b7633737db690a3cc8352be758aa1f6 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 28 Apr 2025 11:58:47 -0300 Subject: [PATCH 40/56] Changed From for PassIdentifier to TryFrom for PassIdentifier Also change From<&Options> to TryFrom<&Options>, since it also used passIdentifier Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass/manager.rs | 17 +++++++++++------ hir/src/pass/pass.rs | 22 ++++++++++++---------- midenc-compile/src/stages/rewrite.rs | 2 +- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index b54650bae..716e7f0fa 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -42,16 +42,21 @@ pub struct IRPrintingConfig { pub flags: OpPrintingFlags, } -impl From<&Options> for IRPrintingConfig { - fn from(options: &Options) -> Self { - let pass_filters: Vec = - options.print_ir_after_pass.iter().map(|a| a.into()).collect(); - IRPrintingConfig { +impl TryFrom<&Options> for IRPrintingConfig { + type Error = Report; + + fn try_from(options: &Options) -> Result { + let pass_filters: Vec = options + .print_ir_after_pass + .iter() + .map(|a| a.try_into()) + .collect::, Report>>()?; + Ok(IRPrintingConfig { print_ir_after_all: options.print_ir_after_all, print_ir_after_pass: pass_filters, print_ir_after_modified: options.print_ir_after_modified, ..Default::default() - } + }) } } diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index 9412d7475..b3faac12b 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -1,4 +1,4 @@ -use alloc::{boxed::Box, rc::Rc}; +use alloc::{boxed::Box, format, rc::Rc}; use core::{any::Any, fmt}; use super::*; @@ -154,18 +154,20 @@ pub enum PassIdentifier { TransformSpills, } -impl From<&String> for PassIdentifier { - fn from(pass_name: &String) -> Self { +impl TryFrom<&String> for PassIdentifier { + type Error = Report; + + fn try_from(pass_name: &String) -> Result { match pass_name.as_str() { - "canonicalizer" => PassIdentifier::Canonicalizer, - "control-flow-sink" => PassIdentifier::ControlFlowSink, - "lift-control-flow" => PassIdentifier::LiftControlFlowToSCF, - "sink-operand-defs" => PassIdentifier::SinkOperandDefs, + "canonicalizer" => Ok(PassIdentifier::Canonicalizer), + "control-flow-sink" => Ok(PassIdentifier::ControlFlowSink), + "lift-control-flow" => Ok(PassIdentifier::LiftControlFlowToSCF), + "sink-operand-defs" => Ok(PassIdentifier::SinkOperandDefs), "sparse-conditional-constant-propagation" => { - PassIdentifier::SparseConditionalConstantPropagation + Ok(PassIdentifier::SparseConditionalConstantPropagation) } - "transform-spills" => PassIdentifier::TransformSpills, - _ => panic!("ERROR: '{pass_name}' unrecognized pass."), + "transform-spills" => Ok(PassIdentifier::TransformSpills), + _ => Err(Report::msg(format!("'{pass_name}' unrecognized pass."))), } } } diff --git a/midenc-compile/src/stages/rewrite.rs b/midenc-compile/src/stages/rewrite.rs index eef8e8d2b..a7d8ffa06 100644 --- a/midenc-compile/src/stages/rewrite.rs +++ b/midenc-compile/src/stages/rewrite.rs @@ -22,7 +22,7 @@ impl Stage for ApplyRewritesStage { } fn run(&mut self, input: Self::Input, context: Rc) -> CompilerResult { - let ir_print_config: IRPrintingConfig = (&context.as_ref().session().options).into(); + let ir_print_config: IRPrintingConfig = (&context.as_ref().session().options).try_into()?; log::debug!(target: "driver", "applying rewrite passes"); // TODO(pauls): Set up pass registration for new pass infra /* From a1fc2ee7ca0ad25e21203e3e29663f03518be7cc Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 28 Apr 2025 12:31:27 -0300 Subject: [PATCH 41/56] Simplify if let with a unwrap or return statement for "IRUnchanged" Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass/manager.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 716e7f0fa..087129c58 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -984,15 +984,11 @@ impl OpToOpPassAdaptor { } if let Some(instrumentor) = pi.as_deref() { - let in_result = if let Ok(result) = result { - result - } else { - PostPassStatus::IRUnchanged - }; if result.is_err() { instrumentor.run_after_pass_failed(pass, &op); } else { - instrumentor.run_after_pass(pass, &op, in_result); + let in_result = result.as_ref().unwrap_or(&PostPassStatus::IRUnchanged); + instrumentor.run_after_pass(pass, &op, *in_result); } } From a112434bc6fb360df6958fbf38c4853190b08960 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 28 Apr 2025 14:48:41 -0300 Subject: [PATCH 42/56] Remove pub mod pass. Make pass private again. Signed-off-by: Tomas Fabrizio Orsi --- dialects/hir/src/transforms/spill.rs | 2 +- dialects/scf/src/transforms/cfg_to_scf.rs | 2 +- hir-transform/src/canonicalization.rs | 2 +- hir-transform/src/sccp.rs | 2 +- hir-transform/src/sink.rs | 2 +- hir/src/pass.rs | 3 +-- 6 files changed, 6 insertions(+), 7 deletions(-) diff --git a/dialects/hir/src/transforms/spill.rs b/dialects/hir/src/transforms/spill.rs index 5e778728b..188f92a1c 100644 --- a/dialects/hir/src/transforms/spill.rs +++ b/dialects/hir/src/transforms/spill.rs @@ -3,7 +3,7 @@ use alloc::rc::Rc; use midenc_hir::{ adt::SmallDenseMap, dialects::builtin::{Function, FunctionRef, LocalVariable}, - pass::{pass::PassIdentifier, Pass, PassExecutionState, PostPassStatus}, + pass::{Pass, PassExecutionState, PassIdentifier, PostPassStatus}, BlockRef, BuilderExt, EntityMut, Op, OpBuilder, OperationName, OperationRef, Report, Rewriter, SourceSpan, Spanned, Symbol, ValueRef, }; diff --git a/dialects/scf/src/transforms/cfg_to_scf.rs b/dialects/scf/src/transforms/cfg_to_scf.rs index 7f36d6b6e..cddbce58d 100644 --- a/dialects/scf/src/transforms/cfg_to_scf.rs +++ b/dialects/scf/src/transforms/cfg_to_scf.rs @@ -7,7 +7,7 @@ use midenc_hir::{ diagnostics::Severity, dialects::builtin, dominance::DominanceInfo, - pass::{pass::PassIdentifier, Pass, PassExecutionState, PostPassStatus}, + pass::{Pass, PassExecutionState, PassIdentifier, PostPassStatus}, Builder, EntityMut, Forward, Op, Operation, OperationName, OperationRef, RawWalk, Report, SmallVec, Spanned, Type, ValueRange, ValueRef, WalkResult, }; diff --git a/hir-transform/src/canonicalization.rs b/hir-transform/src/canonicalization.rs index 112a48e5c..16ca8838d 100644 --- a/hir-transform/src/canonicalization.rs +++ b/hir-transform/src/canonicalization.rs @@ -1,7 +1,7 @@ use alloc::{boxed::Box, format, rc::Rc}; use midenc_hir::{ - pass::{pass::PassIdentifier, OperationPass, Pass, PassExecutionState, PostPassStatus}, + pass::{OperationPass, Pass, PassExecutionState, PassIdentifier, PostPassStatus}, patterns::{self, FrozenRewritePatternSet, GreedyRewriteConfig, RewritePatternSet}, Context, EntityMut, Operation, OperationName, Report, Spanned, }; diff --git a/hir-transform/src/sccp.rs b/hir-transform/src/sccp.rs index 06177b6db..e701de2e4 100644 --- a/hir-transform/src/sccp.rs +++ b/hir-transform/src/sccp.rs @@ -1,5 +1,5 @@ use midenc_hir::{ - pass::{pass::PassIdentifier, Pass, PassExecutionState, PostPassStatus}, + pass::{Pass, PassExecutionState, PassIdentifier, PostPassStatus}, patterns::NoopRewriterListener, BlockRef, Builder, EntityMut, OpBuilder, Operation, OperationFolder, OperationName, RegionList, Report, SmallVec, ValueRef, diff --git a/hir-transform/src/sink.rs b/hir-transform/src/sink.rs index e812535f8..e601bb253 100644 --- a/hir-transform/src/sink.rs +++ b/hir-transform/src/sink.rs @@ -4,7 +4,7 @@ use midenc_hir::{ adt::SmallDenseMap, dominance::DominanceInfo, matchers::{self, Matcher}, - pass::{pass::PassIdentifier, Pass, PassExecutionState, PostPassStatus}, + pass::{Pass, PassExecutionState, PassIdentifier, PostPassStatus}, traits::{ConstantLike, Terminator}, Backward, Builder, EntityMut, Forward, FxHashSet, OpBuilder, Operation, OperationName, OperationRef, ProgramPoint, RawWalk, Region, RegionBranchOpInterface, diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 6aa848126..03f6f3702 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -1,9 +1,8 @@ mod analysis; mod instrumentation; mod manager; -/// Made public momentarily. #[allow(clippy::module_inception)] -pub mod pass; +mod pass; pub mod registry; mod specialization; pub mod statistics; From d57c768967445cb68ed4947537e9202d4c64c1d9 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 28 Apr 2025 16:55:22 -0300 Subject: [PATCH 43/56] Refactor: Change match bool in favor of if ... else Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass/pass.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index b3faac12b..e6686098e 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -180,9 +180,10 @@ pub enum PostPassStatus { impl From for PostPassStatus { fn from(ir_was_changed: bool) -> Self { - match ir_was_changed { - true => PostPassStatus::IRChanged, - false => PostPassStatus::IRUnchanged, + if ir_was_changed { + PostPassStatus::IRChanged + } else { + PostPassStatus::IRUnchanged } } } From 768acb8920016b80e756bdf806d6e6ecba829249 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 28 Apr 2025 17:48:16 -0300 Subject: [PATCH 44/56] Move "with_*" functions inside the ::new() constructor. Also, made the private. This was done in order to make the construction logic "coupled" to avoid decoupled logic problems. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 13 ++++++++----- hir/src/pass/manager.rs | 4 +--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 03f6f3702..1899abdaa 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -39,6 +39,7 @@ enum PassFilter { Certain(Vec), } +#[allow(dead_code)] #[derive(Default, Debug)] enum OpFilter { /// Print all operations @@ -56,14 +57,15 @@ enum OpFilter { impl Print { pub fn new(config: &IRPrintingConfig) -> Option { - if config.print_ir_after_all + let print = if config.print_ir_after_all || !config.print_ir_after_pass.is_empty() || config.print_ir_after_modified { Some(Self::default()) } else { None - } + }; + print.map(|p| p.with_pass_filter(config)).map(|p| p.with_symbol_filter(config)) } pub fn with_type_filter(mut self) -> Self { @@ -73,14 +75,15 @@ impl Print { self } + #[allow(dead_code)] /// Create a printer that only prints `Symbol` operations containing `name` - pub fn with_symbol_matching(mut self, name: &'static str) -> Self { + fn with_symbol_matching(mut self, name: &'static str) -> Self { self.filter = Some(OpFilter::Symbol(Some(name))); self } #[allow(unused_mut)] - pub fn with_symbol_filter(mut self, _config: &IRPrintingConfig) -> Self { + fn with_symbol_filter(mut self, _config: &IRPrintingConfig) -> Self { // NOTE: At the moment, symbol filtering is not processed by the CLI. However, were it to be // added, it could be done inside this function self.with_all_symbols() @@ -91,7 +94,7 @@ impl Print { self } - pub fn with_pass_filter(mut self, config: &IRPrintingConfig) -> Self { + fn with_pass_filter(mut self, config: &IRPrintingConfig) -> Self { let is_ir_filter_set = if config.print_ir_after_all { self.pass_filter = Some(PassFilter::All); true diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 087129c58..376e9fb1f 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -194,9 +194,7 @@ impl PassManager { } pub fn enable_ir_printing(mut self, config: IRPrintingConfig) -> Self { - let print = Print::new(&config) - .map(|p| p.with_pass_filter(&config)) - .map(|p| p.with_symbol_filter(&config)); + let print = Print::new(&config); if let Some(print) = print { let print = Box::new(print); From 3c951e7a25fb4b977833cb21efed423c04c176a2 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Tue, 29 Apr 2025 09:36:24 -0300 Subject: [PATCH 45/56] Rename: enum PassFilter -> enum SelectedPasses Alos update documentaiton for enum PassFilter Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 1899abdaa..304328527 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -25,18 +25,18 @@ use crate::{ #[derive(Default)] pub struct Print { filter: Option, - pass_filter: Option, + pass_filter: Option, target: Option, only_when_modified: bool, } -/// Filter for the different passes. +/// Select passes for IR printing. #[derive(Debug)] -enum PassFilter { - /// Print IR regardless of which pass is executed. +enum SelectedPasses { + /// Select all passes for IR Printing. All, - /// Only print IR if the pass's name is present in the vector. - Certain(Vec), + /// Just select a subset of passes for IR printing. + Just(Vec), } #[allow(dead_code)] @@ -96,10 +96,10 @@ impl Print { fn with_pass_filter(mut self, config: &IRPrintingConfig) -> Self { let is_ir_filter_set = if config.print_ir_after_all { - self.pass_filter = Some(PassFilter::All); + self.pass_filter = Some(SelectedPasses::All); true } else if !config.print_ir_after_pass.is_empty() { - self.pass_filter = Some(PassFilter::Certain(config.print_ir_after_pass.clone())); + self.pass_filter = Some(SelectedPasses::Just(config.print_ir_after_pass.clone())); true } else { false @@ -111,7 +111,7 @@ impl Print { // any IR pass filter flag; then we assume that the desired behavior is to set the "all // pass" filter. if !is_ir_filter_set { - self.pass_filter = Some(PassFilter::All); + self.pass_filter = Some(SelectedPasses::All); } }; @@ -164,8 +164,8 @@ impl Print { fn pass_filter(&self, pass: &dyn OperationPass) -> bool { match &self.pass_filter { - Some(PassFilter::All) => true, - Some(PassFilter::Certain(passes)) => passes.iter().any(|p| { + Some(SelectedPasses::All) => true, + Some(SelectedPasses::Just(passes)) => passes.iter().any(|p| { if let Some(p_type) = pass.pass_id() { *p == p_type } else { From d4972637ffeb52b13b2a2202cbccceeddd1941fa Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Tue, 29 Apr 2025 09:47:27 -0300 Subject: [PATCH 46/56] Add InstructionScheduler to the PassIdentifier enum, but commented out. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass/pass.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index e6686098e..b53cb25c8 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -147,6 +147,8 @@ where pub enum PassIdentifier { Canonicalizer, ControlFlowSink, + // NOTE: Not yet enabled. + // InstructionScheduler, LiftControlFlowToSCF, OpToOpPassAdaptor, SinkOperandDefs, From 21aa490c1c14ea12ea4d177e2ef62f03a0d8d9a4 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 5 May 2025 09:12:01 -0300 Subject: [PATCH 47/56] Remove InstructionScheduler variant (and comment) from PassIdentifier enum Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass/pass.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index b53cb25c8..e6686098e 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -147,8 +147,6 @@ where pub enum PassIdentifier { Canonicalizer, ControlFlowSink, - // NOTE: Not yet enabled. - // InstructionScheduler, LiftControlFlowToSCF, OpToOpPassAdaptor, SinkOperandDefs, From 09aa5afb9694dea5cf86de419265dfd0641baeeb Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 5 May 2025 09:21:14 -0300 Subject: [PATCH 48/56] Update Print struct member variable names. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 304328527..7b6b5c2f0 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -24,8 +24,8 @@ use crate::{ /// A `Pass` which prints IR it is run on, based on provided configuration. #[derive(Default)] pub struct Print { - filter: Option, - pass_filter: Option, + op_filter: Option, + selected_passes: Option, target: Option, only_when_modified: bool, } @@ -71,14 +71,14 @@ impl Print { pub fn with_type_filter(mut self) -> Self { let dialect = ::dialect_name(); let op = ::name(); - self.filter = Some(OpFilter::Type { dialect, op }); + self.op_filter = Some(OpFilter::Type { dialect, op }); self } #[allow(dead_code)] /// Create a printer that only prints `Symbol` operations containing `name` fn with_symbol_matching(mut self, name: &'static str) -> Self { - self.filter = Some(OpFilter::Symbol(Some(name))); + self.op_filter = Some(OpFilter::Symbol(Some(name))); self } @@ -90,16 +90,16 @@ impl Print { } fn with_all_symbols(mut self) -> Self { - self.filter = Some(OpFilter::All); + self.op_filter = Some(OpFilter::All); self } fn with_pass_filter(mut self, config: &IRPrintingConfig) -> Self { let is_ir_filter_set = if config.print_ir_after_all { - self.pass_filter = Some(SelectedPasses::All); + self.selected_passes = Some(SelectedPasses::All); true } else if !config.print_ir_after_pass.is_empty() { - self.pass_filter = Some(SelectedPasses::Just(config.print_ir_after_pass.clone())); + self.selected_passes = Some(SelectedPasses::Just(config.print_ir_after_pass.clone())); true } else { false @@ -111,7 +111,7 @@ impl Print { // any IR pass filter flag; then we assume that the desired behavior is to set the "all // pass" filter. if !is_ir_filter_set { - self.pass_filter = Some(SelectedPasses::All); + self.selected_passes = Some(SelectedPasses::All); } }; @@ -129,7 +129,7 @@ impl Print { } fn print_ir(&self, op: EntityRef<'_, Operation>) { - match self.filter { + match self.op_filter { Some(OpFilter::All) => { let target = self.target.as_deref().unwrap_or("printer"); log::trace!(target: target, "{op}"); @@ -163,7 +163,7 @@ impl Print { } fn pass_filter(&self, pass: &dyn OperationPass) -> bool { - match &self.pass_filter { + match &self.selected_passes { Some(SelectedPasses::All) => true, Some(SelectedPasses::Just(passes)) => passes.iter().any(|p| { if let Some(p_type) = pass.pass_id() { From e0669e892bec7685ac148e3382cfc0ecd24e5876 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 5 May 2025 10:13:44 -0300 Subject: [PATCH 49/56] Error if conflicting options are passed. Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass/manager.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 376e9fb1f..246accccc 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -51,6 +51,13 @@ impl TryFrom<&Options> for IRPrintingConfig { .iter() .map(|a| a.try_into()) .collect::, Report>>()?; + if options.print_ir_after_all && !pass_filters.is_empty() { + return Err(Report::msg( + "Flags `print_ir_after_all` and `print_ir_after_pass` are mutually exclusive. \ + Please select only one." + .to_string(), + )); + }; Ok(IRPrintingConfig { print_ir_after_all: options.print_ir_after_all, print_ir_after_pass: pass_filters, From 3838fb6aef803c02b22f52ee37bc5ad7025d96e6 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 5 May 2025 10:50:56 -0300 Subject: [PATCH 50/56] Add documentation for the Print struct Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 7b6b5c2f0..74fdaaff3 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -21,7 +21,11 @@ use crate::{ EntityRef, Operation, OperationName, OperationRef, }; -/// A `Pass` which prints IR it is run on, based on provided configuration. +/// Struct that handles IR printing, based on provided configuration. +/// It is configured via the following CLI flags: +/// -Z print-ir-after-all: Enable IR printing for every pass. +/// -Z print-ir-after-pass: Enable IR printing only for some passes. +/// -Z print-ir-after-modified.: Only print the IR if it has been been modified. #[derive(Default)] pub struct Print { op_filter: Option, From 6522baedfd6e6a059789ed54224a8e65f5580e72 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 5 May 2025 12:23:41 -0300 Subject: [PATCH 51/56] Change Print struct documentation Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 74fdaaff3..499552bad 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -21,11 +21,30 @@ use crate::{ EntityRef, Operation, OperationName, OperationRef, }; -/// Struct that handles IR printing, based on provided configuration. -/// It is configured via the following CLI flags: -/// -Z print-ir-after-all: Enable IR printing for every pass. -/// -Z print-ir-after-pass: Enable IR printing only for some passes. -/// -Z print-ir-after-modified.: Only print the IR if it has been been modified. +/// Struct that handles IR printing, based on the IRPRintingConfig passed during creation. Currently, +/// this struct is managed by the PassManager's PassInstrumentor, which calls the Print struct via +/// its PassInstrumentation trait implementation. +/// +///Print currently implements the following PassInstrumentation functions: +/// - run_before_pipeline: This gets called before the entire pass pipeline, but it will only +/// display output if the "only_when_modified" option is set to true. This is done to showcase the +/// IR before any pass gets applied. +/// - run_before_pass: This is called before each pass. This function is used to display the IR +/// right before a specific Pass gets applied. If "only_when_modified" is set to true, this +/// function will not display output. +/// - run_after_pass: This is called after each pass. This function is used to display the +/// modifications done to the IR by a specific Pass. +/// +/// NOTE: "only_when_modified" disables "run_before_pass" because it prints the IR before the pass +/// is run, so there is no opportunity to check if the pass will actually modify the IR (since this +/// is done later in the pipeline). So, in order to display the changes, "run_before_pipeline" is +/// enabled. This has the effect of showing the IR before any passes, and then showing the IR only +/// when it gets modified. the IR twice. All those functions call the [Print::print_ir] function, +/// which handles IR printing based on Print's configuration. +/// +/// It is important to note that the [Print::print_ir] function is only calling the Display +/// implementation of the specified operation. This means, that the format in which the Operation is +/// displayed is handled by each Operation. #[derive(Default)] pub struct Print { op_filter: Option, From 1249baa260eea11af9f7d30c24f54365722ab7b8 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 5 May 2025 14:31:09 -0300 Subject: [PATCH 52/56] Move selected_passes up to reflect the flow better Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 499552bad..807cbd83f 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -47,16 +47,18 @@ use crate::{ /// displayed is handled by each Operation. #[derive(Default)] pub struct Print { - op_filter: Option, selected_passes: Option, - target: Option, + only_when_modified: bool, + op_filter: Option, + + target: Option, } -/// Select passes for IR printing. +/// Which passes are enabled for IR printing. #[derive(Debug)] enum SelectedPasses { - /// Select all passes for IR Printing. + /// Enable all passes for IR Printing. All, /// Just select a subset of passes for IR printing. Just(Vec), From ee4694039c8c01cf8809622e20a957156260b3e5 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Mon, 5 May 2025 15:39:54 -0300 Subject: [PATCH 53/56] Re-write Print struct documentation Signed-off-by: Tomas Fabrizio Orsi --- hir/src/pass.rs | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 807cbd83f..cfc0f9a31 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -21,30 +21,23 @@ use crate::{ EntityRef, Operation, OperationName, OperationRef, }; -/// Struct that handles IR printing, based on the IRPRintingConfig passed during creation. Currently, -/// this struct is managed by the PassManager's PassInstrumentor, which calls the Print struct via -/// its PassInstrumentation trait implementation. +/// Handles IR printing, based on the [`IRPrintingConfig`] passed in +/// [Print::new]. Currently, this struct is managed by the [`PassManager`]'s [`PassInstrumentor`], +/// which calls the Print struct via its [`PassInstrumentation`] trait implementation. /// -///Print currently implements the following PassInstrumentation functions: -/// - run_before_pipeline: This gets called before the entire pass pipeline, but it will only -/// display output if the "only_when_modified" option is set to true. This is done to showcase the -/// IR before any pass gets applied. -/// - run_before_pass: This is called before each pass. This function is used to display the IR -/// right before a specific Pass gets applied. If "only_when_modified" is set to true, this -/// function will not display output. -/// - run_after_pass: This is called after each pass. This function is used to display the -/// modifications done to the IR by a specific Pass. +/// The configuration passed by [`IRPrintingConfig`] controls *when* the IR gets displayed, rather +/// than *how*. The display format itself depends on the `Display` implementation done by each +/// [`Operation`]. /// -/// NOTE: "only_when_modified" disables "run_before_pass" because it prints the IR before the pass -/// is run, so there is no opportunity to check if the pass will actually modify the IR (since this -/// is done later in the pipeline). So, in order to display the changes, "run_before_pipeline" is -/// enabled. This has the effect of showing the IR before any passes, and then showing the IR only -/// when it gets modified. the IR twice. All those functions call the [Print::print_ir] function, -/// which handles IR printing based on Print's configuration. +/// [`Print::selected_passes`] controls which passes are selected to be printable. This means that +/// those selected passes will run all the configured filters; which will determine whether +/// the pass displays the IR or not. The available options are [`SelectedPasses::All`] to enable all +/// the passes and [`SelectedPasses::Just`] to enable a select set of passes. /// -/// It is important to note that the [Print::print_ir] function is only calling the Display -/// implementation of the specified operation. This means, that the format in which the Operation is -/// displayed is handled by each Operation. +/// The filters that run on the selected passes are: +/// - [`Print::only_when_modified`] will only print the IR if said pass modified the IR. +/// +/// - [`Print::op_filter`] will only display a specific subset of operations. #[derive(Default)] pub struct Print { selected_passes: Option, From f841e70f1bbdd29b480094d3acbf5e25d0355980 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Fri, 9 May 2025 15:47:19 -0300 Subject: [PATCH 54/56] Move PostPassStatus to PassExecutionState Signed-off-by: Tomas Fabrizio Orsi --- dialects/hir/src/transforms/spill.rs | 20 ++++++++++++++++---- dialects/scf/src/transforms/cfg_to_scf.rs | 9 ++++++--- hir-transform/src/canonicalization.rs | 9 ++++++--- hir-transform/src/sccp.rs | 12 +++++++----- hir-transform/src/sink.rs | 13 ++++++++----- hir/src/pass.rs | 6 ++++-- hir/src/pass/instrumentation.rs | 12 ++++++------ hir/src/pass/manager.rs | 11 +++++------ hir/src/pass/pass.rs | 20 ++++++++++++++++---- 9 files changed, 74 insertions(+), 38 deletions(-) diff --git a/dialects/hir/src/transforms/spill.rs b/dialects/hir/src/transforms/spill.rs index 188f92a1c..b1b52239e 100644 --- a/dialects/hir/src/transforms/spill.rs +++ b/dialects/hir/src/transforms/spill.rs @@ -35,14 +35,15 @@ impl Pass for TransformSpills { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result<(), Report> { let function = op.into_entity_ref(); log::debug!(target: "insert-spills", "computing and inserting spills for {}", function.as_operation()); if function.is_declaration() { log::debug!(target: "insert-spills", "function has no body, no spills needed!"); state.preserved_analyses_mut().preserve_all(); - return Ok(PostPassStatus::IRUnchanged); + state.set_post_pass_status(PostPassStatus::IRUnchanged); + return Ok(()); } let mut analysis = state.analysis_manager().get_analysis_for::()?; @@ -50,7 +51,8 @@ impl Pass for TransformSpills { if !analysis.has_spills() { log::debug!(target: "insert-spills", "no spills needed!"); state.preserved_analyses_mut().preserve_all(); - return Ok(PostPassStatus::IRUnchanged); + state.set_post_pass_status(PostPassStatus::IRUnchanged); + return Ok(()); } // Take ownership of the spills analysis so that we can mutate the analysis state during @@ -66,7 +68,17 @@ impl Pass for TransformSpills { let op = function.as_operation_ref(); drop(function); - transforms::transform_spills(op, analysis, &mut interface, state.analysis_manager().clone()) + + let transform_result = transforms::transform_spills( + op, + analysis, + &mut interface, + state.analysis_manager().clone(), + )?; + + state.set_post_pass_status(transform_result); + + return Ok(()); } } diff --git a/dialects/scf/src/transforms/cfg_to_scf.rs b/dialects/scf/src/transforms/cfg_to_scf.rs index cddbce58d..73af4035d 100644 --- a/dialects/scf/src/transforms/cfg_to_scf.rs +++ b/dialects/scf/src/transforms/cfg_to_scf.rs @@ -55,7 +55,7 @@ impl Pass for LiftControlFlowToSCF { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result<(), Report> { let mut transformation = ControlFlowToSCFTransformation; let mut changed = false; @@ -134,7 +134,8 @@ impl Pass for LiftControlFlowToSCF { }); if result.was_interrupted() { - return result.into_result().map(|_| PostPassStatus::IRUnchanged); + state.set_post_pass_status(PostPassStatus::IRUnchanged); + return result.into_result(); } log::debug!( @@ -145,7 +146,9 @@ impl Pass for LiftControlFlowToSCF { state.preserved_analyses_mut().preserve_all(); } - Ok(changed.into()) + state.set_post_pass_status(changed.into()); + + Ok(()) } } diff --git a/hir-transform/src/canonicalization.rs b/hir-transform/src/canonicalization.rs index 16ca8838d..830d9a2fb 100644 --- a/hir-transform/src/canonicalization.rs +++ b/hir-transform/src/canonicalization.rs @@ -97,10 +97,11 @@ impl Pass for Canonicalizer { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result<(), Report> { let Some(rewrites) = self.rewrites.as_ref() else { log::debug!("skipping canonicalization as there are no rewrite patterns to apply"); - return Ok(PostPassStatus::IRUnchanged); + state.set_post_pass_status(PostPassStatus::IRUnchanged); + return Ok(()); }; let op = { let ptr = op.as_operation_ref(); @@ -146,7 +147,9 @@ impl Pass for Canonicalizer { changed } }; + let ir_changed = changed.into(); + state.set_post_pass_status(ir_changed); - Ok(changed.into()) + Ok(()) } } diff --git a/hir-transform/src/sccp.rs b/hir-transform/src/sccp.rs index e701de2e4..21dc92007 100644 --- a/hir-transform/src/sccp.rs +++ b/hir-transform/src/sccp.rs @@ -1,5 +1,5 @@ use midenc_hir::{ - pass::{Pass, PassExecutionState, PassIdentifier, PostPassStatus}, + pass::{Pass, PassExecutionState, PassIdentifier}, patterns::NoopRewriterListener, BlockRef, Builder, EntityMut, OpBuilder, Operation, OperationFolder, OperationName, RegionList, Report, SmallVec, ValueRef, @@ -42,7 +42,7 @@ impl Pass for SparseConditionalConstantPropagation { &mut self, mut op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result<(), Report> { // Run sparse constant propagation + dead code analysis let mut solver = DataFlowSolver::default(); solver.load::(); @@ -60,9 +60,9 @@ impl SparseConditionalConstantPropagation { fn rewrite( &mut self, op: &mut Operation, - _state: &mut PassExecutionState, + state: &mut PassExecutionState, solver: &DataFlowSolver, - ) -> Result { + ) -> Result<(), Report> { let mut worklist = SmallVec::<[BlockRef; 8]>::default(); let add_to_worklist = |regions: &RegionList, worklist: &mut SmallVec<[BlockRef; 8]>| { @@ -128,7 +128,9 @@ impl SparseConditionalConstantPropagation { } } - Ok(replaced_any.into()) + state.set_post_pass_status(replaced_any.into()); + + Ok(()) } } diff --git a/hir-transform/src/sink.rs b/hir-transform/src/sink.rs index e601bb253..93ddcc9db 100644 --- a/hir-transform/src/sink.rs +++ b/hir-transform/src/sink.rs @@ -100,7 +100,7 @@ impl Pass for ControlFlowSink { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result<(), Report> { let op = op.into_entity_ref(); log::debug!(target: "control-flow-sink", "sinking operations in {op}"); @@ -137,7 +137,9 @@ impl Pass for ControlFlowSink { ); }); - Ok(sunk) + state.set_post_pass_status(sunk); + + Ok(()) } } @@ -180,8 +182,8 @@ impl Pass for SinkOperandDefs { fn run_on_operation( &mut self, op: EntityMut<'_, Self::Target>, - _state: &mut PassExecutionState, - ) -> Result { + state: &mut PassExecutionState, + ) -> Result<(), Report> { let operation = op.as_operation_ref(); drop(op); @@ -409,7 +411,8 @@ impl Pass for SinkOperandDefs { } } - Ok(changed) + state.set_post_pass_status(changed); + Ok(()) } } diff --git a/hir/src/pass.rs b/hir/src/pass.rs index cfc0f9a31..69b8e9ff5 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -194,7 +194,7 @@ impl Print { } } - fn should_print(&self, pass: &dyn OperationPass, ir_changed: PostPassStatus) -> bool { + fn should_print(&self, pass: &dyn OperationPass, ir_changed: &PostPassStatus) -> bool { let pass_filter = self.pass_filter(pass); // Always print, unless "only_when_modified" has been set and there have not been changes. @@ -236,8 +236,10 @@ impl PassInstrumentation for Print { &mut self, pass: &dyn OperationPass, op: &OperationRef, - changed: PostPassStatus, + post_execution_state: &PassExecutionState, ) { + let changed = post_execution_state.post_pass_status(); + if self.should_print(pass, changed) { log::trace!("After the {} pass", pass.name()); let op = op.borrow(); diff --git a/hir/src/pass/instrumentation.rs b/hir/src/pass/instrumentation.rs index 7db417f25..ccddaac05 100644 --- a/hir/src/pass/instrumentation.rs +++ b/hir/src/pass/instrumentation.rs @@ -5,7 +5,7 @@ use compact_str::CompactString; use smallvec::SmallVec; use super::OperationPass; -use crate::{pass::PostPassStatus, OperationName, OperationRef}; +use crate::{pass::PassExecutionState, OperationName, OperationRef}; #[allow(unused_variables)] pub trait PassInstrumentation { @@ -27,7 +27,7 @@ pub trait PassInstrumentation { &mut self, pass: &dyn OperationPass, op: &OperationRef, - changed: PostPassStatus, + post_execution_state: &PassExecutionState, ) { } fn run_after_pass_failed(&mut self, pass: &dyn OperationPass, op: &OperationRef) {} @@ -66,9 +66,9 @@ impl PassInstrumentation for Box

{ &mut self, pass: &dyn OperationPass, op: &OperationRef, - changed: PostPassStatus, + post_execution_state: &PassExecutionState, ) { - (**self).run_after_pass(pass, op, changed); + (**self).run_after_pass(pass, op, post_execution_state); } fn run_after_pass_failed(&mut self, pass: &dyn OperationPass, op: &OperationRef) { @@ -115,9 +115,9 @@ impl PassInstrumentor { &self, pass: &dyn OperationPass, op: &OperationRef, - changed: PostPassStatus, + post_execution_state: &PassExecutionState, ) { - self.instrument(|pi| pi.run_after_pass(pass, op, changed)); + self.instrument(|pi| pi.run_after_pass(pass, op, post_execution_state)); } pub fn run_after_pass_failed(&self, pass: &dyn OperationPass, op: &OperationRef) { diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index 246accccc..d6b98517a 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -992,8 +992,7 @@ impl OpToOpPassAdaptor { if result.is_err() { instrumentor.run_after_pass_failed(pass, &op); } else { - let in_result = result.as_ref().unwrap_or(&PostPassStatus::IRUnchanged); - instrumentor.run_after_pass(pass, &op, *in_result); + instrumentor.run_after_pass(pass, &op, &execution_state); } } @@ -1015,7 +1014,7 @@ impl OpToOpPassAdaptor { op: OperationRef, state: &mut PassExecutionState, verify: bool, - ) -> Result { + ) -> Result<(), Report> { let analysis_manager = state.analysis_manager(); let instrumentor = analysis_manager.pass_instrumentor(); let parent_info = PipelineParentInfo { @@ -1049,8 +1048,8 @@ impl OpToOpPassAdaptor { } } } - - Ok(PostPassStatus::IRUnchanged) + state.set_post_pass_status(PostPassStatus::IRUnchanged); + Ok(()) } } @@ -1091,7 +1090,7 @@ impl Pass for OpToOpPassAdaptor { &mut self, _op: EntityMut<'_, Operation>, _state: &mut PassExecutionState, - ) -> Result { + ) -> Result<(), Report> { unreachable!("unexpected call to `Pass::run_on_operation` for OpToOpPassAdaptor") } } diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index e6686098e..8c14aee97 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -47,7 +47,7 @@ pub trait OperationPass { &mut self, op: OperationRef, state: &mut PassExecutionState, - ) -> Result; + ) -> Result<(), Report>; fn run_pipeline( &mut self, pipeline: &mut OpPassManager, @@ -128,7 +128,7 @@ where &mut self, mut op: OperationRef, state: &mut PassExecutionState, - ) -> Result { + ) -> Result<(), Report> { let op = <

::Target as PassTarget>::into_target_mut(&mut op);

::run_on_operation(self, op, state) } @@ -283,7 +283,7 @@ pub trait Pass: Sized + Any { &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result; + ) -> Result<(), Report>; /// Schedule an arbitrary pass pipeline on the provided operation. /// /// This can be invoke any time in a pass to dynamic schedule more passes. The provided @@ -388,7 +388,7 @@ where &mut self, op: EntityMut<'_, Self::Target>, state: &mut PassExecutionState, - ) -> Result { + ) -> Result<(), Report> { (**self).run_on_operation(op, state) } @@ -419,6 +419,7 @@ pub struct PassExecutionState { // rooted at the provided operation. #[allow(unused)] pipeline_executor: Option>, + post_pass_status: PostPassStatus, } impl PassExecutionState { pub fn new( @@ -433,6 +434,7 @@ impl PassExecutionState { analysis_manager, preserved_analyses: Default::default(), pipeline_executor, + post_pass_status: PostPassStatus::IRUnchanged, } } @@ -461,6 +463,16 @@ impl PassExecutionState { &mut self.preserved_analyses } + #[inline(always)] + pub fn post_pass_status(&self) -> &PostPassStatus { + &self.post_pass_status + } + + #[inline(always)] + pub fn set_post_pass_status(&mut self, post_pass_status: PostPassStatus) { + self.post_pass_status = post_pass_status; + } + pub fn run_pipeline( &mut self, pipeline: &mut OpPassManager, From 14f4fc255dc8c357af987c0a943df9e08f96e186 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Fri, 9 May 2025 17:10:10 -0300 Subject: [PATCH 55/56] rename: IRChanged -> Changed; IRUnchanged -> Unchanged Signed-off-by: Tomas Fabrizio Orsi --- dialects/hir/src/transforms/spill.rs | 4 ++-- dialects/scf/src/transforms/cfg_to_scf.rs | 2 +- hir-transform/src/canonicalization.rs | 2 +- hir-transform/src/sink.rs | 8 ++++---- hir-transform/src/spill.rs | 2 +- hir/src/pass.rs | 2 +- hir/src/pass/manager.rs | 2 +- hir/src/pass/pass.rs | 10 +++++----- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/dialects/hir/src/transforms/spill.rs b/dialects/hir/src/transforms/spill.rs index b1b52239e..cb23de94e 100644 --- a/dialects/hir/src/transforms/spill.rs +++ b/dialects/hir/src/transforms/spill.rs @@ -42,7 +42,7 @@ impl Pass for TransformSpills { if function.is_declaration() { log::debug!(target: "insert-spills", "function has no body, no spills needed!"); state.preserved_analyses_mut().preserve_all(); - state.set_post_pass_status(PostPassStatus::IRUnchanged); + state.set_post_pass_status(PostPassStatus::Unchanged); return Ok(()); } let mut analysis = @@ -51,7 +51,7 @@ impl Pass for TransformSpills { if !analysis.has_spills() { log::debug!(target: "insert-spills", "no spills needed!"); state.preserved_analyses_mut().preserve_all(); - state.set_post_pass_status(PostPassStatus::IRUnchanged); + state.set_post_pass_status(PostPassStatus::Unchanged); return Ok(()); } diff --git a/dialects/scf/src/transforms/cfg_to_scf.rs b/dialects/scf/src/transforms/cfg_to_scf.rs index 73af4035d..5ea147d58 100644 --- a/dialects/scf/src/transforms/cfg_to_scf.rs +++ b/dialects/scf/src/transforms/cfg_to_scf.rs @@ -134,7 +134,7 @@ impl Pass for LiftControlFlowToSCF { }); if result.was_interrupted() { - state.set_post_pass_status(PostPassStatus::IRUnchanged); + state.set_post_pass_status(PostPassStatus::Unchanged); return result.into_result(); } diff --git a/hir-transform/src/canonicalization.rs b/hir-transform/src/canonicalization.rs index 830d9a2fb..1ccbbe957 100644 --- a/hir-transform/src/canonicalization.rs +++ b/hir-transform/src/canonicalization.rs @@ -100,7 +100,7 @@ impl Pass for Canonicalizer { ) -> Result<(), Report> { let Some(rewrites) = self.rewrites.as_ref() else { log::debug!("skipping canonicalization as there are no rewrite patterns to apply"); - state.set_post_pass_status(PostPassStatus::IRUnchanged); + state.set_post_pass_status(PostPassStatus::Unchanged); return Ok(()); }; let op = { diff --git a/hir-transform/src/sink.rs b/hir-transform/src/sink.rs index 93ddcc9db..f72f64e0a 100644 --- a/hir-transform/src/sink.rs +++ b/hir-transform/src/sink.rs @@ -109,7 +109,7 @@ impl Pass for ControlFlowSink { let dominfo = state.analysis_manager().get_analysis::()?; - let mut sunk = PostPassStatus::IRUnchanged; + let mut sunk = PostPassStatus::Unchanged; operation.raw_prewalk_all::(|op: OperationRef| { let regions_to_sink = { let op = op.borrow(); @@ -195,7 +195,7 @@ impl Pass for SinkOperandDefs { // then process the worklist, moving everything into position. let mut worklist = alloc::collections::VecDeque::default(); - let mut changed = PostPassStatus::IRUnchanged; + let mut changed = PostPassStatus::Unchanged; // Visit ops in "true" post-order (i.e. block bodies are visited bottom-up). operation.raw_postwalk_all::(|operation: OperationRef| { // Determine if any of this operation's operands represent one of the following: @@ -320,7 +320,7 @@ impl Pass for SinkOperandDefs { log::trace!(target: "sink-operand-defs", " rewriting operand {operand_value} as {replacement}"); operand.borrow_mut().set(replacement); - changed = PostPassStatus::IRChanged; + changed = PostPassStatus::Changed; // If no other uses of this value remain, then remove the original // operation, as it is now dead. if !operand_value.borrow().is_used() { @@ -367,7 +367,7 @@ impl Pass for SinkOperandDefs { log::trace!(target: "sink-operand-defs", " rewriting operand {operand_value} as {replacement}"); sink_state.replacements.insert(operand_value, replacement); operand.borrow_mut().set(replacement); - changed = PostPassStatus::IRChanged; + changed = PostPassStatus::Changed; } else { log::trace!(target: "sink-operand-defs", " defining op is a constant with no other uses, moving into place"); // The original op can be moved diff --git a/hir-transform/src/spill.rs b/hir-transform/src/spill.rs index 58c5ba978..65f4f814e 100644 --- a/hir-transform/src/spill.rs +++ b/hir-transform/src/spill.rs @@ -252,7 +252,7 @@ pub fn transform_spills( )?; } - Ok(PostPassStatus::IRChanged) + Ok(PostPassStatus::Changed) } fn rewrite_single_block_spills( diff --git a/hir/src/pass.rs b/hir/src/pass.rs index 69b8e9ff5..d8b6b65e2 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -199,7 +199,7 @@ impl Print { // Always print, unless "only_when_modified" has been set and there have not been changes. let modification_filter = - !matches!((self.only_when_modified, ir_changed), (true, PostPassStatus::IRUnchanged)); + !matches!((self.only_when_modified, ir_changed), (true, PostPassStatus::Unchanged)); pass_filter && modification_filter } diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index d6b98517a..cfe15b6a7 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -1048,7 +1048,7 @@ impl OpToOpPassAdaptor { } } } - state.set_post_pass_status(PostPassStatus::IRUnchanged); + state.set_post_pass_status(PostPassStatus::Unchanged); Ok(()) } } diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index 8c14aee97..7a8fe294f 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -174,16 +174,16 @@ impl TryFrom<&String> for PassIdentifier { #[derive(Debug, PartialEq, Clone, Copy)] pub enum PostPassStatus { - IRUnchanged, - IRChanged, + Unchanged, + Changed, } impl From for PostPassStatus { fn from(ir_was_changed: bool) -> Self { if ir_was_changed { - PostPassStatus::IRChanged + PostPassStatus::Changed } else { - PostPassStatus::IRUnchanged + PostPassStatus::Unchanged } } } @@ -434,7 +434,7 @@ impl PassExecutionState { analysis_manager, preserved_analyses: Default::default(), pipeline_executor, - post_pass_status: PostPassStatus::IRUnchanged, + post_pass_status: PostPassStatus::Unchanged, } } From 6f832799d77492af4efd19fbff8f2da2f8930453 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Fri, 9 May 2025 17:33:45 -0300 Subject: [PATCH 56/56] Remove PassIdentifier enum and use SmallVec instead of Vec. Now to compare pass equality, the `pass.name()` method is used. Signed-off-by: Tomas Fabrizio Orsi --- dialects/hir/src/transforms/spill.rs | 6 +--- dialects/scf/src/transforms/cfg_to_scf.rs | 6 +--- hir-transform/src/canonicalization.rs | 6 +--- hir-transform/src/sccp.rs | 6 +--- hir-transform/src/sink.rs | 10 +----- hir/src/pass.rs | 19 ++++------ hir/src/pass/manager.rs | 26 +++++++------- hir/src/pass/pass.rs | 43 +---------------------- 8 files changed, 25 insertions(+), 97 deletions(-) diff --git a/dialects/hir/src/transforms/spill.rs b/dialects/hir/src/transforms/spill.rs index cb23de94e..973e63fa8 100644 --- a/dialects/hir/src/transforms/spill.rs +++ b/dialects/hir/src/transforms/spill.rs @@ -3,7 +3,7 @@ use alloc::rc::Rc; use midenc_hir::{ adt::SmallDenseMap, dialects::builtin::{Function, FunctionRef, LocalVariable}, - pass::{Pass, PassExecutionState, PassIdentifier, PostPassStatus}, + pass::{Pass, PassExecutionState, PostPassStatus}, BlockRef, BuilderExt, EntityMut, Op, OpBuilder, OperationName, OperationRef, Report, Rewriter, SourceSpan, Spanned, Symbol, ValueRef, }; @@ -19,10 +19,6 @@ impl Pass for TransformSpills { "transform-spills" } - fn pass_id(&self) -> Option { - Some(PassIdentifier::TransformSpills) - } - fn argument(&self) -> &'static str { "transform-spills" } diff --git a/dialects/scf/src/transforms/cfg_to_scf.rs b/dialects/scf/src/transforms/cfg_to_scf.rs index 5ea147d58..f54aa919b 100644 --- a/dialects/scf/src/transforms/cfg_to_scf.rs +++ b/dialects/scf/src/transforms/cfg_to_scf.rs @@ -7,7 +7,7 @@ use midenc_hir::{ diagnostics::Severity, dialects::builtin, dominance::DominanceInfo, - pass::{Pass, PassExecutionState, PassIdentifier, PostPassStatus}, + pass::{Pass, PassExecutionState, PostPassStatus}, Builder, EntityMut, Forward, Op, Operation, OperationName, OperationRef, RawWalk, Report, SmallVec, Spanned, Type, ValueRange, ValueRef, WalkResult, }; @@ -35,10 +35,6 @@ impl Pass for LiftControlFlowToSCF { "lift-control-flow" } - fn pass_id(&self) -> Option { - Some(PassIdentifier::LiftControlFlowToSCF) - } - fn argument(&self) -> &'static str { "lift-control-flow" } diff --git a/hir-transform/src/canonicalization.rs b/hir-transform/src/canonicalization.rs index 1ccbbe957..8adac3ed4 100644 --- a/hir-transform/src/canonicalization.rs +++ b/hir-transform/src/canonicalization.rs @@ -1,7 +1,7 @@ use alloc::{boxed::Box, format, rc::Rc}; use midenc_hir::{ - pass::{OperationPass, Pass, PassExecutionState, PassIdentifier, PostPassStatus}, + pass::{OperationPass, Pass, PassExecutionState, PostPassStatus}, patterns::{self, FrozenRewritePatternSet, GreedyRewriteConfig, RewritePatternSet}, Context, EntityMut, Operation, OperationName, Report, Spanned, }; @@ -62,10 +62,6 @@ impl Pass for Canonicalizer { "canonicalizer" } - fn pass_id(&self) -> Option { - Some(PassIdentifier::Canonicalizer) - } - fn argument(&self) -> &'static str { "canonicalizer" } diff --git a/hir-transform/src/sccp.rs b/hir-transform/src/sccp.rs index 21dc92007..a80a6e194 100644 --- a/hir-transform/src/sccp.rs +++ b/hir-transform/src/sccp.rs @@ -1,5 +1,5 @@ use midenc_hir::{ - pass::{Pass, PassExecutionState, PassIdentifier}, + pass::{Pass, PassExecutionState}, patterns::NoopRewriterListener, BlockRef, Builder, EntityMut, OpBuilder, Operation, OperationFolder, OperationName, RegionList, Report, SmallVec, ValueRef, @@ -26,10 +26,6 @@ impl Pass for SparseConditionalConstantPropagation { "sparse-conditional-constant-propagation" } - fn pass_id(&self) -> Option { - Some(PassIdentifier::SparseConditionalConstantPropagation) - } - fn argument(&self) -> &'static str { "sparse-conditional-constant-propagation" } diff --git a/hir-transform/src/sink.rs b/hir-transform/src/sink.rs index f72f64e0a..c5c97ef1f 100644 --- a/hir-transform/src/sink.rs +++ b/hir-transform/src/sink.rs @@ -4,7 +4,7 @@ use midenc_hir::{ adt::SmallDenseMap, dominance::DominanceInfo, matchers::{self, Matcher}, - pass::{Pass, PassExecutionState, PassIdentifier, PostPassStatus}, + pass::{Pass, PassExecutionState, PostPassStatus}, traits::{ConstantLike, Terminator}, Backward, Builder, EntityMut, Forward, FxHashSet, OpBuilder, Operation, OperationName, OperationRef, ProgramPoint, RawWalk, Region, RegionBranchOpInterface, @@ -84,10 +84,6 @@ impl Pass for ControlFlowSink { "control-flow-sink" } - fn pass_id(&self) -> Option { - Some(PassIdentifier::ControlFlowSink) - } - fn argument(&self) -> &'static str { "control-flow-sink" } @@ -167,10 +163,6 @@ impl Pass for SinkOperandDefs { "sink-operand-defs" } - fn pass_id(&self) -> Option { - Some(PassIdentifier::SinkOperandDefs) - } - fn argument(&self) -> &'static str { "sink-operand-defs" } diff --git a/hir/src/pass.rs b/hir/src/pass.rs index d8b6b65e2..8e2182a8f 100644 --- a/hir/src/pass.rs +++ b/hir/src/pass.rs @@ -7,19 +7,18 @@ pub mod registry; mod specialization; pub mod statistics; +use alloc::string::String; + pub use self::{ analysis::{Analysis, AnalysisManager, OperationAnalysis, PreservedAnalyses}, instrumentation::{PassInstrumentation, PassInstrumentor, PipelineParentInfo}, manager::{IRPrintingConfig, Nesting, OpPassManager, PassDisplayMode, PassManager}, - pass::{OperationPass, Pass, PassExecutionState, PassIdentifier, PostPassStatus}, + pass::{OperationPass, Pass, PassExecutionState, PostPassStatus}, registry::{PassInfo, PassPipelineInfo}, specialization::PassTarget, statistics::{PassStatistic, Statistic, StatisticValue}, }; -use crate::{ - alloc::{string::String, vec::Vec}, - EntityRef, Operation, OperationName, OperationRef, -}; +use crate::{EntityRef, Operation, OperationName, OperationRef, SmallVec}; /// Handles IR printing, based on the [`IRPrintingConfig`] passed in /// [Print::new]. Currently, this struct is managed by the [`PassManager`]'s [`PassInstrumentor`], @@ -54,7 +53,7 @@ enum SelectedPasses { /// Enable all passes for IR Printing. All, /// Just select a subset of passes for IR printing. - Just(Vec), + Just(SmallVec<[String; 1]>), } #[allow(dead_code)] @@ -183,13 +182,7 @@ impl Print { fn pass_filter(&self, pass: &dyn OperationPass) -> bool { match &self.selected_passes { Some(SelectedPasses::All) => true, - Some(SelectedPasses::Just(passes)) => passes.iter().any(|p| { - if let Some(p_type) = pass.pass_id() { - *p == p_type - } else { - false - } - }), + Some(SelectedPasses::Just(passes)) => passes.iter().any(|p| pass.name() == *p), None => false, } } diff --git a/hir/src/pass/manager.rs b/hir/src/pass/manager.rs index cfe15b6a7..c0e2b9ad5 100644 --- a/hir/src/pass/manager.rs +++ b/hir/src/pass/manager.rs @@ -1,4 +1,10 @@ -use alloc::{boxed::Box, collections::BTreeMap, format, rc::Rc, string::ToString, vec::Vec}; +use alloc::{ + boxed::Box, + collections::BTreeMap, + format, + rc::Rc, + string::{String, ToString}, +}; use compact_str::{CompactString, ToCompactString}; use midenc_session::{diagnostics::Severity, Options}; @@ -9,7 +15,7 @@ use super::{ PassInstrumentor, PipelineParentInfo, Statistic, }; use crate::{ - pass::{pass::PassIdentifier, PostPassStatus, Print}, + pass::{PostPassStatus, Print}, traits::IsolatedFromAbove, Context, EntityMut, OpPrintingFlags, OpRegistration, Operation, OperationName, OperationRef, Report, @@ -37,7 +43,7 @@ pub struct IRPrintingConfig { pub print_after_only_on_failure: bool, // NOTE: Taken from the Options struct pub print_ir_after_all: bool, - pub print_ir_after_pass: Vec, + pub print_ir_after_pass: SmallVec<[String; 1]>, pub print_ir_after_modified: bool, pub flags: OpPrintingFlags, } @@ -46,11 +52,8 @@ impl TryFrom<&Options> for IRPrintingConfig { type Error = Report; fn try_from(options: &Options) -> Result { - let pass_filters: Vec = options - .print_ir_after_pass - .iter() - .map(|a| a.try_into()) - .collect::, Report>>()?; + let pass_filters = options.print_ir_after_pass.clone(); + if options.print_ir_after_all && !pass_filters.is_empty() { return Err(Report::msg( "Flags `print_ir_after_all` and `print_ir_after_pass` are mutually exclusive. \ @@ -58,9 +61,10 @@ impl TryFrom<&Options> for IRPrintingConfig { .to_string(), )); }; + Ok(IRPrintingConfig { print_ir_after_all: options.print_ir_after_all, - print_ir_after_pass: pass_filters, + print_ir_after_pass: pass_filters.into(), print_ir_after_modified: options.print_ir_after_modified, ..Default::default() }) @@ -1060,10 +1064,6 @@ impl Pass for OpToOpPassAdaptor { crate::interner::Symbol::intern(self.name()).as_str() } - fn pass_id(&self) -> Option { - None - } - #[inline(always)] fn target_name(&self, _context: &Context) -> Option { None diff --git a/hir/src/pass/pass.rs b/hir/src/pass/pass.rs index 7a8fe294f..224ee31c6 100644 --- a/hir/src/pass/pass.rs +++ b/hir/src/pass/pass.rs @@ -1,4 +1,4 @@ -use alloc::{boxed::Box, format, rc::Rc}; +use alloc::{boxed::Box, rc::Rc}; use core::{any::Any, fmt}; use super::*; @@ -16,8 +16,6 @@ pub trait OperationPass { fn into_any(self: Box) -> Box; fn name(&self) -> &'static str; - fn pass_id(&self) -> Option; - fn argument(&self) -> &'static str { // NOTE: Could we compute an argument string from the type name? "" @@ -64,10 +62,6 @@ where

::as_any(self) } - fn pass_id(&self) -> Option { -

::pass_id(self) - } - fn as_any_mut(&mut self) -> &mut dyn Any {

::as_any_mut(self) } @@ -143,35 +137,6 @@ where } } -#[derive(Debug, PartialEq, Clone, Copy)] -pub enum PassIdentifier { - Canonicalizer, - ControlFlowSink, - LiftControlFlowToSCF, - OpToOpPassAdaptor, - SinkOperandDefs, - SparseConditionalConstantPropagation, - TransformSpills, -} - -impl TryFrom<&String> for PassIdentifier { - type Error = Report; - - fn try_from(pass_name: &String) -> Result { - match pass_name.as_str() { - "canonicalizer" => Ok(PassIdentifier::Canonicalizer), - "control-flow-sink" => Ok(PassIdentifier::ControlFlowSink), - "lift-control-flow" => Ok(PassIdentifier::LiftControlFlowToSCF), - "sink-operand-defs" => Ok(PassIdentifier::SinkOperandDefs), - "sparse-conditional-constant-propagation" => { - Ok(PassIdentifier::SparseConditionalConstantPropagation) - } - "transform-spills" => Ok(PassIdentifier::TransformSpills), - _ => Err(Report::msg(format!("'{pass_name}' unrecognized pass."))), - } - } -} - #[derive(Debug, PartialEq, Clone, Copy)] pub enum PostPassStatus { Unchanged, @@ -296,8 +261,6 @@ pub trait Pass: Sized + Any { ) -> Result<(), Report> { state.run_pipeline(pipeline, op) } - - fn pass_id(&self) -> Option; } impl

Pass for Box

@@ -324,10 +287,6 @@ where (**self).name() } - fn pass_id(&self) -> Option { - (**self).pass_id() - } - #[inline] fn argument(&self) -> &'static str { (**self).argument()