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 ↗

(On Diff #435357)

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

15425 ↗

(On Diff #435357)

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 ↗

(On Diff #435362)

Declare Known on the line where it is assigned.

KnownBits Known = DAG.computeKnownBits...

15432 ↗

(On Diff #435362)

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

10665–10666

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

10665–10666

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

10665–10666

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–10666

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/CodeGen/SelectionDAG/SelectionDAG.cpp

 bool llvm::isNullOrNullSplat(SDValue N, bool AllowUndefs) {
   // TODO: may want to use peekThroughBitcast() here.
   ConstantSDNode *C =
       isConstOrConstSplat(N, AllowUndefs, /*AllowTruncation=*/true);
   return C && C->isZero();
 }
 
 bool llvm::isOneOrOneSplat(SDValue N, bool AllowUndefs) {
   // TODO: may want to use peekThroughBitcast() here.
-  unsigned BitWidth = N.getScalarValueSizeInBits();
-  ConstantSDNode *C = isConstOrConstSplat(N, AllowUndefs);
-  return C && C->isOne() && C->getValueSizeInBits(0) == BitWidth;
+  ConstantSDNode *C =
+      isConstOrConstSplat(N, AllowUndefs, /*AllowTruncation*/ true);
+  return C && C->isOne();
 }
 
 bool llvm::isAllOnesOrAllOnesSplat(SDValue N, bool AllowUndefs) {
   N = peekThroughBitcasts(N);
   unsigned BitWidth = N.getScalarValueSizeInBits();
   ConstantSDNode *C = isConstOrConstSplat(N, AllowUndefs);
   return C && C->isAllOnes() && C->getValueSizeInBits(0) == BitWidth;
 }

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
+
+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
 }