[llvm] 4eb8359 - [EarlyCSE] delete abs/nabs handling

Chen Zheng via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 10 18:12:31 PST 2020


Author: Chen Zheng
Date: 2020-11-10T21:10:58-05:00
New Revision: 4eb8359e742beac06f1594e64396256a8ecc30a3

URL: https://github.com/llvm/llvm-project/commit/4eb8359e742beac06f1594e64396256a8ecc30a3
DIFF: https://github.com/llvm/llvm-project/commit/4eb8359e742beac06f1594e64396256a8ecc30a3.diff

LOG: [EarlyCSE] delete abs/nabs handling

delete abs/nabs handling in earlycse pass to avoid bugs related to
hashing values. After abs/nabs is canonicalized to intrinsics in D87188,
we should get CSE ability for abs/nabs back.

Reviewed By: spatel

Differential Revision: https://reviews.llvm.org/D90734

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/EarlyCSE.cpp
    llvm/test/Transforms/EarlyCSE/commute.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
index bd8358d12bfd..be2860960500 100644
--- a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -154,33 +154,13 @@ static bool matchSelectWithOptionalNotCond(Value *V, Value *&Cond, Value *&A,
     std::swap(A, B);
   }
 
-  // Match canonical forms of abs/nabs/min/max. We are not using ValueTracking's
+  // Match canonical forms of min/max. We are not using ValueTracking's
   // more powerful matchSelectPattern() because it may rely on instruction flags
   // such as "nsw". That would be incompatible with the current hashing
   // mechanism that may remove flags to increase the likelihood of CSE.
 
-  // These are the canonical forms of abs(X) and nabs(X) created by instcombine:
-  // %N = sub i32 0, %X
-  // %C = icmp slt i32 %X, 0
-  // %ABS = select i1 %C, i32 %N, i32 %X
-  //
-  // %N = sub i32 0, %X
-  // %C = icmp slt i32 %X, 0
-  // %NABS = select i1 %C, i32 %X, i32 %N
   Flavor = SPF_UNKNOWN;
   CmpInst::Predicate Pred;
-  if (match(Cond, m_ICmp(Pred, m_Specific(B), m_ZeroInt())) &&
-      Pred == ICmpInst::ICMP_SLT && match(A, m_Neg(m_Specific(B)))) {
-    // ABS: B < 0 ? -B : B
-    Flavor = SPF_ABS;
-    return true;
-  }
-  if (match(Cond, m_ICmp(Pred, m_Specific(A), m_ZeroInt())) &&
-      Pred == ICmpInst::ICMP_SLT && match(B, m_Neg(m_Specific(A)))) {
-    // NABS: A < 0 ? A : -A
-    Flavor = SPF_NABS;
-    return true;
-  }
 
   if (!match(Cond, m_ICmp(Pred, m_Specific(A), m_Specific(B)))) {
     // Check for commuted variants of min/max by swapping predicate.
@@ -239,7 +219,7 @@ static unsigned getHashValueImpl(SimpleValue Val) {
   SelectPatternFlavor SPF;
   Value *Cond, *A, *B;
   if (matchSelectWithOptionalNotCond(Inst, Cond, A, B, SPF)) {
-    // Hash min/max/abs (cmp + select) to allow for commuted operands.
+    // Hash min/max (cmp + select) to allow for commuted operands.
     // Min/max may also have non-canonical compare predicate (eg, the compare for
     // smin may use 'sgt' rather than 'slt'), and non-canonical operands in the
     // compare.
@@ -250,10 +230,6 @@ static unsigned getHashValueImpl(SimpleValue Val) {
         std::swap(A, B);
       return hash_combine(Inst->getOpcode(), SPF, A, B);
     }
-    if (SPF == SPF_ABS || SPF == SPF_NABS) {
-      // ABS/NABS always puts the input in A and its negation in B.
-      return hash_combine(Inst->getOpcode(), SPF, A, B);
-    }
 
     // Hash general selects to allow matching commuted true/false operands.
 
@@ -365,7 +341,7 @@ static bool isEqualImpl(SimpleValue LHS, SimpleValue RHS) {
            LII->getArgOperand(1) == RII->getArgOperand(0);
   }
 
-  // Min/max/abs can occur with commuted operands, non-canonical predicates,
+  // Min/max can occur with commuted operands, non-canonical predicates,
   // and/or non-canonical operands.
   // Selects can be non-trivially equivalent via inverted conditions and swaps.
   SelectPatternFlavor LSPF, RSPF;
@@ -379,11 +355,6 @@ static bool isEqualImpl(SimpleValue LHS, SimpleValue RHS) {
         return ((LHSA == RHSA && LHSB == RHSB) ||
                 (LHSA == RHSB && LHSB == RHSA));
 
-      if (LSPF == SPF_ABS || LSPF == SPF_NABS) {
-        // Abs results are placed in a defined order by matchSelectPattern.
-        return LHSA == RHSA && LHSB == RHSB;
-      }
-
       // select Cond, A, B <--> select not(Cond), B, A
       if (CondL == CondR && LHSA == RHSA && LHSB == RHSB)
         return true;
@@ -401,7 +372,7 @@ static bool isEqualImpl(SimpleValue LHS, SimpleValue RHS) {
     // This intentionally does NOT handle patterns with a double-negation in
     // the sense of not + not, because doing so could result in values
     // comparing
-    // as equal that hash 
diff erently in the min/max/abs cases like:
+    // as equal that hash 
diff erently in the min/max cases like:
     // select (cmp slt, X, Y), X, Y <--> select (not (not (cmp slt, X, Y))), X, Y
     //   ^ hashes as min                  ^ would not hash as min
     // In the context of the EarlyCSE pass, however, such cases never reach

diff  --git a/llvm/test/Transforms/EarlyCSE/commute.ll b/llvm/test/Transforms/EarlyCSE/commute.ll
index a172ba81c652..de14d6eb9bfb 100644
--- a/llvm/test/Transforms/EarlyCSE/commute.ll
+++ b/llvm/test/Transforms/EarlyCSE/commute.ll
@@ -301,6 +301,41 @@ define i8 @smax_nsw(i8 %a, i8 %b) {
   ret i8 %r
 }
 
+
+define i8 @abs_swapped_sge(i8 %a) {
+; CHECK-LABEL: @abs_swapped_sge(
+; CHECK-NEXT:    [[NEG:%.*]] = sub i8 0, [[A:%.*]]
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp sge i8 [[A]], 0
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp slt i8 [[A]], 0
+; CHECK-NEXT:    [[M1:%.*]] = select i1 [[CMP1]], i8 [[A]], i8 [[NEG]]
+; CHECK-NEXT:    ret i8 0
+;
+  %neg = sub i8 0, %a
+  %cmp1 = icmp sge i8 %a, 0
+  %cmp2 = icmp slt i8 %a, 0
+  %m1 = select i1 %cmp1, i8 %a, i8 %neg
+  %m2 = select i1 %cmp2, i8 %neg, i8 %a
+  %r = xor i8 %m2, %m1
+  ret i8 %r
+}
+
+define i8 @nabs_swapped_sge(i8 %a) {
+; CHECK-LABEL: @nabs_swapped_sge(
+; CHECK-NEXT:    [[NEG:%.*]] = sub i8 0, [[A:%.*]]
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp slt i8 [[A]], 0
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp sge i8 [[A]], 0
+; CHECK-NEXT:    [[M1:%.*]] = select i1 [[CMP1]], i8 [[A]], i8 [[NEG]]
+; CHECK-NEXT:    ret i8 0
+;
+  %neg = sub i8 0, %a
+  %cmp1 = icmp slt i8 %a, 0
+  %cmp2 = icmp sge i8 %a, 0
+  %m1 = select i1 %cmp1, i8 %a, i8 %neg
+  %m2 = select i1 %cmp2, i8 %neg, i8 %a
+  %r = xor i8 %m2, %m1
+  ret i8 %r
+}
+
 ; Abs/nabs may exist with non-canonical operands. Value tracking can match those.
 ; But we do not use value tracking, so we expect instcombine will canonicalize
 ; this code to a form that allows CSE.
@@ -694,9 +729,10 @@ define i32 @inverted_max(i32 %i) {
 ; CHECK-LABEL: @inverted_max(
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp sle i32 0, [[I:%.*]]
 ; CHECK-NEXT:    [[M1:%.*]] = select i1 [[CMP]], i32 [[I]], i32 0
-; CHECK-NEXT:    [[CMPINV:%.*]] = icmp sgt i32 0, [[I:%.*]]
+; CHECK-NEXT:    [[CMPINV:%.*]] = icmp sgt i32 0, [[I]]
 ; CHECK-NEXT:    [[R:%.*]] = add i32 [[M1]], [[M1]]
 ; CHECK-NEXT:    ret i32 [[R]]
+;
   %cmp = icmp sle i32 0, %i
   %m1 = select i1 %cmp, i32 %i, i32 0
   %cmpinv = icmp sgt i32 0, %i


        


More information about the llvm-commits mailing list