Skip to content

Commit

Permalink
Rollup merge of #77693 - bugadani:issue-59352, r=oli-obk
Browse files Browse the repository at this point in the history 
Add test for #59352

Issue #59352 reported an optimization regression with rustc 1.32.0+. That regression could be tracked to a change that caused a function to miss the size limit of llvm's inlining, which results in an unreachable panicing branch being generated.
Enabling mir inline solves the issue, but is currently only done for `mir-opt-level>=2`.

This PR adds a test that can serve as a regression test for #59352, if/when mir inlining gets mature enough for opt-level 1, or some other optimization can remove the panic.
  • Loading branch information
@GuillaumeGomez
GuillaumeGomez authored Jan 15, 2021
2 parents bc39d4d + a0c5857 commit b7a9d6a
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 0 deletions.
18 changes: 18 additions & 0 deletions src/test/codegen/issue-59352.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// This test is a mirror of mir-opt/issues/issue-59352.rs. The LLVM inliner doesn't inline
// `char::method::is_digit()` and `char::method::to_digit()`, probably because of their size.
//
// Currently, the MIR optimizer isn't capable of removing the unreachable panic in this test case.
// Once the optimizer can do that, mir-opt/issues/issue-59352.rs will need to be updated and this
// test case should be removed as it will become redundant.

// mir-opt-level=2 enables inlining and enables LLVM to optimize away the unreachable panic call.
// compile-flags: -O -Z mir-opt-level=2

#![crate_type = "rlib"]

// CHECK-LABEL: @num_to_digit
#[no_mangle]
pub fn num_to_digit(num: char) -> u32 {
// CHECK-NOT: panic
if num.is_digit(8) { num.to_digit(8).unwrap() } else { 0 }
}
19 changes: 19 additions & 0 deletions src/test/mir-opt/issues/issue-59352.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// This test is a mirror of codegen/issue-59352.rs.
// The LLVM inliner doesn't inline `char::method::is_digit()` and so it doesn't recognize this case
// as effectively `if x.is_some() { x.unwrap() } else { 0 }`.
//
// Currently, the MIR optimizer isn't capable of removing the unreachable panic in this test case.
// Once the optimizer can do that, this test case will need to be updated and codegen/issue-59352.rs
// removed.

// EMIT_MIR issue_59352.num_to_digit.PreCodegen.after.mir
// compile-flags: -Z mir-opt-level=2 -Z span_free_formats

pub fn num_to_digit(num: char) -> u32 {
// CHECK-NOT: panic
if num.is_digit(8) { num.to_digit(8).unwrap() } else { 0 }
}

pub fn main() {
num_to_digit('2');
}
102 changes: 102 additions & 0 deletions src/test/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.mir
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// MIR for `num_to_digit` after PreCodegen

fn num_to_digit(_1: char) -> u32 {
debug num => _1; // in scope 0 at $DIR/issue-59352.rs:12:21: 12:24
let mut _0: u32; // return place in scope 0 at $DIR/issue-59352.rs:12:35: 12:38
let mut _2: bool; // in scope 0 at $DIR/issue-59352.rs:14:8: 14:23
let mut _3: std::option::Option<u32>; // in scope 0 at $DIR/issue-59352.rs:14:26: 14:41
let mut _4: char; // in scope 0 at $DIR/issue-59352.rs:14:26: 14:29
let mut _5: u32; // in scope 0 at $DIR/issue-59352.rs:14:8: 14:23
let mut _10: isize; // in scope 0 at $DIR/issue-59352.rs:14:8: 14:23
scope 1 (inlined char::methods::<impl char>::is_digit) { // at $DIR/issue-59352.rs:14:8: 14:23
debug self => _8; // in scope 1 at $DIR/issue-59352.rs:14:8: 14:23
debug radix => _5; // in scope 1 at $DIR/issue-59352.rs:14:8: 14:23
let mut _6: &std::option::Option<u32>; // in scope 1 at $DIR/issue-59352.rs:14:8: 14:23
let _7: std::option::Option<u32>; // in scope 1 at $DIR/issue-59352.rs:14:8: 14:23
let mut _8: char; // in scope 1 at $DIR/issue-59352.rs:14:8: 14:23
scope 2 (inlined Option::<u32>::is_some) { // at $DIR/issue-59352.rs:14:8: 14:23
debug self => _6; // in scope 2 at $DIR/issue-59352.rs:14:8: 14:23
}
}
scope 3 (inlined #[track_caller] Option::<u32>::unwrap) { // at $DIR/issue-59352.rs:14:26: 14:50
debug self => _3; // in scope 3 at $DIR/issue-59352.rs:14:26: 14:50
let mut _9: isize; // in scope 3 at $DIR/issue-59352.rs:14:26: 14:50
scope 4 {
debug val => _0; // in scope 4 at $DIR/issue-59352.rs:14:26: 14:50
}
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/issue-59352.rs:14:8: 14:23
_8 = _1; // scope 0 at $DIR/issue-59352.rs:14:8: 14:11
StorageLive(_5); // scope 0 at $DIR/issue-59352.rs:14:8: 14:23
_5 = const 8_u32; // scope 0 at $DIR/issue-59352.rs:14:8: 14:23
StorageLive(_6); // scope 1 at $DIR/issue-59352.rs:14:8: 14:23
StorageLive(_7); // scope 1 at $DIR/issue-59352.rs:14:8: 14:23
_7 = char::methods::<impl char>::to_digit(move _8, const 8_u32) -> bb5; // scope 1 at $DIR/issue-59352.rs:14:8: 14:23
// mir::Constant
// + span: $DIR/issue-59352.rs:14:8: 14:23
// + literal: Const { ty: fn(char, u32) -> std::option::Option<u32> {std::char::methods::<impl char>::to_digit}, val: Value(Scalar(<ZST>)) }
}

bb1: {
StorageLive(_3); // scope 0 at $DIR/issue-59352.rs:14:26: 14:41
StorageLive(_4); // scope 0 at $DIR/issue-59352.rs:14:26: 14:29
_4 = _1; // scope 0 at $DIR/issue-59352.rs:14:26: 14:29
_3 = char::methods::<impl char>::to_digit(move _4, const 8_u32) -> bb3; // scope 0 at $DIR/issue-59352.rs:14:26: 14:41
// mir::Constant
// + span: $DIR/issue-59352.rs:14:30: 14:38
// + literal: Const { ty: fn(char, u32) -> std::option::Option<u32> {std::char::methods::<impl char>::to_digit}, val: Value(Scalar(<ZST>)) }
}

bb2: {
_0 = const 0_u32; // scope 0 at $DIR/issue-59352.rs:14:60: 14:61
goto -> bb4; // scope 0 at $DIR/issue-59352.rs:14:5: 14:63
}

bb3: {
StorageDead(_4); // scope 0 at $DIR/issue-59352.rs:14:40: 14:41
StorageLive(_9); // scope 0 at $DIR/issue-59352.rs:14:26: 14:50
_9 = discriminant(_3); // scope 3 at $DIR/issue-59352.rs:14:26: 14:50
switchInt(move _9) -> [0_isize: bb6, 1_isize: bb8, otherwise: bb7]; // scope 3 at $DIR/issue-59352.rs:14:26: 14:50
}

bb4: {
StorageDead(_2); // scope 0 at $DIR/issue-59352.rs:14:62: 14:63
return; // scope 0 at $DIR/issue-59352.rs:15:2: 15:2
}

bb5: {
_6 = &_7; // scope 1 at $DIR/issue-59352.rs:14:8: 14:23
_10 = discriminant((*_6)); // scope 2 at $DIR/issue-59352.rs:14:8: 14:23
_2 = Eq(_10, const 1_isize); // scope 2 at $DIR/issue-59352.rs:14:8: 14:23
StorageDead(_6); // scope 1 at $DIR/issue-59352.rs:14:8: 14:23
StorageDead(_7); // scope 1 at $DIR/issue-59352.rs:14:8: 14:23
StorageDead(_5); // scope 0 at $DIR/issue-59352.rs:14:8: 14:23
switchInt(move _2) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/issue-59352.rs:14:5: 14:63
}

bb6: {
core::panicking::panic(const "called `Option::unwrap()` on a `None` value"); // scope 3 at $DIR/issue-59352.rs:14:26: 14:50
// mir::Constant
// + span: $DIR/issue-59352.rs:14:26: 14:50
// + literal: Const { ty: fn(&'static str) -> ! {core::panicking::panic}, val: Value(Scalar(<ZST>)) }
// ty::Const
// + ty: &str
// + val: Value(Slice { data: Allocation { bytes: [99, 97, 108, 108, 101, 100, 32, 96, 79, 112, 116, 105, 111, 110, 58, 58, 117, 110, 119, 114, 97, 112, 40, 41, 96, 32, 111, 110, 32, 97, 32, 96, 78, 111, 110, 101, 96, 32, 118, 97, 108, 117, 101], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [8796093022207], len: Size { raw: 43 } }, size: Size { raw: 43 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 43 })
// mir::Constant
// + span: $DIR/issue-59352.rs:14:26: 14:50
// + literal: Const { ty: &str, val: Value(Slice { data: Allocation { bytes: [99, 97, 108, 108, 101, 100, 32, 96, 79, 112, 116, 105, 111, 110, 58, 58, 117, 110, 119, 114, 97, 112, 40, 41, 96, 32, 111, 110, 32, 97, 32, 96, 78, 111, 110, 101, 96, 32, 118, 97, 108, 117, 101], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [8796093022207], len: Size { raw: 43 } }, size: Size { raw: 43 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 43 }) }
}

bb7: {
unreachable; // scope 3 at $DIR/issue-59352.rs:14:26: 14:50
}

bb8: {
_0 = move ((_3 as Some).0: u32); // scope 3 at $DIR/issue-59352.rs:14:26: 14:50
StorageDead(_9); // scope 0 at $DIR/issue-59352.rs:14:26: 14:50
StorageDead(_3); // scope 0 at $DIR/issue-59352.rs:14:49: 14:50
goto -> bb4; // scope 0 at $DIR/issue-59352.rs:14:5: 14:63
}
}

0 comments on commit b7a9d6a

Please sign in to comment.