Skip to content

Commit 89d331a

Browse files
committed
Address review comment from D97219 (follow up to 8051156)
Probably should have done this before landing, but I forgot. Basic idea is to avoid using the SCEV predicate when it doesn't buy us anything. Also happens to set us up for handling non-add recurrences in the future if desired.
1 parent 99f5417 commit 89d331a

File tree

1 file changed

+18
-13
lines changed

1 file changed

+18
-13
lines changed

llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
#include "llvm/Analysis/ScalarEvolutionNormalization.h"
7878
#include "llvm/Analysis/TargetLibraryInfo.h"
7979
#include "llvm/Analysis/TargetTransformInfo.h"
80+
#include "llvm/Analysis/ValueTracking.h"
8081
#include "llvm/Config/llvm-config.h"
8182
#include "llvm/IR/BasicBlock.h"
8283
#include "llvm/IR/Constant.h"
@@ -5570,33 +5571,37 @@ void LSRInstance::ImplementSolution(
55705571
// chosen a non-optimal result for the actual schedule. (And yes, this
55715572
// scheduling decision does impact later codegen.)
55725573
for (PHINode &PN : L->getHeader()->phis()) {
5573-
// First, filter out anything not an obvious addrec
5574-
if (!SE.isSCEVable(PN.getType()) || !isa<SCEVAddRecExpr>(SE.getSCEV(&PN)))
5575-
continue;
5576-
Instruction *IncV =
5577-
dyn_cast<Instruction>(PN.getIncomingValueForBlock(L->getLoopLatch()));
5578-
if (!IncV)
5574+
BinaryOperator *BO = nullptr;
5575+
Value *Start = nullptr, *Step = nullptr;
5576+
if (!matchSimpleRecurrence(&PN, BO, Start, Step))
55795577
continue;
55805578

5581-
if (IncV->getOpcode() != Instruction::Add &&
5582-
IncV->getOpcode() != Instruction::Sub)
5579+
switch (BO->getOpcode()) {
5580+
case Instruction::Sub:
5581+
if (BO->getOperand(0) != &PN)
5582+
// sub is non-commutative - match handling elsewhere in LSR
5583+
continue;
5584+
break;
5585+
case Instruction::Add:
5586+
break;
5587+
default:
55835588
continue;
5589+
};
55845590

5585-
if (IncV->getOperand(0) != &PN &&
5586-
!isa<Constant>(IncV->getOperand(1)))
5591+
if (!isa<Constant>(Step))
55875592
// If not a constant step, might increase register pressure
55885593
// (We assume constants have been canonicalized to RHS)
55895594
continue;
55905595

5591-
if (IncV->getParent() == IVIncInsertPos->getParent())
5596+
if (BO->getParent() == IVIncInsertPos->getParent())
55925597
// Only bother moving across blocks. Isel can handle block local case.
55935598
continue;
55945599

55955600
// Can we legally schedule inc at the desired point?
5596-
if (!llvm::all_of(IncV->uses(),
5601+
if (!llvm::all_of(BO->uses(),
55975602
[&](Use &U) {return DT.dominates(IVIncInsertPos, U);}))
55985603
continue;
5599-
IncV->moveBefore(IVIncInsertPos);
5604+
BO->moveBefore(IVIncInsertPos);
56005605
Changed = true;
56015606
}
56025607

0 commit comments

Comments
 (0)