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

[Hal Finkel via Phabricator via llvm-commits]

hfinkel 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;
----------------
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?


Repository:
  rL LLVM

https://reviews.llvm.org/D32433