Missed optimization: RVO isn't applied

[gootorov]

Consider the code below. At the first glace, it looks like RVO/destination propagation isn't applied, resulting in a stack overflow.

File structure

tree .

.
├── bin
│   └── rvo.rs
├── Cargo.toml
├── src
│   └── lib.rs

Code

Cargo.toml

[package]
name = "rvo"
version = "0.1.0"
edition = "2021"

[[bin]]
name = "rvo"
path = "bin/rvo.rs"

[dependencies]

[profile.release]
lto = "fat"

bin/rvo.rs

use rvo::Outer;

fn main() {
    let _foo = Box::new(Outer::new());
}

src/lib.rs

#![allow(unused)]

const SIZE: usize = 1024 * 1024 * 1024;

pub struct Outer {
    a: LargeStruct,
}

impl Outer {
    #[inline(always)]
    pub fn new() -> Self {
        let a = LargeStruct::new();

        Self { a }
    }
}

pub struct LargeStruct {
    d0: Option<Box<u128>>,
    d1: Option<Box<u128>>,
    d2: Option<Box<u128>>,
    d3: Option<Box<u128>>,
    d4: Option<Box<u128>>,
    huge: [u8; SIZE],
}

impl LargeStruct {
    #[inline(always)]
    pub fn new() -> Self {
        Self {
            d0: None,
            d1: None,
            d2: None,
            d3: None,
            d4: None,
            huge: [0; SIZE],
        }
    }
}

Running cargo run --release results in:

    Finished release [optimized] target(s) in 0.01s
     Running `target/release/rvo`

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
Aborted (core dumped)

Compiler version:
rustc --version --verbose

rustc 1.73.0 (cc66ad468 2023-10-03)
binary: rustc
commit-hash: cc66ad468955717ab92600c770da8c1601a4ff33
commit-date: 2023-10-03
host: x86_64-unknown-linux-gnu
release: 1.73.0
LLVM version: 17.0.2

[gootorov]

Interestingly, the stack no longer overflows when just a few fields are removed:

diff --git a/src/lib.rs b/src/lib.rs
index d84519e..3d43db8 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -18,9 +18,6 @@ impl Outer {
 pub struct LargeStruct {
     d0: Option<Box<u128>>,
     d1: Option<Box<u128>>,
-    d2: Option<Box<u128>>,
-    d3: Option<Box<u128>>,
-    d4: Option<Box<u128>>,
     huge: [u8; SIZE],
 }
 
@@ -30,9 +27,6 @@ impl LargeStruct {
         Self {
             d0: None,
             d1: None,
-            d2: None,
-            d3: None,
-            d4: None,
             huge: [0; SIZE],
         }
     }

[saethlin]

At the first glace, it looks like RVO/destination propagation isn't applied

Both these MIR optimizations are off by default, and the NRVO MIR opt is unsound.

[gootorov]

Both these MIR optimizations are off by default, and the NRVO MIR opt is unsound.

Interesting, thank you. I was always under the impression that optimizations of the Box::new(returns_large_object()) type are RVO. Or is it (currently) just LLVM's RVO and not MIR?

Also, is there a way to turn on these MIR optimizations via a compiler flag/crate attribute?

[workingjubilee]

Yes, currently LLVM performs the relevant RVO (or doesn't).
You can use -Zmir-enable-passes=+DestinationPropagation,+RenameReturnPlace.

Alternately, I believe most unsound opts are enabled at -Zmir-opt-level=4, but that can cause other hilarious bugs.

[saethlin]

Unsound optimizations must be enabled by their own flag, -Zunsound-mir-opts. Unfortunately perhaps, -Zmir-opt-level=4 is used in the ecosystem.

It's possible your request here will be fulfilled by #116531.

[gootorov]

Unfortunately, if I did everything correctly, #116531 doesn't help in this case :(

I've checked-out #116531 and built stage2. Then ran the binary again:

RUSTFLAGS="-Zmir-opt-level=4 --emit=mir" cargo +stage2 run --release

Still overflows.

This is the emitted MIR:

mir

// WARNING: This output format is intended for human consumers only
// and is subject to change without notice. Knock yourself out.
const SIZE: usize = {
  let mut _0: usize;
  let mut _1: usize;
  let mut _2: (usize, bool);
  let mut _3: (usize, bool);

  bb0: {
      StorageLive(_1);
      _2 = CheckedMul(const 1024_usize, const 1024_usize);
      assert(!move (_2.1: bool), "attempt to compute `{} * {}`, which would overflow", const 1024_usize, const 1024_usize) -> [success: bb1, unwind continue];
  }

  bb1: {
      _1 = move (_2.0: usize);
      _3 = CheckedMul(_1, const 1024_usize);
      assert(!move (_3.1: bool), "attempt to compute `{} * {}`, which would overflow", move _1, const 1024_usize) -> [success: bb2, unwind continue];
  }

  bb2: {
      _0 = move (_3.0: usize);
      StorageDead(_1);
      return;
  }
}

fn <impl at src/lib.rs:9:1: 9:11>::new() -> Outer {
  let mut _0: Outer;
  let _1: LargeStruct;
  scope 1 {
      debug a => _1;
  }
  scope 2 (inlined LargeStruct::new) {
      let mut _2: [u8; 1073741824];
  }

  bb0: {
      StorageLive(_2);
      _2 = [const 0_u8; 1073741824];
      _1 = LargeStruct { d0: const Option::<Box<u128>>::None, d1: const Option::<Box<u128>>::None, d2: const Option::<Box<u128>>::None, d3: const Option::<Box<u128>>::None, d4: const Option::<Box<u128>>::None, huge: move _2 };
      StorageDead(_2);
      _0 = Outer { a: move _1 };
      return;
  }
}

LargeStruct::huge::{constant#0}: usize = {
  let mut _0: usize;

  bb0: {
      _0 = const _;
      return;
  }
}

fn <impl at src/lib.rs:27:1: 27:17>::new() -> LargeStruct {
  let mut _0: LargeStruct;
  let mut _1: [u8; 1073741824];

  bb0: {
      StorageLive(_1);
      _1 = [const 0_u8; 1073741824];
      _0 = LargeStruct { d0: const Option::<Box<u128>>::None, d1: const Option::<Box<u128>>::None, d2: const Option::<Box<u128>>::None, d3: const Option::<Box<u128>>::None, d4: const Option::<Box<u128>>::None, huge: move _1 };
      StorageDead(_1);
      return;
  }
}

<impl at src/lib.rs:27:1: 27:17>::new::{constant#0}: usize = {
  let mut _0: usize;

  bb0: {
      _0 = const _;
      return;
  }
}

// WARNING: This output format is intended for human consumers only
// and is subject to change without notice. Knock yourself out.
fn main() -> () {
  let mut _0: ();
  let _1: std::boxed::Box<rvo::Outer>;
  let mut _2: rvo::Outer;
  scope 1 {
      debug _foo => _1;
  }
  scope 2 (inlined Outer::new) {
      let _3: rvo::LargeStruct;
      scope 3 {
          debug a => _3;
      }
      scope 4 (inlined LargeStruct::new) {
          let mut _4: [u8; 1073741824];
      }
  }

  bb0: {
      StorageLive(_1);
      StorageLive(_2);
      StorageLive(_3);
      StorageLive(_4);
      _4 = [const 0_u8; 1073741824];
      _3 = LargeStruct { d0: const Option::<Box<u128>>::None, d1: const Option::<Box<u128>>::None, d2: const Option::<Box<u128>>::None, d3: const Option::<Box<u128>>::None, d4: const Option::<Box<u128>>::None, huge: move _4 };
      StorageDead(_4);
      _2 = Outer { a: move _3 };
      StorageDead(_3);
      _1 = Box::<Outer>::new(move _2) -> [return: bb1, unwind continue];
  }

  bb1: {
      StorageDead(_2);
      drop(_1) -> [return: bb2, unwind continue];
  }

  bb2: {
      StorageDead(_1);
      return;
  }
}

Also, just in case, I tried perhaps less aggressive

RUSTFLAGS="-Zmir-enable-passes=+DestinationPropagation,+RenameReturnPlace --emit=mir" cargo +stage2 run --release

and there were no difference in the emitted MIR

[workingjubilee]

Did you use -Zunsound-mir-opts as well?

[gootorov]

I thought that wasn't necessary judging by the diff in #116531, but to be sure, I've just tried these two variants:

RUSTFLAGS="-Zunsound-mir-opts -Zmir-enable-passes=+DestinationPropagation,+RenameReturnPlace --emit=mir" cargo +stage2 run --release

RUSTFLAGS="-Zunsound-mir-opts -Zmir-opt-level=4 --emit=mir" cargo +stage2 run --release

Unfortunately, same outcome, and the emitted MIR is also the same as before.

[gootorov]

Actually, I think I'm doing something wrong. Just tried comparing this MIR to stable MIR (RUSTFLAGS="--emit=mir" cargo run --release) and there also isn't any difference

[saethlin]

--release is about equivalent to -Zmir-opt-level=2. You're doing things fine, the MIR optimization as-written simply doesn't help for this specific case.