Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/49203 )
Change subject: arch-x86: Fix how MediaOps sets the size of its operands.
......................................................................
arch-x86: Fix how MediaOps sets the size of its operands.
Because MediaOps have two sizes, one for sources and one for
destinations, it does not have a single dataSize member to set the size
of its operands. This was difficult to correct at the time, so a
dataSize member was created which was fixed at 0, and the instructions
themselves would use srcSize and destSize internally to do the actual
computation.
That causes problems when tracing, since the printReg function needs to
know what size to use to print some registers properly, specifically
integer registers.
To now fix that problem, some SFINAE constructors have been added which
will either pass through a dataSize member if one exists, or pass
through a pointer to the instruction itself so that operand index
selector class can pick out the member that makes sense for it (destSize
for DestOp, srcSize for Src1Op and Src2Op).
Change-Id: I6b8259a5ab27f809b81453bcf987cc6d1be4811a
---
M src/arch/x86/insts/micromediaop.hh
M src/arch/x86/insts/microop_args.hh
2 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/src/arch/x86/insts/micromediaop.hh
b/src/arch/x86/insts/micromediaop.hh
index 8c0b5ce..0fb8940 100644
--- a/src/arch/x86/insts/micromediaop.hh
+++ b/src/arch/x86/insts/micromediaop.hh
@@ -46,9 +46,11 @@
class MediaOpBase : public X86MicroopBase
{
- protected:
+ public:
const uint8_t srcSize;
const uint8_t destSize;
+
+ protected:
const uint8_t ext;
// Constructor
@@ -82,9 +84,6 @@
{
return ext & MediaSignedOp;
}
-
- public:
- static constexpr uint8_t dataSize = 0;
};
} // namespace X86ISA
diff --git a/src/arch/x86/insts/microop_args.hh
b/src/arch/x86/insts/microop_args.hh
index 47c003e..906ada0 100644
--- a/src/arch/x86/insts/microop_args.hh
+++ b/src/arch/x86/insts/microop_args.hh
@@ -32,6 +32,7 @@
#include <sstream>
#include <string>
#include <tuple>
+#include <type_traits>
#include <utility>
#include "arch/x86/insts/static_inst.hh"
@@ -55,6 +56,9 @@
RegIndex opIndex() const { return dest; }
DestOp(RegIndex _dest, size_t _size) : dest(_dest), size(_size) {}
+ template <class InstType>
+ DestOp(RegIndex _dest, InstType *inst) : dest(_dest),
size(inst->destSize)
+ {}
};
struct Src1Op
@@ -64,6 +68,9 @@
RegIndex opIndex() const { return src1; }
Src1Op(RegIndex _src1, size_t _size) : src1(_src1), size(_size) {}
+ template <class InstType>
+ Src1Op(RegIndex _src1, InstType *inst) : src1(_src1),
size(inst->srcSize)
+ {}
};
struct Src2Op
@@ -73,6 +80,9 @@
RegIndex opIndex() const { return src2; }
Src2Op(RegIndex _src2, size_t _size) : src2(_src2), size(_size) {}
+ template <class InstType>
+ Src2Op(RegIndex _src2, InstType *inst) : src2(_src2),
size(inst->srcSize)
+ {}
};
struct DataOp
@@ -103,13 +113,29 @@
{}
};
+template <class T, class Enabled=void>
+struct HasDataSize : public std::false_type {};
+
+template <class T>
+struct HasDataSize<T, decltype((void)&T::dataSize)> : public
std::true_type {};
+
+template <class T>
+constexpr bool HasDataSizeV = HasDataSize<T>::value;
+
template <class Base>
struct IntOp : public Base
{
using ArgType = GpRegIndex;
- template <class InstType>
- IntOp(InstType *inst, ArgType idx) : Base(idx.index, inst->dataSize) {}
+ template <class Inst>
+ IntOp(Inst *inst, std::enable_if_t<HasDataSizeV<Inst>, ArgType> idx) :
+ Base(idx.index, inst->dataSize)
+ {}
+
+ template <class Inst>
+ IntOp(Inst *inst, std::enable_if_t<!HasDataSizeV<Inst>, ArgType> idx) :
+ Base(idx.index, inst)
+ {}
void
print(std::ostream &os) const
@@ -204,8 +230,15 @@
{
using ArgType = FpRegIndex;
- template <class InstType>
- FloatOp(InstType *inst, ArgType idx) : Base(idx.index, inst->dataSize)
{}
+ template <class Inst>
+ FloatOp(Inst *inst, std::enable_if_t<HasDataSizeV<Inst>, ArgType>
idx) :
+ Base(idx.index, inst->dataSize)
+ {}
+
+ template <class Inst>
+ FloatOp(Inst *inst, std::enable_if_t<!HasDataSizeV<Inst>, ArgType>
idx) :
+ Base(idx.index, inst)
+ {}
void
print(std::ostream &os) const
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49203
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: I6b8259a5ab27f809b81453bcf987cc6d1be4811a
Gerrit-Change-Number: 49203
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <[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
