[llvm] [HashRecognize] Track visited in ValueEvolution (PR #147812)
Ramkumar Ramachandra via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 14 04:26:05 PDT 2025
https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/147812
>From c13d2cafce93b842dcfea9ea8716897c5986b6a1 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Wed, 9 Jul 2025 19:08:00 +0100
Subject: [PATCH 1/7] [HashRecognize] Pre-commit test for stray-unvisited
---
.../HashRecognize/cyclic-redundancy-check.ll | 45 +++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
index 247a105940e6e..add4efd0bfca8 100644
--- a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
+++ b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
@@ -1189,3 +1189,48 @@ loop: ; preds = %loop, %entry
exit: ; preds = %loop
ret i16 %crc.next
}
+
+define i16 @not.crc.stray.unvisited.inst(i16 %crc.init) {
+; CHECK-LABEL: 'not.crc.stray.unvisited.inst'
+; CHECK-NEXT: Found big-endian CRC-16 loop with trip count 8
+; CHECK-NEXT: Initial CRC: i16 %crc.init
+; CHECK-NEXT: Generating polynomial: 4129
+; CHECK-NEXT: Computed CRC: %crc.next = select i1 %check.sb, i16 %crc.xor, i16 %crc.shl
+; CHECK-NEXT: Computed CRC lookup table:
+; CHECK-NEXT: 0 4129 8258 12387 16516 20645 24774 28903 33032 37161 41290 45419 49548 53677 57806 61935
+; CHECK-NEXT: 4657 528 12915 8786 21173 17044 29431 25302 37689 33560 45947 41818 54205 50076 62463 58334
+; CHECK-NEXT: 9314 13379 1056 5121 25830 29895 17572 21637 42346 46411 34088 38153 58862 62927 50604 54669
+; CHECK-NEXT: 13907 9842 5649 1584 30423 26358 22165 18100 46939 42874 38681 34616 63455 59390 55197 51132
+; CHECK-NEXT: 18628 22757 26758 30887 2112 6241 10242 14371 51660 55789 59790 63919 35144 39273 43274 47403
+; CHECK-NEXT: 23285 19156 31415 27286 6769 2640 14899 10770 56317 52188 64447 60318 39801 35672 47931 43802
+; CHECK-NEXT: 27814 31879 19684 23749 11298 15363 3168 7233 60846 64911 52716 56781 44330 48395 36200 40265
+; CHECK-NEXT: 32407 28342 24277 20212 15891 11826 7761 3696 65439 61374 57309 53244 48923 44858 40793 36728
+; CHECK-NEXT: 37256 33193 45514 41451 53516 49453 61774 57711 4224 161 12482 8419 20484 16421 28742 24679
+; CHECK-NEXT: 33721 37784 41979 46042 49981 54044 58239 62302 689 4752 8947 13010 16949 21012 25207 29270
+; CHECK-NEXT: 46570 42443 38312 34185 62830 58703 54572 50445 13538 9411 5280 1153 29798 25671 21540 17413
+; CHECK-NEXT: 42971 47098 34713 38840 59231 63358 50973 55100 9939 14066 1681 5808 26199 30326 17941 22068
+; CHECK-NEXT: 55628 51565 63758 59695 39368 35305 47498 43435 22596 18533 30726 26663 6336 2273 14466 10403
+; CHECK-NEXT: 52093 56156 60223 64286 35833 39896 43963 48026 19061 23124 27191 31254 2801 6864 10931 14994
+; CHECK-NEXT: 64814 60687 56684 52557 48554 44427 40424 36297 31782 27655 23652 19525 15522 11395 7392 3265
+; CHECK-NEXT: 61215 65342 53085 57212 44955 49082 36825 40952 28183 32310 20053 24180 11923 16050 3793 7920
+;
+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, 0
+ %crc.next = select i1 %check.sb, i16 %crc.xor, i16 %crc.shl
+ call void @print(i16 %crc.next)
+ %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
+}
+
+declare void @print(i16)
>From bc7936eb3e7b5f0f850b4ff67641e00ea901cb00 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Wed, 9 Jul 2025 20:29:13 +0100
Subject: [PATCH 2/7] [HashRecognize] Track visited in ValueEvolution
Require that all Instructions in the Loop are visited by ValueEvolution,
as any stray instructions would complicate life for the optimization.
---
llvm/lib/Analysis/HashRecognize.cpp | 52 +++++++++++++++----
.../HashRecognize/cyclic-redundancy-check.ll | 33 +++---------
2 files changed, 50 insertions(+), 35 deletions(-)
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index 2cc3ad5f18482..f032593492287 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -91,6 +91,10 @@ class ValueEvolution {
APInt GenPoly;
StringRef ErrStr;
+ // A set of instructions visited by ValueEvolution. Anything that's not in the
+ // use-def chain of the PHIs' evolution will be reported as unvisited.
+ SmallPtrSet<const Instruction *, 16> Visited;
+
// Compute the KnownBits of a BinaryOperator.
KnownBits computeBinOp(const BinaryOperator *I);
@@ -102,15 +106,19 @@ class ValueEvolution {
public:
// ValueEvolution is meant to be constructed with the TripCount of the loop,
- // and whether the polynomial algorithm is big-endian, for the significant-bit
- // check.
- ValueEvolution(unsigned TripCount, bool ByteOrderSwapped);
+ // whether the polynomial algorithm is big-endian for the significant-bit
+ // check, and an initial value for the Visited set.
+ ValueEvolution(unsigned TripCount, bool ByteOrderSwapped,
+ ArrayRef<const Instruction *> InitVisited);
// 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);
+ // Query the Visited set.
+ bool isVisited(const Instruction *I) const { return Visited.contains(I); }
+
// In case ValueEvolution encounters an error, this is meant to be used for a
// precise error message.
StringRef getError() const { return ErrStr; }
@@ -120,8 +128,11 @@ class ValueEvolution {
KnownPhiMap KnownPhis;
};
-ValueEvolution::ValueEvolution(unsigned TripCount, bool ByteOrderSwapped)
- : TripCount(TripCount), ByteOrderSwapped(ByteOrderSwapped) {}
+ValueEvolution::ValueEvolution(unsigned TripCount, bool ByteOrderSwapped,
+ ArrayRef<const Instruction *> InitVisited)
+ : TripCount(TripCount), ByteOrderSwapped(ByteOrderSwapped) {
+ Visited.insert_range(InitVisited);
+}
KnownBits ValueEvolution::computeBinOp(const BinaryOperator *I) {
KnownBits KnownL(compute(I->getOperand(0)));
@@ -177,6 +188,9 @@ KnownBits ValueEvolution::computeBinOp(const BinaryOperator *I) {
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))
@@ -185,9 +199,14 @@ KnownBits ValueEvolution::computeInstr(const Instruction *I) {
// Compute the KnownBits for a Select(Cmp()), forcing it to take the branch
// that is predicated on the (least|most)-significant-bit check.
CmpPredicate Pred;
- Value *L, *R, *TV, *FV;
- if (match(I, m_Select(m_ICmp(Pred, m_Value(L), m_Value(R)), m_Value(TV),
- m_Value(FV)))) {
+ 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)));
+ Visited.insert(TV);
+ Visited.insert(FV);
+
// 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) {
@@ -209,6 +228,9 @@ KnownBits ValueEvolution::computeInstr(const Instruction *I) {
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)
return compute(TV);
if (AllowedR.inverse() == CheckRCR)
@@ -629,11 +651,23 @@ HashRecognize::recognizeCRC() const {
if (SimpleRecurrence)
PhiEvolutions.emplace_back(SimpleRecurrence.Phi, SimpleRecurrence.BO);
- ValueEvolution VE(TC, *ByteOrderSwapped);
+ // Initialize the Visited set in ValueEvolution with the IndVar-related
+ // instructions.
+ std::initializer_list<const Instruction *> InitVisited = {
+ IndVar, Latch->getTerminator(), L.getLatchCmpInst(),
+ cast<Instruction>(IndVar->getIncomingValueForBlock(Latch))};
+
+ ValueEvolution VE(TC, *ByteOrderSwapped, InitVisited);
if (!VE.computeEvolutions(PhiEvolutions))
return VE.getError();
KnownBits ResultBits = VE.KnownPhis.at(ConditionalRecurrence.Phi);
+ // Any unvisited instructions from the KnownBits propagation can complicate
+ // the optimization, which would just replace the entire loop with the
+ // table-lookup version of the hash algorithm.
+ if (any_of(*Latch, [VE](const Instruction &I) { return !VE.isVisited(&I); }))
+ 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))
diff --git a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
index add4efd0bfca8..3926c467375ed 100644
--- a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
+++ b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
@@ -909,10 +909,10 @@ exit: ; preds = %loop
ret i16 %crc.next
}
-define i16 @not.crc.bad.cast(i8 %msg, i16 %checksum) {
-; CHECK-LABEL: 'not.crc.bad.cast'
+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: Expected bottom 8 bits zero (????????00001011)
+; CHECK-NEXT: Reason: Found stray unvisited instructions
;
entry:
br label %loop
@@ -1190,29 +1190,10 @@ exit: ; preds = %loop
ret i16 %crc.next
}
-define i16 @not.crc.stray.unvisited.inst(i16 %crc.init) {
-; CHECK-LABEL: 'not.crc.stray.unvisited.inst'
-; CHECK-NEXT: Found big-endian CRC-16 loop with trip count 8
-; CHECK-NEXT: Initial CRC: i16 %crc.init
-; CHECK-NEXT: Generating polynomial: 4129
-; CHECK-NEXT: Computed CRC: %crc.next = select i1 %check.sb, i16 %crc.xor, i16 %crc.shl
-; CHECK-NEXT: Computed CRC lookup table:
-; CHECK-NEXT: 0 4129 8258 12387 16516 20645 24774 28903 33032 37161 41290 45419 49548 53677 57806 61935
-; CHECK-NEXT: 4657 528 12915 8786 21173 17044 29431 25302 37689 33560 45947 41818 54205 50076 62463 58334
-; CHECK-NEXT: 9314 13379 1056 5121 25830 29895 17572 21637 42346 46411 34088 38153 58862 62927 50604 54669
-; CHECK-NEXT: 13907 9842 5649 1584 30423 26358 22165 18100 46939 42874 38681 34616 63455 59390 55197 51132
-; CHECK-NEXT: 18628 22757 26758 30887 2112 6241 10242 14371 51660 55789 59790 63919 35144 39273 43274 47403
-; CHECK-NEXT: 23285 19156 31415 27286 6769 2640 14899 10770 56317 52188 64447 60318 39801 35672 47931 43802
-; CHECK-NEXT: 27814 31879 19684 23749 11298 15363 3168 7233 60846 64911 52716 56781 44330 48395 36200 40265
-; CHECK-NEXT: 32407 28342 24277 20212 15891 11826 7761 3696 65439 61374 57309 53244 48923 44858 40793 36728
-; CHECK-NEXT: 37256 33193 45514 41451 53516 49453 61774 57711 4224 161 12482 8419 20484 16421 28742 24679
-; CHECK-NEXT: 33721 37784 41979 46042 49981 54044 58239 62302 689 4752 8947 13010 16949 21012 25207 29270
-; CHECK-NEXT: 46570 42443 38312 34185 62830 58703 54572 50445 13538 9411 5280 1153 29798 25671 21540 17413
-; CHECK-NEXT: 42971 47098 34713 38840 59231 63358 50973 55100 9939 14066 1681 5808 26199 30326 17941 22068
-; CHECK-NEXT: 55628 51565 63758 59695 39368 35305 47498 43435 22596 18533 30726 26663 6336 2273 14466 10403
-; CHECK-NEXT: 52093 56156 60223 64286 35833 39896 43963 48026 19061 23124 27191 31254 2801 6864 10931 14994
-; CHECK-NEXT: 64814 60687 56684 52557 48554 44427 40424 36297 31782 27655 23652 19525 15522 11395 7392 3265
-; CHECK-NEXT: 61215 65342 53085 57212 44955 49082 36825 40952 28183 32310 20053 24180 11923 16050 3793 7920
+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
;
entry:
br label %loop
>From 3a96e7f6a8758220820add38485cd5c673dbc704 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Thu, 10 Jul 2025 11:12:45 +0100
Subject: [PATCH 3/7] [HashRecognize] Minor simplification (NFC)
---
llvm/lib/Analysis/HashRecognize.cpp | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index f032593492287..662260a79ab7a 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -91,10 +91,6 @@ class ValueEvolution {
APInt GenPoly;
StringRef ErrStr;
- // A set of instructions visited by ValueEvolution. Anything that's not in the
- // use-def chain of the PHIs' evolution will be reported as unvisited.
- SmallPtrSet<const Instruction *, 16> Visited;
-
// Compute the KnownBits of a BinaryOperator.
KnownBits computeBinOp(const BinaryOperator *I);
@@ -116,13 +112,14 @@ class ValueEvolution {
// the final iteration. Returns true on success and false on error.
bool computeEvolutions(ArrayRef<PhiStepPair> PhiEvolutions);
- // Query the Visited set.
- bool isVisited(const Instruction *I) const { return Visited.contains(I); }
-
// 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. Anything that's not in the
+ // use-def chain of the PHIs' evolution will not be visited.
+ SmallPtrSet<const Instruction *, 16> Visited;
+
// The computed KnownBits for each PHI node, which is populated after
// computeEvolutions is called.
KnownPhiMap KnownPhis;
@@ -130,9 +127,8 @@ class ValueEvolution {
ValueEvolution::ValueEvolution(unsigned TripCount, bool ByteOrderSwapped,
ArrayRef<const Instruction *> InitVisited)
- : TripCount(TripCount), ByteOrderSwapped(ByteOrderSwapped) {
- Visited.insert_range(InitVisited);
-}
+ : TripCount(TripCount), ByteOrderSwapped(ByteOrderSwapped),
+ Visited(InitVisited.begin(), InitVisited.end()) {}
KnownBits ValueEvolution::computeBinOp(const BinaryOperator *I) {
KnownBits KnownL(compute(I->getOperand(0)));
@@ -665,7 +661,7 @@ HashRecognize::recognizeCRC() const {
// Any unvisited instructions from the KnownBits propagation can complicate
// the optimization, which would just replace the entire loop with the
// table-lookup version of the hash algorithm.
- if (any_of(*Latch, [VE](const Instruction &I) { return !VE.isVisited(&I); }))
+ if (std::distance(Latch->begin(), Latch->end()) != VE.Visited.size())
return "Found stray unvisited instructions";
unsigned N = std::min(TC, ResultBits.getBitWidth());
>From 1a4cda36473996b0177fa874e9ff4240bf9c17ec Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Fri, 11 Jul 2025 00:02:03 +0100
Subject: [PATCH 4/7] [HashRecognize] NFC simplification
---
llvm/lib/Analysis/HashRecognize.cpp | 40 ++++++++++++++---------------
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index 662260a79ab7a..c4905f3290c17 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -102,10 +102,9 @@ class ValueEvolution {
public:
// ValueEvolution is meant to be constructed with the TripCount of the loop,
- // whether the polynomial algorithm is big-endian for the significant-bit
- // check, and an initial value for the Visited set.
- ValueEvolution(unsigned TripCount, bool ByteOrderSwapped,
- ArrayRef<const Instruction *> InitVisited);
+ // 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
@@ -116,8 +115,8 @@ class ValueEvolution {
// precise error message.
StringRef getError() const { return ErrStr; }
- // A set of instructions visited by ValueEvolution. Anything that's not in the
- // use-def chain of the PHIs' evolution will not be visited.
+ // 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
@@ -125,10 +124,8 @@ class ValueEvolution {
KnownPhiMap KnownPhis;
};
-ValueEvolution::ValueEvolution(unsigned TripCount, bool ByteOrderSwapped,
- ArrayRef<const Instruction *> InitVisited)
- : TripCount(TripCount), ByteOrderSwapped(ByteOrderSwapped),
- Visited(InitVisited.begin(), InitVisited.end()) {}
+ValueEvolution::ValueEvolution(unsigned TripCount, bool ByteOrderSwapped)
+ : TripCount(TripCount), ByteOrderSwapped(ByteOrderSwapped) {}
KnownBits ValueEvolution::computeBinOp(const BinaryOperator *I) {
KnownBits KnownL(compute(I->getOperand(0)));
@@ -647,21 +644,22 @@ HashRecognize::recognizeCRC() const {
if (SimpleRecurrence)
PhiEvolutions.emplace_back(SimpleRecurrence.Phi, SimpleRecurrence.BO);
- // Initialize the Visited set in ValueEvolution with the IndVar-related
- // instructions.
- std::initializer_list<const Instruction *> InitVisited = {
- IndVar, Latch->getTerminator(), L.getLatchCmpInst(),
- cast<Instruction>(IndVar->getIncomingValueForBlock(Latch))};
-
- ValueEvolution VE(TC, *ByteOrderSwapped, InitVisited);
+ ValueEvolution VE(TC, *ByteOrderSwapped);
if (!VE.computeEvolutions(PhiEvolutions))
return VE.getError();
KnownBits ResultBits = VE.KnownPhis.at(ConditionalRecurrence.Phi);
- // Any unvisited instructions from the KnownBits propagation can complicate
- // the optimization, which would just replace the entire loop with the
- // table-lookup version of the hash algorithm.
- if (std::distance(Latch->begin(), Latch->end()) != VE.Visited.size())
+ // There must be exactly four unvisited instructions, corresponding to the
+ // IndVar PHI:
+ // IndVar
+ // Latch->getTerminator()
+ // L.getLatchCmpInst(),
+ // IndVar->getIncomingValueForBlock(Latch))
+ //
+ // Any other unvisited instructions from the KnownBits propagation can
+ // complicate the optimization, which would just replace the entire loop with
+ // the table-lookup version of the hash algorithm.
+ if (std::distance(Latch->begin(), Latch->end()) != VE.Visited.size() + 4)
return "Found stray unvisited instructions";
unsigned N = std::min(TC, ResultBits.getBitWidth());
>From 886b49486b344da6ec028383d4b169d9850a56a8 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 14 Jul 2025 10:03:30 +0100
Subject: [PATCH 5/7] [HashRecognize] Address review
---
llvm/lib/Analysis/HashRecognize.cpp | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index c4905f3290c17..92c9e37dbb484 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -197,8 +197,6 @@ KnownBits ValueEvolution::computeInstr(const Instruction *I) {
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)));
- Visited.insert(TV);
- Visited.insert(FV);
// We need to check LCR against [0, 2) in the little-endian case, because
// the RCR check is insufficient: it is simply [0, 1).
@@ -224,10 +222,14 @@ KnownBits ValueEvolution::computeInstr(const Instruction *I) {
// 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)
+ if (AllowedR == CheckRCR) {
+ Visited.insert(FV);
return compute(TV);
- if (AllowedR.inverse() == CheckRCR)
+ }
+ if (AllowedR.inverse() == CheckRCR) {
+ Visited.insert(TV);
return compute(FV);
+ }
ErrStr = "Bad RHS of significant-bit-check";
return {BitWidth};
@@ -650,16 +652,14 @@ HashRecognize::recognizeCRC() const {
KnownBits ResultBits = VE.KnownPhis.at(ConditionalRecurrence.Phi);
// There must be exactly four unvisited instructions, corresponding to the
- // IndVar PHI:
- // IndVar
- // Latch->getTerminator()
- // L.getLatchCmpInst(),
- // IndVar->getIncomingValueForBlock(Latch))
- //
- // Any other unvisited instructions from the KnownBits propagation can
- // complicate the optimization, which would just replace the entire loop with
- // the table-lookup version of the hash algorithm.
- if (std::distance(Latch->begin(), Latch->end()) != VE.Visited.size() + 4)
+ // 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());
>From 30e2e1b9b65d13a4ad99607de2c14e4235c80d23 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 14 Jul 2025 11:02:49 +0100
Subject: [PATCH 6/7] [HashRecognize] Add call.sb.check test
---
.../HashRecognize/cyclic-redundancy-check.ll | 26 +++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
index 3926c467375ed..fe140d01e8818 100644
--- a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
+++ b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
@@ -1215,3 +1215,29 @@ exit: ; preds = %loop
}
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
+;
+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
+ %call = call i16 @side.effect()
+ %check.sb = icmp slt i16 %call, 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
+}
+
+declare i16 @side.effect()
>From c4de4503fe57210790d546c2dc45388ea4f5a633 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 7/7] [HashRecognize] Fix bug related to not checking LHS
Co-authored-by: Piotr Fusik <p.fusik at samsung.com>
---
llvm/lib/Analysis/HashRecognize.cpp | 25 +++++++++----
.../HashRecognize/cyclic-redundancy-check.ll | 37 +++++++++++++++----
2 files changed, 48 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index 92c9e37dbb484..ba67478835b34 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);
@@ -198,22 +199,30 @@ 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).
+ KnownBits KnownL = compute(L);
+ unsigned ICmpBW = KnownL.getBitWidth();
+ auto LCR = ConstantRange::fromKnownBits(KnownL, false);
if (!ByteOrderSwapped) {
- KnownBits KnownL = compute(L);
- unsigned ICmpBW = KnownL.getBitWidth();
- auto LCR = ConstantRange::fromKnownBits(KnownL, false);
+ // We need to check LCR against [0, 2) in the little-endian case, because
+ // the RCR check is insufficient: it is simply [0, 1).
auto CheckLCR = ConstantRange(APInt::getZero(ICmpBW), APInt(ICmpBW, 2));
if (LCR != CheckLCR) {
ErrStr = "Bad LHS of significant-bit-check";
return {BitWidth};
}
+ } else {
+ // We need to check LCR against full-set, [0, -1), [0, -3), [0, -7), etc.
+ // depending on which iteration we're at.
+ auto CheckLCR = ConstantRange::getNonEmpty(
+ APInt::getZero(ICmpBW), -APInt::getLowBitsSet(ICmpBW, AtIter));
+ 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),
@@ -264,9 +273,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
+}
More information about the llvm-commits
mailing list