[PATCH] Use invariants in ScalarEvolution

Philip Reames listmail at philipreames.com
Wed Jul 23 15:41:21 PDT 2014


I'm not knowledgeable of this code, but I don't see anything which is obviously wrong.  Feel free to take or ignore the ordering and style comments.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:6327
@@ -6325,3 +6326,3 @@
                        LoopContinuePredicate->getSuccessor(0) != L->getHeader());
 }
 
----------------
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)

================
Comment at: lib/Analysis/ScalarEvolution.cpp:6351
@@ +6350,3 @@
+
+    if (LoopEntryPredicate)
+      for (BasicBlock::iterator I : *LoopEntryPredicate->getParent())
----------------
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.  

================
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))
----------------
You have this pattern of checks a lot in different changes.  Time to extract a helper function somewhere common?  isAssumeIntrinsic(Instruction* I)?

http://reviews.llvm.org/D4568






More information about the llvm-commits mailing list