Skip to content

[SCEV] Try to re-use existing LCSSA phis when expanding SCEVAddRecExpr. #147214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+25 −3
Open

[SCEV] Try to re-use existing LCSSA phis when expanding SCEVAddRecExpr. #147214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01a3d46
[SCEV] Try to re-use existing LCSSA phis when expanding SCEVAddRecExpr.
fhahn Jul 4, 2025
1faa3bd
!fixup adjust comment, thanks
fhahn Jul 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,7 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {

bool isExpandedAddRecExprPHI(PHINode *PN, Instruction *IncV, const Loop *L);

Value *tryToReuseLCSSAPhi(const SCEVAddRecExpr *S);
Value *expandAddRecExprLiterally(const SCEVAddRecExpr *);
PHINode *getAddRecExprPHILiterally(const SCEVAddRecExpr *Normalized,
const Loop *L, Type *&TruncTy,
Expand Down
23 changes: 23 additions & 0 deletions llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1223,6 +1223,24 @@ Value *SCEVExpander::expandAddRecExprLiterally(const SCEVAddRecExpr *S) {
return Result;
}

Value *SCEVExpander::tryToReuseLCSSAPhi(const SCEVAddRecExpr *S) {
const Loop *L = S->getLoop();
BasicBlock *EB = L->getExitBlock();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Limitation to single-exit loops is unfortunate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be possible to extend this to multi-exit loops, and just check for the exit dominating the insertion point as follow-up?

The main thing is that we need phis with single incoming values.

if (!EB || !EB->getSinglePredecessor() ||
!SE.DT.dominates(EB, Builder.GetInsertBlock()))
return nullptr;

for (auto &PN : EB->phis()) {
if (!SE.isSCEVable(PN.getType()) || PN.getType() != S->getType())
continue;
auto *ExitV = SE.getSCEV(&PN);
if (S == ExitV)
return &PN;
}

return nullptr;
}

Value *SCEVExpander::visitAddRecExpr(const SCEVAddRecExpr *S) {
// In canonical mode we compute the addrec as an expression of a canonical IV
// using evaluateAtIteration and expand the resulting SCEV expression. This
Expand Down Expand Up @@ -1262,6 +1280,11 @@ Value *SCEVExpander::visitAddRecExpr(const SCEVAddRecExpr *S) {
return V;
}

// If S is expanded outside the defining loop, check if there is a
// matching LCSSA phi node for it.
if (Value *V = tryToReuseLCSSAPhi(S))
return V;

// {X,+,F} --> X + {0,+,F}
if (!S->getStart()->isZero()) {
if (isa<PointerType>(S->getType())) {
Expand Down
4 changes: 1 addition & 3 deletions llvm/test/Transforms/LoopVectorize/reuse-lcssa-phi-scev-expansion.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,9 @@ define void @reuse_lcssa_phi_for_add_rec1(ptr %head) {
; CHECK-NEXT: [[IV_NEXT]] = add nuw i64 [[IV]], 1
; CHECK-NEXT: br i1 [[EC_1]], label %[[PH:.*]], label %[[LOOP_1]]
; CHECK: [[PH]]:
; CHECK-NEXT: [[IV_2_LCSSA:%.*]] = phi i32 [ [[IV_2]], %[[LOOP_1]] ]
; CHECK-NEXT: [[IV_LCSSA:%.*]] = phi i64 [ [[IV]], %[[LOOP_1]] ]
; CHECK-NEXT: [[IV_2_NEXT_LCSSA:%.*]] = phi i32 [ [[IV_2_NEXT]], %[[LOOP_1]] ]
; CHECK-NEXT: [[TMP0:%.*]] = phi i32 [ [[IV_2_NEXT]], %[[LOOP_1]] ]
; CHECK-NEXT: [[SRC_2:%.*]] = tail call noalias noundef dereferenceable_or_null(8) ptr @calloc(i64 1, i64 8)
; CHECK-NEXT: [[TMP0:%.*]] = add i32 [[IV_2_LCSSA]], 1
Comment on lines -23 to -25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct? Isn't tmp0 = iv_2_lcssa + 1 where the + 1 is dropped now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original version has tmp0 = %iv.2 + 1, in the new version tmp0 = iv.2.next which should also be %iv.2 + 1

; CHECK-NEXT: [[SMIN:%.*]] = call i32 @llvm.smin.i32(i32 [[TMP0]], i32 1)
; CHECK-NEXT: [[TMP1:%.*]] = sub i32 [[TMP0]], [[SMIN]]
; CHECK-NEXT: [[TMP2:%.*]] = zext i32 [[TMP1]] to i64
Expand Down
Loading