[PATCH] D136169: [AMDGPU] Avoid SCC clobbering before S_CSELECT_B32

[Alexander via Phabricator via llvm-commits]

alex-t added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:2231-2232
+          SCCCopy = RS->scavengeRegister(getBoolRC(), MI, 0, !UseSGPR);
+          if (!SCCCopy)
+            report_fatal_error("Cannot scavenge register in FI elimination!");
+          P = BuildMI(*MBB, MI, DL,
----------------
foad wrote:
> This still makes me nervous. For graphics we use LLVM as a JIT compiler, and we definitely have cases where SGPRs are spilled (so presumably there are no free SGPRs). What are we supposed to do if this scavenge fails?
What we were supposed to do if failed the scavenge at line 2198 and line 2243?
In fact, I have just followed the error handling strategy implemented for the same condition in this file earlier. I assumed that if it worked so far it should be okay further.

In general, no strategy exists that ensures we never run out of registers.

We can try to use vector addition with an SGPR and constant but it is possible for a few targets only. Otherwise, we'd need to scavenge VGPR but we already failed to scavenge SGPR which means that we were unable to spill one and probably has no VGPRs available.

The trick with using FrameRegister as a temporary room is already used by frame lowering itself, also SCC copy should be alive from the point before the S_ADD insertion and last to the point right after it.

Moving the insertion point before the SCC definition is restricted by the result register placement as follows:

```
(1) def R
(2) def SCC
(3) use R
<==== here we're about to insert R = S_ADD_I32 FR, Off
(4) use SCC
```
We cannot move the insertion point over another use of R.
In the general case, this is unsolvable.

My point is that:
1. "Failed to scavenge" recovery strategy is a complex problem that should be addressed separately.
2. There is no general solution as one always can invent the input on which the compiler runs out of registers.

Please correct me if I am wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136169