[PATCH] D133593: [AMDGPU] Always select s_cselect_b32 for uniform 'select' SDNode

[Alexander via Phabricator via llvm-commits]

alex-t added a comment.

In D133593#3783316 <https://reviews.llvm.org/D133593#3783316>, @foad wrote:

>> This patch contains changes necessary to carry physical condition register (SCC) dependencies through the SDNode scheduler. This allows avoiding inserting an SCC copy between each definition and use.
>
> Please explain more. Why doesn't the generic support for physical registers work already? Why do we need to "avoid inserting an SCC copy"?
>
> I was also working on this in D124450 <https://reviews.llvm.org/D124450> but I have not looked at that patch for a few months.

If we allow SCC to be copied (CopyCost() != -1) then any pair of the def-use will look like:

  s_cmp_***
  %1 = s_cselect_b64 -1, 0  <-- lowered CopyToReg  where srcreg == SCC
  s_and_b64 %1, exec          <-- lowered CopyFromReg where dstreg == SCC
  %res = s_cselect_b32 %trueval, %falseval  // not necessarily  s_cselect_b32 may be any SCC user

We could handle the trivial cases where the SCC copy is immediately copied back to SCC. Some more complex cases where these copies are interleaved by other instructions are more difficult to analyze. 
At the same time, if we keep the SCC copy cost high, we let the SDNode scheduler place the instructions in an optimal way according to the register carried dependencies.
Literally, we will insert a copy only in case there is no other way to meet the data dependency.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:119
   unsigned Reg = cast<RegisterSDNode>(User->getOperand(1))->getReg();
+  if (TLI.checkForPhysRegDependency(Def, User, Op, TRI, TII, PhysReg, Cost))
+    return;
----------------
rampitec wrote:
> Place it after the check for isVirtualRegister below?
It is deliberately here. In case we have a cross-BB SCC-carried dependency it is CopyToReg with the VReg as an operand 1.

```
CopyToReg chain, VregN, SCC def
```
and in successor BB:

```
CopyFromReg SCC, VregN
```
This is really SCC-carried dependency but it implemented via virtual register copy


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

https://reviews.llvm.org/D133593