[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