[llvm] [HashRecognize] Fix LHS ConstantRange check for BE (PR #148620)
Ramkumar Ramachandra via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 16 08:14:37 PDT 2025
https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/148620
>From 894c8e45dce2f0452c41eb1ae5ac43e12fc60f87 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] [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 | 44 +++++++++++--------
.../HashRecognize/cyclic-redundancy-check.ll | 37 +++++++++++++---
2 files changed, 55 insertions(+), 26 deletions(-)
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index 92c9e37dbb484..ae65d5c3e5188 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,35 +199,38 @@ 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.
+ KnownBits KnownL = compute(L);
+ unsigned ICmpBW = KnownL.getBitWidth();
+ auto LCR = ConstantRange::fromKnownBits(KnownL, false);
+ // Check LCR against full-set, [0, -1), [0, -3), [0, -7), etc. depending on
+ // AtIter in the big-endian case, and against [0, 2) in the little-endian
+ // case.
+ 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));
+ // Check AllowedR against [0, smin) in the big-endian case, and against
+ // [0, 1) in the little-endian case.
+ ConstantRange CheckAllowedR(
+ 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) {
+ if (AllowedR == CheckAllowedR) {
Visited.insert(FV);
return compute(TV);
}
- if (AllowedR.inverse() == CheckRCR) {
+ if (AllowedR.inverse() == CheckAllowedR) {
Visited.insert(TV);
return compute(FV);
}
@@ -264,9 +268,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