Recognize discriminant reads as no-ops in RemoveNoopLandingPads

[tmiasko]

The cleanup blocks often contain read of discriminants. Teach
RemoveNoopLandingPads to recognize them as no-ops to remove
additional no-op landing pads.

[jonas-schievink]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit ecd7862 with merge 847577274ba21becb5fce245fb53d7d6a392e951...

[bors]

☀️ Try build successful - checks-actions, checks-azure
Build commit: 847577274ba21becb5fce245fb53d7d6a392e951 (847577274ba21becb5fce245fb53d7d6a392e951)

[rust-timer]

Queued 847577274ba21becb5fce245fb53d7d6a392e951 with parent cae8bc1, future comparison URL.

[ecstatic-morse]

I wonder how common it is to have writes to unprojected locals that are not drop flags along the cleanup path? Seems like we could ignore these as well.

[ecstatic-morse]

LGTM, I'd like to see a MIR opt test in which these changes have an impact, although maybe this is more speculative? I'll check back in once perf results are up.

[rust-timer]

Finished benchmarking try commit (847577274ba21becb5fce245fb53d7d6a392e951): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never

[tmiasko]

I was examining MIR of functions making extensive use of ? together with a Result type where Err needs a drop, but Ok doesn't. The cleanup path contained mostly no-op code but it wasn't detected as such, and this was the reason why. When rebuilding the standard library, this increases the number of no-op jumps folded from 0 to 190, and no-op landing pads from 139220 to 139994. A small improvement.

I also experimented with a more aggressive variant, but that had no additional effect. Maybe someone more familiar with code generated by drop elaboration could tell if there is some particular pattern still missing, but I didn't notice any.

Regarding tests, this pass runs twice with the same name, and diff is unfortunately for the wrong one, which is why so far I didn't add any.

[ecstatic-morse]

Got it! You could add a name to RemoveNoopLandingPads like SimplifyCfg currently has, but that's not really your responsibility, and it doesn't need to block this. r=me when you're happy.

@bors delegate+

[bors]

✌️ @tmiasko can now approve this pull request

[tmiasko]

@bors r=ecstatic-morse

[jonas-schievink]

Hmm, seems like delegation is broken

@bors r=ecstatic-morse

[bors]

📌 Commit ecd7862 has been approved by ecstatic-morse

[bors]

⌛ Testing commit ecd7862 with merge 8cc82ee...

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: ecstatic-morse
Pushing 8cc82ee to master...

[Mark-Simulacrum]

Excellent (2-3% instructions, <1% wall-time) improvements on several real-world benchmarks, including style-servo. Great work!