[x86] allow 64-bit extracted vector element integer stores on a 32-bit system

[spatel]

Summary

With SSE2, we can generate a 'movq' or other 64-bit store op on a 32-bit system even though 64-bit integers are not legal types.

So instead of producing this:

pshufd	$229, %xmm0, %xmm1      ## xmm1 = xmm0[1,1,2,3]
movd	%xmm0, (%eax)
movd	%xmm1, 4(%eax)

We can do:

movq %xmm0, (%eax)

This is a fix for the problem noted in D7296.

[ab]

lib/Target/X86/X86ISelLowering.cpp

22941–22942 ↗

(On Diff #24077)

Isn't v2f64 too specific? What if you have a 256bit vector? (I haven't seen any related check in the function)

[spatel]

lib/Target/X86/X86ISelLowering.cpp

22941–22942 ↗

(On Diff #24077)

Yes, you're absolutely right. This won't work as-is for an AVX-capable target in 32-bit mode. Nice catch!

[spatel]

Patch updated:

1. Fixed to handle more than just 128-bit vectors.
2. Added test case to make sure a 256-bit AVX vector is processed; this test case crashed with the previous version of the patch.

[ab]

A couple nits, but LGTM, thanks!

lib/Target/X86/X86ISelLowering.cpp

22939–22944 ↗

(On Diff #24092)

Very subjective, but I feel like there's a couple variables too many. You decide.

22946–22950 ↗

(On Diff #24092)

Reflow formatting?

test/CodeGen/X86/i64-mem-copy.ll

27–28 ↗

(On Diff #24092)

Perhaps also test a lane index != 0?

[spatel]

Thanks, Ahmed - r235460.
I changed the last test case to use a non-zero index and fixed the formatting.
Re: local vars - if I have my choice, I add as many locals as possible to make for easier debugging, so I left that part as-is. :)

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

SDValue LoSt = DAG.getStore(NewChain, StDL, LoLd, LoAddr,
                                 St->getAlignment());
     SDValue HiSt = DAG.getStore(NewChain, StDL, HiLd, HiAddr,
                                 St->getPointerInfo().getWithOffset(4),
                                 St->isVolatile(),
                                 St->isNonTemporal(),
                                 MinAlign(St->getAlignment(), 4));
     return DAG.getNode(ISD::TokenFactor, StDL, MVT::Other, LoSt, HiSt);
   }
+
+  // This is similar to the above case, but here we handle a scalar 64-bit
+  // integer store that is extracted from a vector on a 32-bit target.
+  // If we have SSE2, then we can treat it like a floating-point double
+  // to get past legalization. The execution dependencies fixup pass will
+  // choose the optimal machine instruction for the store if this really is
+  // an integer or v2f32 rather than an f64.
+  if (VT == MVT::i64 && F64IsLegal && !Subtarget->is64Bit() &&
+      St->getOperand(1).getOpcode() == ISD::EXTRACT_VECTOR_ELT) {
+    SDValue OldExtract = St->getOperand(1);
+    SDValue ExtOp0 = OldExtract.getOperand(0);
+    unsigned VecSize = ExtOp0.getValueSizeInBits();
+    MVT VecVT = MVT::getVectorVT(MVT::f64, VecSize / 64);
+    SDValue BitCast = DAG.getNode(ISD::BITCAST, dl, VecVT, ExtOp0);
+    SDValue NewExtract = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl, MVT::f64,
+                                     BitCast, OldExtract.getOperand(1));
+    return DAG.getStore(St->getChain(), dl, NewExtract, St->getBasePtr(),
+                        St->getPointerInfo(), St->isVolatile(),
+                        St->isNonTemporal(), St->getAlignment());
+  }
+
   return SDValue();
 }
 
 /// Return 'true' if this vector operation is "horizontal"
 /// and return the operands for the horizontal operation in LHS and RHS.  A
 /// horizontal operation performs the binary operation on successive elements
 /// of its first operand, then on successive elements of its second operand,
 /// returning the resulting values in a vector.  For example, if

llvm/trunk/test/CodeGen/X86/i64-mem-copy.ll

 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=sse2 | FileCheck %s --check-prefix=X64
 ; RUN: llc < %s -mtriple=i386-unknown-unknown -mattr=sse2 | FileCheck %s --check-prefix=X32
+; RUN: llc < %s -mtriple=i386-unknown-unknown -mattr=avx2 | FileCheck %s --check-prefix=X32AVX
 
 ; Use movq or movsd to load / store i64 values if sse2 is available.
 ; rdar://6659858
 
 define void @foo(i64* %x, i64* %y) {
 ; X64-LABEL: foo:
 ; X64:       # BB#0:
 ; X64-NEXT:    movq (%rsi), %rax
 ; X64-NEXT:    movq %rax, (%rdi)
 ; X64-NEXT:    retq
 ;
 ; X32-LABEL: foo:
 ; X32:       # BB#0:
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X32-NEXT:    movl {{[0-9]+}}(%esp), %ecx
 ; X32-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
 ; X32-NEXT:    movsd %xmm0, (%eax)
 ; X32-NEXT:    retl
-
   %tmp1 = load i64, i64* %y, align 8
   store i64 %tmp1, i64* %x, align 8
   ret void
 }
 
+; Verify that a 64-bit chunk extracted from a vector is stored with a movq
+; regardless of whether the system is 64-bit.
+
+define void @store_i64_from_vector(<8 x i16> %x, <8 x i16> %y, i64* %i) {
+; X64-LABEL: store_i64_from_vector:
+; X64:       # BB#0:
+; X64-NEXT:    paddw %xmm1, %xmm0
+; X64-NEXT:    movq %xmm0, (%rdi)
+; X64-NEXT:    retq
+;
+; X32-LABEL: store_i64_from_vector:
+; X32:       # BB#0:
+; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; X32-NEXT:    paddw %xmm1, %xmm0
+; X32-NEXT:    movq %xmm0, (%eax)
+; X32-NEXT:    retl
+  %z = add <8 x i16> %x, %y                          ; force execution domain
+  %bc = bitcast <8 x i16> %z to <2 x i64>
+  %vecext = extractelement <2 x i64> %bc, i32 0
+  store i64 %vecext, i64* %i, align 8
+  ret void
+}
+
+define void @store_i64_from_vector256(<16 x i16> %x, <16 x i16> %y, i64* %i) {
+; X32AVX-LABEL: store_i64_from_vector256:
+; X32AVX:       # BB#0:
+; X32AVX-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; X32AVX-NEXT:    vpaddw %ymm1, %ymm0, %ymm0
+; X32AVX-NEXT:    vextracti128 $1, %ymm0, %xmm0
+; X32AVX-NEXT:    vmovq %xmm0, (%eax)
+; X32AVX-NEXT:    vzeroupper
+; X32AVX-NEXT:    retl
+  %z = add <16 x i16> %x, %y                          ; force execution domain
+  %bc = bitcast <16 x i16> %z to <4 x i64>
+  %vecext = extractelement <4 x i64> %bc, i32 2
+  store i64 %vecext, i64* %i, align 8
+  ret void
+}
+