Skip to content

Commit 012c415

Browse files
committed
Auto merge of rust-lang#129287 - saethlin:mono-reachable-binops, r=<try>
Let MonoReachable traversal evaluate BinOps This fixes the first bug in rust-lang#129282 I don't know what the right organization is for the code here, but something probably has to get rearranged because this traversal now depends on `rustc_const_eval` so that it can use an `InterpCx` internally, so it can't live in `rustc_middle`. r? `@ghost`
2 parents 4d5b3b1 + 10b7e0e commit 012c415

File tree

11 files changed

+210
-166
lines changed

11 files changed

+210
-166
lines changed

Cargo.lock

+2
Original file line numberDiff line numberDiff line change
@@ -3479,6 +3479,7 @@ dependencies = [
34793479
"rustc_macros",
34803480
"rustc_metadata",
34813481
"rustc_middle",
3482+
"rustc_mir_transform",
34823483
"rustc_monomorphize",
34833484
"rustc_query_system",
34843485
"rustc_serialize",
@@ -4147,6 +4148,7 @@ dependencies = [
41474148
"rustc_hir",
41484149
"rustc_macros",
41494150
"rustc_middle",
4151+
"rustc_mir_transform",
41504152
"rustc_session",
41514153
"rustc_span",
41524154
"rustc_target",

compiler/rustc_codegen_cranelift/src/base.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,8 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
299299
.generic_activity("codegen prelude")
300300
.run(|| crate::abi::codegen_fn_prelude(fx, start_block));
301301

302-
let reachable_blocks = traversal::mono_reachable_as_bitset(fx.mir, fx.tcx, fx.instance);
302+
let reachable_blocks =
303+
rustc_mir_transform::mono_reachable_as_bitset(fx.mir, fx.tcx, fx.instance);
303304

304305
for (bb, bb_data) in fx.mir.basic_blocks.iter_enumerated() {
305306
let block = fx.get_block(bb);

compiler/rustc_codegen_cranelift/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ extern crate rustc_hir;
2424
extern crate rustc_incremental;
2525
extern crate rustc_index;
2626
extern crate rustc_metadata;
27+
extern crate rustc_mir_transform;
2728
extern crate rustc_session;
2829
extern crate rustc_span;
2930
extern crate rustc_target;

compiler/rustc_codegen_ssa/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ rustc_index = { path = "../rustc_index" }
2727
rustc_macros = { path = "../rustc_macros" }
2828
rustc_metadata = { path = "../rustc_metadata" }
2929
rustc_middle = { path = "../rustc_middle" }
30+
rustc_mir_transform = { path = "../rustc_mir_transform" }
3031
rustc_monomorphize = { path = "../rustc_monomorphize" }
3132
rustc_query_system = { path = "../rustc_query_system" }
3233
rustc_serialize = { path = "../rustc_serialize" }

compiler/rustc_codegen_ssa/src/mir/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
277277
// So drop the builder of `start_llbb` to avoid having two at the same time.
278278
drop(start_bx);
279279

280-
let reachable_blocks = traversal::mono_reachable_as_bitset(mir, cx.tcx(), instance);
280+
let reachable_blocks = rustc_mir_transform::mono_reachable_as_bitset(mir, cx.tcx(), instance);
281281

282282
// Codegen the body of each block using reverse postorder
283283
for (bb, _) in traversal::reverse_postorder(mir) {

compiler/rustc_const_eval/src/lib.rs

+100
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,107 @@ pub fn provide(providers: &mut Providers) {
5151
};
5252
}
5353

54+
use rustc_middle::mir::{
55+
BasicBlockData, ConstOperand, NullOp, Operand, Rvalue, StatementKind, SwitchTargets,
56+
TerminatorKind,
57+
};
58+
use rustc_middle::ty::{Instance, TyCtxt};
59+
5460
/// `rustc_driver::main` installs a handler that will set this to `true` if
5561
/// the compiler has been sent a request to shut down, such as by a Ctrl-C.
5662
/// This static lives here because it is only read by the interpreter.
5763
pub static CTRL_C_RECEIVED: AtomicBool = AtomicBool::new(false);
64+
65+
/// If this basic block ends with a [`TerminatorKind::SwitchInt`] for which we can evaluate the
66+
/// dimscriminant in monomorphization, we return the discriminant bits and the
67+
/// [`SwitchTargets`], just so the caller doesn't also have to match on the terminator.
68+
pub fn try_const_mono_switchint<'a, 'tcx>(
69+
tcx: TyCtxt<'tcx>,
70+
instance: Instance<'tcx>,
71+
block: &'a BasicBlockData<'tcx>,
72+
) -> Option<(u128, &'a SwitchTargets)> {
73+
// There are two places here we need to evaluate a constant.
74+
let eval_mono_const = |constant: &ConstOperand<'tcx>| {
75+
let env = ty::ParamEnv::reveal_all();
76+
let mono_literal = instance.instantiate_mir_and_normalize_erasing_regions(
77+
tcx,
78+
env,
79+
crate::ty::EarlyBinder::bind(constant.const_),
80+
);
81+
mono_literal.try_eval_bits(tcx, env)
82+
};
83+
84+
let TerminatorKind::SwitchInt { discr, targets } = &block.terminator().kind else {
85+
return None;
86+
};
87+
88+
// If this is a SwitchInt(const _), then we can just evaluate the constant and return.
89+
let discr = match discr {
90+
Operand::Constant(constant) => {
91+
let bits = eval_mono_const(constant)?;
92+
return Some((bits, targets));
93+
}
94+
Operand::Move(place) | Operand::Copy(place) => place,
95+
};
96+
97+
// MIR for `if false` actually looks like this:
98+
// _1 = const _
99+
// SwitchInt(_1)
100+
//
101+
// And MIR for if intrinsics::ub_checks() looks like this:
102+
// _1 = UbChecks()
103+
// SwitchInt(_1)
104+
//
105+
// So we're going to try to recognize this pattern.
106+
//
107+
// If we have a SwitchInt on a non-const place, we find the most recent statement that
108+
// isn't a storage marker. If that statement is an assignment of a const to our
109+
// discriminant place, we evaluate and return the const, as if we've const-propagated it
110+
// into the SwitchInt.
111+
112+
let last_stmt = block.statements.iter().rev().find(|stmt| {
113+
!matches!(stmt.kind, StatementKind::StorageDead(_) | StatementKind::StorageLive(_))
114+
})?;
115+
116+
let (place, rvalue) = last_stmt.kind.as_assign()?;
117+
118+
if discr != place {
119+
return None;
120+
}
121+
122+
use rustc_span::DUMMY_SP;
123+
124+
use crate::const_eval::DummyMachine;
125+
use crate::interpret::InterpCx;
126+
match rvalue {
127+
Rvalue::NullaryOp(NullOp::UbChecks, _) => Some((tcx.sess.ub_checks() as u128, targets)),
128+
Rvalue::Use(Operand::Constant(constant)) => {
129+
let bits = eval_mono_const(constant)?;
130+
Some((bits, targets))
131+
}
132+
Rvalue::BinaryOp(binop, box (Operand::Constant(lhs), Operand::Constant(rhs))) => {
133+
let env = ty::ParamEnv::reveal_all();
134+
let lhs = instance.instantiate_mir_and_normalize_erasing_regions(
135+
tcx,
136+
env,
137+
crate::ty::EarlyBinder::bind(lhs.const_),
138+
);
139+
let ecx = InterpCx::new(tcx, DUMMY_SP, env, DummyMachine);
140+
let lhs = ecx.eval_mir_constant(&lhs, DUMMY_SP, None).ok()?;
141+
let lhs = ecx.read_immediate(&lhs).ok()?;
142+
143+
let rhs = instance.instantiate_mir_and_normalize_erasing_regions(
144+
tcx,
145+
env,
146+
crate::ty::EarlyBinder::bind(rhs.const_),
147+
);
148+
let rhs = ecx.eval_mir_constant(&rhs, DUMMY_SP, None).ok()?;
149+
let rhs = ecx.read_immediate(&rhs).ok()?;
150+
151+
let res = ecx.binary_op(*binop, &lhs, &rhs).ok()?;
152+
let res = res.to_scalar_int().unwrap().to_bits_unchecked();
153+
Some((res, targets))
154+
}
155+
_ => None,
156+
}
157+
}

compiler/rustc_middle/src/mir/mod.rs

+1-68
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ use crate::ty::fold::{FallibleTypeFolder, TypeFoldable};
4242
use crate::ty::print::{pretty_print_const, with_no_trimmed_paths, FmtPrinter, Printer};
4343
use crate::ty::visit::TypeVisitableExt;
4444
use crate::ty::{
45-
self, AdtDef, GenericArg, GenericArgsRef, Instance, InstanceKind, List, Ty, TyCtxt,
45+
self, AdtDef, GenericArg, GenericArgsRef, InstanceKind, List, Ty, TyCtxt,
4646
UserTypeAnnotationIndex,
4747
};
4848

@@ -687,73 +687,6 @@ impl<'tcx> Body<'tcx> {
687687
self.injection_phase.is_some()
688688
}
689689

690-
/// If this basic block ends with a [`TerminatorKind::SwitchInt`] for which we can evaluate the
691-
/// dimscriminant in monomorphization, we return the discriminant bits and the
692-
/// [`SwitchTargets`], just so the caller doesn't also have to match on the terminator.
693-
fn try_const_mono_switchint<'a>(
694-
tcx: TyCtxt<'tcx>,
695-
instance: Instance<'tcx>,
696-
block: &'a BasicBlockData<'tcx>,
697-
) -> Option<(u128, &'a SwitchTargets)> {
698-
// There are two places here we need to evaluate a constant.
699-
let eval_mono_const = |constant: &ConstOperand<'tcx>| {
700-
let env = ty::ParamEnv::reveal_all();
701-
let mono_literal = instance.instantiate_mir_and_normalize_erasing_regions(
702-
tcx,
703-
env,
704-
crate::ty::EarlyBinder::bind(constant.const_),
705-
);
706-
mono_literal.try_eval_bits(tcx, env)
707-
};
708-
709-
let TerminatorKind::SwitchInt { discr, targets } = &block.terminator().kind else {
710-
return None;
711-
};
712-
713-
// If this is a SwitchInt(const _), then we can just evaluate the constant and return.
714-
let discr = match discr {
715-
Operand::Constant(constant) => {
716-
let bits = eval_mono_const(constant)?;
717-
return Some((bits, targets));
718-
}
719-
Operand::Move(place) | Operand::Copy(place) => place,
720-
};
721-
722-
// MIR for `if false` actually looks like this:
723-
// _1 = const _
724-
// SwitchInt(_1)
725-
//
726-
// And MIR for if intrinsics::ub_checks() looks like this:
727-
// _1 = UbChecks()
728-
// SwitchInt(_1)
729-
//
730-
// So we're going to try to recognize this pattern.
731-
//
732-
// If we have a SwitchInt on a non-const place, we find the most recent statement that
733-
// isn't a storage marker. If that statement is an assignment of a const to our
734-
// discriminant place, we evaluate and return the const, as if we've const-propagated it
735-
// into the SwitchInt.
736-
737-
let last_stmt = block.statements.iter().rev().find(|stmt| {
738-
!matches!(stmt.kind, StatementKind::StorageDead(_) | StatementKind::StorageLive(_))
739-
})?;
740-
741-
let (place, rvalue) = last_stmt.kind.as_assign()?;
742-
743-
if discr != place {
744-
return None;
745-
}
746-
747-
match rvalue {
748-
Rvalue::NullaryOp(NullOp::UbChecks, _) => Some((tcx.sess.ub_checks() as u128, targets)),
749-
Rvalue::Use(Operand::Constant(constant)) => {
750-
let bits = eval_mono_const(constant)?;
751-
Some((bits, targets))
752-
}
753-
_ => None,
754-
}
755-
}
756-
757690
/// For a `Location` in this scope, determine what the "caller location" at that point is. This
758691
/// is interesting because of inlining: the `#[track_caller]` attribute of inlined functions
759692
/// must be honored. Falls back to the `tracked_caller` value for `#[track_caller]` functions,

compiler/rustc_middle/src/mir/traversal.rs

-94
Original file line numberDiff line numberDiff line change
@@ -279,97 +279,3 @@ pub fn reverse_postorder<'a, 'tcx>(
279279
{
280280
body.basic_blocks.reverse_postorder().iter().map(|&bb| (bb, &body.basic_blocks[bb]))
281281
}
282-
283-
/// Traversal of a [`Body`] that tries to avoid unreachable blocks in a monomorphized [`Instance`].
284-
///
285-
/// This is allowed to have false positives; blocks may be visited even if they are not actually
286-
/// reachable.
287-
///
288-
/// Such a traversal is mostly useful because it lets us skip lowering the `false` side
289-
/// of `if <T as Trait>::CONST`, as well as [`NullOp::UbChecks`].
290-
///
291-
/// [`NullOp::UbChecks`]: rustc_middle::mir::NullOp::UbChecks
292-
pub fn mono_reachable<'a, 'tcx>(
293-
body: &'a Body<'tcx>,
294-
tcx: TyCtxt<'tcx>,
295-
instance: Instance<'tcx>,
296-
) -> MonoReachable<'a, 'tcx> {
297-
MonoReachable::new(body, tcx, instance)
298-
}
299-
300-
/// [`MonoReachable`] internally accumulates a [`BitSet`] of visited blocks. This is just a
301-
/// convenience function to run that traversal then extract its set of reached blocks.
302-
pub fn mono_reachable_as_bitset<'a, 'tcx>(
303-
body: &'a Body<'tcx>,
304-
tcx: TyCtxt<'tcx>,
305-
instance: Instance<'tcx>,
306-
) -> BitSet<BasicBlock> {
307-
let mut iter = mono_reachable(body, tcx, instance);
308-
while let Some(_) = iter.next() {}
309-
iter.visited
310-
}
311-
312-
pub struct MonoReachable<'a, 'tcx> {
313-
body: &'a Body<'tcx>,
314-
tcx: TyCtxt<'tcx>,
315-
instance: Instance<'tcx>,
316-
visited: BitSet<BasicBlock>,
317-
// Other traversers track their worklist in a Vec. But we don't care about order, so we can
318-
// store ours in a BitSet and thus save allocations because BitSet has a small size
319-
// optimization.
320-
worklist: BitSet<BasicBlock>,
321-
}
322-
323-
impl<'a, 'tcx> MonoReachable<'a, 'tcx> {
324-
pub fn new(
325-
body: &'a Body<'tcx>,
326-
tcx: TyCtxt<'tcx>,
327-
instance: Instance<'tcx>,
328-
) -> MonoReachable<'a, 'tcx> {
329-
let mut worklist = BitSet::new_empty(body.basic_blocks.len());
330-
worklist.insert(START_BLOCK);
331-
MonoReachable {
332-
body,
333-
tcx,
334-
instance,
335-
visited: BitSet::new_empty(body.basic_blocks.len()),
336-
worklist,
337-
}
338-
}
339-
340-
fn add_work(&mut self, blocks: impl IntoIterator<Item = BasicBlock>) {
341-
for block in blocks.into_iter() {
342-
if !self.visited.contains(block) {
343-
self.worklist.insert(block);
344-
}
345-
}
346-
}
347-
}
348-
349-
impl<'a, 'tcx> Iterator for MonoReachable<'a, 'tcx> {
350-
type Item = (BasicBlock, &'a BasicBlockData<'tcx>);
351-
352-
fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> {
353-
while let Some(idx) = self.worklist.iter().next() {
354-
self.worklist.remove(idx);
355-
if !self.visited.insert(idx) {
356-
continue;
357-
}
358-
359-
let data = &self.body[idx];
360-
361-
if let Some((bits, targets)) =
362-
Body::try_const_mono_switchint(self.tcx, self.instance, data)
363-
{
364-
let target = targets.target_for_value(bits);
365-
self.add_work([target]);
366-
} else {
367-
self.add_work(data.terminator().successors());
368-
}
369-
370-
return Some((idx, data));
371-
}
372-
373-
None
374-
}
375-
}

0 commit comments

Comments
 (0)