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

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp:693
+  case ISD::SETEQ:
+    Opcode = AMDGPU::V_CMP_LG_F32_e64;
+    break;
----------------
critson wrote:
> foad wrote:
> > I still don't trust this table! I think the "O" predicates should map to "N" opcodes e.g. SETOLT -> V_CMP_NLT as explained above.
> Are you expecting the table to look like this?
> ```
>   case ISD::SETEQ:
>     Opcode = AMDGPU::V_CMP_LG_F32_e64;
>     break;
>   case ISD::SETGT:
>     Opcode = AMDGPU::V_CMP_GE_F32_e64;
>     break;
>   case ISD::SETGE:
>     Opcode = AMDGPU::V_CMP_GT_F32_e64;
>     break;
>   case ISD::SETLT:
>     Opcode = AMDGPU::V_CMP_LE_F32_e64;
>     break;
>   case ISD::SETLE:
>     Opcode = AMDGPU::V_CMP_LT_F32_e64;
>     break;
>   case ISD::SETNE:
>     Opcode = AMDGPU::V_CMP_EQ_F32_e64;
>     break;
>   case ISD::SETO:
>     Opcode = AMDGPU::V_CMP_O_F32_e64;
>     break;
>   case ISD::SETUO:
>     Opcode = AMDGPU::V_CMP_U_F32_e64;
>     break;
>   case ISD::SETOEQ:
>   case ISD::SETUEQ:
>     Opcode = AMDGPU::V_CMP_NEQ_F32_e64;
>     break;
>   case ISD::SETOGT:
>   case ISD::SETUGT:
>     Opcode = AMDGPU::V_CMP_NLT_F32_e64;
>     break;
>   case ISD::SETOGE:
>   case ISD::SETUGE:
>     Opcode = AMDGPU::V_CMP_NLE_F32_e64;
>     break;
>   case ISD::SETOLT:
>   case ISD::SETULT:
>     Opcode = AMDGPU::V_CMP_NGT_F32_e64;
>     break;
>   case ISD::SETOLE:
>   case ISD::SETULE:
>     Opcode = AMDGPU::V_CMP_NGE_F32_e64;
>     break;
>   case ISD::SETONE:
>   case ISD::SETUNE:
>     Opcode = AMDGPU::V_CMP_NLG_F32_e64;
>     break;
> ```
> 
> I am still testing, but I am unsure if VulkanCTS or game shaders can tell the differences.
No I'm expecting it to look like this:
```
case ISD::SETOEQ:
case ISD::SETEQ:
  Opcode = AMDGPU::V_CMP_NEQ_F32_e64;
  break;
case ISD::SETOGT:
case ISD::SETGT:
  Opcode = AMDGPU::V_CMP_NLT_F32_e64;
  break;
case ISD::SETOGE:
case ISD::SETGE:
  Opcode = AMDGPU::V_CMP_NLE_F32_e64;
  break;
case ISD::SETOLT:
case ISD::SETLT:
  Opcode = AMDGPU::V_CMP_NGT_F32_e64;
  break;
case ISD::SETOLE:
case ISD::SETLE:
  Opcode = AMDGPU::V_CMP_NGE_F32_e64;
  break;
case ISD::SETONE:
  Opcode = AMDGPU::V_CMP_NLG_F32_e64;
  break;
case ISD::SETO:
  Opcode = AMDGPU::V_CMP_U_F32_e64;
  break;
case ISD::SETUO:
  Opcode = AMDGPU::V_CMP_O_F32_e64;
  break;
case ISD::SETUEQ:
  Opcode = AMDGPU::V_CMP_LG_F32_e64;
  break;
case ISD::SETUGT:
  Opcode = AMDGPU::V_CMP_GE_F32_e64;
  break;
case ISD::SETUGE:
  Opcode = AMDGPU::V_CMP_GT_F32_e64;
  break;
case ISD::SETULT:
  Opcode = AMDGPU::V_CMP_LE_F32_e64;
  break;
case ISD::SETULE:
  Opcode = AMDGPU::V_CMP_LT_F32_e64;
  break;
case ISD::SETUNE:
case ISD::SETNE:
  Opcode = AMDGPU::V_CMP_EQ_F32_e64;
  break;
```
E.g. SETOGT --(swap operands)--> SETOLT --(invert)-->SETUGE which is V_CMP_NLT.

(Though as mentioned previously, SETEQ is undefined on NaNs, so it doesn't matter whether it behaves the same as SETOEQ or the same as SETUEQ. And likewise for all the others that don't have an explicit O or U.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94746