From 89d331a31e08a5bb4704789deb43746226fac8c0 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Wed, 3 Mar 2021 12:20:27 -0800 Subject: [PATCH] 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. --- .../Transforms/Scalar/LoopStrengthReduce.cpp | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp index 50aed8ceac9c0..3e26ef205d15d 100644 --- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp +++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp @@ -77,6 +77,7 @@ #include "llvm/Analysis/ScalarEvolutionNormalization.h" #include "llvm/Analysis/TargetLibraryInfo.h" #include "llvm/Analysis/TargetTransformInfo.h" +#include "llvm/Analysis/ValueTracking.h" #include "llvm/Config/llvm-config.h" #include "llvm/IR/BasicBlock.h" #include "llvm/IR/Constant.h" @@ -5570,33 +5571,37 @@ void LSRInstance::ImplementSolution( // chosen a non-optimal result for the actual schedule. (And yes, this // scheduling decision does impact later codegen.) for (PHINode &PN : L->getHeader()->phis()) { - // First, filter out anything not an obvious addrec - if (!SE.isSCEVable(PN.getType()) || !isa(SE.getSCEV(&PN))) - continue; - Instruction *IncV = - dyn_cast(PN.getIncomingValueForBlock(L->getLoopLatch())); - if (!IncV) + BinaryOperator *BO = nullptr; + Value *Start = nullptr, *Step = nullptr; + if (!matchSimpleRecurrence(&PN, BO, Start, Step)) continue; - if (IncV->getOpcode() != Instruction::Add && - IncV->getOpcode() != Instruction::Sub) + switch (BO->getOpcode()) { + case Instruction::Sub: + if (BO->getOperand(0) != &PN) + // sub is non-commutative - match handling elsewhere in LSR + continue; + break; + case Instruction::Add: + break; + default: continue; + }; - if (IncV->getOperand(0) != &PN && - !isa(IncV->getOperand(1))) + if (!isa(Step)) // If not a constant step, might increase register pressure // (We assume constants have been canonicalized to RHS) continue; - if (IncV->getParent() == IVIncInsertPos->getParent()) + if (BO->getParent() == IVIncInsertPos->getParent()) // Only bother moving across blocks. Isel can handle block local case. continue; // Can we legally schedule inc at the desired point? - if (!llvm::all_of(IncV->uses(), + if (!llvm::all_of(BO->uses(), [&](Use &U) {return DT.dominates(IVIncInsertPos, U);})) continue; - IncV->moveBefore(IVIncInsertPos); + BO->moveBefore(IVIncInsertPos); Changed = true; }