[PATCH] D82737: [AMDGPU] Insert PS early exit at end of control flow

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle accepted this revision.
nhaehnle added a comment.
This revision is now accepted and ready to land.

Okay, thanks, this all makes sense to me. I do have some suggestions for tightening checks up a little, but either way LGTM.



================
Comment at: llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp:222
+      assert(std::next(UseMI) == MRI->use_instr_nodbg_end());
+      MachineOperand &NextExec = UseMI->getOperand(0);
+      Register NextExecReg = NextExec.getReg();
----------------
Would it make sense to add an assertion that `UseMI->getOpcode() == AMDGPU::SI_ELSE`?


================
Comment at: llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp:228-232
+    if (hasKill(MI.getParent(), UseMI->getParent(), TII)) {
+      if (UseMI->getOpcode() == AMDGPU::SI_END_CF)
+        NeedsKillCleanup.insert(&*UseMI);
+      SimpleIf = false;
+    }
----------------
When can the `NextExec.isDead()` case occur? Unreachable code or the like?

In any case, could you move this around a bit as:
```
  if (NextExec.isDead()) {
    assert(!SimpleIf);
    break;
  }
...
if (UseMI->getOpcode() == AMDGPU::SI_END_CF) {
  if (hasKill(...)) {
    NeedsKillCleanup.insert(&*UseMI);
    SimpleIf = false;
  }
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82737