[AArch64][2/4]Regard (shl val, N) as a potential bit-field-positioning op regardless of the number of uses.

[mingmingl]

Summary

Before this patch (and the refactor D135843)

• isBitfieldPositioningOp requires 'SHL' node to have one use for non bigger pattern (code link)

After this patch

• A DAG node of (shl val, N) doesn't have the one use requirement.

The rationale is that, 'val' could be used as bit extraction source as long as N (the left shift amount) fits BiggerPattern requirement (that no extra shift node are created around this line). This would at least reduces one use of SHL if BFI instruction is used.

One existing test case is improved without regressing others. And there is no correctness issues, since BiggerPattern doesn't look at the number of uses before this patch)

[dmgreen]

Do you have a better test? One that doesn't get so heavily optimized by opt.

[mingmingl]

In D135844#3857874, @dmgreen wrote:

Do you have a better test? One that doesn't get so heavily optimized by opt.

Good question. When constructing test case, turns out changing shl (val, N) to UBFIZ could be counter-productive (regardless of number of uses), since shl (val, N) itself might be folded into aarch64 operand2.

Take @test_nouseful_bits as an example, bfxil w9, w0, #0, #8 is not better than orr w9, w0, w8, #8 (with w8 = and w0, 0xff) (higher throughput, shorter latency).

Together with the other motivating test case (https://godbolt.org/z/h96b1sGco for D135102) , planning to make changes when orr with a left shift is better than bfi.

[mingmingl]

For the affected test case test_nonuseful_bits, one BFM and one ORR is generated now (https://godbolt.org/z/a3c68f7dE) with this commit. Going to abandon this patch (rather than rebase it for other BFI improvements). Thanks for the discussions around BFM/ORR.

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

static bool isBitfieldPositioningOpFromShl(SelectionDAG *CurDAG, SDValue Op,
                                            int &Width) {
   assert(isShiftedMask_64(NonZeroBits) && "Caller guaranteed");
 
   SDNode *N = Op.getNode();
   uint64_t ShlImm;
   if (!isOpcWithIntImmediate(N, ISD::SHL, ShlImm))
     return false;
 
-  if (!BiggerPattern && !Op.hasOneUse())
-    return false;
-
   EVT VT = N->getValueType(0);
   assert((VT == MVT::i32 || VT == MVT::i64) &&
          "Caller guarantees that type is i32 or i64");
 
   SDValue ShlOp0 = N->getOperand(0);
 
   DstLSB = countTrailingZeros(NonZeroBits);
   Width = countTrailingOnes(NonZeroBits >> DstLSB);

llvm/test/CodeGen/AArch64/bitfield-insert.ll

; CHECK-NEXT:    ret
   ret void
 }
 
 ; Tests when all the bits from one operand are not useful
 define i32 @test_nouseful_bits(i8 %a, i32 %b) {
 ; CHECK-LABEL: test_nouseful_bits:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    and w8, w0, #0xff
+; CHECK-NEXT:    bfi w0, w8, #8, #8
 ; CHECK-NEXT:    lsl w8, w8, #8
-; CHECK-NEXT:    mov w9, w8
-; CHECK-NEXT:    bfxil w9, w0, #0, #8
-; CHECK-NEXT:    bfi w8, w9, #16, #16
+; CHECK-NEXT:    bfi w8, w0, #16, #16
 ; CHECK-NEXT:    mov w0, w8
 ; CHECK-NEXT:    ret
   %conv = zext i8 %a to i32     ;   0  0  0  A
   %shl = shl i32 %b, 8          ;   B2 B1 B0 0
   %or = or i32 %conv, %shl      ;   B2 B1 B0 A
   %shl.1 = shl i32 %or, 8       ;   B1 B0 A 0
   %or.1 = or i32 %conv, %shl.1  ;   B1 B0 A A
   %shl.2 = shl i32 %or.1, 8     ;   B0 A A 0