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

[Carl Ritson via Phabricator via llvm-commits]

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


================
Comment at: llvm/include/llvm/IR/IntrinsicsAMDGPU.td:1267
+// Like ps.live, but cannot be moved by LICM.
+def int_amdgcn_wqm_helper : Intrinsic <[llvm_i1_ty], [], []>;
+
----------------
arsenm wrote:
> I assume this needs to be convergent
Could be, my understanding is that without flags the intrinsic is marked "has side effects", which is correct as then it will not moved by LICM or removed by CSE.


================
Comment at: llvm/include/llvm/IR/IntrinsicsAMDGPU.td:1485
+// If false, mark all active lanes as helper lanes until the end of program.
+def int_amdgcn_wqm_demote : Intrinsic<[], [llvm_i1_ty], []>;
+
----------------
arsenm wrote:
> Ditto, and nomem
Convergent maybe (as above), but not nomem.
If this is marked nomem then it will be eaten by early CSE.
Since this was modelled on kill, is there a reason we don't mark kill Convergent?


================
Comment at: llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp:778-779
+          break;
+        unsigned Src = MBBI->getOperand(1).getReg();
+        if (!Register::isVirtualRegister(Src) && MBB.isLiveIn(Src)) {
+          MBBI++;
----------------
arsenm wrote:
> use Register
I assume you mean the variable name Src -> Register?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67767