DAG: Don't pass 0 alignment value to allowsMisalignedMemoryAccesses

[arsenm]

Summary

I think not unconditionally passing getDstAlign is broken, but leave
that for another change.

f85d3408e6e3e7710032abfaabdecd02ee0b5ced / D73964 started conditionally passing the alignment, which I don't understand. 0 is not a valid alignment value, so this should not use it.

[rampitec]

LGTM, but probably allowsMisalignedMemoryAccesses() should just take Align argument instead.

[courbet]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

252

There's actually an overload that takes an Align. The old version is scheduled to go away. So please use the overload:

allowsMisalignedMemoryAccesses( VT, DstAS, Op.isFixedDstAlign() ? Op.getDstAlign() : Align(1))

253

Actually the overlapping load/store is not aligned as Op.getDstAlign() (consider the case of a 7-byte copy aligned to 4, then the overlapping 4 byte load-store would have align 1), so I think this should actually always be 1.

[arsenm]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

252

There isn't actually. The LLT version uses Align, but the SelectionDAG version does not

[arsenm]

c7191e318544ea0c30b5678c1f2e79e0c5be9628

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

while (VTSize > Size) {
         } while (!isSafeMemOpType(NewVT.getSimpleVT()));
       }
       NewVTSize = NewVT.getSizeInBits() / 8;
 
       // If the new VT cannot cover all of the remaining bits, then consider
       // issuing a (or a pair of) unaligned and overlapping load / store.
       bool Fast;
       if (NumMemOps && Op.allowOverlap() && NewVTSize < Size &&
           allowsMisalignedMemoryAccesses(
-              VT, DstAS, Op.isFixedDstAlign() ? Op.getDstAlign().value() : 0,
+              VT, DstAS, Op.isFixedDstAlign() ? Op.getDstAlign().value() : 1,
               MachineMemOperand::MONone, &Fast) &&
           Fast)
         VTSize = Size;
       else {
         VT = NewVT;
         VTSize = NewVTSize;
       }
     }

llvm/test/CodeGen/AMDGPU/memcpy-fixed-align.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 < %s | FileCheck %s
+
+; Make sure there's no assertion from passing a 0 alignment value
+define void @memcpy_fixed_align(i8 addrspace(5)*  %dst, i8 addrspace(1)* %src) {
+; CHECK-LABEL: memcpy_fixed_align:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    global_load_dword v0, v[1:2], off offset:36
+; CHECK-NEXT:    s_waitcnt vmcnt(0)
+; CHECK-NEXT:    buffer_store_dword v0, off, s[0:3], s32 offset:36
+; CHECK-NEXT:    global_load_dword v0, v[1:2], off offset:32
+; CHECK-NEXT:    s_waitcnt vmcnt(0)
+; CHECK-NEXT:    buffer_store_dword v0, off, s[0:3], s32 offset:32
+; CHECK-NEXT:    global_load_dwordx4 v[3:6], v[1:2], off offset:16
+; CHECK-NEXT:    s_waitcnt vmcnt(0)
+; CHECK-NEXT:    buffer_store_dword v6, off, s[0:3], s32 offset:28
+; CHECK-NEXT:    buffer_store_dword v5, off, s[0:3], s32 offset:24
+; CHECK-NEXT:    buffer_store_dword v4, off, s[0:3], s32 offset:20
+; CHECK-NEXT:    buffer_store_dword v3, off, s[0:3], s32 offset:16
+; CHECK-NEXT:    global_load_dwordx4 v[0:3], v[1:2], off
+; CHECK-NEXT:    s_waitcnt vmcnt(0)
+; CHECK-NEXT:    buffer_store_dword v3, off, s[0:3], s32 offset:12
+; CHECK-NEXT:    buffer_store_dword v2, off, s[0:3], s32 offset:8
+; CHECK-NEXT:    buffer_store_dword v1, off, s[0:3], s32 offset:4
+; CHECK-NEXT:    buffer_store_dword v0, off, s[0:3], s32
+; CHECK-NEXT:    s_waitcnt vmcnt(0)
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %alloca = alloca [40 x i8], addrspace(5)
+  %cast = bitcast [40 x i8] addrspace(5)* %alloca to i8 addrspace(5)*
+  call void @llvm.memcpy.p5i8.p1i8.i64(i8 addrspace(5)* align 4 dereferenceable(40) %cast, i8 addrspace(1)* align 4 dereferenceable(40) %src, i64 40, i1 false)
+  ret void
+}
+
+declare void @llvm.memcpy.p5i8.p1i8.i64(i8 addrspace(5)* noalias nocapture writeonly, i8 addrspace(1)* noalias nocapture readonly, i64, i1 immarg) #0
+
+attributes #0 = { argmemonly nounwind willreturn }