[Thumb] Teach ISel how to lower compares of AND bitmasks efficiently

[SjoerdMeijer]

Summary

This is recommit of r285893, but with a correctness fix. The problem of the original commit was that this:

bic r5, r7, #31
cbz r5, .LBB2_10

got rewritten into:

lsrs r5, r7, #5
beq .LBB2_10

The result in destination register r5 is not the same, and this is incorrect when r5 is not dead. So the fix includes checking the uses of the AND destination register.

For completeness, this was the original commit message:

For the common pattern (CMPZ (AND x, #bitmask), #0), we can do some more efficient instruction selection if the bitmask is one consecutive sequence of set bits (32 - clz(bm) - ctz(bm) == popcount(bm)).

1. If the bitmask touches the LSB, then we can remove all the upper bits and set the flags by doing one LSLS.
2. If the bitmask touches the MSB, then we can remove all the lower bits and set the flags with one LSRS.
3. If the bitmask has popcount == 1 (only one set bit), we can shift that bit into the sign bit with one LSLS and change the condition query from NE/EQ to MI/PL (we could also implement this by shifting into the carry bit and branching on BCC/BCS).
4. Otherwise, we can emit a sequence of LSLS+LSRS to remove the upper and lower zero bits of the mask.

1-3 require only one 16-bit instruction and can elide the CMP. 4 requires two 16-bit instructions but can elide the CMP and doesn't require materializing a complex immediate, so is also a win.

[jmolloy]

LGTM, based on this code being accepted by multiple reviewers in a previous incarnation and the fix being obvious.

Thanks for fixing my broken code Sjoerd!

llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.cpp

bool ARMBaseInstrInfo::optimizeCompareInstr(
   case ARM::t2ANDri:
   case ARM::ORRrr:
   case ARM::ORRri:
   case ARM::t2ORRrr:
   case ARM::t2ORRri:
   case ARM::EORrr:
   case ARM::EORri:
   case ARM::t2EORrr:
-  case ARM::t2EORri: {
+  case ARM::t2EORri:
+  case ARM::t2LSRri:
+  case ARM::t2LSRrr:
+  case ARM::t2LSLri:
+  case ARM::t2LSLrr: {
     // Scan forward for the use of CPSR
     // When checking against MI: if it's a conditional code that requires
     // checking of the V bit or C bit, then this is not safe to do.
     // It is safe to remove CmpInstr if CPSR is redefined or killed.
     // If we are done with the basic block, we need to check whether CPSR is
     // live-out.
     SmallVector<std::pair<MachineOperand*, ARMCC::CondCodes>, 4>
         OperandsToUpdate;

llvm/trunk/lib/Target/ARM/ARMISelDAGToDAG.cpp

private:
   bool tryABSOp(SDNode *N);
 
   bool tryReadRegister(SDNode *N);
   bool tryWriteRegister(SDNode *N);
 
   bool tryInlineAsm(SDNode *N);
 
   void SelectConcatVector(SDNode *N);
+  void SelectCMPZ(SDNode *N, bool &SwitchEQNEToPLMI);
 
   bool trySMLAWSMULW(SDNode *N);
 
   void SelectCMP_SWAP(SDNode *N);
 
   /// SelectInlineAsmMemoryOperand - Implement addressing mode selection for
   /// inline asm expressions.
   bool SelectInlineAsmMemoryOperand(const SDValue &Op, unsigned ConstraintID,
void ARMDAGToDAGISel::SelectConcatVector(SDNode *N) {
   // The only time a CONCAT_VECTORS operation can have legal types is when
   // two 64-bit vectors are concatenated to a 128-bit vector.
   EVT VT = N->getValueType(0);
   if (!VT.is128BitVector() || N->getNumOperands() != 2)
     llvm_unreachable("unexpected CONCAT_VECTORS");
   ReplaceNode(N, createDRegPairNode(VT, N->getOperand(0), N->getOperand(1)));
 }
 
+static Optional<std::pair<unsigned, unsigned>>
+getContiguousRangeOfSetBits(const APInt &A) {
+  unsigned FirstOne = A.getBitWidth() - A.countLeadingZeros() - 1;
+  unsigned LastOne = A.countTrailingZeros();
+  if (A.countPopulation() != (FirstOne - LastOne + 1))
+    return Optional<std::pair<unsigned,unsigned>>();
+  return std::make_pair(FirstOne, LastOne);
+}
+
+void ARMDAGToDAGISel::SelectCMPZ(SDNode *N, bool &SwitchEQNEToPLMI) {
+  assert(N->getOpcode() == ARMISD::CMPZ);
+  SwitchEQNEToPLMI = false;
+
+  if (!Subtarget->isThumb())
+    // FIXME: Work out whether it is profitable to do this in A32 mode - LSL and
+    // LSR don't exist as standalone instructions - they need the barrel shifter.
+    return;
+
+  // select (cmpz (and X, C), #0) -> (LSLS X) or (LSRS X) or (LSRS (LSLS X))
+  SDValue And = N->getOperand(0);
+  if (!And->hasOneUse())
+    return;
+
+  SDValue Zero = N->getOperand(1);
+  if (!isa<ConstantSDNode>(Zero) || !cast<ConstantSDNode>(Zero)->isNullValue() ||
+      And->getOpcode() != ISD::AND)
+    return;
+  SDValue X = And.getOperand(0);
+  auto C = dyn_cast<ConstantSDNode>(And.getOperand(1));
+
+  if (!C || !X->hasOneUse())
+    return;
+  auto Range = getContiguousRangeOfSetBits(C->getAPIntValue());
+  if (!Range)
+    return;
+
+  // There are several ways to lower this:
+  SDNode *NewN;
+  SDLoc dl(N);
+
+  auto EmitShift = [&](unsigned Opc, SDValue Src, unsigned Imm) -> SDNode* {
+    if (Subtarget->isThumb2()) {
+      Opc = (Opc == ARM::tLSLri) ? ARM::t2LSLri : ARM::t2LSRri;
+      SDValue Ops[] = { Src, CurDAG->getTargetConstant(Imm, dl, MVT::i32),
+                        getAL(CurDAG, dl), CurDAG->getRegister(0, MVT::i32),
+                        CurDAG->getRegister(0, MVT::i32) };
+      return CurDAG->getMachineNode(Opc, dl, MVT::i32, Ops);
+    } else {
+      SDValue Ops[] = {CurDAG->getRegister(ARM::CPSR, MVT::i32), Src,
+                       CurDAG->getTargetConstant(Imm, dl, MVT::i32),
+                       getAL(CurDAG, dl), CurDAG->getRegister(0, MVT::i32)};
+      return CurDAG->getMachineNode(Opc, dl, MVT::i32, Ops);
+    }
+  };
+  
+  if (Range->second == 0) {
+    //  1. Mask includes the LSB -> Simply shift the top N bits off
+    NewN = EmitShift(ARM::tLSLri, X, 31 - Range->first);
+    ReplaceNode(And.getNode(), NewN);
+  } else if (Range->first == 31) {
+    //  2. Mask includes the MSB -> Simply shift the bottom N bits off
+    NewN = EmitShift(ARM::tLSRri, X, Range->second);
+    ReplaceNode(And.getNode(), NewN);
+  } else if (Range->first == Range->second) {
+    //  3. Only one bit is set. We can shift this into the sign bit and use a
+    //     PL/MI comparison.
+    NewN = EmitShift(ARM::tLSLri, X, 31 - Range->first);
+    ReplaceNode(And.getNode(), NewN);
+
+    SwitchEQNEToPLMI = true;
+  } else if (!Subtarget->hasV6T2Ops()) {
+    //  4. Do a double shift to clear bottom and top bits, but only in
+    //     thumb-1 mode as in thumb-2 we can use UBFX.
+    NewN = EmitShift(ARM::tLSLri, X, 31 - Range->first);
+    NewN = EmitShift(ARM::tLSRri, SDValue(NewN, 0),
+                     Range->second + (31 - Range->first));
+    ReplaceNode(And.getNode(), NewN);
+  }
+
+}
+
 void ARMDAGToDAGISel::Select(SDNode *N) {
   SDLoc dl(N);
 
   if (N->isMachineOpcode()) {
     N->setNodeId(-1);
     return;   // Already selected.
   }
 
if (N0.getOpcode() == ISD::OR && N0.getNode()->hasOneUse()) {
         SDValue Imm16 = CurDAG->getTargetConstant((N2CVal & 0xFFFF0000U) >> 16,
                                                   dl, MVT::i32);
         SDValue Ops[] = { N0.getOperand(0), Imm16,
                           getAL(CurDAG, dl), CurDAG->getRegister(0, MVT::i32) };
         ReplaceNode(N, CurDAG->getMachineNode(Opc, dl, VT, Ops));
         return;
       }
     }
+
     break;
   }
   case ARMISD::VMOVRRD:
     ReplaceNode(N, CurDAG->getMachineNode(ARM::VMOVRRD, dl, MVT::i32, MVT::i32,
                                           N->getOperand(0), getAL(CurDAG, dl),
                                           CurDAG->getRegister(0, MVT::i32)));
     return;
   case ISD::UMUL_LOHI: {
case ARMISD::BRCOND: {
     SDValue N1 = N->getOperand(1);
     SDValue N2 = N->getOperand(2);
     SDValue N3 = N->getOperand(3);
     SDValue InFlag = N->getOperand(4);
     assert(N1.getOpcode() == ISD::BasicBlock);
     assert(N2.getOpcode() == ISD::Constant);
     assert(N3.getOpcode() == ISD::Register);
 
-    SDValue Tmp2 = CurDAG->getTargetConstant(((unsigned)
-                               cast<ConstantSDNode>(N2)->getZExtValue()), dl,
-                               MVT::i32);
+    unsigned CC = (unsigned) cast<ConstantSDNode>(N2)->getZExtValue();
+    
+    if (InFlag.getOpcode() == ARMISD::CMPZ) {
+      bool SwitchEQNEToPLMI;
+      SelectCMPZ(InFlag.getNode(), SwitchEQNEToPLMI);
+      InFlag = N->getOperand(4);
+
+      if (SwitchEQNEToPLMI) {
+        switch ((ARMCC::CondCodes)CC) {
+        default: llvm_unreachable("CMPZ must be either NE or EQ!");
+        case ARMCC::NE:
+          CC = (unsigned)ARMCC::MI;
+          break;
+        case ARMCC::EQ:
+          CC = (unsigned)ARMCC::PL;
+          break;
+        }
+      }
+    }
+
+    SDValue Tmp2 = CurDAG->getTargetConstant(CC, dl, MVT::i32);
     SDValue Ops[] = { N1, Tmp2, N3, Chain, InFlag };
     SDNode *ResNode = CurDAG->getMachineNode(Opc, dl, MVT::Other,
                                              MVT::Glue, Ops);
     Chain = SDValue(ResNode, 0);
     if (N->getNumValues() == 2) {
       InFlag = SDValue(ResNode, 1);
       ReplaceUses(SDValue(N, 1), InFlag);
     }
if (C && C->getSExtValue() < 0 && Subtarget->isThumb()) {
       if (Add) {
         SDValue Ops2[] = {SDValue(Add, 0), CurDAG->getConstant(0, dl, MVT::i32)};
         CurDAG->MorphNodeTo(N, ARMISD::CMPZ, CurDAG->getVTList(MVT::Glue), Ops2);
       }
     }
     // Other cases are autogenerated.
     break;
   }
-    
+  case ARMISD::CMOV: {
+    SDValue InFlag = N->getOperand(4);
+
+    if (InFlag.getOpcode() == ARMISD::CMPZ) {
+      bool SwitchEQNEToPLMI;
+      SelectCMPZ(InFlag.getNode(), SwitchEQNEToPLMI);
+
+      if (SwitchEQNEToPLMI) {
+        SDValue ARMcc = N->getOperand(2);
+        ARMCC::CondCodes CC =
+          (ARMCC::CondCodes)cast<ConstantSDNode>(ARMcc)->getZExtValue();
+
+        switch (CC) {
+        default: llvm_unreachable("CMPZ must be either NE or EQ!");
+        case ARMCC::NE:
+          CC = ARMCC::MI;
+          break;
+        case ARMCC::EQ:
+          CC = ARMCC::PL;
+          break;
+        }
+        SDValue NewARMcc = CurDAG->getConstant((unsigned)CC, dl, MVT::i32);
+        SDValue Ops[] = {N->getOperand(0), N->getOperand(1), NewARMcc,
+                         N->getOperand(3), N->getOperand(4)};
+        CurDAG->MorphNodeTo(N, ARMISD::CMOV, N->getVTList(), Ops);
+      }
+
+    }
+    // Other cases are autogenerated.
+    break;
+  }
+    
   case ARMISD::VZIP: {
     unsigned Opc = 0;
     EVT VT = N->getValueType(0);
     switch (VT.getSimpleVT().SimpleTy) {
     default: return;
     case MVT::v8i8:  Opc = ARM::VZIPd8; break;
     case MVT::v4i16: Opc = ARM::VZIPd16; break;
     case MVT::v2f32:

llvm/trunk/test/CodeGen/ARM/and-cmpz.ll

+; RUN: llc -mtriple=thumbv7m-linux-gnu < %s | FileCheck %s --check-prefix=CHECK --check-prefix=T2
+; RUN: llc -mtriple=thumbv6m-linux-gnu < %s | FileCheck %s --check-prefix=CHECK --check-prefix=T1
+
+; CHECK-LABEL: single_bit:
+; CHECK: lsls r0, r0, #23
+; T2-NEXT: mov
+; T2-NEXT: it
+; T1-NEXT: bmi
+define i32 @single_bit(i32 %p) {
+  %a = and i32 %p, 256
+  %b = icmp eq i32 %a, 0
+  br i1 %b, label %true, label %false
+
+true:
+  ret i32 1
+
+false:
+  ret i32 2
+}
+
+; CHECK-LABEL: multi_bit_lsb_ubfx:
+; CHECK: lsls r0, r0, #24
+; T2-NEXT: mov
+; T2-NEXT: it
+; T1-NEXT: beq
+define i32 @multi_bit_lsb_ubfx(i32 %p) {
+  %a = and i32 %p, 255
+  %b = icmp eq i32 %a, 0
+  br i1 %b, label %true, label %false
+
+true:
+  ret i32 1
+
+false:
+  ret i32 2
+}
+
+; CHECK-LABEL: multi_bit_msb:
+; CHECK: lsrs r0, r0, #24
+; T2-NEXT: mov
+; T2-NEXT: it
+; T1-NEXT: beq
+define i32 @multi_bit_msb(i32 %p) {
+  %a = and i32 %p, 4278190080  ; 0xff000000
+  %b = icmp eq i32 %a, 0
+  br i1 %b, label %true, label %false
+
+true:
+  ret i32 1
+
+false:
+  ret i32 2
+}
+
+; CHECK-LABEL: multi_bit_nosb:
+; T1: lsls r0, r0, #8
+; T1-NEXT: lsrs r0, r0, #24
+; T2: tst.w
+; T2-NEXT: it
+; T1-NEXT: beq
+define i32 @multi_bit_nosb(i32 %p) {
+  %a = and i32 %p, 16711680 ; 0x00ff0000
+  %b = icmp eq i32 %a, 0
+  br i1 %b, label %true, label %false
+
+true:
+  ret i32 1
+
+false:
+  ret i32 2
+}

llvm/trunk/test/CodeGen/ARM/arm-and-tst-peephole.ll

 ; T2-LABEL: bar:
 ; V8-LABEL: bar:
 define internal zeroext i8 @bar(%struct.S* %x, %struct.S* nocapture %y) nounwind readonly {
 entry:
   %0 = getelementptr inbounds %struct.S, %struct.S* %x, i32 0, i32 1, i32 0
   %1 = load i8, i8* %0, align 1
   %2 = zext i8 %1 to i32
 ; ARM: ands
-; THUMB: ands
+; THUMB: ands 
 ; T2: ands
 ; V8: ands
 ; V8-NEXT: beq
   %3 = and i32 %2, 112
   %4 = icmp eq i32 %3, 0
   br i1 %4, label %return, label %bb
 
 bb:                                               ; preds = %entry

 ; THUMB-LABEL: test_tst_assessment:
 ; T2-LABEL: test_tst_assessment:
 ; V8-LABEL: test_tst_assessment:
 define i32 @test_tst_assessment(i1 %lhs, i1 %rhs) {
   %lhs32 = zext i1 %lhs to i32
   %rhs32 = zext i1 %rhs to i32
   %diff = sub nsw i32 %lhs32, %rhs32
 ; ARM: tst r1, #1
-; THUMB: movs [[RTMP:r[0-9]+]], #1
-; THUMB: tst r1, [[RTMP]]
-; T2: tst.w r1, #1
-; V8: tst.w r1, #1
+; THUMB: lsls r1, r1, #31
+; T2: lsls r1, r1, #31
+; V8: lsls r1, r1, #31
   ret i32 %diff
 }
 
 !1 = !{!"branch_weights", i32 1, i32 1, i32 3, i32 2 }

llvm/trunk/test/CodeGen/ARM/arm-shrink-wrapping.ll

 declare double @llvm.pow.f64(double, double)
 
 ; This function needs to spill floating point registers to
 ; exercise the path where we were dereferencing the end iterator
 ; to access debug info location while inserting the spill code
 ; during PEI with shrink-wrapping enable.
 ; CHECK-LABEL: debug_info:
 ;
-; ENABLE: tst{{(\.w)?}}  r2, #1
+; ENABLE: {{tst  r2, #1|lsls r1, r2, #31}}
 ; ENABLE-NEXT: beq      [[BB13:LBB[0-9_]+]]
 ;
 ; CHECK: push
 ;
-; DISABLE: tst{{(\.w)?}}  r2, #1
-; DISABLE-NEXT: vst1.64
-; DISABLE-NEXT: beq      [[BB13:LBB[0-9_]+]]
+; DISABLE: {{tst  r2, #1|lsls r1, r2, #31}}
+; DISABLE: beq      [[BB13:LBB[0-9_]+]]
 ;
 ; CHECK: bl{{x?}} _pow
 ;
 ;
 ; ENABLE: pop
 ;
 ; CHECK: [[BB13]]:
 ; CHECK: vldr

llvm/trunk/test/CodeGen/ARM/call-tc.ll

; CHECKT2D: bne.w _a
   br label %return
 
 if.end:                                           ; preds = %entry
   %and1 = and i32 %x, 2
   %tobool2 = icmp eq i32 %and1, 0
   br i1 %tobool2, label %if.end5, label %if.then3
 
 if.then3:                                         ; preds = %if.end
-; CHECKT2D: bne.w _b
+; CHECKT2D: bmi.w _b
   %call4 = tail call i32 @b(i32 %x) nounwind
   br label %return
 
 if.end5:                                          ; preds = %if.end
 ; CHECKT2D: b.w _c
   %call6 = tail call i32 @c(i32 %x) nounwind
   br label %return
 

llvm/trunk/test/CodeGen/ARM/debug-info-branch-folding.ll

 ; RUN: llc < %s - | FileCheck %s
 target datalayout = "e-p:32:32:32-i1:8:32-i8:8:32-i16:16:32-i32:32:32-i64:32:32-f32:32:32-f64:32:32-v64:32:64-v128:32:128-a0:0:32-n32"
 target triple = "thumbv7-apple-macosx10.6.7"
 
 ;CHECK:   vadd.f32  q4, q8, q8
-;CHECK:         LBB0_1:
-;CHECK-NOT:     beq LBB0_1
+;CHECK-NEXT: LBB0_1
 
 ;CHECK:         @DEBUG_VALUE: x <- %Q4{{$}}
 ;CHECK-NEXT:    @DEBUG_VALUE: y <- %Q4{{$}}
 ;CHECK:         beq LBB0_1
 
 
 @.str = external constant [13 x i8]
 

llvm/trunk/test/CodeGen/Thumb/thumb-shrink-wrapping.ll

 ; CHECK-LABEL: beq_to_bx:
 ; DISABLE: push {r4, lr}
 ; CHECK: cmp r2, #0
 ; CHECK-NEXT: beq [[EXIT_LABEL:LBB[0-9_]+]]
 ; ENABLE: push {r4, lr}
 
 ; CHECK: tst r3, r4
 ; ENABLE-NEXT: pop {r4}
-; ENABLE-NEXT: pop {r3}
-; ENABLE-NEXT: mov lr, r3
+; ENABLE-NEXT: mov r12, r{{.*}}
+; ENABLE-NEXT: pop {r0}
+; ENABLE-NEXT: mov lr, r0
+; ENABLE-NEXT: mov r0, r12
 ; CHECK-NEXT: beq [[EXIT_LABEL]]
 
 ; CHECK: str r1, [r2]
+; CHECK: str r3, [r2]
 ; CHECK-NEXT: movs r0, #0
 ; CHECK-NEXT: [[EXIT_LABEL]]: @ %cleanup
 ; ENABLE-NEXT: bx lr
 ; DISABLE-V5-NEXT: pop {r4, pc}
 ; DISABLE-V4T-NEXT: pop {r4}
 ; DISABLE-V4T-NEXT: pop {r1}
 ; DISABLE-V4T-NEXT: bx r1
 
 entry:
   %cmp = icmp eq i32* %y, null
   br i1 %cmp, label %cleanup, label %if.end
 
 if.end:
   %z = load i32, i32* %y, align 4
   %and = and i32 %z, 2
   %cmp2 = icmp eq i32 %and, 0
   br i1 %cmp2, label %cleanup, label %if.end4
 
 if.end4:
   store i32 %head, i32* %y, align 4
+  store volatile i32 %z, i32* %y, align 4
   br label %cleanup
 
 cleanup:
   %retval.0 = phi i1 [ 0, %if.end4 ], [ 1, %entry ], [ 1, %if.end ]
   ret i1 %retval.0
 }
 
 attributes #0 = { noreturn nounwind }

llvm/trunk/test/CodeGen/Thumb2/float-ops.ll

 ; NONE-NOT: mov
 ; HARD: vmov r0, r1, d0
   %1 = bitcast double %a to i64
   ret i64 %1
 }
 
 define float @select_f(float %a, float %b, i1 %c) {
 ; CHECK-LABEL: select_f:
-; NONE: tst.w   r2, #1
+; NONE: lsls    r2, r2, #31
 ; NONE: moveq   r0, r1
-; HARD: tst.w   r0, #1
+; HARD: lsls    r0, r0, #31
 ; VFP4-ALL: vmovne.f32      s1, s0
 ; VFP4-ALL: vmov.f32        s0, s1
 ; FP-ARMv8: vseleq.f32 s0, s1, s0
   %1 = select i1 %c, float %a, float %b
   ret float %1
 }
 
 define double @select_d(double %a, double %b, i1 %c) {
 ; CHECK-LABEL: select_d:
-; NONE: ldr.w   [[REG:r[0-9]+]], [sp]
-; NONE: ands    [[REG]], [[REG]], #1
+; NONE: ldr{{(.w)?}}     [[REG:r[0-9]+]], [sp]
+; NONE  ands    [[REG]], [[REG]], #1
 ; NONE: moveq   r0, r2
 ; NONE: moveq   r1, r3
 ; SP: ands r0, r0, #1
 ; SP-DAG: vmov [[ALO:r[0-9]+]], [[AHI:r[0-9]+]], d0
 ; SP-DAG: vmov [[BLO:r[0-9]+]], [[BHI:r[0-9]+]], d1
 ; SP: itt ne
 ; SP-DAG: movne [[BLO]], [[ALO]]
 ; SP-DAG: movne [[BHI]], [[AHI]]
 ; SP: vmov d0, [[BLO]], [[BHI]]
-; DP: tst.w   r0, #1
+; DP: lsls   r0, r0, #31
 ; VFP4-DP: vmovne.f64      d1, d0
 ; VFP4-DP: vmov.f64        d0, d1
 ; FP-ARMV8: vseleq.f64      d0, d1, d0
   %1 = select i1 %c, double %a, double %b
   ret double %1
 }