[PATCH] D32433: Compute safety information in a much finer granularity.

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/Utils/LoopUtils.cpp:1050
+  for (Instruction *ExitInst : SafetyInfo->EarlyExits)
+    if (!DT->dominates(&Inst, ExitInst))
+      return false;
----------------
hfinkel wrote:
> dberlin wrote:
> > efriedma wrote:
> > > This could be slow: if the two instructions are in the same basic block, dominates() will iterate over the whole block. And you don't attempt to prune the EarlyExits list, which could make LICM O(N^3) overall for a large loop with a single basic block.
> > > 
> > > I guess we'll see if it matters in practice.
> > Two  fixes, one short term we should do, one longer term
> > 
> > 1. Let that dominates call take an OBB as an optional argument
> > 
> > 2. Just provide global + local dominance ordering as an analysis that encapsulates both the OBB + DT solution, and remove that use of dominates in favor of the analysis.
> > 
> > 
> Ack; I forgot about that scanning behavior. Using an OBB is right here. Right now, you need to do:
>   if (I1->getParent() == I2->getParent())
>     return OBB->dominates(I1, I2);
>   else
>     return DT->dominates(I1, I2);
> 
> I don't think we can yet hand an OBB directly to the DT (although that would certainly be a nice enhancement). I presume that we'd actually need a map of OBBs, as in Utils/PredicateInfo.h. Maybe stick it into LoopSafetyInfo?
That is right, we can't do it yet, but seems simple enough.
We'd need a map.
Either add to loop safety info, or just define a struct that has the right properties and pass it along.



Repository:
  rL LLVM

https://reviews.llvm.org/D32433