[llvm] [HashRecognize] Strip ValueEvolution (PR #148620)
Ramkumar Ramachandra via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 2 10:12:41 PDT 2025
https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/148620
>From 9de50b0b0a6936e9d5db6d47b62413ef73b76559 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 14 Jul 2025 12:20:35 +0100
Subject: [PATCH 1/5] [HashRecognize] Fix LHS ConstantRange check for BE
The ConstantRange of the LHS of the ICmp in the Select(ICmp())
significant-bit check is not checked in the big-endian case, leading to
a potential miscompile. Fix this.
Co-authored-by: Piotr Fusik <p.fusik at samsung.com>
---
llvm/lib/Analysis/HashRecognize.cpp | 62 ++++++++++++-------
.../HashRecognize/cyclic-redundancy-check.ll | 37 ++++++++---
2 files changed, 68 insertions(+), 31 deletions(-)
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index 92c9e37dbb484..99ca3cf75b666 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -90,6 +90,7 @@ class ValueEvolution {
const bool ByteOrderSwapped;
APInt GenPoly;
StringRef ErrStr;
+ unsigned AtIter;
// Compute the KnownBits of a BinaryOperator.
KnownBits computeBinOp(const BinaryOperator *I);
@@ -190,7 +191,7 @@ KnownBits ValueEvolution::computeInstr(const Instruction *I) {
return KnownPhis.lookup_or(P, BitWidth);
// Compute the KnownBits for a Select(Cmp()), forcing it to take the branch
- // that is predicated on the (least|most)-significant-bit check.
+ // that is (least|most)-significant-bit-clear.
CmpPredicate Pred;
Value *L, *R;
Instruction *TV, *FV;
@@ -198,35 +199,46 @@ KnownBits ValueEvolution::computeInstr(const Instruction *I) {
m_Instruction(FV)))) {
Visited.insert(cast<Instruction>(I->getOperand(0)));
- // We need to check LCR against [0, 2) in the little-endian case, because
- // the RCR check is insufficient: it is simply [0, 1).
- if (!ByteOrderSwapped) {
- KnownBits KnownL = compute(L);
- unsigned ICmpBW = KnownL.getBitWidth();
- auto LCR = ConstantRange::fromKnownBits(KnownL, false);
- auto CheckLCR = ConstantRange(APInt::getZero(ICmpBW), APInt(ICmpBW, 2));
- if (LCR != CheckLCR) {
- ErrStr = "Bad LHS of significant-bit-check";
- return {BitWidth};
- }
+ // Check that the predication is on (most|least) significant bit. We check
+ // that the compare is `>= 0` in the big-endian case, and `== 0` in the
+ // little-endian case (or the inverse, in which case the branches of the
+ // compare are swapped). We check LCR against CheckLCR, which is full-set,
+ // [0, -1), [0, -3), [0, -7), etc. depending on AtIter in the big-endian
+ // case, and [0, 2) in the little-endian case: CheckLCR checks that we are
+ // shifting in zero bits in every loop iteration in the big-endian case, and
+ // the compare 0 or 1 in the little-endian case (as a value and'ed with 1 is
+ // passed as the operand). We then check AllowedByR against CheckAllowedByR,
+ // which is [0, smin) in the big-endian case, and [0, 1) in the
+ // little-endian case: CheckAllowedByR checks for significant-bit-clear,
+ // which means that we visit the bit-shift branch instead of the
+ // bit-shift-and-xor-poly branch.
+ KnownBits KnownL = compute(L);
+ unsigned ICmpBW = KnownL.getBitWidth();
+ auto LCR = ConstantRange::fromKnownBits(KnownL, false);
+ auto CheckLCR = ConstantRange::getNonEmpty(
+ APInt::getZero(ICmpBW), ByteOrderSwapped
+ ? -APInt::getLowBitsSet(ICmpBW, AtIter)
+ : APInt(ICmpBW, 2));
+ if (LCR != CheckLCR) {
+ ErrStr = "Bad LHS of significant-bit-check";
+ return {BitWidth};
}
- // Check that the predication is on (most|least) significant bit.
KnownBits KnownR = compute(R);
- unsigned ICmpBW = KnownR.getBitWidth();
auto RCR = ConstantRange::fromKnownBits(KnownR, false);
- auto AllowedR = ConstantRange::makeAllowedICmpRegion(Pred, RCR);
- ConstantRange CheckRCR(APInt::getZero(ICmpBW),
- ByteOrderSwapped ? APInt::getSignedMinValue(ICmpBW)
- : APInt(ICmpBW, 1));
-
- // We only compute KnownBits of either TV or FV, as the other value would
- // just be a bit-shift as checked by isBigEndianBitShift.
- if (AllowedR == CheckRCR) {
+ auto AllowedByR = ConstantRange::makeAllowedICmpRegion(Pred, RCR);
+ ConstantRange CheckAllowedByR(
+ APInt::getZero(ICmpBW),
+ ByteOrderSwapped ? APInt::getSignedMinValue(ICmpBW) : APInt(ICmpBW, 1));
+
+ // We only compute KnownBits of either TV or FV, as the other branch would
+ // be the bit-shift-and-xor-poly branch, as determined by the conditional
+ // recurrence.
+ if (AllowedByR == CheckAllowedByR) {
Visited.insert(FV);
return compute(TV);
}
- if (AllowedR.inverse() == CheckRCR) {
+ if (AllowedByR.inverse() == CheckAllowedByR) {
Visited.insert(TV);
return compute(FV);
}
@@ -264,9 +276,11 @@ KnownBits ValueEvolution::compute(const Value *V) {
}
bool ValueEvolution::computeEvolutions(ArrayRef<PhiStepPair> PhiEvolutions) {
- for (unsigned I = 0; I < TripCount; ++I)
+ for (unsigned I = 0; I < TripCount; ++I) {
+ AtIter = I;
for (auto [Phi, Step] : PhiEvolutions)
KnownPhis.emplace_or_assign(Phi, computeInstr(Step));
+ }
return ErrStr.empty();
}
diff --git a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
index fe140d01e8818..8ebe6eaaaf0be 100644
--- a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
+++ b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
@@ -649,7 +649,7 @@ exit: ; preds = %loop
define i16 @not.crc.wrong.sb.check.const(i8 %msg, i16 %checksum) {
; CHECK-LABEL: 'not.crc.wrong.sb.check.const'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Bad RHS of significant-bit-check
+; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
;
entry:
br label %loop
@@ -676,7 +676,7 @@ exit: ; preds = %loop
define i16 @not.crc.wrong.sb.check.pred(i16 %crc.init) {
; CHECK-LABEL: 'not.crc.wrong.sb.check.pred'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Bad RHS of significant-bit-check
+; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
;
entry:
br label %loop
@@ -857,10 +857,10 @@ exit: ; preds = %loop
ret i16 %crc.next
}
-define i16 @crc1.tc8.sb.check.endian.mismatch(i8 %msg, i16 %checksum) {
-; CHECK-LABEL: 'crc1.tc8.sb.check.endian.mismatch'
+define i16 @not.crc.sb.check.endian.mismatch(i8 %msg, i16 %checksum) {
+; CHECK-LABEL: 'not.crc.sb.check.endian.mismatch'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Bad RHS of significant-bit-check
+; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
;
entry:
br label %loop
@@ -912,7 +912,7 @@ exit: ; preds = %loop
define i16 @not.crc.bad.endian.swapped.sb.check(i8 %msg, i16 %checksum) {
; CHECK-LABEL: 'not.crc.bad.endian.swapped.sb.check'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Found stray unvisited instructions
+; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
;
entry:
br label %loop
@@ -1219,7 +1219,7 @@ declare void @print(i16)
define i16 @not.crc.call.sb.check(i16 %crc.init) {
; CHECK-LABEL: 'not.crc.call.sb.check'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Found stray unvisited instructions
+; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
;
entry:
br label %loop
@@ -1241,3 +1241,26 @@ exit: ; preds = %loop
}
declare i16 @side.effect()
+
+define i16 @not.crc.bad.lhs.sb.check.be(i16 %crc.init) {
+; CHECK-LABEL: 'not.crc.bad.lhs.sb.check.be'
+; CHECK-NEXT: Did not find a hash algorithm
+; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
+;
+entry:
+ br label %loop
+
+loop: ; preds = %loop, %entry
+ %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ]
+ %crc = phi i16 [ %crc.init, %entry ], [ %crc.next, %loop ]
+ %crc.shl = shl i16 %crc, 1
+ %crc.xor = xor i16 %crc.shl, 4129
+ %check.sb = icmp slt i16 %crc.shl, 0
+ %crc.next = select i1 %check.sb, i16 %crc.xor, i16 %crc.shl
+ %iv.next = add nuw nsw i32 %iv, 1
+ %exit.cond = icmp samesign ult i32 %iv, 7
+ br i1 %exit.cond, label %loop, label %exit
+
+exit: ; preds = %loop
+ ret i16 %crc.next
+}
>From 622e66b49604316fb0f7274fc5372a986006966b Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Tue, 5 Aug 2025 11:01:04 +0100
Subject: [PATCH 2/5] [HashRecognize] Strip ValueEvolution
Co-authored-by: Piotr Fusik <p.fusik at samsung.com>
---
llvm/include/llvm/Analysis/HashRecognize.h | 8 +-
llvm/lib/Analysis/HashRecognize.cpp | 347 +++++-------------
.../HashRecognize/cyclic-redundancy-check.ll | 140 ++++++-
3 files changed, 211 insertions(+), 284 deletions(-)
diff --git a/llvm/include/llvm/Analysis/HashRecognize.h b/llvm/include/llvm/Analysis/HashRecognize.h
index 0361dfcd23528..6dea3d24885ff 100644
--- a/llvm/include/llvm/Analysis/HashRecognize.h
+++ b/llvm/include/llvm/Analysis/HashRecognize.h
@@ -20,18 +20,12 @@
#include "llvm/Analysis/ScalarEvolution.h"
#include "llvm/IR/PassManager.h"
#include "llvm/IR/Value.h"
-#include "llvm/Support/KnownBits.h"
#include <variant>
namespace llvm {
class LPMUpdater;
-/// A tuple of bits that are expected to be zero, number N of them expected to
-/// be zero, with a boolean indicating whether it's the top or bottom N bits
-/// expected to be zero.
-using ErrBits = std::tuple<KnownBits, unsigned, bool>;
-
/// A custom std::array with 256 entries, that also has a print function.
struct CRCTable : public std::array<APInt, 256> {
void print(raw_ostream &OS) const;
@@ -85,7 +79,7 @@ class HashRecognize {
HashRecognize(const Loop &L, ScalarEvolution &SE);
// The main analysis entry points.
- std::variant<PolynomialInfo, ErrBits, StringRef> recognizeCRC() const;
+ std::variant<PolynomialInfo, StringRef> recognizeCRC() const;
std::optional<PolynomialInfo> getResult() const;
// Auxilary entry point after analysis to interleave the generating polynomial
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index 99ca3cf75b666..49fb07aa51a5c 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -73,216 +73,86 @@ using namespace SCEVPatternMatch;
#define DEBUG_TYPE "hash-recognize"
-// KnownBits for a PHI node. There are at most two PHI nodes, corresponding to
-// the Simple Recurrence and Conditional Recurrence. The IndVar PHI is not
-// relevant.
-using KnownPhiMap = SmallDenseMap<const PHINode *, KnownBits, 2>;
-
-// A pair of a PHI node along with its incoming value from within a loop.
-using PhiStepPair = std::pair<const PHINode *, const Instruction *>;
-
-/// A much simpler version of ValueTracking, in that it computes KnownBits of
-/// values, except that it computes the evolution of KnownBits in a loop with a
-/// given trip count, and predication is specialized for a significant-bit
-/// check.
-class ValueEvolution {
- const unsigned TripCount;
- const bool ByteOrderSwapped;
- APInt GenPoly;
- StringRef ErrStr;
- unsigned AtIter;
-
- // Compute the KnownBits of a BinaryOperator.
- KnownBits computeBinOp(const BinaryOperator *I);
-
- // Compute the KnownBits of an Instruction.
- KnownBits computeInstr(const Instruction *I);
-
- // Compute the KnownBits of a Value.
- KnownBits compute(const Value *V);
-
-public:
- // ValueEvolution is meant to be constructed with the TripCount of the loop,
- // and a boolean indicating whether the polynomial algorithm is big-endian
- // (for the significant-bit check).
- ValueEvolution(unsigned TripCount, bool ByteOrderSwapped);
-
- // Given a list of PHI nodes along with their incoming value from within the
- // loop, computeEvolutions computes the KnownBits of each of the PHI nodes on
- // the final iteration. Returns true on success and false on error.
- bool computeEvolutions(ArrayRef<PhiStepPair> PhiEvolutions);
-
- // In case ValueEvolution encounters an error, this is meant to be used for a
- // precise error message.
- StringRef getError() const { return ErrStr; }
-
- // A set of Instructions visited by ValueEvolution. The only unvisited
- // instructions will be ones not on the use-def chain of the PHIs' evolutions.
- SmallPtrSet<const Instruction *, 16> Visited;
-
- // The computed KnownBits for each PHI node, which is populated after
- // computeEvolutions is called.
- KnownPhiMap KnownPhis;
-};
-
-ValueEvolution::ValueEvolution(unsigned TripCount, bool ByteOrderSwapped)
- : TripCount(TripCount), ByteOrderSwapped(ByteOrderSwapped) {}
-
-KnownBits ValueEvolution::computeBinOp(const BinaryOperator *I) {
- KnownBits KnownL(compute(I->getOperand(0)));
- KnownBits KnownR(compute(I->getOperand(1)));
-
- switch (I->getOpcode()) {
- case Instruction::BinaryOps::And:
- return KnownL & KnownR;
- case Instruction::BinaryOps::Or:
- return KnownL | KnownR;
- case Instruction::BinaryOps::Xor:
- return KnownL ^ KnownR;
- case Instruction::BinaryOps::Shl: {
- auto *OBO = cast<OverflowingBinaryOperator>(I);
- return KnownBits::shl(KnownL, KnownR, OBO->hasNoUnsignedWrap(),
- OBO->hasNoSignedWrap());
- }
- case Instruction::BinaryOps::LShr:
- return KnownBits::lshr(KnownL, KnownR);
- case Instruction::BinaryOps::AShr:
- return KnownBits::ashr(KnownL, KnownR);
- case Instruction::BinaryOps::Add: {
- auto *OBO = cast<OverflowingBinaryOperator>(I);
- return KnownBits::add(KnownL, KnownR, OBO->hasNoUnsignedWrap(),
- OBO->hasNoSignedWrap());
- }
- case Instruction::BinaryOps::Sub: {
- auto *OBO = cast<OverflowingBinaryOperator>(I);
- return KnownBits::sub(KnownL, KnownR, OBO->hasNoUnsignedWrap(),
- OBO->hasNoSignedWrap());
- }
- case Instruction::BinaryOps::Mul: {
- Value *Op0 = I->getOperand(0);
- Value *Op1 = I->getOperand(1);
- bool SelfMultiply = Op0 == Op1 && isGuaranteedNotToBeUndef(Op0);
- return KnownBits::mul(KnownL, KnownR, SelfMultiply);
- }
- case Instruction::BinaryOps::UDiv:
- return KnownBits::udiv(KnownL, KnownR);
- case Instruction::BinaryOps::SDiv:
- return KnownBits::sdiv(KnownL, KnownR);
- case Instruction::BinaryOps::URem:
- return KnownBits::urem(KnownL, KnownR);
- case Instruction::BinaryOps::SRem:
- return KnownBits::srem(KnownL, KnownR);
- default:
- ErrStr = "Unknown BinaryOperator";
- unsigned BitWidth = I->getType()->getScalarSizeInBits();
- return {BitWidth};
- }
-}
-
-KnownBits ValueEvolution::computeInstr(const Instruction *I) {
- unsigned BitWidth = I->getType()->getScalarSizeInBits();
-
- // computeInstr is the only entry-point that needs to update the Visited set.
- Visited.insert(I);
-
- // We look up in the map that contains the KnownBits of the PHI from the
- // previous iteration.
- if (const PHINode *P = dyn_cast<PHINode>(I))
- return KnownPhis.lookup_or(P, BitWidth);
-
- // Compute the KnownBits for a Select(Cmp()), forcing it to take the branch
- // that is (least|most)-significant-bit-clear.
+/// Check the well-formedness of the (most|least) significant bit check \p SI,
+/// where \p BitShift is the bit-shift branch: the other branch must be a
+/// bit-shift-and-xor-poly branch, as already checked by
+/// matchConditionalRecurrence. We check that the compare is `>= 0` in the
+/// big-endian case, and `== 0` in the little-endian case (or the inverse, in
+/// which case the branches of the compare are swapped). We check LCR against
+/// CheckLCR, which is full-set in the big-endian case, and [0, 2) in the
+/// little-endian case: CheckLCR checks that the comparison is `>= 0` in the
+/// big-endian case, and that the compare is to 0 or 1 in the little-endian case
+/// (as a value and'ed with 1 is passed as the operand). We then check
+/// AllowedByR against CheckAllowedByR, which is [0, smin) in the big-endian
+/// case, and [0, 1) in the little-endian case: CheckAllowedByR checks for
+/// significant-bit-clear, and this must be equal to \p BitShift for
+/// well-formedness.
+static bool isSignificantBitCheckWellFormed(const SelectInst *SI,
+ const BinaryOperator *BitShift,
+ bool ByteOrderSwapped) {
+ DataLayout DL = SI->getParent()->getDataLayout();
CmpPredicate Pred;
Value *L, *R;
Instruction *TV, *FV;
- if (match(I, m_Select(m_ICmp(Pred, m_Value(L), m_Value(R)), m_Instruction(TV),
- m_Instruction(FV)))) {
- Visited.insert(cast<Instruction>(I->getOperand(0)));
-
- // Check that the predication is on (most|least) significant bit. We check
- // that the compare is `>= 0` in the big-endian case, and `== 0` in the
- // little-endian case (or the inverse, in which case the branches of the
- // compare are swapped). We check LCR against CheckLCR, which is full-set,
- // [0, -1), [0, -3), [0, -7), etc. depending on AtIter in the big-endian
- // case, and [0, 2) in the little-endian case: CheckLCR checks that we are
- // shifting in zero bits in every loop iteration in the big-endian case, and
- // the compare 0 or 1 in the little-endian case (as a value and'ed with 1 is
- // passed as the operand). We then check AllowedByR against CheckAllowedByR,
- // which is [0, smin) in the big-endian case, and [0, 1) in the
- // little-endian case: CheckAllowedByR checks for significant-bit-clear,
- // which means that we visit the bit-shift branch instead of the
- // bit-shift-and-xor-poly branch.
- KnownBits KnownL = compute(L);
- unsigned ICmpBW = KnownL.getBitWidth();
- auto LCR = ConstantRange::fromKnownBits(KnownL, false);
- auto CheckLCR = ConstantRange::getNonEmpty(
- APInt::getZero(ICmpBW), ByteOrderSwapped
- ? -APInt::getLowBitsSet(ICmpBW, AtIter)
- : APInt(ICmpBW, 2));
- if (LCR != CheckLCR) {
- ErrStr = "Bad LHS of significant-bit-check";
- return {BitWidth};
- }
-
- KnownBits KnownR = compute(R);
- auto RCR = ConstantRange::fromKnownBits(KnownR, false);
- auto AllowedByR = ConstantRange::makeAllowedICmpRegion(Pred, RCR);
- ConstantRange CheckAllowedByR(
- APInt::getZero(ICmpBW),
- ByteOrderSwapped ? APInt::getSignedMinValue(ICmpBW) : APInt(ICmpBW, 1));
-
- // We only compute KnownBits of either TV or FV, as the other branch would
- // be the bit-shift-and-xor-poly branch, as determined by the conditional
- // recurrence.
- if (AllowedByR == CheckAllowedByR) {
- Visited.insert(FV);
- return compute(TV);
- }
- if (AllowedByR.inverse() == CheckAllowedByR) {
- Visited.insert(TV);
- return compute(FV);
- }
-
- ErrStr = "Bad RHS of significant-bit-check";
- return {BitWidth};
- }
+ [[maybe_unused]] bool Match =
+ match(SI, m_Select(m_ICmp(Pred, m_Value(L), m_Value(R)),
+ m_Instruction(TV), m_Instruction(FV)));
+ assert(Match && "Select(ICmp()) expected in signficant-bit-check");
+
+ KnownBits KnownL = computeKnownBits(L, DL);
+ unsigned ICmpBW = KnownL.getBitWidth();
+ auto LCR = ConstantRange::fromKnownBits(KnownL, false);
+ auto CheckLCR = ConstantRange::getNonEmpty(
+ APInt::getZero(ICmpBW),
+ ByteOrderSwapped ? APInt::getZero(ICmpBW) : APInt(ICmpBW, 2));
+ if (LCR != CheckLCR)
+ return false;
- if (auto *BO = dyn_cast<BinaryOperator>(I))
- return computeBinOp(BO);
-
- switch (I->getOpcode()) {
- case Instruction::CastOps::Trunc:
- return compute(I->getOperand(0)).trunc(BitWidth);
- case Instruction::CastOps::ZExt:
- return compute(I->getOperand(0)).zext(BitWidth);
- case Instruction::CastOps::SExt:
- return compute(I->getOperand(0)).sext(BitWidth);
- default:
- ErrStr = "Unknown Instruction";
- return {BitWidth};
- }
+ KnownBits KnownR = computeKnownBits(R, DL);
+ auto RCR = ConstantRange::fromKnownBits(KnownR, false);
+ auto AllowedByR = ConstantRange::makeAllowedICmpRegion(Pred, RCR);
+ ConstantRange CheckAllowedByR(
+ APInt::getZero(ICmpBW),
+ ByteOrderSwapped ? APInt::getSignedMinValue(ICmpBW) : APInt(ICmpBW, 1));
+
+ if (AllowedByR == CheckAllowedByR)
+ return TV == BitShift;
+ if (AllowedByR.inverse() == CheckAllowedByR)
+ return FV == BitShift;
+ return false;
}
-KnownBits ValueEvolution::compute(const Value *V) {
- if (auto *CI = dyn_cast<ConstantInt>(V))
- return KnownBits::makeConstant(CI->getValue());
+/// Checks if Loop \p L contains instructions unreachable or unhandled from \p
+/// Roots on the use-def chain.
+static bool
+containsUnreachableOrUnhandled(const Loop &L,
+ ArrayRef<const Instruction *> Roots) {
+ SmallPtrSet<const Instruction *, 16> Visited;
+ BasicBlock *Latch = L.getLoopLatch();
- if (auto *I = dyn_cast<Instruction>(V))
- return computeInstr(I);
+ SmallVector<const Instruction *, 16> Worklist(Roots);
+ while (!Worklist.empty()) {
+ const Instruction *I = Worklist.pop_back_val();
+ Visited.insert(I);
- ErrStr = "Unknown Value";
- unsigned BitWidth = V->getType()->getScalarSizeInBits();
- return {BitWidth};
-}
+ if (isa<PHINode>(I))
+ continue;
-bool ValueEvolution::computeEvolutions(ArrayRef<PhiStepPair> PhiEvolutions) {
- for (unsigned I = 0; I < TripCount; ++I) {
- AtIter = I;
- for (auto [Phi, Step] : PhiEvolutions)
- KnownPhis.emplace_or_assign(Phi, computeInstr(Step));
- }
+ if (!isa<TruncInst, BinaryOperator, SelectInst, CmpInst, BranchInst>(I))
+ return true;
- return ErrStr.empty();
+ for (const Use &U : I->operands()) {
+ if (auto *UI = dyn_cast<Instruction>(U)) {
+ if (!L.contains(UI))
+ return true;
+ Worklist.push_back(UI);
+ continue;
+ }
+ if (!isa<ConstantInt, BasicBlock>(U))
+ return true;
+ }
+ }
+ return std::distance(Latch->begin(), Latch->end()) != Visited.size();
}
/// A structure that can hold either a Simple Recurrence or a Conditional
@@ -473,26 +343,6 @@ PolynomialInfo::PolynomialInfo(unsigned TripCount, Value *LHS, const APInt &RHS,
: TripCount(TripCount), LHS(LHS), RHS(RHS), ComputedValue(ComputedValue),
ByteOrderSwapped(ByteOrderSwapped), LHSAux(LHSAux) {}
-/// In the big-endian case, checks the bottom N bits against CheckFn, and that
-/// the rest are unknown. In the little-endian case, checks the top N bits
-/// against CheckFn, and that the rest are unknown. Callers usually call this
-/// function with N = TripCount, and CheckFn checking that the remainder bits of
-/// the CRC polynomial division are zero.
-static bool checkExtractBits(const KnownBits &Known, unsigned N,
- function_ref<bool(const KnownBits &)> CheckFn,
- bool ByteOrderSwapped) {
- // Check that the entire thing is a constant.
- if (N == Known.getBitWidth())
- return CheckFn(Known.extractBits(N, 0));
-
- // Check that the {top, bottom} N bits are not unknown and that the {bottom,
- // top} N bits are known.
- unsigned BitPos = ByteOrderSwapped ? 0 : Known.getBitWidth() - N;
- unsigned SwappedBitPos = ByteOrderSwapped ? N : 0;
- return CheckFn(Known.extractBits(N, BitPos)) &&
- Known.extractBits(Known.getBitWidth() - N, SwappedBitPos).isUnknown();
-}
-
/// Generate a lookup table of 256 entries by interleaving the generating
/// polynomial. The optimization technique of table-lookup for CRC is also
/// called the Sarwate algorithm.
@@ -525,9 +375,7 @@ CRCTable HashRecognize::genSarwateTable(const APInt &GenPoly,
/// Checks that \p P1 and \p P2 are used together in an XOR in the use-def chain
/// of \p SI's condition, ignoring any casts. The purpose of this function is to
/// ensure that LHSAux from the SimpleRecurrence is used correctly in the CRC
-/// computation. We cannot check the correctness of casts at this point, and
-/// rely on the KnownBits propagation to check correctness of the CRC
-/// computation.
+/// computation. We cannot check the correctness of casts at this point.
///
/// In other words, it checks for the following pattern:
///
@@ -584,10 +432,8 @@ static std::optional<bool> isBigEndianBitShift(Value *V, ScalarEvolution &SE) {
}
/// The main entry point for analyzing a loop and recognizing the CRC algorithm.
-/// Returns a PolynomialInfo on success, and either an ErrBits or a StringRef on
-/// failure.
-std::variant<PolynomialInfo, ErrBits, StringRef>
-HashRecognize::recognizeCRC() const {
+/// Returns a PolynomialInfo on success, and a StringRef on failure.
+std::variant<PolynomialInfo, StringRef> HashRecognize::recognizeCRC() const {
if (!L.isInnermost())
return "Loop is not innermost";
BasicBlock *Latch = L.getLoopLatch();
@@ -651,35 +497,19 @@ HashRecognize::recognizeCRC() const {
"Expected ExtraConst in conditional recurrence");
const APInt &GenPoly = *ConditionalRecurrence.ExtraConst;
- // PhiEvolutions are pairs of PHINodes along with their incoming value from
- // within the loop, which we term as their step. Note that in the case of a
- // Simple Recurrence, Step is an operand of the BO, while in a Conditional
- // Recurrence, it is a SelectInst.
- SmallVector<PhiStepPair, 2> PhiEvolutions;
- PhiEvolutions.emplace_back(ConditionalRecurrence.Phi, ComputedValue);
+ if (!isSignificantBitCheckWellFormed(
+ cast<SelectInst>(ConditionalRecurrence.Step),
+ ConditionalRecurrence.BO, *ByteOrderSwapped))
+ return "Malformed significant-bit check";
+
+ SmallVector<const Instruction *> Roots(
+ {ComputedValue,
+ cast<Instruction>(IndVar->getIncomingValueForBlock(Latch)),
+ L.getLatchCmpInst(), Latch->getTerminator()});
if (SimpleRecurrence)
- PhiEvolutions.emplace_back(SimpleRecurrence.Phi, SimpleRecurrence.BO);
-
- ValueEvolution VE(TC, *ByteOrderSwapped);
- if (!VE.computeEvolutions(PhiEvolutions))
- return VE.getError();
- KnownBits ResultBits = VE.KnownPhis.at(ConditionalRecurrence.Phi);
-
- // There must be exactly four unvisited instructions, corresponding to the
- // IndVar PHI. Any other unvisited instructions from the KnownBits propagation
- // can complicate the optimization, which replaces the entire loop with the
- // table-lookup version of the hash algorithm.
- std::initializer_list<const Instruction *> AugmentVisited = {
- IndVar, Latch->getTerminator(), L.getLatchCmpInst(),
- cast<Instruction>(IndVar->getIncomingValueForBlock(Latch))};
- VE.Visited.insert_range(AugmentVisited);
- if (std::distance(Latch->begin(), Latch->end()) != VE.Visited.size())
- return "Found stray unvisited instructions";
-
- unsigned N = std::min(TC, ResultBits.getBitWidth());
- auto IsZero = [](const KnownBits &K) { return K.isZero(); };
- if (!checkExtractBits(ResultBits, N, IsZero, *ByteOrderSwapped))
- return ErrBits(ResultBits, TC, *ByteOrderSwapped);
+ Roots.push_back(SimpleRecurrence.BO);
+ if (containsUnreachableOrUnhandled(L, Roots))
+ return "Found stray unvisited or unhandled instructions";
return PolynomialInfo(TC, LHS, GenPoly, ComputedValue, *ByteOrderSwapped,
LHSAux);
@@ -707,13 +537,6 @@ void HashRecognize::print(raw_ostream &OS) const {
OS << "Did not find a hash algorithm\n";
if (std::holds_alternative<StringRef>(Ret))
OS << "Reason: " << std::get<StringRef>(Ret) << "\n";
- if (std::holds_alternative<ErrBits>(Ret)) {
- auto [Actual, Iter, ByteOrderSwapped] = std::get<ErrBits>(Ret);
- OS << "Reason: Expected " << (ByteOrderSwapped ? "bottom " : "top ")
- << Iter << " bits zero (";
- Actual.print(OS);
- OS << ")\n";
- }
return;
}
diff --git a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
index 8ebe6eaaaf0be..48464e69841f1 100644
--- a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
+++ b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
@@ -649,7 +649,7 @@ exit: ; preds = %loop
define i16 @not.crc.wrong.sb.check.const(i8 %msg, i16 %checksum) {
; CHECK-LABEL: 'not.crc.wrong.sb.check.const'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
+; CHECK-NEXT: Reason: Malformed significant-bit check
;
entry:
br label %loop
@@ -676,7 +676,7 @@ exit: ; preds = %loop
define i16 @not.crc.wrong.sb.check.pred(i16 %crc.init) {
; CHECK-LABEL: 'not.crc.wrong.sb.check.pred'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
+; CHECK-NEXT: Reason: Malformed significant-bit check
;
entry:
br label %loop
@@ -750,7 +750,7 @@ exit: ; preds = %loop
define i32 @not.crc.unknown.icmp.rhs(i32 %checksum, i32 %msg, i32 %unknown) {
; CHECK-LABEL: 'not.crc.unknown.icmp.rhs'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
+; CHECK-NEXT: Reason: Malformed significant-bit check
;
entry:
br label %loop
@@ -777,7 +777,7 @@ exit: ; preds = %loop
define i32 @not.crc.unknown.icmp.lhs(i32 %checksum, i32 %msg, i32 %unknown) {
; CHECK-LABEL: 'not.crc.unknown.icmp.lhs'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
+; CHECK-NEXT: Reason: Malformed significant-bit check
;
entry:
br label %loop
@@ -805,7 +805,7 @@ exit: ; preds = %loop
define i16 @not.crc.stray.or(i16 %msg, i16 %checksum) {
; CHECK-LABEL: 'not.crc.stray.or'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
+; CHECK-NEXT: Reason: Malformed significant-bit check
;
entry:
br label %loop
@@ -833,7 +833,7 @@ exit: ; preds = %loop
define i16 @not.crc.inverse.sb.check(i16 %msg, i16 %checksum) {
; CHECK-LABEL: 'not.crc.inverse.sb.check'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Expected top 16 bits zero (1100000000000001)
+; CHECK-NEXT: Reason: Malformed significant-bit check
;
entry:
br label %loop
@@ -860,7 +860,7 @@ exit: ; preds = %loop
define i16 @not.crc.sb.check.endian.mismatch(i8 %msg, i16 %checksum) {
; CHECK-LABEL: 'not.crc.sb.check.endian.mismatch'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
+; CHECK-NEXT: Reason: Malformed significant-bit check
;
entry:
br label %loop
@@ -888,7 +888,7 @@ exit: ; preds = %loop
define i16 @not.crc.init.arg.inverted.select(i16 %crc.init) {
; CHECK-LABEL: 'not.crc.init.arg.inverted.select'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Expected top 8 bits zero (11000000????????)
+; CHECK-NEXT: Reason: Malformed significant-bit check
;
entry:
br label %loop
@@ -912,7 +912,7 @@ exit: ; preds = %loop
define i16 @not.crc.bad.endian.swapped.sb.check(i8 %msg, i16 %checksum) {
; CHECK-LABEL: 'not.crc.bad.endian.swapped.sb.check'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
+; CHECK-NEXT: Reason: Malformed significant-bit check
;
entry:
br label %loop
@@ -1109,7 +1109,7 @@ exit: ; preds = %loop
define i16 @not.crc.unknown.value(i16 %msg, i16 %checksum, i16 %corrupt) {
; CHECK-LABEL: 'not.crc.unknown.value'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Unknown Value
+; CHECK-NEXT: Reason: Found stray unvisited or unhandled instructions
;
entry:
br label %loop
@@ -1134,6 +1134,63 @@ exit: ; preds = %loop
ret i16 %crc.next
}
+define i16 @not.crc.unknown.call.outside.loop(i16 %msg, i16 %checksum) {
+; CHECK-LABEL: 'not.crc.unknown.call.outside.loop'
+; CHECK-NEXT: Did not find a hash algorithm
+; CHECK-NEXT: Reason: Found stray unvisited or unhandled instructions
+;
+entry:
+ %corrupt = call i16 @side.effect()
+ br label %loop
+
+loop: ; preds = %loop, %entry
+ %iv = phi i8 [ 0, %entry ], [ %iv.next, %loop ]
+ %crc = phi i16 [ %checksum, %entry ], [ %crc.next, %loop ]
+ %data = phi i16 [ %msg, %entry ], [ %data.next, %loop ]
+ %xor.crc.data = xor i16 %crc, %data
+ %xor.crc.data.corrupt = mul i16 %xor.crc.data, %corrupt
+ %and.crc.data = and i16 %xor.crc.data.corrupt, 1
+ %data.next = lshr i16 %data, 1
+ %check.sb = icmp eq i16 %and.crc.data, 0
+ %crc.lshr = lshr i16 %crc, 1
+ %crc.xor = xor i16 %crc.lshr, -24575
+ %crc.next = select i1 %check.sb, i16 %crc.lshr, i16 %crc.xor
+ %iv.next = add nuw nsw i8 %iv, 1
+ %exit.cond = icmp samesign ult i8 %iv, 15
+ br i1 %exit.cond, label %loop, label %exit
+
+exit: ; preds = %loop
+ ret i16 %crc.next
+}
+
+define i16 @not.crc.constant.sb.check.corruption(i16 %msg, i16 %checksum) {
+; CHECK-LABEL: 'not.crc.constant.sb.check.corruption'
+; CHECK-NEXT: Did not find a hash algorithm
+; CHECK-NEXT: Reason: Malformed significant-bit check
+;
+entry:
+ br label %loop
+
+loop: ; preds = %loop, %entry
+ %iv = phi i8 [ 0, %entry ], [ %iv.next, %loop ]
+ %crc = phi i16 [ %checksum, %entry ], [ %crc.next, %loop ]
+ %data = phi i16 [ %msg, %entry ], [ %data.next, %loop ]
+ %xor.crc.data = xor i16 %crc, %data
+ %xor.crc.data.corrupt = mul i16 %xor.crc.data, 2
+ %and.crc.data = and i16 %xor.crc.data.corrupt, 1
+ %data.next = lshr i16 %data, 1
+ %check.sb = icmp eq i16 %and.crc.data, 0
+ %crc.lshr = lshr i16 %crc, 1
+ %crc.xor = xor i16 %crc.lshr, -24575
+ %crc.next = select i1 %check.sb, i16 %crc.lshr, i16 %crc.xor
+ %iv.next = add nuw nsw i8 %iv, 1
+ %exit.cond = icmp samesign ult i8 %iv, 15
+ br i1 %exit.cond, label %loop, label %exit
+
+exit: ; preds = %loop
+ ret i16 %crc.next
+}
+
define i16 @not.crc.float.simple.recurrence(float %msg, i16 %checksum) {
; CHECK-LABEL: 'not.crc.float.simple.recurrence'
; CHECK-NEXT: Did not find a hash algorithm
@@ -1193,7 +1250,7 @@ exit: ; preds = %loop
define i16 @not.crc.stray.unvisited.call(i16 %crc.init) {
; CHECK-LABEL: 'not.crc.stray.unvisited.call'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Found stray unvisited instructions
+; CHECK-NEXT: Reason: Found stray unvisited or unhandled instructions
;
entry:
br label %loop
@@ -1219,7 +1276,7 @@ declare void @print(i16)
define i16 @not.crc.call.sb.check(i16 %crc.init) {
; CHECK-LABEL: 'not.crc.call.sb.check'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
+; CHECK-NEXT: Reason: Found stray unvisited or unhandled instructions
;
entry:
br label %loop
@@ -1240,12 +1297,10 @@ exit: ; preds = %loop
ret i16 %crc.next
}
-declare i16 @side.effect()
-
define i16 @not.crc.bad.lhs.sb.check.be(i16 %crc.init) {
; CHECK-LABEL: 'not.crc.bad.lhs.sb.check.be'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Bad LHS of significant-bit-check
+; CHECK-NEXT: Reason: Malformed significant-bit check
;
entry:
br label %loop
@@ -1264,3 +1319,58 @@ loop: ; preds = %loop, %entry
exit: ; preds = %loop
ret i16 %crc.next
}
+
+define i16 @not.crc.knownbits.sb.check.fail(i16 %crc.init) {
+; CHECK-LABEL: 'not.crc.knownbits.sb.check.fail'
+; CHECK-NEXT: Did not find a hash algorithm
+; CHECK-NEXT: Reason: Malformed significant-bit check
+;
+entry:
+ br label %loop
+
+loop: ; preds = %loop, %entry
+ %iv = phi i16 [ 0, %entry ], [ %iv.next, %loop ]
+ %crc = phi i16 [ %crc.init, %entry ], [ %crc.next, %loop ]
+ %crc.shl = shl i16 %crc, 1
+ %evil.and.iv = and i16 %iv, 2
+ %evil.and.1 = add i16 %evil.and.iv, 1
+ %evil.mul = mul i16 %crc.shl, %evil.and.1
+ %evil.xor = xor i16 %evil.mul, 4129
+ %check.sb = icmp slt i16 %crc, 0
+ %crc.next = select i1 %check.sb, i16 %evil.xor, i16 %evil.mul
+ %iv.next = add nuw nsw i16 %iv, 1
+ %exitcond.not = icmp eq i16 %iv.next, 8
+ br i1 %exitcond.not, label %exit, label %loop
+
+exit: ; preds = %loop
+ ret i16 %crc.next
+}
+
+define i16 @not.crc.knownbits.sb.check.fail.call.outside.loop(i16 %crc.init) {
+; CHECK-LABEL: 'not.crc.knownbits.sb.check.fail.call.outside.loop'
+; CHECK-NEXT: Did not find a hash algorithm
+; CHECK-NEXT: Reason: Malformed significant-bit check
+;
+entry:
+ %corrupt = call i16 @side.effect()
+ br label %loop
+
+loop: ; preds = %loop, %entry
+ %iv = phi i16 [ 0, %entry ], [ %iv.next, %loop ]
+ %crc = phi i16 [ %crc.init, %entry ], [ %crc.next, %loop ]
+ %crc.shl = shl i16 %crc, 1
+ %evil.and.corrupt = and i16 %corrupt, 2
+ %evil.and.1 = add i16 %evil.and.corrupt, 1
+ %evil.mul = mul i16 %crc.shl, %evil.and.1
+ %evil.xor = xor i16 %evil.mul, 4129
+ %check.sb = icmp slt i16 %crc, 0
+ %crc.next = select i1 %check.sb, i16 %evil.xor, i16 %evil.mul
+ %iv.next = add nuw nsw i16 %iv, 1
+ %exitcond.not = icmp eq i16 %iv.next, 8
+ br i1 %exitcond.not, label %exit, label %loop
+
+exit: ; preds = %loop
+ ret i16 %crc.next
+}
+
+declare i16 @side.effect()
>From 5d29eb09c59426d943970697bffec9edfa927a1f Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 1 Sep 2025 11:35:06 +0100
Subject: [PATCH 3/5] [HashRecognize] Match sb-check strongly
---
llvm/lib/Analysis/HashRecognize.cpp | 128 +++++++++---------
.../HashRecognize/cyclic-redundancy-check.ll | 16 +--
2 files changed, 71 insertions(+), 73 deletions(-)
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index 49fb07aa51a5c..a8e480a78fde5 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -73,60 +73,10 @@ using namespace SCEVPatternMatch;
#define DEBUG_TYPE "hash-recognize"
-/// Check the well-formedness of the (most|least) significant bit check \p SI,
-/// where \p BitShift is the bit-shift branch: the other branch must be a
-/// bit-shift-and-xor-poly branch, as already checked by
-/// matchConditionalRecurrence. We check that the compare is `>= 0` in the
-/// big-endian case, and `== 0` in the little-endian case (or the inverse, in
-/// which case the branches of the compare are swapped). We check LCR against
-/// CheckLCR, which is full-set in the big-endian case, and [0, 2) in the
-/// little-endian case: CheckLCR checks that the comparison is `>= 0` in the
-/// big-endian case, and that the compare is to 0 or 1 in the little-endian case
-/// (as a value and'ed with 1 is passed as the operand). We then check
-/// AllowedByR against CheckAllowedByR, which is [0, smin) in the big-endian
-/// case, and [0, 1) in the little-endian case: CheckAllowedByR checks for
-/// significant-bit-clear, and this must be equal to \p BitShift for
-/// well-formedness.
-static bool isSignificantBitCheckWellFormed(const SelectInst *SI,
- const BinaryOperator *BitShift,
- bool ByteOrderSwapped) {
- DataLayout DL = SI->getParent()->getDataLayout();
- CmpPredicate Pred;
- Value *L, *R;
- Instruction *TV, *FV;
- [[maybe_unused]] bool Match =
- match(SI, m_Select(m_ICmp(Pred, m_Value(L), m_Value(R)),
- m_Instruction(TV), m_Instruction(FV)));
- assert(Match && "Select(ICmp()) expected in signficant-bit-check");
-
- KnownBits KnownL = computeKnownBits(L, DL);
- unsigned ICmpBW = KnownL.getBitWidth();
- auto LCR = ConstantRange::fromKnownBits(KnownL, false);
- auto CheckLCR = ConstantRange::getNonEmpty(
- APInt::getZero(ICmpBW),
- ByteOrderSwapped ? APInt::getZero(ICmpBW) : APInt(ICmpBW, 2));
- if (LCR != CheckLCR)
- return false;
-
- KnownBits KnownR = computeKnownBits(R, DL);
- auto RCR = ConstantRange::fromKnownBits(KnownR, false);
- auto AllowedByR = ConstantRange::makeAllowedICmpRegion(Pred, RCR);
- ConstantRange CheckAllowedByR(
- APInt::getZero(ICmpBW),
- ByteOrderSwapped ? APInt::getSignedMinValue(ICmpBW) : APInt(ICmpBW, 1));
-
- if (AllowedByR == CheckAllowedByR)
- return TV == BitShift;
- if (AllowedByR.inverse() == CheckAllowedByR)
- return FV == BitShift;
- return false;
-}
-
-/// Checks if Loop \p L contains instructions unreachable or unhandled from \p
-/// Roots on the use-def chain.
-static bool
-containsUnreachableOrUnhandled(const Loop &L,
- ArrayRef<const Instruction *> Roots) {
+/// Checks if Loop \p L contains instructions unreachable \p Roots on the
+/// use-def chain.
+static bool containsUnreachable(const Loop &L,
+ ArrayRef<const Instruction *> Roots) {
SmallPtrSet<const Instruction *, 16> Visited;
BasicBlock *Latch = L.getLoopLatch();
@@ -138,9 +88,6 @@ containsUnreachableOrUnhandled(const Loop &L,
if (isa<PHINode>(I))
continue;
- if (!isa<TruncInst, BinaryOperator, SelectInst, CmpInst, BranchInst>(I))
- return true;
-
for (const Use &U : I->operands()) {
if (auto *UI = dyn_cast<Instruction>(U)) {
if (!L.contains(UI))
@@ -148,8 +95,6 @@ containsUnreachableOrUnhandled(const Loop &L,
Worklist.push_back(UI);
continue;
}
- if (!isa<ConstantInt, BasicBlock>(U))
- return true;
}
}
return std::distance(Latch->begin(), Latch->end()) != Visited.size();
@@ -204,6 +149,60 @@ struct RecurrenceInfo {
Instruction::BinaryOps BOWithConstOpToMatch = Instruction::BinaryOpsEnd);
};
+/// Check the well-formedness of the (most|least) significant bit check given \p
+/// ConditionalRecurrence, \p SimpleRecurrence, depending on \p
+/// ByteOrderSwapped. We check that ConditionalRecurrence.Step is a
+/// Select(Cmp()) where the compare is `>= 0` in the big-endian case, and `== 0`
+/// in the little-endian case (or the inverse, in which case the branches of the
+/// compare are swapped). We check that the LHS is (ConditionalRecurrence.Phi
+/// [xor SimpleRecurrence.Phi]) in the big-endian case, and additionally check
+/// for an AND with one in the little-endian case. We then check AllowedByR
+/// against CheckAllowedByR, which is [0, smin) in the big-endian case, and [0,
+/// 1) in the little-endian case: CheckAllowedByR checks for
+/// significant-bit-clear, and this must be equal to ConditionalRecurrence.BO
+/// (which is the bit-shift, as already checked by isBigEndianBitShift) for
+/// well-formedness.
+static bool
+isSignificantBitCheckWellFormed(const RecurrenceInfo &ConditionalRecurrence,
+ const RecurrenceInfo &SimpleRecurrence,
+ bool ByteOrderSwapped) {
+ auto *SI = cast<SelectInst>(ConditionalRecurrence.Step);
+ DataLayout DL = SI->getParent()->getDataLayout();
+ CmpPredicate Pred;
+ const Value *L;
+ const APInt *R;
+ Instruction *TV, *FV;
+ if (!match(SI, m_Select(m_ICmp(Pred, m_Value(L), m_APInt(R)),
+ m_Instruction(TV), m_Instruction(FV))))
+ return false;
+
+ // Match predicate with or without a SimpleRecurrence (the corresponding data
+ // is LHSAux).
+ auto MatchPred =
+ m_CombineOr(m_Specific(ConditionalRecurrence.Phi),
+ m_c_Xor(m_CastOrSelf(m_Specific(ConditionalRecurrence.Phi)),
+ m_CastOrSelf(m_Specific(SimpleRecurrence.Phi))));
+ bool LWellFormed = ByteOrderSwapped ? match(L, MatchPred)
+ : match(L, m_c_And(MatchPred, m_One()));
+ if (!LWellFormed)
+ return false;
+
+ KnownBits KnownR = KnownBits::makeConstant(*R);
+ unsigned BW = KnownR.getBitWidth();
+ auto RCR = ConstantRange::fromKnownBits(KnownR, false);
+ auto AllowedByR = ConstantRange::makeAllowedICmpRegion(Pred, RCR);
+ ConstantRange CheckAllowedByR(APInt::getZero(BW),
+ ByteOrderSwapped ? APInt::getSignedMinValue(BW)
+ : APInt(BW, 1));
+
+ BinaryOperator *BitShift = ConditionalRecurrence.BO;
+ if (AllowedByR == CheckAllowedByR)
+ return TV == BitShift;
+ if (AllowedByR.inverse() == CheckAllowedByR)
+ return FV == BitShift;
+ return false;
+}
+
/// Wraps llvm::matchSimpleRecurrence. Match a simple first order recurrence
/// cycle of the form:
///
@@ -375,7 +374,7 @@ CRCTable HashRecognize::genSarwateTable(const APInt &GenPoly,
/// Checks that \p P1 and \p P2 are used together in an XOR in the use-def chain
/// of \p SI's condition, ignoring any casts. The purpose of this function is to
/// ensure that LHSAux from the SimpleRecurrence is used correctly in the CRC
-/// computation. We cannot check the correctness of casts at this point.
+/// computation.
///
/// In other words, it checks for the following pattern:
///
@@ -497,9 +496,8 @@ std::variant<PolynomialInfo, StringRef> HashRecognize::recognizeCRC() const {
"Expected ExtraConst in conditional recurrence");
const APInt &GenPoly = *ConditionalRecurrence.ExtraConst;
- if (!isSignificantBitCheckWellFormed(
- cast<SelectInst>(ConditionalRecurrence.Step),
- ConditionalRecurrence.BO, *ByteOrderSwapped))
+ if (!isSignificantBitCheckWellFormed(ConditionalRecurrence, SimpleRecurrence,
+ *ByteOrderSwapped))
return "Malformed significant-bit check";
SmallVector<const Instruction *> Roots(
@@ -508,8 +506,8 @@ std::variant<PolynomialInfo, StringRef> HashRecognize::recognizeCRC() const {
L.getLatchCmpInst(), Latch->getTerminator()});
if (SimpleRecurrence)
Roots.push_back(SimpleRecurrence.BO);
- if (containsUnreachableOrUnhandled(L, Roots))
- return "Found stray unvisited or unhandled instructions";
+ if (containsUnreachable(L, Roots))
+ return "Found stray unvisited instructions";
return PolynomialInfo(TC, LHS, GenPoly, ComputedValue, *ByteOrderSwapped,
LHSAux);
diff --git a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
index 48464e69841f1..57898465b1d24 100644
--- a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
+++ b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
@@ -1109,7 +1109,7 @@ exit: ; preds = %loop
define i16 @not.crc.unknown.value(i16 %msg, i16 %checksum, i16 %corrupt) {
; CHECK-LABEL: 'not.crc.unknown.value'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Found stray unvisited or unhandled instructions
+; CHECK-NEXT: Reason: Malformed significant-bit check
;
entry:
br label %loop
@@ -1137,7 +1137,7 @@ exit: ; preds = %loop
define i16 @not.crc.unknown.call.outside.loop(i16 %msg, i16 %checksum) {
; CHECK-LABEL: 'not.crc.unknown.call.outside.loop'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Found stray unvisited or unhandled instructions
+; CHECK-NEXT: Reason: Malformed significant-bit check
;
entry:
%corrupt = call i16 @side.effect()
@@ -1250,7 +1250,7 @@ exit: ; preds = %loop
define i16 @not.crc.stray.unvisited.call(i16 %crc.init) {
; CHECK-LABEL: 'not.crc.stray.unvisited.call'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Found stray unvisited or unhandled instructions
+; CHECK-NEXT: Reason: Found stray unvisited instructions
;
entry:
br label %loop
@@ -1276,7 +1276,7 @@ declare void @print(i16)
define i16 @not.crc.call.sb.check(i16 %crc.init) {
; CHECK-LABEL: 'not.crc.call.sb.check'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Found stray unvisited or unhandled instructions
+; CHECK-NEXT: Reason: Malformed significant-bit check
;
entry:
br label %loop
@@ -1320,8 +1320,8 @@ exit: ; preds = %loop
ret i16 %crc.next
}
-define i16 @not.crc.knownbits.sb.check.fail(i16 %crc.init) {
-; CHECK-LABEL: 'not.crc.knownbits.sb.check.fail'
+define i16 @not.crc.sb.check.patternmatch.fail(i16 %crc.init) {
+; CHECK-LABEL: 'not.crc.sb.check.patternmatch.fail'
; CHECK-NEXT: Did not find a hash algorithm
; CHECK-NEXT: Reason: Malformed significant-bit check
;
@@ -1346,8 +1346,8 @@ exit: ; preds = %loop
ret i16 %crc.next
}
-define i16 @not.crc.knownbits.sb.check.fail.call.outside.loop(i16 %crc.init) {
-; CHECK-LABEL: 'not.crc.knownbits.sb.check.fail.call.outside.loop'
+define i16 @not.crc.sb.check.patternmatch.fail.call.outside.loop(i16 %crc.init) {
+; CHECK-LABEL: 'not.crc.sb.check.patternmatch.fail.call.outside.loop'
; CHECK-NEXT: Did not find a hash algorithm
; CHECK-NEXT: Reason: Malformed significant-bit check
;
>From 365023c3c3b1c989f9bc35035e97375dd35af193 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 1 Sep 2025 12:31:53 +0100
Subject: [PATCH 4/5] [HashRecognize] Forbid sexts
---
llvm/include/llvm/IR/PatternMatch.h | 8 ++
llvm/lib/Analysis/HashRecognize.cpp | 12 +--
.../HashRecognize/cyclic-redundancy-check.ll | 75 +++++++++++++++++++
3 files changed, 89 insertions(+), 6 deletions(-)
diff --git a/llvm/include/llvm/IR/PatternMatch.h b/llvm/include/llvm/IR/PatternMatch.h
index d68be03d4ba7d..2cb78904dd799 100644
--- a/llvm/include/llvm/IR/PatternMatch.h
+++ b/llvm/include/llvm/IR/PatternMatch.h
@@ -2283,6 +2283,14 @@ m_ZExtOrSExtOrSelf(const OpTy &Op) {
return m_CombineOr(m_ZExtOrSExt(Op), Op);
}
+template <typename OpTy>
+inline match_combine_or<match_combine_or<CastInst_match<OpTy, ZExtInst>,
+ CastInst_match<OpTy, TruncInst>>,
+ OpTy>
+m_ZExtOrTruncOrSelf(const OpTy &Op) {
+ return m_CombineOr(m_CombineOr(m_ZExt(Op), m_Trunc(Op)), Op);
+}
+
template <typename OpTy>
inline CastInst_match<OpTy, UIToFPInst> m_UIToFP(const OpTy &Op) {
return CastInst_match<OpTy, UIToFPInst>(Op);
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index a8e480a78fde5..1a025a734b8ea 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -178,10 +178,10 @@ isSignificantBitCheckWellFormed(const RecurrenceInfo &ConditionalRecurrence,
// Match predicate with or without a SimpleRecurrence (the corresponding data
// is LHSAux).
- auto MatchPred =
- m_CombineOr(m_Specific(ConditionalRecurrence.Phi),
- m_c_Xor(m_CastOrSelf(m_Specific(ConditionalRecurrence.Phi)),
- m_CastOrSelf(m_Specific(SimpleRecurrence.Phi))));
+ auto MatchPred = m_CombineOr(
+ m_Specific(ConditionalRecurrence.Phi),
+ m_c_Xor(m_ZExtOrTruncOrSelf(m_Specific(ConditionalRecurrence.Phi)),
+ m_ZExtOrTruncOrSelf(m_Specific(SimpleRecurrence.Phi))));
bool LWellFormed = ByteOrderSwapped ? match(L, MatchPred)
: match(L, m_c_And(MatchPred, m_One()));
if (!LWellFormed)
@@ -401,8 +401,8 @@ static bool isConditionalOnXorOfPHIs(const SelectInst *SI, const PHINode *P1,
continue;
// If we match an XOR of the two PHIs ignoring casts, we're done.
- if (match(I, m_c_Xor(m_CastOrSelf(m_Specific(P1)),
- m_CastOrSelf(m_Specific(P2)))))
+ if (match(I, m_c_Xor(m_ZExtOrTruncOrSelf(m_Specific(P1)),
+ m_ZExtOrTruncOrSelf(m_Specific(P2)))))
return true;
// Continue along the use-def chain.
diff --git a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
index 57898465b1d24..432a4d72fafb4 100644
--- a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
+++ b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
@@ -427,6 +427,53 @@ exit: ; preds = %loop
ret i32 %crc.next
}
+define i16 @crc16.be.tc8.zext.data(i8 %msg, i16 %checksum) {
+; CHECK-LABEL: 'crc16.be.tc8.zext.data'
+; CHECK-NEXT: Found big-endian CRC-16 loop with trip count 8
+; CHECK-NEXT: Initial CRC: i16 %checksum
+; CHECK-NEXT: Generating polynomial: 258
+; CHECK-NEXT: Computed CRC: %crc.next = select i1 %check.sb, i16 %crc.shl, i16 %crc.xor
+; CHECK-NEXT: Auxiliary data: i8 %msg
+; CHECK-NEXT: Computed CRC lookup table:
+; CHECK-NEXT: 0 258 516 774 1032 1290 1548 1806 2064 2322 2580 2838 3096 3354 3612 3870
+; CHECK-NEXT: 4128 4386 4644 4902 5160 5418 5676 5934 6192 6450 6708 6966 7224 7482 7740 7998
+; CHECK-NEXT: 8256 8514 8772 9030 9288 9546 9804 10062 10320 10578 10836 11094 11352 11610 11868 12126
+; CHECK-NEXT: 12384 12642 12900 13158 13416 13674 13932 14190 14448 14706 14964 15222 15480 15738 15996 16254
+; CHECK-NEXT: 16512 16770 17028 17286 17544 17802 18060 18318 18576 18834 19092 19350 19608 19866 20124 20382
+; CHECK-NEXT: 20640 20898 21156 21414 21672 21930 22188 22446 22704 22962 23220 23478 23736 23994 24252 24510
+; CHECK-NEXT: 24768 25026 25284 25542 25800 26058 26316 26574 26832 27090 27348 27606 27864 28122 28380 28638
+; CHECK-NEXT: 28896 29154 29412 29670 29928 30186 30444 30702 30960 31218 31476 31734 31992 32250 32508 32766
+; CHECK-NEXT: 33024 32770 33540 33286 34056 33802 34572 34318 35088 34834 35604 35350 36120 35866 36636 36382
+; CHECK-NEXT: 37152 36898 37668 37414 38184 37930 38700 38446 39216 38962 39732 39478 40248 39994 40764 40510
+; CHECK-NEXT: 41280 41026 41796 41542 42312 42058 42828 42574 43344 43090 43860 43606 44376 44122 44892 44638
+; CHECK-NEXT: 45408 45154 45924 45670 46440 46186 46956 46702 47472 47218 47988 47734 48504 48250 49020 48766
+; CHECK-NEXT: 49536 49282 50052 49798 50568 50314 51084 50830 51600 51346 52116 51862 52632 52378 53148 52894
+; CHECK-NEXT: 53664 53410 54180 53926 54696 54442 55212 54958 55728 55474 56244 55990 56760 56506 57276 57022
+; CHECK-NEXT: 57792 57538 58308 58054 58824 58570 59340 59086 59856 59602 60372 60118 60888 60634 61404 61150
+; CHECK-NEXT: 61920 61666 62436 62182 62952 62698 63468 63214 63984 63730 64500 64246 65016 64762 65532 65278
+;
+entry:
+ br label %loop
+
+loop: ; preds = %loop, %entry
+ %iv = phi i8 [ 0, %entry ], [ %iv.next, %loop ]
+ %data = phi i8 [ %msg, %entry ], [ %data.next, %loop ]
+ %crc = phi i16 [ %checksum, %entry ], [ %crc.next, %loop ]
+ %data.ext = zext i8 %data to i16
+ %xor.crc.data = xor i16 %crc, %data.ext
+ %check.sb = icmp sge i16 %xor.crc.data, 0
+ %crc.shl = shl i16 %crc, 1
+ %crc.xor = xor i16 %crc.shl, 258
+ %crc.next = select i1 %check.sb, i16 %crc.shl, i16 %crc.xor
+ %data.next = shl i8 %data, 1
+ %iv.next = add nuw nsw i8 %iv, 1
+ %exit.cond = icmp samesign ult i8 %iv, 7
+ br i1 %exit.cond, label %loop, label %exit
+
+exit: ; preds = %loop
+ ret i16 %crc.next
+}
+
; Negative tests
define i16 @not.crc.non.const.tc(i16 %crc.init, i32 %loop.limit) {
@@ -1320,6 +1367,34 @@ exit: ; preds = %loop
ret i16 %crc.next
}
+define i16 @not.crc.bad.cast.sext(i8 %msg, i16 %checksum) {
+; CHECK-LABEL: 'not.crc.bad.cast.sext'
+; CHECK-NEXT: Did not find a hash algorithm
+; CHECK-NEXT: Reason: Recurrences not intertwined with XOR
+;
+entry:
+ br label %loop
+
+loop: ; preds = %loop, %entry
+ %iv = phi i8 [ 0, %entry ], [ %iv.next, %loop ]
+ %data = phi i8 [ %msg, %entry ], [ %data.next, %loop ]
+ %crc = phi i16 [ %checksum, %entry ], [ %crc.next, %loop ]
+ %data.ext = sext i8 %data to i16
+ %xor.crc.data = xor i16 %crc, %data.ext
+ %check.sb = icmp sge i16 %xor.crc.data, 0
+ %crc.shl = shl i16 %crc, 1
+ %crc.xor = xor i16 %crc.shl, 258
+ %crc.next = select i1 %check.sb, i16 %crc.shl, i16 %crc.xor
+ %data.next = shl i8 %data, 1
+ %iv.next = add nuw nsw i8 %iv, 1
+ %exit.cond = icmp samesign ult i8 %iv, 7
+ br i1 %exit.cond, label %loop, label %exit
+
+exit: ; preds = %loop
+ ret i16 %crc.next
+}
+
+
define i16 @not.crc.sb.check.patternmatch.fail(i16 %crc.init) {
; CHECK-LABEL: 'not.crc.sb.check.patternmatch.fail'
; CHECK-NEXT: Did not find a hash algorithm
>From 047330ff5d4fb1cb5ab60d7a2ddcf62981c45dbe Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Tue, 2 Sep 2025 18:11:51 +0100
Subject: [PATCH 5/5] [HashRecognize] Address review
---
llvm/lib/Analysis/HashRecognize.cpp | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index 1a025a734b8ea..ad4c0ab413a91 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -73,8 +73,9 @@ using namespace SCEVPatternMatch;
#define DEBUG_TYPE "hash-recognize"
-/// Checks if Loop \p L contains instructions unreachable \p Roots on the
-/// use-def chain.
+/// Checks that all instructions reachable from \p Roots on the use-def chain
+/// are contained within loop \p L, and that that are no stray instructions in
+/// the loop not visited by the use-def walk.
static bool containsUnreachable(const Loop &L,
ArrayRef<const Instruction *> Roots) {
SmallPtrSet<const Instruction *, 16> Visited;
@@ -93,7 +94,6 @@ static bool containsUnreachable(const Loop &L,
if (!L.contains(UI))
return true;
Worklist.push_back(UI);
- continue;
}
}
}
@@ -157,17 +157,15 @@ struct RecurrenceInfo {
/// compare are swapped). We check that the LHS is (ConditionalRecurrence.Phi
/// [xor SimpleRecurrence.Phi]) in the big-endian case, and additionally check
/// for an AND with one in the little-endian case. We then check AllowedByR
-/// against CheckAllowedByR, which is [0, smin) in the big-endian case, and [0,
-/// 1) in the little-endian case: CheckAllowedByR checks for
-/// significant-bit-clear, and this must be equal to ConditionalRecurrence.BO
-/// (which is the bit-shift, as already checked by isBigEndianBitShift) for
-/// well-formedness.
+/// against CheckAllowedByR, which is [0, smin) in the big-endian case, and
+/// against [0, 1) in the little-endian case: CheckAllowedByR checks for
+/// significant-bit-clear, and we match the corresponding arms of the select
+/// against bit-shift and bit-shift-and-xor-gen-poly.
static bool
isSignificantBitCheckWellFormed(const RecurrenceInfo &ConditionalRecurrence,
const RecurrenceInfo &SimpleRecurrence,
bool ByteOrderSwapped) {
auto *SI = cast<SelectInst>(ConditionalRecurrence.Step);
- DataLayout DL = SI->getParent()->getDataLayout();
CmpPredicate Pred;
const Value *L;
const APInt *R;
@@ -197,9 +195,11 @@ isSignificantBitCheckWellFormed(const RecurrenceInfo &ConditionalRecurrence,
BinaryOperator *BitShift = ConditionalRecurrence.BO;
if (AllowedByR == CheckAllowedByR)
- return TV == BitShift;
+ return TV == BitShift &&
+ match(FV, m_c_Xor(m_Specific(BitShift), m_Constant()));
if (AllowedByR.inverse() == CheckAllowedByR)
- return FV == BitShift;
+ return FV == BitShift &&
+ match(TV, m_c_Xor(m_Specific(BitShift), m_Constant()));
return false;
}
@@ -219,8 +219,11 @@ isSignificantBitCheckWellFormed(const RecurrenceInfo &ConditionalRecurrence,
/// %BO = binop %step, %rec
///
bool RecurrenceInfo::matchSimpleRecurrence(const PHINode *P) {
- Phi = P;
- return llvm::matchSimpleRecurrence(Phi, BO, Start, Step);
+ if (llvm::matchSimpleRecurrence(P, BO, Start, Step)) {
+ Phi = P;
+ return true;
+ }
+ return false;
}
/// Digs for a recurrence starting with \p V hitting the PHI node in a use-def
More information about the llvm-commits
mailing list