From a0b4d6dfb87440c1454d475941f16ff2d09c5d37 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 26 Jun 2024 09:44:55 +0000 Subject: [PATCH 1/7] Do not normalize constants eagerly. --- compiler/rustc_mir_transform/src/gvn.rs | 5 +---- tests/incremental/hashes/for_loops.rs | 4 ++-- .../slice_len.main.GVN.32bit.panic-abort.diff | 9 +++------ .../slice_len.main.GVN.32bit.panic-unwind.diff | 9 +++------ .../slice_len.main.GVN.64bit.panic-abort.diff | 9 +++------ .../slice_len.main.GVN.64bit.panic-unwind.diff | 9 +++------ tests/mir-opt/const_prop/slice_len.rs | 2 +- ...t_invalid_constant.main.GVN.32bit.panic-abort.diff | 11 ++++------- ..._invalid_constant.main.GVN.32bit.panic-unwind.diff | 11 ++++------- ...t_invalid_constant.main.GVN.64bit.panic-abort.diff | 11 ++++------- ..._invalid_constant.main.GVN.64bit.panic-unwind.diff | 11 ++++------- 11 files changed, 32 insertions(+), 59 deletions(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 336aa1fd43f06..96b2f9cbbdc94 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -773,10 +773,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { location: Location, ) -> Option { match *operand { - Operand::Constant(ref mut constant) => { - let const_ = constant.const_.normalize(self.tcx, self.param_env); - self.insert_constant(const_) - } + Operand::Constant(ref constant) => self.insert_constant(constant.const_), Operand::Copy(ref mut place) | Operand::Move(ref mut place) => { let value = self.simplify_place_value(place, location)?; if let Some(const_) = self.try_as_constant(value) { diff --git a/tests/incremental/hashes/for_loops.rs b/tests/incremental/hashes/for_loops.rs index 9cd99bf76b7ae..b827ad9cc6f89 100644 --- a/tests/incremental/hashes/for_loops.rs +++ b/tests/incremental/hashes/for_loops.rs @@ -103,9 +103,9 @@ pub fn change_iterable() { } #[cfg(not(any(cfail1,cfail4)))] -#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes, promoted_mir, optimized_mir")] +#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes, promoted_mir")] #[rustc_clean(cfg="cfail3")] -#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes, promoted_mir, optimized_mir")] +#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes, promoted_mir")] #[rustc_clean(cfg="cfail6")] pub fn change_iterable() { let mut _x = 0; diff --git a/tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-abort.diff b/tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-abort.diff index 8415789de6ecd..21d91d0320ae2 100644 --- a/tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-abort.diff +++ b/tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-abort.diff @@ -22,12 +22,11 @@ StorageLive(_3); StorageLive(_4); _9 = const main::promoted[0]; -- _4 = _9; + _4 = _9; - _3 = _4; - _2 = move _3 as &[u32] (PointerCoercion(Unsize)); -+ _4 = const {ALLOC0: &[u32; 3]}; -+ _3 = const {ALLOC0: &[u32; 3]}; -+ _2 = const {ALLOC0: &[u32; 3]} as &[u32] (PointerCoercion(Unsize)); ++ _3 = _9; ++ _2 = _9 as &[u32] (PointerCoercion(Unsize)); StorageDead(_3); StorageLive(_6); _6 = const 1_usize; @@ -50,6 +49,4 @@ return; } } -+ -+ ALLOC0 (size: 12, align: 4) { .. } diff --git a/tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-unwind.diff b/tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-unwind.diff index fea7caac3cdce..889114c986236 100644 --- a/tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-unwind.diff +++ b/tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-unwind.diff @@ -22,12 +22,11 @@ StorageLive(_3); StorageLive(_4); _9 = const main::promoted[0]; -- _4 = _9; + _4 = _9; - _3 = _4; - _2 = move _3 as &[u32] (PointerCoercion(Unsize)); -+ _4 = const {ALLOC0: &[u32; 3]}; -+ _3 = const {ALLOC0: &[u32; 3]}; -+ _2 = const {ALLOC0: &[u32; 3]} as &[u32] (PointerCoercion(Unsize)); ++ _3 = _9; ++ _2 = _9 as &[u32] (PointerCoercion(Unsize)); StorageDead(_3); StorageLive(_6); _6 = const 1_usize; @@ -50,6 +49,4 @@ return; } } -+ -+ ALLOC0 (size: 12, align: 4) { .. } diff --git a/tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-abort.diff b/tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-abort.diff index 8415789de6ecd..21d91d0320ae2 100644 --- a/tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-abort.diff +++ b/tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-abort.diff @@ -22,12 +22,11 @@ StorageLive(_3); StorageLive(_4); _9 = const main::promoted[0]; -- _4 = _9; + _4 = _9; - _3 = _4; - _2 = move _3 as &[u32] (PointerCoercion(Unsize)); -+ _4 = const {ALLOC0: &[u32; 3]}; -+ _3 = const {ALLOC0: &[u32; 3]}; -+ _2 = const {ALLOC0: &[u32; 3]} as &[u32] (PointerCoercion(Unsize)); ++ _3 = _9; ++ _2 = _9 as &[u32] (PointerCoercion(Unsize)); StorageDead(_3); StorageLive(_6); _6 = const 1_usize; @@ -50,6 +49,4 @@ return; } } -+ -+ ALLOC0 (size: 12, align: 4) { .. } diff --git a/tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-unwind.diff b/tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-unwind.diff index fea7caac3cdce..889114c986236 100644 --- a/tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-unwind.diff +++ b/tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-unwind.diff @@ -22,12 +22,11 @@ StorageLive(_3); StorageLive(_4); _9 = const main::promoted[0]; -- _4 = _9; + _4 = _9; - _3 = _4; - _2 = move _3 as &[u32] (PointerCoercion(Unsize)); -+ _4 = const {ALLOC0: &[u32; 3]}; -+ _3 = const {ALLOC0: &[u32; 3]}; -+ _2 = const {ALLOC0: &[u32; 3]} as &[u32] (PointerCoercion(Unsize)); ++ _3 = _9; ++ _2 = _9 as &[u32] (PointerCoercion(Unsize)); StorageDead(_3); StorageLive(_6); _6 = const 1_usize; @@ -50,6 +49,4 @@ return; } } -+ -+ ALLOC0 (size: 12, align: 4) { .. } diff --git a/tests/mir-opt/const_prop/slice_len.rs b/tests/mir-opt/const_prop/slice_len.rs index 221fb18f92c61..3d1b58965ac4e 100644 --- a/tests/mir-opt/const_prop/slice_len.rs +++ b/tests/mir-opt/const_prop/slice_len.rs @@ -7,7 +7,7 @@ fn main() { // CHECK-LABEL: fn main( // CHECK: debug a => [[a:_.*]]; - // CHECK: [[slice:_.*]] = const {{.*}} as &[u32] (PointerCoercion(Unsize)); + // CHECK: [[slice:_.*]] = {{.*}} as &[u32] (PointerCoercion(Unsize)); // CHECK: assert(const true, // CHECK: [[a]] = const 2_u32; let a = (&[1u32, 2, 3] as &[u32])[1]; diff --git a/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.32bit.panic-abort.diff b/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.32bit.panic-abort.diff index 959efa2a54870..30c122ddea03b 100644 --- a/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.32bit.panic-abort.diff +++ b/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.32bit.panic-abort.diff @@ -73,13 +73,12 @@ StorageLive(_6); StorageLive(_7); _9 = const main::promoted[0]; -- _7 = _9; -+ _7 = const {ALLOC1: &std::alloc::Global}; + _7 = _9; StorageLive(_8); - _8 = _1; - _6 = std::alloc::Global::alloc_impl(move _7, move _8, const false) -> [return: bb4, unwind unreachable]; + _8 = const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(4 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x00000000): std::ptr::alignment::AlignmentEnum) }}; -+ _6 = std::alloc::Global::alloc_impl(const {ALLOC1: &std::alloc::Global}, const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(4 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x00000000): std::ptr::alignment::AlignmentEnum) }}, const false) -> [return: bb4, unwind unreachable]; ++ _6 = std::alloc::Global::alloc_impl(_9, const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(4 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x00000000): std::ptr::alignment::AlignmentEnum) }}, const false) -> [return: bb4, unwind unreachable]; } bb4: { @@ -119,11 +118,9 @@ + nop; return; } - } ++ } + + ALLOC0 (size: 8, align: 4) { + 00 00 00 00 __ __ __ __ │ ....░░░░ -+ } -+ -+ ALLOC1 (size: 0, align: 1) {} + } diff --git a/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.32bit.panic-unwind.diff b/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.32bit.panic-unwind.diff index 4a37c8603204b..93449462c3d08 100644 --- a/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.32bit.panic-unwind.diff +++ b/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.32bit.panic-unwind.diff @@ -81,13 +81,12 @@ StorageLive(_6); StorageLive(_7); _9 = const main::promoted[0]; -- _7 = _9; -+ _7 = const {ALLOC1: &std::alloc::Global}; + _7 = _9; StorageLive(_8); - _8 = _1; - _6 = std::alloc::Global::alloc_impl(move _7, move _8, const false) -> [return: bb5, unwind continue]; + _8 = const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(4 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x00000000): std::ptr::alignment::AlignmentEnum) }}; -+ _6 = std::alloc::Global::alloc_impl(const {ALLOC1: &std::alloc::Global}, const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(4 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x00000000): std::ptr::alignment::AlignmentEnum) }}, const false) -> [return: bb5, unwind continue]; ++ _6 = std::alloc::Global::alloc_impl(_9, const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(4 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x00000000): std::ptr::alignment::AlignmentEnum) }}, const false) -> [return: bb5, unwind continue]; } bb5: { @@ -95,11 +94,9 @@ StorageDead(_7); _5 = Result::, std::alloc::AllocError>::unwrap(move _6) -> [return: bb1, unwind continue]; } - } ++ } + + ALLOC0 (size: 8, align: 4) { + 00 00 00 00 __ __ __ __ │ ....░░░░ -+ } -+ -+ ALLOC1 (size: 0, align: 1) {} + } diff --git a/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.64bit.panic-abort.diff b/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.64bit.panic-abort.diff index 97f5245a8c927..44435956ec418 100644 --- a/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.64bit.panic-abort.diff +++ b/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.64bit.panic-abort.diff @@ -73,13 +73,12 @@ StorageLive(_6); StorageLive(_7); _9 = const main::promoted[0]; -- _7 = _9; -+ _7 = const {ALLOC1: &std::alloc::Global}; + _7 = _9; StorageLive(_8); - _8 = _1; - _6 = std::alloc::Global::alloc_impl(move _7, move _8, const false) -> [return: bb4, unwind unreachable]; + _8 = const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(8 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x0000000000000000): std::ptr::alignment::AlignmentEnum) }}; -+ _6 = std::alloc::Global::alloc_impl(const {ALLOC1: &std::alloc::Global}, const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(8 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x0000000000000000): std::ptr::alignment::AlignmentEnum) }}, const false) -> [return: bb4, unwind unreachable]; ++ _6 = std::alloc::Global::alloc_impl(_9, const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(8 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x0000000000000000): std::ptr::alignment::AlignmentEnum) }}, const false) -> [return: bb4, unwind unreachable]; } bb4: { @@ -119,11 +118,9 @@ + nop; return; } - } ++ } + + ALLOC0 (size: 16, align: 8) { + 00 00 00 00 00 00 00 00 __ __ __ __ __ __ __ __ │ ........░░░░░░░░ -+ } -+ -+ ALLOC1 (size: 0, align: 1) {} + } diff --git a/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.64bit.panic-unwind.diff b/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.64bit.panic-unwind.diff index ec2e95fecb620..c958480a9c71b 100644 --- a/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.64bit.panic-unwind.diff +++ b/tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.64bit.panic-unwind.diff @@ -81,13 +81,12 @@ StorageLive(_6); StorageLive(_7); _9 = const main::promoted[0]; -- _7 = _9; -+ _7 = const {ALLOC1: &std::alloc::Global}; + _7 = _9; StorageLive(_8); - _8 = _1; - _6 = std::alloc::Global::alloc_impl(move _7, move _8, const false) -> [return: bb5, unwind continue]; + _8 = const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(8 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x0000000000000000): std::ptr::alignment::AlignmentEnum) }}; -+ _6 = std::alloc::Global::alloc_impl(const {ALLOC1: &std::alloc::Global}, const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(8 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x0000000000000000): std::ptr::alignment::AlignmentEnum) }}, const false) -> [return: bb5, unwind continue]; ++ _6 = std::alloc::Global::alloc_impl(_9, const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(8 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x0000000000000000): std::ptr::alignment::AlignmentEnum) }}, const false) -> [return: bb5, unwind continue]; } bb5: { @@ -95,11 +94,9 @@ StorageDead(_7); _5 = Result::, std::alloc::AllocError>::unwrap(move _6) -> [return: bb1, unwind continue]; } - } ++ } + + ALLOC0 (size: 16, align: 8) { + 00 00 00 00 00 00 00 00 __ __ __ __ __ __ __ __ │ ........░░░░░░░░ -+ } -+ -+ ALLOC1 (size: 0, align: 1) {} + } From 95986dd279d0bd7b292d0b0907fcb5e8d4b64c9c Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 26 Jun 2024 09:44:36 +0000 Subject: [PATCH 2/7] Indirect places can only appear as first projection in runtime MIR. --- compiler/rustc_mir_transform/src/gvn.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 96b2f9cbbdc94..d9bfe44633ab7 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -671,7 +671,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { fn simplify_place_projection(&mut self, place: &mut Place<'tcx>, location: Location) { // If the projection is indirect, we treat the local as a value, so can replace it with // another local. - if place.is_indirect() + if place.is_indirect_first_projection() && let Some(base) = self.locals[place.local] && let Some(new_local) = self.try_as_local(base, location) && place.local != new_local From 70ee6e4b23f455b62554c3ee9365657f9e700abd Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 26 Jun 2024 09:44:03 +0000 Subject: [PATCH 3/7] Amortize growing rev_locals. --- compiler/rustc_mir_transform/src/gvn.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index d9bfe44633ab7..8eab9890d634f 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -293,9 +293,15 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { let (index, new) = self.values.insert_full(value); let index = VnIndex::from_usize(index); if new { + // Grow `evaluated` and `rev_locals` here to amortize the allocations. let evaluated = self.eval_to_const(index); let _index = self.evaluated.push(evaluated); debug_assert_eq!(index, _index); + // No need to push to `rev_locals` if we finished listing assignments. + if self.next_opaque.is_some() { + let _index = self.rev_locals.push(SmallVec::new()); + debug_assert_eq!(index, _index); + } } index } @@ -332,7 +338,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { let is_sized = !self.feature_unsized_locals || self.local_decls[local].ty.is_sized(self.tcx, self.param_env); if is_sized { - self.rev_locals.ensure_contains_elem(value, SmallVec::new).push(local); + self.rev_locals[value].push(local); } } From 9d23c8617632690326ac04e25646ac669179de4b Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 26 Jun 2024 12:16:40 +0000 Subject: [PATCH 4/7] Reduce allocations in GVN. --- compiler/rustc_mir_transform/src/gvn.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 8eab9890d634f..c290ad6fa2cbf 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -128,7 +128,7 @@ fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { // Clone dominators as we need them while mutating the body. let dominators = body.basic_blocks.dominators().clone(); - let mut state = VnState::new(tcx, param_env, &ssa, &dominators, &body.local_decls); + let mut state = VnState::new(tcx, body, param_env, &ssa, &dominators, &body.local_decls); ssa.for_each_assignment_mut( body.basic_blocks.as_mut_preserves_cfg(), |local, value, location| { @@ -266,20 +266,28 @@ struct VnState<'body, 'tcx> { impl<'body, 'tcx> VnState<'body, 'tcx> { fn new( tcx: TyCtxt<'tcx>, + body: &Body<'tcx>, param_env: ty::ParamEnv<'tcx>, ssa: &'body SsaLocals, dominators: &'body Dominators, local_decls: &'body LocalDecls<'tcx>, ) -> Self { + // Compute a rough estimate of the number of values in the body from the number of + // statements. This is meant to reduce the number of allocations, but it's all right if + // we miss the exact amount. We estimate based on 2 values per statement (one in LHS and + // one in RHS) and 4 values per terminator (for call operands). + let num_values = + 2 * body.basic_blocks.iter().map(|bbdata| bbdata.statements.len()).sum::() + + 4 * body.basic_blocks.len(); VnState { tcx, ecx: InterpCx::new(tcx, DUMMY_SP, param_env, DummyMachine), param_env, local_decls, locals: IndexVec::from_elem(None, local_decls), - rev_locals: IndexVec::default(), - values: FxIndexSet::default(), - evaluated: IndexVec::new(), + rev_locals: IndexVec::with_capacity(num_values), + values: FxIndexSet::with_capacity_and_hasher(num_values, Default::default()), + evaluated: IndexVec::with_capacity(num_values), next_opaque: Some(0), feature_unsized_locals: tcx.features().unsized_locals, ssa, From 61ef0441b8a618b80f72a6a5f0d4c8f7a983566e Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 26 Jun 2024 13:27:38 +0000 Subject: [PATCH 5/7] Encode constant determinism in disambiguator. --- compiler/rustc_mir_transform/src/gvn.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index c290ad6fa2cbf..92a112ceff191 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -204,6 +204,7 @@ enum Value<'tcx> { value: Const<'tcx>, /// Some constants do not have a deterministic value. To avoid merging two instances of the /// same `Const`, we assign them an additional integer index. + // `disambiguator` is 0 iff the constant is deterministic. disambiguator: usize, }, /// An aggregate value, either tuple/closure/struct/enum. @@ -288,7 +289,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { rev_locals: IndexVec::with_capacity(num_values), values: FxIndexSet::with_capacity_and_hasher(num_values, Default::default()), evaluated: IndexVec::with_capacity(num_values), - next_opaque: Some(0), + next_opaque: Some(1), feature_unsized_locals: tcx.features().unsized_locals, ssa, dominators, @@ -360,6 +361,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { let next_opaque = self.next_opaque.as_mut()?; let disambiguator = *next_opaque; *next_opaque += 1; + assert_ne!(disambiguator, 0); disambiguator }; Some(self.insert(Value::Constant { value, disambiguator })) @@ -1447,12 +1449,11 @@ impl<'tcx> VnState<'_, 'tcx> { /// If `index` is a `Value::Constant`, return the `Constant` to be put in the MIR. fn try_as_constant(&mut self, index: VnIndex) -> Option> { - // This was already constant in MIR, do not change it. - if let Value::Constant { value, disambiguator: _ } = *self.get(index) - // 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() - { + // This was already constant in MIR, do not change it. If the constant is not + // deterministic, adding an additional mention of it in MIR will not give the same value as + // the former mention. + if let Value::Constant { value, disambiguator: 0 } = *self.get(index) { + assert!(value.is_deterministic()); return Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_: value }); } From 98c1ea8e82e276a42159c5ff75d3581ed33e7bec Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 26 Jun 2024 13:55:57 +0000 Subject: [PATCH 6/7] Simplify constant creation. --- compiler/rustc_mir_transform/src/gvn.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 92a112ceff191..7845fc6e4f6a4 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -361,7 +361,8 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { let next_opaque = self.next_opaque.as_mut()?; let disambiguator = *next_opaque; *next_opaque += 1; - assert_ne!(disambiguator, 0); + // `disambiguator: 0` means deterministic. + debug_assert_ne!(disambiguator, 0); disambiguator }; Some(self.insert(Value::Constant { value, disambiguator })) @@ -369,12 +370,16 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { fn insert_bool(&mut self, flag: bool) -> VnIndex { // Booleans are deterministic. - self.insert(Value::Constant { value: Const::from_bool(self.tcx, flag), disambiguator: 0 }) + let value = Const::from_bool(self.tcx, flag); + debug_assert!(value.is_deterministic()); + self.insert(Value::Constant { value, disambiguator: 0 }) } fn insert_scalar(&mut self, scalar: Scalar, ty: Ty<'tcx>) -> VnIndex { - self.insert_constant(Const::from_scalar(self.tcx, scalar, ty)) - .expect("scalars are deterministic") + // Scalars are deterministic. + let value = Const::from_scalar(self.tcx, scalar, ty); + debug_assert!(value.is_deterministic()); + self.insert(Value::Constant { value, disambiguator: 0 }) } fn insert_tuple(&mut self, values: Vec) -> VnIndex { @@ -1453,7 +1458,7 @@ impl<'tcx> VnState<'_, 'tcx> { // deterministic, adding an additional mention of it in MIR will not give the same value as // the former mention. if let Value::Constant { value, disambiguator: 0 } = *self.get(index) { - assert!(value.is_deterministic()); + debug_assert!(value.is_deterministic()); return Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_: value }); } From 4067795d8c88fd38f3295fc4f9fc2658ea4d9ff7 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 26 Jun 2024 13:27:50 +0000 Subject: [PATCH 7/7] Do not intern if we have provenance. --- compiler/rustc_mir_transform/src/gvn.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 7845fc6e4f6a4..e16911d79c378 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -1389,8 +1389,13 @@ fn op_to_prop_const<'tcx>( // If this constant has scalar ABI, return it as a `ConstValue::Scalar`. if let Abi::Scalar(abi::Scalar::Initialized { .. }) = op.layout.abi && let Ok(scalar) = ecx.read_scalar(op) - && scalar.try_to_scalar_int().is_ok() { + if !scalar.try_to_scalar_int().is_ok() { + // Check that we do not leak a pointer. + // Those pointers may lose part of their identity in codegen. + // FIXME: remove this hack once https://github.com/rust-lang/rust/issues/79738 is fixed. + return None; + } return Some(ConstValue::Scalar(scalar)); }