Enabling GVN produces a const allocation with the wrong alignment?

[saethlin]

rustc +nightly ice.rs -Copt-level=0 -Zmir-opt-level=2 -Zinline-mir -Zmir-enable-passes=+GVN

struct Affine2 {
    matrix2: Mat2,
    translation: Vec2,
}

impl Affine2 {
    #[inline]
    fn inverse(&self) {
        mat2_ref(&self.matrix2);
        vec2_move(self.translation);
    }
}

#[derive(Clone, Copy)]
#[repr(align(8))]
struct Mat2([f32; 4]);

#[derive(Clone, Copy)]
struct Vec2 {
    x: f32, 
    y: f32, 
}

#[inline(never)]
fn mat2_ref(_: &Mat2) {
    loop {}
}

#[inline(never)]
fn vec2_move(_: Vec2) {
    loop {}
}

fn main() {
    Affine2 {
        matrix2: Mat2([0.0, 0.0, 0.0, 0.0]),
        translation: Vec2 { x: 0.0, y: 0.0 },
    }.inverse();
}

Error:

thread 'rustc' panicked at /rustc/f967532a47eb728ada44473a5c4c2eca1a45fe30/compiler/rustc_codegen_ssa/src/mir/operand.rs:135:9:
assertion `left == right` failed
  left: Align(8 bytes)
 right: Align(4 bytes)

That's this:

rust/compiler/rustc_codegen_ssa/src/mir/operand.rs

Lines 128 to 135 in 06e02d5

	fn from_const_alloc<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
	bx: &mut Bx,
	layout: TyAndLayout<'tcx>,
	alloc: rustc_middle::mir::interpret::ConstAllocation<'tcx>,
	offset: Size,
	) -> Self {
	let alloc_align = alloc.inner().align;
	assert_eq!(alloc_align, layout.align.abi);

The GVN MIR diff is this:

diff --git a/ice.main.005-017.GVN.before.mir b/ice.main.005-017.GVN.after.mir
index 512091e..af00cdf 100644
--- a/ice.main.005-017.GVN.before.mir
+++ b/ice.main.005-017.GVN.after.mir
@@ -1,4 +1,4 @@
-// MIR for `main` before GVN
+// MIR for `main` after GVN
 
 fn main() -> () {
     let mut _0: ();
@@ -22,7 +22,12 @@ fn main() -> () {
     }
 
     bb2: {
-        _5 = ((*_1).1: Vec2);
-        _4 = vec2_move(move _5) -> [return: bb1, unwind continue];
+        _5 = const Vec2 {{ x: 0f32, y: 0f32 }};
+        _4 = vec2_move(const Vec2 {{ x: 0f32, y: 0f32 }}) -> [return: bb1, unwind continue];
     }
 }
+
+alloc11 (size: 24, align: 8) {
+    0x00 │ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 │ ................
+    0x10 │ 00 00 00 00 00 00 00 00                         │ ........
+}

searched nightlies: from nightly-2023-06-01 to nightly-2023-11-08
regressed nightly: nightly-2023-10-30
searched commit range: e5cfc55...608e968
regressed commit: 83c9732

bisected with cargo-bisect-rustc v0.6.7

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2023-06-01 --end=2023-11-08 --script script 

rustc 1.75.0-nightly (fdaaaf9f9 2023-11-08)
binary: rustc
commit-hash: fdaaaf9f923281ab98b865259aa40fbf93d72c7a
commit-date: 2023-11-08
host: x86_64-unknown-linux-gnu
release: 1.75.0-nightly
LLVM version: 17.0.4

[saethlin]

This issue is a bit old but I've now minimized the reproducer, it's in the description.

@cjgillot the assertion that's failing here looks to me like it is simply too picky, and it should be checking if the alignment of the allocation is >= the alignment of the type we're reading from it, instead of exactly equal. Do you agree that this is just a too-picky assertion?

[cjgillot]

I agree the assertion is just too picky. I expect the subtlety of needing to pass the larger alignment for the llvm constant.

[saethlin]

I expect the subtlety of needing to pass the larger alignment for the llvm constant.

I can't figure out what you mean here. What LLVM constant? Why is there subtelty? Maybe I just don't understand what's going on with GVN here, I'm assuming that GVN has merged two constants together so that we're now loading part of a more-aligned constant to get a less-aligned one. Is that wrong?

[cjgillot]

I mean, this is codegen code. It creates a constant for llvm. That constant has some alignment. We need to pick the right one, between the lhs and rhs of the assert.

[saethlin]

We need to pick the right one, between the lhs and rhs of the assert.

Where are we picking an alignment? Is it down here?

rust/compiler/rustc_codegen_ssa/src/mir/operand.rs

Line 188 in c71c246

let base_addr = bx.static_addr_of(init, alloc_align, None);

[matthiaskrgr]

smaller repro:

auto-reduced (treereduce-rust):

struct S(i32);

struct SmallStruct(f32, Option<S>, &'static [f32]);

fn main() {
    let mut s = S(1);

    s.0 = 3;

    const SMALL_VAL: SmallStruct = SmallStruct(4., Some(S(1)), &[]);
    let SmallStruct(a, b, c) = SMALL_VAL;
}

original code

original:

// skip-filecheck
// unit-test: DataflowConstProp
// EMIT_MIR_FOR_EACH_BIT_WIDTH

#[derive(Copy, Clone)]
struct S(i32);

#[derive(Copy, Clone)]
struct SmallStruct(f32, Option<S>, &'static [f32]);

#[derive(Copy, Clone)]
struct BigStruct(f32, Option<S>, &'static [f32]);

// EMIT_MIR struct.main.DataflowConstProp.diff
fn main() {
    let mut s = S(1);
    let a = s.0 + 2;
    s.0 = 3;
    let b = a + s.0;

    const SMALL_VAL: SmallStruct = SmallStruct(4., Some(S(1)), &[]);
    let SmallStruct(a, b, c) = SMALL_VAL;

    static SMALL_STAT: &SmallStruct = &SmallStruct(9., None, &[13.]);
    let SmallStruct(a, b, c) = *SMALL_STAT;

    let ss = SmallStruct(a, b, c);

    const BIG_VAL: BigStruct = BigStruct(25., None, &[]);
    let BigStruct(a, b, c) = BIG_VAL;

    static BIG_STAT: &BigStruct = &BigStruct(82., Some(S(35)), &[45., 72.]);
    let BigStruct(a, b, c) = *BIG_STAT;

    // We arbitrarily limit the size of synthetized values to 4 pointers.
    // `BigStruct` can be read, but we will keep a MIR aggregate for this.
    let bs = BigStruct(a, b, c);
}

Version information

rustc 1.76.0-nightly (77d169975 2023-12-13)
binary: rustc
commit-hash: 77d16997566dfb2384bcbc504e2579c7ae973666
commit-date: 2023-12-13
host: x86_64-unknown-linux-gnu
release: 1.76.0-nightly
LLVM version: 17.0.5

Command:
/home/matthias/.rustup/toolchains/master/bin/rustc -Zmir-opt-level=5 -Cdebuginfo=2

Program output

warning: unused variable: `a`
  --> /tmp/icemaker_global_tempdir.YesGFy3vBeN9/rustc_testrunner_tmpdir_reporting.dU6Kp2JxObaA/mvce.rs:11:21
   |
11 |     let SmallStruct(a, b, c) = SMALL_VAL;
   |                     ^ help: if this is intentional, prefix it with an underscore: `_a`
   |
   = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `b`
  --> /tmp/icemaker_global_tempdir.YesGFy3vBeN9/rustc_testrunner_tmpdir_reporting.dU6Kp2JxObaA/mvce.rs:11:24
   |
11 |     let SmallStruct(a, b, c) = SMALL_VAL;
   |                        ^ help: if this is intentional, prefix it with an underscore: `_b`

warning: unused variable: `c`
  --> /tmp/icemaker_global_tempdir.YesGFy3vBeN9/rustc_testrunner_tmpdir_reporting.dU6Kp2JxObaA/mvce.rs:11:27
   |
11 |     let SmallStruct(a, b, c) = SMALL_VAL;
   |                           ^ help: if this is intentional, prefix it with an underscore: `_c`

thread 'rustc' panicked at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/compiler/rustc_codegen_ssa/src/mir/operand.rs:135:9:
assertion `left == right` failed
  left: Align(8 bytes)
 right: Align(4 bytes)
stack backtrace:
   0:     0x7f9fdaf8b45c - std::backtrace_rs::backtrace::libunwind::trace::h8eaf990e4a504b86
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/std/src/../../backtrace/src/backtrace/libunwind.rs:104:5
   1:     0x7f9fdaf8b45c - std::backtrace_rs::backtrace::trace_unsynchronized::hbc236c387b7f1e2f
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x7f9fdaf8b45c - std::sys_common::backtrace::_print_fmt::h854fa9a625b79fe7
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/std/src/sys_common/backtrace.rs:68:5
   3:     0x7f9fdaf8b45c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h367ac729761cc34a
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x7f9fdafde510 - core::fmt::rt::Argument::fmt::hc045d8fab7f11469
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/core/src/fmt/rt.rs:142:9
   5:     0x7f9fdafde510 - core::fmt::write::h5adaab5759d35e9c
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/core/src/fmt/mod.rs:1120:17
   6:     0x7f9fdaf7f34f - std::io::Write::write_fmt::hdf01958bd3129cc1
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/std/src/io/mod.rs:1810:15
   7:     0x7f9fdaf8b244 - std::sys_common::backtrace::_print::hef493aa049da10fb
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/std/src/sys_common/backtrace.rs:47:5
   8:     0x7f9fdaf8b244 - std::sys_common::backtrace::print::hceb63a7aa0b95526
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/std/src/sys_common/backtrace.rs:34:9
   9:     0x7f9fdaf8df47 - std::panicking::default_hook::{{closure}}::h379064c2e91c2583
  10:     0x7f9fdaf8dcaf - std::panicking::default_hook::h74d2197136da14a8
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/std/src/panicking.rs:292:9
  11:     0x7f9fddd35c30 - std[5e29b6ead2e77b88]::panicking::update_hook::<alloc[57adf5c47884753f]::boxed::Box<rustc_driver_impl[143a96d1e9339ddc]::install_ice_hook::{closure#0}>>::{closure#0}
  12:     0x7f9fdaf8e688 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::he81fb398d05e4b0f
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/alloc/src/boxed.rs:2029:9
  13:     0x7f9fdaf8e688 - std::panicking::rust_panic_with_hook::h2b6da4caf2f04b60
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/std/src/panicking.rs:783:13
  14:     0x7f9fdaf8e3de - std::panicking::begin_panic_handler::{{closure}}::h830b6617ba119779
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/std/src/panicking.rs:657:13
  15:     0x7f9fdaf8b926 - std::sys_common::backtrace::__rust_end_short_backtrace::h55a117102e902ec7
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/std/src/sys_common/backtrace.rs:171:18
  16:     0x7f9fdaf8e142 - rust_begin_unwind
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/std/src/panicking.rs:645:5
  17:     0x7f9fdafdabd5 - core::panicking::panic_fmt::hf8c847bebf55f403
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/core/src/panicking.rs:72:14
  18:     0x7f9fdafdb16b - core::panicking::assert_failed_inner::h69aedbea0b3a1592
  19:     0x7f9fddbacba3 - core[52cdeeaf75e7a502]::panicking::assert_failed::<rustc_abi[5034139aa1a3bb3a]::Align, rustc_abi[5034139aa1a3bb3a]::Align>
  20:     0x7f9fdfd63474 - rustc_codegen_ssa[8440f8116a4e2fa3]::mir::codegen_mir::<rustc_codegen_llvm[d7aba16b4ff04fd4]::builder::Builder>
  21:     0x7f9fdffb2d08 - rustc_codegen_llvm[d7aba16b4ff04fd4]::base::compile_codegen_unit::module_codegen
  22:     0x7f9fdffb0b2d - <rustc_codegen_llvm[d7aba16b4ff04fd4]::LlvmCodegenBackend as rustc_codegen_ssa[8440f8116a4e2fa3]::traits::backend::ExtraBackendMethods>::compile_codegen_unit
  23:     0x7f9fdfd86380 - rustc_codegen_ssa[8440f8116a4e2fa3]::base::codegen_crate::<rustc_codegen_llvm[d7aba16b4ff04fd4]::LlvmCodegenBackend>
  24:     0x7f9fdfd85bfa - <rustc_codegen_llvm[d7aba16b4ff04fd4]::LlvmCodegenBackend as rustc_codegen_ssa[8440f8116a4e2fa3]::traits::backend::CodegenBackend>::codegen_crate
  25:     0x7f9fdfd83dc5 - rustc_interface[1010bb494e88232]::passes::start_codegen
  26:     0x7f9fdfd8352e - <rustc_interface[1010bb494e88232]::queries::Queries>::codegen_and_build_linker
  27:     0x7f9fe003afad - rustc_interface[1010bb494e88232]::interface::run_compiler::<core[52cdeeaf75e7a502]::result::Result<(), rustc_span[80493f288ab27be]::ErrorGuaranteed>, rustc_driver_impl[143a96d1e9339ddc]::run_compiler::{closure#0}>::{closure#0}
  28:     0x7f9fdffaf54a - std[5e29b6ead2e77b88]::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface[1010bb494e88232]::util::run_in_thread_with_globals<rustc_interface[1010bb494e88232]::util::run_in_thread_pool_with_globals<rustc_interface[1010bb494e88232]::interface::run_compiler<core[52cdeeaf75e7a502]::result::Result<(), rustc_span[80493f288ab27be]::ErrorGuaranteed>, rustc_driver_impl[143a96d1e9339ddc]::run_compiler::{closure#0}>::{closure#0}, core[52cdeeaf75e7a502]::result::Result<(), rustc_span[80493f288ab27be]::ErrorGuaranteed>>::{closure#0}, core[52cdeeaf75e7a502]::result::Result<(), rustc_span[80493f288ab27be]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[52cdeeaf75e7a502]::result::Result<(), rustc_span[80493f288ab27be]::ErrorGuaranteed>>
  29:     0x7f9fdffaf373 - <<std[5e29b6ead2e77b88]::thread::Builder>::spawn_unchecked_<rustc_interface[1010bb494e88232]::util::run_in_thread_with_globals<rustc_interface[1010bb494e88232]::util::run_in_thread_pool_with_globals<rustc_interface[1010bb494e88232]::interface::run_compiler<core[52cdeeaf75e7a502]::result::Result<(), rustc_span[80493f288ab27be]::ErrorGuaranteed>, rustc_driver_impl[143a96d1e9339ddc]::run_compiler::{closure#0}>::{closure#0}, core[52cdeeaf75e7a502]::result::Result<(), rustc_span[80493f288ab27be]::ErrorGuaranteed>>::{closure#0}, core[52cdeeaf75e7a502]::result::Result<(), rustc_span[80493f288ab27be]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[52cdeeaf75e7a502]::result::Result<(), rustc_span[80493f288ab27be]::ErrorGuaranteed>>::{closure#1} as core[52cdeeaf75e7a502]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
  30:     0x7f9fdaf98515 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::ha17502cbcaf4d2cc
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/alloc/src/boxed.rs:2015:9
  31:     0x7f9fdaf98515 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::h928bfdaf626c1d3d
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/alloc/src/boxed.rs:2015:9
  32:     0x7f9fdaf98515 - std::sys::unix::thread::Thread::new::thread_start::he4ab37baefb01514
                               at /rustc/77d16997566dfb2384bcbc504e2579c7ae973666/library/std/src/sys/unix/thread.rs:108:17
  33:     0x7f9fdad889eb - <unknown>
  34:     0x7f9fdae0c7cc - <unknown>
  35:                0x0 - <unknown>

error: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.76.0-nightly (77d169975 2023-12-13) running on x86_64-unknown-linux-gnu

note: compiler flags: -Z mir-opt-level=5 -C debuginfo=2 -Z dump-mir-dir=dir

query stack during panic:
end of query stack
warning: 3 warnings emitted

[cjgillot]

Where are we picking an alignment? Is it down here?

I don't really know.

TBH, the simplest solution is a codegen test, and check that the generated constant has the right over-alignment.

[saethlin]

I don't have confidence in my ability to do that, so I think it would unblock you faster if you took this issue.

I feel like just this diff is a fix for this ICE:

diff --git a/compiler/rustc_codegen_ssa/src/mir/operand.rs b/compiler/rustc_codegen_ssa/src/mir/operand.rs
index e8c58f6b6f8..794cbd315b7 100644
--- a/compiler/rustc_codegen_ssa/src/mir/operand.rs
+++ b/compiler/rustc_codegen_ssa/src/mir/operand.rs
@@ -132,7 +132,7 @@ fn from_const_alloc<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
         offset: Size,
     ) -> Self {
         let alloc_align = alloc.inner().align;
-        assert_eq!(alloc_align, layout.align.abi);
+        assert!(alloc_align >= layout.align.abi);
 
         let read_scalar = |start, size, s: abi::Scalar, ty| {
             match alloc.0.read_scalar(

But I don't understand why the assertion was exact in the first place, or what the hazard would be from over-aligning the allocation.