Rollup merge of #122287 - RalfJung:simd-static-assert, r=pnkfelix

add test ensuring simd codegen checks don't run when a static assertion failed

stdarch relies on this to ensure that SIMD indices are in bounds.

I would love to know why this works, but I can't figure out where codegen decides to not codegen a function if a required-const does not evaluate. `@oli-obk` `@bjorn3` do you have any idea?

compiler/rustc_codegen_cranelift/src/constant.rs

@@ -71,7 +71,7 @@ pub(crate) fn eval_mir_constant<'tcx>(
     // This cannot fail because we checked all required_consts in advance.
     let val = cv
         .eval(fx.tcx, ty::ParamEnv::reveal_all(), Some(constant.span))
-        .expect("erroneous constant not captured by required_consts");
+        .expect("erroneous constant missed by mono item collection");
     (val, cv.ty())
 }
 

compiler/rustc_codegen_ssa/src/mir/constant.rs

@@ -21,11 +21,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
     }
 
     pub fn eval_mir_constant(&self, constant: &mir::ConstOperand<'tcx>) -> mir::ConstValue<'tcx> {
-        // `MirUsedCollector` visited all constants before codegen began, so if we got here there
+        // `MirUsedCollector` visited all required_consts before codegen began, so if we got here
-        // can be no more constants that fail to evaluate.
+        // there can be no more constants that fail to evaluate.
         self.monomorphize(constant.const_)
             .eval(self.cx.tcx(), ty::ParamEnv::reveal_all(), Some(constant.span))
-            .expect("erroneous constant not captured by required_consts")
+            .expect("erroneous constant missed by mono item collection")
     }
 
     /// This is a convenience helper for `simd_shuffle_indices`. It has the precondition

compiler/rustc_codegen_ssa/src/mir/mod.rs

@@ -211,7 +211,8 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
 
     // It may seem like we should iterate over `required_consts` to ensure they all successfully
     // evaluate; however, the `MirUsedCollector` already did that during the collection phase of
-    // monomorphization so we don't have to do it again.
+    // monomorphization, and if there is an error during collection then codegen never starts -- so
+    // we don't have to do it again.
 
     fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info(&mut start_bx);
 

compiler/rustc_monomorphize/src/collector.rs

@@ -818,13 +818,16 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
         self.super_rvalue(rvalue, location);
     }
 
-    /// This does not walk the constant, as it has been handled entirely here and trying
+    /// This does not walk the MIR of the constant as that is not needed for codegen, all we need is
-    /// to walk it would attempt to evaluate the `ty::Const` inside, which doesn't necessarily
+    /// to ensure that the constant evaluates successfully and walk the result.
-    /// work, as some constants cannot be represented in the type system.
     #[instrument(skip(self), level = "debug")]
     fn visit_constant(&mut self, constant: &mir::ConstOperand<'tcx>, location: Location) {
         let const_ = self.monomorphize(constant.const_);
         let param_env = ty::ParamEnv::reveal_all();
+        // Evaluate the constant. This makes const eval failure a collection-time error (rather than
+        // a codegen-time error). rustc stops after collection if there was an error, so this
+        // ensures codegen never has to worry about failing consts.
+        // (codegen relies on this and ICEs will happen if this is violated.)
         let val = match const_.eval(self.tcx, param_env, None) {
             Ok(v) => v,
             Err(ErrorHandled::Reported(..)) => return,

compiler/rustc_monomorphize/src/partitioning.rs

@@ -1112,6 +1112,9 @@ fn collect_and_partition_mono_items(tcx: TyCtxt<'_>, (): ()) -> (&DefIdSet, &[Co
 
     let (items, usage_map) = collector::collect_crate_mono_items(tcx, collection_mode);
 
+    // If there was an error during collection (e.g. from one of the constants we evaluated),
+    // then we stop here. This way codegen does not have to worry about failing constants.
+    // (codegen relies on this and ICEs will happen if this is violated.)
     tcx.dcx().abort_if_errors();
 
     let (codegen_units, _) = tcx.sess.time("partition_and_assert_distinct_symbols", || {

tests/ui/simd/const-err-trumps-simd-err.rs

@@ -0,0 +1,24 @@
+//@build-fail
+//! Make sure that monomorphization-time const errors from `static_assert` take priority over the
+//! error from simd_extract. Basically this checks that if a const fails to evaluate in some
+//! function, we don't bother codegen'ing the function.
+#![feature(generic_arg_infer)]
+#![feature(core_intrinsics)]
+#![feature(repr_simd)]
+#![feature(inline_const)]
+use std::intrinsics::simd::*;
+
+#[repr(simd)]
+#[allow(non_camel_case_types)]
+struct int8x4_t(u8,u8,u8,u8);
+
+fn get_elem<const LANE: u32>(a: int8x4_t) -> u8 {
+    const { assert!(LANE < 4); } // the error should be here...
+    //~^ ERROR failed
+    //~| assertion failed
+    unsafe { simd_extract(a, LANE) } // ...not here
+}
+
+fn main() {
+    get_elem::<4>(int8x4_t(0,0,0,0));
+}

tests/ui/simd/const-err-trumps-simd-err.stderr

@@ -0,0 +1,17 @@
+error[E0080]: evaluation of `get_elem::<4>::{constant#0}` failed
+  --> $DIR/const-err-trumps-simd-err.rs:16:13
+   |
+LL |     const { assert!(LANE < 4); } // the error should be here...
+   |             ^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: LANE < 4', $DIR/const-err-trumps-simd-err.rs:16:13
+   |
+   = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+note: the above error was encountered while instantiating `fn get_elem::<4>`
+  --> $DIR/const-err-trumps-simd-err.rs:23:5
+   |
+LL |     get_elem::<4>(int8x4_t(0,0,0,0));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.