[MIR] Implement move up propagation without post dominators

[scottcarr]

This is a re-implementation of #34585 to not use post dominators.

Further potential improvements include:

Calculate live variable sets to avoid re-walking the graph for each potential replacement

Somehow cache whether a Call terminator is reachable from a given BasicBlock (to again avoid re-walking the graph)

I'm not sure if we still need the Call terminator restriction with the current precondition (the DEST is never borrowed)

See also: https://internals.rust-lang.org/t/mir-move-up-propagation/3610

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @arielb1 (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[scottcarr]

r? @nikomatsakis

[arielb1]

why? WHY?

I think you are intending to compare locations or something.

[arielb1]

A move up-propagation can invalidate other move up-propagations. For example, a swap of 2 variables:

tmp0 = x
tmp1 = y
y = tmp0
x = tmp1

Here you can remove either of the temporaries, but not both of them.

This does not actually work out, but I am still not sure MUPs are independent.

Actual example:

start:
    tmp0 = x
    tmp1 = y
    br *, a, b
a:
    ret = tmp0
    goto exit
b:
    ret = tmp1
    goto exit

This can be optimized into either

start:
    ret = x
    tmp1 = y
    br *, a, b
a:
    goto exit
b:
    ret = tmp1
    goto exit

or

start:
    tmp0 = x
    ret = y
    br *, a, b
a:
    ret = tmp0
    goto exit
b:
    goto exit

but not

start:
    ret = x
    ret = y
    br *, a, b
a:
    goto exit
b:
    goto exit

obviously.

[arielb1]

I would prefer to return a Vec<(Temp, Def, Use)> or something.

BTW, you can use filter_map or flat_map instead of .filter followed by .map.

[nikomatsakis]

@arielb1

This does not actually work out, but I am still not sure MUPs are independent.

I agree they are not necessarily independent -- this is partly why "re-analyzing" the graph for each candidate can be easier (versus trying to do some kind of overall live analysis), since any changes we make will be automatically accounted for.

In any case, were you just noting the potential problem, or proposing some kind of smarter, more global way to choose which optimizations to do? (Or were you suggesting that there is a problem where interference could cause the current code to do the wrong thing?)

[arielb1]

@nikomatsakis

The current code is basically local, but a naïve non-local version of it would handle this case incorrectly.

In any case, re-analyzing the graph for each candidate could get expensive. LLVM has some crazy heuristics for memdep - might be wise to try and copy them.

[nikomatsakis]

I would like to see more comments here. Ideally, we would explain what the optimization will do, like showing one example of the transformation, and labeling the various parts of it. Then we can have comments that link the code to that transformation.

[bors]

☔ The latest upstream changes (presumably #35168) made this pull request unmergeable. Please resolve the merge conflicts.

[eddyb]

This file's name has "slice" in it but these are arrays being copied.
(move_chain seems more relevant, the type itself doesn't really matter)

[bors]

☔ The latest upstream changes (presumably #36504) made this pull request unmergeable. Please resolve the merge conflicts.

[nikomatsakis]

OK, I'm going to close this PR because of inactivity, but I think we probably want to "rejuvenate" this code. @scottcarr if you are still interested in hacking on it, let's talk.