Skip to content

gvn: Promote/propagate const local array #126444

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

gvn: Promote/propagate const local array #126444

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
023838f
add regression tests for gvn of local arrays
tesuji Jun 14, 2024
5203642
add codegen test for issue 73825
tesuji Jun 14, 2024
7b96b9c
gvn: promote propagatable const local arrays
tesuji Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions compiler/rustc_mir_transform/src/gvn.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let value = state.simplify_rvalue(rvalue, location);
// FIXME(#112651) `rvalue` may have a subtype to `local`. We can only mark `local` as
// reusable if we have an exact type match.
if state.local_decls[local].ty != rvalue.ty(state.local_decls, tcx) {
if state.local_decls[local].ty != rvalue.ty(state.local_decls, state.tcx) {
return;
}
value
Expand Down Expand Up @@ -382,7 +382,8 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
let ty = match kind {
AggregateTy::Array => {
assert!(fields.len() > 0);
Ty::new_array(self.tcx, fields[0].layout.ty, fields.len() as u64)
let field_ty = fields[0].layout.ty;
Ty::new_array(self.tcx, field_ty, fields.len() as u64)
}
AggregateTy::Tuple => {
Ty::new_tup_from_iter(self.tcx, fields.iter().map(|f| f.layout.ty))
Expand All @@ -406,7 +407,9 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
};
let ptr_imm = Immediate::new_pointer_with_meta(data, meta, &self.ecx);
ImmTy::from_immediate(ptr_imm, ty).into()
} else if matches!(ty.abi, Abi::Scalar(..) | Abi::ScalarPair(..)) {
} else if matches!(kind, AggregateTy::Array)
|| matches!(ty.abi, Abi::Scalar(..) | Abi::ScalarPair(..))
{
let dest = self.ecx.allocate(ty, MemoryKind::Stack).ok()?;
let variant_dest = if let Some(variant) = variant {
self.ecx.project_downcast(&dest, variant).ok()?
Expand All @@ -418,9 +421,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
self.ecx.copy_op(op, &field_dest).ok()?;
}
self.ecx.write_discriminant(variant.unwrap_or(FIRST_VARIANT), &dest).ok()?;
self.ecx
.alloc_mark_immutable(dest.ptr().provenance.unwrap().alloc_id())
.ok()?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we stop marking as immutable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I'm wrong but I think let dest = dest.map_provenance(|prov| prov.as_immutable()); in the line below could serve the same purpose.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for future record, no, that has an entirely different purpose. That marks the pointer as immutable, which is very different from marking the allocation as immutable.

let dest = dest.map_provenance(|prov| prov.as_immutable());
dest.into()
} else {
return None;
Expand Down Expand Up @@ -704,7 +705,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
place.projection = self.tcx.mk_place_elems(&projection);
}

trace!(?place);
trace!(after_place = ?place);
}

/// Represent the *value* which would be read from `place`, and point `place` to a preexisting
Expand Down Expand Up @@ -884,7 +885,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
}

let (mut ty, variant_index) = match *kind {
AggregateKind::Array(..) => {
AggregateKind::Array(_) => {
assert!(!field_ops.is_empty());
(AggregateTy::Array, FIRST_VARIANT)
}
Expand Down Expand Up @@ -1347,6 +1348,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
}
}

#[instrument(level = "trace", skip(ecx), ret)]
fn op_to_prop_const<'tcx>(
ecx: &mut InterpCx<'tcx, DummyMachine>,
op: &OpTy<'tcx>,
Expand All @@ -1361,8 +1363,11 @@ fn op_to_prop_const<'tcx>(
return Some(ConstValue::ZeroSized);
}

// Do not synthetize too large constants. Codegen will just memcpy them, which we'd like to avoid.
if !matches!(op.layout.abi, Abi::Scalar(..) | Abi::ScalarPair(..)) {
// Do not synthesize too large constants, except constant arrays.
// For arrays, codegen will just memcpy them, but LLVM will optimize out those unneeded memcpy.
// For others, we'd prefer in-place initialization over memcpy them.
if !(op.layout.ty.is_array() || matches!(op.layout.abi, Abi::Scalar(..) | Abi::ScalarPair(..)))
{
return None;
}

Expand Down Expand Up @@ -1433,6 +1438,7 @@ impl<'tcx> VnState<'_, 'tcx> {
}

/// If `index` is a `Value::Constant`, return the `Constant` to be put in the MIR.
#[instrument(level = "trace", skip(self, index), ret)]
fn try_as_constant(&mut self, index: VnIndex) -> Option<ConstOperand<'tcx>> {
// This was already constant in MIR, do not change it.
if let Value::Constant { value, disambiguator: _ } = *self.get(index)
Expand All @@ -1444,10 +1450,6 @@ impl<'tcx> VnState<'_, 'tcx> {
}

let op = self.evaluated[index].as_ref()?;
if op.layout.is_unsized() {
// Do not attempt to propagate unsized locals.
return None;
}

let value = op_to_prop_const(&mut self.ecx, op)?;

Expand Down Expand Up @@ -1484,6 +1486,7 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
self.simplify_operand(operand, location);
}

#[instrument(level = "trace", skip(self, stmt))]
fn visit_statement(&mut self, stmt: &mut Statement<'tcx>, location: Location) {
if let StatementKind::Assign(box (ref mut lhs, ref mut rvalue)) = stmt.kind {
self.simplify_place_projection(lhs, location);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_transform/src/ssa.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ impl SsaLocals {
})
}

#[instrument(level = "trace", skip_all)]
pub fn for_each_assignment_mut<'tcx>(
&self,
basic_blocks: &mut IndexSlice<BasicBlock, BasicBlockData<'tcx>>,
Expand Down
68 changes: 50 additions & 18 deletions tests/mir-opt/const_array_locals.main.GVN.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,49 +37,60 @@
}

bb0: {
- StorageLive(_1);
+ nop;
_1 = [const 255_i32, const 105_i32, const 15_i32, const 39_i32, const 62_i32];
StorageLive(_1);
- _1 = [const 255_i32, const 105_i32, const 15_i32, const 39_i32, const 62_i32];
+ _1 = const [255_i32, 105_i32, 15_i32, 39_i32, 62_i32];
StorageLive(_2);
- _2 = [const 255_i32, const 105_i32, const 15_i32, const 39_i32, const 62_i32];
+ _2 = _1;
+ _2 = const [255_i32, 105_i32, 15_i32, 39_i32, 62_i32];
StorageLive(_3);
StorageLive(_4);
_4 = [const 178_i32, const 9_i32, const 4_i32, const 56_i32, const 221_i32];
- _4 = [const 178_i32, const 9_i32, const 4_i32, const 56_i32, const 221_i32];
+ _4 = const [178_i32, 9_i32, 4_i32, 56_i32, 221_i32];
StorageLive(_5);
_5 = [const 193_i32, const 164_i32, const 194_i32, const 197_i32, const 6_i32];
_3 = [move _4, move _5];
- _5 = [const 193_i32, const 164_i32, const 194_i32, const 197_i32, const 6_i32];
- _3 = [move _4, move _5];
+ _5 = const [193_i32, 164_i32, 194_i32, 197_i32, 6_i32];
+ _3 = const [[178_i32, 9_i32, 4_i32, 56_i32, 221_i32], [193_i32, 164_i32, 194_i32, 197_i32, 6_i32]];
StorageDead(_5);
StorageDead(_4);
StorageLive(_6);
StorageLive(_7);
_17 = const main::promoted[0];
_7 = &(*_17);
- _6 = (*_7);
+ _6 = (*_17);
+ _6 = const [254_i32, 42_i32, 15_i32, 39_i32, 62_i32];
StorageDead(_7);
StorageLive(_9);
StorageLive(_10);
_10 = [const 31_u32, const 96_u32, const 173_u32, const 50_u32, const 1_u32];
_9 = consume(move _10) -> [return: bb1, unwind continue];
- _10 = [const 31_u32, const 96_u32, const 173_u32, const 50_u32, const 1_u32];
- _9 = consume(move _10) -> [return: bb1, unwind continue];
+ _10 = const [31_u32, 96_u32, 173_u32, 50_u32, 1_u32];
+ _9 = consume(const [31_u32, 96_u32, 173_u32, 50_u32, 1_u32]) -> [return: bb1, unwind continue];
}

bb1: {
StorageDead(_10);
StorageDead(_9);
StorageLive(_11);
StorageLive(_12);
_12 = [const 1f32, const 2f32, const 3f32, const 1f32, const 1f32, const 1f32, const 1f32, const 42f32];
_11 = F32x8(move _12);
- _12 = [const 1f32, const 2f32, const 3f32, const 1f32, const 1f32, const 1f32, const 1f32, const 42f32];
- _11 = F32x8(move _12);
+ _12 = const [1f32, 2f32, 3f32, 1f32, 1f32, 1f32, 1f32, 42f32];
+ _11 = F32x8(const [1f32, 2f32, 3f32, 1f32, 1f32, 1f32, 1f32, 42f32]);
StorageDead(_12);
StorageLive(_13);
StorageLive(_14);
_14 = [const 1_i32, const 0_i32, const 0_i32];
- _14 = [const 1_i32, const 0_i32, const 0_i32];
+ _14 = const [1_i32, 0_i32, 0_i32];
StorageLive(_15);
_15 = [const 0_i32, const 1_i32, const 0_i32];
- _15 = [const 0_i32, const 1_i32, const 0_i32];
+ _15 = const [0_i32, 1_i32, 0_i32];
StorageLive(_16);
_16 = [const 0_i32, const 0_i32, const 1_i32];
_13 = [move _14, move _15, move _16];
- _16 = [const 0_i32, const 0_i32, const 1_i32];
- _13 = [move _14, move _15, move _16];
+ _16 = const [0_i32, 0_i32, 1_i32];
+ _13 = const [[1_i32, 0_i32, 0_i32], [0_i32, 1_i32, 0_i32], [0_i32, 0_i32, 1_i32]];
StorageDead(_16);
StorageDead(_15);
StorageDead(_14);
Expand All @@ -89,9 +100,30 @@
StorageDead(_6);
StorageDead(_3);
StorageDead(_2);
- StorageDead(_1);
+ nop;
StorageDead(_1);
return;
}
}
+
+ ALLOC0 (size: 36, align: 4) { .. }
+
+ ALLOC1 (size: 12, align: 4) { .. }
+
+ ALLOC2 (size: 12, align: 4) { .. }
+
+ ALLOC3 (size: 12, align: 4) { .. }
+
+ ALLOC4 (size: 32, align: 4) { .. }
+
+ ALLOC5 (size: 20, align: 4) { .. }
+
+ ALLOC6 (size: 40, align: 4) { .. }
+
+ ALLOC7 (size: 20, align: 4) { .. }
+
+ ALLOC8 (size: 20, align: 4) { .. }
+
+ ALLOC9 (size: 20, align: 4) { .. }
+
+ ALLOC10 (size: 20, align: 4) { .. }

5 changes: 4 additions & 1 deletion tests/mir-opt/const_prop/array_index.main.GVN.32bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
bb0: {
StorageLive(_1);
StorageLive(_2);
_2 = [const 0_u32, const 1_u32, const 2_u32, const 3_u32];
- _2 = [const 0_u32, const 1_u32, const 2_u32, const 3_u32];
+ _2 = const [0_u32, 1_u32, 2_u32, 3_u32];
StorageLive(_3);
_3 = const 2_usize;
- _4 = Len(_2);
Expand All @@ -36,4 +37,6 @@
return;
}
}
+
+ ALLOC0 (size: 16, align: 4) { .. }

5 changes: 4 additions & 1 deletion tests/mir-opt/const_prop/array_index.main.GVN.64bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
bb0: {
StorageLive(_1);
StorageLive(_2);
_2 = [const 0_u32, const 1_u32, const 2_u32, const 3_u32];
- _2 = [const 0_u32, const 1_u32, const 2_u32, const 3_u32];
+ _2 = const [0_u32, 1_u32, 2_u32, 3_u32];
StorageLive(_3);
_3 = const 2_usize;
- _4 = Len(_2);
Expand All @@ -36,4 +37,6 @@
return;
}
}
+
+ ALLOC0 (size: 16, align: 4) { .. }

1 change: 1 addition & 0 deletions tests/mir-opt/const_prop/array_index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//@ test-mir-pass: GVN
//@ compile-flags: -Zdump-mir-exclude-alloc-bytes
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
// EMIT_MIR_FOR_EACH_BIT_WIDTH

Expand Down
5 changes: 4 additions & 1 deletion tests/mir-opt/pre-codegen/optimizes_into_variable.main.GVN.32bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
+ _1 = const 4_i32;
StorageLive(_3);
StorageLive(_4);
_4 = [const 0_i32, const 1_i32, const 2_i32, const 3_i32, const 4_i32, const 5_i32];
- _4 = [const 0_i32, const 1_i32, const 2_i32, const 3_i32, const 4_i32, const 5_i32];
+ _4 = const [0_i32, 1_i32, 2_i32, 3_i32, 4_i32, 5_i32];
StorageLive(_5);
_5 = const 3_usize;
_6 = const 6_usize;
Expand Down Expand Up @@ -64,4 +65,6 @@
}
+
+ ALLOC0 (size: 8, align: 4) { .. }
+
+ ALLOC1 (size: 24, align: 4) { .. }

5 changes: 4 additions & 1 deletion tests/mir-opt/pre-codegen/optimizes_into_variable.main.GVN.64bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
+ _1 = const 4_i32;
StorageLive(_3);
StorageLive(_4);
_4 = [const 0_i32, const 1_i32, const 2_i32, const 3_i32, const 4_i32, const 5_i32];
- _4 = [const 0_i32, const 1_i32, const 2_i32, const 3_i32, const 4_i32, const 5_i32];
+ _4 = const [0_i32, 1_i32, 2_i32, 3_i32, 4_i32, 5_i32];
StorageLive(_5);
_5 = const 3_usize;
_6 = const 6_usize;
Expand Down Expand Up @@ -64,4 +65,6 @@
}
+
+ ALLOC0 (size: 8, align: 4) { .. }
+
+ ALLOC1 (size: 24, align: 4) { .. }

Loading