Gabe Black has submitted this change. (
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
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49203
Tested-by: kokoro <[email protected]>
Maintainer: Gabe Black <[email protected]>
Reviewed-by: Matt Sinclair <[email protected]>
---
M src/arch/x86/insts/micromediaop.hh
M src/arch/x86/insts/microop_args.hh
2 files changed, 42 insertions(+), 5 deletions(-)
Approvals:
Matt Sinclair: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/x86/insts/micromediaop.hh
b/src/arch/x86/insts/micromediaop.hh
index 8c0b5ce..bd897f9 100644
--- a/src/arch/x86/insts/micromediaop.hh
+++ b/src/arch/x86/insts/micromediaop.hh
@@ -84,7 +84,8 @@
}
public:
- static constexpr uint8_t dataSize = 0;
+ uint8_t getSrcSize() const { return srcSize; }
+ uint8_t getDestSize() const { return destSize; }
};
} // namespace X86ISA
diff --git a/src/arch/x86/insts/microop_args.hh
b/src/arch/x86/insts/microop_args.hh
index 47c003e..c1e4b12 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,10 @@
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->getDestSize())
+ {}
};
struct Src1Op
@@ -64,6 +69,10 @@
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->getSrcSize())
+ {}
};
struct Src2Op
@@ -73,6 +82,10 @@
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->getSrcSize())
+ {}
};
struct DataOp
@@ -103,13 +116,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 +233,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
2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the
submitted one.
--
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: 4
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: Matt Sinclair <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
