[HashRecognize] Address review

Author: artagnon

llvm/lib/Analysis/HashRecognize.cpp

@@ -90,7 +90,6 @@ class ValueEvolution {
   const bool ByteOrderSwapped;
   APInt GenPoly;
   StringRef ErrStr;
-  KnownPhiMap KnownPhis;
 
   KnownBits computeBinOp(const BinaryOperator *I);
   KnownBits computeInstr(const Instruction *I);
@@ -107,8 +106,10 @@ class ValueEvolution {
   // Given a list of PHI nodes along with their incoming value from within the
   // loop, and the trip-count of the loop, computeEvolutions
   // computes the KnownBits of each of the PHI nodes on the final iteration.
-  std::optional<KnownPhiMap>
-  computeEvolutions(ArrayRef<PhiStepPair> PhiEvolutions);
+  bool computeEvolutions(ArrayRef<PhiStepPair> PhiEvolutions);
+
+  // The resulting KnownBits for each PHI node.
+  KnownPhiMap KnownPhis;
 };
 
 ValueEvolution::ValueEvolution(unsigned TripCount, bool ByteOrderSwapped)
@@ -184,7 +185,8 @@ KnownBits ValueEvolution::computeInstr(const Instruction *I) {
     // 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).zextOrTrunc(BitWidth);
+      KnownBits KnownL = compute(L);
+      BitWidth = KnownL.getBitWidth();
       auto LCR = ConstantRange::fromKnownBits(KnownL, false);
       auto CheckLCR =
           ConstantRange(APInt::getZero(BitWidth), APInt(BitWidth, 2));
@@ -195,13 +197,13 @@ KnownBits ValueEvolution::computeInstr(const Instruction *I) {
     }
 
     // Check that the predication is on (most|least) significant bit.
-    KnownBits KnownR = compute(R).zextOrTrunc(BitWidth);
+    KnownBits KnownR = compute(R);
+    BitWidth = KnownR.getBitWidth();
     auto RCR = ConstantRange::fromKnownBits(KnownR, false);
     auto AllowedR = ConstantRange::makeAllowedICmpRegion(Pred, RCR);
-    ConstantRange LSBRange(APInt::getZero(BitWidth), APInt(BitWidth, 1));
-    ConstantRange MSBRange(APInt::getZero(BitWidth),
-                           APInt::getSignedMinValue(BitWidth));
-    const ConstantRange &CheckRCR = ByteOrderSwapped ? MSBRange : LSBRange;
+    ConstantRange CheckRCR(APInt::getZero(BitWidth),
+                           ByteOrderSwapped ? APInt::getSignedMinValue(BitWidth)
+                                            : APInt(BitWidth, 1));
     if (AllowedR == CheckRCR)
       return compute(TV);
     if (AllowedR.inverse() == CheckRCR)
@@ -242,19 +244,18 @@ KnownBits ValueEvolution::compute(const Value *V) {
 
 // Takes every PHI-step pair in PhiEvolutions, and computes KnownBits on the
 // final iteration, using KnownBits from the previous iteration.
-std::optional<KnownPhiMap>
-ValueEvolution::computeEvolutions(ArrayRef<PhiStepPair> PhiEvolutions) {
+bool ValueEvolution::computeEvolutions(ArrayRef<PhiStepPair> PhiEvolutions) {
   for (unsigned I = 0; I < TripCount; ++I) {
     for (auto [Phi, Step] : PhiEvolutions) {
       KnownBits KnownAtIter = computeInstr(Step);
       if (KnownAtIter.getBitWidth() < I + 1) {
         ErrStr = "Loop iterations exceed bitwidth of result";
-        return std::nullopt;
+        return false;
       }
       KnownPhis.emplace_or_assign(Phi, KnownAtIter);
     }
   }
-  return hasError() ? std::nullopt : std::make_optional(KnownPhis);
+  return !hasError();
 }
 
 /// A structure that can hold either a Simple Recurrence or a Conditional
@@ -605,12 +606,10 @@ HashRecognize::recognizeCRC() const {
     PhiEvolutions.emplace_back(SimpleRecurrence.Phi, SimpleRecurrence.BO);
 
   ValueEvolution VE(TC, *ByteOrderSwapped);
-  std::optional<KnownPhiMap> KnownPhis = VE.computeEvolutions(PhiEvolutions);
-
-  if (VE.hasError())
+  if (!VE.computeEvolutions(PhiEvolutions))
     return VE.getError();
+  KnownBits ResultBits = VE.KnownPhis.at(ConditionalRecurrence.Phi);
 
-  KnownBits ResultBits = KnownPhis->at(ConditionalRecurrence.Phi);
   auto IsZero = [](const KnownBits &K) { return K.isZero(); };
   if (!checkExtractBits(ResultBits, TC, IsZero, *ByteOrderSwapped))
     return ErrBits(ResultBits, TC, *ByteOrderSwapped);