Skip to content

Make simd_shuffle_indices use valtrees #112718

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -863,13 +863,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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which code emits an error if this is not a branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same for adts.

.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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know enough of valtrees. Is there code which emits an error if this is not a branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler created a bad valtree if a valtree of array type is not a branch. So we don't report an error, but we never get a Some from eval_const_to_valtree

},
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