[PATCH] D94645: [AMDGPU] Fix llvm.amdgcn.init.exec and frame materialization

Ruiling, Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 21 04:19:32 PST 2021


ruiling added a comment.



> There is still an issue if the SGPR used to hold the input llvm.amdgcn.init.exec.from.input is spilt; however, this is not a new issue.
> From my testing llvm.amdgcn.init.exec.from.input actually only worked in the entry block previous to this change, so we could tighten its description even further.

I am not sure what the problem is. May be we can fix it later. But I don't want to restrict it can only be used in the entry block for now unless we later prove that is really hard to make it correct. We may possibly use it for the second part of the merged shader.



================
Comment at: llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp:689-714
+  MachineInstr *FirstMI = &*MBB->begin();
+  Register InputReg = MI.getOperand(0).getReg();
+  bool IsEntryBlock = &*MF.begin() == MBB;
+
+  if (IsEntryBlock) {
+    // Move COPY to beginning of block.
+    bool Found = false;
----------------
I think this piece of code can be simplified something like below:

```
        Register InputReg = MI.getOperand(0).getReg();
        MachineInstr *FirstMI = &*MBB->begin();
        if (InputReg.isVirtual()) {
          MachineInstr *DefInstr = MRI->getVRegDef(InputReg);
          assert(DefInstr && DefInstr->isCopy());
          if (DefInstr->getParent() == MBB && DefInstr != FirstMI) {
            // If the `InputReg` is defined in current block, we also need to
            // move that instruction to the beginning of the block.
            DefInstr->removeFromParent();
            MBB->insert(FirstMI, DefInstr);
            if (LIS)
              LIS->handleMove(*DefInstr);
          }
        }

```


================
Comment at: llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp:753
+  LIS->createAndComputeVirtRegInterval(InputReg);
+  LIS->createAndComputeVirtRegInterval(CountReg);
+}
----------------
Maybe also ` LIS->removeAllRegUnitsForPhysReg(AMDGPU::EXEC);`? This also applies to SI_INIT_EXEC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94645



More information about the llvm-commits mailing list