[PATCH] D130784: [AMDGPU] Support LDS spilling

Piotr Sobczak via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 22 05:57:57 PDT 2022


piotr marked an inline comment as done.
piotr added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.h:824-830
+  void setWorkgroupInfoReg(Register Reg) {
+    assert(Reg != 0);
+    WorkgroupInfoReg = Reg;
+  }
+
+  Register getWorkgroupInfoReg() const { return WorkgroupInfoReg; }
+
----------------
sebastian-ne wrote:
> Maybe this should be below `getStackPtrOffsetReg`? Currently, it divides `setStackPtrOffsetReg` and `getStackPtrOffsetReg`.
> Maybe this should be below `getStackPtrOffsetReg`? Currently, it divides `setStackPtrOffsetReg` and `getStackPtrOffsetReg`.

Yes, good idea.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:1295
+            LdsSpillInfo.M0SaveRestoreReg)
+        .addReg(AMDGPU::M0);
+    if (LdsSpillInfo.M0InitVal == AMDGPU::NoRegister)
----------------
sebastian-ne wrote:
> I guess M0 could have a kill flag here?
> I guess M0 could have a kill flag here?

Yes, thanks.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:2195
+        if (ldsSpill) {
+          MFI->addToSpilledVGPRs(getNumSubRegsForSpillOp(MI->getOpcode()));
+          MI->eraseFromParent();
----------------
sebastian-ne wrote:
> The non-lds restore code does not call `addToSpilledVGPRs`. Is this intentional here?
> The non-lds restore code does not call `addToSpilledVGPRs`. Is this intentional here?

Good catch - copy & pasta error on my part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130784



More information about the llvm-commits mailing list