[PATCH] Use invariants in ScalarEvolution

hfinkel at anl.gov hfinkel at anl.gov
Fri Jul 25 13:58:17 PDT 2014


================
Comment at: lib/Analysis/ScalarEvolution.cpp:6327
@@ -6325,3 +6326,3 @@
                        LoopContinuePredicate->getSuccessor(0) != L->getHeader());
 }
 
----------------
Philip Reames wrote:
> It seems like the assumption code could also apply here.  The patterns are very similar.  
> 
> I can't think of a case of the top of my head where this would be profitable though.  Maybe something about pre and post alignment loops in vectorized code?  (i.e. we can conclude that only one iteration is needed from alignment info)
Good point. I also could not particularly think of a place where this would help, but the cost of finding the intrinsics and determining applicability here is low, so we might as well.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:6351
@@ +6350,3 @@
+
+    if (LoopEntryPredicate)
+      for (BasicBlock::iterator I : *LoopEntryPredicate->getParent())
----------------
Philip Reames wrote:
> Shouldn't this be after the if(!LoopEntryPredicate) early return?
> 
> It also feels like it should be after the branch condition check since that's likely to be cheaper.
> 
> I even wonder if the assumption processing shouldn't be done as a separate loop to avoid searching out invariants in what might be a common case.  
It could be, but then I'd also need to split the early return (because we don't want it after the LoopEntryPredicate->isUnconditional() part of the early return). Thinking about it, splitting the early-return check is the cleaner thing to do.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:6355
@@ +6354,3 @@
+          if (Function *F = CI->getCalledFunction())
+            if (F->getIntrinsicID() == Intrinsic::assume &&
+                isImpliedCond(Pred, LHS, RHS, CI->getArgOperand(0), false))
----------------
Philip Reames wrote:
> You have this pattern of checks a lot in different changes.  Time to extract a helper function somewhere common?  isAssumeIntrinsic(Instruction* I)?
True. In many places I use the PatternMatch header facilities to make this easier, and I should perhaps do so more often (like here, for example).

http://reviews.llvm.org/D4568






More information about the llvm-commits mailing list