[PATCH] D52827: [LICM] Make LICM able to hoist phis

[John Brawn via Phabricator via llvm-commits]

john.brawn added a comment.

In https://reviews.llvm.org/D52827#1254146, @efriedma wrote:

> > This has no impact by itself that I've been able to see, as LICM typically doesn't see such phis as they will have been converted into selects by the time LICM is run
>
> That's surprising... I would have expected this to show up in some cases. Some branches can't eliminated due to side-effects.


It's quite likely that the various TODOs are preventing anything anything significant from being hoisted.



================
Comment at: lib/Transforms/Scalar/LICM.cpp:797
+  // so we iterate through them in first-in-last-out order which ensures that
+  // when we hoist an instruction we hoist its operands.
+  Instruction *HoistPoint = OriginalPreheader->getTerminator();
----------------
efriedma wrote:
> Would it be better to insert a PHI node, rather than re-hoist?
I considered that, but went with this as it gives more similar results to the current behaviour in the cases where we end up not hoisting any phis. However I'm currently working on fixing the various TODOs here and it looks like maybe I will need to go for the approach of inserting phis. I'm currently working on it.


Repository:
  rL LLVM

https://reviews.llvm.org/D52827