[PATCH] D88777: [AMDGPU] Add SI_EARLY_TERMINATE_SCC0 for early terminating shader

[Carl Ritson via Phabricator via llvm-commits]

critson marked an inline comment as done.
critson added a comment.

In D88777#2486221 <https://reviews.llvm.org/D88777#2486221>, @foad wrote:

> Patch looks fine to me. Why is it useful to terminate on scc0 instead of scc1, or is it an arbitrary choice? Could you give a slightly more realistic example of how it would be used? Your tests all have:
>
>   S_CMP_EQ_U32 killed $sgpr0, 0, implicit-def $scc
>   SI_EARLY_TERMINATE_SCC0 implicit $scc, implicit $exec
>
> which will terminate early if sgpr0 is //not// 0. Is the idea that this would be used when we're just about to AND sgpr0 into exec?

The expectation is that this is used after updating a stored EXEC mask (not EXEC itself).
So SCC will be set from S_AND or S_ANDN2 in most cases.
SCC0 means mask is empty and program can be terminated.



================
Comment at: llvm/lib/Target/AMDGPU/SIInsertSkips.cpp:494
+    else
+      Instr->eraseFromParent();
+  }
----------------
foad wrote:
> Could erase here unconditionally, to avoid having an erase in earlyTerm as well.
Yes.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:328
+// Only valid in pixel shaders.
+def SI_EARLY_TERMINATE_SCC0 : SPseudoInstSI <(outs), (ins)> {
+  let Uses = [EXEC,SCC];
----------------
arsenm wrote:
> foad wrote:
> > Am I right in thinking that this is not a terminator, so it can appear in the middle of an MBB? Is that normal for this kind of pseudo?
> If this does any branching to any point in the program, this should be a terminator. If it behaves more like a trap/abort, it's OK if it's not a terminator
I think it is reasonable to consider this an abort. It always branches to terminate the program.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88777