[HashRecognize] Rewrite arePHIsIntertwined

llvm/include/llvm/IR/PatternMatch.h

@@ -2119,6 +2119,13 @@ m_IntToPtr(const OpTy &Op) {
   return CastOperator_match<OpTy, Instruction::IntToPtr>(Op);
 }
 
+/// Matches any cast or self. Used to ignore casts.
+template <typename OpTy>
+inline match_combine_or<CastInst_match<OpTy, CastInst>, OpTy>
+m_CastOrSelf(const OpTy &Op) {
+  return m_CombineOr(CastInst_match<OpTy, CastInst>(Op), Op);
+}
+
 /// Matches Trunc.
 template <typename OpTy>
 inline CastInst_match<OpTy, TruncInst> m_Trunc(const OpTy &Op) {

llvm/lib/Analysis/HashRecognize.cpp

@@ -497,42 +497,46 @@ CRCTable HashRecognize::genSarwateTable(const APInt &GenPoly,
   return Table;
 }
 
-/// Checks if \p Reference is reachable from \p Needle on the use-def chain, and
-/// that there are no stray PHI nodes while digging the use-def chain. \p
-/// BOToMatch is a CRC peculiarity: at least one of the Users of Needle needs to
-/// match this OpCode, which is XOR for CRC.
-static bool arePHIsIntertwined(
-    const PHINode *Needle, const PHINode *Reference, const Loop &L,
-    Instruction::BinaryOps BOToMatch = Instruction::BinaryOpsEnd) {
-  // Initialize the worklist with Users of the Needle.
-  SmallVector<const Instruction *> Worklist;
-  for (const User *U : Needle->users()) {
-    if (auto *UI = dyn_cast<Instruction>(U))
-      if (L.contains(UI))
-        Worklist.push_back(UI);
-  }
+/// Checks that \p L and \p R are used together in an XOR in the use-def chain
+/// of \p SI's condition, ignoring any casts. The purpose of this function is to
+/// ensure that LHSAux from the SimpleRecurrence is used correctly in the CRC
+/// computation. We cannot check the correctness of casts at this point, and
+/// rely on the KnownBits propagation to check correctness of the CRC
+/// computation.
+///
+/// In other words, it checks for the following pattern:
+///
+/// loop:
+///   %L = phi [_, %entry], [%L.next, %loop]
+///   %R = phi [_, %entry], [%R.next, %loop]
+///   ...
+///   %xor = xor (CastOrSelf %L), (CastOrSelf %R)
+///
+/// where %xor is in the use-def chain of \p SI's condition.
+static bool isConditionalOnXorOfPHIs(SelectInst *SI, const PHINode *P1,
+                                     const PHINode *P2, const Loop &L) {
+  SmallVector<Instruction *> Worklist;
 
-  // BOToMatch is usually XOR for CRC.
-  if (BOToMatch != Instruction::BinaryOpsEnd) {
-    if (count_if(Worklist, [BOToMatch](const Instruction *I) {
-          return I->getOpcode() == BOToMatch;
-        }) != 1)
-      return false;
-  }
+  // matchConditionalRecurrence has already ensured that the SelectInst's
+  // condition is an Instruction.
+  Worklist.push_back(cast<Instruction>(SI->getCondition()));
 
   while (!Worklist.empty()) {
-    const Instruction *I = Worklist.pop_back_val();
+    Instruction *I = Worklist.pop_back_val();
 
-    // Since Needle is never pushed onto the Worklist, I must either be the
-    // Reference PHI node (in which case we're done), or a stray PHI node (in
-    // which case we abort).
+    // Don't add a PHI's operands to the Worklist.
     if (isa<PHINode>(I))
-      return I == Reference;
+      continue;
+
+    // If we match an XOR of the two PHIs ignoring casts, we're done.
+    if (match(I, m_c_Xor(m_CastOrSelf(m_Specific(P1)),
+                         m_CastOrSelf(m_Specific(P2)))))
+      return true;
 
-    for (const Use &U : I->operands())
+    // Continue along the use-def chain.
+    for (Use &U : I->operands())
       if (auto *UI = dyn_cast<Instruction>(U))
-        // Don't push Needle back onto the Worklist.
-        if (UI != Needle && L.contains(UI))
+        if (L.contains(UI))
           Worklist.push_back(UI);
   }
   return false;
@@ -586,9 +590,13 @@ HashRecognize::recognizeCRC() const {
   if (SimpleRecurrence) {
     if (isBigEndianBitShift(SimpleRecurrence.BO, SE) != ByteOrderSwapped)
       return "Loop with non-unit bitshifts";
-    if (!arePHIsIntertwined(SimpleRecurrence.Phi, ConditionalRecurrence.Phi, L,
-                            Instruction::BinaryOps::Xor))
-      return "Simple recurrence doesn't use conditional recurrence with XOR";
+
+    // Check that the SelectInst ConditionalRecurrence.Step is conditional on
+    // the XOR of SimpleRecurrence.Phi and ConditionalRecurrence.Phi.
+    if (!isConditionalOnXorOfPHIs(cast<SelectInst>(ConditionalRecurrence.Step),
+                                  SimpleRecurrence.Phi,
+                                  ConditionalRecurrence.Phi, L))
+      return "Recurrences not intertwined with XOR";
   }
 
   // Make sure that the computed value is used in the exit block: this should be

llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll

@@ -144,6 +144,34 @@ exit:                                              ; preds = %loop
   ret i16 %crc.next
 }
 
+define i8 @crc8.le.tc16(i16 %msg, i8 %checksum) {
+; CHECK-LABEL: 'crc8.le.tc16'
+; CHECK-NEXT:  Did not find a hash algorithm
+; CHECK-NEXT:  Reason: Loop iterations exceed bitwidth of result
+;
+entry:
+  br label %loop
+
+loop:                                              ; preds = %loop, %entry
+  %iv = phi i8 [ 0, %entry ], [ %iv.next, %loop ]
+  %crc = phi i8 [ %checksum, %entry ], [ %crc.next, %loop ]
+  %data = phi i16 [ %msg, %entry ], [ %data.next, %loop ]
+  %data.trunc = trunc i16 %data to i8
+  %xor.crc.data = xor i8 %crc, %data.trunc
+  %and.crc.data = and i8 %xor.crc.data, 1
+  %data.next = lshr i16 %data, 1
+  %check.sb = icmp eq i8 %and.crc.data, 0
+  %crc.lshr = lshr i8 %crc, 1
+  %crc.xor = xor i8 %crc.lshr, 29
+  %crc.next = select i1 %check.sb, i8 %crc.lshr, i8 %crc.xor
+  %iv.next = add nuw nsw i8 %iv, 1
+  %exit.cond = icmp samesign ult i8 %iv, 15
+  br i1 %exit.cond, label %loop, label %exit
+
+exit:                                              ; preds = %loop
+  ret i8 %crc.next
+}
+
 define i16 @crc16.be.tc8.crc.init.li(i16 %checksum, i8 %msg) {
 ; CHECK-LABEL: 'crc16.be.tc8.crc.init.li'
 ; CHECK-NEXT:  Found big-endian CRC-16 loop with trip count 8
@@ -601,7 +629,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: Simple recurrence doesn't use conditional recurrence with XOR
+; CHECK-NEXT:  Reason: Bad RHS of significant-bit-check
 ;
 entry:
   br label %loop
@@ -610,9 +638,8 @@ loop:                                              ; preds = %loop, %entry
   %iv = phi i8 [ 0, %entry ], [ %iv.next, %loop ]
   %data = phi i8 [ %msg, %entry ], [ %data.next, %loop ]
   %crc = phi i16 [ %checksum, %entry ], [ %crc.next, %loop ]
-  %crc.lshr = lshr i16 %crc, 8
   %data.ext = zext i8 %data to i16
-  %xor.crc.data = xor i16 %crc.lshr, %data.ext
+  %xor.crc.data = xor i16 %crc, %data.ext
   %check.sb = icmp samesign ult i16 %xor.crc.data, 128
   %crc.shl = shl i16 %crc, 1
   %crc.xor = xor i16 %crc.shl, 258
@@ -838,10 +865,37 @@ exit:                                              ; preds = %loop
   ret i16 %crc.next
 }
 
+define i16 @not.crc.bad.cast(i8 %msg, i16 %checksum) {
+; CHECK-LABEL: 'not.crc.bad.cast'
+; CHECK-NEXT:  Did not find a hash algorithm
+; CHECK-NEXT:  Reason: Expected bottom 8 bits zero (????????00001011)
+;
+entry:
+  br label %loop
+
+loop:                                              ; preds = %loop, %entry
+  %iv = phi i8 [ 0, %entry ], [ %iv.next, %loop ]
+  %data = phi i8 [ %msg, %entry ], [ %data.next, %loop ]
+  %crc = phi i16 [ %checksum, %entry ], [ %crc.next, %loop ]
+  %data.ext = zext i8 %data to i16
+  %xor.crc.data = xor i16 %crc, %data.ext
+  %check.sb = icmp slt i16 %xor.crc.data, 0
+  %crc.shl = shl i16 %crc, 1
+  %crc.xor = xor i16 %crc.shl, 29
+  %crc.next = select i1 %check.sb, i16 %crc.shl, i16 %crc.xor
+  %data.next = shl i8 %data, 1
+  %iv.next = add nuw nsw i8 %iv, 1
+  %exit.cond = icmp samesign ult i8 %iv, 7
+  br i1 %exit.cond, label %loop, label %exit
+
+exit:                                              ; preds = %loop
+  ret i16 %crc.next
+}
+
 define i32 @not.crc.dead.msg.bad.use(i32 %checksum, i32 %msg) {
 ; CHECK-LABEL: 'not.crc.dead.msg.bad.use'
 ; CHECK-NEXT:  Did not find a hash algorithm
-; CHECK-NEXT:  Reason: Simple recurrence doesn't use conditional recurrence with XOR
+; CHECK-NEXT:  Reason: Recurrences not intertwined with XOR
 ;
 entry:
   br label %loop
@@ -869,7 +923,7 @@ exit:                                              ; preds = %loop
 define i16 @not.crc.dead.msg.no.use(i8 %msg, i16 %checksum) {
 ; CHECK-LABEL: 'not.crc.dead.msg.no.use'
 ; CHECK-NEXT:  Did not find a hash algorithm
-; CHECK-NEXT:  Reason: Simple recurrence doesn't use conditional recurrence with XOR
+; CHECK-NEXT:  Reason: Recurrences not intertwined with XOR
 ;
 entry:
   br label %loop
@@ -898,7 +952,7 @@ exit:                                              ; preds = %loop
 define i32 @not.crc.dead.msg.wrong.op(i32 %checksum, i32 %msg) {
 ; CHECK-LABEL: 'not.crc.dead.msg.wrong.op'
 ; CHECK-NEXT:  Did not find a hash algorithm
-; CHECK-NEXT:  Reason: Simple recurrence doesn't use conditional recurrence with XOR
+; CHECK-NEXT:  Reason: Recurrences not intertwined with XOR
 ;
 entry:
   br label %loop
@@ -922,6 +976,34 @@ exit:                                              ; preds = %loop
   ret i32 %crc.next
 }
 
+define i16 @not.crc.dead.msg.xor.notin.select.chain(i16 %msg, i16 %checksum) {
+; CHECK-LABEL: 'not.crc.dead.msg.xor.notin.select.chain'
+; CHECK-NEXT:  Did not find a hash algorithm
+; CHECK-NEXT:  Reason: Recurrences not intertwined with XOR
+;
+entry:
+  br label %loop
+
+loop:                                              ; preds = %loop, %entry
+  %iv = phi i8 [ 0, %entry ], [ %iv.next, %loop ]
+  %crc = phi i16 [ %checksum, %entry ], [ %crc.next, %loop ]
+  %data = phi i16 [ %msg, %entry ], [ %data.next, %loop ]
+  %xor.crc.data = xor i16 %crc, %data
+  %or.crc.data = or i16 %crc, %data
+  %and.crc.data = and i16 %or.crc.data, 1
+  %data.next = lshr i16 %data, 1
+  %check.sb = icmp eq i16 %and.crc.data, 0
+  %crc.lshr = lshr i16 %crc, 1
+  %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, 15
+  br i1 %exit.cond, label %loop, label %exit
+
+exit:                                              ; preds = %loop
+  ret i16 %crc.next
+}
+
 define i16 @not.crc.float.simple.recurrence(float %msg, i16 %checksum) {
 ; CHECK-LABEL: 'not.crc.float.simple.recurrence'
 ; CHECK-NEXT:  Did not find a hash algorithm