[PATCH] D69830: [LoopPred/WC] Use a dominating widenable condition to remove analyze loop exits

[Fedor Sergeev via Phabricator via llvm-commits]

fedor.sergeev added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:1108
+    BasicBlock *ExitBB = BI->getSuccessor(ExitIfTrue ? 0 : 1);
+    if (!ExitBB->getTerminatingDeoptimizeCall())
+      // Profitability: indicator of rarely/never taken exit
----------------
ebrevnov wrote:
> fedor.sergeev wrote:
> > ebrevnov wrote:
> > > Would it be better to check profitability in a more general way using BPI or similar?
> > Since widenable branch jumping to deoptimize is the main intended usecase for widenable branch itself
> > (we even show it in LangRef as a prime example of widenable.condition usage)
> > it seems to be a very good special case to start with.
> > 
> > BPI is tricky to use (and tricky to preserve), so it can not be the main source of truth for profitability purposes.
> > 
> First I don't see anything tricky in using BPI. It is widely used across optimizer and if BPI is "incorrect" that may affect lot of places. Moreover LoopPredication has explicit dependence on BPI and won't work as expected with "broken" BPI anway.  Also note that current implementation doesn't handle widenable branches because such branches are not analyzable.
> LoopPredication has explicit dependence on BPI and won't work as expected with "broken" BPI anway.
I'm not talking about "broken BPI". 
In new PassManager BPI is not always preserved through the loop passes, so it can be just "absent".

And yes, current profitability checks do depend on BPI, yet LoopPredication will just consider loop as unconditionally profitable in absence of BPI.



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

https://reviews.llvm.org/D69830