[PATCH] D75977: [JumpThreading] Fix PR44611

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:455
+      // We may hit cycles, just add it to Unreachable.
+      if (!DTU->getDomTree().isReachableFromEntry(&BB)) {
+        Unreachable.insert(&BB);
----------------
junparser wrote:
> efriedma wrote:
> > We've been trying to avoid calling getDomTree() in JumpThreading where possible because there's a performance cost: it actually flushes all the pending DomTree updates.  Calling it for every basic block will likely cause performance issues.  (You should be able to find some discussion of this in the review threads for the related JumpThreading patches.)
> Yes, I have already know about that.  That's why i add this check at the end of for statement after other checks. However, for unreachable cycles,  I afraid maybe this is the only way to check (maybe I am wrong).
> 
> Btw, the condition can be update to:
> 
> ```
>  if (Changed && !DTU->getDomTree().isReachableFromEntry(&BB)) 
> ```
> which can reduce the performance cost.
I'd like to see some actual numbers for the performance impact, similar to what was done in the past.

Where exactly is the infinite loop coming from?  We already have some code which should guard the use of LazyValueInfo.


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

https://reviews.llvm.org/D75977