[PATCH] D124884: [AMDGPU] Add intrinsic llvm.amdgcn.raw.buffer.load.lds

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:4277-4279
+  B.buildInstr(AMDGPU::COPY)
+    .addDef(AMDGPU::M0)
+    .addUse(M0Val);
----------------
piotr wrote:
> What is preventing this from clobbering M0? There are intrinsics like int_amdgcn_interp* that have a dependency on M0. Shouldn't the code save the existing M0 and restore it after the load?
m0 isn't treated as a preserved value. Each user is supposed to initialize m0 itself


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:8260
+
+    return DAG.getMemIntrinsicNode(AMDGPUISD::BUFFER_LOAD, DL,
+                                   M->getVTList(), Ops, M->getMemoryVT(), MMO);
----------------
I don't see how / where this preserves the LDS bit


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:389
+    if (DataOpIdx == -1) { // LDS DMA
+      Width = (*LdSt.memoperands_begin())->getSize();
+      return true;
----------------
If you're going to rely on the memory operand, the verifier needs to start enforcing these have one memory operand (well, 2 actually with the same sizes)


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

https://reviews.llvm.org/D124884