[DAG] Fold setcc eq with ashr to compare to zero.

[dmgreen]

Summary

Pulled out of D109149, this folds set_cc seteq (ashr X, BW-1), -1 -> set_cc setlt X, 0 to prevent some regressions later on when folding select_cc setgt X, -1, C, ~C -> xor (ashr X, BW-1), C

[foad]

Looks reasonable to me, but doesn't instcombine pick this up at the IR level? Or don't the .ll test cases run instcombine?

[dmgreen]

This is part of D109149, it prevents that from causing larger code when this pattern comes up from other DAG Combines.

[spatel]

LGTM.
The motivating transform in D109149 only works on scalars if I'm seeing it correctly. This part could apply to vectors without much effort, but it's already in a FIXME block, so nothing more to do at the moment.

[uabelho]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

3921

The comment at line 3914 says we should use "setge" for the "setne" case, but the code actually use "SETGT"?

I'm seeing a miscompile with this patch for my out of tree target and I'm not sure what the problem actually is, but the difference between the comment and the code caught my eye.

[foad]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

3921

Yes it should definitely be SETGE here!

[uabelho]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

3921

When it goes wrong for me the above code rewrites

  t22: i32 = sra t20, Constant:i16<31>, foo.c:15:24
t29: i16 = setcc t22, Constant:i32<-1>, setne:ch, foo.c:15:22

into

t36: i16 = setcc t20, Constant:i32<0>, setgt:ch, foo.c:15:22

t20 is 0, so t22 is also 0 and t29 is 1.
But t36 is 0. With "setge" instead of "setgt" t36 would be 1.

[dmgreen]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

3921

Oh yeah. Does changing it to SETGE fix the miscompile?

[dmgreen]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

3921

OK. I'll switch that to the correct thing. Thanks for the report.

[uabelho]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

3921

Yep it does

[uabelho]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

3921

Sounds good! Thanks!

[dmgreen]

d8d24c64fe21 hopefully fixes the issue. Let me know if not.

[uabelho]

In D109214#2989225, @dmgreen wrote:

d8d24c64fe21 hopefully fixes the issue. Let me know if not.

Yes, it works now. Thanks!

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

if (auto *N1C = dyn_cast<ConstantSDNode>(N1.getNode())) {
     if (N0.getOpcode() == ISD::UREM && N1C->isNullValue() &&
         (Cond == ISD::SETEQ || Cond == ISD::SETNE)) {
       KnownBits XKnown = DAG.computeKnownBits(N0.getOperand(0));
       KnownBits YKnown = DAG.computeKnownBits(N0.getOperand(1));
       if (XKnown.countMaxPopulation() == 1 && YKnown.countMinPopulation() >= 2)
         return DAG.getSetCC(dl, VT, N0.getOperand(0), N1, Cond);
     }
 
+    // Fold set_cc seteq (ashr X, BW-1), -1 -> set_cc setlt X, 0
+    //  and set_cc setne (ashr X, BW-1), -1 -> set_cc setge X, 0
+    if ((Cond == ISD::SETEQ || Cond == ISD::SETNE) &&
+        N0.getOpcode() == ISD::SRA && isa<ConstantSDNode>(N0.getOperand(1)) &&
+        N0.getConstantOperandAPInt(1) == OpVT.getScalarSizeInBits() - 1 &&
+        N1C && N1C->isAllOnesValue()) {
+      return DAG.getSetCC(dl, VT, N0.getOperand(0),
+                          DAG.getConstant(0, dl, OpVT),
+                          Cond == ISD::SETEQ ? ISD::SETLT : ISD::SETGT);
+    }
+
     if (SDValue V =
             optimizeSetCCOfSignedTruncationCheck(VT, N0, N1, Cond, DCI, dl))
       return V;
   }
 
   // These simplifications apply to splat vectors as well.
   // TODO: Handle more splat vector cases.
   if (auto *N1C = isConstOrConstSplat(N1)) {

llvm/test/CodeGen/AArch64/select-constant-xor.ll

; CHECK-NEXT:    ret
   %c = icmp sgt i8 %a, -1
   %s = select i1 %c, i32 84, i32 -85
   ret i32 %s
 }
 
 define i32 @icmpasreq(i32 %input, i32 %a, i32 %b) {
 ; CHECK-LABEL: icmpasreq:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    mov w8, #-1
-; CHECK-NEXT:    cmp w8, w0, asr #31
-; CHECK-NEXT:    csel w0, w1, w2, eq
+; CHECK-NEXT:    cmp w0, #0
+; CHECK-NEXT:    csel w0, w1, w2, lt
 ; CHECK-NEXT:    ret
   %sh = ashr i32 %input, 31
   %c = icmp eq i32 %sh, -1
   %s = select i1 %c, i32 %a, i32 %b
   ret i32 %s
 }
 
 define i32 @icmpasrne(i32 %input, i32 %a, i32 %b) {
 ; CHECK-LABEL: icmpasrne:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    mov w8, #-1
-; CHECK-NEXT:    cmp w8, w0, asr #31
-; CHECK-NEXT:    csel w0, w1, w2, ne
+; CHECK-NEXT:    cmp w0, #0
+; CHECK-NEXT:    csel w0, w1, w2, gt
 ; CHECK-NEXT:    ret
   %sh = ashr i32 %input, 31
   %c = icmp ne i32 %sh, -1
   %s = select i1 %c, i32 %a, i32 %b
   ret i32 %s
 }
 
 define i32 @selecti32i32_0(i32 %a) {

llvm/test/CodeGen/AMDGPU/select-constant-xor.ll

; CHECK-NEXT:    s_setpc_b64 s[30:31]
   ret i32 %s
 }
 
 define i32 @icmpasreq(i32 %input, i32 %a, i32 %b) {
 ; CHECK-LABEL: icmpasreq:
 ; CHECK:       ; %bb.0:
 ; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; CHECK-NEXT:    s_waitcnt_vscnt null, 0x0
-; CHECK-NEXT:    v_ashrrev_i32_e32 v0, 31, v0
-; CHECK-NEXT:    v_cmp_eq_u32_e32 vcc_lo, -1, v0
+; CHECK-NEXT:    v_cmp_gt_i32_e32 vcc_lo, 0, v0
 ; CHECK-NEXT:    v_cndmask_b32_e32 v0, v2, v1, vcc_lo
 ; CHECK-NEXT:    s_setpc_b64 s[30:31]
   %sh = ashr i32 %input, 31
   %c = icmp eq i32 %sh, -1
   %s = select i1 %c, i32 %a, i32 %b
   ret i32 %s
 }
 
 define i32 @icmpasrne(i32 %input, i32 %a, i32 %b) {
 ; CHECK-LABEL: icmpasrne:
 ; CHECK:       ; %bb.0:
 ; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; CHECK-NEXT:    s_waitcnt_vscnt null, 0x0
-; CHECK-NEXT:    v_ashrrev_i32_e32 v0, 31, v0
-; CHECK-NEXT:    v_cmp_ne_u32_e32 vcc_lo, -1, v0
+; CHECK-NEXT:    v_cmp_lt_i32_e32 vcc_lo, 0, v0
 ; CHECK-NEXT:    v_cndmask_b32_e32 v0, v2, v1, vcc_lo
 ; CHECK-NEXT:    s_setpc_b64 s[30:31]
   %sh = ashr i32 %input, 31
   %c = icmp ne i32 %sh, -1
   %s = select i1 %c, i32 %a, i32 %b
   ret i32 %s
 }
 

llvm/test/CodeGen/ARM/select-constant-xor.ll

; CHECK81M-NEXT:    bx lr
   %c = icmp sgt i8 %a, -1
   %s = select i1 %c, i32 84, i32 -85
   ret i32 %s
 }
 
 define i32 @icmpasreq(i32 %input, i32 %a, i32 %b) {
 ; CHECK7A-LABEL: icmpasreq:
 ; CHECK7A:       @ %bb.0:
-; CHECK7A-NEXT:    mvn r3, #0
-; CHECK7A-NEXT:    cmp r3, r0, asr #31
-; CHECK7A-NEXT:    movne r1, r2
+; CHECK7A-NEXT:    cmp r0, #0
+; CHECK7A-NEXT:    movpl r1, r2
 ; CHECK7A-NEXT:    mov r0, r1
 ; CHECK7A-NEXT:    bx lr
 ;
 ; CHECK6M-LABEL: icmpasreq:
 ; CHECK6M:       @ %bb.0:
-; CHECK6M-NEXT:    asrs r0, r0, #31
-; CHECK6M-NEXT:    adds r0, r0, #1
-; CHECK6M-NEXT:    beq .LBB8_2
+; CHECK6M-NEXT:    cmp r0, #0
+; CHECK6M-NEXT:    bmi .LBB8_2
 ; CHECK6M-NEXT:  @ %bb.1:
 ; CHECK6M-NEXT:    mov r1, r2
 ; CHECK6M-NEXT:  .LBB8_2:
 ; CHECK6M-NEXT:    mov r0, r1
 ; CHECK6M-NEXT:    bx lr
 ;
 ; CHECK7M-LABEL: icmpasreq:
 ; CHECK7M:       @ %bb.0:
-; CHECK7M-NEXT:    asrs r0, r0, #31
-; CHECK7M-NEXT:    adds r0, #1
-; CHECK7M-NEXT:    it ne
-; CHECK7M-NEXT:    movne r1, r2
+; CHECK7M-NEXT:    cmp r0, #0
+; CHECK7M-NEXT:    it pl
+; CHECK7M-NEXT:    movpl r1, r2
 ; CHECK7M-NEXT:    mov r0, r1
 ; CHECK7M-NEXT:    bx lr
 ;
 ; CHECK81M-LABEL: icmpasreq:
 ; CHECK81M:       @ %bb.0:
-; CHECK81M-NEXT:    asrs r0, r0, #31
-; CHECK81M-NEXT:    adds r0, #1
-; CHECK81M-NEXT:    csel r0, r1, r2, eq
+; CHECK81M-NEXT:    cmp r0, #0
+; CHECK81M-NEXT:    csel r0, r1, r2, mi
 ; CHECK81M-NEXT:    bx lr
   %sh = ashr i32 %input, 31
   %c = icmp eq i32 %sh, -1
   %s = select i1 %c, i32 %a, i32 %b
   ret i32 %s
 }
 
 define i32 @icmpasrne(i32 %input, i32 %a, i32 %b) {
 ; CHECK7A-LABEL: icmpasrne:
 ; CHECK7A:       @ %bb.0:
-; CHECK7A-NEXT:    mvn r3, #0
-; CHECK7A-NEXT:    cmp r3, r0, asr #31
-; CHECK7A-NEXT:    moveq r1, r2
+; CHECK7A-NEXT:    cmp r0, #0
+; CHECK7A-NEXT:    movle r1, r2
 ; CHECK7A-NEXT:    mov r0, r1
 ; CHECK7A-NEXT:    bx lr
 ;
 ; CHECK6M-LABEL: icmpasrne:
 ; CHECK6M:       @ %bb.0:
-; CHECK6M-NEXT:    asrs r0, r0, #31
-; CHECK6M-NEXT:    adds r0, r0, #1
-; CHECK6M-NEXT:    bne .LBB9_2
+; CHECK6M-NEXT:    cmp r0, #0
+; CHECK6M-NEXT:    bgt .LBB9_2
 ; CHECK6M-NEXT:  @ %bb.1:
 ; CHECK6M-NEXT:    mov r1, r2
 ; CHECK6M-NEXT:  .LBB9_2:
 ; CHECK6M-NEXT:    mov r0, r1
 ; CHECK6M-NEXT:    bx lr
 ;
 ; CHECK7M-LABEL: icmpasrne:
 ; CHECK7M:       @ %bb.0:
-; CHECK7M-NEXT:    asrs r0, r0, #31
-; CHECK7M-NEXT:    adds r0, #1
-; CHECK7M-NEXT:    it eq
-; CHECK7M-NEXT:    moveq r1, r2
+; CHECK7M-NEXT:    cmp r0, #0
+; CHECK7M-NEXT:    it le
+; CHECK7M-NEXT:    movle r1, r2
 ; CHECK7M-NEXT:    mov r0, r1
 ; CHECK7M-NEXT:    bx lr
 ;
 ; CHECK81M-LABEL: icmpasrne:
 ; CHECK81M:       @ %bb.0:
-; CHECK81M-NEXT:    asrs r0, r0, #31
-; CHECK81M-NEXT:    adds r0, #1
-; CHECK81M-NEXT:    csel r0, r1, r2, ne
+; CHECK81M-NEXT:    cmp r0, #0
+; CHECK81M-NEXT:    csel r0, r1, r2, gt
 ; CHECK81M-NEXT:    bx lr
   %sh = ashr i32 %input, 31
   %c = icmp ne i32 %sh, -1
   %s = select i1 %c, i32 %a, i32 %b
   ret i32 %s
 }
 
 define i32 @oneusecmp(i32 %a, i32 %b, i32 %d) {

llvm/test/CodeGen/PowerPC/select-constant-xor.ll

; CHECK-NEXT:    blr
   %c = icmp sgt i8 %a, -1
   %s = select i1 %c, i32 84, i32 -85
   ret i32 %s
 }
 
 define i32 @icmpasreq(i32 %input, i32 %a, i32 %b) {
 ; CHECK-LABEL: icmpasreq:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    srawi 3, 3, 31
-; CHECK-NEXT:    cmpwi 3, -1
-; CHECK-NEXT:    iseleq 3, 4, 5
+; CHECK-NEXT:    cmpwi 3, 0
+; CHECK-NEXT:    isellt 3, 4, 5
 ; CHECK-NEXT:    blr
   %sh = ashr i32 %input, 31
   %c = icmp eq i32 %sh, -1
   %s = select i1 %c, i32 %a, i32 %b
   ret i32 %s
 }
 
 define i32 @icmpasrne(i32 %input, i32 %a, i32 %b) {
 ; CHECK-LABEL: icmpasrne:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    srawi 3, 3, 31
-; CHECK-NEXT:    cmpwi 3, -1
-; CHECK-NEXT:    iseleq 3, 5, 4
+; CHECK-NEXT:    cmpwi 3, 0
+; CHECK-NEXT:    iselgt 3, 4, 5
 ; CHECK-NEXT:    blr
   %sh = ashr i32 %input, 31
   %c = icmp ne i32 %sh, -1
   %s = select i1 %c, i32 %a, i32 %b
   ret i32 %s
 }
 
 define i32 @oneusecmp(i32 %a, i32 %b, i32 %d) {

llvm/test/CodeGen/RISCV/select-constant-xor.ll

; CHECK64-NEXT:    ret
   %c = icmp sgt i8 %a, -1
   %s = select i1 %c, i32 84, i32 -85
   ret i32 %s
 }
 
 define i32 @icmpasreq(i32 %input, i32 %a, i32 %b) {
 ; CHECK32-LABEL: icmpasreq:
 ; CHECK32:       # %bb.0:
-; CHECK32-NEXT:    srai a3, a0, 31
-; CHECK32-NEXT:    addi a4, zero, -1
-; CHECK32-NEXT:    mv a0, a1
-; CHECK32-NEXT:    beq a3, a4, .LBB8_2
+; CHECK32-NEXT:    bltz a0, .LBB8_2
 ; CHECK32-NEXT:  # %bb.1:
-; CHECK32-NEXT:    mv a0, a2
+; CHECK32-NEXT:    mv a1, a2
 ; CHECK32-NEXT:  .LBB8_2:
+; CHECK32-NEXT:    mv a0, a1
 ; CHECK32-NEXT:    ret
 ;
 ; CHECK64-LABEL: icmpasreq:
 ; CHECK64:       # %bb.0:
-; CHECK64-NEXT:    sraiw a3, a0, 31
-; CHECK64-NEXT:    addi a4, zero, -1
+; CHECK64-NEXT:    sext.w a3, a0
 ; CHECK64-NEXT:    mv a0, a1
-; CHECK64-NEXT:    beq a3, a4, .LBB8_2
+; CHECK64-NEXT:    bltz a3, .LBB8_2
 ; CHECK64-NEXT:  # %bb.1:
 ; CHECK64-NEXT:    mv a0, a2
 ; CHECK64-NEXT:  .LBB8_2:
 ; CHECK64-NEXT:    ret
   %sh = ashr i32 %input, 31
   %c = icmp eq i32 %sh, -1
   %s = select i1 %c, i32 %a, i32 %b
   ret i32 %s
 }
 
 define i32 @icmpasrne(i32 %input, i32 %a, i32 %b) {
 ; CHECK32-LABEL: icmpasrne:
 ; CHECK32:       # %bb.0:
-; CHECK32-NEXT:    srai a3, a0, 31
-; CHECK32-NEXT:    addi a4, zero, -1
-; CHECK32-NEXT:    mv a0, a1
-; CHECK32-NEXT:    bne a3, a4, .LBB9_2
+; CHECK32-NEXT:    bgtz a0, .LBB9_2
 ; CHECK32-NEXT:  # %bb.1:
-; CHECK32-NEXT:    mv a0, a2
+; CHECK32-NEXT:    mv a1, a2
 ; CHECK32-NEXT:  .LBB9_2:
+; CHECK32-NEXT:    mv a0, a1
 ; CHECK32-NEXT:    ret
 ;
 ; CHECK64-LABEL: icmpasrne:
 ; CHECK64:       # %bb.0:
-; CHECK64-NEXT:    sraiw a3, a0, 31
-; CHECK64-NEXT:    addi a4, zero, -1
+; CHECK64-NEXT:    sext.w a3, a0
 ; CHECK64-NEXT:    mv a0, a1
-; CHECK64-NEXT:    bne a3, a4, .LBB9_2
+; CHECK64-NEXT:    bgtz a3, .LBB9_2
 ; CHECK64-NEXT:  # %bb.1:
 ; CHECK64-NEXT:    mv a0, a2
 ; CHECK64-NEXT:  .LBB9_2:
 ; CHECK64-NEXT:    ret
   %sh = ashr i32 %input, 31
   %c = icmp ne i32 %sh, -1
   %s = select i1 %c, i32 %a, i32 %b
   ret i32 %s

llvm/test/CodeGen/X86/select-constant-xor.ll

; CHECK-NEXT:    retq
   %s = select i1 %c, i32 84, i32 -85
   ret i32 %s
 }
 
 define i32 @icmpasreq(i32 %input, i32 %a, i32 %b) {
 ; CHECK-LABEL: icmpasreq:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    movl %esi, %eax
-; CHECK-NEXT:    sarl $31, %edi
-; CHECK-NEXT:    cmpl $-1, %edi
-; CHECK-NEXT:    cmovnel %edx, %eax
+; CHECK-NEXT:    testl %edi, %edi
+; CHECK-NEXT:    cmovnsl %edx, %eax
 ; CHECK-NEXT:    retq
   %sh = ashr i32 %input, 31
   %c = icmp eq i32 %sh, -1
   %s = select i1 %c, i32 %a, i32 %b
   ret i32 %s
 }
 
 define i32 @icmpasrne(i32 %input, i32 %a, i32 %b) {
 ; CHECK-LABEL: icmpasrne:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    movl %esi, %eax
-; CHECK-NEXT:    sarl $31, %edi
-; CHECK-NEXT:    cmpl $-1, %edi
-; CHECK-NEXT:    cmovel %edx, %eax
+; CHECK-NEXT:    testl %edi, %edi
+; CHECK-NEXT:    cmovlel %edx, %eax
 ; CHECK-NEXT:    retq
   %sh = ashr i32 %input, 31
   %c = icmp ne i32 %sh, -1
   %s = select i1 %c, i32 %a, i32 %b
   ret i32 %s
 }
 
 define i32 @oneusecmp(i32 %a, i32 %b, i32 %d) {