Skip to content

Commit ef9788b

Browse files
authored
BE-264: HashQL: Rename SROA to ForwardSubstitution for better semantic clarity (#8230)
1 parent d451c18 commit ef9788b

35 files changed

+985
-917
lines changed

libs/@local/hashql/compiletest/src/suite/mir_pass_transform_sroa.rs renamed to libs/@local/hashql/compiletest/src/suite/mir_pass_transform_forward_substitution.rs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use hashql_mir::{
1111
context::MirContext,
1212
def::{DefId, DefIdSlice, DefIdVec},
1313
intern::Interner,
14-
pass::{Changed, TransformPass as _, transform::Sroa},
14+
pass::{Changed, TransformPass as _, transform::ForwardSubstitution},
1515
};
1616

1717
use super::{
@@ -24,7 +24,7 @@ use crate::suite::{
2424
mir_reify::{d2_output_enabled, mir_format_d2, mir_format_text, mir_spawn_d2},
2525
};
2626

27-
pub(crate) fn mir_pass_transform_sroa<'heap>(
27+
pub(crate) fn mir_pass_transform_forward_substitution<'heap>(
2828
heap: &'heap Heap,
2929
expr: Expr<'heap>,
3030
interner: &Interner<'heap>,
@@ -42,7 +42,7 @@ pub(crate) fn mir_pass_transform_sroa<'heap>(
4242
diagnostics: DiagnosticIssues::new(),
4343
};
4444

45-
let mut pass = Sroa::new_in(&mut scratch);
45+
let mut pass = ForwardSubstitution::new_in(&mut scratch);
4646
for body in bodies.as_mut_slice() {
4747
let _: Changed = pass.run(&mut context, body);
4848
}
@@ -51,19 +51,19 @@ pub(crate) fn mir_pass_transform_sroa<'heap>(
5151
Ok((root, bodies, scratch))
5252
}
5353

54-
pub(crate) struct MirPassTransformSroa;
54+
pub(crate) struct MirPassTransformForwardSubstitution;
5555

56-
impl Suite for MirPassTransformSroa {
56+
impl Suite for MirPassTransformForwardSubstitution {
5757
fn priority(&self) -> usize {
5858
1
5959
}
6060

6161
fn name(&self) -> &'static str {
62-
"mir/pass/transform/sroa"
62+
"mir/pass/transform/forward-substitution"
6363
}
6464

6565
fn description(&self) -> &'static str {
66-
"Scalar Replacement of Aggregates in the MIR"
66+
"Forward Substitution in the MIR"
6767
}
6868

6969
fn secondary_file_extensions(&self) -> &[&str] {
@@ -88,7 +88,7 @@ impl Suite for MirPassTransformSroa {
8888
let mut buffer = Vec::new();
8989
let mut d2 = d2_output_enabled(self, suite_directives, reports).then(mir_spawn_d2);
9090

91-
let (root, bodies, _) = mir_pass_transform_sroa(
91+
let (root, bodies, _) = mir_pass_transform_forward_substitution(
9292
heap,
9393
expr,
9494
&interner,
@@ -100,11 +100,15 @@ impl Suite for MirPassTransformSroa {
100100
diagnostics,
101101
)?;
102102

103-
let _ = writeln!(buffer, "\n{}\n", Header::new("MIR after SROA"));
103+
let _ = writeln!(
104+
buffer,
105+
"\n{}\n",
106+
Header::new("MIR after Forward Substitution")
107+
);
104108
mir_format_text(heap, &environment, &mut buffer, root, &bodies);
105109

106110
if let Some((mut writer, handle)) = d2 {
107-
writeln!(writer, "final: 'MIR after SROA' {{")
111+
writeln!(writer, "final: 'MIR after Forward Substitution' {{")
108112
.expect("should be able to write to buffer");
109113
mir_format_d2(heap, &environment, &mut writer, root, &bodies);
110114
writeln!(writer, "}}").expect("should be able to write to buffer");

libs/@local/hashql/compiletest/src/suite/mir_pass_transform_inst_simplify.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use hashql_mir::{
1616

1717
use super::{
1818
RunContext, Suite, SuiteDiagnostic, common::process_issues,
19-
mir_pass_transform_sroa::mir_pass_transform_sroa,
19+
mir_pass_transform_forward_substitution::mir_pass_transform_forward_substitution,
2020
};
2121
use crate::suite::{
2222
common::Header,
@@ -32,8 +32,14 @@ pub(crate) fn mir_pass_transform_inst_simplify<'heap>(
3232
environment: &mut Environment<'heap>,
3333
diagnostics: &mut Vec<SuiteDiagnostic>,
3434
) -> Result<(DefId, DefIdVec<Body<'heap>>, Scratch), SuiteDiagnostic> {
35-
let (root, mut bodies, mut scratch) =
36-
mir_pass_transform_sroa(heap, expr, interner, render, environment, diagnostics)?;
35+
let (root, mut bodies, mut scratch) = mir_pass_transform_forward_substitution(
36+
heap,
37+
expr,
38+
interner,
39+
render,
40+
environment,
41+
diagnostics,
42+
)?;
3743

3844
let mut context = MirContext {
3945
heap,

libs/@local/hashql/compiletest/src/suite/mod.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ mod mir_pass_analysis_data_dependency;
2424
mod mir_pass_transform_administrative_reduction;
2525
mod mir_pass_transform_cfg_simplify;
2626
mod mir_pass_transform_dse;
27+
mod mir_pass_transform_forward_substitution;
2728
mod mir_pass_transform_inst_simplify;
28-
mod mir_pass_transform_sroa;
2929
mod mir_reify;
3030
mod parse_syntax_dump;
3131

@@ -59,8 +59,8 @@ use self::{
5959
mir_pass_transform_administrative_reduction::MirPassTransformAdministrativeReduction,
6060
mir_pass_transform_cfg_simplify::MirPassTransformCfgSimplify,
6161
mir_pass_transform_dse::MirPassTransformDse,
62-
mir_pass_transform_inst_simplify::MirPassTransformInstSimplify,
63-
mir_pass_transform_sroa::MirPassTransformSroa, mir_reify::MirReifySuite,
62+
mir_pass_transform_forward_substitution::MirPassTransformForwardSubstitution,
63+
mir_pass_transform_inst_simplify::MirPassTransformInstSimplify, mir_reify::MirReifySuite,
6464
parse_syntax_dump::ParseSyntaxDumpSuite,
6565
};
6666
use crate::executor::TrialError;
@@ -159,8 +159,8 @@ const SUITES: &[&dyn Suite] = &[
159159
&MirPassTransformAdministrativeReduction,
160160
&MirPassTransformCfgSimplify,
161161
&MirPassTransformDse,
162+
&MirPassTransformForwardSubstitution,
162163
&MirPassTransformInstSimplify,
163-
&MirPassTransformSroa,
164164
&MirReifySuite,
165165
&ParseSyntaxDumpSuite,
166166
];

libs/@local/hashql/mir/benches/transform.rs

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use hashql_mir::{
2121
op,
2222
pass::{
2323
TransformPass,
24-
transform::{CfgSimplify, DeadStoreElimination, InstSimplify, Sroa},
24+
transform::{CfgSimplify, DeadStoreElimination, ForwardSubstitution, InstSimplify},
2525
},
2626
};
2727

@@ -385,17 +385,17 @@ fn cfg_simplify(criterion: &mut Criterion) {
385385
});
386386
}
387387

388-
fn sroa(criterion: &mut Criterion) {
389-
let mut group = criterion.benchmark_group("sroa");
388+
fn forward_substitution(criterion: &mut Criterion) {
389+
let mut group = criterion.benchmark_group("forward_substitution");
390390

391391
group.bench_function("linear", |bencher| {
392-
run(bencher, create_linear_cfg, Sroa::new());
392+
run(bencher, create_linear_cfg, ForwardSubstitution::new());
393393
});
394394
group.bench_function("diamond", |bencher| {
395-
run(bencher, create_diamond_cfg, Sroa::new());
395+
run(bencher, create_diamond_cfg, ForwardSubstitution::new());
396396
});
397397
group.bench_function("complex", |bencher| {
398-
run(bencher, create_complex_cfg, Sroa::new());
398+
run(bencher, create_complex_cfg, ForwardSubstitution::new());
399399
});
400400
}
401401

@@ -441,7 +441,10 @@ fn pipeline(criterion: &mut Criterion) {
441441

442442
run_bencher(bencher, create_linear_cfg, |context, body| {
443443
let mut changed = CfgSimplify::new_in(&mut scratch).run(context, body);
444-
changed = cmp::max(changed, Sroa::new_in(&mut scratch).run(context, body));
444+
changed = cmp::max(
445+
changed,
446+
ForwardSubstitution::new_in(&mut scratch).run(context, body),
447+
);
445448
changed = cmp::max(changed, InstSimplify::new().run(context, body));
446449
changed = cmp::max(
447450
changed,
@@ -456,7 +459,10 @@ fn pipeline(criterion: &mut Criterion) {
456459

457460
run_bencher(bencher, create_diamond_cfg, |context, body| {
458461
let mut changed = CfgSimplify::new_in(&mut scratch).run(context, body);
459-
changed = cmp::max(changed, Sroa::new_in(&mut scratch).run(context, body));
462+
changed = cmp::max(
463+
changed,
464+
ForwardSubstitution::new_in(&mut scratch).run(context, body),
465+
);
460466
changed = cmp::max(changed, InstSimplify::new().run(context, body));
461467
changed = cmp::max(
462468
changed,
@@ -471,7 +477,10 @@ fn pipeline(criterion: &mut Criterion) {
471477

472478
run_bencher(bencher, create_complex_cfg, |context, body| {
473479
let mut changed = CfgSimplify::new_in(&mut scratch).run(context, body);
474-
changed = cmp::max(changed, Sroa::new_in(&mut scratch).run(context, body));
480+
changed = cmp::max(
481+
changed,
482+
ForwardSubstitution::new_in(&mut scratch).run(context, body),
483+
);
475484
changed = cmp::max(changed, InstSimplify::new().run(context, body));
476485
changed = cmp::max(
477486
changed,
@@ -483,5 +492,12 @@ fn pipeline(criterion: &mut Criterion) {
483492
});
484493
}
485494

486-
criterion_group!(benches, cfg_simplify, sroa, dse, inst_simplify, pipeline);
495+
criterion_group!(
496+
benches,
497+
cfg_simplify,
498+
forward_substitution,
499+
dse,
500+
inst_simplify,
501+
pipeline
502+
);
487503
criterion_main!(benches);

libs/@local/hashql/mir/src/pass/analysis/callgraph/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@
3333
//! `_1 = @fn; _2 = _1(...)`) produce an [`Opaque`] edge at the assignment site, not an
3434
//! [`Apply`] edge at the call site.
3535
//!
36-
//! This is intentional: the analysis is designed to run after SROA, which propagates function
37-
//! references through locals, eliminating most indirect call patterns.
36+
//! This is intentional: the analysis is designed to run after forward substitution, which
37+
//! propagates function references through locals, eliminating most indirect call patterns.
3838
//!
3939
//! [`Opaque`]: CallKind::Opaque
4040
//! [`DataDependencyAnalysis`]: super::DataDependencyAnalysis

libs/@local/hashql/mir/src/pass/transform/administrative_reduction/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ use crate::{
7272
def::{DefId, DefIdSlice, DefIdVec},
7373
pass::{
7474
Changed, GlobalTransformPass, TransformPass, analysis::CallGraph,
75-
transform::cp::propagate_block_params,
75+
transform::copy_propagation::propagate_block_params,
7676
},
7777
visit::VisitorMut as _,
7878
};

libs/@local/hashql/mir/src/pass/transform/cp/mod.rs renamed to libs/@local/hashql/mir/src/pass/transform/copy_propagation/mod.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,32 @@
1+
//! Copy and constant propagation transformation pass.
2+
//!
3+
//! This pass propagates constant values through the MIR by tracking which locals hold known
4+
//! constants and substituting uses of those locals with the constant values directly.
5+
//!
6+
//! Unlike [`ForwardSubstitution`], this pass does not perform full data dependency analysis and
7+
//! cannot resolve values through projections or chained access paths. It is faster but less
8+
//! comprehensive, making it suitable for quick constant folding in simpler cases.
9+
//!
10+
//! # Algorithm
11+
//!
12+
//! The pass operates in a single forward traversal (reverse postorder):
13+
//!
14+
//! 1. For each block, propagates constants through block parameters when all predecessors pass the
15+
//! same constant value
16+
//! 2. For each assignment `_x = <operand>`, if the operand is a constant or a local known to hold a
17+
//! constant, records that `_x` holds that constant
18+
//! 3. For each use of a local known to hold a constant, substitutes the use with the constant
19+
//!
20+
//! # Limitations
21+
//!
22+
//! - Does not handle projections: `_2 = (_1,); use(_2.0)` is not simplified
23+
//! - Does not perform fix-point iteration for loops
24+
//! - Only tracks constants, not arbitrary value equivalences
25+
//!
26+
//! For more comprehensive value propagation including projections, see [`ForwardSubstitution`].
27+
//!
28+
//! [`ForwardSubstitution`]: super::ForwardSubstitution
29+
130
#[cfg(test)]
231
mod tests;
332

libs/@local/hashql/mir/src/pass/transform/cp/tests.rs renamed to libs/@local/hashql/mir/src/pass/transform/copy_propagation/tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ fn assert_cp_pass<'heap>(
6565

6666
let dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
6767
let mut settings = Settings::clone_current();
68-
settings.set_snapshot_path(dir.join("tests/ui/pass/cp"));
68+
settings.set_snapshot_path(dir.join("tests/ui/pass/copy_propagation"));
6969
settings.set_prepend_module_to_snapshot(false);
7070

7171
let _drop = settings.bind_to_scope();

libs/@local/hashql/mir/src/pass/transform/sroa.rs renamed to libs/@local/hashql/mir/src/pass/transform/forward_substitution.rs

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,24 @@
1-
//! Scalar Replacement of Aggregates (SROA) transformation pass.
1+
//! Forward substitution transformation pass.
22
//!
33
//! This pass resolves place operands to their ultimate sources by leveraging data dependency
44
//! analysis. It effectively "looks through" assignments, projections, and block parameters to
5-
//! replace places with either:
5+
//! substitute places with either:
66
//!
77
//! - **Constants**: When the place can be traced back to a constant value
88
//! - **Simplified places**: When the place can be traced to a simpler location
99
//!
10+
//! This is a more comprehensive form of value propagation than [`CopyPropagation`], as it handles
11+
//! projections, chained access paths, and closure environments through full data dependency
12+
//! analysis.
13+
//!
1014
//! # Algorithm
1115
//!
1216
//! The pass operates by:
1317
//!
1418
//! 1. Running [`DataDependencyAnalysis`] to build a graph of data flow relationships
1519
//! 2. Walking all operands in the MIR body
1620
//! 3. For each place operand, resolving it through the dependency graph to find its source
17-
//! 4. Replacing the operand with the resolved value (constant or simplified place)
21+
//! 4. Substituting the operand with the resolved value (constant or simplified place)
1822
//!
1923
//! The resolution handles several cases:
2024
//!
@@ -46,14 +50,19 @@
4650
//!
4751
//! # Interaction with Other Passes
4852
//!
49-
//! SROA runs after [`CfgSimplify`] in the optimization pipeline, which ensures that unreachable
50-
//! code paths have been eliminated before resolution. This allows SROA to make more precise
51-
//! determinations about constant values at join points.
53+
//! Forward substitution runs after [`CfgSimplify`] in the optimization pipeline, which ensures
54+
//! that unreachable code paths have been eliminated before resolution. This allows the pass to
55+
//! make more precise determinations about constant values at join points.
56+
//!
57+
//! When combined with dead store elimination (DSE), forward substitution enables SROA-like
58+
//! decomposition of aggregates: uses are substituted with their original values, and DSE
59+
//! then removes the now-dead aggregate constructions.
5260
//!
53-
//! When block parameters receive the same constant from all predecessors, SROA resolves uses
54-
//! of that parameter to the constant. When predecessors diverge (provide different constants),
55-
//! the place is preserved unchanged.
61+
//! When block parameters receive the same constant from all predecessors, forward substitution
62+
//! resolves uses of that parameter to the constant. When predecessors diverge (provide different
63+
//! constants), the place is preserved unchanged.
5664
//!
65+
//! [`CopyPropagation`]: super::CopyPropagation
5766
//! [`DataDependencyAnalysis`]: crate::pass::analysis::DataDependencyAnalysis
5867
//! [`CfgSimplify`]: super::CfgSimplify
5968
@@ -105,16 +114,17 @@ impl<'heap, A: Allocator + Clone> VisitorMut<'heap> for PlaceVisitor<'_, 'heap,
105114
}
106115
}
107116

108-
/// Scalar Replacement of Aggregates transformation pass.
117+
/// Forward substitution transformation pass.
109118
///
110-
/// Resolves place operands to their ultimate sources by tracing data dependencies. This enables
111-
/// downstream passes to work with simplified operands and can expose constant propagation
112-
/// opportunities.
113-
pub struct Sroa<A: BumpAllocator = Scratch> {
119+
/// Resolves place operands to their ultimate sources by tracing data dependencies through
120+
/// projections, assignments, and block parameters. This enables downstream passes to work with
121+
/// simplified operands and, when combined with dead store elimination, achieves SROA-like
122+
/// decomposition of aggregates.
123+
pub struct ForwardSubstitution<A: BumpAllocator = Scratch> {
114124
alloc: A,
115125
}
116126

117-
impl Sroa {
127+
impl ForwardSubstitution {
118128
#[must_use]
119129
pub fn new() -> Self {
120130
Self {
@@ -123,14 +133,14 @@ impl Sroa {
123133
}
124134
}
125135

126-
impl<A: BumpAllocator> Sroa<A> {
136+
impl<A: BumpAllocator> ForwardSubstitution<A> {
127137
#[must_use]
128138
pub const fn new_in(alloc: A) -> Self {
129139
Self { alloc }
130140
}
131141
}
132142

133-
impl<'env, 'heap, A: ResetAllocator> TransformPass<'env, 'heap> for Sroa<A> {
143+
impl<'env, 'heap, A: ResetAllocator> TransformPass<'env, 'heap> for ForwardSubstitution<A> {
134144
fn run(&mut self, context: &mut MirContext<'env, 'heap>, body: &mut Body<'heap>) -> Changed {
135145
self.alloc.reset();
136146

@@ -151,7 +161,7 @@ impl<'env, 'heap, A: ResetAllocator> TransformPass<'env, 'heap> for Sroa<A> {
151161
}
152162
}
153163

154-
impl Default for Sroa {
164+
impl Default for ForwardSubstitution {
155165
fn default() -> Self {
156166
Self::new()
157167
}

0 commit comments

Comments
 (0)