[X86] Disable creating BEXTR from shift and mask operations with BMI. Only do it for TBM.

[craig.topper]

Summary

The BMI version of BEXTR takes a bit count and shift value packed into a register. TBM passes it as an immediate. Currently we have a DAG combine that creates BEXTR from shift and mask if either BMI or TBM is supported.

For the BMI case this means we have to move the immediate into a register first and then do the BEXTR. So its always 2 instructions. The shift and mask we replaced would have also been 2 instructions. On Intel hardware, BEXTR is 2 uops according to Agner Fog's tables so that means we probably went from 2 uops to do shift and mask, to 3 uops to move the immediate and do the BEXTR. So I doubt this is a win.

This patch disables the combine for BMI and leaves it only for TBM.

I'm trying to figure out if we can move the TBM version to isel as we are currently using BEXTR when we could zero extend AH/BH/CH/DH. The latter is handled by isel patterns while the BEXTR is handled as a DAG combine.

[RKSimon]

What about BMI cases that reuse the constant?

[craig.topper]

That's a fair point. Is there a way we can detect that case?

[RKSimon]

In D36055#826154, @craig.topper wrote:

That's a fair point. Is there a way we can detect that case?

It's tricky - maybe replace X86ISD::BEXTR with a X86ISD::BEXTRI that keeps the constant length/index separate to make it easier to split to SHR+AND in isel - would that help?

TBM usage of BEXTR is curious as well - annoyingly the instruction takes a 32-bit immediate, wasting code size. In some circumstances its better to use the reg form and reuse the value.....

And TBM is also useful to replace AND + low bit mask (index = 0) as it makes folding a load easier and is non-destructive, but if the length % 8 = 0 then its better to fold to movz.

[craig.topper]

If we create X86ISD::BEXTRI with split immediates. How would we create the combined immediate to select BEXTRI instruction. Maybe a custom inserter?

[RKSimon]

In D36055#826654, @craig.topper wrote:

If we create X86ISD::BEXTRI with split immediates. How would we create the combined immediate to select BEXTRI instruction. Maybe a custom inserter?

Not sure - I'm struggling to think of something that will deal with all the useful cases where we should still try to use BEXTR (foldable loads on BMI + multiple uses on BMI/TBM).

lib/Target/X86/X86ISelLowering.cpp

if (combineX86ShufflesRecursively({Op}, 0, Op, NonceMask, {},
       return SDValue(); // This routine will use CombineTo to replace N.
   }
 
   // Create BEXTR instructions
   // BEXTR is ((X >> imm) & (2**size-1))
   if (VT != MVT::i32 && VT != MVT::i64)
     return SDValue();
 
-  if (!Subtarget.hasBMI() && !Subtarget.hasTBM())
+  if (!Subtarget.hasTBM())
     return SDValue();
   if (N0.getOpcode() != ISD::SRA && N0.getOpcode() != ISD::SRL)
     return SDValue();
 
   ConstantSDNode *MaskNode = dyn_cast<ConstantSDNode>(N1);
   ConstantSDNode *ShiftNode = dyn_cast<ConstantSDNode>(N0.getOperand(1));
   if (MaskNode && ShiftNode) {
     uint64_t Mask = MaskNode->getZExtValue();

lib/Target/X86/X86InstrInfo.td

def : Pat<(srl (shl GR64:$src, (i8 (trunc (sub 64, GR32:$lz)))),
             (BZHI64rr GR64:$src,
               (INSERT_SUBREG (i64 (IMPLICIT_DEF)), GR32:$lz, sub_32bit))>;
   def : Pat<(srl (shl (loadi64 addr:$src), (i8 (trunc (sub 64, GR32:$lz)))),
                  (i8 (trunc (sub 64, GR32:$lz)))),
             (BZHI64rm addr:$src,
               (INSERT_SUBREG (i64 (IMPLICIT_DEF)), GR32:$lz, sub_32bit))>;
 } // HasBMI2
 
-let Predicates = [HasBMI] in {
-  def : Pat<(X86bextr GR32:$src1, GR32:$src2),
-            (BEXTR32rr GR32:$src1, GR32:$src2)>;
-  def : Pat<(X86bextr (loadi32 addr:$src1), GR32:$src2),
-            (BEXTR32rm addr:$src1, GR32:$src2)>;
-  def : Pat<(X86bextr GR64:$src1, GR64:$src2),
-            (BEXTR64rr GR64:$src1, GR64:$src2)>;
-  def : Pat<(X86bextr (loadi64 addr:$src1), GR64:$src2),
-            (BEXTR64rm addr:$src1, GR64:$src2)>;
-} // HasBMI
-
 multiclass bmi_pdep_pext<string mnemonic, RegisterClass RC,
                          X86MemOperand x86memop, Intrinsic Int,
                          PatFrag ld_frag> {
   def rr : I<0xF5, MRMSrcReg, (outs RC:$dst), (ins RC:$src1, RC:$src2),
              !strconcat(mnemonic, "\t{$src2, $src1, $dst|$dst, $src1, $src2}"),
              [(set RC:$dst, (Int RC:$src1, RC:$src2))]>,
              VEX_4V;
   def rm : I<0xF5, MRMSrcMem, (outs RC:$dst), (ins RC:$src1, x86memop:$src2),

test/CodeGen/X86/bmi.ll

; CHECK-NEXT:    retq
   ret i32 %tmp
 }
 
 declare i32 @llvm.x86.bmi.bextr.32(i32, i32)
 
 define i32 @bextr32b(i32 %x)  uwtable  ssp {
 ; CHECK-LABEL: bextr32b:
 ; CHECK:       # BB#0:
-; CHECK-NEXT:    movl $3076, %eax # imm = 0xC04
-; CHECK-NEXT:    bextrl %eax, %edi, %eax
+; CHECK-NEXT:    shrl $4, %edi
+; CHECK-NEXT:    andl $4095, %edi # imm = 0xFFF
+; CHECK-NEXT:    movl %edi, %eax
 ; CHECK-NEXT:    retq
   %1 = lshr i32 %x, 4
   %2 = and i32 %1, 4095
   ret i32 %2
 }
 
 define i32 @bextr32b_load(i32* %x)  uwtable  ssp {
 ; CHECK-LABEL: bextr32b_load:
 ; CHECK:       # BB#0:
-; CHECK-NEXT:    movl $3076, %eax # imm = 0xC04
-; CHECK-NEXT:    bextrl %eax, (%rdi), %eax
+; CHECK-NEXT:    movl (%rdi), %eax
+; CHECK-NEXT:    shrl $4, %eax
+; CHECK-NEXT:    andl $4095, %eax # imm = 0xFFF
 ; CHECK-NEXT:    retq
   %1 = load i32, i32* %x
   %2 = lshr i32 %1, 4
   %3 = and i32 %2, 4095
   ret i32 %3
 }
 
 define i64 @bextr64(i64 %x, i64 %y)   {
 ; CHECK-LABEL: bextr64:
 ; CHECK:       # BB#0:
 ; CHECK-NEXT:    bextrq %rsi, %rdi, %rax
 ; CHECK-NEXT:    retq
   %tmp = tail call i64 @llvm.x86.bmi.bextr.64(i64 %x, i64 %y)
   ret i64 %tmp
 }
 
 declare i64 @llvm.x86.bmi.bextr.64(i64, i64)
 
 define i64 @bextr64b(i64 %x)  uwtable  ssp {
 ; CHECK-LABEL: bextr64b:
 ; CHECK:       # BB#0:
-; CHECK-NEXT:    movl $3076, %eax # imm = 0xC04
-; CHECK-NEXT:    bextrl %eax, %edi, %eax
+; CHECK-NEXT:    shrl $4, %edi
+; CHECK-NEXT:    andl $4095, %edi # imm = 0xFFF
+; CHECK-NEXT:    movq %rdi, %rax
 ; CHECK-NEXT:    retq
   %1 = lshr i64 %x, 4
   %2 = and i64 %1, 4095
   ret i64 %2
 }
 
 define i64 @bextr64b_load(i64* %x) {
 ; CHECK-LABEL: bextr64b_load:
 ; CHECK:       # BB#0:
-; CHECK-NEXT:    movl $3076, %eax # imm = 0xC04
-; CHECK-NEXT:    bextrl %eax, (%rdi), %eax
+; CHECK-NEXT:    movl (%rdi), %eax
+; CHECK-NEXT:    shrl $4, %eax
+; CHECK-NEXT:    andl $4095, %eax # imm = 0xFFF
 ; CHECK-NEXT:    retq
   %1 = load i64, i64* %x, align 8
   %2 = lshr i64 %1, 4
   %3 = and i64 %2, 4095
   ret i64 %3
 }
 
 define i32 @non_bextr32(i32 %x) {