[PATCH] D32663: [SCEV] createAddRecFromPHI: Optimize for the most common case.
Sanjoy Das via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 2 19:11:19 PDT 2017
sanjoy requested changes to this revision.
sanjoy added inline comments.
This revision now requires changes to proceed.
================
Comment at: lib/Analysis/ScalarEvolution.cpp:4092
+/// technique for finding the AddRec expression.
+const SCEV *ScalarEvolution::createAddRecFromPHISimpleCase(PHINode *PN,
+ Value *BEValueV,
----------------
How about `createSimpleAffineAddRec`?
================
Comment at: lib/Analysis/ScalarEvolution.cpp:4097
+ assert(L && L->getHeader() == PN->getParent());
+ assert (BEValueV && StartValueV);
+
----------------
Bad space?
================
Comment at: lib/Analysis/ScalarEvolution.cpp:4108
+ if (BO->LHS == PN)
+ Accum = getExistingSCEV(BO->RHS);
+ else if (BO->RHS == PN)
----------------
I'd avoid using `getExistingSCEV` here. Instead, how about checking `L->isLoopInvariant(BO->RHS or BO->LHS)` and if so, call `getSCEV`? Then you won't waste work since if `L->isLoopInvariant(BO->RHS or BO->LHS)` is true then you know you're going to be able to return an add recurrence.
================
Comment at: lib/Analysis/ScalarEvolution.cpp:4109
+ Accum = getExistingSCEV(BO->RHS);
+ else if (BO->RHS == PN)
+ Accum = getExistingSCEV(BO->LHS);
----------------
As discussed on IRC, please check if this is actually viable.
================
Comment at: lib/Analysis/ScalarEvolution.cpp:4118
+ if (!isLoopInvariant(Accum, L) &&
+ (!isa<SCEVAddRecExpr>(Accum) ||
+ cast<SCEVAddRecExpr>(Accum)->getLoop() != L))
----------------
If you do the check above on `L->isLoopInvariant(BO->RHS or BO->LHS)` then this won't be necessary.
================
Comment at: lib/Analysis/ScalarEvolution.cpp:4176
+ // value for PN.
+ if (auto S = createAddRecFromPHISimpleCase(PN, BEValueV, StartValueV))
+ return S;
----------------
`auto *`.
https://reviews.llvm.org/D32663
More information about the llvm-commits
mailing list