[PATCH] D115747: [AMDGPU] Flush the vmcnt counter in loop preheader when necessary

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/waitcnt-vmcnt-loop.mir:73
+
+    renamable $vgpr0 = BUFFER_LOAD_FORMAT_X_IDXEN killed renamable $vgpr0, renamable $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    S_BRANCH %bb.1
----------------
You could remove "renamable" everywhere, just to reduce clutter.


================
Comment at: llvm/test/CodeGen/AMDGPU/waitcnt-vmcnt-loop.mir:91
+
+# GFX9-LABEL: waitcnt_vm_loop_noterm
+# GFX9-LABEL: bb.0:
----------------
I'd appreciate a very brief comment with each function, saying what it's for and whether the waitcnt should be hoisted or not. E.g. for this one "same as before but bb.0 has no terminator" (it took me a while to spot the difference).


================
Comment at: llvm/test/CodeGen/AMDGPU/waitcnt-vmcnt-loop.mir:275
+# GFX9-LABEL: bb.1:
+# GFX9: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
----------------
I am confused by this test case. Why does GFX9 need a waitcnt inside the loop?


================
Comment at: llvm/test/CodeGen/AMDGPU/waitcnt-vmcnt-loop.mir:395
+
+# GFX9-LABEL: waitcnt_vm_loop2_interval
+# GFX9-LABEL: bb.0:
----------------
Does "_interval" here refer to the RegInterval stuff in SIInsertWaitcnts? Maybe use "_reginterval" to make this clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115747