[DAGCombine] Enable more srl + load combines

[samparker]

Summary

Change the calculation for the desired ValueType for non-sign extending loads, as in those cases we don't care about the higher bits. This creates a smaller ExtVT and allows for such combinations as:

(srl (zextload i16, [addr]), 8) -> (zextload i8, [addr + 1])

[SjoerdMeijer]

Hi Sam, that's a very nice optimisation, but I think we need more tests. The "variables" that we have here are the values types (load byte, load half, load word), and the different possible offsets. Guessing a bit here, but that must be something like 3 * 3(?) = 9 tests.
And another question is how this interacts with -munaligned-access and -mno-unaligned-access?

[samparker]

Hi Sjoerd,

Yes I agree, more tests would be good. I've added V6-M to test whether no unaligned accesses are permitted, but admittedly that is for only one of the test cases. So I will add some more tests for different byte positions and some more for no unaligned accesses.

Many thanks!
sam

[samparker]

Increased the number of tests by adding different shift values, plus I've added a +strict-align target.

[SjoerdMeijer]

Looks good to me.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

     return SDValue();
 
   // Special case: SIGN_EXTEND_INREG is basically truncating to ExtVT then
   // extended to VT.
   if (Opc == ISD::SIGN_EXTEND_INREG) {
     ExtType = ISD::SEXTLOAD;
     ExtVT = cast<VTSDNode>(N->getOperand(1))->getVT();
   } else if (Opc == ISD::SRL) {
-    // Another special-case: SRL is basically zero-extending a narrower value.
+    // Another special-case: SRL is basically zero-extending a narrower value,
+    // or it maybe shifting a higher subword, half or byte into the lowest
+    // bits.
     ExtType = ISD::ZEXTLOAD;
     N0 = SDValue(N, 0);
-    ConstantSDNode *N01 = dyn_cast<ConstantSDNode>(N0.getOperand(1));
-    if (!N01) return SDValue();
-    ExtVT = EVT::getIntegerVT(*DAG.getContext(),
-                              VT.getSizeInBits() - N01->getZExtValue());
+
+    auto *LN0 = dyn_cast<LoadSDNode>(N0.getOperand(0));
+    auto *N01 = dyn_cast<ConstantSDNode>(N0.getOperand(1));
+    if (!N01 || !LN0)
+      return SDValue();
+
+    uint64_t ShiftAmt = N01->getZExtValue();
+    uint64_t MemoryWidth = LN0->getMemoryVT().getSizeInBits();
+    if (LN0->getExtensionType() != ISD::SEXTLOAD && MemoryWidth > ShiftAmt)
+      ExtVT = EVT::getIntegerVT(*DAG.getContext(), MemoryWidth - ShiftAmt);
+    else
+      ExtVT = EVT::getIntegerVT(*DAG.getContext(),
+                                VT.getSizeInBits() - N01->getZExtValue());
   }
   if (LegalOperations && !TLI.isLoadExtLegal(ExtType, VT, ExtVT))
     return SDValue();
 
   unsigned EVTBits = ExtVT.getSizeInBits();
 
   // Do not generate loads of non-round integer types since these can
   // be expensive (and would be wrong if the type is not byte sized).

test/CodeGen/ARM/shift-combine.ll

-; RUN: llc -mtriple=armv7-linux-gnueabihf %s -o - | FileCheck %s
+; RUN: llc -mtriple=armv7-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-ARM --check-prefix=CHECK-COMMON
+; RUN: llc -mtriple=armv7eb-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-BE
+; RUN: llc -mtriple=thumbv7-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-THUMB --check-prefix=CHECK-COMMON
+; RUN: llc -mtriple=thumbv7m %s -o - | FileCheck %s --check-prefix=CHECK-THUMB --check-prefix=CHECK-COMMON
+; RUN: llc -mtriple=thumbv6m %s -o - | FileCheck %s --check-prefix=CHECK-V6M
 
 @array = weak global [4 x i32] zeroinitializer
 
 define i32 @test_lshr_and1(i32 %x) {
 entry:
 ;CHECK-LABLE: test_lshr_and1:
-;CHECK:         movw r1, :lower16:array
-;CHECK-NEXT:    and  r0, r0, #12
-;CHECK-NEXT:    movt r1, :upper16:array
-;CHECK-NEXT:    ldr  r0, [r1, r0]
-;CHECK-NEXT:    bx   lr
+;CHECK-COMMON:         movw r1, :lower16:array
+;CHECK-COMMON-NEXT:    and  r0, r0, #12
+;CHECK-COMMON-NEXT:    movt r1, :upper16:array
+;CHECK-COMMON-NEXT:    ldr  r0, [r1, r0]
+;CHECK-COMMON-NEXT:    bx   lr
   %tmp2 = lshr i32 %x, 2
   %tmp3 = and i32 %tmp2, 3
   %tmp4 = getelementptr [4 x i32], [4 x i32]* @array, i32 0, i32 %tmp3
   %tmp5 = load i32, i32* %tmp4, align 4
   ret i32 %tmp5
 }
 define i32 @test_lshr_and2(i32 %x) {
 entry:
-;CHECK-LABLE: test_lshr_and2:
-;CHECK:         ubfx r0, r0, #1, #15
-;CHECK-NEXT:    add  r0, r0, r0
-;CHECK-NEXT:    bx   lr
+;CHECK-LABEL: test_lshr_and2:
+;CHECK-COMMON:      ubfx r0, r0, #1, #15
+;CHECK-ARM:         add  r0, r0, r0
+;CHECK-THUMB:       add  r0, r0
+;CHECK-COMMON-NEXT: bx   lr
   %a = and i32 %x, 65534
   %b = lshr i32 %a, 1
   %c = and i32 %x, 65535
   %d = lshr i32 %c, 1
   %e = add i32 %b, %d
   ret i32 %e
 }
+
+; CHECK-LABEL: test_lshr_load1
+; CHECK-BE: ldrb r0, [r0]
+; CHECK-COMMON: ldrb r0, [r0, #1]
+; CHECK-COMMON-NEXT: bx
+define arm_aapcscc i32 @test_lshr_load1(i16* %a) {
+entry:
+  %0 = load i16, i16* %a, align 2
+  %conv1 = zext i16 %0 to i32
+  %1 = lshr i32 %conv1, 8
+  ret i32 %1
+}
+
+; CHECK-LABEL: test_lshr_load1_sext
+; CHECK-ARM: ldrsh r0, [r0]
+; CHECK-ARM-NEXT: lsr r0, r0, #8
+; CHECK-THUMB: ldrsh.w r0, [r0]
+; CHECK-THUMB-NEXT: lsrs r0, r0, #8
+; CHECK-COMMON: bx
+define arm_aapcscc i32 @test_lshr_load1_sext(i16* %a) {
+entry:
+  %0 = load i16, i16* %a, align 2
+  %conv1 = sext i16 %0 to i32
+  %1 = lshr i32 %conv1, 8
+  ret i32 %1
+}
+
+; CHECK-LABEL: test_lshr_load1_fail
+; CHECK-COMMON: ldrh r0, [r0]
+; CHECK-ARM: lsr r0, r0, #9
+; CHECK-THUMB: lsrs r0, r0, #9
+; CHECK-COMMON-NEXT: bx
+define arm_aapcscc i32 @test_lshr_load1_fail(i16* %a) {
+entry:
+  %0 = load i16, i16* %a, align 2
+  %conv1 = zext i16 %0 to i32
+  %1 = lshr i32 %conv1, 9
+  ret i32 %1
+}
+
+; CHECK-LABEL: test_lshr_load2
+; CHECK-BE: ldrh r0, [r0]
+; CHECK-COMMON: ldrh r0, [r0, #2]
+; CHECK-COMMON-NEXT: bx
+define arm_aapcscc i32 @test_lshr_load2(i32* %a) {
+entry:
+  %0 = load i32, i32* %a, align 4
+  %1 = lshr i32 %0, 16
+  ret i32 %1
+}
+
+; CHECK-LABEL: test_lshr_load2_fail
+; CHECK-COMMON: ldr r0, [r0]
+; CHECK-ARM: lsr r0, r0, #15
+; CHECK-THUMB: lsrs r0, r0, #15
+; CHECK-COMMON-NEXT: bx
+define arm_aapcscc i32 @test_lshr_load2_fail(i32* %a) {
+entry:
+  %0 = load i32, i32* %a, align 4
+  %1 = lshr i32 %0, 15
+  ret i32 %1
+}
+
+; CHECK-LABEL: test_lshr_load4
+; CHECK-BE: ldr r0, [r0]
+; CHECK-COMMON: ldr r0, [r0, #4]
+; CHECK-COMMON-NEXT: bx
+define arm_aapcscc i32 @test_lshr_load4(i64* %a) {
+entry:
+  %0 = load i64, i64* %a, align 8
+  %1 = lshr i64 %0, 32
+  %conv = trunc i64 %1 to i32
+  ret i32 %conv
+}
+
+; CHECK-LABEL: test_lshr_load4_fail
+; CHECK-COMMON: ldrd r0, r1, [r0]
+; CHECK-ARM: lsr r0, r0, #8
+; CHECK-ARM-NEXT: orr r0, r0, r1, lsl #24
+; CHECK-THUMB: lsrs r0, r0, #8
+; CHECK-THUMB-NEXT: orr.w r0, r0, r1, lsl #24
+; CHECK-COMMON: bx
+define arm_aapcscc i32 @test_lshr_load4_fail(i64* %a) {
+entry:
+  %0 = load i64, i64* %a, align 8
+  %1 = lshr i64 %0, 8
+  %conv = trunc i64 %1 to i32
+  ret i32 %conv
+}
+
+; CHECK-LABEL: test_lshr_load5
+; CHECK-BE: ldr r1, [r0]
+; CHECK-BE-NEXT: ldrh r0, [r0, #4]
+; CHECK-BE-NEXT: orr r0, r0, r1, lsl #16
+; CHECK-V6M: ldrh r1, [r0, #2]
+; CHECK-V6M-NEXT: ldr r0, [r0, #4]
+; CHECK-V6M-NEXT: lsls r0, r0, #16
+; CHECK-V6M-NEXT: orrs r0, r1
+; CHECK-ARM: ldr r0, [r0, #2]
+; CHECK-THUMB: ldr.w r0, [r0, #2]
+; CHECK-COMMON-NEXT: bx
+define arm_aapcscc i32 @test_lshr_load5(i64* %a) {
+entry:
+  %0 = load i64, i64* %a, align 8
+  %1 = lshr i64 %0, 16
+  %conv = trunc i64 %1 to i32
+  ret i32 %conv
+}