Skip to content

Commit

Permalink
Unrolled build for rust-lang#116277
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#116277 - RalfJung:post-mono, r=oli-obk

dont call mir.post_mono_checks in codegen

It seems like all tests are still passing when I remove this... let's see what CI says.
  • Loading branch information
rust-timer authored Oct 7, 2023
2 parents 8fdb0a9 + fa248cd commit 3f314fa
Show file tree
Hide file tree
Showing 11 changed files with 32 additions and 81 deletions.
11 changes: 0 additions & 11 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,17 +250,6 @@ pub(crate) fn verify_func(
}

fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
if let Err(err) =
fx.mir.post_mono_checks(fx.tcx, ty::ParamEnv::reveal_all(), |c| Ok(fx.monomorphize(c)))
{
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
.mir
.args_iter()
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ 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
// 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")
Expand Down
15 changes: 4 additions & 11 deletions compiler/rustc_codegen_ssa/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,18 +209,11 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
caller_location: None,
};

fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info(&mut start_bx);
// 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.

// Rust post-monomorphization checks; we later rely on them.
if let Err(err) =
mir.post_mono_checks(cx.tcx(), ty::ParamEnv::reveal_all(), |c| Ok(fx.monomorphize(c)))
{
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;
}
fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info(&mut start_bx);

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

Expand Down
29 changes: 18 additions & 11 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -750,12 +750,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

// Make sure all the constants required by this frame evaluate successfully (post-monomorphization check).
if M::POST_MONO_CHECKS {
// `ctfe_query` does some error message decoration that we want to be in effect here.
self.ctfe_query(None, |tcx| {
body.post_mono_checks(*tcx, self.param_env, |c| {
self.subst_from_current_frame_and_normalize_erasing_regions(c)
})
})?;
for &const_ in &body.required_consts {
let c =
self.subst_from_current_frame_and_normalize_erasing_regions(const_.const_)?;
c.eval(*self.tcx, self.param_env, Some(const_.span)).map_err(|err| {
err.emit_note(*self.tcx);
err
})?;
}
}

// done
Expand Down Expand Up @@ -1054,14 +1056,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Ok(())
}

/// Call a query that can return `ErrorHandled`. If `span` is `Some`, point to that span when an error occurs.
/// Call a query that can return `ErrorHandled`. Should be used for statics and other globals.
/// (`mir::Const`/`ty::Const` have `eval` methods that can be used directly instead.)
pub fn ctfe_query<T>(
&self,
span: Option<Span>,
query: impl FnOnce(TyCtxtAt<'tcx>) -> Result<T, ErrorHandled>,
) -> Result<T, ErrorHandled> {
// Use a precise span for better cycle errors.
query(self.tcx.at(span.unwrap_or_else(|| self.cur_span()))).map_err(|err| {
query(self.tcx.at(self.cur_span())).map_err(|err| {
err.emit_note(*self.tcx);
err
})
Expand All @@ -1082,7 +1084,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
} else {
self.param_env
};
let val = self.ctfe_query(None, |tcx| tcx.eval_to_allocation_raw(param_env.and(gid)))?;
let val = self.ctfe_query(|tcx| tcx.eval_to_allocation_raw(param_env.and(gid)))?;
self.raw_const_to_mplace(val)
}

Expand All @@ -1092,7 +1094,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
span: Option<Span>,
layout: Option<TyAndLayout<'tcx>>,
) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
let const_val = self.ctfe_query(span, |tcx| val.eval(*tcx, self.param_env, span))?;
let const_val = val.eval(*self.tcx, self.param_env, span).map_err(|err| {
// FIXME: somehow this is reachable even when POST_MONO_CHECKS is on.
// Are we not always populating `required_consts`?
err.emit_note(*self.tcx);
err
})?;
self.const_val_to_op(const_val, val.ty(), layout)
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
sym::type_name => Ty::new_static_str(self.tcx.tcx),
_ => bug!(),
};
let val = self.ctfe_query(None, |tcx| {
let val = self.ctfe_query(|tcx| {
tcx.const_eval_global_id(self.param_env, gid, Some(tcx.span))
})?;
let val = self.const_val_to_op(val, ty, Some(dest.layout))?;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

// We don't give a span -- statics don't need that, they cannot be generic or associated.
let val = self.ctfe_query(None, |tcx| tcx.eval_static_initializer(def_id))?;
let val = self.ctfe_query(|tcx| tcx.eval_static_initializer(def_id))?;
(val, Some(def_id))
}
};
Expand Down
15 changes: 0 additions & 15 deletions compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,6 @@ impl ErrorHandled {
}
}

pub fn emit_err(&self, tcx: TyCtxt<'_>) -> ErrorGuaranteed {
match self {
&ErrorHandled::Reported(err, span) => {
if !err.is_tainted_by_errors && !span.is_dummy() {
tcx.sess.emit_err(error::ErroneousConstant { span });
}
err.error
}
&ErrorHandled::TooGeneric(span) => tcx.sess.delay_span_bug(
span,
"encountered TooGeneric error when monomorphic data was expected",
),
}
}

pub fn emit_note(&self, tcx: TyCtxt<'_>) {
match self {
&ErrorHandled::Reported(err, span) => {
Expand Down
30 changes: 1 addition & 29 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//!
//! [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/mir/index.html
use crate::mir::interpret::{AllocRange, ConstAllocation, ErrorHandled, Scalar};
use crate::mir::interpret::{AllocRange, ConstAllocation, Scalar};
use crate::mir::visit::MirVisitable;
use crate::ty::codec::{TyDecoder, TyEncoder};
use crate::ty::fold::{FallibleTypeFolder, TypeFoldable};
Expand Down Expand Up @@ -568,34 +568,6 @@ impl<'tcx> Body<'tcx> {
pub fn is_custom_mir(&self) -> bool {
self.injection_phase.is_some()
}

/// *Must* be called once the full substitution for this body is known, to ensure that the body
/// is indeed fit for code generation or consumption more generally.
///
/// Sadly there's no nice way to represent an "arbitrary normalizer", so we take one for
/// constants specifically. (`Option<GenericArgsRef>` could be used for that, but the fact
/// that `Instance::args_for_mir_body` is private and instead instance exposes normalization
/// functions makes it seem like exposing the generic args is not the intended strategy.)
///
/// Also sadly, CTFE doesn't even know whether it runs on MIR that is already polymorphic or still monomorphic,
/// so we cannot just immediately ICE on TooGeneric.
///
/// Returns Ok(()) if everything went fine, and `Err` if a problem occurred and got reported.
pub fn post_mono_checks(
&self,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
normalize_const: impl Fn(Const<'tcx>) -> Result<Const<'tcx>, ErrorHandled>,
) -> Result<(), ErrorHandled> {
// For now, the only thing we have to check is is to ensure that all the constants used in
// the body successfully evaluate.
for &const_ in &self.required_consts {
let c = normalize_const(const_.const_)?;
c.eval(tcx, param_env, Some(const_.span))?;
}

Ok(())
}
}

#[derive(Copy, Clone, PartialEq, Eq, Debug, TyEncodable, TyDecodable, HashStable)]
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@ macro_rules! make_mir_visitor {

visit_place_fns!($($mutability)?);

/// This is called for every constant in the MIR body and every `required_consts`
/// (i.e., including consts that have been dead-code-eliminated).
fn visit_constant(
&mut self,
constant: & $($mutability)? ConstOperand<'tcx>,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1426,6 +1426,8 @@ fn collect_used_items<'tcx>(
);
}

// Here we rely on the visitor also visiting `required_consts`, so that we evaluate them
// and abort compilation if any of them errors.
MirUsedCollector {
tcx,
body: &body,
Expand Down
3 changes: 1 addition & 2 deletions src/tools/miri/src/concurrency/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,8 +803,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
if tcx.is_foreign_item(def_id) {
throw_unsup_format!("foreign thread-local statics are not supported");
}
// We don't give a span -- statics don't need that, they cannot be generic or associated.
let allocation = this.ctfe_query(None, |tcx| tcx.eval_static_initializer(def_id))?;
let allocation = this.ctfe_query(|tcx| tcx.eval_static_initializer(def_id))?;
let mut allocation = allocation.inner().clone();
// This allocation will be deallocated when the thread dies, so it is not in read-only memory.
allocation.mutability = Mutability::Mut;
Expand Down

0 comments on commit 3f314fa

Please sign in to comment.