[llvm-dev] [RFC] Simple GVN hoist

[Momchil Velikov via llvm-dev]

On Wed, Sep 15, 2021 at 12:18 PM Momchil Velikov
<momchil.velikov at gmail.com> wrote:
>
> On Tue, Sep 14, 2021 at 7:56 PM Sjoerd Meijer via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> > Fix GVNHoist. I like your patch because it's small and concise, but missing in the RFC is a discussion why we don't try to re-enable GVNHoist. I know it was briefly enabled by default, which was reverted due to correctness (or was it regressions?) problems. But if this belongs in GVNHoist, could this for example be added to GVNHoist, and only this part enabled? Not sure if that's possible as I haven't looked at GVNHoist.
>
> Yes, GVNHoist will do what we need

[hit send too soon]

So, GVNHoist will do the hoisting we want.. However, while technically
possible there a few drawbacks in taking the existing GVNHoist
approach:
* GVNHoist is large and algorithmically complex
* Even if enabled, it won't solve this specific issue - after hoisting
of the loads the loop size falls below some unrolling threshold and
the loop is fully unrolled - instead the loop should be
loop-vectorised, IMHO - so we get a pass order issue.
* After unrolling the SLP vectoriser does not vectorise the loop - so
we have to fix the SLP vectoriser too.

While I think all of these are worthwhile, we have on one hand some
rather simple transformation (the mini-GVNHoist) vs. on the other hand
the actual  GVNHoist+pass manager+SLP Vectoriser, which
could easily turn into a time sink with no end in sight.