Allow bitwidth difference when checking for isOneOrOneSplat.

[adriantong1024]

Summary

This helps handling a case where the BUILD_VECTOR has i16 element type
and i32 constant operands

t2: v8i16 = setcc t8, t17, setult:ch
t3: v8i16 = BUILD_VECTOR Constant:i32<1>, ...
   t4: v8i16 = and t2, t3
      t5: v8i16 = add t8, t4

This can be turned into t5: v8i16 = sub t8, t2, and allows us to remove
t3 and t4 from the DAG.

[craig.topper]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

15422

No need to declare Known outside of the blocks they are used in.

15425

Why Depth is 1 here? Shouldn't we just use the default which is 0?

[adriantong1024]

Address comments.

[craig.topper]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

15425

Declare Known on the line where it is assigned.

KnownBits Known = DAG.computeKnownBits...

15432

This is no inconsistent with the other call to computeKnownBits. I think for both you can do DAG.computeKnownBits(LHS) or DAG.computeKnownBits(RHS).

[craig.topper]

This shoudl have been picked up by foldAddSubMasked1 in the generic DAG combiner but wasn't because the BUILD_VECTOR has i8 element type and i32 constant operands. We should fix that to not require the widths to match.

[adriantong1024]

In D127354#3568503, @craig.topper wrote:

This shoudl have been picked up by foldAddSubMasked1 in the generic DAG combiner but wasn't because the BUILD_VECTOR has i8 element type and i32 constant operands. We should fix that to not require the widths to match.

Oh. let me look into it. Thanks for the pointer

[adriantong1024]

Address comments from Craig. Thanks for the pointer, this does simplify things.

[craig.topper]

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

10634 ↗

(On Diff #436802)

Are all of the callers of isOneOrOneSplat ok with this change?

Is this change correct for the caller you’re interested in if the element type of the build_vector is i1?

[adriantong1024]

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

10634 ↗

(On Diff #436802)

Let me go through the callers one by one. There are 8 uses of isOneOrOneSplat in llvm, all in DAGCombiner.cpp.

so far, I ran through check-llvm and looks good.

[adriantong1024]

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

10634 ↗

(On Diff #436802)

The bitwidth check was added https://reviews.llvm.org/D25374 and I have ran check-llvm-codegen for all available targets and things are fine.

[RKSimon]

llvm/test/CodeGen/AArch64/add-negative.ll

3

Remove " --check-prefixes=CHECK" - its the default

20

Please can you pre-commit this and rebase so the patch shows the diff?

[adriantong1024]

Address the comments from @RKSimon. Thanks!
And rebase.

[adriantong1024]

I am unable to reproduce the failures in check-openmp locally.

[RKSimon]

@adriantong1024 Please can you rebase after rGadfcdb0d0d4af1194121211c125c291162b43ccd?

[adriantong1024]

Rebase after rGadfcdb0d0d4af1194121211c125c291162b43ccd

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

10665 ↗

(On Diff #437217)

This comment can probably go - unlike Null/AllOnes this isn't going to work well if the bitcast is to a vector type with different scalar sizes

[adriantong1024]

Address comment from Simon. Thanks!

[adriantong1024]

I ran ninja check-lld check-libc check-flang check-llvm check-polly check-bolt check-mlir check-all check-clang check-clang-tools check-openmp locally and things are good.

[RKSimon]

LGTM

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

static SDValue performNegCSelCombine(SDNode *N, SelectionDAG &DAG) {
   SDValue N1N = getNegatedInteger(N1, DAG);
 
   SDLoc DL(N);
   EVT VT = CSel.getValueType();
   return DAG.getNode(AArch64ISD::CSEL, DL, VT, N0N, N1N, CSel.getOperand(2),
                      CSel.getOperand(3));
 }
 
+// Return the negative of the current node in the DAG if it exists.
+static SDValue simplifyToNegative(SDValue N, SelectionDAG &DAG) {
+  //  t2: v8i16 = AArch64ISD::CMHI t0, t1
+  //  t3: v8i16 = BUILD_VECTOR Constant:i32<1>, ...
+  //    t4: v8i16 = and t2, t3
+  if (N->getOpcode() == ISD::AND) {
+    SDValue LHS = N->getOperand(0);
+    SDValue RHS = N->getOperand(1);
+    if (LHS.getOpcode() == AArch64ISD::CMHI) {
+      KnownBits Known;
+      unsigned Depth = 0;
+      Known = DAG.computeKnownBits(RHS, Depth + 1);
+      if (Known.isConstant() && Known.getConstant().isOne())
+        return LHS;
+    }
+    if (RHS.getOpcode() == AArch64ISD::CMHI) {
+      KnownBits Known;
+      unsigned Depth = 0;
+      Known = DAG.computeKnownBits(LHS, Depth);
+      if (Known.isConstant() && Known.getConstant().isOne())
+        return RHS;
+    }
+  }
+  return SDValue();
+}
+
+// Try to fold add (-a b) or add (a -b) -> (sub b a) or (sub b a)
+// in case it is profitable to do so.
+static SDValue performAddNegCombine(SDNode *N, SelectionDAG &DAG) {
+  if (N->getOpcode() != ISD::ADD)
+    return SDValue();
+
+  EVT VT = N->getValueType(0);
+  SDValue LHS = N->getOperand(0);
+  SDValue RHS = N->getOperand(1);
+
+  // Replace with subtraction if LHS has only one use and we can find a its
+  // negative in the DAG. This way, we can get rid of LHS.
+  if (LHS.hasOneUse())
+    if (SDValue NE = simplifyToNegative(LHS, DAG))
+      return DAG.getNode(ISD::SUB, SDLoc(N), VT, RHS, NE);
+
+  if (RHS.hasOneUse())
+    if (SDValue NE = simplifyToNegative(RHS, DAG))
+      return DAG.getNode(ISD::SUB, SDLoc(N), VT, LHS, NE);
+
+  return SDValue();
+}
+
 // The basic add/sub long vector instructions have variants with "2" on the end
 // which act on the high-half of their inputs. They are normally matched by
 // patterns like:
 //
 // (add (zeroext (extract_high LHS)),
 //      (zeroext (extract_high RHS)))
 // -> uaddl2 vD, vN, vM
 //
static SDValue performAddSubCombine(SDNode *N,
   if (SDValue Val = performAddUADDVCombine(N, DAG))
     return Val;
   if (SDValue Val = performAddDotCombine(N, DAG))
     return Val;
   if (SDValue Val = performAddCSelIntoCSinc(N, DAG))
     return Val;
   if (SDValue Val = performNegCSelCombine(N, DAG))
     return Val;
+  if (SDValue Val = performAddNegCombine(N, DAG))
+    return Val;
 
   return performAddSubLongCombine(N, DCI, DAG);
 }
 
 // Massage DAGs which we can use the high-half "long" operations on into
 // something isel will recognize better. E.g.
 //
 // (aarch64_neon_umull (extract_high vec) (dupv64 scalar)) -->

llvm/test/CodeGen/AArch64/add-negative.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -o - %s -mtriple=aarch64-linux-gnu | FileCheck %s --check-prefixes=CHECK
+
+define <8 x i16> @add_to_sub(<8 x i16> %0, <8 x i16> %1) {
+; CHECK-LABEL: add_to_sub:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    adrp x8, .LCPI0_0
+; CHECK-NEXT:    ldr q2, [x8, :lo12:.LCPI0_0]
+; CHECK-NEXT:    cmhi v0.8h, v2.8h, v0.8h
+; CHECK-NEXT:    cmhi v1.8h, v2.8h, v1.8h
+; CHECK-NEXT:    sub v0.8h, v0.8h, v1.8h
+; CHECK-NEXT:    ret
+    %3 = icmp ult <8 x i16> %0, <i16 1, i16 2, i16 3, i16 4, i16 5, i16 6, i16 7, i16 8>
+    %4 = sext <8 x i1> %3 to <8 x i16>
+    %5 = icmp ult <8 x i16> %1, <i16 1, i16 2, i16 3, i16 4, i16 5, i16 6, i16 7, i16 8>
+    %6 = zext <8 x i1> %5 to <8 x i16>
+    %7 = add nsw <8 x i16> %6, %4
+    ret <8 x i16> %7
+}
+

llvm/test/CodeGen/AArch64/minmax.ll

; CHECK-NEXT:    ret
   ret <16 x i32> %t2
 }
 
 ; The icmp is used by two instructions, so don't produce a umin node.
 define <16 x i8> @t12(<16 x i8> %a, <16 x i8> %b) {
 ; CHECK-LABEL: t12:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    cmhi v2.16b, v1.16b, v0.16b
-; CHECK-NEXT:    movi v3.16b, #1
 ; CHECK-NEXT:    bif v0.16b, v1.16b, v2.16b
-; CHECK-NEXT:    and v1.16b, v2.16b, v3.16b
-; CHECK-NEXT:    add v0.16b, v1.16b, v0.16b
+; CHECK-NEXT:    sub v0.16b, v0.16b, v2.16b
 ; CHECK-NEXT:    ret
   %t1 = icmp ugt <16 x i8> %b, %a
   %t2 = select <16 x i1> %t1, <16 x i8> %a, <16 x i8> %b
   %t3 = zext <16 x i1> %t1 to <16 x i8>
   %t4 = add <16 x i8> %t3, %t2
   ret <16 x i8> %t4
 }