[DAGCombiner] In foldSelectOfConstants, use 'sub C1+1, (zext Cond)' instead of 'add (sext Cond), C1+1'

[craig.topper]

Summary

On X86, we usually end up converting the add to the sub form anyway. On PowerPC, this seems to avoid some nots in some of the cases.

The changes are in both the select_const tests are regressions. The PowerPC regression was largely just getting lucky that the sext worked well with the AssertSExt of the input. The code is now similar to another test case that uses 'add (zext Cond), C1' where that doesn't work well with the AssertSExt.

I haven't looked into the X86 regression yet, but it looks like some sort of reassociation issue.

[craig.topper]

@spatel, what are you thoughts on this?

[spatel]

In D48530#1147067, @craig.topper wrote:

@spatel, what are you thoughts on this?

Did we get the motivating cases with our other select patches? If not, can you post those? As-is, I don't see why we'd change this from these test diffs.

[craig.topper]

Yeah I think the other patches took care of my perf regression. I'm fine to abandon this if you don't think the PowerPC changes are worthwhile.

[spatel]

In D48530#1147103, @craig.topper wrote:

Yeah I think the other patches took care of my perf regression. I'm fine to abandon this if you don't think the PowerPC changes are worthwhile.

Yep, PPC has a lot of missed opportunities. That could probably be solved with better pattern matching later on. For x86, if cmov really is implemented as a single-cycle and full-throughput op, then we probably shouldn't be doing these kinds of folds anyway.

Also worth noting: no other in-tree targets have enabled the convertSelectOfConstantsToMath() hook to get this transform, but I think that's just an oversight. Not sure what differences we'd see if more targets override the default.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if (CondVT == MVT::i1 && !LegalOperations) {
     if (TLI.convertSelectOfConstantsToMath(VT)) {
       if (C1->getAPIntValue() - 1 == C2->getAPIntValue()) {
         // select Cond, C1, C1-1 --> add (zext Cond), C1-1
         if (VT != MVT::i1)
           Cond = DAG.getNode(ISD::ZERO_EXTEND, DL, VT, Cond);
         return DAG.getNode(ISD::ADD, DL, VT, Cond, N2);
       }
       if (C1->getAPIntValue() + 1 == C2->getAPIntValue()) {
-        // select Cond, C1, C1+1 --> add (sext Cond), C1+1
+        // select Cond, C1, C1+1 --> sub C1+1, (zext Cond)
         if (VT != MVT::i1)
-          Cond = DAG.getNode(ISD::SIGN_EXTEND, DL, VT, Cond);
-        return DAG.getNode(ISD::ADD, DL, VT, Cond, N2);
+          Cond = DAG.getNode(ISD::ZERO_EXTEND, DL, VT, Cond);
+        return DAG.getNode(ISD::SUB, DL, VT, N2, Cond);
       }
     }
 
     return SDValue();
   }
 
   // fold (select Cond, 0, 1) -> (xor Cond, 1)
   // We can't do this reliably if integer based booleans have different contents

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) {

test/CodeGen/PowerPC/select_const.ll

 ; ALL-NEXT:    blr
   %sel = select i1 %cond, i32 41, i32 42
   ret i32 %sel
 }
 
 define i32 @select_C_Cplus1_signext(i1 signext %cond) {
 ; ALL-LABEL: select_C_Cplus1_signext:
 ; ALL:       # %bb.0:
-; ALL-NEXT:    addi 3, 3, 42
+; ALL-NEXT:    clrldi 3, 3, 63
+; ALL-NEXT:    subfic 3, 3, 42
 ; ALL-NEXT:    blr
   %sel = select i1 %cond, i32 41, i32 42
   ret i32 %sel
 }
 
 ; In general, select of 2 constants could be:
 ; select Cond, C1, C2 --> add (mul (zext Cond), C1-C2), C2 --> add (and (sext Cond), C1-C2), C2
 

test/CodeGen/X86/select_const.ll

 }
 
 ; Should this be 'sbb x,x' or 'sbb 0,x' with simpler LEA or add?
 
 define i64 @sel_1_2(i64 %x, i64 %y) {
 ; CHECK-LABEL: sel_1_2:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    cmpq $42, %rdi
-; CHECK-NEXT:    sbbq $0, %rsi
-; CHECK-NEXT:    leaq 2(%rsi), %rax
+; CHECK-NEXT:    movl $2, %eax
+; CHECK-NEXT:    sbbq $0, %rax
+; CHECK-NEXT:    addq %rsi, %rax
 ; CHECK-NEXT:    retq
   %cmp = icmp ult i64 %x, 42
   %sel = select i1 %cmp, i64 1, i64 2
   %sub = add i64 %sel, %y
   ret i64 %sub
 }
 
 ; No LEA with 8-bit, but this shouldn't need branches or cmov.