[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/SIInstrInfo.td:530
   (setcc node:$lhs, node:$rhs, node:$cond), [{
-  for (SDNode *Use : N->uses()) {
-    if (Use->isMachineOpcode() || Use->getOpcode() != ISD::CopyToReg)
-      return false;
-
-    unsigned Reg = cast<RegisterSDNode>(Use->getOperand(1))->getReg();
-    if (Reg != AMDGPU::SCC)
-      return false;
-  }
-  return true;
+  return !N->isDivergent();
 }]>;
----------------
nhaehnle wrote:
> alex-t wrote:
> > foad wrote:
> > > I think you probably need to look ahead to see if this setcc is used by a divergent select. Otherwise you regress: `a == b ? c : d` where a and b are uniform but c or d are divergent. In this case it is better to use v_cmp than s_cmp, even though the operands are uniform, because it can feed straight into v_cndmask.
> > That's right. And is planned for the next change. I am just trying to keep it staged. In general, I am planning to add the dedicated procedure that checks the "VALU only" SDNode users beforehand and selects to VALU even uniform if any.
> Is it so clear-cut?
> 
> For `a == b ? c : d` with a and b uniform but c or d divergent, there's a choice between v_cmp + v_cndmask vs. s_cmp + s_cselect + v_cndmask.
> 
> So this change increases code size and trades 1 VALU for 2 SALU, but SALU utilization tends to be low, so purely based on instruction counts the s_cmp-based sequence is still better most of the time. On gfx10 there's a scheduling pitfall to watch out for though.
For this concrete example, S_CMP is still better. In my comment above I meant that I am planning to add a selection predicate as the right place to decide if the given node should be selected to VALU or SALU.
Further, we can try different strategies - one described above or another one, more complex.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106079