Skip to content

Commit 7e5e8ee

Browse files
(refactor): moving unsafe panic optimization into new dataflow format
1 parent 06f2a64 commit 7e5e8ee

File tree

4 files changed

+165
-41
lines changed

4 files changed

+165
-41
lines changed

crates/cairo-lang-lowering/src/analysis/backward.rs

Lines changed: 117 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
44
use std::collections::HashMap;
55

6-
use crate::analysis::Analyzer;
7-
use crate::{BlockEnd, BlockId, Lowered};
6+
use crate::analysis::{Analyzer, DataflowAnalyzer, Direction, Edge, StatementLocation};
7+
use crate::{Block, BlockEnd, BlockId, Lowered, MatchInfo, Statement, VarRemapping, VarUsage};
88

99
/// Main analysis type that allows traversing the flow backwards.
1010
pub struct BackAnalysis<'db, 'a, TAnalyzer: Analyzer<'db, 'a>> {
@@ -103,3 +103,118 @@ impl<'db, 'a, TAnalyzer: Analyzer<'db, 'a>> BackAnalysis<'db, 'a, TAnalyzer> {
103103
}
104104
}
105105
}
106+
107+
/// Backward analysis runner using `DataflowAnalyzer`.
108+
///
109+
/// This is an adapter that wraps `BackAnalysis` internally, translating
110+
/// between the new `DataflowAnalyzer` trait and the legacy `Analyzer` trait.
111+
/// Once all analyses are migrated, this can be simplified to inline the
112+
/// traversal logic directly.
113+
pub struct DataflowBackAnalysis<'db, 'a, TAnalyzer: DataflowAnalyzer<'db, 'a>> {
114+
inner: BackAnalysis<'db, 'a, AnalyzerAdapter<'db, 'a, TAnalyzer>>,
115+
}
116+
117+
impl<'db, 'a, TAnalyzer: DataflowAnalyzer<'db, 'a>> DataflowBackAnalysis<'db, 'a, TAnalyzer> {
118+
/// Creates a new DataflowBackAnalysis instance.
119+
pub fn new(lowered: &'a Lowered<'db>, analyzer: &'a mut TAnalyzer) -> Self {
120+
assert!(
121+
TAnalyzer::DIRECTION == Direction::Backward,
122+
"DataflowBackAnalysis requires a backward analyzer"
123+
);
124+
let adapter = AnalyzerAdapter { analyzer, lowered };
125+
Self { inner: BackAnalysis::new(lowered, adapter) }
126+
}
127+
128+
/// Runs the analysis and returns the result.
129+
///
130+
/// For backward analysis, returns the info at the function entry (root block).
131+
pub fn run(mut self) -> TAnalyzer::Info {
132+
self.inner.get_root_info()
133+
}
134+
}
135+
136+
/// Adapter that implements the legacy `Analyzer` trait by delegating to `DataflowAnalyzer`.
137+
pub struct AnalyzerAdapter<'db, 'a, TAnalyzer: DataflowAnalyzer<'db, 'a>> {
138+
pub analyzer: &'a mut TAnalyzer,
139+
lowered: &'a Lowered<'db>,
140+
}
141+
142+
impl<'db, 'a, TAnalyzer: DataflowAnalyzer<'db, 'a>> Analyzer<'db, 'a>
143+
for AnalyzerAdapter<'db, 'a, TAnalyzer>
144+
{
145+
type Info = TAnalyzer::Info;
146+
147+
fn visit_block_start(&mut self, info: &mut Self::Info, block_id: BlockId, _block: &Block<'db>) {
148+
// Get block from lowered with correct lifetime 'a.
149+
let block = &self.lowered.blocks[block_id];
150+
// First apply transfer_block (which processes statements in reverse for backward).
151+
self.analyzer.transfer_block(info, block_id, block);
152+
// Then call the block start hook.
153+
self.analyzer.visit_block_start(info, block_id, block);
154+
}
155+
156+
fn visit_stmt(
157+
&mut self,
158+
_info: &mut Self::Info,
159+
_statement_location: StatementLocation,
160+
_stmt: &'a Statement<'db>,
161+
) {
162+
// Statements are handled by transfer_block in visit_block_start.
163+
// This is intentionally empty.
164+
}
165+
166+
fn visit_goto(
167+
&mut self,
168+
info: &mut Self::Info,
169+
_statement_location: StatementLocation,
170+
target_block_id: BlockId,
171+
remapping: &'a VarRemapping<'db>,
172+
) {
173+
let edge = Edge::Goto { target: target_block_id, remapping };
174+
*info = self.analyzer.transfer_edge(info, &edge);
175+
}
176+
177+
fn merge_match(
178+
&mut self,
179+
statement_location: StatementLocation,
180+
match_info: &'a MatchInfo<'db>,
181+
infos: impl Iterator<Item = Self::Info>,
182+
) -> Self::Info {
183+
// Transfer each arm's info through its edge, then merge.
184+
let transferred_infos: Vec<_> = match_info
185+
.arms()
186+
.iter()
187+
.zip(infos)
188+
.map(|(arm, info)| {
189+
let edge = Edge::MatchArm { arm, match_info };
190+
let transferred = self.analyzer.transfer_edge(&info, &edge);
191+
(arm.block_id, transferred)
192+
})
193+
.collect();
194+
self.analyzer.merge(self.lowered, statement_location, transferred_infos.into_iter())
195+
}
196+
197+
fn info_from_return(
198+
&mut self,
199+
statement_location: StatementLocation,
200+
vars: &'a [VarUsage<'db>],
201+
) -> Self::Info {
202+
let block_end = &self.lowered.blocks[statement_location.0].end;
203+
let BlockEnd::Return(_, location) = block_end else {
204+
unreachable!("Unexpected block end type")
205+
};
206+
let location = *location;
207+
let info = self.analyzer.initial_info(statement_location.0, block_end);
208+
self.analyzer.transfer_edge(&info, &Edge::Return { vars, location })
209+
}
210+
211+
fn info_from_panic(
212+
&mut self,
213+
statement_location: StatementLocation,
214+
var: &VarUsage<'db>,
215+
) -> Self::Info {
216+
let block_end = &self.lowered.blocks[statement_location.0].end;
217+
let info = self.analyzer.initial_info(statement_location.0, block_end);
218+
self.analyzer.transfer_edge(&info, &Edge::Panic { var: *var })
219+
}
220+
}

crates/cairo-lang-lowering/src/analysis/core.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ use crate::{
2525
pub type StatementLocation = (BlockId, usize);
2626

2727
/// The direction of dataflow analysis.
28-
#[expect(dead_code)]
2928
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
3029
pub enum Direction {
3130
Forward,
@@ -37,7 +36,6 @@ pub enum Direction {
3736
/// Each variant captures the specific information needed for that edge type,
3837
/// enabling analyzers to handle variable introductions and remappings.
3938
#[derive(Debug)]
40-
#[expect(dead_code)]
4139
pub enum Edge<'db, 'a> {
4240
/// A goto edge with variable remapping.
4341
Goto { target: BlockId, remapping: &'a VarRemapping<'db> },
@@ -77,7 +75,6 @@ pub enum Edge<'db, 'a> {
7775
/// }
7876
/// }
7977
/// ```
80-
#[expect(dead_code)]
8178
pub trait DataflowAnalyzer<'db, 'a> {
8279
/// The analysis state/info type.
8380
type Info: Clone;

crates/cairo-lang-lowering/src/analysis/mod.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
//! This module provides generic analysis frameworks that can be used by various
44
//! optimization passes and semantic checks.
55
6-
mod backward;
7-
mod core;
6+
pub mod backward;
7+
pub mod core;
88

9-
pub use backward::BackAnalysis;
9+
pub use core::{DataflowAnalyzer, Direction, Edge};
10+
11+
pub use backward::{BackAnalysis, DataflowBackAnalysis};
1012

1113
use crate::{Block, BlockId, MatchInfo, Statement, VarRemapping, VarUsage};
1214

crates/cairo-lang-lowering/src/optimizations/early_unsafe_panic.rs

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,12 @@ use std::collections::HashSet;
77
use cairo_lang_defs::ids::ExternFunctionId;
88
use cairo_lang_filesystem::flag::FlagsGroup;
99
use cairo_lang_semantic::helper::ModuleHelper;
10-
use itertools::zip_eq;
1110
use salsa::Database;
1211

13-
use crate::analysis::{Analyzer, BackAnalysis, StatementLocation};
12+
use crate::analysis::core::StatementLocation;
13+
use crate::analysis::{DataflowAnalyzer, DataflowBackAnalysis, Direction};
1414
use crate::ids::{LocationId, SemanticFunctionIdEx};
15-
use crate::{
16-
BlockEnd, BlockId, Lowered, MatchExternInfo, MatchInfo, Statement, StatementCall, VarUsage,
17-
};
15+
use crate::{BlockEnd, BlockId, Lowered, MatchExternInfo, MatchInfo, Statement, StatementCall};
1816

1917
/// Adds an early unsafe_panic when we detect that `return` is unreachable from a certain point in
2018
/// the code. This step is needed to avoid issues with undroppable references in Sierra to CASM.
@@ -32,13 +30,14 @@ pub fn early_unsafe_panic<'db>(db: &'db dyn Database, lowered: &mut Lowered<'db>
3230
core.submodule("internal").extern_function_id("trace"),
3331
]);
3432

35-
let ctx = UnsafePanicContext { db, fixes: vec![], libfuncs_with_sideffect };
36-
let mut analysis = BackAnalysis::new(lowered, ctx);
37-
let fixes = if let ReachableSideEffects::Unreachable(location) = analysis.get_root_info() {
38-
vec![((BlockId::root(), 0), location)]
39-
} else {
40-
analysis.analyzer.fixes
41-
};
33+
let mut ctx = UnsafePanicContext { db, libfuncs_with_sideffect, fixes: Vec::new() };
34+
let result = DataflowBackAnalysis::new(lowered, &mut ctx).run();
35+
let UnsafePanicContext { mut fixes, .. } = ctx;
36+
37+
// If the entry point itself is unreachable, add a fix for it.
38+
if let ReachableSideEffects::Unreachable(location) = result {
39+
fixes.push(((BlockId::root(), 0), location));
40+
}
4241

4342
let panic_func_id = core.submodule("panics").function_id("unsafe_panic", vec![]).lowered(db);
4443
for ((block_id, statement_idx), location) in fixes {
@@ -60,7 +59,7 @@ pub struct UnsafePanicContext<'db> {
6059
db: &'db dyn Database,
6160

6261
/// The list of blocks where we can insert unsafe_panic.
63-
fixes: Vec<(StatementLocation, LocationId<'db>)>,
62+
pub fixes: Vec<(StatementLocation, LocationId<'db>)>,
6463

6564
/// libfuncs with side effects that we need to ignore.
6665
libfuncs_with_sideffect: HashSet<ExternFunctionId<'db>>,
@@ -84,57 +83,68 @@ impl<'db> UnsafePanicContext<'db> {
8483
}
8584

8685
/// Can this state lead to a return or a statement with side effect.
87-
#[derive(Clone, Default, PartialEq, Debug)]
86+
#[derive(Clone, Copy, Default, PartialEq, Debug)]
8887
pub enum ReachableSideEffects<'db> {
8988
/// Some return statement or statement with side effect is reachable.
9089
#[default]
9190
Reachable,
9291
/// No return statement or statement with side effect is reachable.
93-
/// holds the location of the closest match with no returning arms.
92+
/// Holds the location of the closest match with no returning arms.
9493
Unreachable(LocationId<'db>),
9594
}
9695

97-
impl<'db> Analyzer<'db, '_> for UnsafePanicContext<'db> {
96+
impl<'db, 'a> DataflowAnalyzer<'db, 'a> for UnsafePanicContext<'db> {
9897
type Info = ReachableSideEffects<'db>;
98+
const DIRECTION: Direction = Direction::Backward;
9999

100-
fn visit_stmt(
100+
fn transfer_stmt(
101101
&mut self,
102102
info: &mut Self::Info,
103103
statement_location: StatementLocation,
104-
stmt: &Statement<'db>,
104+
stmt: &'a Statement<'db>,
105105
) {
106106
if self.has_side_effects(stmt)
107-
&& let ReachableSideEffects::Unreachable(locations) = *info
107+
&& let ReachableSideEffects::Unreachable(loc) = *info
108108
{
109-
self.fixes.push((statement_location, locations));
110-
*info = ReachableSideEffects::Reachable
109+
self.fixes.push((statement_location, loc));
110+
*info = ReachableSideEffects::Reachable;
111111
}
112112
}
113113

114-
fn merge_match(
114+
fn merge(
115115
&mut self,
116+
lowered: &Lowered<'db>,
116117
statement_location: StatementLocation,
117-
match_info: &MatchInfo<'db>,
118-
infos: impl Iterator<Item = Self::Info>,
118+
infos: impl Iterator<Item = (BlockId, Self::Info)>,
119119
) -> Self::Info {
120-
let mut res = ReachableSideEffects::Unreachable(*match_info.location());
121-
for (arm, info) in zip_eq(match_info.arms(), infos) {
122-
match info {
120+
let mut result = ReachableSideEffects::default();
121+
let mut all_unreachable = true;
122+
123+
for (src, reachability) in infos {
124+
match reachability {
123125
ReachableSideEffects::Reachable => {
124-
res = ReachableSideEffects::Reachable;
126+
all_unreachable = false;
127+
}
128+
ReachableSideEffects::Unreachable(loc) => {
129+
// Fix at the entry of this unreachable branch.
130+
self.fixes.push(((src, 0), loc));
125131
}
126-
ReachableSideEffects::Unreachable(l) => self.fixes.push(((arm.block_id, 0), l)),
127132
}
128133
}
129134

130-
if let ReachableSideEffects::Unreachable(location) = res {
135+
// All branches are unreachable (or there are no branches, e.g., match on `never`).
136+
// Use the match location as the source of unreachability.
137+
if all_unreachable {
138+
// In a backward runner location is always available
139+
let location = self.block_entry_location(lowered, statement_location.0);
140+
result = ReachableSideEffects::Unreachable(location);
131141
self.fixes.push((statement_location, location));
132142
}
133143

134-
res
144+
result
135145
}
136146

137-
fn info_from_return(&mut self, _: StatementLocation, _vars: &[VarUsage<'db>]) -> Self::Info {
138-
ReachableSideEffects::Reachable
147+
fn initial_info(&mut self, _block_id: BlockId, _block_end: &'a BlockEnd<'db>) -> Self::Info {
148+
ReachableSideEffects::default()
139149
}
140150
}

0 commit comments

Comments
 (0)