Skip to content

Commit

Permalink
Auto merge of rust-lang#126444 - tesuji:gvn-const-arrays, r=<try>
Browse files Browse the repository at this point in the history 
[WIP] gvn: Promote/propagate const local array

Rewriting of rust-lang#125916 which used PromoteTemps pass.

Fix rust-lang#73825

### Current status

- [ ] Waiting for [consensus](https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/Could.20const.20read-only.20arrays.20be.20const.20promoted.3F).

r? ghost
  • Loading branch information
@bors
bors committed Jun 13, 2024
2 parents f158600 + 550fb81 commit e26c0b3
Show file tree
Hide file tree
Showing 6 changed files with 298 additions and 17 deletions.
94 changes: 82 additions & 12 deletions compiler/rustc_mir_transform/src/gvn.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,15 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {

#[instrument(level = "trace", skip(self), ret)]
fn eval_to_const(&mut self, value: VnIndex) -> Option<OpTy<'tcx>> {
use abi::HasDataLayout;
use Value::*;
let op = match *self.get(value) {
// LLVM optimizes the load of `sizeof(size_t) * 2` as a single `mov`,
// which is cheap. Bigger values make more `mov` instructions generated.
// After GVN, it becomes a single load (`lea`) of an address in `.rodata`.
let stack_threshold = self.tcx.data_layout().pointer_size * 2;
let vvalue = self.get(value);
debug!(?vvalue);
let op = match *vvalue {
Opaque(_) => return None,
// Do not bother evaluating repeat expressions. This would uselessly consume memory.
Repeat(..) => return None,
Expand All @@ -376,10 +383,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
self.ecx.eval_mir_constant(value, DUMMY_SP, None).ok()?
}
Aggregate(kind, variant, ref fields) => {
debug!(?kind, ?variant, ?fields);
let fields = fields
.iter()
.map(|&f| self.evaluated[f].as_ref())
.collect::<Option<Vec<_>>>()?;
.collect::<Option<Vec<&OpTy<'_>>>>()?;
let ty = match kind {
AggregateTy::Array => {
assert!(fields.len() > 0);
Expand Down Expand Up @@ -407,6 +415,35 @@ 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!(kind, AggregateTy::Array) {
if ty.layout.size() <= stack_threshold {
return None;
}
let mut mplace = None;
let alloc_id = self
.ecx
.intern_with_temp_alloc(ty, |ecx, dest| {
for (field_index, op) in fields.iter().copied().enumerate() {
// ignore nested arrays
if let Either::Left(_) = op.as_mplace_or_imm() {
interpret::throw_inval!(TooGeneric);
}
let field_dest = ecx.project_field(dest, field_index)?;
ecx.copy_op(op, &field_dest)?;
}

let dest =
dest.assert_mem_place().map_provenance(|prov| prov.as_immutable());
mplace.replace(dest);
Ok(())
})
.ok()?;
let GlobalAlloc::Memory(_alloc) = self.tcx.global_alloc(alloc_id) else {
bug!()
};
let mplace = mplace.unwrap();
debug!(?mplace);
return Some(mplace.into());
} else if matches!(ty.abi, Abi::Scalar(..) | Abi::ScalarPair(..)) {
let dest = self.ecx.allocate(ty, MemoryKind::Stack).ok()?;
let variant_dest = if let Some(variant) = variant {
Expand All @@ -429,6 +466,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
}

Projection(base, elem) => {
debug!(?base, ?elem);
let value = self.evaluated[base].as_ref()?;
let elem = match elem {
ProjectionElem::Deref => ProjectionElem::Deref,
Expand All @@ -450,6 +488,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
self.ecx.project(value, elem).ok()?
}
Address { place, kind, provenance: _ } => {
debug!(?place, ?kind);
if !place.is_indirect_first_projection() {
return None;
}
Expand Down Expand Up @@ -709,7 +748,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 @@ -1233,6 +1272,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 @@ -1248,7 +1288,8 @@ fn op_to_prop_const<'tcx>(
}

// 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(..)) {
if !(op.layout.ty.is_array() || matches!(op.layout.abi, Abi::Scalar(..) | Abi::ScalarPair(..)))
{
return None;
}

Expand Down Expand Up @@ -1302,21 +1343,42 @@ fn op_to_prop_const<'tcx>(

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)
let value = self.get(index);
debug!(?index, ?value);
match value {
// If the constant is not deterministic, adding an additional mention of it in MIR will
// not give the same value as the former mention.
&& value.is_deterministic()
{
return Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_: value });
Value::Constant { value, disambiguator: _ } if value.is_deterministic() => {
return Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_: *value });
}
// ignore nested arrays
Value::Aggregate(AggregateTy::Array, _, fields) => {
for f in fields {
if let Value::Constant { value: Const::Val(const_, _), .. } = self.get(*f)
&& let ConstValue::Indirect { .. } = const_
{
return None;
}
}
}
// ignore promoted arrays
Value::Projection(index, ProjectionElem::Deref) => {
if let Value::Constant { value: Const::Val(const_, ty), .. } = self.get(*index)
&& let ConstValue::Scalar(Scalar::Ptr(..)) = const_
&& let ty::Ref(region, ty, _mutability) = ty.kind()
&& region.is_erased()
&& ty.is_array()
{
return None;
}
}
_ => {}
}

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 All @@ -1326,6 +1388,10 @@ impl<'tcx> VnState<'_, 'tcx> {
assert!(!value.may_have_provenance(self.tcx, op.layout.size));

let const_ = Const::Val(value, op.layout.ty);
// cache the propagated const
if let Some(new_index) = self.insert_constant(const_) {
self.values.swap_indices(index.as_usize(), new_index.as_usize());
}
Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_ })
}

Expand Down Expand Up @@ -1353,6 +1419,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 All @@ -1366,8 +1433,10 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
.as_local()
.and_then(|local| self.locals[local])
.or_else(|| self.simplify_rvalue(rvalue, location));
debug!(?value);
let Some(value) = value else { return };

debug!(before_rvalue = ?rvalue);
if let Some(const_) = self.try_as_constant(value) {
*rvalue = Rvalue::Use(Operand::Constant(Box::new(const_)));
} else if let Some(local) = self.try_as_local(value, location)
Expand All @@ -1376,6 +1445,7 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
*rvalue = Rvalue::Use(Operand::Copy(local.into()));
self.reused_locals.insert(local);
}
debug!(after_rvalue = ?rvalue);

return;
}
Expand Down
25 changes: 25 additions & 0 deletions tests/codegen/issue-73825-gvn-const-local-array.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// issue: <https://github.com/rust-lang/rust/issues/73825>
//@ compile-flags: -C opt-level=1
#![crate_type = "lib"]

// CHECK-LABEL: @foo
// CHECK-NEXT: {{.*}}:
// CHECK-NEXT: and
// CHECK-NEXT: getelementptr inbounds
// CHECK-NEXT: load i32
// CHECK-NEXT: ret i32
#[no_mangle]
#[rustfmt::skip]
pub fn foo(x: usize) -> i32 {
let base: [i32; 64] = [
67, 754, 860, 559, 368, 870, 548, 972,
141, 731, 351, 664, 32, 4, 996, 741,
203, 292, 237, 480, 151, 940, 777, 540,
143, 587, 747, 65, 152, 517, 882, 880,
712, 595, 370, 901, 237, 53, 789, 785,
912, 650, 896, 367, 316, 392, 62, 473,
675, 691, 281, 192, 445, 970, 225, 425,
628, 324, 322, 206, 912, 867, 462, 92
];
base[x % 64]
}
138 changes: 138 additions & 0 deletions tests/mir-opt/const_array_locals.main.GVN.diff
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
- // MIR for `main` before GVN
+ // MIR for `main` after GVN

fn main() -> () {
let mut _0: ();
let _1: [i32; 32];
let mut _4: [i32; 12];
let mut _5: [i32; 12];
let mut _7: &[i32; 32];
let _8: [i32; 32];
let _9: ();
let mut _10: [u16; 32];
let mut _12: [f32; 8];
let _13: [[i32; 3]; 3];
let mut _14: [i32; 3];
let mut _15: [i32; 3];
let mut _16: [i32; 3];
scope 1 {
debug _arr => _1;
let _2: [i32; 32];
scope 2 {
debug _barr => _2;
let _3: [[i32; 12]; 2];
scope 3 {
debug _foo => _3;
let _6: [i32; 32];
let mut _17: &[i32; 32];
scope 4 {
debug _darr => _6;
let _11: F32x8;
scope 5 {
debug _f => _11;
}
}
}
}
}

bb0: {
StorageLive(_1);
- _1 = [const 255_i32, const 105_i32, const 15_i32, const 39_i32, const 62_i32, const 251_i32, const 191_i32, const 178_i32, const 9_i32, const 4_i32, const 56_i32, const 221_i32, const 193_i32, const 164_i32, const 194_i32, const 197_i32, const 6_i32, const 243_i32, const 218_i32, const 171_i32, const 87_i32, const 247_i32, const 104_i32, const 159_i32, const 22_i32, const 157_i32, const 105_i32, const 31_i32, const 96_i32, const 173_i32, const 50_i32, const 1_i32];
+ _1 = const [255_i32, 105_i32, 15_i32, 39_i32, 62_i32, 251_i32, 191_i32, 178_i32, 9_i32, 4_i32, 56_i32, 221_i32, 193_i32, 164_i32, 194_i32, 197_i32, 6_i32, 243_i32, 218_i32, 171_i32, 87_i32, 247_i32, 104_i32, 159_i32, 22_i32, 157_i32, 105_i32, 31_i32, 96_i32, 173_i32, 50_i32, 1_i32];
StorageLive(_2);
- _2 = [const 255_i32, const 105_i32, const 15_i32, const 39_i32, const 62_i32, const 251_i32, const 191_i32, const 178_i32, const 9_i32, const 4_i32, const 56_i32, const 221_i32, const 193_i32, const 164_i32, const 194_i32, const 197_i32, const 6_i32, const 243_i32, const 218_i32, const 171_i32, const 87_i32, const 247_i32, const 104_i32, const 159_i32, const 22_i32, const 157_i32, const 105_i32, const 31_i32, const 96_i32, const 173_i32, const 50_i32, const 1_i32];
+ _2 = const [255_i32, 105_i32, 15_i32, 39_i32, 62_i32, 251_i32, 191_i32, 178_i32, 9_i32, 4_i32, 56_i32, 221_i32, 193_i32, 164_i32, 194_i32, 197_i32, 6_i32, 243_i32, 218_i32, 171_i32, 87_i32, 247_i32, 104_i32, 159_i32, 22_i32, 157_i32, 105_i32, 31_i32, 96_i32, 173_i32, 50_i32, 1_i32];
StorageLive(_3);
StorageLive(_4);
- _4 = [const 255_i32, const 105_i32, const 15_i32, const 39_i32, const 62_i32, const 251_i32, const 191_i32, const 178_i32, const 9_i32, const 4_i32, const 56_i32, const 221_i32];
+ _4 = const [255_i32, 105_i32, 15_i32, 39_i32, 62_i32, 251_i32, 191_i32, 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, const 243_i32, const 218_i32, const 171_i32, const 87_i32, const 247_i32, const 104_i32, const 42_i32];
- _3 = [move _4, move _5];
+ _5 = const [193_i32, 164_i32, 194_i32, 197_i32, 6_i32, 243_i32, 218_i32, 171_i32, 87_i32, 247_i32, 104_i32, 42_i32];
+ _3 = [const [255_i32, 105_i32, 15_i32, 39_i32, 62_i32, 251_i32, 191_i32, 178_i32, 9_i32, 4_i32, 56_i32, 221_i32], const [193_i32, 164_i32, 194_i32, 197_i32, 6_i32, 243_i32, 218_i32, 171_i32, 87_i32, 247_i32, 104_i32, 42_i32]];
StorageDead(_5);
StorageDead(_4);
StorageLive(_6);
StorageLive(_7);
_17 = const main::promoted[0];
_7 = &(*_17);
- _6 = (*_7);
+ _6 = (*_17);
StorageDead(_7);
StorageLive(_9);
StorageLive(_10);
- _10 = [const 255_u16, const 105_u16, const 15_u16, const 39_u16, const 62_u16, const 251_u16, const 191_u16, const 178_u16, const 9_u16, const 4_u16, const 56_u16, const 221_u16, const 193_u16, const 164_u16, const 194_u16, const 197_u16, const 6_u16, const 243_u16, const 218_u16, const 171_u16, const 87_u16, const 247_u16, const 104_u16, const 159_u16, const 22_u16, const 157_u16, const 105_u16, const 31_u16, const 96_u16, const 173_u16, const 50_u16, const 1_u16];
- _9 = consume(move _10) -> [return: bb1, unwind continue];
+ _10 = const [255_u16, 105_u16, 15_u16, 39_u16, 62_u16, 251_u16, 191_u16, 178_u16, 9_u16, 4_u16, 56_u16, 221_u16, 193_u16, 164_u16, 194_u16, 197_u16, 6_u16, 243_u16, 218_u16, 171_u16, 87_u16, 247_u16, 104_u16, 159_u16, 22_u16, 157_u16, 105_u16, 31_u16, 96_u16, 173_u16, 50_u16, 1_u16];
+ _9 = consume(const [255_u16, 105_u16, 15_u16, 39_u16, 62_u16, 251_u16, 191_u16, 178_u16, 9_u16, 4_u16, 56_u16, 221_u16, 193_u16, 164_u16, 194_u16, 197_u16, 6_u16, 243_u16, 218_u16, 171_u16, 87_u16, 247_u16, 104_u16, 159_u16, 22_u16, 157_u16, 105_u16, 31_u16, 96_u16, 173_u16, 50_u16, 1_u16]) -> [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, 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];
StorageLive(_15);
_15 = [const 0_i32, const 1_i32, const 0_i32];
StorageLive(_16);
_16 = [const 0_i32, const 0_i32, const 1_i32];
_13 = [move _14, move _15, move _16];
StorageDead(_16);
StorageDead(_15);
StorageDead(_14);
StorageDead(_13);
_0 = const ();
StorageDead(_11);
StorageDead(_6);
StorageDead(_3);
StorageDead(_2);
StorageDead(_1);
return;
}
+ }
+
+ ALLOC0 (size: 32, align: 4) {
+ 0x00 │ 00 00 80 3f 00 00 00 40 00 00 40 40 00 00 80 3f │ ...?...@..@@...?
+ 0x10 │ 00 00 80 3f 00 00 80 3f 00 00 80 3f 00 00 28 42 │ ...?...?...?..(B
+ }
+
+ ALLOC1 (size: 64, align: 2) {
+ 0x00 │ ff 00 69 00 0f 00 27 00 3e 00 fb 00 bf 00 b2 00 │ ..i...'.>.......
+ 0x10 │ 09 00 04 00 38 00 dd 00 c1 00 a4 00 c2 00 c5 00 │ ....8...........
+ 0x20 │ 06 00 f3 00 da 00 ab 00 57 00 f7 00 68 00 9f 00 │ ........W...h...
+ 0x30 │ 16 00 9d 00 69 00 1f 00 60 00 ad 00 32 00 01 00 │ ....i...`...2...
+ }
+
+ ALLOC2 (size: 48, align: 4) {
+ 0x00 │ c1 00 00 00 a4 00 00 00 c2 00 00 00 c5 00 00 00 │ ................
+ 0x10 │ 06 00 00 00 f3 00 00 00 da 00 00 00 ab 00 00 00 │ ................
+ 0x20 │ 57 00 00 00 f7 00 00 00 68 00 00 00 2a 00 00 00 │ W.......h...*...
+ }
+
+ ALLOC3 (size: 48, align: 4) {
+ 0x00 │ ff 00 00 00 69 00 00 00 0f 00 00 00 27 00 00 00 │ ....i.......'...
+ 0x10 │ 3e 00 00 00 fb 00 00 00 bf 00 00 00 b2 00 00 00 │ >...............
+ 0x20 │ 09 00 00 00 04 00 00 00 38 00 00 00 dd 00 00 00 │ ........8.......
+ }
+
+ ALLOC4 (size: 128, align: 4) {
+ 0x00 │ ff 00 00 00 69 00 00 00 0f 00 00 00 27 00 00 00 │ ....i.......'...
+ 0x10 │ 3e 00 00 00 fb 00 00 00 bf 00 00 00 b2 00 00 00 │ >...............
+ 0x20 │ 09 00 00 00 04 00 00 00 38 00 00 00 dd 00 00 00 │ ........8.......
+ 0x30 │ c1 00 00 00 a4 00 00 00 c2 00 00 00 c5 00 00 00 │ ................
+ 0x40 │ 06 00 00 00 f3 00 00 00 da 00 00 00 ab 00 00 00 │ ................
+ 0x50 │ 57 00 00 00 f7 00 00 00 68 00 00 00 9f 00 00 00 │ W.......h.......
+ 0x60 │ 16 00 00 00 9d 00 00 00 69 00 00 00 1f 00 00 00 │ ........i.......
+ 0x70 │ 60 00 00 00 ad 00 00 00 32 00 00 00 01 00 00 00 │ `.......2.......
}

Loading

0 comments on commit e26c0b3

Please sign in to comment.