Skip to content

Commit

Permalink
Auto merge of rust-lang#115748 - RalfJung:post-mono, r=<try>
Browse files Browse the repository at this point in the history
move required_consts check to general post-mono-check function

I was hoping this would simplify the code. That didn't entirely happen, but I still think it could be useful to have a single place for the "required const" check -- and this does simplify some code in codegen, where the const-eval functions no longer have to return a `Result`.

Also this is in preparation for another post-mono check that will be needed for (the current hackfix for) rust-lang#115709: ensuring that all locals are dynamically sized.

I didn't expect this to change diagnostics, but the changes seem either harmless (for the cycle errors) or improvements (where more spans are shown when something goes wrong in a constant).

r? `@oli-obk`
  • Loading branch information
bors committed Sep 11, 2023
2 parents e2b3676 + ed1575c commit 34093c9
Show file tree
Hide file tree
Showing 98 changed files with 676 additions and 737 deletions.
23 changes: 12 additions & 11 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,12 +250,16 @@ pub(crate) fn verify_func(
}

fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
if !crate::constant::check_constants(fx) {
fx.bcx.append_block_params_for_function_params(fx.block_map[START_BLOCK]);
fx.bcx.switch_to_block(fx.block_map[START_BLOCK]);
// compilation should have been aborted
fx.bcx.ins().trap(TrapCode::UnreachableCodeReached);
return;
match fx.mir.post_mono_checks(fx.tcx, ty::ParamEnv::reveal_all(), |c| Ok(fx.monomorphize(c))) {
Ok(()) => {}
Err(err) => {
err.emit_err(fx.tcx);
fx.bcx.append_block_params_for_function_params(fx.block_map[START_BLOCK]);
fx.bcx.switch_to_block(fx.block_map[START_BLOCK]);
// compilation should have been aborted
fx.bcx.ins().trap(TrapCode::UnreachableCodeReached);
return;
}
}

let arg_uninhabited = fx
Expand Down Expand Up @@ -723,11 +727,8 @@ fn codegen_stmt<'tcx>(
}
Rvalue::Repeat(ref operand, times) => {
let operand = codegen_operand(fx, operand);
let times = fx
.monomorphize(times)
.eval(fx.tcx, ParamEnv::reveal_all())
.try_to_bits(fx.tcx.data_layout.pointer_size)
.unwrap();
let times =
fx.monomorphize(times).eval_target_usize(fx.tcx, ParamEnv::reveal_all());
if operand.layout().size.bytes() == 0 {
// Do nothing for ZST's
} else if fx.clif_type(operand.layout().ty) == Some(types::I8) {
Expand Down
62 changes: 10 additions & 52 deletions compiler/rustc_codegen_cranelift/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::mir::interpret::{
read_target_uint, AllocId, ConstAllocation, ConstValue, ErrorHandled, GlobalAlloc, Scalar,
read_target_uint, AllocId, ConstAllocation, ConstValue, GlobalAlloc, Scalar,
};

use cranelift_module::*;
Expand Down Expand Up @@ -33,16 +33,6 @@ impl ConstantCx {
}
}

pub(crate) fn check_constants(fx: &mut FunctionCx<'_, '_, '_>) -> bool {
let mut all_constants_ok = true;
for constant in &fx.mir.required_consts {
if eval_mir_constant(fx, constant).is_none() {
all_constants_ok = false;
}
}
all_constants_ok
}

pub(crate) fn codegen_static(tcx: TyCtxt<'_>, module: &mut dyn Module, def_id: DefId) {
let mut constants_cx = ConstantCx::new();
constants_cx.todo.push(TodoItem::Static(def_id));
Expand Down Expand Up @@ -76,52 +66,20 @@ pub(crate) fn codegen_tls_ref<'tcx>(
pub(crate) fn eval_mir_constant<'tcx>(
fx: &FunctionCx<'_, '_, 'tcx>,
constant: &Constant<'tcx>,
) -> Option<(ConstValue<'tcx>, Ty<'tcx>)> {
let constant_kind = fx.monomorphize(constant.literal);
let uv = match constant_kind {
ConstantKind::Ty(const_) => match const_.kind() {
ty::ConstKind::Unevaluated(uv) => uv.expand(),
ty::ConstKind::Value(val) => {
return Some((fx.tcx.valtree_to_const_val((const_.ty(), val)), const_.ty()));
}
err => span_bug!(
constant.span,
"encountered bad ConstKind after monomorphizing: {:?}",
err
),
},
ConstantKind::Unevaluated(mir::UnevaluatedConst { def, .. }, _)
if fx.tcx.is_static(def) =>
{
span_bug!(constant.span, "MIR constant refers to static");
}
ConstantKind::Unevaluated(uv, _) => uv,
ConstantKind::Val(val, _) => return Some((val, constant_kind.ty())),
};

let val = fx
.tcx
.const_eval_resolve(ty::ParamEnv::reveal_all(), uv, None)
.map_err(|err| match err {
ErrorHandled::Reported(_) => {
fx.tcx.sess.span_err(constant.span, "erroneous constant encountered");
}
ErrorHandled::TooGeneric => {
span_bug!(constant.span, "codegen encountered polymorphic constant: {:?}", err);
}
})
.ok();
val.map(|val| (val, constant_kind.ty()))
) -> (ConstValue<'tcx>, Ty<'tcx>) {
let cv = fx.monomorphize(constant.literal);
// 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");
(val, cv.ty())
}

pub(crate) fn codegen_constant_operand<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
constant: &Constant<'tcx>,
) -> CValue<'tcx> {
let (const_val, ty) = eval_mir_constant(fx, constant).unwrap_or_else(|| {
span_bug!(constant.span, "erroneous constant not captured by required_consts")
});

let (const_val, ty) = eval_mir_constant(fx, constant);
codegen_const_value(fx, const_val, ty)
}

Expand Down Expand Up @@ -479,7 +437,7 @@ pub(crate) fn mir_operand_get_const_val<'tcx>(
operand: &Operand<'tcx>,
) -> Option<ConstValue<'tcx>> {
match operand {
Operand::Constant(const_) => Some(eval_mir_constant(fx, const_).unwrap().0),
Operand::Constant(const_) => Some(eval_mir_constant(fx, const_).0),
// FIXME(rust-lang/rust#85105): Casts like `IMM8 as u32` result in the const being stored
// inside a temporary before being passed to the intrinsic requiring the const argument.
// This code tries to find a single constant defining definition of the referenced local.
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_codegen_cranelift/src/inline_asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,7 @@ pub(crate) fn codegen_inline_asm<'tcx>(
}
}
InlineAsmOperand::Const { ref value } => {
let (const_value, ty) = crate::constant::eval_mir_constant(fx, value)
.unwrap_or_else(|| span_bug!(span, "asm const cannot be resolved"));
let (const_value, ty) = crate::constant::eval_mir_constant(fx, value);
let value = rustc_codegen_ssa::common::asm_const_to_str(
fx.tcx,
span,
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_codegen_ssa/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ codegen_ssa_copy_path_buf = unable to copy {$source_file} to {$output_path}: {$e
codegen_ssa_create_temp_dir = couldn't create a temp dir: {$error}
codegen_ssa_erroneous_constant = erroneous constant encountered
codegen_ssa_error_creating_remark_dir = failed to create remark directory: {$error}
codegen_ssa_expected_used_symbol = expected `used`, `used(compiler)` or `used(linker)`
Expand Down Expand Up @@ -172,8 +170,6 @@ codegen_ssa_no_natvis_directory = error enumerating natvis directory: {$error}
codegen_ssa_option_gcc_only = option `-Z gcc-ld` is used even though linker flavor is not gcc
codegen_ssa_polymorphic_constant_too_generic = codegen encountered polymorphic constant: TooGeneric
codegen_ssa_processing_dymutil_failed = processing debug info with `dsymutil` failed: {$status}
.note = {$output}
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,10 +670,8 @@ fn push_const_param<'tcx>(tcx: TyCtxt<'tcx>, ct: ty::Const<'tcx>, output: &mut S
// avoiding collisions and will make the emitted type names shorter.
let hash_short = tcx.with_stable_hashing_context(|mut hcx| {
let mut hasher = StableHasher::new();
let ct = ct.eval(tcx, ty::ParamEnv::reveal_all());
hcx.while_hashing_spans(false, |hcx| {
ct.to_valtree().hash_stable(hcx, &mut hasher)
});
let ct = ct.eval(tcx, ty::ParamEnv::reveal_all(), None);
hcx.while_hashing_spans(false, |hcx| ct.hash_stable(hcx, &mut hasher));
hasher.finish::<Hash64>()
});

Expand Down
14 changes: 0 additions & 14 deletions compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -588,20 +588,6 @@ pub struct InvalidWindowsSubsystem {
pub subsystem: Symbol,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_erroneous_constant)]
pub struct ErroneousConstant {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_polymorphic_constant_too_generic)]
pub struct PolymorphicConstantTooGeneric {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_shuffle_indices_evaluation)]
pub struct ShuffleIndicesEvaluation {
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1109,9 +1109,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
InlineAsmOperandRef::InOut { reg, late, in_value, out_place }
}
mir::InlineAsmOperand::Const { ref value } => {
let const_value = self
.eval_mir_constant(value)
.unwrap_or_else(|_| span_bug!(span, "asm const cannot be resolved"));
let const_value = self.eval_mir_constant(value);
let string = common::asm_const_to_str(
bx.tcx(),
span,
Expand Down
51 changes: 11 additions & 40 deletions compiler/rustc_codegen_ssa/src/mir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,53 +14,24 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
&self,
bx: &mut Bx,
constant: &mir::Constant<'tcx>,
) -> Result<OperandRef<'tcx, Bx::Value>, ErrorHandled> {
let val = self.eval_mir_constant(constant)?;
) -> OperandRef<'tcx, Bx::Value> {
let val = self.eval_mir_constant(constant);
let ty = self.monomorphize(constant.ty());
Ok(OperandRef::from_const(bx, val, ty))
OperandRef::from_const(bx, val, ty)
}

pub fn eval_mir_constant(
&self,
constant: &mir::Constant<'tcx>,
) -> Result<ConstValue<'tcx>, ErrorHandled> {
let ct = self.monomorphize(constant.literal);
let uv = match ct {
mir::ConstantKind::Ty(ct) => match ct.kind() {
ty::ConstKind::Unevaluated(uv) => uv.expand(),
ty::ConstKind::Value(val) => {
return Ok(self.cx.tcx().valtree_to_const_val((ct.ty(), val)));
}
err => span_bug!(
constant.span,
"encountered bad ConstKind after monomorphizing: {:?}",
err
),
},
mir::ConstantKind::Unevaluated(uv, _) => uv,
mir::ConstantKind::Val(val, _) => return Ok(val),
};

self.cx.tcx().const_eval_resolve(ty::ParamEnv::reveal_all(), uv, None).map_err(|err| {
match err {
ErrorHandled::Reported(_) => {
self.cx.tcx().sess.emit_err(errors::ErroneousConstant { span: constant.span });
}
ErrorHandled::TooGeneric => {
self.cx
.tcx()
.sess
.diagnostic()
.emit_bug(errors::PolymorphicConstantTooGeneric { span: constant.span });
}
}
err
})
pub fn eval_mir_constant(&self, constant: &mir::Constant<'tcx>) -> ConstValue<'tcx> {
// This cannot fail because we checked all required_consts in advance.
self.monomorphize(constant.literal)
.eval(self.cx.tcx(), ty::ParamEnv::reveal_all(), Some(constant.span))
.expect("erroneous constant not captured by required_consts")
}

/// This is a convenience helper for `simd_shuffle_indices`. It has the precondition
/// that the given `constant` is an `ConstantKind::Unevaluated` and must be convertible to
/// a `ValTree`. If you want a more general version of this, talk to `wg-const-eval` on zulip.
///
/// Note that this function is cursed, since usually MIR consts should not be evaluated to valtrees!
pub fn eval_unevaluated_mir_constant_to_valtree(
&self,
constant: &mir::Constant<'tcx>,
Expand All @@ -80,7 +51,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// `simd_shuffle` call without wrapping the constant argument in a `const {}` block, but
// the user pass through arbitrary expressions.
// FIXME(oli-obk): replace the magic const generic argument of `simd_shuffle` with a real
// const generic.
// const generic, and get rid of this entire function.
other => span_bug!(constant.span, "{other:#?}"),
};
let uv = self.monomorphize(uv);
Expand Down
23 changes: 6 additions & 17 deletions compiler/rustc_codegen_ssa/src/mir/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -579,23 +579,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
if let Some(dbg_var) = dbg_var {
let Some(dbg_loc) = self.dbg_loc(var.source_info) else { continue };

if let Ok(operand) = self.eval_mir_constant_to_operand(bx, &c) {
self.set_debug_loc(bx, var.source_info);
let base = Self::spill_operand_to_stack(
operand,
Some(var.name.to_string()),
bx,
);

bx.dbg_var_addr(
dbg_var,
dbg_loc,
base.llval,
Size::ZERO,
&[],
fragment,
);
}
let operand = self.eval_mir_constant_to_operand(bx, &c);
self.set_debug_loc(bx, var.source_info);
let base =
Self::spill_operand_to_stack(operand, Some(var.name.to_string()), bx);

bx.dbg_var_addr(dbg_var, dbg_loc, base.llval, Size::ZERO, &[], fragment);
}
}
}
Expand Down
27 changes: 9 additions & 18 deletions compiler/rustc_codegen_ssa/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::traits::*;
use rustc_index::bit_set::BitSet;
use rustc_index::IndexVec;
use rustc_middle::mir;
use rustc_middle::mir::interpret::ErrorHandled;
use rustc_middle::mir::traversal;
use rustc_middle::mir::UnwindTerminateReason;
use rustc_middle::ty::layout::{FnAbiOf, HasTyCtxt, TyAndLayout};
Expand Down Expand Up @@ -212,25 +211,17 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(

fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info(&mut start_bx);

// Evaluate all required consts; codegen later assumes that CTFE will never fail.
let mut all_consts_ok = true;
for const_ in &mir.required_consts {
if let Err(err) = fx.eval_mir_constant(const_) {
all_consts_ok = false;
match err {
// errored or at least linted
ErrorHandled::Reported(_) => {}
ErrorHandled::TooGeneric => {
span_bug!(const_.span, "codegen encountered polymorphic constant: {:?}", err)
}
}
// Rust post-monomorphization checks; we later rely on them.
match mir.post_mono_checks(cx.tcx(), ty::ParamEnv::reveal_all(), |c| Ok(fx.monomorphize(c))) {
Ok(()) => {}
Err(err) => {
err.emit_err(cx.tcx());
// This IR shouldn't ever be emitted, but let's try to guard against any of this code
// ever running.
start_bx.abort();
return;
}
}
if !all_consts_ok {
// We leave the IR in some half-built state here, and rely on this code not even being
// submitted to LLVM once an error was raised.
return;
}

let memory_locals = analyze::non_ssa_locals(&fx);

Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,12 +569,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
self.codegen_consume(bx, place.as_ref())
}

mir::Operand::Constant(ref constant) => {
// This cannot fail because we checked all required_consts in advance.
self.eval_mir_constant_to_operand(bx, constant).unwrap_or_else(|_err| {
span_bug!(constant.span, "erroneous constant not captured by required_consts")
})
}
mir::Operand::Constant(ref constant) => self.eval_mir_constant_to_operand(bx, constant),
}
}
}
3 changes: 0 additions & 3 deletions compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,6 @@ const_eval_dyn_call_vtable_mismatch =
const_eval_dyn_star_call_vtable_mismatch =
`dyn*` call on a pointer whose vtable does not match its type
const_eval_erroneous_constant =
erroneous constant used
const_eval_error = {$error_kind ->
[static] could not evaluate static initializer
[const] evaluation of constant value failed
Expand Down
Loading

0 comments on commit 34093c9

Please sign in to comment.