[PATCH] D32614: [GVNHoist] Fix: PR32821, add check for anticipability in case of infinite loops

[Aditya Kumar via Phabricator via llvm-commits]

hiraditya added a comment.

In https://reviews.llvm.org/D32614#740129, @dberlin wrote:

> In https://reviews.llvm.org/D32614#740119, @hiraditya wrote:
>
> > In https://reviews.llvm.org/D32614#739947, @dberlin wrote:
> >
> > > I'm at a loss to understand why you believe you need to compute joint post-dominance (which is what this is) to compute ANTIC.
> >
> >
> > If the set of BasicBlocks (WL) do not joint-post-dominate the hoisting point (HoistBB), then the expression to be hoisted (from WL) cannot be ANTIC in HoistBB.
>
>
> Sure.
>  That is definitely a way of computing it.
>  It is a generally slow and unused way of computing it, because there are only certain points in the SSA graph where ANTIC can actually change.
>
> > It is a necessary condition what I'm checking here.
>
> It is not necessary to perform graph reachability to do this.
>
> https://pdfs.semanticscholar.org/6d0f/07ff330402b46e83d46142e202069d9aeceb.pdf
>
> Stare at the down-safety step.
>  With a few bits, it is only actually necessary to compute and check antic at the possible phis you would insert to do your hoisting.


Thanks for the reference, I'll read the paper and try to implement if that appears more efficient. Currently, I'm in the middle of preparing for a conference so I'll get back to this in a few days.



================
Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:334
+      for (const BasicBlock *Succ : BB->getTerminator()->successors()) {
+        if (Remaining.count(Succ)) // Loop.
+          return false;
----------------
dberlin wrote:
> You can just use the return value of insert.
> 
I'll do that. Thanks.


https://reviews.llvm.org/D32614