-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[HashRecognize] Tighten pre-conditions for analysis #144757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Exit early if the TC is not a byte-multiple, as optimization works by dividing TC by 8. Also delay the SCEV TC query.
@llvm/pr-subscribers-llvm-analysis Author: Ramkumar Ramachandra (artagnon) ChangesExit early if the TC is not a byte-multiple, as optimization works by dividing TC by 8. Also delay the SCEV TC query. Full diff: https://github.com/llvm/llvm-project/pull/144757.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index 1edb8b3bdc9a8..60d2480a07096 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -561,14 +561,14 @@ std::variant<PolynomialInfo, ErrBits, StringRef>
HashRecognize::recognizeCRC() const {
if (!L.isInnermost())
return "Loop is not innermost";
- unsigned TC = SE.getSmallConstantMaxTripCount(&L);
- if (!TC || TC > 256)
- return "Unable to find a small constant trip count";
BasicBlock *Latch = L.getLoopLatch();
BasicBlock *Exit = L.getExitBlock();
const PHINode *IndVar = L.getCanonicalInductionVariable();
- if (!Latch || !Exit || !IndVar)
+ if (!Latch || !Exit || !IndVar || L.getNumBlocks() != 1)
return "Loop not in canonical form";
+ unsigned TC = SE.getSmallConstantTripCount(&L);
+ if (!TC || TC > 256 || TC % 8)
+ return "Unable to find a small constant byte-multiple trip count";
auto R = getRecurrences(Latch, IndVar, L);
if (!R)
diff --git a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
index 7a3082056ad29..2c2e45aa09589 100644
--- a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
+++ b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll
@@ -384,7 +384,7 @@ exit: ; preds = %loop
define i16 @not.crc.non.const.tc(i16 %crc.init, i32 %loop.limit) {
; CHECK-LABEL: 'not.crc.non.const.tc'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Unable to find a small constant trip count
+; CHECK-NEXT: Reason: Unable to find a small constant byte-multiple trip count
;
entry:
br label %loop
@@ -430,7 +430,7 @@ exit: ; preds = %loop
define i16 @not.crc.tc.limit(i16 %crc.init) {
; CHECK-LABEL: 'not.crc.tc.limit'
; CHECK-NEXT: Did not find a hash algorithm
-; CHECK-NEXT: Reason: Unable to find a small constant trip count
+; CHECK-NEXT: Reason: Unable to find a small constant byte-multiple trip count
;
entry:
br label %loop
@@ -617,7 +617,7 @@ loop: ; preds = %loop, %entry
%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, 20
+ %exit.cond = icmp samesign ult i8 %iv, 31
br i1 %exit.cond, label %loop, label %exit
exit: ; preds = %loop
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need tests for these two new conditions.
And for TC > 256.
I added two new tests, thanks.
This already exists as not.crc.tc.limit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Exit early if the TC is not a byte-multiple, as optimization works by dividing TC by 8. Also delay the SCEV TC query.