[DAGCombiner] fold add/sub with bool operand based on target's boolean contents

[spatel]

Summary

I noticed that we are missing this canonicalization in IR:
rL352515
...and then realized that we don't get this right in SDAG either, so this has to be fixed first regardless of what we choose to do in IR.

The existing fold was limited to scalars and using the wrong predicate to guard the transform. We have a boolean contents TLI query that can be used to decide which direction to fold.

This may eventually lead back to the problems/question in:
https://bugs.llvm.org/show_bug.cgi?id=40486
...but it makes no difference to that yet.

[efriedma]

test/CodeGen/PowerPC/select_const.ll

268–269 ↗

(On Diff #184121)

This looks like a missing target-independent pattern; we're ending up with something like "42 - (cond & 1)" instead of "42 + cond".

[spatel]

test/CodeGen/PowerPC/select_const.ll

268–269 ↗

(On Diff #184121)

Yes, we're missing a generic combine here, but I thought we had the 1 you mentioned. Ie, if I saw it correctly while glancing at it the first time, this requires an AssertSext to uncover.

I'll have a closer look to avoid this, but I figured the other diffs were more realistic, so we might want to commit this patch first if it looks reasonable.

[efriedma]

test/CodeGen/PowerPC/select_const.ll

268–269 ↗

(On Diff #184121)

If the issue is actually just a missing combine for "X - (Y & 1)", where Y is a 0/-1 value, I'm fine with temporarily regressing it. But please confirm the DAG actually looks like that.

[spatel]

Patch updated:
The missing fold that was shown in the previous rev should be fixed here:
rL352680 (I added some extra tests to verify that was behaving as expected)

Interestingly, that actually caught my motivating cases for x86 and AArch in the earlier rev of this patch. So now we only show diffs for PPC tests, and those are all improvements. These tests also exist for x86, but there are no diffs there...I didn't step through all of those, but it looks like custom hacks to form LEA instructions mask any underlying changes.

[spatel]

Ping.

[efriedma]

LGTM

llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if (N1.getOpcode() == ISD::SHL && N1.getOperand(0).getOpcode() == ISD::SUB &&
     return DAG.getNode(ISD::SUB, DL, VT, N0,
                        DAG.getNode(ISD::SHL, DL, VT,
                                    N1.getOperand(0).getOperand(1),
                                    N1.getOperand(1)));
 
   if (SDValue V = foldAddSubMasked1(true, N0, N1, DAG, DL))
     return V;
 
-  // add (sext i1), X -> sub X, (zext i1)
+  // If the target's bool is represented as 0/1, prefer to make this 'sub 0/1'
+  // rather than 'add 0/-1' (the zext should get folded).
+  // add (sext i1 Y), X --> sub X, (zext i1 Y)
   if (N0.getOpcode() == ISD::SIGN_EXTEND &&
-      N0.getOperand(0).getValueType() == MVT::i1 &&
-      !TLI.isOperationLegal(ISD::SIGN_EXTEND, MVT::i1)) {
+      N0.getOperand(0).getScalarValueSizeInBits() == 1 &&
+      TLI.getBooleanContents(VT) == TargetLowering::ZeroOrOneBooleanContent) {
     SDValue ZExt = DAG.getNode(ISD::ZERO_EXTEND, DL, VT, N0.getOperand(0));
     return DAG.getNode(ISD::SUB, DL, VT, N1, ZExt);
   }
 
   // add X, (sextinreg Y i1) -> sub X, (and Y 1)
   if (N1.getOpcode() == ISD::SIGN_EXTEND_INREG) {
     VTSDNode *TN = cast<VTSDNode>(N1.getOperand(1));
     if (TN->getVT() == MVT::i1) {
if (SDValue V = foldAddSubBoolOfMaskedVal(N, DAG))
     return V;
 
   if (SDValue V = foldAddSubOfSignBit(N, DAG))
     return V;
 
   if (SDValue V = foldAddSubMasked1(false, N0, N1, DAG, SDLoc(N)))
     return V;
 
+  // If the target's bool is represented as 0/-1, prefer to make this 'add 0/-1'
+  // rather than 'sub 0/1' (the sext should get folded).
+  // sub X, (zext i1 Y) --> add X, (sext i1 Y)
+  if (N1.getOpcode() == ISD::ZERO_EXTEND &&
+      N1.getOperand(0).getScalarValueSizeInBits() == 1 &&
+      TLI.getBooleanContents(VT) ==
+          TargetLowering::ZeroOrNegativeOneBooleanContent) {
+    SDValue SExt = DAG.getNode(ISD::SIGN_EXTEND, DL, VT, N1.getOperand(0));
+    return DAG.getNode(ISD::ADD, DL, VT, N0, SExt);
+  }
+
   // fold Y = sra (X, size(X)-1); sub (xor (X, Y), Y) -> (abs X)
   if (TLI.isOperationLegalOrCustom(ISD::ABS, VT)) {
     if (N0.getOpcode() == ISD::XOR && N1.getOpcode() == ISD::SRA) {
       SDValue X0 = N0.getOperand(0), X1 = N0.getOperand(1);
       SDValue S0 = N1.getOperand(0);
       if ((X0 == S0 && X1 == N1) || (X0 == N1 && X1 == S0)) {
         unsigned OpSizeInBits = VT.getScalarSizeInBits();
         if (ConstantSDNode *C = isConstOrConstSplat(N1.getOperand(1)))

llvm/trunk/test/CodeGen/PowerPC/bool-math.ll

; CHECK-NEXT:    blr
   %z = zext i1 %c to i8
   %r = add i8 %z, 42
   ret i8 %r
 }
 
 define i32 @low_bit_select_constants_bigger_false_same_size_result(i32 %x) {
 ; CHECK-LABEL: low_bit_select_constants_bigger_false_same_size_result:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    not 3, 3
 ; CHECK-NEXT:    clrldi 3, 3, 63
-; CHECK-NEXT:    subfic 3, 3, 43
+; CHECK-NEXT:    ori 3, 3, 42
 ; CHECK-NEXT:    blr
   %a = and i32 %x, 1
   %c = icmp eq i32 %a, 0
   %r = select i1 %c, i32 42, i32 43
   ret i32 %r
 }
 
 define i64 @low_bit_select_constants_bigger_false_wider_result(i32 %x) {
 ; CHECK-LABEL: low_bit_select_constants_bigger_false_wider_result:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    not 3, 3
 ; CHECK-NEXT:    clrldi 3, 3, 63
-; CHECK-NEXT:    subfic 3, 3, 27
+; CHECK-NEXT:    ori 3, 3, 26
 ; CHECK-NEXT:    blr
   %a = and i32 %x, 1
   %c = icmp eq i32 %a, 0
   %r = select i1 %c, i64 26, i64 27
   ret i64 %r
 }
 
 define i16 @low_bit_select_constants_bigger_false_narrower_result(i32 %x) {
 ; CHECK-LABEL: low_bit_select_constants_bigger_false_narrower_result:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    nor 3, 3, 3
-; CHECK-NEXT:    clrlwi 3, 3, 31
-; CHECK-NEXT:    subfic 3, 3, 37
+; CHECK-NEXT:    clrldi 3, 3, 63
+; CHECK-NEXT:    ori 3, 3, 36
 ; CHECK-NEXT:    blr
   %a = and i32 %x, 1
   %c = icmp eq i32 %a, 0
   %r = select i1 %c, i16 36, i16 37
   ret i16 %r
 }
 
 define i8 @low_bit_select_constants_bigger_true_same_size_result(i8 %x) {

llvm/trunk/test/CodeGen/PowerPC/signbit-shift.ll

; CHECK-NEXT:    blr
   %c = icmp sgt i32 %x, -1
   %e = sext i1 %c to i32
   ret i32 %e
 }
 
 define i32 @add_sext_ifpos(i32 %x) {
 ; CHECK-LABEL: add_sext_ifpos:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    nor 3, 3, 3
-; CHECK-NEXT:    srawi 3, 3, 31
-; CHECK-NEXT:    addi 3, 3, 42
+; CHECK-NEXT:    srwi 3, 3, 31
+; CHECK-NEXT:    addi 3, 3, 41
 ; CHECK-NEXT:    blr
   %c = icmp sgt i32 %x, -1
   %e = sext i1 %c to i32
   %r = add i32 %e, 42
   ret i32 %r
 }
 
 define <4 x i32> @add_sext_ifpos_vec_splat(<4 x i32> %x) {