Skip to content

Commit c2f9278

Browse files
Lireeroli-obk
Lireer
authored and
oli-obk
committed
remove optimizations from const_prop_lint
1 parent 5e4ff26 commit c2f9278

File tree

2 files changed

+21
-281
lines changed

2 files changed

+21
-281
lines changed

‎compiler/rustc_mir_transform/src/const_prop_lint.rs

Lines changed: 20 additions & 280 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@ use rustc_hir::def::DefKind;
99
use rustc_hir::HirId;
1010
use rustc_index::bit_set::BitSet;
1111
use rustc_index::vec::IndexVec;
12-
use rustc_middle::mir::visit::{
13-
MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor,
14-
};
12+
use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
1513
use rustc_middle::mir::{
1614
AssertKind, BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalDecl, LocalKind,
1715
Location, Operand, Place, Rvalue, SourceInfo, SourceScope, SourceScopeData, Statement,
@@ -28,12 +26,12 @@ use rustc_target::abi::{HasDataLayout, Size, TargetDataLayout};
2826
use rustc_target::spec::abi::Abi;
2927
use rustc_trait_selection::traits;
3028

31-
use crate::MirPass;
29+
use crate::MirLint;
3230
use rustc_const_eval::const_eval::ConstEvalErr;
3331
use rustc_const_eval::interpret::{
34-
self, compile_time_machine, AllocId, ConstAllocation, ConstValue, CtfeValidationMode, Frame,
35-
ImmTy, Immediate, InterpCx, InterpResult, LocalState, LocalValue, MemPlace, MemoryKind, OpTy,
36-
Operand as InterpOperand, PlaceTy, Scalar, ScalarMaybeUninit, StackPopCleanup, StackPopUnwind,
32+
self, compile_time_machine, AllocId, ConstAllocation, Frame, ImmTy, InterpCx, InterpResult,
33+
LocalState, LocalValue, MemPlace, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy, Scalar,
34+
ScalarMaybeUninit, StackPopCleanup, StackPopUnwind,
3735
};
3836

3937
/// The maximum number of bytes that we'll allocate space for a local or the return value.
@@ -61,15 +59,8 @@ macro_rules! throw_machine_stop_str {
6159

6260
pub struct ConstProp;
6361

64-
impl<'tcx> MirPass<'tcx> for ConstProp {
65-
fn is_enabled(&self, _sess: &rustc_session::Session) -> bool {
66-
// FIXME(#70073): Unlike the other passes in "optimizations", this one emits errors, so it
67-
// runs even when MIR optimizations are disabled. We should separate the lint out from the
68-
// transform and move the lint as early in the pipeline as possible.
69-
true
70-
}
71-
72-
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
62+
impl<'tcx> MirLint<'tcx> for ConstProp {
63+
fn run_lint(&self, tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
7364
// will be evaluated by miri and produce its errors there
7465
if body.source.promoted.is_some() {
7566
return;
@@ -630,32 +621,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
630621
Some(())
631622
}
632623

633-
fn propagate_operand(&mut self, operand: &mut Operand<'tcx>) {
634-
match *operand {
635-
Operand::Copy(l) | Operand::Move(l) => {
636-
if let Some(value) = self.get_const(l) && self.should_const_prop(&value) {
637-
// FIXME(felix91gr): this code only handles `Scalar` cases.
638-
// For now, we're not handling `ScalarPair` cases because
639-
// doing so here would require a lot of code duplication.
640-
// We should hopefully generalize `Operand` handling into a fn,
641-
// and use it to do const-prop here and everywhere else
642-
// where it makes sense.
643-
if let interpret::Operand::Immediate(interpret::Immediate::Scalar(
644-
ScalarMaybeUninit::Scalar(scalar),
645-
)) = *value
646-
{
647-
*operand = self.operand_from_scalar(
648-
scalar,
649-
value.layout.ty,
650-
self.source_info.unwrap().span,
651-
);
652-
}
653-
}
654-
}
655-
Operand::Constant(_) => (),
656-
}
657-
}
658-
659624
fn const_prop(
660625
&mut self,
661626
rvalue: &Rvalue<'tcx>,
@@ -728,191 +693,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
728693
return None;
729694
}
730695

731-
if self.tcx.sess.mir_opt_level() >= 4 {
732-
self.eval_rvalue_with_identities(rvalue, place)
733-
} else {
734-
self.use_ecx(|this| this.ecx.eval_rvalue_into_place(rvalue, place))
735-
}
736-
}
737-
738-
// Attempt to use albegraic identities to eliminate constant expressions
739-
fn eval_rvalue_with_identities(
740-
&mut self,
741-
rvalue: &Rvalue<'tcx>,
742-
place: Place<'tcx>,
743-
) -> Option<()> {
744-
self.use_ecx(|this| match rvalue {
745-
Rvalue::BinaryOp(op, box (left, right))
746-
| Rvalue::CheckedBinaryOp(op, box (left, right)) => {
747-
let l = this.ecx.eval_operand(left, None);
748-
let r = this.ecx.eval_operand(right, None);
749-
750-
let const_arg = match (l, r) {
751-
(Ok(ref x), Err(_)) | (Err(_), Ok(ref x)) => this.ecx.read_immediate(x)?,
752-
(Err(e), Err(_)) => return Err(e),
753-
(Ok(_), Ok(_)) => return this.ecx.eval_rvalue_into_place(rvalue, place),
754-
};
755-
756-
let arg_value = const_arg.to_scalar()?.to_bits(const_arg.layout.size)?;
757-
let dest = this.ecx.eval_place(place)?;
758-
759-
match op {
760-
BinOp::BitAnd if arg_value == 0 => this.ecx.write_immediate(*const_arg, &dest),
761-
BinOp::BitOr
762-
if arg_value == const_arg.layout.size.truncate(u128::MAX)
763-
|| (const_arg.layout.ty.is_bool() && arg_value == 1) =>
764-
{
765-
this.ecx.write_immediate(*const_arg, &dest)
766-
}
767-
BinOp::Mul if const_arg.layout.ty.is_integral() && arg_value == 0 => {
768-
if let Rvalue::CheckedBinaryOp(_, _) = rvalue {
769-
let val = Immediate::ScalarPair(
770-
const_arg.to_scalar()?.into(),
771-
Scalar::from_bool(false).into(),
772-
);
773-
this.ecx.write_immediate(val, &dest)
774-
} else {
775-
this.ecx.write_immediate(*const_arg, &dest)
776-
}
777-
}
778-
_ => this.ecx.eval_rvalue_into_place(rvalue, place),
779-
}
780-
}
781-
_ => this.ecx.eval_rvalue_into_place(rvalue, place),
782-
})
783-
}
784-
785-
/// Creates a new `Operand::Constant` from a `Scalar` value
786-
fn operand_from_scalar(&self, scalar: Scalar, ty: Ty<'tcx>, span: Span) -> Operand<'tcx> {
787-
Operand::Constant(Box::new(Constant {
788-
span,
789-
user_ty: None,
790-
literal: ty::Const::from_scalar(self.tcx, scalar, ty).into(),
791-
}))
792-
}
793-
794-
fn replace_with_const(
795-
&mut self,
796-
rval: &mut Rvalue<'tcx>,
797-
value: &OpTy<'tcx>,
798-
source_info: SourceInfo,
799-
) {
800-
if let Rvalue::Use(Operand::Constant(c)) = rval {
801-
match c.literal {
802-
ConstantKind::Ty(c) if matches!(c.val(), ConstKind::Unevaluated(..)) => {}
803-
_ => {
804-
trace!("skipping replace of Rvalue::Use({:?} because it is already a const", c);
805-
return;
806-
}
807-
}
808-
}
809-
810-
trace!("attempting to replace {:?} with {:?}", rval, value);
811-
if let Err(e) = self.ecx.const_validate_operand(
812-
value,
813-
vec![],
814-
// FIXME: is ref tracking too expensive?
815-
// FIXME: what is the point of ref tracking if we do not even check the tracked refs?
816-
&mut interpret::RefTracking::empty(),
817-
CtfeValidationMode::Regular,
818-
) {
819-
trace!("validation error, attempt failed: {:?}", e);
820-
return;
821-
}
822-
823-
// FIXME> figure out what to do when try_read_immediate fails
824-
let imm = self.use_ecx(|this| this.ecx.try_read_immediate(value));
825-
826-
if let Some(Ok(imm)) = imm {
827-
match *imm {
828-
interpret::Immediate::Scalar(ScalarMaybeUninit::Scalar(scalar)) => {
829-
*rval = Rvalue::Use(self.operand_from_scalar(
830-
scalar,
831-
value.layout.ty,
832-
source_info.span,
833-
));
834-
}
835-
Immediate::ScalarPair(
836-
ScalarMaybeUninit::Scalar(_),
837-
ScalarMaybeUninit::Scalar(_),
838-
) => {
839-
// Found a value represented as a pair. For now only do const-prop if the type
840-
// of `rvalue` is also a tuple with two scalars.
841-
// FIXME: enable the general case stated above ^.
842-
let ty = value.layout.ty;
843-
// Only do it for tuples
844-
if let ty::Tuple(types) = ty.kind() {
845-
// Only do it if tuple is also a pair with two scalars
846-
if let [ty1, ty2] = types[..] {
847-
let alloc = self.use_ecx(|this| {
848-
let ty_is_scalar = |ty| {
849-
this.ecx.layout_of(ty).ok().map(|layout| layout.abi.is_scalar())
850-
== Some(true)
851-
};
852-
if ty_is_scalar(ty1) && ty_is_scalar(ty2) {
853-
let alloc = this
854-
.ecx
855-
.intern_with_temp_alloc(value.layout, |ecx, dest| {
856-
ecx.write_immediate(*imm, dest)
857-
})
858-
.unwrap();
859-
Ok(Some(alloc))
860-
} else {
861-
Ok(None)
862-
}
863-
});
864-
865-
if let Some(Some(alloc)) = alloc {
866-
// Assign entire constant in a single statement.
867-
// We can't use aggregates, as we run after the aggregate-lowering `MirPhase`.
868-
*rval = Rvalue::Use(Operand::Constant(Box::new(Constant {
869-
span: source_info.span,
870-
user_ty: None,
871-
literal: self
872-
.ecx
873-
.tcx
874-
.mk_const(ty::ConstS {
875-
ty,
876-
val: ty::ConstKind::Value(ConstValue::ByRef {
877-
alloc,
878-
offset: Size::ZERO,
879-
}),
880-
})
881-
.into(),
882-
})));
883-
}
884-
}
885-
}
886-
}
887-
// Scalars or scalar pairs that contain undef values are assumed to not have
888-
// successfully evaluated and are thus not propagated.
889-
_ => {}
890-
}
891-
}
892-
}
893-
894-
/// Returns `true` if and only if this `op` should be const-propagated into.
895-
fn should_const_prop(&mut self, op: &OpTy<'tcx>) -> bool {
896-
let mir_opt_level = self.tcx.sess.mir_opt_level();
897-
898-
if mir_opt_level == 0 {
899-
return false;
900-
}
901-
902-
if !self.tcx.consider_optimizing(|| format!("ConstantPropagation - OpTy: {:?}", op)) {
903-
return false;
904-
}
905-
906-
match **op {
907-
interpret::Operand::Immediate(Immediate::Scalar(ScalarMaybeUninit::Scalar(s))) => {
908-
s.try_to_int().is_ok()
909-
}
910-
interpret::Operand::Immediate(Immediate::ScalarPair(
911-
ScalarMaybeUninit::Scalar(l),
912-
ScalarMaybeUninit::Scalar(r),
913-
)) => l.try_to_int().is_ok() && r.try_to_int().is_ok(),
914-
_ => false,
915-
}
696+
self.use_ecx(|this| this.ecx.eval_rvalue_into_place(rvalue, place))
916697
}
917698
}
918699

@@ -1047,52 +828,30 @@ impl Visitor<'_> for CanConstProp {
1047828
}
1048829
}
1049830

1050-
impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
1051-
fn tcx(&self) -> TyCtxt<'tcx> {
1052-
self.tcx
1053-
}
1054-
1055-
fn visit_body(&mut self, body: &mut Body<'tcx>) {
1056-
for (bb, data) in body.basic_blocks_mut().iter_enumerated_mut() {
831+
impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> {
832+
fn visit_body(&mut self, body: &Body<'tcx>) {
833+
for (bb, data) in body.basic_blocks().iter_enumerated() {
1057834
self.visit_basic_block_data(bb, data);
1058835
}
1059836
}
1060837

1061-
fn visit_operand(&mut self, operand: &mut Operand<'tcx>, location: Location) {
838+
fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) {
1062839
self.super_operand(operand, location);
1063-
1064-
// Only const prop copies and moves on `mir_opt_level=3` as doing so
1065-
// currently slightly increases compile time in some cases.
1066-
if self.tcx.sess.mir_opt_level() >= 3 {
1067-
self.propagate_operand(operand)
1068-
}
1069840
}
1070841

1071-
fn visit_constant(&mut self, constant: &mut Constant<'tcx>, location: Location) {
842+
fn visit_constant(&mut self, constant: &Constant<'tcx>, location: Location) {
1072843
trace!("visit_constant: {:?}", constant);
1073844
self.super_constant(constant, location);
1074845
self.eval_constant(constant, self.source_info.unwrap());
1075846
}
1076847

1077-
fn visit_statement(&mut self, statement: &mut Statement<'tcx>, location: Location) {
848+
fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
1078849
trace!("visit_statement: {:?}", statement);
1079850
let source_info = statement.source_info;
1080851
self.source_info = Some(source_info);
1081-
if let StatementKind::Assign(box (place, ref mut rval)) = statement.kind {
852+
if let StatementKind::Assign(box (place, ref rval)) = statement.kind {
1082853
let can_const_prop = self.ecx.machine.can_const_prop[place.local];
1083854
if let Some(()) = self.const_prop(rval, source_info, place) {
1084-
// This will return None if the above `const_prop` invocation only "wrote" a
1085-
// type whose creation requires no write. E.g. a generator whose initial state
1086-
// consists solely of uninitialized memory (so it doesn't capture any locals).
1087-
if let Some(ref value) = self.get_const(place) && self.should_const_prop(value) {
1088-
trace!("replacing {:?} with {:?}", rval, value);
1089-
self.replace_with_const(rval, value, source_info);
1090-
if can_const_prop == ConstPropMode::FullConstProp
1091-
|| can_const_prop == ConstPropMode::OnlyInsideOwnBlock
1092-
{
1093-
trace!("propagated into {:?}", place);
1094-
}
1095-
}
1096855
match can_const_prop {
1097856
ConstPropMode::OnlyInsideOwnBlock => {
1098857
trace!(
@@ -1159,12 +918,12 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
1159918
self.super_statement(statement, location);
1160919
}
1161920

1162-
fn visit_terminator(&mut self, terminator: &mut Terminator<'tcx>, location: Location) {
921+
fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
1163922
let source_info = terminator.source_info;
1164923
self.source_info = Some(source_info);
1165924
self.super_terminator(terminator, location);
1166-
match &mut terminator.kind {
1167-
TerminatorKind::Assert { expected, ref msg, ref mut cond, .. } => {
925+
match &terminator.kind {
926+
TerminatorKind::Assert { expected, ref msg, ref cond, .. } => {
1168927
if let Some(ref value) = self.eval_operand(&cond, source_info) {
1169928
trace!("assertion on {:?} should be {:?}", value, expected);
1170929
let expected = ScalarMaybeUninit::from(Scalar::from_bool(*expected));
@@ -1231,25 +990,9 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
1231990
msg,
1232991
);
1233992
}
1234-
} else {
1235-
if self.should_const_prop(value) {
1236-
if let ScalarMaybeUninit::Scalar(scalar) = value_const {
1237-
*cond = self.operand_from_scalar(
1238-
scalar,
1239-
self.tcx.types.bool,
1240-
source_info.span,
1241-
);
1242-
}
1243-
}
1244993
}
1245994
}
1246995
}
1247-
TerminatorKind::SwitchInt { ref mut discr, .. } => {
1248-
// FIXME: This is currently redundant with `visit_operand`, but sadly
1249-
// always visiting operands currently causes a perf regression in LLVM codegen, so
1250-
// `visit_operand` currently only runs for propagates places for `mir_opt_level=4`.
1251-
self.propagate_operand(discr)
1252-
}
1253996
// None of these have Operands to const-propagate.
1254997
TerminatorKind::Goto { .. }
1255998
| TerminatorKind::Resume
@@ -1262,12 +1005,9 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
12621005
| TerminatorKind::GeneratorDrop
12631006
| TerminatorKind::FalseEdge { .. }
12641007
| TerminatorKind::FalseUnwind { .. }
1008+
| TerminatorKind::SwitchInt { .. }
1009+
| TerminatorKind::Call { .. }
12651010
| TerminatorKind::InlineAsm { .. } => {}
1266-
// Every argument in our function calls have already been propagated in `visit_operand`.
1267-
//
1268-
// NOTE: because LLVM codegen gives slight performance regressions with it, so this is
1269-
// gated on `mir_opt_level=3`.
1270-
TerminatorKind::Call { .. } => {}
12711011
}
12721012

12731013
// We remove all Locals which are restricted in propagation to their containing blocks and

‎compiler/rustc_mir_transform/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ fn run_post_borrowck_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tc
431431
// `Deaggregator` is conceptually part of MIR building, some backends rely on it happening
432432
// and it can help optimizations.
433433
&deaggregator::Deaggregator,
434-
&const_prop_lint::ConstProp,
434+
&Lint(const_prop_lint::ConstProp),
435435
];
436436

437437
pm::run_passes(tcx, body, post_borrowck_cleanup);

0 commit comments

Comments
 (0)