rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.6k stars 12.74k forks source link

`Range<usize>::next` should fully MIR-inline #130590

Open scottmcm opened 1 month ago

scottmcm commented 1 month ago

If you compile this to optimized MIR:

// @compile-flags: -O -C debuginfo=0 --emit=mir -Z inline-mir-threshold=9999 -Z inline-mir-hint-threshold=9999
use std::ops::Range;
#[no_mangle]
pub fn demo(num: &mut Range<usize>) -> Option<usize> {
    num.next()
}

https://rust.godbolt.org/z/zsh6b6Y8n

You'll see that it still contains a call to forward_unchecked:

    bb1: {
        _3 = copy ((*_1).0: usize);
        StorageLive(_4);
        _4 = <usize as Step>::forward_unchecked(copy _3, const 1_usize) -> [return: bb2, unwind continue];
    }

That's pretty unfortunate, because forward_unchecked(a, 1) is just AddUnchecked(a, 1), a single MIR operator.

scottmcm commented 1 month ago

I tried adding #[inline] to the default step methods just in case that was relevant, but it looks like it wasn't.

Maybe something to do with specialization since I think this ends up at

https://github.com/rust-lang/rust/blob/506f22b4663f3e756e1e6a4f66c6309fdc00819c/library/core/src/iter/range.rs#L750-L760

EDIT: Also tried increasing

https://github.com/rust-lang/rust/blob/5793a9e90289382e447f5bc411cab2bc01c5836f/compiler/rustc_mir_transform/src/inline.rs#L33

and that wasn't enough either.

scottmcm commented 1 month ago

this fixes it, but also breaks the exponential growth test, so needs something smarter...

         let inline_limit = match self.history.len() {
             0 => usize::MAX,
-            1..=TOP_DOWN_DEPTH_LIMIT => 1,
+            1..=TOP_DOWN_DEPTH_LIMIT => 3,
             _ => return,
         };