Skip to content

Commit 5528388

Browse files
authored
EarlyCSE: fix CmpPredicate duplicate-hashing (#119902)
Strip hash_value() for CmpPredicate, as different callers have different hashing use-cases. In this case, there is just one caller, namely EarlyCSE, which calls hash_combine() on a CmpPredicate, which used to call hash_combine() on a CmpInst::Predicate prior to 4a0d53a (PatternMatch: migrate to CmpPredicate). This has uncovered a bug where two icmp instructions differing in just the fact that one of them has the samesign flag on it are hashed differently, leading to divergent hashing, and a crash. Fix this crash by dropping samesign information on icmp instructions before hashing them, preserving the former behavior. Fixes #119893.
1 parent cd093c2 commit 5528388

File tree

4 files changed

+23
-11
lines changed

4 files changed

+23
-11
lines changed

llvm/include/llvm/IR/CmpPredicate.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,7 @@ class CmpPredicate {
7171

7272
/// Get the swapped predicate of a CmpInst.
7373
static CmpPredicate getSwapped(const CmpInst *Cmp);
74-
75-
/// Provided to facilitate storing a CmpPredicate in data structures that
76-
/// require hashing.
77-
friend hash_code hash_value(const CmpPredicate &Arg); // NOLINT
7874
};
79-
80-
[[nodiscard]] hash_code hash_value(const CmpPredicate &Arg);
8175
} // namespace llvm
8276

8377
#endif

llvm/lib/IR/Instructions.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3946,10 +3946,6 @@ CmpPredicate CmpPredicate::getSwapped(const CmpInst *Cmp) {
39463946
return getSwapped(get(Cmp));
39473947
}
39483948

3949-
hash_code llvm::hash_value(const CmpPredicate &Arg) { // NOLINT
3950-
return hash_combine(Arg.Pred, Arg.HasSameSign);
3951-
}
3952-
39533949
//===----------------------------------------------------------------------===//
39543950
// SwitchInst Implementation
39553951
//===----------------------------------------------------------------------===//

llvm/lib/Transforms/Scalar/EarlyCSE.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,8 @@ static unsigned getHashValueImpl(SimpleValue Val) {
290290
Pred = CmpInst::getInversePredicate(Pred);
291291
std::swap(A, B);
292292
}
293-
return hash_combine(Inst->getOpcode(), Pred, X, Y, A, B);
293+
return hash_combine(Inst->getOpcode(),
294+
static_cast<CmpInst::Predicate>(Pred), X, Y, A, B);
294295
}
295296

296297
if (CastInst *CI = dyn_cast<CastInst>(Inst))
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
2+
; RUN: opt -passes=early-cse -S %s | FileCheck %s
3+
4+
define i32 @samesign_hash_bug(i16 %v) {
5+
; CHECK-LABEL: define i32 @samesign_hash_bug(
6+
; CHECK-SAME: i16 [[V:%.*]]) {
7+
; CHECK-NEXT: [[ZEXT:%.*]] = zext i16 [[V]] to i32
8+
; CHECK-NEXT: [[ICMP_SAMESIGN:%.*]] = icmp ugt i32 [[ZEXT]], 31
9+
; CHECK-NEXT: [[SELECT_ICMP_SAMESIGN:%.*]] = select i1 [[ICMP_SAMESIGN]], i32 0, i32 1
10+
; CHECK-NEXT: [[SELECT_ICMP:%.*]] = select i1 [[ICMP_SAMESIGN]], i32 1, i32 0
11+
; CHECK-NEXT: ret i32 [[SELECT_ICMP]]
12+
;
13+
%zext = zext i16 %v to i32
14+
%icmp.samesign = icmp samesign ugt i32 %zext, 31
15+
%select.icmp.samesign = select i1 %icmp.samesign, i32 0, i32 1
16+
%ashr = ashr i32 0, %select.icmp.samesign
17+
%icmp = icmp ugt i32 %zext, 31
18+
%select.icmp = select i1 %icmp, i32 1, i32 0
19+
%ret = add i32 %ashr, %select.icmp
20+
ret i32 %ret
21+
}

0 commit comments

Comments
 (0)