Anthony Gutierrez has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/29927 )
Change subject: arch-gcn3, gpu-compute: Fix issue when reading const
operands
......................................................................
arch-gcn3, gpu-compute: Fix issue when reading const operands
Currently, when an instruction has an operand that reads a const
value, it goes thru the same readMiscReg() api call as other
misc registers (real HW registers, not constant values). There
is an issue, however, when casting from the const values (which are
32b) to higher precision values, like 64b.
This change creates a separate, templated function call to the GPU's
ISA state that will return the correct type.
Change-Id: I41965ebeeed20bb70e919fce5ad94d957b3af802
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/29927
Reviewed-by: Anthony Gutierrez <anthony.gutier...@amd.com>
Maintainer: Anthony Gutierrez <anthony.gutier...@amd.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/arch/gcn3/gpu_isa.hh
M src/arch/gcn3/isa.cc
M src/arch/gcn3/operand.hh
M src/arch/gcn3/registers.cc
M src/arch/gcn3/registers.hh
M src/gpu-compute/gpu_exec_context.hh
6 files changed, 66 insertions(+), 17 deletions(-)
Approvals:
Anthony Gutierrez: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/gcn3/gpu_isa.hh b/src/arch/gcn3/gpu_isa.hh
index 26b79c7..228c3fe 100644
--- a/src/arch/gcn3/gpu_isa.hh
+++ b/src/arch/gcn3/gpu_isa.hh
@@ -37,6 +37,7 @@
#define __ARCH_GCN3_GPU_ISA_HH__
#include <array>
+#include <type_traits>
#include "arch/gcn3/registers.hh"
#include "gpu-compute/dispatcher.hh"
@@ -52,6 +53,24 @@
public:
GPUISA(Wavefront &wf);
+ template<typename T> T
+ readConstVal(int opIdx) const
+ {
+ panic_if(!std::is_integral<T>::value, "Constant values must "
+ "be an integer.\n");
+ T val(0);
+
+ if (isPosConstVal(opIdx)) {
+ val = (T)readPosConstReg(opIdx);
+ }
+
+ if (isNegConstVal(opIdx)) {
+ val = (T)readNegConstReg(opIdx);
+ }
+
+ return val;
+ }
+
ScalarRegU32 readMiscReg(int opIdx) const;
void writeMiscReg(int opIdx, ScalarRegU32 operandVal);
bool hasScalarUnit() const { return true; }
@@ -63,10 +82,9 @@
return posConstRegs[opIdx - REG_INT_CONST_POS_MIN];
}
- ScalarRegU32 readNegConstReg(int opIdx) const
+ ScalarRegI32 readNegConstReg(int opIdx) const
{
- return *((ScalarRegU32*)
- &negConstRegs[opIdx - REG_INT_CONST_NEG_MIN]);
+ return negConstRegs[opIdx - REG_INT_CONST_NEG_MIN];
}
static const std::array<const ScalarRegU32, NumPosConstRegs>
diff --git a/src/arch/gcn3/isa.cc b/src/arch/gcn3/isa.cc
index 036c771..3bd122d 100644
--- a/src/arch/gcn3/isa.cc
+++ b/src/arch/gcn3/isa.cc
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2016-2017 Advanced Micro Devices, Inc.
+ * Copyright (c) 2016-2018 Advanced Micro Devices, Inc.
* All rights reserved.
*
* For use for simulation and test purposes only
@@ -49,14 +49,6 @@
ScalarRegU32
GPUISA::readMiscReg(int opIdx) const
{
- if (opIdx >= REG_INT_CONST_POS_MIN && opIdx <=
REG_INT_CONST_POS_MAX) {
- return readPosConstReg(opIdx);
- }
-
- if (opIdx >= REG_INT_CONST_NEG_MIN && opIdx <=
REG_INT_CONST_NEG_MAX) {
- return readNegConstReg(opIdx);
- }
-
switch (opIdx) {
case REG_M0:
return m0;
diff --git a/src/arch/gcn3/operand.hh b/src/arch/gcn3/operand.hh
index 218faf8..7f70fab 100644
--- a/src/arch/gcn3/operand.hh
+++ b/src/arch/gcn3/operand.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2017 Advanced Micro Devices, Inc.
+ * Copyright (c) 2017-2018 Advanced Micro Devices, Inc.
* All rights reserved.
*
* For use for simulation and test purposes only
@@ -583,10 +583,15 @@
default:
{
assert(sizeof(DataType) <= sizeof(srfData));
- DataType misc_val
- = (DataType)_gpuDynInst->readMiscReg(_opIdx);
+ DataType misc_val(0);
+ if (isConstVal(_opIdx)) {
+ misc_val = (DataType)_gpuDynInst
+ ->readConstVal<DataType>(_opIdx);
+ } else {
+ misc_val =
(DataType)_gpuDynInst->readMiscReg(_opIdx);
+ }
std::memcpy((void*)srfData.data(), (void*)&misc_val,
- sizeof(DataType));
+ sizeof(DataType));
}
}
}
diff --git a/src/arch/gcn3/registers.cc b/src/arch/gcn3/registers.cc
index 0872ff9..016160f 100644
--- a/src/arch/gcn3/registers.cc
+++ b/src/arch/gcn3/registers.cc
@@ -163,6 +163,31 @@
}
bool
+ isPosConstVal(int opIdx)
+ {
+ bool is_pos_const_val = (opIdx >= REG_INT_CONST_POS_MIN
+ && opIdx <= REG_INT_CONST_POS_MAX);
+
+ return is_pos_const_val;
+ }
+
+ bool
+ isNegConstVal(int opIdx)
+ {
+ bool is_neg_const_val = (opIdx >= REG_INT_CONST_NEG_MIN
+ && opIdx <= REG_INT_CONST_NEG_MAX);
+
+ return is_neg_const_val;
+ }
+
+ bool
+ isConstVal(int opIdx)
+ {
+ bool is_const_val = isPosConstVal(opIdx) || isNegConstVal(opIdx);
+ return is_const_val;
+ }
+
+ bool
isLiteral(int opIdx)
{
return opIdx == REG_SRC_LITERAL;
diff --git a/src/arch/gcn3/registers.hh b/src/arch/gcn3/registers.hh
index 9922e5d..6e95807 100644
--- a/src/arch/gcn3/registers.hh
+++ b/src/arch/gcn3/registers.hh
@@ -238,6 +238,9 @@
std::string opSelectorToRegSym(int opIdx, int numRegs=0);
int opSelectorToRegIdx(int opIdx, int numScalarRegs);
+ bool isPosConstVal(int opIdx);
+ bool isNegConstVal(int opIdx);
+ bool isConstVal(int opIdx);
bool isLiteral(int opIdx);
bool isScalarReg(int opIdx);
bool isVectorReg(int opIdx);
diff --git a/src/gpu-compute/gpu_exec_context.hh
b/src/gpu-compute/gpu_exec_context.hh
index 6fc71f0..15cbd55 100644
--- a/src/gpu-compute/gpu_exec_context.hh
+++ b/src/gpu-compute/gpu_exec_context.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015 Advanced Micro Devices, Inc.
+ * Copyright (c) 2015-2018 Advanced Micro Devices, Inc.
* All rights reserved.
*
* For use for simulation and test purposes only
@@ -48,6 +48,12 @@
Wavefront* wavefront();
ComputeUnit* computeUnit();
+ template<typename T> T
+ readConstVal(int opIdx) const
+ {
+ return gpuISA->readConstVal<T>(opIdx);
+ }
+
RegVal readMiscReg(int opIdx) const;
void writeMiscReg(int opIdx, RegVal operandVal);
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/29927
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: I41965ebeeed20bb70e919fce5ad94d957b3af802
Gerrit-Change-Number: 29927
Gerrit-PatchSet: 6
Gerrit-Owner: Anthony Gutierrez <anthony.gutier...@amd.com>
Gerrit-Reviewer: Anthony Gutierrez <anthony.gutier...@amd.com>
Gerrit-Reviewer: Tony Gutierrez <anthony.gutier...@amd.com>
Gerrit-Reviewer: Tuan Ta <q...@cornell.edu>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
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
