diff --git a/llvm/lib/Analysis/HashRecognize.cpp b/llvm/lib/Analysis/HashRecognize.cpp index 2cc3ad5f18482..662260a79ab7a 100644 --- a/llvm/lib/Analysis/HashRecognize.cpp +++ b/llvm/lib/Analysis/HashRecognize.cpp @@ -102,9 +102,10 @@ class ValueEvolution { public: // ValueEvolution is meant to be constructed with the TripCount of the loop, - // and whether the polynomial algorithm is big-endian, for the significant-bit - // check. - ValueEvolution(unsigned TripCount, bool ByteOrderSwapped); + // whether the polynomial algorithm is big-endian for the significant-bit + // check, and an initial value for the Visited set. + ValueEvolution(unsigned TripCount, bool ByteOrderSwapped, + ArrayRef InitVisited); // Given a list of PHI nodes along with their incoming value from within the // loop, computeEvolutions computes the KnownBits of each of the PHI nodes on @@ -115,13 +116,19 @@ class ValueEvolution { // precise error message. StringRef getError() const { return ErrStr; } + // A set of instructions visited by ValueEvolution. Anything that's not in the + // use-def chain of the PHIs' evolution will not be visited. + SmallPtrSet Visited; + // The computed KnownBits for each PHI node, which is populated after // computeEvolutions is called. KnownPhiMap KnownPhis; }; -ValueEvolution::ValueEvolution(unsigned TripCount, bool ByteOrderSwapped) - : TripCount(TripCount), ByteOrderSwapped(ByteOrderSwapped) {} +ValueEvolution::ValueEvolution(unsigned TripCount, bool ByteOrderSwapped, + ArrayRef InitVisited) + : TripCount(TripCount), ByteOrderSwapped(ByteOrderSwapped), + Visited(InitVisited.begin(), InitVisited.end()) {} KnownBits ValueEvolution::computeBinOp(const BinaryOperator *I) { KnownBits KnownL(compute(I->getOperand(0))); @@ -177,6 +184,9 @@ KnownBits ValueEvolution::computeBinOp(const BinaryOperator *I) { KnownBits ValueEvolution::computeInstr(const Instruction *I) { unsigned BitWidth = I->getType()->getScalarSizeInBits(); + // computeInstr is the only entry-point that needs to update the Visited set. + Visited.insert(I); + // We look up in the map that contains the KnownBits of the PHI from the // previous iteration. if (const PHINode *P = dyn_cast(I)) @@ -185,9 +195,14 @@ KnownBits ValueEvolution::computeInstr(const Instruction *I) { // Compute the KnownBits for a Select(Cmp()), forcing it to take the branch // that is predicated on the (least|most)-significant-bit check. CmpPredicate Pred; - Value *L, *R, *TV, *FV; - if (match(I, m_Select(m_ICmp(Pred, m_Value(L), m_Value(R)), m_Value(TV), - m_Value(FV)))) { + Value *L, *R; + Instruction *TV, *FV; + if (match(I, m_Select(m_ICmp(Pred, m_Value(L), m_Value(R)), m_Instruction(TV), + m_Instruction(FV)))) { + Visited.insert(cast(I->getOperand(0))); + Visited.insert(TV); + Visited.insert(FV); + // 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) { @@ -209,6 +224,9 @@ KnownBits ValueEvolution::computeInstr(const Instruction *I) { ConstantRange CheckRCR(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) return compute(TV); if (AllowedR.inverse() == CheckRCR) @@ -629,11 +647,23 @@ HashRecognize::recognizeCRC() const { if (SimpleRecurrence) PhiEvolutions.emplace_back(SimpleRecurrence.Phi, SimpleRecurrence.BO); - ValueEvolution VE(TC, *ByteOrderSwapped); + // Initialize the Visited set in ValueEvolution with the IndVar-related + // instructions. + std::initializer_list InitVisited = { + IndVar, Latch->getTerminator(), L.getLatchCmpInst(), + cast(IndVar->getIncomingValueForBlock(Latch))}; + + ValueEvolution VE(TC, *ByteOrderSwapped, InitVisited); if (!VE.computeEvolutions(PhiEvolutions)) return VE.getError(); KnownBits ResultBits = VE.KnownPhis.at(ConditionalRecurrence.Phi); + // Any unvisited instructions from the KnownBits propagation can complicate + // the optimization, which would just replace the entire loop with the + // table-lookup version of the hash algorithm. + if (std::distance(Latch->begin(), Latch->end()) != VE.Visited.size()) + return "Found stray unvisited instructions"; + unsigned N = std::min(TC, ResultBits.getBitWidth()); auto IsZero = [](const KnownBits &K) { return K.isZero(); }; if (!checkExtractBits(ResultBits, N, IsZero, *ByteOrderSwapped)) diff --git a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll index 247a105940e6e..3926c467375ed 100644 --- a/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll +++ b/llvm/test/Analysis/HashRecognize/cyclic-redundancy-check.ll @@ -909,10 +909,10 @@ exit: ; preds = %loop ret i16 %crc.next } -define i16 @not.crc.bad.cast(i8 %msg, i16 %checksum) { -; CHECK-LABEL: 'not.crc.bad.cast' +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: Expected bottom 8 bits zero (????????00001011) +; CHECK-NEXT: Reason: Found stray unvisited instructions ; entry: br label %loop @@ -1189,3 +1189,29 @@ loop: ; preds = %loop, %entry exit: ; preds = %loop ret i16 %crc.next } + +define i16 @not.crc.stray.unvisited.call(i16 %crc.init) { +; CHECK-LABEL: 'not.crc.stray.unvisited.call' +; CHECK-NEXT: Did not find a hash algorithm +; CHECK-NEXT: Reason: Found stray unvisited instructions +; +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, 0 + %crc.next = select i1 %check.sb, i16 %crc.xor, i16 %crc.shl + call void @print(i16 %crc.next) + %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 +} + +declare void @print(i16)