Skip to content

Commit

Permalink
Auto merge of #100759 - fee1-dead-contrib:const_eval_select_real_intr…
Browse files Browse the repository at this point in the history
…insic, r=oli-obk,RalfJung

Make `const_eval_select` a real intrinsic

This fixes issues where `track_caller` functions do not have nice panic
messages anymore when there is a call to the function, and uses the
MIR system to replace the call instead of dispatching via lang items.

Fixes #100696.
  • Loading branch information
bors committed Sep 5, 2022
2 parents e7cdd4c + 65b685e commit 9358d09
Show file tree
Hide file tree
Showing 27 changed files with 438 additions and 285 deletions.
3 changes: 1 addition & 2 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ use rustc_ast as ast;
use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
use rustc_hir::lang_items::LangItem;
use rustc_index::vec::Idx;
use rustc_middle::mir::AssertKind;
use rustc_middle::mir::{self, SwitchTargets};
use rustc_middle::mir::{self, AssertKind, SwitchTargets};
use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf};
use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths};
use rustc_middle::ty::{self, Instance, Ty, TypeVisitable};
Expand Down
16 changes: 1 addition & 15 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,7 @@ impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>> {
// All `#[rustc_do_not_const_check]` functions should be hooked here.
let def_id = instance.def_id();

if Some(def_id) == self.tcx.lang_items().const_eval_select() {
// redirect to const_eval_select_ct
if let Some(const_eval_select) = self.tcx.lang_items().const_eval_select_ct() {
return Ok(Some(
ty::Instance::resolve(
*self.tcx,
ty::ParamEnv::reveal_all(),
const_eval_select,
instance.substs,
)
.unwrap()
.unwrap(),
));
}
} else if Some(def_id) == self.tcx.lang_items().panic_display()
if Some(def_id) == self.tcx.lang_items().panic_display()
|| Some(def_id) == self.tcx.lang_items().begin_panic_fn()
{
// &str or &&str
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,6 @@ language_item_table! {
DropInPlace, sym::drop_in_place, drop_in_place_fn, Target::Fn, GenericRequirement::Minimum(1);
Oom, sym::oom, oom, Target::Fn, GenericRequirement::None;
AllocLayout, sym::alloc_layout, alloc_layout, Target::Struct, GenericRequirement::None;
ConstEvalSelect, sym::const_eval_select, const_eval_select, Target::Fn, GenericRequirement::Exact(4);
ConstConstEvalSelect, sym::const_eval_select_ct,const_eval_select_ct, Target::Fn, GenericRequirement::Exact(4);

Start, sym::start, start_fn, Target::Fn, GenericRequirement::Exact(1);

Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_middle/src/ty/consts/valtree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_macros::{HashStable, TyDecodable, TyEncodable};
/// `ValTree` does not have this problem with representation, as it only contains integers or
/// lists of (nested) `ValTree`.
pub enum ValTree<'tcx> {
/// ZSTs, integers, `bool`, `char` are represented as scalars.
/// integers, `bool`, `char` are represented as scalars.
/// See the `ScalarInt` documentation for how `ScalarInt` guarantees that equal values
/// of these types have the same representation.
Leaf(ScalarInt),
Expand All @@ -27,8 +27,11 @@ pub enum ValTree<'tcx> {
// dont use SliceOrStr for now
/// The fields of any kind of aggregate. Structs, tuples and arrays are represented by
/// listing their fields' values in order.
///
/// Enums are represented by storing their discriminant as a field, followed by all
/// the fields of the variant.
///
/// ZST types are represented as an empty slice.
Branch(&'tcx [ValTree<'tcx>]),
}

Expand Down
175 changes: 89 additions & 86 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ fn layout_of<'tcx>(
Ok(layout)
}

#[derive(Clone, Copy)]
pub struct LayoutCx<'tcx, C> {
pub tcx: C,
pub param_env: ty::ParamEnv<'tcx>,
Expand Down Expand Up @@ -3063,6 +3064,93 @@ fn fn_abi_of_instance<'tcx>(
)
}

// Handle safe Rust thin and fat pointers.
pub fn adjust_for_rust_scalar<'tcx>(
cx: LayoutCx<'tcx, TyCtxt<'tcx>>,
attrs: &mut ArgAttributes,
scalar: Scalar,
layout: TyAndLayout<'tcx>,
offset: Size,
is_return: bool,
) {
// Booleans are always a noundef i1 that needs to be zero-extended.
if scalar.is_bool() {
attrs.ext(ArgExtension::Zext);
attrs.set(ArgAttribute::NoUndef);
return;
}

// Scalars which have invalid values cannot be undef.
if !scalar.is_always_valid(&cx) {
attrs.set(ArgAttribute::NoUndef);
}

// Only pointer types handled below.
let Scalar::Initialized { value: Pointer, valid_range} = scalar else { return };

if !valid_range.contains(0) {
attrs.set(ArgAttribute::NonNull);
}

if let Some(pointee) = layout.pointee_info_at(&cx, offset) {
if let Some(kind) = pointee.safe {
attrs.pointee_align = Some(pointee.align);

// `Box` (`UniqueBorrowed`) are not necessarily dereferenceable
// for the entire duration of the function as they can be deallocated
// at any time. Same for shared mutable references. If LLVM had a
// way to say "dereferenceable on entry" we could use it here.
attrs.pointee_size = match kind {
PointerKind::UniqueBorrowed
| PointerKind::UniqueBorrowedPinned
| PointerKind::Frozen => pointee.size,
PointerKind::SharedMutable | PointerKind::UniqueOwned => Size::ZERO,
};

// `Box`, `&T`, and `&mut T` cannot be undef.
// Note that this only applies to the value of the pointer itself;
// this attribute doesn't make it UB for the pointed-to data to be undef.
attrs.set(ArgAttribute::NoUndef);

// The aliasing rules for `Box<T>` are still not decided, but currently we emit
// `noalias` for it. This can be turned off using an unstable flag.
// See https://github.com/rust-lang/unsafe-code-guidelines/issues/326
let noalias_for_box = cx.tcx.sess.opts.unstable_opts.box_noalias.unwrap_or(true);

// `&mut` pointer parameters never alias other parameters,
// or mutable global data
//
// `&T` where `T` contains no `UnsafeCell<U>` is immutable,
// and can be marked as both `readonly` and `noalias`, as
// LLVM's definition of `noalias` is based solely on memory
// dependencies rather than pointer equality
//
// Due to past miscompiles in LLVM, we apply a separate NoAliasMutRef attribute
// for UniqueBorrowed arguments, so that the codegen backend can decide whether
// or not to actually emit the attribute. It can also be controlled with the
// `-Zmutable-noalias` debugging option.
let no_alias = match kind {
PointerKind::SharedMutable
| PointerKind::UniqueBorrowed
| PointerKind::UniqueBorrowedPinned => false,
PointerKind::UniqueOwned => noalias_for_box,
PointerKind::Frozen => !is_return,
};
if no_alias {
attrs.set(ArgAttribute::NoAlias);
}

if kind == PointerKind::Frozen && !is_return {
attrs.set(ArgAttribute::ReadOnly);
}

if kind == PointerKind::UniqueBorrowed && !is_return {
attrs.set(ArgAttribute::NoAliasMutRef);
}
}
}
}

impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
// FIXME(eddyb) perhaps group the signature/type-containing (or all of them?)
// arguments of this method, into a separate `struct`.
Expand Down Expand Up @@ -3118,91 +3206,6 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
use SpecAbi::*;
let rust_abi = matches!(sig.abi, RustIntrinsic | PlatformIntrinsic | Rust | RustCall);

// Handle safe Rust thin and fat pointers.
let adjust_for_rust_scalar = |attrs: &mut ArgAttributes,
scalar: Scalar,
layout: TyAndLayout<'tcx>,
offset: Size,
is_return: bool| {
// Booleans are always a noundef i1 that needs to be zero-extended.
if scalar.is_bool() {
attrs.ext(ArgExtension::Zext);
attrs.set(ArgAttribute::NoUndef);
return;
}

// Scalars which have invalid values cannot be undef.
if !scalar.is_always_valid(self) {
attrs.set(ArgAttribute::NoUndef);
}

// Only pointer types handled below.
let Scalar::Initialized { value: Pointer, valid_range} = scalar else { return };

if !valid_range.contains(0) {
attrs.set(ArgAttribute::NonNull);
}

if let Some(pointee) = layout.pointee_info_at(self, offset) {
if let Some(kind) = pointee.safe {
attrs.pointee_align = Some(pointee.align);

// `Box` (`UniqueBorrowed`) are not necessarily dereferenceable
// for the entire duration of the function as they can be deallocated
// at any time. Same for shared mutable references. If LLVM had a
// way to say "dereferenceable on entry" we could use it here.
attrs.pointee_size = match kind {
PointerKind::UniqueBorrowed
| PointerKind::UniqueBorrowedPinned
| PointerKind::Frozen => pointee.size,
PointerKind::SharedMutable | PointerKind::UniqueOwned => Size::ZERO,
};

// `Box`, `&T`, and `&mut T` cannot be undef.
// Note that this only applies to the value of the pointer itself;
// this attribute doesn't make it UB for the pointed-to data to be undef.
attrs.set(ArgAttribute::NoUndef);

// The aliasing rules for `Box<T>` are still not decided, but currently we emit
// `noalias` for it. This can be turned off using an unstable flag.
// See https://github.com/rust-lang/unsafe-code-guidelines/issues/326
let noalias_for_box =
self.tcx().sess.opts.unstable_opts.box_noalias.unwrap_or(true);

// `&mut` pointer parameters never alias other parameters,
// or mutable global data
//
// `&T` where `T` contains no `UnsafeCell<U>` is immutable,
// and can be marked as both `readonly` and `noalias`, as
// LLVM's definition of `noalias` is based solely on memory
// dependencies rather than pointer equality
//
// Due to past miscompiles in LLVM, we apply a separate NoAliasMutRef attribute
// for UniqueBorrowed arguments, so that the codegen backend can decide whether
// or not to actually emit the attribute. It can also be controlled with the
// `-Zmutable-noalias` debugging option.
let no_alias = match kind {
PointerKind::SharedMutable
| PointerKind::UniqueBorrowed
| PointerKind::UniqueBorrowedPinned => false,
PointerKind::UniqueOwned => noalias_for_box,
PointerKind::Frozen => !is_return,
};
if no_alias {
attrs.set(ArgAttribute::NoAlias);
}

if kind == PointerKind::Frozen && !is_return {
attrs.set(ArgAttribute::ReadOnly);
}

if kind == PointerKind::UniqueBorrowed && !is_return {
attrs.set(ArgAttribute::NoAliasMutRef);
}
}
}
};

let arg_of = |ty: Ty<'tcx>, arg_idx: Option<usize>| -> Result<_, FnAbiError<'tcx>> {
let is_return = arg_idx.is_none();

Expand All @@ -3218,7 +3221,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {

let mut arg = ArgAbi::new(self, layout, |layout, scalar, offset| {
let mut attrs = ArgAttributes::new();
adjust_for_rust_scalar(&mut attrs, scalar, *layout, offset, is_return);
adjust_for_rust_scalar(*self, &mut attrs, scalar, *layout, offset, is_return);
attrs
});

Expand Down
71 changes: 68 additions & 3 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#![feature(trusted_step)]
#![feature(try_blocks)]
#![feature(yeet_expr)]
#![feature(if_let_guard)]
#![recursion_limit = "256"]

#[macro_use]
Expand All @@ -27,10 +28,13 @@ use rustc_hir::intravisit::{self, Visitor};
use rustc_index::vec::IndexVec;
use rustc_middle::mir::visit::Visitor as _;
use rustc_middle::mir::{
traversal, AnalysisPhase, Body, ConstQualifs, MirPass, MirPhase, Promoted, RuntimePhase,
traversal, AnalysisPhase, Body, ConstQualifs, Constant, LocalDecl, MirPass, MirPhase, Operand,
Place, ProjectionElem, Promoted, RuntimePhase, Rvalue, SourceInfo, Statement, StatementKind,
TerminatorKind,
};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{self, TyCtxt, TypeVisitable};
use rustc_span::sym;

#[macro_use]
mod pass_manager;
Expand Down Expand Up @@ -140,6 +144,64 @@ pub fn provide(providers: &mut Providers) {
};
}

fn remap_mir_for_const_eval_select<'tcx>(
tcx: TyCtxt<'tcx>,
mut body: Body<'tcx>,
context: hir::Constness,
) -> Body<'tcx> {
for bb in body.basic_blocks.as_mut().iter_mut() {
let terminator = bb.terminator.as_mut().expect("invalid terminator");
match terminator.kind {
TerminatorKind::Call {
func: Operand::Constant(box Constant { ref literal, .. }),
ref mut args,
destination,
target,
cleanup,
fn_span,
..
} if let ty::FnDef(def_id, _) = *literal.ty().kind()
&& tcx.item_name(def_id) == sym::const_eval_select
&& tcx.is_intrinsic(def_id) =>
{
let [tupled_args, called_in_const, called_at_rt]: [_; 3] = std::mem::take(args).try_into().unwrap();
let ty = tupled_args.ty(&body.local_decls, tcx);
let fields = ty.tuple_fields();
let num_args = fields.len();
let func = if context == hir::Constness::Const { called_in_const } else { called_at_rt };
let (method, place): (fn(Place<'tcx>) -> Operand<'tcx>, Place<'tcx>) = match tupled_args {
Operand::Constant(_) => {
// there is no good way of extracting a tuple arg from a constant (const generic stuff)
// so we just create a temporary and deconstruct that.
let local = body.local_decls.push(LocalDecl::new(ty, fn_span));
bb.statements.push(Statement {
source_info: SourceInfo::outermost(fn_span),
kind: StatementKind::Assign(Box::new((local.into(), Rvalue::Use(tupled_args.clone())))),
});
(Operand::Move, local.into())
}
Operand::Move(place) => (Operand::Move, place),
Operand::Copy(place) => (Operand::Copy, place),
};
let place_elems = place.projection;
let arguments = (0..num_args).map(|x| {
let mut place_elems = place_elems.to_vec();
place_elems.push(ProjectionElem::Field(x.into(), fields[x]));
let projection = tcx.intern_place_elems(&place_elems);
let place = Place {
local: place.local,
projection,
};
method(place)
}).collect();
terminator.kind = TerminatorKind::Call { func, args: arguments, destination, target, cleanup, from_hir_call: false, fn_span };
}
_ => {}
}
}
body
}

fn is_mir_available(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
let def_id = def_id.expect_local();
tcx.mir_keys(()).contains(&def_id)
Expand Down Expand Up @@ -325,7 +387,9 @@ fn inner_mir_for_ctfe(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -
.body_const_context(def.did)
.expect("mir_for_ctfe should not be used for runtime functions");

let mut body = tcx.mir_drops_elaborated_and_const_checked(def).borrow().clone();
let body = tcx.mir_drops_elaborated_and_const_checked(def).borrow().clone();

let mut body = remap_mir_for_const_eval_select(tcx, body, hir::Constness::Const);

match context {
// Do not const prop functions, either they get executed at runtime or exported to metadata,
Expand Down Expand Up @@ -558,8 +622,9 @@ fn inner_optimized_mir(tcx: TyCtxt<'_>, did: LocalDefId) -> Body<'_> {
Some(other) => panic!("do not use `optimized_mir` for constants: {:?}", other),
}
debug!("about to call mir_drops_elaborated...");
let mut body =
let body =
tcx.mir_drops_elaborated_and_const_checked(ty::WithOptConstParam::unknown(did)).steal();
let mut body = remap_mir_for_const_eval_select(tcx, body, hir::Constness::NotConst);
debug!("body: {:#?}", body);
run_optimization_passes(tcx, &mut body);

Expand Down
8 changes: 1 addition & 7 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,6 @@
//! method in operand position, we treat it as a neighbor of the current
//! mono item. Calls are just a special case of that.
//!
//! #### Closures
//! In a way, closures are a simple case. Since every closure object needs to be
//! constructed somewhere, we can reliably discover them by observing
//! `RValue::Aggregate` expressions with `AggregateKind::Closure`. This is also
//! true for closures inlined from other crates.
//!
//! #### Drop glue
//! Drop glue mono items are introduced by MIR drop-statements. The
//! generated mono item will again have drop-glue item neighbors if the
Expand Down Expand Up @@ -835,7 +829,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
mir::TerminatorKind::Call { ref func, .. } => {
let callee_ty = func.ty(self.body, tcx);
let callee_ty = self.monomorphize(callee_ty);
visit_fn_use(self.tcx, callee_ty, true, source, &mut self.output);
visit_fn_use(self.tcx, callee_ty, true, source, &mut self.output)
}
mir::TerminatorKind::Drop { ref place, .. }
| mir::TerminatorKind::DropAndReplace { ref place, .. } => {
Expand Down
Loading

0 comments on commit 9358d09

Please sign in to comment.