[gem5-dev] Change in gem5/gem5[develop]: gpu-compute: Fix FLAT insts decrementing lgkm count early

2021-01-07 Thread Matt Sinclair (Gerrit) via gem5-dev
Matt Sinclair has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/38696 )


Change subject: gpu-compute: Fix FLAT insts decrementing lgkm count early
..

gpu-compute: Fix FLAT insts decrementing lgkm count early

FLAT instructions used to decrement lgkm count on execute, while the
GCN3 ISA specifies that lgkm count should be decremented on data being
returned or data being written.

This patch changes it so that lgkm is decremented after initiateAcc (for
stores) and after completeAcc (for loads) to better reflect the ISA
definition.

This fixes a bug where waitcnts would be satisfied even though the
memory access wasn't completed, which lead to instructions using the
wrong data.

Change-Id: I596cb031af9cda8d47a1b5e146e4a4ffd793d36c
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38696
Reviewed-by: Matt Sinclair 
Reviewed-by: Matthew Poremba 
Maintainer: Matt Sinclair 
Tested-by: kokoro 
---
M src/gpu-compute/global_memory_pipeline.cc
M src/gpu-compute/gpu_dyn_inst.cc
2 files changed, 7 insertions(+), 1 deletion(-)

Approvals:
  Matthew Poremba: Looks good to me, approved
  Matt Sinclair: Looks good to me, but someone else must approve; Looks  
good to me, approved

  kokoro: Regressions pass



diff --git a/src/gpu-compute/global_memory_pipeline.cc  
b/src/gpu-compute/global_memory_pipeline.cc

index bcd93f8..a2b24e4 100644
--- a/src/gpu-compute/global_memory_pipeline.cc
+++ b/src/gpu-compute/global_memory_pipeline.cc
@@ -130,6 +130,9 @@
 DPRINTF(GPUMem, "CU%d: WF[%d][%d]: Completing global mem  
instr %s\n",

 m->cu_id, m->simdId, m->wfSlotId, m->disassemble());
 m->completeAcc(m);
+if (m->isFlat() && m->isLoad()) {
+w->decLGKMInstsIssued();
+}
 w->decVMemInstsIssued();

 if (m->isLoad() || m->isAtomicRet()) {
@@ -193,6 +196,10 @@
 mp->disassemble(), mp->seqNum());
 mp->initiateAcc(mp);

+if (mp->isFlat() && mp->isStore()) {
+mp->wavefront()->decLGKMInstsIssued();
+}
+
 if (mp->isStore() && mp->isGlobalSeg()) {
 mp->wavefront()->decExpInstsIssued();
 }
diff --git a/src/gpu-compute/gpu_dyn_inst.cc  
b/src/gpu-compute/gpu_dyn_inst.cc

index 03ed689..38e4ecf 100644
--- a/src/gpu-compute/gpu_dyn_inst.cc
+++ b/src/gpu-compute/gpu_dyn_inst.cc
@@ -819,7 +819,6 @@
 if (executedAs() == Enums::SC_GLOBAL) {
 // no transormation for global segment
 wavefront()->execUnitId =  wavefront()->flatGmUnitId;
-wavefront()->decLGKMInstsIssued();
 if (isLoad()) {
 wavefront()->rdLmReqsInPipe--;
 } else if (isStore()) {

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38696
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I596cb031af9cda8d47a1b5e146e4a4ffd793d36c
Gerrit-Change-Number: 38696
Gerrit-PatchSet: 4
Gerrit-Owner: Kyle Roarty 
Gerrit-Reviewer: Alexandru Duțu 
Gerrit-Reviewer: Matt Sinclair 
Gerrit-Reviewer: Matthew Poremba 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: gpu-compute: Fix FLAT insts decrementing lgkm count early

2020-12-23 Thread Kyle Roarty (Gerrit) via gem5-dev
Kyle Roarty has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/38696 )



Change subject: gpu-compute: Fix FLAT insts decrementing lgkm count early
..

gpu-compute: Fix FLAT insts decrementing lgkm count early

FLAT instructions used to decrement lgkm count on execute, while the
GCN3 ISA specifies that lgkm count should be decremented on data being
returned or data being written.

This patch changes it so that lgkm is decremented after initiateAcc (for
stores) and after completeAcc (for loads) to better reflect the ISA
definition.

This fixes a bug where waitcnts would be satisfied even though the
memory access wasn't completed, which lead to instructions using the
wrong data.

Change-Id: I596cb031af9cda8d47a1b5e146e4a4ffd793d36c
---
M src/gpu-compute/global_memory_pipeline.cc
M src/gpu-compute/gpu_dyn_inst.cc
2 files changed, 3 insertions(+), 1 deletion(-)



diff --git a/src/gpu-compute/global_memory_pipeline.cc  
b/src/gpu-compute/global_memory_pipeline.cc

index bcd93f8..e71f1a9 100644
--- a/src/gpu-compute/global_memory_pipeline.cc
+++ b/src/gpu-compute/global_memory_pipeline.cc
@@ -130,6 +130,9 @@
 DPRINTF(GPUMem, "CU%d: WF[%d][%d]: Completing global mem  
instr %s\n",

 m->cu_id, m->simdId, m->wfSlotId, m->disassemble());
 m->completeAcc(m);
+if (m->isFlat()) {
+w->decLGKMInstsIssued();
+}
 w->decVMemInstsIssued();

 if (m->isLoad() || m->isAtomicRet()) {
diff --git a/src/gpu-compute/gpu_dyn_inst.cc  
b/src/gpu-compute/gpu_dyn_inst.cc

index 03ed689..38e4ecf 100644
--- a/src/gpu-compute/gpu_dyn_inst.cc
+++ b/src/gpu-compute/gpu_dyn_inst.cc
@@ -819,7 +819,6 @@
 if (executedAs() == Enums::SC_GLOBAL) {
 // no transormation for global segment
 wavefront()->execUnitId =  wavefront()->flatGmUnitId;
-wavefront()->decLGKMInstsIssued();
 if (isLoad()) {
 wavefront()->rdLmReqsInPipe--;
 } else if (isStore()) {

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38696
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I596cb031af9cda8d47a1b5e146e4a4ffd793d36c
Gerrit-Change-Number: 38696
Gerrit-PatchSet: 1
Gerrit-Owner: Kyle Roarty 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s