[mir-opt] optimization idea: bubble return terminators to predecessor blocks

[oli-obk]

cc @rust-lang/wg-mir-opt

if we have a basic block with no statements and just a return terminator, any other basic block that just gotos into that block could just as well call return itself. Or do we have some rule that we can only have a single return terminator per mir::Body?

Maybe we can generalize this and bubble any terminator of a statement-less block into its predecessor terminators iff that predecessor terminator is a goto.

As an example take

rust/src/test/mir-opt/issue-62289/rustc.test.ElaborateDrops.before.mir

Line 84 in 7b80539

bb7: {

which is a basic block with no statements and just a return terminator, and both predecessor blocks have a goto to this block.

[hanna-kruppe]

if we have a basic block with no statements and just a return terminator, any other basic block that just gotos into that block could just as well call return itself. Or do we have some rule that we can only have a single return terminator per mir::Body?

I don't know if we have such a rule in (any part of) rustc, but in my experience this is a convenient assumption for some analyses and transforms. So depending on the expected value of "bubbling up" returns (at a glance, it seems low to me), I'd be cautious about doing it just because we can.

[ecstatic-morse]

The docs for TerminatorKind::Return do say that "this should occur at most once". I don't know how many places depend on this, however, and it shouldn't be too hard to fix up the ones that do.

[matthewjasper]

The built MIR only contains a single Return terminator. However the generator transform code appears to output MIR with multiple return terminators in src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir

[nagisa]

this should occur at most once

I recall this being motivated by LLVM liking a singular return better than multiple returns. "Should" is not a "must", so it isn’t incorrect that we generate (sometimes) multiple returns. But it is still a guidance that stands.

As thus I don’t love what #72563 did /me shrugs.

[ecstatic-morse]

I concur with @nagisa. Canonicalizing a mistake made in the generator transform is wrongheaded IMO. If we want to do this, we should make a conscious decision. There's probably code that relies on only one Return terminator existing; we just haven't found those bugs yet.

[bjorn3]

Cranelift likes multiple returns better than single returns. It never duplicates ret instructions, even when it would decrease the amount of register moves and jumps.

[oli-obk]

This was only added under mir-opt-level 3, in case anyone is wondering why this is closed. We haven't finished the discussion on whether we want multiple return terminators per body or whether we should fix generators