merge consecutive loads that are offset from a base address (PR21771)

[spatel]

Summary

SelectionDAG::isConsecutiveLoad() was not detecting consecutive loads when the first load was offset from a base address. This patch recognizes that pattern and subtracts the offset before comparing the second load to see if it is consecutive.

The codegen change in the new test case improves from:

vmovsd	32(%rdi), %xmm0
vmovsd	48(%rdi), %xmm1 
vmovhpd	56(%rdi), %xmm1, %xmm1
vmovhpd	40(%rdi), %xmm0, %xmm0
vinsertf128	$1, %xmm1, %ymm0, %ymm0

To:

vmovups	32(%rdi), %ymm0

An existing test case is also improved from:

vmovsd	(%rdi), %xmm0
vmovsd	16(%rdi), %xmm1
vmovsd	24(%rdi), %xmm2
vunpcklpd	%xmm2, %xmm0, %xmm0 ## xmm0 = xmm0[0],xmm2[0]
vmovhpd	8(%rdi), %xmm1, %xmm3

To:

vmovsd	(%rdi), %xmm0
vmovsd	16(%rdi), %xmm1
vmovhpd	24(%rdi), %xmm0, %xmm0
vmovhpd	8(%rdi), %xmm1, %xmm1

This patch fixes PR21771 ( http://llvm.org/bugs/show_bug.cgi?id=21771 ).

[RKSimon]

Minor test improvement.

test/CodeGen/X86/chain_order.ll

11

Ideally it'd be better to use CHECK-NEXT - especially as there are more load / float instructions than what is being checked for.

[spatel]

Thanks, Simon! Patch updated with tighter checking of the outputs on the existing test case.

[hfinkel]

With the type changes noted below, LGTM.

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

6558

This is a generic offset, and should be int64_t.

6562

The LLVM convention is to call the type 'int', not 'signed'. Please say (int)Bytes.

6567

Same here, this needs to be int64_t.

6569

Same here.

[spatel]

Thanks, Hal!

Fixed variable type slop and checked in at r224379.

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

if (Loc.getOpcode() == ISD::FrameIndex) {
     int FI  = cast<FrameIndexSDNode>(Loc)->getIndex();
     int BFI = cast<FrameIndexSDNode>(BaseLoc)->getIndex();
     int FS  = MFI->getObjectSize(FI);
     int BFS = MFI->getObjectSize(BFI);
     if (FS != BFS || FS != (int)Bytes) return false;
     return MFI->getObjectOffset(FI) == (MFI->getObjectOffset(BFI) + Dist*Bytes);
   }
 
-  // Handle X+C
-  if (isBaseWithConstantOffset(Loc) && Loc.getOperand(0) == BaseLoc &&
-      cast<ConstantSDNode>(Loc.getOperand(1))->getSExtValue() == Dist*Bytes)
+  // Handle X + C.
+  if (isBaseWithConstantOffset(Loc)) {
+    int LocOffset = cast<ConstantSDNode>(Loc.getOperand(1))->getSExtValue();
+    if (Loc.getOperand(0) == BaseLoc) {
+      // If the base location is a simple address with no offset itself, then
+      // the second load's first add operand should be the base address.
+      if (LocOffset == Dist * (signed)Bytes)
         return true;
-
+    } else if (isBaseWithConstantOffset(BaseLoc)) {
+      // The base location itself has an offset, so subtract that value from the
+      // second load's offset before comparing to distance * size.
+      int BOffset = cast<ConstantSDNode>(BaseLoc.getOperand(1))->getSExtValue();
+      if (Loc.getOperand(0) == BaseLoc.getOperand(0)) {
+        if ((LocOffset - BOffset) == Dist * (signed)Bytes)
+          return true;
+      }
+    }
+  }
   const GlobalValue *GV1 = nullptr;
   const GlobalValue *GV2 = nullptr;
   int64_t Offset1 = 0;
   int64_t Offset2 = 0;
   bool isGA1 = TLI->isGAPlusOffset(Loc.getNode(), GV1, Offset1);
   bool isGA2 = TLI->isGAPlusOffset(BaseLoc.getNode(), GV2, Offset2);
   if (isGA1 && isGA2 && GV1 == GV2)
     return Offset1 == (Offset2 + Dist*Bytes);

test/CodeGen/X86/chain_order.ll

 ; RUN: llc < %s -mcpu=corei7-avx -mtriple=x86_64-linux | FileCheck %s
 
-;CHECK-LABEL: cftx020:
-;CHECK: vmovsd  (%rdi), %xmm{{.*}}
-;CHECK: vmovsd  16(%rdi), %xmm{{.*}}
-;CHECK: vmovsd  24(%rdi), %xmm{{.*}}
-;CHECK: vmovhpd  8(%rdi), %xmm{{.*}}
-;CHECK: vmovupd %xmm{{.*}}, (%rdi)
-;CHECK: vmovupd %xmm{{.*}}, 16(%rdi)
-;CHECK: ret
+; CHECK-LABEL: cftx020:
+; CHECK: vmovsd  (%rdi), %xmm{{.*}}
+; CHECK-NEXT: vmovsd  16(%rdi), %xmm{{.*}}
+; CHECK-NEXT: vmovhpd  24(%rdi), %xmm{{.*}}
+; CHECK-NEXT: vmovhpd  8(%rdi), %xmm{{.*}}
+; CHECK: vmovupd %xmm{{.*}}, (%rdi)
+; CHECK-NEXT: vmovupd %xmm{{.*}}, 16(%rdi)
+; CHECK: ret
 
 ; A test from pifft (after SLP-vectorization) that fails when we drop the chain on newly merged loads.
 define void @cftx020(double* nocapture %a) {
 entry:
   %0 = load double* %a, align 8
   %arrayidx1 = getelementptr inbounds double* %a, i64 2
   %1 = load double* %arrayidx1, align 8
   %arrayidx2 = getelementptr inbounds double* %a, i64 1
   %2 = load double* %arrayidx2, align 8

test/CodeGen/X86/vec_loadsingles.ll

define <8 x float> @merge_8_floats(float* %ptr) {
   ret <8 x float> %vec14
 
 ; ALL-LABEL: merge_8_floats
 
 ; FAST32: vmovups
 ; FAST32-NEXT: retq
 
 ; SLOW32: vmovups
-; SLOW32: vinsertf128
+; SLOW32-NEXT: vinsertf128
 ; SLOW32-NEXT: retq
 }
 
 define <4 x double> @merge_4_doubles(double* %ptr) {
   %a = load double* %ptr, align 8
   %vec = insertelement <4 x double> undef, double %a, i32 0
   %idx1 = getelementptr inbounds double* %ptr, i64 1
   %b = load double* %idx1, align 8
   %vec2 = insertelement <4 x double> %vec, double %b, i32 1
   %idx3 = getelementptr inbounds double* %ptr, i64 2
   %c = load double* %idx3, align 8
   %vec4 = insertelement <4 x double> %vec2, double %c, i32 2
   %idx5 = getelementptr inbounds double* %ptr, i64 3
   %d = load double* %idx5, align 8
   %vec6 = insertelement <4 x double> %vec4, double %d, i32 3
   ret <4 x double> %vec6
 
 ; ALL-LABEL: merge_4_doubles
 ; FAST32: vmovups
 ; FAST32-NEXT: retq
 
 ; SLOW32: vmovups
-; SLOW32: vinsertf128
+; SLOW32-NEXT: vinsertf128
+; SLOW32-NEXT: retq
+}
+
+; PR21771 ( http://llvm.org/bugs/show_bug.cgi?id=21771 ) 
+; Recognize and combine consecutive loads even when the
+; first of the combined loads is offset from the base address.
+define <4 x double> @merge_4_doubles_offset(double* %ptr) {
+  %arrayidx4 = getelementptr inbounds double* %ptr, i64 4
+  %arrayidx5 = getelementptr inbounds double* %ptr, i64 5
+  %arrayidx6 = getelementptr inbounds double* %ptr, i64 6
+  %arrayidx7 = getelementptr inbounds double* %ptr, i64 7
+  %e = load double* %arrayidx4, align 8
+  %f = load double* %arrayidx5, align 8
+  %g = load double* %arrayidx6, align 8
+  %h = load double* %arrayidx7, align 8
+  %vecinit4 = insertelement <4 x double> undef, double %e, i32 0
+  %vecinit5 = insertelement <4 x double> %vecinit4, double %f, i32 1
+  %vecinit6 = insertelement <4 x double> %vecinit5, double %g, i32 2
+  %vecinit7 = insertelement <4 x double> %vecinit6, double %h, i32 3
+  ret <4 x double> %vecinit7
+
+; ALL-LABEL: merge_4_doubles_offset
+; FAST32: vmovups
+; FAST32-NEXT: retq
+
+; SLOW32: vmovups
+; SLOW32-NEXT: vinsertf128
 ; SLOW32-NEXT: retq
 }