[PATCH] D112840: [PassManager] `buildFunctionSimplificationPipeline()`: schedule another `LoopDeletion` pass run before last `LICM`

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

In D112840#3097852 <https://reviews.llvm.org/D112840#3097852>, @lebedev.ri wrote:

> In D112840#3097844 <https://reviews.llvm.org/D112840#3097844>, @aeubanks wrote:
>
>> In D112840#3097842 <https://reviews.llvm.org/D112840#3097842>, @lebedev.ri wrote:
>>
>>> Hmm, actually, i looked, and we catch a lot more cases if we do this in `buildModuleOptimizationPipeline()`, so let me do that instead. BRB.
>>
>> it'd be nice to make this work in the function simplification pipeline so that more passes have a chance to take advantage of deleted loops, e.g. the inliner
>> putting passes at the end of the pipeline doesn't seem ideal unless there's a specific reason for it
>
> From where i stand, we roughly have 3 obvious choices:
>
> 1. keep dead loops :)
> 2. the current diff, delete dead loops at the end of function simplification pipeline
> 3. upcoming diff, delete dead loops before vectorization in module optimization pipeline. catches more cases, but perhaps pessimizes inliner.
> 4. 2 & 3, slowest of them all.
>
> What do you have in mind instead?

I looked into `@is_not_empty_variant1()`. It seems like GVN + an instcombine cleanup is what's allowing the loop to be deleted. Currently GVN runs after the loop passes which is the issue.
I tried replacing the EarlyCSEPass at the beginning of the function simplification pipeline with a NewGVNPass and that fixes `@is_not_empty_variant1`. I think something along those lines is more principled than adding an LoopDeletionPass somewhere.
(Of course, if we want to turn on NewGVN specifically we need to iron out remaining issues)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112840/new/

https://reviews.llvm.org/D112840