[llvm] f3056dc - [InstCombine] Negator: -(cond ? x : -x) --> cond ? -x : x

[Roman Lebedev via llvm-commits]

Author: Roman Lebedev
Date: 2020-08-05T21:47:30+03:00
New Revision: f3056dcc02d5dcde1669f61084a940e1ded3dfda

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

LOG: [InstCombine] Negator: -(cond ? x : -x)  -->  cond ? -x : x

We were errneously only doing that for old-style abs/nabs,
but we have no such legality check on the condition of the select.

https://rise4fun.com/Alive/xBHS

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
    llvm/test/Transforms/InstCombine/sub-of-negatible.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp b/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
index 34f31e5f3f685..bd3613c89c222 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
@@ -257,20 +257,16 @@ LLVM_NODISCARD Value *Negator::visitImpl(Value *V, unsigned Depth) {
     return NegatedPHI;
   }
   case Instruction::Select: {
-    {
-      // `abs`/`nabs` is always negatible.
-      Value *LHS, *RHS;
-      SelectPatternFlavor SPF =
-          matchSelectPattern(I, LHS, RHS, /*CastOp=*/nullptr, Depth).Flavor;
-      if (SPF == SPF_ABS || SPF == SPF_NABS) {
-        auto *NewSelect = cast<SelectInst>(I->clone());
-        // Just swap the operands of the select.
-        NewSelect->swapValues();
-        // Don't swap prof metadata, we didn't change the branch behavior.
-        NewSelect->setName(I->getName() + ".neg");
-        Builder.Insert(NewSelect);
-        return NewSelect;
-      }
+    if (isKnownNegation(I->getOperand(1), I->getOperand(2))) {
+      // Of one hand of select is known to be negation of another hand,
+      // just swap the hands around.
+      auto *NewSelect = cast<SelectInst>(I->clone());
+      // Just swap the operands of the select.
+      NewSelect->swapValues();
+      // Don't swap prof metadata, we didn't change the branch behavior.
+      NewSelect->setName(I->getName() + ".neg");
+      Builder.Insert(NewSelect);
+      return NewSelect;
     }
     // `select` is negatible if both hands of `select` are negatible.
     Value *NegOp1 = negate(I->getOperand(1), Depth + 1);

diff  --git a/llvm/test/Transforms/InstCombine/sub-of-negatible.ll b/llvm/test/Transforms/InstCombine/sub-of-negatible.ll
index d4b2de3db4d32..43bedaa9b157a 100644
--- a/llvm/test/Transforms/InstCombine/sub-of-negatible.ll
+++ b/llvm/test/Transforms/InstCombine/sub-of-negatible.ll
@@ -1177,8 +1177,8 @@ define i8 @negate_select_of_op_vs_negated_op(i8 %x, i8 %y, i1 %c) {
 ; CHECK-LABEL: @negate_select_of_op_vs_negated_op(
 ; CHECK-NEXT:    [[T0:%.*]] = sub i8 0, [[X:%.*]]
 ; CHECK-NEXT:    call void @use8(i8 [[T0]])
-; CHECK-NEXT:    [[T1:%.*]] = select i1 [[C:%.*]], i8 [[T0]], i8 [[X]], !prof !0
-; CHECK-NEXT:    [[T2:%.*]] = sub i8 [[Y:%.*]], [[T1]]
+; CHECK-NEXT:    [[TMP1:%.*]] = select i1 [[C:%.*]], i8 [[X]], i8 [[T0]], !prof !0
+; CHECK-NEXT:    [[T2:%.*]] = add i8 [[TMP1]], [[Y:%.*]]
 ; CHECK-NEXT:    ret i8 [[T2]]
 ;
   %t0 = sub i8 0, %x