Skip to content

Commit cca454b

Browse files
authored
[ValueTracking] Remove opcode whitelist from matchSimpleRecurrence. (#144031)
This also patches HashRecognize to avoid it mishandling some opcodes.
1 parent fbade95 commit cca454b

File tree

3 files changed

+47
-34
lines changed

3 files changed

+47
-34
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 & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9071,6 +9071,7 @@ bool llvm::matchSimpleRecurrence(const PHINode *P, BinaryOperator *&BO,
90719071
// Handle the case of a simple two-predecessor recurrence PHI.
90729072
// There's a lot more that could theoretically be done here, but
90739073
// this is sufficient to catch some interesting cases.
9074+
// TODO: Expand list -- gep, uadd.sat etc.
90749075
if (P->getNumIncomingValues() != 2)
90759076
return false;
90769077

@@ -9081,35 +9082,16 @@ bool llvm::matchSimpleRecurrence(const PHINode *P, BinaryOperator *&BO,
90819082
if (!LU)
90829083
continue;
90839084
unsigned Opcode = LU->getOpcode();
9084-
9085-
switch (Opcode) {
9086-
default:
9087-
continue;
9088-
// TODO: Expand list -- xor, gep, uadd.sat etc.
9089-
case Instruction::LShr:
9090-
case Instruction::AShr:
9091-
case Instruction::Shl:
9092-
case Instruction::Add:
9093-
case Instruction::Sub:
9094-
case Instruction::UDiv:
9095-
case Instruction::URem:
9096-
case Instruction::And:
9097-
case Instruction::Or:
9098-
case Instruction::Mul:
9099-
case Instruction::FMul: {
9100-
Value *LL = LU->getOperand(0);
9101-
Value *LR = LU->getOperand(1);
9102-
// Find a recurrence.
9103-
if (LL == P)
9104-
L = LR;
9105-
else if (LR == P)
9106-
L = LL;
9107-
else
9108-
continue; // Check for recurrence with L and R flipped.
9109-
9110-
break; // Match!
9111-
}
9112-
};
9085+
Value *LL = LU->getOperand(0);
9086+
Value *LR = LU->getOperand(1);
9087+
9088+
// Find a recurrence.
9089+
if (LL == P)
9090+
L = LR;
9091+
else if (LR == P)
9092+
L = LL;
9093+
else
9094+
continue; // Check for recurrence with L and R flipped.
91139095

91149096
// We have matched a recurrence of the form:
91159097
// %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)