[PATCH] D146225: [AMDGPU] Handle memset users in PromoteAlloca

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:397
+  uint64_t ByteVal;
+  if (!match(I, m_Intrinsic<Intrinsic::memset>(m_Value(Ptr),
+                                               m_ConstantInt(ByteVal),
----------------
You can hide the intrinsic checks by dyn_casting to MemSetInst


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:405-407
+  // For now we only care about memsets that affect the whole type (start at
+  // index 0 and fill the whole alloca).
+  return Ptr == AI && Len == AllocaSize;
----------------
I don't see why you need this restriction but if you want to lift it in a separate patch that's fine


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:561-564
+    if (isSupportedMemset(Inst, Alloca, DL.getTypeSizeInBits(AllocaTy) / 8)) {
+      WorkList.push_back(Inst);
+      continue;
+    }
----------------
I'd expect this to be up with the MemTransferInst handling, dyn_casting to MemSetInst


================
Comment at: llvm/test/CodeGen/AMDGPU/promote-alloca-memset.ll:31
+; CHECK-LABEL: @memset_volatile_nopromote(
+; CHECK-NOT: <4 x i64>
+define amdgpu_kernel void @memset_volatile_nopromote(i64 %val) {
----------------
Not checks are fragile,, might as well generate the checks here


================
Comment at: llvm/test/CodeGen/AMDGPU/promote-alloca-memset.ll:61
+
+declare void @llvm.memset.p5.double(ptr addrspace(5) nocapture writeonly, i8, i64, i1 immarg)
+declare void @llvm.memset.p5.i64(ptr addrspace(5) nocapture writeonly, i8, i64, i1 immarg)
----------------
.double?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146225