[PATCH] D106079: [AMDGPU] Divergence-driven compare operations instruction selection

[Alexander via Phabricator via llvm-commits]

alex-t added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:592
+        if (MI.isCopy()) {
+          Register SrcReg = MI.getOperand(1).getReg();
+          if (SrcReg == AMDGPU::SCC) {
----------------
arsenm wrote:
> When do copies from SCC appear? I thought the InstrEmitter could essentially always avoid these
They appear if we have a uniform comparison that provides the operand for the select node. We haven't yet made it to be S_CSELECT if uniform and V_CNDMASK if divergent. That's why InstrEmitter has to adjust operands by adding the SCC copies.  It may also happen for any VALU operation that takes SReg_64_XEXECRegClass operand.


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:594
+          if (SrcReg == AMDGPU::SCC) {
+            Register SCCCopy = MRI->createVirtualRegister(ST.isWave32() ?
+               &AMDGPU::SReg_32RegClass : &AMDGPU::SReg_64_XEXECRegClass);
----------------
rampitec wrote:
> TRI->getBoolRC()?
No. getBoolRC returns the SReg_32RegClass but I really need SReg_32_XM0_XEXEC. 


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:4475
+  case AMDGPU::S_CMP_EQ_U64: return AMDGPU::V_CMP_EQ_U64_e64;
+  case AMDGPU::S_CMP_LG_U64: return AMDGPU::V_CMP_NE_U64_e64;
   case AMDGPU::S_BCNT1_I32_B32: return AMDGPU::V_BCNT_U32_B32_e64;
----------------
critson wrote:
> Do we now have to always use larger e64 instructions or are these later reduced if we can use vcc instead?
SIShrinkInstructions does whatever is possible to reduce back to e32.
As for the change motivation, the main goal is to select SALU instructions for uniform SDNodes and VALU for the divergent ones.
It increases code size a bit but it changes the VALU instruction that operates over the 64 32bit lanes to SALU instruction that operates over 3 scalar registers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106079