rust-lang / rust

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

dead-code optimize `if const { expr }` even in opt-level=0 #85836

Closed scottmcm closed 6 months ago

scottmcm commented 3 years ago

inline_const tracking issue: https://github.com/rust-lang/rust/issues/76001

Inspired by @oli-obk's comment in https://github.com/rust-lang/rust/pull/85828#discussion_r642046617

For things like if const { expr } { 0 } else { do something complex } it'd be nice to not have to even give LLVM the code for the else branch to have to emit. Ought to save time and binary size.

Tentatively marked mir-opt, since we presumably still need to make the MIR for it, in order to borrow-check and such the unreachable code. But might need to be codegen-time instead to pick up on monomorphized inline constants.

Hopefully should be doable relatively easily when the const is an inline const, since that forces const folding already, and thus there should just be a const true in the MIR.

scottmcm commented 3 years ago

Well, turns out this already works for simple constants in MIR! 🎉 https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=4d1fe08eec8b705525535ec9ec9d1c89

However, the place I actually needed it was more like if const { size_of::<T>() < 100 }, which doesn't work with inline_const today. So maybe come back to this once that's fixed. https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=80c275a75dc280ac3fcdfb24b6484adc

nagisa commented 3 years ago

This should be done by SimplifyBranches which AFAIK is run unconditionally. I wonder if its match needs to be expanded?

Is there mcve?

oli-obk commented 3 years ago

The problem is that this can't be done on generic constants, as we don't mir-optimize post-monomorphization. We could start doing some benchmarks to see how bad running all mir opts (or at least a select few) would be if we duplicated, substituted and optimized MIR bodies for concrete generic parameters.

oli-obk commented 3 years ago

I would say

trait SizeBiggerFour: Sized {
    const VALUE: bool = std::mem::size_of::<Self>() > 4;
}

impl<T> SizeBiggerFour for T {}

pub fn foo<T>() -> i32 { if <T as SizeBiggerFour>::VALUE { 3 } else { 5 } }
pub fn bar() -> i32 { foo::<isize>() }

qualifies as an MCVE as the monomorphized foo::<isize> gives the following llvm ir

; playground::foo
; Function Attrs: nonlazybind uwtable
define i32 @_ZN10playground3foo17hb9992b01cb0c258eE() unnamed_addr #0 !dbg !6 {
start:
  %0 = alloca i32, align 4
  br i1 true, label %bb1, label %bb2, !dbg !15

bb2:                                              ; preds = %start
  store i32 5, i32* %0, align 4, !dbg !16
  br label %bb3, !dbg !15

bb1:                                              ; preds = %start
  store i32 3, i32* %0, align 4, !dbg !17
  br label %bb3, !dbg !15

bb3:                                              ; preds = %bb2, %bb1
  %1 = load i32, i32* %0, align 4, !dbg !18
  ret i32 %1, !dbg !18
}
the8472 commented 3 years ago

We could start doing some benchmarks to see how bad running all mir opts (or at least a select few) would be if we duplicated, substituted and optimized MIR bodies for concrete generic parameters.

Maybe it should also be evaluated together with polymorphization or const_evaluatable_checked since those can cut down on the duplication before applying further optimizations.

scottmcm commented 3 years ago

Bonus points if this can avoid monomorphizing functions called in the dead code too.

Demo extracted from core: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=40cc11066ca1fa8462d0f0f403ee1d8f (cc #52051)

Would be great to not codegen all the downstream stuff that's only called in the else block of a br i1 true.

nbdd0121 commented 2 years ago

However, the place I actually needed it was more like if const { size_of::<T>() < 100 }, which doesn't work with inline_const today. So maybe come back to this once that's fixed.

FYI this should work with #96557

DemiMarie commented 2 years ago

Totally wild (and probably bad) idea: could we avoid borrow-checking unreachable code? After all, the purpose of borrow-checking is to prevent memory unsafety at runtime, but unreachable code cannot be memory unsafe, because it never executes!

scottmcm commented 2 years ago

@DemiMarie We already do! As a quick demo,

fn foo(x: &mut String) {
    let r = &*x;
    x.push_str("asdf");
    return; // comment this to get a borrowck error
    dbg!(r);
}

(But this isn't really the right issue to discuss possible extensions to that.)

RalfJung commented 7 months ago

This seems closely related to https://github.com/rust-lang/rfcs/issues/3582 and https://github.com/rust-lang/rust/issues/122301.

RalfJung commented 7 months ago

@saethlin's recent PR https://github.com/rust-lang/rust/pull/121421 seems related here? If the if condition becomes a constant during codegen, we should be entirely skipping it now and not even generate LLVM IR.

However, if that dead code calls functions, those still get codegen'd as usual -- the collector that determines which function to codegen doesn't know about dead branches. We'd have to embed a (simple) monomorphization-time reachability analysis in the collector to fix that.