[PATCH] D94746: [AMDGPU] Move kill lowering to WQM pass and add live mask tracking

Tue Jan 19 16:56:24 PST 2021

critson marked 9 inline comments as done.
critson added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp:685
+  case ISD::SETGT:
+    Opcode = AMDGPU::V_CMP_GT_F32_e64;
+    break;
----------------
foad wrote:
> I'm a bit sceptical of some of the opcodes in this table.
> 
> E.g. SETOGT --(swap operands)--> SETOLT --(invert)-->SETUGE which is V_CMP_NLT, but the table here says V_CMP_GT.
> 
> It might be more obvious what's going on if you call getSetCCSwappedOperands and getSetCCInverse first, and then have a simple lookup from SETxx to opcode. Or you could avoid the "invert" by using AND instead of ANDN2?
I agree these are wrong, but the inversion is required because this will only generate comparison results for active lanes -- so in non-uniform control flow this will not generate a complete mask to use AND.

================
Comment at: llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp:700
+  case ISD::SETONE:
+  case ISD::SETNE:
+    Opcode = AMDGPU::V_CMP_EQ_F32_e64;
----------------
foad wrote:
> It's a bit more conventional to lump SETNE together with SET**U**NE (unlike all the other comparisons which go with the **O** form), though of course it doesn't really matter.
The branches of the switch are lifted from the old kill lowering code.
So I do not plan to touch it for now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94746