[llvm] [SCEVExpander] Relax hoisting condition for AddRec start (PR #75916)

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 21 07:58:22 PST 2023


================
@@ -980,11 +980,14 @@ SCEVExpander::getAddRecExprPHILiterally(const SCEVAddRecExpr *Normalized,
   PostIncLoopSet SavedPostIncLoops = PostIncLoops;
   PostIncLoops.clear();
 
-  // Expand code for the start value into the loop preheader.
-  assert(L->getLoopPreheader() &&
-         "Can't expand add recurrences without a loop preheader!");
-  Value *StartV =
-      expand(Normalized->getStart(), L->getLoopPreheader()->getTerminator());
+  // Expand code for the start value into the loop predecessor. The loop is not
----------------
preames wrote:

This looks suspect.  The difference between a preheader and a predecessor is whether the loop header is guaranteed to execute after the execution of the former.  There's two concerns here:
1) This ends up speculating instructions onto paths which didn't previous execute them.  Might be legally fine, but profitability is suspect.  
2) I believe expanded sometimes uses "must execute" reasoning to prove safety (i.e. non speculation), such logic would become unsound with this change (I think? - haven't fully traced it through.)

I agree with @nikic that this looks like a isSafeToExpand check missing somewhere.  

https://github.com/llvm/llvm-project/pull/75916


More information about the llvm-commits mailing list