[PATCH] D31161: [AMDGPU] New Waitcnt Insertion Pass

Stanislav Mekhanoshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 23 11:19:38 PDT 2017


rampitec added a comment.

Please add test with s_barrier.



================
Comment at: lib/Target/AMDGPU/SIInsertWaitcnts.cpp:74
+// special tokens like SCMEM_LDS (needed for buffer load to LDS).
+#define SQ_MAX_PGM_VGPRS 256
+#define SQ_MAX_PGM_SGPRS 104
----------------
kzhuravl wrote:
> @rampitec wrote:
> Should not we get these numbers from TD files for target which we already have?
Should not we get these numbers from TD files for target which we already have?


================
Comment at: lib/Target/AMDGPU/SIInsertWaitcnts.cpp:1017
+  if (MI.getOpcode() == AMDGPU::S_BARRIER && ST->needWaitcntBeforeBarrier()) {
+    ForceZero = true;
+    EmitSwaitcnt = true;
----------------
kzhuravl wrote:
> @rampitec wrote:
> For a barrier it will always insert strongest:
> 
> s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
> 
> regardless of what was an argument of the barrier.
> 
> This also seems to completely ignore atomic fences inserted around the barrier from the library, which shall be a real source of wait argument. Note, that semantics of needWaitcntBeforeBarrier() is not that we always need to insert wait with barrier, but that we may need to insert it.
> 
> Also note that existing pass does not seem to do it for a barrier.
For a barrier it will always insert strongest:

s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)

regardless of what was an argument of the barrier.

This also seems to completely ignore atomic fences inserted around the barrier from the library, which shall be a real source of wait argument. Note, that semantics of needWaitcntBeforeBarrier() is not that we always need to insert wait with barrier, but that we may need to insert it.

Also note that existing pass does not seem to do it for a barrier.


================
Comment at: lib/Target/AMDGPU/SIInsertWaitcnts.cpp:1613
+
+    if (ST->getGeneration() >= SISubtarget::VOLCANIC_ISLANDS) {
+
----------------
kzhuravl wrote:
> @rampitec wrote:
> Need to check for XNACK support.
Need to check for XNACK support.


https://reviews.llvm.org/D31161





More information about the llvm-commits mailing list