Skip to content

[HashRecognize] Don't const-qualify Values in result #144752

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

Merged
merged 1 commit into from
Jun 18, 2025

Conversation

artagnon
Copy link
Contributor

Const-qualifying Values in the analysis result makes them unusable with IRBuilder. The issue was discovered when attempting to use the result of the analysis for a transform.

Const-qualifying Values in the analysis result makes them unusable with
IRBuilder. The issue was discovered when attempting to use the result of
the analysis for a transform.
@artagnon artagnon requested review from nikic and pfusik June 18, 2025 16:45
@llvmbot
Copy link
Member

llvmbot commented Jun 18, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Ramkumar Ramachandra (artagnon)

Changes

Const-qualifying Values in the analysis result makes them unusable with IRBuilder. The issue was discovered when attempting to use the result of the analysis for a transform.


Full diff: https://github.com/llvm/llvm-project/pull/144752.diff

2 Files Affected:

  • (modified) llvm/include/llvm/Analysis/HashRecognize.h (+6-6)
  • (modified) llvm/lib/Analysis/HashRecognize.cpp (+4-4)
diff --git a/llvm/include/llvm/Analysis/HashRecognize.h b/llvm/include/llvm/Analysis/HashRecognize.h
index 8ab68a5dc2cb1..c169383bf7b08 100644
--- a/llvm/include/llvm/Analysis/HashRecognize.h
+++ b/llvm/include/llvm/Analysis/HashRecognize.h
@@ -53,7 +53,7 @@ struct PolynomialInfo {
   // division in the case of CRC. Since polynomial division is an XOR in
   // GF(2^m), this variable must be XOR'ed with RHS in a loop to yield the
   // ComputedValue.
-  const Value *LHS;
+  Value *LHS;
 
   // The generating polynomial, or the RHS of the polynomial division in the
   // case of CRC.
@@ -61,7 +61,7 @@ struct PolynomialInfo {
 
   // The final computed value. This is a remainder of a polynomial division in
   // the case of CRC, which must be zero.
-  const Value *ComputedValue;
+  Value *ComputedValue;
 
   // Set to true in the case of big-endian.
   bool ByteOrderSwapped;
@@ -69,11 +69,11 @@ struct PolynomialInfo {
   // An optional auxiliary checksum that augments the LHS. In the case of CRC,
   // it is XOR'ed with the LHS, so that the computation's final remainder is
   // zero.
-  const Value *LHSAux;
+  Value *LHSAux;
 
-  PolynomialInfo(unsigned TripCount, const Value *LHS, const APInt &RHS,
-                 const Value *ComputedValue, bool ByteOrderSwapped,
-                 const Value *LHSAux = nullptr);
+  PolynomialInfo(unsigned TripCount, Value *LHS, const APInt &RHS,
+                 Value *ComputedValue, bool ByteOrderSwapped,
+                 Value *LHSAux = nullptr);
 };
 
 /// The analysis.
diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp
index 1edb8b3bdc9a8..f7ac02be2e039 100644
--- a/llvm/lib/Analysis/HashRecognize.cpp
+++ b/llvm/lib/Analysis/HashRecognize.cpp
@@ -442,9 +442,9 @@ getRecurrences(BasicBlock *LoopLatch, const PHINode *IndVar, const Loop &L) {
   return std::make_pair(SimpleRecurrence, ConditionalRecurrence);
 }
 
-PolynomialInfo::PolynomialInfo(unsigned TripCount, const Value *LHS,
-                               const APInt &RHS, const Value *ComputedValue,
-                               bool ByteOrderSwapped, const Value *LHSAux)
+PolynomialInfo::PolynomialInfo(unsigned TripCount, Value *LHS, const APInt &RHS,
+                               Value *ComputedValue, bool ByteOrderSwapped,
+                               Value *LHSAux)
     : TripCount(TripCount), LHS(LHS), RHS(RHS), ComputedValue(ComputedValue),
       ByteOrderSwapped(ByteOrderSwapped), LHSAux(LHSAux) {}
 
@@ -623,7 +623,7 @@ HashRecognize::recognizeCRC() const {
   if (!checkExtractBits(ResultBits, TC, IsZero, *ByteOrderSwapped))
     return ErrBits(ResultBits, TC, *ByteOrderSwapped);
 
-  const Value *LHSAux = SimpleRecurrence ? SimpleRecurrence.Start : nullptr;
+  Value *LHSAux = SimpleRecurrence ? SimpleRecurrence.Start : nullptr;
   return PolynomialInfo(TC, ConditionalRecurrence.Start, GenPoly, ComputedValue,
                         *ByteOrderSwapped, LHSAux);
 }

Copy link
Contributor

@pfusik pfusik left a comment

Choose a reason for hiding this comment

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

LGTM
Are you splitting #143208 to smaller changes?

@artagnon
Copy link
Contributor Author

Are you splitting #143208 to smaller changes?

Yes, that was always my intent. Hopefully, we can get #143208 enabled by default in the PR: the current blocker is HexagonLoopIdiom, which I think can be fixed with TTI hooks.

@artagnon artagnon merged commit f13b9e3 into llvm:main Jun 18, 2025
9 checks passed
@artagnon artagnon deleted the hr-irbuilder-const branch June 18, 2025 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants