Skip to content

Commit bbf60c8

Browse files
committed
Auto merge of #127324 - DianQK:match-br, r=cjgillot
Simplify match based on the cast result of `IntToInt` Continue to complete #124150. The condition in #120614 is wrong, e.g. `-1i8` cannot be converted to `255i16`. I've rethought the issue and simplified the conditional judgment for a more straightforward approach. The new approach is to check **if the case value after the `IntToInt` conversion equals the target value**. In different types, `IntToInt` uses different casting methods. This rule is as follows: - `i8`/`u8` to `i8`/`u8`: do nothing. - `i8` to `i16`/`u16`: sign extension. - `u8` to `i16`/`u16`: zero extension. - `i16`/`u16` to `i8`/`u8`: truncate to the target size. The previous error was a mix of zext and sext. r? mir-opt
2 parents edc4dc3 + 1f9d960 commit bbf60c8

File tree

39 files changed

+1936
-536
lines changed

39 files changed

+1936
-536
lines changed

Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -4346,6 +4346,7 @@ dependencies = [
43464346
"rustc_span",
43474347
"rustc_target",
43484348
"rustc_trait_selection",
4349+
"rustc_type_ir",
43494350
"smallvec",
43504351
"tracing",
43514352
]

compiler/rustc_mir_transform/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ rustc_session = { path = "../rustc_session" }
2525
rustc_span = { path = "../rustc_span" }
2626
rustc_target = { path = "../rustc_target" }
2727
rustc_trait_selection = { path = "../rustc_trait_selection" }
28+
rustc_type_ir = { path = "../rustc_type_ir" }
2829
smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
2930
tracing = "0.1"
3031
# tidy-alphabetical-end

compiler/rustc_mir_transform/src/match_branches.rs

+80-66
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ use std::iter;
33
use rustc_index::IndexSlice;
44
use rustc_middle::mir::patch::MirPatch;
55
use rustc_middle::mir::*;
6+
use rustc_middle::ty::layout::{IntegerExt, TyAndLayout};
67
use rustc_middle::ty::{ParamEnv, ScalarInt, Ty, TyCtxt};
7-
use rustc_target::abi::Size;
8+
use rustc_target::abi::Integer;
9+
use rustc_type_ir::TyKind::*;
810

911
use super::simplify::simplify_cfg;
1012

@@ -42,10 +44,7 @@ impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
4244
should_cleanup = true;
4345
continue;
4446
}
45-
// unsound: https://github.com/rust-lang/rust/issues/124150
46-
if tcx.sess.opts.unstable_opts.unsound_mir_opts
47-
&& SimplifyToExp::default().simplify(tcx, body, bb_idx, param_env).is_some()
48-
{
47+
if SimplifyToExp::default().simplify(tcx, body, bb_idx, param_env).is_some() {
4948
should_cleanup = true;
5049
continue;
5150
}
@@ -264,33 +263,56 @@ impl<'tcx> SimplifyMatch<'tcx> for SimplifyToIf {
264263
}
265264
}
266265

266+
/// Check if the cast constant using `IntToInt` is equal to the target constant.
267+
fn can_cast(
268+
tcx: TyCtxt<'_>,
269+
src_val: impl Into<u128>,
270+
src_layout: TyAndLayout<'_>,
271+
cast_ty: Ty<'_>,
272+
target_scalar: ScalarInt,
273+
) -> bool {
274+
let from_scalar = ScalarInt::try_from_uint(src_val.into(), src_layout.size).unwrap();
275+
let v = match src_layout.ty.kind() {
276+
Uint(_) => from_scalar.to_uint(src_layout.size),
277+
Int(_) => from_scalar.to_int(src_layout.size) as u128,
278+
_ => unreachable!("invalid int"),
279+
};
280+
let size = match *cast_ty.kind() {
281+
Int(t) => Integer::from_int_ty(&tcx, t).size(),
282+
Uint(t) => Integer::from_uint_ty(&tcx, t).size(),
283+
_ => unreachable!("invalid int"),
284+
};
285+
let v = size.truncate(v);
286+
let cast_scalar = ScalarInt::try_from_uint(v, size).unwrap();
287+
cast_scalar == target_scalar
288+
}
289+
267290
#[derive(Default)]
268291
struct SimplifyToExp {
269-
transfrom_types: Vec<TransfromType>,
292+
transfrom_kinds: Vec<TransfromKind>,
270293
}
271294

272295
#[derive(Clone, Copy)]
273-
enum CompareType<'tcx, 'a> {
296+
enum ExpectedTransformKind<'tcx, 'a> {
274297
/// Identical statements.
275298
Same(&'a StatementKind<'tcx>),
276299
/// Assignment statements have the same value.
277-
Eq(&'a Place<'tcx>, Ty<'tcx>, ScalarInt),
300+
SameByEq { place: &'a Place<'tcx>, ty: Ty<'tcx>, scalar: ScalarInt },
278301
/// Enum variant comparison type.
279-
Discr { place: &'a Place<'tcx>, ty: Ty<'tcx>, is_signed: bool },
302+
Cast { place: &'a Place<'tcx>, ty: Ty<'tcx> },
280303
}
281304

282-
enum TransfromType {
305+
enum TransfromKind {
283306
Same,
284-
Eq,
285-
Discr,
307+
Cast,
286308
}
287309

288-
impl From<CompareType<'_, '_>> for TransfromType {
289-
fn from(compare_type: CompareType<'_, '_>) -> Self {
310+
impl From<ExpectedTransformKind<'_, '_>> for TransfromKind {
311+
fn from(compare_type: ExpectedTransformKind<'_, '_>) -> Self {
290312
match compare_type {
291-
CompareType::Same(_) => TransfromType::Same,
292-
CompareType::Eq(_, _, _) => TransfromType::Eq,
293-
CompareType::Discr { .. } => TransfromType::Discr,
313+
ExpectedTransformKind::Same(_) => TransfromKind::Same,
314+
ExpectedTransformKind::SameByEq { .. } => TransfromKind::Same,
315+
ExpectedTransformKind::Cast { .. } => TransfromKind::Cast,
294316
}
295317
}
296318
}
@@ -354,7 +376,7 @@ impl<'tcx> SimplifyMatch<'tcx> for SimplifyToExp {
354376
return None;
355377
}
356378
let mut target_iter = targets.iter();
357-
let (first_val, first_target) = target_iter.next().unwrap();
379+
let (first_case_val, first_target) = target_iter.next().unwrap();
358380
let first_terminator_kind = &bbs[first_target].terminator().kind;
359381
// Check that destinations are identical, and if not, then don't optimize this block
360382
if !targets
@@ -364,24 +386,20 @@ impl<'tcx> SimplifyMatch<'tcx> for SimplifyToExp {
364386
return None;
365387
}
366388

367-
let discr_size = tcx.layout_of(param_env.and(discr_ty)).unwrap().size;
389+
let discr_layout = tcx.layout_of(param_env.and(discr_ty)).unwrap();
368390
let first_stmts = &bbs[first_target].statements;
369-
let (second_val, second_target) = target_iter.next().unwrap();
391+
let (second_case_val, second_target) = target_iter.next().unwrap();
370392
let second_stmts = &bbs[second_target].statements;
371393
if first_stmts.len() != second_stmts.len() {
372394
return None;
373395
}
374396

375-
fn int_equal(l: ScalarInt, r: impl Into<u128>, size: Size) -> bool {
376-
l.to_bits_unchecked() == ScalarInt::try_from_uint(r, size).unwrap().to_bits_unchecked()
377-
}
378-
379397
// We first compare the two branches, and then the other branches need to fulfill the same conditions.
380-
let mut compare_types = Vec::new();
398+
let mut expected_transform_kinds = Vec::new();
381399
for (f, s) in iter::zip(first_stmts, second_stmts) {
382400
let compare_type = match (&f.kind, &s.kind) {
383401
// If two statements are exactly the same, we can optimize.
384-
(f_s, s_s) if f_s == s_s => CompareType::Same(f_s),
402+
(f_s, s_s) if f_s == s_s => ExpectedTransformKind::Same(f_s),
385403

386404
// If two statements are assignments with the match values to the same place, we can optimize.
387405
(
@@ -395,22 +413,29 @@ impl<'tcx> SimplifyMatch<'tcx> for SimplifyToExp {
395413
f_c.const_.try_eval_scalar_int(tcx, param_env),
396414
s_c.const_.try_eval_scalar_int(tcx, param_env),
397415
) {
398-
(Some(f), Some(s)) if f == s => CompareType::Eq(lhs_f, f_c.const_.ty(), f),
399-
// Enum variants can also be simplified to an assignment statement if their values are equal.
400-
// We need to consider both unsigned and signed scenarios here.
416+
(Some(f), Some(s)) if f == s => ExpectedTransformKind::SameByEq {
417+
place: lhs_f,
418+
ty: f_c.const_.ty(),
419+
scalar: f,
420+
},
421+
// Enum variants can also be simplified to an assignment statement,
422+
// if we can use `IntToInt` cast to get an equal value.
401423
(Some(f), Some(s))
402-
if ((f_c.const_.ty().is_signed() || discr_ty.is_signed())
403-
&& int_equal(f, first_val, discr_size)
404-
&& int_equal(s, second_val, discr_size))
405-
|| (Some(f) == ScalarInt::try_from_uint(first_val, f.size())
406-
&& Some(s)
407-
== ScalarInt::try_from_uint(second_val, s.size())) =>
424+
if (can_cast(
425+
tcx,
426+
first_case_val,
427+
discr_layout,
428+
f_c.const_.ty(),
429+
f,
430+
) && can_cast(
431+
tcx,
432+
second_case_val,
433+
discr_layout,
434+
f_c.const_.ty(),
435+
s,
436+
)) =>
408437
{
409-
CompareType::Discr {
410-
place: lhs_f,
411-
ty: f_c.const_.ty(),
412-
is_signed: f_c.const_.ty().is_signed() || discr_ty.is_signed(),
413-
}
438+
ExpectedTransformKind::Cast { place: lhs_f, ty: f_c.const_.ty() }
414439
}
415440
_ => {
416441
return None;
@@ -421,47 +446,36 @@ impl<'tcx> SimplifyMatch<'tcx> for SimplifyToExp {
421446
// Otherwise we cannot optimize. Try another block.
422447
_ => return None,
423448
};
424-
compare_types.push(compare_type);
449+
expected_transform_kinds.push(compare_type);
425450
}
426451

427452
// All remaining BBs need to fulfill the same pattern as the two BBs from the previous step.
428453
for (other_val, other_target) in target_iter {
429454
let other_stmts = &bbs[other_target].statements;
430-
if compare_types.len() != other_stmts.len() {
455+
if expected_transform_kinds.len() != other_stmts.len() {
431456
return None;
432457
}
433-
for (f, s) in iter::zip(&compare_types, other_stmts) {
458+
for (f, s) in iter::zip(&expected_transform_kinds, other_stmts) {
434459
match (*f, &s.kind) {
435-
(CompareType::Same(f_s), s_s) if f_s == s_s => {}
460+
(ExpectedTransformKind::Same(f_s), s_s) if f_s == s_s => {}
436461
(
437-
CompareType::Eq(lhs_f, f_ty, val),
462+
ExpectedTransformKind::SameByEq { place: lhs_f, ty: f_ty, scalar },
438463
StatementKind::Assign(box (lhs_s, Rvalue::Use(Operand::Constant(s_c)))),
439464
) if lhs_f == lhs_s
440465
&& s_c.const_.ty() == f_ty
441-
&& s_c.const_.try_eval_scalar_int(tcx, param_env) == Some(val) => {}
466+
&& s_c.const_.try_eval_scalar_int(tcx, param_env) == Some(scalar) => {}
442467
(
443-
CompareType::Discr { place: lhs_f, ty: f_ty, is_signed },
468+
ExpectedTransformKind::Cast { place: lhs_f, ty: f_ty },
444469
StatementKind::Assign(box (lhs_s, Rvalue::Use(Operand::Constant(s_c)))),
445-
) if lhs_f == lhs_s && s_c.const_.ty() == f_ty => {
446-
let Some(f) = s_c.const_.try_eval_scalar_int(tcx, param_env) else {
447-
return None;
448-
};
449-
if is_signed
450-
&& s_c.const_.ty().is_signed()
451-
&& int_equal(f, other_val, discr_size)
452-
{
453-
continue;
454-
}
455-
if Some(f) == ScalarInt::try_from_uint(other_val, f.size()) {
456-
continue;
457-
}
458-
return None;
459-
}
470+
) if let Some(f) = s_c.const_.try_eval_scalar_int(tcx, param_env)
471+
&& lhs_f == lhs_s
472+
&& s_c.const_.ty() == f_ty
473+
&& can_cast(tcx, other_val, discr_layout, f_ty, f) => {}
460474
_ => return None,
461475
}
462476
}
463477
}
464-
self.transfrom_types = compare_types.into_iter().map(|c| c.into()).collect();
478+
self.transfrom_kinds = expected_transform_kinds.into_iter().map(|c| c.into()).collect();
465479
Some(())
466480
}
467481

@@ -479,13 +493,13 @@ impl<'tcx> SimplifyMatch<'tcx> for SimplifyToExp {
479493
let (_, first) = targets.iter().next().unwrap();
480494
let first = &bbs[first];
481495

482-
for (t, s) in iter::zip(&self.transfrom_types, &first.statements) {
496+
for (t, s) in iter::zip(&self.transfrom_kinds, &first.statements) {
483497
match (t, &s.kind) {
484-
(TransfromType::Same, _) | (TransfromType::Eq, _) => {
498+
(TransfromKind::Same, _) => {
485499
patch.add_statement(parent_end, s.kind.clone());
486500
}
487501
(
488-
TransfromType::Discr,
502+
TransfromKind::Cast,
489503
StatementKind::Assign(box (lhs, Rvalue::Use(Operand::Constant(f_c)))),
490504
) => {
491505
let operand = Operand::Copy(Place::from(discr_local));

tests/mir-opt/matches_reduce_branches.match_i128_u128.MatchBranchSimplification.diff

+33-28
Original file line numberDiff line numberDiff line change
@@ -5,37 +5,42 @@
55
debug i => _1;
66
let mut _0: u128;
77
let mut _2: i128;
8+
+ let mut _3: i128;
89

910
bb0: {
1011
_2 = discriminant(_1);
11-
switchInt(move _2) -> [1: bb5, 2: bb4, 3: bb3, 340282366920938463463374607431768211455: bb2, otherwise: bb1];
12-
}
13-
14-
bb1: {
15-
unreachable;
16-
}
17-
18-
bb2: {
19-
_0 = const core::num::<impl u128>::MAX;
20-
goto -> bb6;
21-
}
22-
23-
bb3: {
24-
_0 = const 3_u128;
25-
goto -> bb6;
26-
}
27-
28-
bb4: {
29-
_0 = const 2_u128;
30-
goto -> bb6;
31-
}
32-
33-
bb5: {
34-
_0 = const 1_u128;
35-
goto -> bb6;
36-
}
37-
38-
bb6: {
12+
- switchInt(move _2) -> [1: bb5, 2: bb4, 3: bb3, 340282366920938463463374607431768211455: bb2, otherwise: bb1];
13+
- }
14+
-
15+
- bb1: {
16+
- unreachable;
17+
- }
18+
-
19+
- bb2: {
20+
- _0 = const core::num::<impl u128>::MAX;
21+
- goto -> bb6;
22+
- }
23+
-
24+
- bb3: {
25+
- _0 = const 3_u128;
26+
- goto -> bb6;
27+
- }
28+
-
29+
- bb4: {
30+
- _0 = const 2_u128;
31+
- goto -> bb6;
32+
- }
33+
-
34+
- bb5: {
35+
- _0 = const 1_u128;
36+
- goto -> bb6;
37+
- }
38+
-
39+
- bb6: {
40+
+ StorageLive(_3);
41+
+ _3 = move _2;
42+
+ _0 = _3 as u128 (IntToInt);
43+
+ StorageDead(_3);
3944
return;
4045
}
4146
}

tests/mir-opt/matches_reduce_branches.match_i16_i8.MatchBranchSimplification.diff

-37
This file was deleted.

0 commit comments

Comments
 (0)