[PATCH] D82370: [AMDGPU] Rework SCC copy

Tue Jun 23 10:10:32 PDT 2020

arsenm added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:674-675
+
+    // We only care about the low bit here.
+    if (AMDGPU::SReg_64RegClass.contains(SrcReg)) {
+      SrcReg = RI.getSubReg(SrcReg, AMDGPU::sub0);
----------------
piotr wrote:
> arsenm wrote:
> > I think you should never end up in this situation to begin with. Maybe this should be a verifier error instead?
> This comes directly from the isel, so I guess is one of the peculiarities of flag registers handling.
> 
> ===== Instruction selection ends
> ...
>             t89: i1 = V_CMP_LT_U64_e64 t103, t144 
>           t131: ch,glue = CopyToReg t0, Register:i1 $scc, t89
>         t86: i32 = S_CSELECT_B32 t55, t141, t131:1
> ...
> 
> *** MachineFunction at end of ISel ***
> ...
>   %46:sreg_64 = V_CMP_LT_U64_e64 %27:sreg_64, %47:vreg_64, implicit $exec
>   $scc = COPY %46:sreg_64
>   %48:sreg_32 = S_CSELECT_B32 killed %44:sreg_32, %42:sreg_32, implicit $scc
> ...
> 
> 
> 
> 
I think this needs a comment that this is only to tolerate SelectionDAG nonsense. When we finally delete it, we should remove this.
Can you also add a fixme that we should be using S_BITCMP0_B32 instead and only consider the 0th bit?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82370