Skip to content

Commit 72b2101

Browse files
committed
Auto merge of #112718 - oli-obk:SIMD-destructure_mir_const, 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?
2 parents be6e38c + 1c992c0 commit 72b2101

File tree

7 files changed

+106
-58
lines changed

7 files changed

+106
-58
lines changed

compiler/rustc_codegen_ssa/src/mir/block.rs

+1-7
Original file line numberDiff line numberDiff line change
@@ -870,13 +870,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
870870
// promotes any complex rvalues to constants.
871871
if i == 2 && intrinsic.as_str().starts_with("simd_shuffle") {
872872
if let mir::Operand::Constant(constant) = arg {
873-
let c = self.eval_mir_constant(constant);
874-
let (llval, ty) = self.simd_shuffle_indices(
875-
&bx,
876-
constant.span,
877-
self.monomorphize(constant.ty()),
878-
c,
879-
);
873+
let (llval, ty) = self.simd_shuffle_indices(&bx, constant);
880874
return OperandRef {
881875
val: Immediate(llval),
882876
layout: bx.layout_of(ty),

compiler/rustc_codegen_ssa/src/mir/constant.rs

+33-17
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use rustc_middle::mir;
55
use rustc_middle::mir::interpret::{ConstValue, ErrorHandled};
66
use rustc_middle::ty::layout::HasTyCtxt;
77
use rustc_middle::ty::{self, Ty};
8-
use rustc_span::source_map::Span;
98
use rustc_target::abi::Abi;
109

1110
use super::FunctionCx;
@@ -59,22 +58,40 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
5958
})
6059
}
6160

61+
/// This is a convenience helper for `simd_shuffle_indices`. It has the precondition
62+
/// that the given `constant` is an `ConstantKind::Unevaluated` and must be convertible to
63+
/// a `ValTree`. If you want a more general version of this, talk to `wg-const-eval` on zulip.
64+
pub fn eval_unevaluated_mir_constant_to_valtree(
65+
&self,
66+
constant: &mir::Constant<'tcx>,
67+
) -> Result<Option<ty::ValTree<'tcx>>, ErrorHandled> {
68+
let uv = match constant.literal {
69+
mir::ConstantKind::Unevaluated(uv, _) => uv.shrink(),
70+
other => span_bug!(constant.span, "{other:#?}"),
71+
};
72+
let uv = self.monomorphize(uv);
73+
self.cx.tcx().const_eval_resolve_for_typeck(
74+
ty::ParamEnv::reveal_all(),
75+
uv,
76+
Some(constant.span),
77+
)
78+
}
79+
6280
/// process constant containing SIMD shuffle indices
6381
pub fn simd_shuffle_indices(
6482
&mut self,
6583
bx: &Bx,
66-
span: Span,
67-
ty: Ty<'tcx>,
68-
constant: Result<ConstValue<'tcx>, ErrorHandled>,
84+
constant: &mir::Constant<'tcx>,
6985
) -> (Bx::Value, Ty<'tcx>) {
70-
constant
86+
let ty = self.monomorphize(constant.ty());
87+
let val = self
88+
.eval_unevaluated_mir_constant_to_valtree(constant)
89+
.ok()
90+
.flatten()
7191
.map(|val| {
7292
let field_ty = ty.builtin_index().unwrap();
73-
let c = mir::ConstantKind::from_value(val, ty);
74-
let values: Vec<_> = bx
75-
.tcx()
76-
.destructure_mir_constant(ty::ParamEnv::reveal_all(), c)
77-
.fields
93+
let values: Vec<_> = val
94+
.unwrap_branch()
7895
.iter()
7996
.map(|field| {
8097
if let Some(prim) = field.try_to_scalar() {
@@ -88,15 +105,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
88105
}
89106
})
90107
.collect();
91-
let llval = bx.const_struct(&values, false);
92-
(llval, c.ty())
108+
bx.const_struct(&values, false)
93109
})
94-
.unwrap_or_else(|_| {
95-
bx.tcx().sess.emit_err(errors::ShuffleIndicesEvaluation { span });
110+
.unwrap_or_else(|| {
111+
bx.tcx().sess.emit_err(errors::ShuffleIndicesEvaluation { span: constant.span });
96112
// We've errored, so we don't have to produce working code.
97-
let ty = self.monomorphize(ty);
98113
let llty = bx.backend_type(bx.layout_of(ty));
99-
(bx.const_undef(llty), ty)
100-
})
114+
bx.const_undef(llty)
115+
});
116+
(val, ty)
101117
}
102118
}

compiler/rustc_const_eval/src/const_eval/mod.rs

-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ pub(crate) fn try_destructure_mir_constant<'tcx>(
9292
param_env: ty::ParamEnv<'tcx>,
9393
val: mir::ConstantKind<'tcx>,
9494
) -> InterpResult<'tcx, mir::DestructuredConstant<'tcx>> {
95-
trace!("destructure_mir_constant: {:?}", val);
9695
let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, CanAccessStatics::No);
9796
let op = ecx.eval_mir_constant(&val, None, None)?;
9897

compiler/rustc_middle/src/mir/interpret/queries.rs

+9-17
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,15 @@ impl<'tcx> TyCtxt<'tcx> {
9595
// used generic parameters is a bug of evaluation, so checking for it
9696
// here does feel somewhat sensible.
9797
if !self.features().generic_const_exprs && ct.substs.has_non_region_param() {
98-
assert!(matches!(
99-
self.def_kind(ct.def),
100-
DefKind::InlineConst | DefKind::AnonConst
101-
));
102-
let mir_body = self.mir_for_ctfe(ct.def);
98+
let def_kind = self.def_kind(instance.def_id());
99+
assert!(
100+
matches!(
101+
def_kind,
102+
DefKind::InlineConst | DefKind::AnonConst | DefKind::AssocConst
103+
),
104+
"{cid:?} is {def_kind:?}",
105+
);
106+
let mir_body = self.mir_for_ctfe(instance.def_id());
103107
if mir_body.is_polymorphic {
104108
let Some(local_def_id) = ct.def.as_local() else { return };
105109
self.struct_span_lint_hir(
@@ -239,15 +243,3 @@ impl<'tcx> TyCtxtEnsure<'tcx> {
239243
self.eval_to_allocation_raw(param_env.and(gid))
240244
}
241245
}
242-
243-
impl<'tcx> TyCtxt<'tcx> {
244-
/// Destructure a mir constant ADT or array into its variant index and its field values.
245-
/// Panics if the destructuring fails, use `try_destructure_mir_constant` for fallible version.
246-
pub fn destructure_mir_constant(
247-
self,
248-
param_env: ty::ParamEnv<'tcx>,
249-
constant: mir::ConstantKind<'tcx>,
250-
) -> mir::DestructuredConstant<'tcx> {
251-
self.try_destructure_mir_constant(param_env.and(constant)).unwrap()
252-
}
253-
}

compiler/rustc_middle/src/mir/mod.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -2581,10 +2581,9 @@ pub struct UnevaluatedConst<'tcx> {
25812581
}
25822582

25832583
impl<'tcx> UnevaluatedConst<'tcx> {
2584-
// FIXME: probably should get rid of this method. It's also wrong to
2585-
// shrink and then later expand a promoted.
25862584
#[inline]
25872585
pub fn shrink(self) -> ty::UnevaluatedConst<'tcx> {
2586+
assert_eq!(self.promoted, None);
25882587
ty::UnevaluatedConst { def: self.def, substs: self.substs }
25892588
}
25902589
}

src/tools/clippy/clippy_lints/src/non_copy_const.rs

+50-14
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@ use rustc_hir::{
1515
};
1616
use rustc_hir_analysis::hir_ty_to_ty;
1717
use rustc_lint::{LateContext, LateLintPass, Lint};
18-
use rustc_middle::mir;
19-
use rustc_middle::mir::interpret::{ConstValue, ErrorHandled};
18+
use rustc_middle::mir::interpret::ErrorHandled;
2019
use rustc_middle::ty::adjustment::Adjust;
21-
use rustc_middle::ty::{self, Ty};
20+
use rustc_middle::ty::{self, Ty, TyCtxt};
2221
use rustc_session::{declare_lint_pass, declare_tool_lint};
2322
use rustc_span::{sym, InnerSpan, Span};
23+
use rustc_target::abi::VariantIdx;
24+
use rustc_middle::mir::interpret::EvalToValTreeResult;
25+
use rustc_middle::mir::interpret::GlobalId;
2426

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

142144
fn is_value_unfrozen_raw<'tcx>(
143145
cx: &LateContext<'tcx>,
144-
result: Result<ConstValue<'tcx>, ErrorHandled>,
146+
result: Result<Option<ty::ValTree<'tcx>>, ErrorHandled>,
145147
ty: Ty<'tcx>,
146148
) -> bool {
147-
fn inner<'tcx>(cx: &LateContext<'tcx>, val: mir::ConstantKind<'tcx>) -> bool {
148-
match val.ty().kind() {
149+
fn inner<'tcx>(cx: &LateContext<'tcx>, val: ty::ValTree<'tcx>, ty: Ty<'tcx>) -> bool {
150+
match *ty.kind() {
149151
// the fact that we have to dig into every structs to search enums
150152
// leads us to the point checking `UnsafeCell` directly is the only option.
151153
ty::Adt(ty_def, ..) if ty_def.is_unsafe_cell() => true,
152154
// As of 2022-09-08 miri doesn't track which union field is active so there's no safe way to check the
153155
// contained value.
154156
ty::Adt(def, ..) if def.is_union() => false,
155-
ty::Array(..) | ty::Adt(..) | ty::Tuple(..) => {
156-
let val = cx.tcx.destructure_mir_constant(cx.param_env, val);
157-
val.fields.iter().any(|field| inner(cx, *field))
157+
ty::Array(ty, _) => {
158+
val.unwrap_branch().iter().any(|field| inner(cx, *field, ty))
158159
},
160+
ty::Adt(def, _) if def.is_union() => false,
161+
ty::Adt(def, substs) if def.is_enum() => {
162+
let (&variant_index, fields) = val.unwrap_branch().split_first().unwrap();
163+
let variant_index =
164+
VariantIdx::from_u32(variant_index.unwrap_leaf().try_to_u32().ok().unwrap());
165+
fields.iter().copied().zip(
166+
def.variants()[variant_index]
167+
.fields
168+
.iter()
169+
.map(|field| field.ty(cx.tcx, substs))).any(|(field, ty)| inner(cx, field, ty))
170+
}
171+
ty::Adt(def, substs) => {
172+
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))
173+
}
174+
ty::Tuple(tys) => val.unwrap_branch().iter().zip(tys).any(|(field, ty)| inner(cx, *field, ty)),
159175
_ => false,
160176
}
161177
}
@@ -184,24 +200,44 @@ fn is_value_unfrozen_raw<'tcx>(
184200
// I chose this way because unfrozen enums as assoc consts are rare (or, hopefully, none).
185201
err == ErrorHandled::TooGeneric
186202
},
187-
|val| inner(cx, mir::ConstantKind::from_value(val, ty)),
203+
|val| val.map_or(true, |val| inner(cx, val, ty)),
188204
)
189205
}
190206

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

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

199-
let result = cx
200-
.tcx
201-
.const_eval_resolve(cx.param_env, mir::UnevaluatedConst::new(def_id, substs), None);
220+
let result = const_eval_resolve(cx.tcx, cx.param_env, ty::UnevaluatedConst::new(def_id, substs), None);
202221
is_value_unfrozen_raw(cx, result, ty)
203222
}
204223

224+
225+
pub fn const_eval_resolve<'tcx>(
226+
tcx: TyCtxt<'tcx>,
227+
param_env: ty::ParamEnv<'tcx>,
228+
ct: ty::UnevaluatedConst<'tcx>,
229+
span: Option<Span>,
230+
) -> EvalToValTreeResult<'tcx> {
231+
match ty::Instance::resolve(tcx, param_env, ct.def, ct.substs) {
232+
Ok(Some(instance)) => {
233+
let cid = GlobalId { instance, promoted: None };
234+
tcx.const_eval_global_id_for_typeck(param_env, cid, span)
235+
}
236+
Ok(None) => Err(ErrorHandled::TooGeneric),
237+
Err(err) => Err(ErrorHandled::Reported(err.into())),
238+
}
239+
}
240+
205241
#[derive(Copy, Clone)]
206242
enum Source {
207243
Item { item: Span },
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
error: a `const` item should never be interior mutable
2+
--> $DIR/ice-9445.rs:1:1
3+
|
4+
LL | const UNINIT: core::mem::MaybeUninit<core::cell::Cell<&'static ()>> = core::mem::MaybeUninit::uninit();
5+
| -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
| |
7+
| make this a static item (maybe with lazy_static)
8+
|
9+
= note: `-D clippy::declare-interior-mutable-const` implied by `-D warnings`
10+
11+
error: aborting due to previous error
12+

0 commit comments

Comments
 (0)