[EarlyCSE] Fix hashing of self-compares

JosephTremoulet · JosephTremoulet · commit daa1ae614295 · 2019-06-17T19:11:28.000Z
Summary: Update compare normalization in SimpleValue hashing to break ties (when the same value is being compared to itself) by switching to the swapped predicate if it has a lower numerical value. This brings the hashing in line with isEqual, which already recognizes the self-compares with swapped predicates as equal. Fixes PR 42280. Reviewers: spatel, efriedma, nikic, fhahn, uabelho Reviewed By: nikic Subscribers: hiraditya, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D63349 llvm-svn: 363598
diff --git a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -174,12 +174,17 @@ static unsigned getHashValueImpl(SimpleValue Val) {
   }
 
   if (CmpInst *CI = dyn_cast<CmpInst>(Inst)) {
+    // Compares can be commuted by swapping the comparands and
+    // updating the predicate.  Choose the form that has the
+    // comparands in sorted order, or in the case of a tie, the
+    // one with the lower predicate.
     Value *LHS = CI->getOperand(0);
     Value *RHS = CI->getOperand(1);
     CmpInst::Predicate Pred = CI->getPredicate();
-    if (Inst->getOperand(0) > Inst->getOperand(1)) {
+    CmpInst::Predicate SwappedPred = CI->getSwappedPredicate();
+    if (std::tie(LHS, Pred) > std::tie(RHS, SwappedPred)) {
       std::swap(LHS, RHS);
-      Pred = CI->getSwappedPredicate();
+      Pred = SwappedPred;
     }
     return hash_combine(Inst->getOpcode(), Pred, LHS, RHS);
   }
diff --git a/llvm/test/Transforms/EarlyCSE/commute.ll b/llvm/test/Transforms/EarlyCSE/commute.ll
@@ -72,6 +72,22 @@ define void @test5(i32 %A, i32 %B, i1* %PA, i1* %PB) {
   ret void
 }
 
+; Test degenerate case of commuted compare of identical comparands.
+
+define void @test6(float %f, i1* %p1, i1* %p2) {
+; CHECK-LABEL: @test6(
+; CHECK-NEXT:    [[C1:%.*]] = fcmp ult float [[F:%.*]], [[F]]
+; CHECK-NEXT:    store i1 [[C1]], i1* [[P1:%.*]]
+; CHECK-NEXT:    store i1 [[C1]], i1* [[P2:%.*]]
+; CHECK-NEXT:    ret void
+;
+  %c1 = fcmp ult float %f, %f
+  %c2 = fcmp ugt float %f, %f
+  store i1 %c1, i1* %p1
+  store i1 %c2, i1* %p2
+  ret void
+}
+
 ; Min/max operands may be commuted in the compare and select.
 
 define i8 @smin_commute(i8 %a, i8 %b) {

Original file line number	Diff line number	Diff line change
`@@ -174,12 +174,17 @@ static unsigned getHashValueImpl(SimpleValue Val) {`
`174`	`174`	`}`
`175`	`175`
`176`	`176`	`if (CmpInst *CI = dyn_cast<CmpInst>(Inst)) {`
	`177`	`+ // Compares can be commuted by swapping the comparands and`
	`178`	`+ // updating the predicate. Choose the form that has the`
	`179`	`+ // comparands in sorted order, or in the case of a tie, the`
	`180`	`+ // one with the lower predicate.`
`177`	`181`	`Value *LHS = CI->getOperand(0);`
`178`	`182`	`Value *RHS = CI->getOperand(1);`
`179`	`183`	`CmpInst::Predicate Pred = CI->getPredicate();`
`180`		`- if (Inst->getOperand(0) > Inst->getOperand(1)) {`
	`184`	`+ CmpInst::Predicate SwappedPred = CI->getSwappedPredicate();`
	`185`	`+ if (std::tie(LHS, Pred) > std::tie(RHS, SwappedPred)) {`
`181`	`186`	`std::swap(LHS, RHS);`
`182`		`- Pred = CI->getSwappedPredicate();`
	`187`	`+ Pred = SwappedPred;`
`183`	`188`	`}`
`184`	`189`	`return hash_combine(Inst->getOpcode(), Pred, LHS, RHS);`
`185`	`190`	`}`