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

[Carl Ritson via Phabricator via llvm-commits]

critson marked 2 inline comments as done.
critson added inline comments.


================
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;
----------------
ruiling wrote:
> 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);
>           }
>         }
> 
> ```
Sure.  Although I had to update the code to handle the case the definition is the first instruction.


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94645