Hello Tony Gutierrez,
I'd like you to do a code review. Please visit
https://gem5-review.googlesource.com/c/public/gem5/+/29954
to review the following change.
Change subject: arch-gcn3: Fix s_getpc operand information
......................................................................
arch-gcn3: Fix s_getpc operand information
s_getpc was currently reporting only a single operand,
and was only considering the SSRC operand. However,
this instruction' source is implicitly the PC.
Because its destination register was never tracked for
dependence checking purposes, dependence violations
are possible.
Change-Id: Ia80b8b3e24d5885f646a9ee41212a2cb35b9ffe6
---
M src/arch/gcn3/insts/instructions.hh
M src/arch/gcn3/insts/op_encodings.cc
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/src/arch/gcn3/insts/instructions.hh
b/src/arch/gcn3/insts/instructions.hh
index b0cc37e..f561043 100644
--- a/src/arch/gcn3/insts/instructions.hh
+++ b/src/arch/gcn3/insts/instructions.hh
@@ -5846,9 +5846,7 @@
getOperandSize(int opIdx) override
{
switch (opIdx) {
- case 0: //ssrc
- return 8;
- case 1: //sdst
+ case 0: //sdst
return 8;
default:
fatal("op idx %i out of bounds\n", opIdx);
@@ -5860,9 +5858,7 @@
isSrcOperand(int opIdx) override
{
switch (opIdx) {
- case 0: //ssrc
- return true;
- case 1: //sdst
+ case 0: //sdst
return false;
default:
fatal("op idx %i out of bounds\n", opIdx);
@@ -5874,9 +5870,7 @@
isDstOperand(int opIdx) override
{
switch (opIdx) {
- case 0: //ssrc
- return false;
- case 1: //sdst
+ case 0: //sdst
return true;
default:
fatal("op idx %i out of bounds\n", opIdx);
diff --git a/src/arch/gcn3/insts/op_encodings.cc
b/src/arch/gcn3/insts/op_encodings.cc
index 22d0f48..997b22f 100644
--- a/src/arch/gcn3/insts/op_encodings.cc
+++ b/src/arch/gcn3/insts/op_encodings.cc
@@ -326,7 +326,12 @@
switch (opIdx) {
case 0:
- return isScalarReg(instData.SSRC0);
+ if (instData.OP == 0x1C) {
+ // Special case for s_getpc, which has no source reg.
+ // Instead, it implicitly reads the PC.
+ return isScalarReg(instData.SDST);
+ }
+ return isScalarReg(instData.SSRC0);
case 1:
return isScalarReg(instData.SDST);
default:
@@ -353,6 +358,12 @@
switch (opIdx) {
case 0:
+ if (instData.OP == 0x1C) {
+ // Special case for s_getpc, which has no source reg.
+ // Instead, it implicitly reads the PC.
+ return opSelectorToRegIdx(instData.SDST,
+ gpuDynInst->wavefront()->reservedScalarRegs);
+ }
return opSelectorToRegIdx(instData.SSRC0,
gpuDynInst->wavefront()->reservedScalarRegs);
case 1:
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/29954
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: Ia80b8b3e24d5885f646a9ee41212a2cb35b9ffe6
Gerrit-Change-Number: 29954
Gerrit-PatchSet: 1
Gerrit-Owner: Anthony Gutierrez <[email protected]>
Gerrit-Reviewer: Tony Gutierrez <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
