DAGCombiner: Relax alignment restriction when changing store type

[arsenm]

Summary

If the target allows the alignment, this should be OK. This is the partner change to D17306.

This currently breaks 1 ARM test (test/CodeGen/ARM/vector-store.ll), which looks to me like allowsMisalignedMemoryAccesses is incorrectly reporting fast.

The changes all look like this:

define void @store_v8i8_update(<8 x i8>** %ptr, <8 x i8> %val) {
  ;CHECK-LABEL: store_v8i8_update:
  ;CHECK: vst1.8 {{{d[0-9]+}}}, [{{r[0-9]+}}]!
  %A = load <8 x i8>*, <8 x i8>** %ptr
  store  <8 x i8> %val, <8 x i8>* %A, align 1
  %inc = getelementptr <8 x i8>, <8 x i8>* %A, i38 1
  store <8 x i8>* %inc, <8 x i8>** %ptr
  ret void
}

DAG at replacement time:

SelectionDAG has 17 nodes:
  t0: ch = EntryToken
  t2: i32,ch = CopyFromReg t0, Register:i32 %vreg0
  t9: i32 = Constant<0>
  t11: i32,ch = load<LD4[%ptr]> t0, t2, undef:i32
            t4: i32,ch = CopyFromReg t0, Register:i32 %vreg1
            t6: i32,ch = CopyFromReg t0, Register:i32 %vreg2
          t7: f64 = ARMISD::VMOVDRR t4, t6
        t8: v8i8 = bitcast t7
      t12: ch = store<ST8[%A](align=1)> t11:1, t8, t11, undef:i32
      t14: i32 = add t11, Constant:i32<8>
    t15: ch = store<ST4[%ptr]> t12, t14, t2, undef:i32
  t16: ch = ARMISD::RET_FLAG t15

Before:

_store_v8i8_update:
@ BB#0:
  vmov	d16, r1, r2
  ldr	r1, [r0]
  vst1.8	{d16}, [r1]!
  str	r1, [r0]
  bx	lr

After:

_store_v8i8_update:
@ BB#0:
	ldr	r3, [r0]
	str	r1, [r3]
	str	r2, [r3, #4]
	add.w	r1, r3, #8
	str	r1, [r0]
	bx	lr

It also breaks 2 other X86 tests, both of which appear to be i1-vector related:
X86/avx-vextractf128.ll, which replaces a

vxorps	%xmm0, %xmm0, %xmm0

with a

vxorps	%ymm0, %ymm0, %ymm0

X86/avx512-mask-op.ll, where an extra move is introduced

[arsenm]

Fix missing most of the patch

[arsenm]

ping

[• tstellarAMD]

Adding ARM reviewers as suggested by Owen.

[t.p.northover]

I think the ARM change is probably fine. There's real ARM code designed to transform a VMOVDRR/VSTR into two STRs (for legitimate, if debatable reasons: vmov was really slow on some cores), it just doesn't happen to trigger if the type is a vector.

Tim.

[arsenm]

Include ARM/x86 test update

[• tstellarAMD]

LGTM.

[arsenm]

r267217

include/llvm/Target/TargetLowering.h

public:
   /// efficiently, casting the load to a smaller vector of larger types and
   /// loading is more efficient, however, this can be undone by optimizations in
   /// dag combiner.
   virtual bool isLoadBitCastBeneficial(EVT /* Load */,
                                        EVT /* Bitcast */) const {
     return true;
   }
 
+  /// isStoreBitCastBeneficial() - Mirror of isLoadBitCastBeneficial(). Return
+  /// true if the following transform is beneficial.
+  ///
+  /// (store (y (conv x)), y*)) -> (store x, (x*))
+  virtual bool isStoreBitCastBeneficial(EVT StoreVT, EVT BitcastVT) const {
+    // Default to the same logic as stores.
+    return isLoadBitCastBeneficial(StoreVT, BitcastVT);
+  }
+
   /// Return true if it is expected to be cheaper to do a store of a non-zero
   /// vector constant with the given size and type for the address space than to
   /// store the individual scalar element constants.
   virtual bool storeOfVectorConstantIsCheap(EVT MemVT,
                                             unsigned NumElem,
                                             unsigned AddrSpace) const {
     return false;
   }

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

SDValue DAGCombiner::visitSTORE(SDNode *N) {
   SDValue Chain = ST->getChain();
   SDValue Value = ST->getValue();
   SDValue Ptr   = ST->getBasePtr();
 
   // If this is a store of a bit convert, store the input value if the
   // resultant store does not need a higher alignment than the original.
   if (Value.getOpcode() == ISD::BITCAST && !ST->isTruncatingStore() &&
       ST->isUnindexed()) {
-    unsigned OrigAlign = ST->getAlignment();
     EVT SVT = Value.getOperand(0).getValueType();
-    unsigned Align = DAG.getDataLayout().getABITypeAlignment(
-        SVT.getTypeForEVT(*DAG.getContext()));
-    if (Align <= OrigAlign &&
-        ((!LegalOperations && !ST->isVolatile()) ||
-         TLI.isOperationLegalOrCustom(ISD::STORE, SVT)))
+    if (((!LegalOperations && !ST->isVolatile()) ||
+         TLI.isOperationLegalOrCustom(ISD::STORE, SVT)) &&
+        TLI.isStoreBitCastBeneficial(Value.getValueType(), SVT)) {
+      unsigned OrigAlign = ST->getAlignment();
+      bool Fast = false;
+      if (TLI.allowsMemoryAccess(*DAG.getContext(), DAG.getDataLayout(), SVT,
+                                 ST->getAddressSpace(), OrigAlign, &Fast) &&
+          Fast) {
         return DAG.getStore(Chain, SDLoc(N), Value.getOperand(0),
                             Ptr, ST->getPointerInfo(), ST->isVolatile(),
                             ST->isNonTemporal(), OrigAlign,
                             ST->getAAInfo());
       }
+    }
+  }
 
   // Turn 'store undef, Ptr' -> nothing.
   if (Value.isUndef() && ST->isUnindexed())
     return Chain;
 
   // Try to infer better alignment information than the store already has.
   if (OptLevel != CodeGenOpt::None && ST->isUnindexed()) {
     if (unsigned Align = DAG.InferPtrAlignment(Ptr)) {

test/CodeGen/AMDGPU/reduce-store-width-alignment.ll

+; RUN: llc -march=amdgcn -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
+
+; GCN-LABEL: {{^}}store_v2i32_as_v4i16_align_4:
+; GCN: s_load_dwordx2
+; GCN: ds_write2_b32 v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}} offset1:1{{$}}
+define void @store_v2i32_as_v4i16_align_4(<4 x i16> addrspace(3)* align 4 %out, <2 x i32> %x) #0 {
+  %x.bc = bitcast <2 x i32> %x to <4 x i16>
+  store <4 x i16> %x.bc, <4 x i16> addrspace(3)* %out, align 4
+  ret void
+}
+
+; GCN-LABEL: {{^}}store_v4i32_as_v8i16_align_4:
+; GCN: s_load_dwordx4
+; GCN-DAG: ds_write2_b32 v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}} offset0:2 offset1:3
+; GCN-DAG: ds_write2_b32 v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}} offset1:1{{$}}
+define void @store_v4i32_as_v8i16_align_4(<8 x i16> addrspace(3)* align 4 %out, <4 x i32> %x) #0 {
+  %x.bc = bitcast <4 x i32> %x to <8 x i16>
+  store <8 x i16> %x.bc, <8 x i16> addrspace(3)* %out, align 4
+  ret void
+}
+
+; GCN-LABEL: {{^}}store_v2i32_as_i64_align_4:
+; GCN: s_load_dwordx2
+; GCN: ds_write2_b32 v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}} offset1:1{{$}}
+define void @store_v2i32_as_i64_align_4(<4 x i16> addrspace(3)* align 4 %out, <2 x i32> %x) #0 {
+  %x.bc = bitcast <2 x i32> %x to <4 x i16>
+  store <4 x i16> %x.bc, <4 x i16> addrspace(3)* %out, align 4
+  ret void
+}
+
+; GCN-LABEL: {{^}}store_v4i32_as_v2i64_align_4:
+; GCN: s_load_dwordx4
+; GCN-DAG: ds_write2_b32 v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}} offset0:2 offset1:3
+; GCN-DAG: ds_write2_b32 v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}} offset1:1{{$}}
+define void @store_v4i32_as_v2i64_align_4(<2 x i64> addrspace(3)* align 4 %out, <4 x i32> %x) #0 {
+  %x.bc = bitcast <4 x i32> %x to <2 x i64>
+  store <2 x i64> %x.bc, <2 x i64> addrspace(3)* %out, align 4
+  ret void
+}
+
+; GCN-LABEL: {{^}}store_v4i16_as_v2i32_align_4:
+; GCN: buffer_load_ushort
+; GCN: buffer_load_ushort
+; GCN: buffer_load_ushort
+; GCN: buffer_load_ushort
+; GCN: ds_write2_b32 v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}} offset1:1{{$}}
+define void @store_v4i16_as_v2i32_align_4(<2 x i32> addrspace(3)* align 4 %out, <4 x i16> %x) #0 {
+  %x.bc = bitcast <4 x i16> %x to <2 x i32>
+  store <2 x i32> %x.bc, <2 x i32> addrspace(3)* %out, align 4
+  ret void
+}
+
+attributes #0 = { nounwind }

test/CodeGen/ARM/vector-store.ll

 ; RUN: llc < %s | FileCheck %s
 
 target datalayout = "e-m:o-p:32:32-i1:8:32-i8:8:32-i16:16:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
 target triple = "thumbv7s-apple-ios8.0.0"
 
 define void @store_v8i8(<8 x i8>** %ptr, <8 x i8> %val) {
 ;CHECK-LABEL: store_v8i8:
 ;CHECK: str r1, [r0]
   %A = load <8 x i8>*, <8 x i8>** %ptr
   store  <8 x i8> %val, <8 x i8>* %A, align 1
   ret void
 }
 
 define void @store_v8i8_update(<8 x i8>** %ptr, <8 x i8> %val) {
-;CHECK-LABEL: store_v8i8_update:
-;CHECK: vst1.8 {{{d[0-9]+}}}, [{{r[0-9]+}}]!
+; CHECK-LABEL: store_v8i8_update:
+; CHECK: str r{{[0-9]+}}, [{{r[0-9]+}}]
+; CHECK: str r{{[0-9]+}}, [{{r[0-9]+}}, #4]
+; CHECK: add.w r{{[0-9]+}}, r{{[0-9]+}}, #8
   %A = load <8 x i8>*, <8 x i8>** %ptr
   store  <8 x i8> %val, <8 x i8>* %A, align 1
   %inc = getelementptr <8 x i8>, <8 x i8>* %A, i38 1
-        store <8 x i8>* %inc, <8 x i8>** %ptr
   ret void
 }
 
 define void @store_v4i16(<4 x i16>** %ptr, <4 x i16> %val) {
 ;CHECK-LABEL: store_v4i16:
 ;CHECK: str r1, [r0]
   %A = load <4 x i16>*, <4 x i16>** %ptr
   store  <4 x i16> %val, <4 x i16>* %A, align 1
   ret void
 }
 
 define void @store_v4i16_update(<4 x i16>** %ptr, <4 x i16> %val) {
-;CHECK-LABEL: store_v4i16_update:
-;CHECK: vst1.8 {{{d[0-9]+}}}, [{{r[0-9]+}}]!
+; CHECK-LABEL: store_v4i16_update:
+; CHECK: str r{{[0-9]+}}, [{{r[0-9]+}}]
+; CHECK: str r{{[0-9]+}}, [{{r[0-9]+}}, #4]
+; CHECK: add.w r{{[0-9]+}}, r{{[0-9]+}}, #8
   %A = load <4 x i16>*, <4 x i16>** %ptr
   store  <4 x i16> %val, <4 x i16>* %A, align 1
   %inc = getelementptr <4 x i16>, <4 x i16>* %A, i34 1
         store <4 x i16>* %inc, <4 x i16>** %ptr
   ret void
 }
 
 define void @store_v2i32(<2 x i32>** %ptr, <2 x i32> %val) {
 ;CHECK-LABEL: store_v2i32:
 ;CHECK: str r1, [r0]
   %A = load <2 x i32>*, <2 x i32>** %ptr
   store  <2 x i32> %val, <2 x i32>* %A, align 1
   ret void
 }
 
 define void @store_v2i32_update(<2 x i32>** %ptr, <2 x i32> %val) {
-;CHECK-LABEL: store_v2i32_update:
-;CHECK: vst1.8 {{{d[0-9]+}}}, [{{r[0-9]+}}]!
+; CHECK-LABEL: store_v2i32_update:
+; CHECK: str r{{[0-9]+}}, [{{r[0-9]+}}]
+; CHECK: str r{{[0-9]+}}, [{{r[0-9]+}}, #4]
+; CHECK: add.w r{{[0-9]+}}, r{{[0-9]+}}, #8
+
   %A = load <2 x i32>*, <2 x i32>** %ptr
   store  <2 x i32> %val, <2 x i32>* %A, align 1
   %inc = getelementptr <2 x i32>, <2 x i32>* %A, i32 1
         store <2 x i32>* %inc, <2 x i32>** %ptr
   ret void
 }
 
 define void @store_v2f32(<2 x float>** %ptr, <2 x float> %val) {
 ;CHECK-LABEL: store_v2f32:
 ;CHECK: str r1, [r0]
   %A = load <2 x float>*, <2 x float>** %ptr
   store  <2 x float> %val, <2 x float>* %A, align 1
   ret void
 }
 
 define void @store_v2f32_update(<2 x float>** %ptr, <2 x float> %val) {
-;CHECK-LABEL: store_v2f32_update:
-;CHECK: vst1.8 {{{d[0-9]+}}}, [{{r[0-9]+}}]!
-  %A = load <2 x float>*, <2 x float>** %ptr
-  store  <2 x float> %val, <2 x float>* %A, align 1
-  %inc = getelementptr <2 x float>, <2 x float>* %A, i32 1
-        store <2 x float>* %inc, <2 x float>** %ptr
-  ret void
+; CHECK-LABEL: store_v2f32_update:
+; CHECK: str r{{[0-9]+}}, [{{r[0-9]+}}]
+; CHECK: str r{{[0-9]+}}, [{{r[0-9]+}}, #4]
+; CHECK: add.w r{{[0-9]+}}, r{{[0-9]+}}, #8
+    %A = load <2 x float>*, <2 x float>** %ptr
+    store  <2 x float> %val, <2 x float>* %A, align 1
+    %inc = getelementptr <2 x float>, <2 x float>* %A, i32 1
+    ret void
 }
 
 define void @store_v1i64(<1 x i64>** %ptr, <1 x i64> %val) {
 ;CHECK-LABEL: store_v1i64:
 ;CHECK: str r1, [r0]
   %A = load <1 x i64>*, <1 x i64>** %ptr
   store  <1 x i64> %val, <1 x i64>* %A, align 1
   ret void
 }
 
 define void @store_v1i64_update(<1 x i64>** %ptr, <1 x i64> %val) {
-;CHECK-LABEL: store_v1i64_update:
-;CHECK: vst1.8 {{{d[0-9]+}}}, [{{r[0-9]+}}]!
+; CHECK-LABEL: store_v1i64_update:
+; CHECK: str r{{[0-9]+}}, [{{r[0-9]+}}]
+; CHECK: str r{{[0-9]+}}, [{{r[0-9]+}}, #4]
+; CHECK: add.w r{{[0-9]+}}, r{{[0-9]+}}, #8
+
   %A = load <1 x i64>*, <1 x i64>** %ptr
   store  <1 x i64> %val, <1 x i64>* %A, align 1
   %inc = getelementptr <1 x i64>, <1 x i64>* %A, i31 1
-        store <1 x i64>* %inc, <1 x i64>** %ptr
   ret void
 }
 
 define void @store_v16i8(<16 x i8>** %ptr, <16 x i8> %val) {
 ;CHECK-LABEL: store_v16i8:
 ;CHECK: vst1.8 {{{d[0-9]+, d[0-9]+}}}, [{{r[0-9]+}}]
   %A = load <16 x i8>*, <16 x i8>** %ptr
   store  <16 x i8> %val, <16 x i8>* %A, align 1

test/CodeGen/X86/avx-vextractf128.ll

entry:
   store <2 x i64> %2, <2 x i64>* %addr, align 1
   ret void
 }
 
 ; PR15462
 define void @t9(i64* %p) {
 ; CHECK-LABEL: t9:
 ; CHECK:       ## BB#0:
-; CHECK-NEXT:    vxorps %xmm0, %xmm0, %xmm0
+; CHECK-NEXT:    vxorps %ymm0, %ymm0, %ymm0
 ; CHECK-NEXT:    vmovups %ymm0, (%rdi)
 ; CHECK-NEXT:    vzeroupper
 ; CHECK-NEXT:    retq
  store i64 0, i64* %p
  %q = getelementptr i64, i64* %p, i64 1
  store i64 0, i64* %q
  %r = getelementptr i64, i64* %p, i64 2
  store i64 0, i64* %r
  %s = getelementptr i64, i64* %p, i64 3
  store i64 0, i64* %s
  ret void
 }
 
 declare <2 x double> @llvm.x86.avx.vextractf128.pd.256(<4 x double>, i8) nounwind readnone
 declare <4 x float> @llvm.x86.avx.vextractf128.ps.256(<8 x float>, i8) nounwind readnone
 declare <4 x i32> @llvm.x86.avx.vextractf128.si.256(<8 x i32>, i8) nounwind readnone