Skip to content

Commit 4a6d675

Browse files
indietypindietyp
authored
BE-265: HashQL: Make CopyPropagation aware of local re-assignments (#8231)
1 parent ef9788b commit 4a6d675

File tree

20 files changed

+468
-139
lines changed

20 files changed

+468
-139
lines changed

‎libs/@local/hashql/mir/src/body/operand.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//! Operands represent the inputs to MIR operations. They can either reference
44
//! a storage location (place) or contain an immediate constant value.
55
6-
use super::{constant::Constant, place::Place};
6+
use super::{constant::Constant, local::Local, place::Place};
77

88
/// An operand in a HashQL MIR operation.
99
///
@@ -59,6 +59,12 @@ impl<'heap> Operand<'heap> {
5959
}
6060
}
6161

62+
impl From<Local> for Operand<'_> {
63+
fn from(local: Local) -> Self {
64+
Operand::Place(Place::local(local))
65+
}
66+
}
67+
6268
impl<'heap> From<Place<'heap>> for Operand<'heap> {
6369
fn from(place: Place<'heap>) -> Self {
6470
Operand::Place(place)

‎libs/@local/hashql/mir/src/body/place.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,10 +315,11 @@ impl<'heap> Place<'heap> {
315315
/// This is the simplest form of a place, representing direct access to a local variable
316316
/// without navigating through any structured data. The resulting place has an empty
317317
/// projection sequence.
318-
pub fn local(local: Local, interner: &Interner<'heap>) -> Self {
318+
#[must_use]
319+
pub const fn local(local: Local) -> Self {
319320
Self {
320321
local,
321-
projections: interner.projections.intern_slice(&[]),
322+
projections: Interned::empty(),
322323
}
323324
}
324325

‎libs/@local/hashql/mir/src/body/terminator/switch_int.rs‎

Lines changed: 39 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -74,20 +74,17 @@ pub enum SwitchIntValue {
7474
/// let targets = SwitchTargets::new(
7575
/// &heap,
7676
/// [
77-
/// (0, Target::block(bb0, &interner)),
78-
/// (1, Target::block(bb1, &interner)),
79-
/// (2, Target::block(bb2, &interner)),
77+
/// (0, Target::block(bb0)),
78+
/// (1, Target::block(bb1)),
79+
/// (2, Target::block(bb2)),
8080
/// ],
81-
/// Some(Target::block(otherwise, &interner)),
81+
/// Some(Target::block(otherwise)),
8282
/// );
8383
///
8484
/// // Values are automatically sorted
8585
/// assert_eq!(targets.values(), &[0, 1, 2]);
86-
/// assert_eq!(targets.target(1), Some(Target::block(bb1, &interner)));
87-
/// assert_eq!(
88-
/// targets.target(99),
89-
/// Some(Target::block(otherwise, &interner))
90-
/// );
86+
/// assert_eq!(targets.target(1), Some(Target::block(bb1)));
87+
/// assert_eq!(targets.target(99), Some(Target::block(otherwise)));
9188
/// ```
9289
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
9390
pub struct SwitchTargets<'heap> {
@@ -131,11 +128,11 @@ impl<'heap> SwitchTargets<'heap> {
131128
/// let targets = SwitchTargets::new(
132129
/// &heap,
133130
/// [
134-
/// (10, Target::block(BasicBlockId::new(0), &interner)),
135-
/// (20, Target::block(BasicBlockId::new(1), &interner)),
136-
/// (30, Target::block(BasicBlockId::new(2), &interner)),
131+
/// (10, Target::block(BasicBlockId::new(0))),
132+
/// (20, Target::block(BasicBlockId::new(1))),
133+
/// (30, Target::block(BasicBlockId::new(2))),
137134
/// ],
138-
/// Some(Target::block(BasicBlockId::new(3), &interner)),
135+
/// Some(Target::block(BasicBlockId::new(3))),
139136
/// );
140137
///
141138
/// assert_eq!(targets.values(), &[10, 20, 30]);
@@ -158,11 +155,7 @@ impl<'heap> SwitchTargets<'heap> {
158155
/// let heap = Heap::new();
159156
/// let interner = Interner::new(&heap);
160157
///
161-
/// let targets = SwitchTargets::new(
162-
/// &heap,
163-
/// [(0, Target::block(BasicBlockId::new(0), &interner))],
164-
/// None,
165-
/// );
158+
/// let targets = SwitchTargets::new(&heap, [(0, Target::block(BasicBlockId::new(0)))], None);
166159
///
167160
/// assert_eq!(targets.otherwise(), None);
168161
/// assert_eq!(targets.target(99), None); // No otherwise, so None for unmatched values
@@ -214,8 +207,8 @@ impl<'heap> SwitchTargets<'heap> {
214207
/// let heap = Heap::new();
215208
/// let interner = Interner::new(&heap);
216209
///
217-
/// let then_block = Target::block(BasicBlockId::new(1), &interner);
218-
/// let else_block = Target::block(BasicBlockId::new(2), &interner);
210+
/// let then_block = Target::block(BasicBlockId::new(1));
211+
/// let else_block = Target::block(BasicBlockId::new(2));
219212
///
220213
/// let targets = SwitchTargets::new_if(&heap, then_block, else_block);
221214
///
@@ -261,8 +254,8 @@ impl<'heap> SwitchTargets<'heap> {
261254
/// let heap = Heap::new();
262255
/// let interner = Interner::new(&heap);
263256
///
264-
/// let then_block = Target::block(BasicBlockId::new(1), &interner);
265-
/// let else_block = Target::block(BasicBlockId::new(2), &interner);
257+
/// let then_block = Target::block(BasicBlockId::new(1));
258+
/// let else_block = Target::block(BasicBlockId::new(2));
266259
///
267260
/// // Binary switch can be converted
268261
/// let binary = SwitchTargets::new_if(&heap, then_block, else_block);
@@ -316,11 +309,11 @@ impl<'heap> SwitchTargets<'heap> {
316309
/// let heap = Heap::new();
317310
/// let interner = Interner::new(&heap);
318311
///
319-
/// let default = Target::block(BasicBlockId::new(99), &interner);
312+
/// let default = Target::block(BasicBlockId::new(99));
320313
///
321314
/// let with_otherwise = SwitchTargets::new(
322315
/// &heap,
323-
/// [(1, Target::block(BasicBlockId::new(0), &interner))],
316+
/// [(1, Target::block(BasicBlockId::new(0)))],
324317
/// Some(default),
325318
/// );
326319
/// assert_eq!(with_otherwise.otherwise(), Some(default));
@@ -353,9 +346,9 @@ impl<'heap> SwitchTargets<'heap> {
353346
/// let heap = Heap::new();
354347
/// let interner = Interner::new(&heap);
355348
///
356-
/// let bb0 = Target::block(BasicBlockId::new(0), &interner);
357-
/// let bb1 = Target::block(BasicBlockId::new(1), &interner);
358-
/// let otherwise = Target::block(BasicBlockId::new(99), &interner);
349+
/// let bb0 = Target::block(BasicBlockId::new(0));
350+
/// let bb1 = Target::block(BasicBlockId::new(1));
351+
/// let otherwise = Target::block(BasicBlockId::new(99));
359352
///
360353
/// let targets = SwitchTargets::new(&heap, [(10, bb0), (20, bb1)], Some(otherwise));
361354
///
@@ -417,8 +410,8 @@ impl<'heap> SwitchTargets<'heap> {
417410
/// let mut targets = SwitchTargets::new(
418411
/// &heap,
419412
/// [
420-
/// (1, Target::block(BasicBlockId::new(0), &interner)),
421-
/// (2, Target::block(BasicBlockId::new(1), &interner)),
413+
/// (1, Target::block(BasicBlockId::new(0))),
414+
/// (2, Target::block(BasicBlockId::new(1))),
422415
/// ],
423416
/// None,
424417
/// );
@@ -459,10 +452,10 @@ impl<'heap> SwitchTargets<'heap> {
459452
/// let targets = SwitchTargets::new(
460453
/// &heap,
461454
/// [
462-
/// (10, Target::block(BasicBlockId::new(0), &interner)),
463-
/// (20, Target::block(BasicBlockId::new(1), &interner)),
455+
/// (10, Target::block(BasicBlockId::new(0))),
456+
/// (20, Target::block(BasicBlockId::new(1))),
464457
/// ],
465-
/// Some(Target::block(BasicBlockId::new(99), &interner)),
458+
/// Some(Target::block(BasicBlockId::new(99))),
466459
/// );
467460
///
468461
/// let pairs: Vec<_> = targets.iter().collect();
@@ -501,8 +494,8 @@ impl<'heap> SwitchTargets<'heap> {
501494
/// let mut targets = SwitchTargets::new(
502495
/// &heap,
503496
/// [
504-
/// (10, Target::block(BasicBlockId::new(0), &interner)),
505-
/// (20, Target::block(BasicBlockId::new(1), &interner)),
497+
/// (10, Target::block(BasicBlockId::new(0))),
498+
/// (20, Target::block(BasicBlockId::new(1))),
506499
/// ],
507500
/// None,
508501
/// );
@@ -545,8 +538,8 @@ impl<'heap> SwitchTargets<'heap> {
545538
///
546539
/// let mut targets = SwitchTargets::new(&heap, [], None);
547540
///
548-
/// targets.add_target(10, Target::block(BasicBlockId::new(0), &interner));
549-
/// targets.add_target(5, Target::block(BasicBlockId::new(1), &interner));
541+
/// targets.add_target(10, Target::block(BasicBlockId::new(0)));
542+
/// targets.add_target(5, Target::block(BasicBlockId::new(1)));
550543
///
551544
/// // Values are kept sorted
552545
/// assert_eq!(targets.values(), &[5, 10]);
@@ -587,16 +580,12 @@ impl<'heap> SwitchTargets<'heap> {
587580
/// let heap = Heap::new();
588581
/// let interner = Interner::new(&heap);
589582
///
590-
/// let mut first = SwitchTargets::new(
591-
/// &heap,
592-
/// [(10, Target::block(BasicBlockId::new(0), &interner))],
593-
/// None,
594-
/// );
583+
/// let mut first = SwitchTargets::new(&heap, [(10, Target::block(BasicBlockId::new(0)))], None);
595584
///
596585
/// let mut second = SwitchTargets::new(
597586
/// &heap,
598-
/// [(20, Target::block(BasicBlockId::new(1), &interner))],
599-
/// Some(Target::block(BasicBlockId::new(99), &interner)),
587+
/// [(20, Target::block(BasicBlockId::new(1)))],
588+
/// Some(Target::block(BasicBlockId::new(99))),
600589
/// );
601590
///
602591
/// first.append(&mut second);
@@ -702,11 +691,11 @@ impl<'heap> SwitchTargets<'heap> {
702691
/// let targets = SwitchTargets::new(
703692
/// &heap,
704693
/// [
705-
/// (0, Target::block(BasicBlockId::new(0), &interner)),
706-
/// (1, Target::block(BasicBlockId::new(1), &interner)),
707-
/// (2, Target::block(BasicBlockId::new(2), &interner)),
694+
/// (0, Target::block(BasicBlockId::new(0))),
695+
/// (1, Target::block(BasicBlockId::new(1))),
696+
/// (2, Target::block(BasicBlockId::new(2))),
708697
/// ],
709-
/// Some(Target::block(BasicBlockId::new(3), &interner)), // otherwise
698+
/// Some(Target::block(BasicBlockId::new(3))), // otherwise
710699
/// );
711700
///
712701
/// // Create the switch with an integer discriminant
@@ -734,12 +723,12 @@ impl<'heap> SwitchTargets<'heap> {
734723
/// let heap = Heap::new();
735724
/// let interner = Interner::new(&heap);
736725
///
737-
/// let then_target = Target::block(BasicBlockId::new(1), &interner);
738-
/// let else_target = Target::block(BasicBlockId::new(2), &interner);
726+
/// let then_target = Target::block(BasicBlockId::new(1));
727+
/// let else_target = Target::block(BasicBlockId::new(2));
739728
///
740729
/// // Create a binary switch for if-else
741730
/// let switch = SwitchInt {
742-
/// discriminant: Operand::Place(Place::local(Local::new(0), &interner)),
731+
/// discriminant: Operand::Place(Place::local(Local::new(0))),
743732
/// targets: SwitchTargets::new_if(&heap, then_target, else_target),
744733
/// };
745734
///

‎libs/@local/hashql/mir/src/body/terminator/target.rs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,7 @@
55
66
use hashql_core::intern::Interned;
77

8-
use crate::{
9-
body::{basic_block::BasicBlockId, operand::Operand},
10-
intern::Interner,
11-
};
8+
use crate::body::{basic_block::BasicBlockId, operand::Operand};
129

1310
/// A control flow target in the HashQL MIR.
1411
///
@@ -39,11 +36,12 @@ pub struct Target<'heap> {
3936
pub args: Interned<'heap, [Operand<'heap>]>,
4037
}
4138

42-
impl<'heap> Target<'heap> {
43-
pub fn block(block: impl Into<BasicBlockId>, interner: &Interner<'heap>) -> Self {
39+
impl Target<'_> {
40+
#[must_use]
41+
pub const fn block(block: BasicBlockId) -> Self {
4442
Self {
45-
block: block.into(),
46-
args: interner.operands.intern_slice(&[]),
43+
block,
44+
args: Interned::empty(),
4745
}
4846
}
4947
}

‎libs/@local/hashql/mir/src/builder/body.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ impl<'env, 'heap> BodyBuilder<'env, 'heap> {
7070
};
7171
let local = self.local_decls.push(decl);
7272

73-
Place::local(local, self.interner)
73+
Place::local(local)
7474
}
7575

7676
/// Reserves a new basic block and returns its ID.

‎libs/@local/hashql/mir/src/pass/analysis/data_dependency/resolve.rs‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ fn resolve_params_const<'heap, A: Allocator + Clone>(
299299
} else {
300300
// We have finished (we have terminated on a param, which is divergent, therefore the place
301301
// is still valid, just doesn't have a constant value)
302-
ResolutionResult::Resolved(Operand::Place(Place::local(place.local, state.interner)))
302+
ResolutionResult::Resolved(Operand::Place(Place::local(place.local)))
303303
}
304304
}
305305

@@ -382,15 +382,14 @@ pub(crate) fn resolve<'heap, A: Allocator + Clone>(
382382
[] => {
383383
// Base case: no more projections to resolve.
384384
// Check for constant propagation through Load.
385-
let operand = if let Some(constant) = state
385+
let operand = state
386386
.graph
387387
.constant_bindings
388388
.find_by_kind(place.local, EdgeKind::Load)
389-
{
390-
Operand::Constant(constant)
391-
} else {
392-
Operand::Place(Place::local(place.local, state.interner))
393-
};
389+
.map_or_else(
390+
|| Operand::Place(Place::local(place.local)),
391+
Operand::Constant,
392+
);
394393

395394
return ResolutionResult::Resolved(operand);
396395
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ impl<'heap, A: Allocator> AdministrativeReductionVisitor<'_, '_, 'heap, A> {
221221
.enumerate()
222222
.map(|(param, argument)| Statement {
223223
kind: StatementKind::Assign(Assign {
224-
lhs: Place::local(Local::new(local_offset + param), self.interner),
224+
lhs: Place::local(Local::new(local_offset + param)),
225225
rhs: RValue::Load(argument),
226226
}),
227227
span,

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,7 @@ impl<A: BumpAllocator> CfgSimplify<A> {
122122
/// 3. Replace `A`'s terminator with `B`'s terminator
123123
///
124124
/// SSA invariants may be temporarily broken; the [`SsaRepair`] runs afterward to fix them.
125-
fn simplify_goto<'heap>(
126-
context: &MirContext<'_, 'heap>,
127-
body: &mut Body<'heap>,
128-
id: BasicBlockId,
129-
goto: Goto<'heap>,
130-
) -> bool {
125+
fn simplify_goto<'heap>(body: &mut Body<'heap>, id: BasicBlockId, goto: Goto<'heap>) -> bool {
131126
// Self-loops cannot be optimized as there's no simplification possible.
132127
if goto.target.block == id {
133128
return false;
@@ -170,7 +165,7 @@ impl<A: BumpAllocator> CfgSimplify<A> {
170165
block.statements.push(Statement {
171166
span: block.terminator.span,
172167
kind: StatementKind::Assign(Assign {
173-
lhs: Place::local(param, context.interner),
168+
lhs: Place::local(param),
174169
rhs: RValue::Load(arg),
175170
}),
176171
});
@@ -424,7 +419,7 @@ impl<A: BumpAllocator> CfgSimplify<A> {
424419
.transfer_into(&self.alloc);
425420

426421
let changed = match &body.basic_blocks[id].terminator.kind {
427-
&TerminatorKind::Goto(goto) => Self::simplify_goto(context, body, id, goto),
422+
&TerminatorKind::Goto(goto) => Self::simplify_goto(body, id, goto),
428423
TerminatorKind::SwitchInt(_) => Self::simplify_switch_int(context, body, id),
429424
TerminatorKind::Return(_)
430425
| TerminatorKind::GraphRead(_)

0 commit comments

Comments
 (0)