Skip to content

Commit

Permalink
Rollup merge of rust-lang#112718 - oli-obk:SIMD-destructure_mir_const…
Browse files Browse the repository at this point in the history
…, r=cjgillot

Make simd_shuffle_indices use valtrees

This removes the second-to-last user of the `destructure_mir_constant` query. So in a follow-up we can remove the query and just move the query provider function directly into pretty printing (which is the last user).

cc `@rust-lang/clippy` there's a small functional change, but I think it is correct?
  • Loading branch information
Dylan-DPC authored Jun 30, 2023
2 parents 56d507d + 1c992c0 commit 53aadc0
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 58 deletions.
8 changes: 1 addition & 7 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -870,13 +870,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// promotes any complex rvalues to constants.
if i == 2 && intrinsic.as_str().starts_with("simd_shuffle") {
if let mir::Operand::Constant(constant) = arg {
let c = self.eval_mir_constant(constant);
let (llval, ty) = self.simd_shuffle_indices(
&bx,
constant.span,
self.monomorphize(constant.ty()),
c,
);
let (llval, ty) = self.simd_shuffle_indices(&bx, constant);
return OperandRef {
val: Immediate(llval),
layout: bx.layout_of(ty),
Expand Down
50 changes: 33 additions & 17 deletions compiler/rustc_codegen_ssa/src/mir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use rustc_middle::mir;
use rustc_middle::mir::interpret::{ConstValue, ErrorHandled};
use rustc_middle::ty::layout::HasTyCtxt;
use rustc_middle::ty::{self, Ty};
use rustc_span::source_map::Span;
use rustc_target::abi::Abi;

use super::FunctionCx;
Expand Down Expand Up @@ -59,22 +58,40 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
})
}

/// 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.
pub fn eval_unevaluated_mir_constant_to_valtree(
&self,
constant: &mir::Constant<'tcx>,
) -> Result<Option<ty::ValTree<'tcx>>, ErrorHandled> {
let uv = match constant.literal {
mir::ConstantKind::Unevaluated(uv, _) => uv.shrink(),
other => span_bug!(constant.span, "{other:#?}"),
};
let uv = self.monomorphize(uv);
self.cx.tcx().const_eval_resolve_for_typeck(
ty::ParamEnv::reveal_all(),
uv,
Some(constant.span),
)
}

/// process constant containing SIMD shuffle indices
pub fn simd_shuffle_indices(
&mut self,
bx: &Bx,
span: Span,
ty: Ty<'tcx>,
constant: Result<ConstValue<'tcx>, ErrorHandled>,
constant: &mir::Constant<'tcx>,
) -> (Bx::Value, Ty<'tcx>) {
constant
let ty = self.monomorphize(constant.ty());
let val = self
.eval_unevaluated_mir_constant_to_valtree(constant)
.ok()
.flatten()
.map(|val| {
let field_ty = ty.builtin_index().unwrap();
let c = mir::ConstantKind::from_value(val, ty);
let values: Vec<_> = bx
.tcx()
.destructure_mir_constant(ty::ParamEnv::reveal_all(), c)
.fields
let values: Vec<_> = val
.unwrap_branch()
.iter()
.map(|field| {
if let Some(prim) = field.try_to_scalar() {
Expand All @@ -88,15 +105,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}
})
.collect();
let llval = bx.const_struct(&values, false);
(llval, c.ty())
bx.const_struct(&values, false)
})
.unwrap_or_else(|_| {
bx.tcx().sess.emit_err(errors::ShuffleIndicesEvaluation { span });
.unwrap_or_else(|| {
bx.tcx().sess.emit_err(errors::ShuffleIndicesEvaluation { span: constant.span });
// We've errored, so we don't have to produce working code.
let ty = self.monomorphize(ty);
let llty = bx.backend_type(bx.layout_of(ty));
(bx.const_undef(llty), ty)
})
bx.const_undef(llty)
});
(val, ty)
}
}
1 change: 0 additions & 1 deletion compiler/rustc_const_eval/src/const_eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ pub(crate) fn try_destructure_mir_constant<'tcx>(
param_env: ty::ParamEnv<'tcx>,
val: mir::ConstantKind<'tcx>,
) -> InterpResult<'tcx, mir::DestructuredConstant<'tcx>> {
trace!("destructure_mir_constant: {:?}", val);
let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, CanAccessStatics::No);
let op = ecx.eval_mir_constant(&val, None, None)?;

Expand Down
26 changes: 9 additions & 17 deletions compiler/rustc_middle/src/mir/interpret/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,15 @@ impl<'tcx> TyCtxt<'tcx> {
// used generic parameters is a bug of evaluation, so checking for it
// here does feel somewhat sensible.
if !self.features().generic_const_exprs && ct.substs.has_non_region_param() {
assert!(matches!(
self.def_kind(ct.def),
DefKind::InlineConst | DefKind::AnonConst
));
let mir_body = self.mir_for_ctfe(ct.def);
let def_kind = self.def_kind(instance.def_id());
assert!(
matches!(
def_kind,
DefKind::InlineConst | DefKind::AnonConst | DefKind::AssocConst
),
"{cid:?} is {def_kind:?}",
);
let mir_body = self.mir_for_ctfe(instance.def_id());
if mir_body.is_polymorphic {
let Some(local_def_id) = ct.def.as_local() else { return };
self.struct_span_lint_hir(
Expand Down Expand Up @@ -239,15 +243,3 @@ impl<'tcx> TyCtxtEnsure<'tcx> {
self.eval_to_allocation_raw(param_env.and(gid))
}
}

impl<'tcx> TyCtxt<'tcx> {
/// Destructure a mir constant ADT or array into its variant index and its field values.
/// Panics if the destructuring fails, use `try_destructure_mir_constant` for fallible version.
pub fn destructure_mir_constant(
self,
param_env: ty::ParamEnv<'tcx>,
constant: mir::ConstantKind<'tcx>,
) -> mir::DestructuredConstant<'tcx> {
self.try_destructure_mir_constant(param_env.and(constant)).unwrap()
}
}
3 changes: 1 addition & 2 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2581,10 +2581,9 @@ pub struct UnevaluatedConst<'tcx> {
}

impl<'tcx> UnevaluatedConst<'tcx> {
// FIXME: probably should get rid of this method. It's also wrong to
// shrink and then later expand a promoted.
#[inline]
pub fn shrink(self) -> ty::UnevaluatedConst<'tcx> {
assert_eq!(self.promoted, None);
ty::UnevaluatedConst { def: self.def, substs: self.substs }
}
}
Expand Down
64 changes: 50 additions & 14 deletions src/tools/clippy/clippy_lints/src/non_copy_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ use rustc_hir::{
};
use rustc_hir_analysis::hir_ty_to_ty;
use rustc_lint::{LateContext, LateLintPass, Lint};
use rustc_middle::mir;
use rustc_middle::mir::interpret::{ConstValue, ErrorHandled};
use rustc_middle::mir::interpret::ErrorHandled;
use rustc_middle::ty::adjustment::Adjust;
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{sym, InnerSpan, Span};
use rustc_target::abi::VariantIdx;
use rustc_middle::mir::interpret::EvalToValTreeResult;
use rustc_middle::mir::interpret::GlobalId;

// FIXME: this is a correctness problem but there's no suitable
// warn-by-default category.
Expand Down Expand Up @@ -141,21 +143,35 @@ fn is_unfrozen<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {

fn is_value_unfrozen_raw<'tcx>(
cx: &LateContext<'tcx>,
result: Result<ConstValue<'tcx>, ErrorHandled>,
result: Result<Option<ty::ValTree<'tcx>>, ErrorHandled>,
ty: Ty<'tcx>,
) -> bool {
fn inner<'tcx>(cx: &LateContext<'tcx>, val: mir::ConstantKind<'tcx>) -> bool {
match val.ty().kind() {
fn inner<'tcx>(cx: &LateContext<'tcx>, val: ty::ValTree<'tcx>, ty: Ty<'tcx>) -> bool {
match *ty.kind() {
// the fact that we have to dig into every structs to search enums
// leads us to the point checking `UnsafeCell` directly is the only option.
ty::Adt(ty_def, ..) if ty_def.is_unsafe_cell() => true,
// As of 2022-09-08 miri doesn't track which union field is active so there's no safe way to check the
// contained value.
ty::Adt(def, ..) if def.is_union() => false,
ty::Array(..) | ty::Adt(..) | ty::Tuple(..) => {
let val = cx.tcx.destructure_mir_constant(cx.param_env, val);
val.fields.iter().any(|field| inner(cx, *field))
ty::Array(ty, _) => {
val.unwrap_branch().iter().any(|field| inner(cx, *field, ty))
},
ty::Adt(def, _) if def.is_union() => false,
ty::Adt(def, substs) if def.is_enum() => {
let (&variant_index, fields) = val.unwrap_branch().split_first().unwrap();
let variant_index =
VariantIdx::from_u32(variant_index.unwrap_leaf().try_to_u32().ok().unwrap());
fields.iter().copied().zip(
def.variants()[variant_index]
.fields
.iter()
.map(|field| field.ty(cx.tcx, substs))).any(|(field, ty)| inner(cx, field, ty))
}
ty::Adt(def, substs) => {
val.unwrap_branch().iter().zip(def.non_enum_variant().fields.iter().map(|field| field.ty(cx.tcx, substs))).any(|(field, ty)| inner(cx, *field, ty))
}
ty::Tuple(tys) => val.unwrap_branch().iter().zip(tys).any(|(field, ty)| inner(cx, *field, ty)),
_ => false,
}
}
Expand Down Expand Up @@ -184,24 +200,44 @@ fn is_value_unfrozen_raw<'tcx>(
// I chose this way because unfrozen enums as assoc consts are rare (or, hopefully, none).
err == ErrorHandled::TooGeneric
},
|val| inner(cx, mir::ConstantKind::from_value(val, ty)),
|val| val.map_or(true, |val| inner(cx, val, ty)),
)
}

fn is_value_unfrozen_poly<'tcx>(cx: &LateContext<'tcx>, body_id: BodyId, ty: Ty<'tcx>) -> bool {
let result = cx.tcx.const_eval_poly(body_id.hir_id.owner.to_def_id());
let def_id = body_id.hir_id.owner.to_def_id();
let substs = ty::InternalSubsts::identity_for_item(cx.tcx, def_id);
let instance = ty::Instance::new(def_id, substs);
let cid = rustc_middle::mir::interpret::GlobalId { instance, promoted: None };
let param_env = cx.tcx.param_env(def_id).with_reveal_all_normalized(cx.tcx);
let result = cx.tcx.const_eval_global_id_for_typeck(param_env, cid, None);
is_value_unfrozen_raw(cx, result, ty)
}

fn is_value_unfrozen_expr<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId, def_id: DefId, ty: Ty<'tcx>) -> bool {
let substs = cx.typeck_results().node_substs(hir_id);

let result = cx
.tcx
.const_eval_resolve(cx.param_env, mir::UnevaluatedConst::new(def_id, substs), None);
let result = const_eval_resolve(cx.tcx, cx.param_env, ty::UnevaluatedConst::new(def_id, substs), None);
is_value_unfrozen_raw(cx, result, ty)
}


pub fn const_eval_resolve<'tcx>(
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
ct: ty::UnevaluatedConst<'tcx>,
span: Option<Span>,
) -> EvalToValTreeResult<'tcx> {
match ty::Instance::resolve(tcx, param_env, ct.def, ct.substs) {
Ok(Some(instance)) => {
let cid = GlobalId { instance, promoted: None };
tcx.const_eval_global_id_for_typeck(param_env, cid, span)
}
Ok(None) => Err(ErrorHandled::TooGeneric),
Err(err) => Err(ErrorHandled::Reported(err.into())),
}
}

#[derive(Copy, Clone)]
enum Source {
Item { item: Span },
Expand Down
12 changes: 12 additions & 0 deletions src/tools/clippy/tests/ui/crashes/ice-9445.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error: a `const` item should never be interior mutable
--> $DIR/ice-9445.rs:1:1
|
LL | const UNINIT: core::mem::MaybeUninit<core::cell::Cell<&'static ()>> = core::mem::MaybeUninit::uninit();
| -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| make this a static item (maybe with lazy_static)
|
= note: `-D clippy::declare-interior-mutable-const` implied by `-D warnings`

error: aborting due to previous error

0 comments on commit 53aadc0

Please sign in to comment.