[PATCH] D140599: AMDGPU: Promote array alloca if used by memmove/memcpy

[Matt Arsenault via Phabricator via llvm-commits]

arsenm accepted this revision.
arsenm added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:593-595
+          case Intrinsic::memcpy:
+          case Intrinsic::memcpy_inline:
+          case Intrinsic::memmove: {
----------------
ruiling wrote:
> arsenm wrote:
> > probably should dyn_cast to MemIntrinsicInst here to defend agains these becoming out of sync
> Agree, I have changed to cast to catch the possible broken case.
This still has the switch over specific IDs instead of the matching cast


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:611
+            Type *VecPtrTy = VectorTy->getPointerTo(Alloca->getAddressSpace());
+            Value *BitCast = Builder.CreateBitCast(Alloca, VecPtrTy);
+            Value *VecValue = Builder.CreateLoad(VectorTy, BitCast);
----------------
foad wrote:
> ruiling wrote:
> > arsenm wrote:
> > > No reason to care about type pointers anymore
> > Our graphics compiler has not fully switched to opaque pointers yet. @foad am I right?
> Yes.
Well, that needs to be fixed soon since code is going to start getting ripped out very shortly


================
Comment at: llvm/test/CodeGen/AMDGPU/promote-alloca-array-aggregate.ll:307
+}
+
+declare void @llvm.memcpy.p5i8.p5i8.i32(ptr addrspace(5) nocapture writeonly, ptr addrspace(5) nocapture readonly, i32, i1 immarg)
----------------
Another edge case to maybe test is the identity copy of the full alloca to itself


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140599/new/

https://reviews.llvm.org/D140599