[PATCH] D94747: [AMDGPU] Add llvm.amdgcn.wqm.demote intrinsic

[Piotr Sobczak via Phabricator via llvm-commits]

piotr added a comment.

LGTM with a few nits - feel free to ignore them.



================
Comment at: llvm/include/llvm/IR/IntrinsicsAMDGPU.td:1352
 // invocation for derivative computation).
 def int_amdgcn_ps_live : Intrinsic <
   [llvm_i1_ty],
----------------
Mark as deprecated?


================
Comment at: llvm/include/llvm/IR/IntrinsicsAMDGPU.td:1357
 
+// Similar to int_amdgcn_ps_live, but cannot be moved by LICM.
+// Returns true if lane is not a helper.
----------------
arsenm wrote:
> Should ps_live eventually be replaced? I would rather make that clear rather than adding another intrinsic to void updating users
If the old ps_live is slated to go away, it would be better to not refer to it here.


================
Comment at: llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp:727
+    }
+    if (CanDemote && !KillBlocks.contains(&MBB)) {
+      for (auto &MI : MBB) {
----------------
You can avoid calling .contains() by adding a local variable in the outer "for" loop (or slightly refactoring the first inner loop and testing the iterator).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94747