Skip to content

Commit d41fa0c

Browse files
committed
Remove whitelist from matchSimpleRecurrence.
Note: This also fixes a latent bug in HashRecognize due to unhandled opcodes.
1 parent ffa3807 commit d41fa0c

File tree

3 files changed

+47
-36
lines changed

3 files changed

+47
-36
lines changed

llvm/lib/Analysis/HashRecognize.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,11 @@ static bool arePHIsIntertwined(
542542
// doing this, we're immune to whether the IR expression is mul/udiv or
543543
// equivalently shl/lshr. Return false when it is a UDiv, true when it is a Mul,
544544
// and std::nullopt otherwise.
545-
static std::optional<bool> isBigEndianBitShift(const SCEV *E) {
545+
static std::optional<bool> isBigEndianBitShift(Value *V, ScalarEvolution &SE) {
546+
if (!V->getType()->isIntegerTy())
547+
return {};
548+
549+
const SCEV *E = SE.getSCEV(V);
546550
if (match(E, m_scev_UDiv(m_SCEV(), m_scev_SpecificInt(2))))
547551
return false;
548552
if (match(E, m_scev_Mul(m_scev_SpecificInt(2), m_SCEV())))
@@ -576,12 +580,11 @@ HashRecognize::recognizeCRC() const {
576580
// Make sure that all recurrences are either all SCEVMul with two or SCEVDiv
577581
// with two, or in other words, that they're single bit-shifts.
578582
std::optional<bool> ByteOrderSwapped =
579-
isBigEndianBitShift(SE.getSCEV(ConditionalRecurrence.BO));
583+
isBigEndianBitShift(ConditionalRecurrence.BO, SE);
580584
if (!ByteOrderSwapped)
581585
return "Loop with non-unit bitshifts";
582586
if (SimpleRecurrence) {
583-
if (isBigEndianBitShift(SE.getSCEV(SimpleRecurrence.BO)) !=
584-
ByteOrderSwapped)
587+
if (isBigEndianBitShift(SimpleRecurrence.BO, SE) != ByteOrderSwapped)
585588
return "Loop with non-unit bitshifts";
586589
if (!arePHIsIntertwined(SimpleRecurrence.Phi, ConditionalRecurrence.Phi, L,
587590
Instruction::BinaryOps::Xor))

llvm/lib/Analysis/ValueTracking.cpp

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -9058,6 +9058,7 @@ bool llvm::matchSimpleRecurrence(const PHINode *P, BinaryOperator *&BO,
90589058
// Handle the case of a simple two-predecessor recurrence PHI.
90599059
// There's a lot more that could theoretically be done here, but
90609060
// this is sufficient to catch some interesting cases.
9061+
// TODO: Expand list -- gep, uadd.sat etc.
90619062
if (P->getNumIncomingValues() != 2)
90629063
return false;
90639064

@@ -9068,37 +9069,16 @@ bool llvm::matchSimpleRecurrence(const PHINode *P, BinaryOperator *&BO,
90689069
if (!LU)
90699070
continue;
90709071
unsigned Opcode = LU->getOpcode();
9071-
9072-
switch (Opcode) {
9073-
default:
9074-
continue;
9075-
// TODO: Expand list -- gep, uadd.sat etc.
9076-
case Instruction::LShr:
9077-
case Instruction::AShr:
9078-
case Instruction::Shl:
9079-
case Instruction::Add:
9080-
case Instruction::Sub:
9081-
case Instruction::UDiv:
9082-
case Instruction::URem:
9083-
case Instruction::And:
9084-
case Instruction::Or:
9085-
case Instruction::Xor:
9086-
case Instruction::Mul:
9087-
case Instruction::FAdd:
9088-
case Instruction::FMul: {
9089-
Value *LL = LU->getOperand(0);
9090-
Value *LR = LU->getOperand(1);
9091-
// Find a recurrence.
9092-
if (LL == P)
9093-
L = LR;
9094-
else if (LR == P)
9095-
L = LL;
9096-
else
9097-
continue; // Check for recurrence with L and R flipped.
9098-
9099-
break; // Match!
9100-
}
9101-
};
9072+
Value *LL = LU->getOperand(0);
9073+
Value *LR = LU->getOperand(1);
9074+
9075+
// Find a recurrence.
9076+
if (LL == P)
9077+
L = LR;
9078+
else if (LR == P)
9079+
L = LL;
9080+
else
9081+
continue; // Check for recurrence with L and R flipped.
91029082

91039083
// We have matched a recurrence of the form:
91049084
// %iv = [R, %entry], [%iv.next, %backedge]

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

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -873,7 +873,7 @@ exit: ; preds = %loop
873873
define i16 @not.crc.float.simple.recurrence(float %msg, i16 %checksum) {
874874
; CHECK-LABEL: 'not.crc.float.simple.recurrence'
875875
; CHECK-NEXT: Did not find a hash algorithm
876-
; CHECK-NEXT: Reason: Found stray PHI
876+
; CHECK-NEXT: Reason: Loop with non-unit bitshifts
877877
;
878878
entry:
879879
br label %loop
@@ -897,3 +897,31 @@ loop: ; preds = %loop, %entry
897897
exit: ; preds = %loop
898898
ret i16 %crc.next
899899
}
900+
901+
define i16 @not.crc.stray.phi(i8 %msg, i16 %checksum, i1 %c) {
902+
; CHECK-LABEL: 'not.crc.stray.phi'
903+
; CHECK-NEXT: Did not find a hash algorithm
904+
; CHECK-NEXT: Reason: Found stray PHI
905+
;
906+
entry:
907+
br label %loop
908+
909+
loop: ; preds = %loop, %entry
910+
%iv = phi i8 [ 0, %entry ], [ %iv.next, %loop ]
911+
%crc = phi i16 [ %checksum, %entry ], [ %crc.next, %loop ]
912+
%data = phi i8 [ %msg, %entry ], [ %data.next, %loop ]
913+
%crc.trunc = trunc i16 %crc to i8
914+
%xor.data.crc = xor i8 %data, %crc.trunc
915+
%and.data.crc = and i8 %xor.data.crc, 1
916+
%data.next = select i1 %c, i8 %data, i8 1
917+
%check.sb = icmp eq i8 %and.data.crc, 0
918+
%crc.lshr = lshr i16 %crc, 1
919+
%xor = xor i16 %crc.lshr, -24575
920+
%crc.next = select i1 %check.sb, i16 %crc.lshr, i16 %xor
921+
%iv.next = add nuw nsw i8 %iv, 1
922+
%exit.cond = icmp samesign ult i8 %iv, 7
923+
br i1 %exit.cond, label %loop, label %exit
924+
925+
exit: ; preds = %loop
926+
ret i16 %crc.next
927+
}

0 commit comments

Comments
 (0)