Skip to content

[IVDesc] Use SCEVPatternMatch to improve code (NFC) #168397

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
+18 −26
Open

[IVDesc] Use SCEVPatternMatch to improve code (NFC) #168397

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b304148
[IVDesc] Use SCEVPatternMatch to improve code (NFC)
artagnon Nov 17, 2025
8749faa
[IVDesc] Address review
artagnon Nov 18, 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
44 changes: 18 additions & 26 deletions llvm/lib/Analysis/IVDescriptors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/ScalarEvolution.h"
#include "llvm/Analysis/ScalarEvolutionExpressions.h"
#include "llvm/Analysis/ScalarEvolutionPatternMatch.h"
#include "llvm/Analysis/ValueTracking.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/Instructions.h"
Expand All @@ -25,6 +26,7 @@

using namespace llvm;
using namespace llvm::PatternMatch;
using namespace llvm::SCEVPatternMatch;

#define DEBUG_TYPE "iv-descriptors"

Expand Down Expand Up @@ -719,11 +721,12 @@ RecurrenceDescriptor::isFindIVPattern(RecurKind Kind, Loop *TheLoop,
if (!SE.isSCEVable(Ty))
return std::nullopt;

auto *AR = dyn_cast<SCEVAddRecExpr>(SE.getSCEV(V));
if (!AR || AR->getLoop() != TheLoop)
auto *AR = SE.getSCEV(V);
const SCEV *Step;
if (!match(AR, m_scev_AffineAddRec(m_SCEV(), m_SCEV(Step),
m_SpecificLoop(TheLoop))))
return std::nullopt;

const SCEV *Step = AR->getStepRecurrence(SE);
Comment on lines -722 to -726
Copy link
Contributor Author

@artagnon artagnon Nov 18, 2025

Choose a reason for hiding this comment

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

This code isn't directly equivalent, as we're restricting to affine AddRecs, when we weren't previously. However, I think Find(First|Last)IV wouldn't work with a non-affine AddRec anyway.

if ((isFindFirstIVRecurrenceKind(Kind) && !SE.isKnownNegative(Step)) ||
(isFindLastIVRecurrenceKind(Kind) && !SE.isKnownPositive(Step)))
return std::nullopt;
Expand Down Expand Up @@ -1396,8 +1399,8 @@ InductionDescriptor::InductionDescriptor(Value *Start, InductionKind K,
}

ConstantInt *InductionDescriptor::getConstIntStepValue() const {
if (isa<SCEVConstant>(Step))
return dyn_cast<ConstantInt>(cast<SCEVConstant>(Step)->getValue());
if (auto *C = dyn_cast<SCEVConstant>(Step))
return dyn_cast<ConstantInt>(C->getValue());
return nullptr;
}

Expand Down Expand Up @@ -1614,41 +1617,30 @@ bool InductionDescriptor::isInductionPHI(

// Check that the PHI is consecutive.
const SCEV *PhiScev = Expr ? Expr : SE->getSCEV(Phi);
const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(PhiScev);
const SCEV *Step;

if (!AR) {
// FIXME: We are currently matching the specific loop TheLoop; if it doesn't
// match, we should treat it as a uniform. Unfortunately, we don't currently
// know how to handled uniform PHIs.
if (!match(PhiScev, m_scev_AffineAddRec(m_SCEV(), m_SCEV(Step),
m_SpecificLoop(TheLoop)))) {
LLVM_DEBUG(dbgs() << "LV: PHI is not a poly recurrence.\n");
return false;
}

if (AR->getLoop() != TheLoop) {
// FIXME: We should treat this as a uniform. Unfortunately, we
// don't currently know how to handled uniform PHIs.
LLVM_DEBUG(
dbgs() << "LV: PHI is a recurrence with respect to an outer loop.\n");
return false;
}

// This function assumes that InductionPhi is called only on Phi nodes
// present inside loop headers. Check for the same, and throw an assert if
// the current Phi is not present inside the loop header.
assert(Phi->getParent() == AR->getLoop()->getHeader()
&& "Invalid Phi node, not present in loop header");
assert(Phi->getParent() == TheLoop->getHeader() &&
"Invalid Phi node, not present in loop header");

Value *StartValue =
Phi->getIncomingValueForBlock(AR->getLoop()->getLoopPreheader());
Phi->getIncomingValueForBlock(TheLoop->getLoopPreheader());

BasicBlock *Latch = AR->getLoop()->getLoopLatch();
BasicBlock *Latch = TheLoop->getLoopLatch();
if (!Latch)
return false;

const SCEV *Step = AR->getStepRecurrence(*SE);
// Calculate the pointer stride and check if it is consecutive.
// The stride may be a constant or a loop invariant integer value.
const SCEVConstant *ConstStep = dyn_cast<SCEVConstant>(Step);
if (!ConstStep && !SE->isLoopInvariant(Step, TheLoop))
return false;
Comment on lines -1646 to -1650
Copy link
Contributor

@Mel-Chen Mel-Chen Nov 18, 2025

Choose a reason for hiding this comment

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

Could we drop this? Do m_SCEV(Step) guarantee this?

Copy link
Contributor Author

@artagnon artagnon Nov 18, 2025

Choose a reason for hiding this comment

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

Doesn't m_scev_AffineAddRec guarantee this?

Copy link
Contributor

@Mel-Chen Mel-Chen Nov 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think m_scev_AffineAddRec guarantee this. It should depend on m_SCEV(Step).
If we use m_SCEVConstant(Step), Step is SCEVConstant. But, we use m_SCEV(Step), so it should only get a SCEV, and not guarantee anything, right?

Copy link
Contributor Author

@artagnon artagnon Nov 18, 2025

Choose a reason for hiding this comment

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

From ScalarEvolutionExpressions:

  /// Return true if this represents an expression A + B*x where A
  /// and B are loop invariant values.
  bool isAffine() const {

They must be loop-invariant if the AddRec is affine.

Copy link
Contributor

@Mel-Chen Mel-Chen Nov 18, 2025

Choose a reason for hiding this comment

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

Hmm, this makes me wonder if const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(PhiScev); is equivalent to match(PhiScev, m_scev_AffineAddRec()). Could PhiScev originally have been a quadratic?

Copy link
Contributor Author

@artagnon artagnon Nov 18, 2025

Choose a reason for hiding this comment

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

It could have, but we'd have bailed out anyway when checking that the step is loop invariant? We're just bailing out ahead of time now, and this should be NFC?


if (PhiTy->isIntegerTy()) {
BinaryOperator *BOp =
dyn_cast<BinaryOperator>(Phi->getIncomingValueForBlock(Latch));
Expand Down
Loading