[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Fix how MediaOps sets the size of its operands.
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
Maintainer: Gabe Black
Reviewed-by: Matt Sinclair
---
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
#include
#include
+#include
#include
#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
+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
+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
+Src2Op(RegIndex _src2, InstType *inst) : src2(_src2),
+size(inst->getSrcSize())
+{}
};
struct DataOp
@@ -103,13 +116,29 @@
{}
};
+template
+struct HasDataSize : public std::false_type {};
+
+template
+struct HasDataSize : public
std::true_type {};
+
+template
+constexpr bool HasDataSizeV = HasDataSize::value;
+
template
struct IntOp : public Base
{
using ArgType = GpRegIndex;
-template
-IntOp(InstType *inst, ArgType idx) : Base(idx.index, inst->dataSize) {}
+template
+IntOp(Inst *inst, std::enable_if_t, ArgType> idx) :
+Base(idx.index, inst->dataSize)
+{}
+
+template
+IntOp(Inst *inst, std::enable_if_t, ArgType> idx) :
+Base(idx.index, inst)
+{}
void
print(std::ostream ) const
@@ -204,8 +233,15 @@
{
using ArgType = FpRegIndex;
-template
-FloatOp(InstType *inst, ArgType idx) : Base(idx.index, inst->dataSize)
{}
+template
+FloatOp(Inst *inst, std::enable_if_t, ArgType>
idx) :
+Base(idx.index, inst->dataSize)
+{}
+
+template
+FloatOp(Inst *inst, std::enable_if_t, ArgType>
idx) :
+Base(idx.index, inst)
+{}
void
print(std::ostream ) 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
Gerrit-Reviewer: Bobby R. Bruce
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: Matt Sinclair
Gerrit-Reviewer: kokoro
Gerrit-MessageType: merged
___
gem5-dev
[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Use existing constants to simplify some code in operands.isa.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/49244 )
Change subject: arch-x86: Use existing constants to simplify some code in
operands.isa.
..
arch-x86: Use existing constants to simplify some code in operands.isa.
The "predicate"s for reading/writing some condition code registers were
written with constants which were built up from other constants which
represent individual bits in the condition code register. There are
existing constants which already exactly match those sets of bits, so we
can juse use those instead of building up the same thing in-situ.
Change-Id: Iab5a5de04d0fd858414451531a357770ca9fde14
---
M src/arch/x86/isa/operands.isa
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/src/arch/x86/isa/operands.isa b/src/arch/x86/isa/operands.isa
index 2bf253e..f07db14 100644
--- a/src/arch/x86/isa/operands.isa
+++ b/src/arch/x86/isa/operands.isa
@@ -153,27 +153,22 @@
# would be retained, the write predicate checks if any of the bits
# are being written.
-'PredccFlagBits': ('CCReg', 'uqw', '(X86ISA::CCREG_ZAPS)', None,
+'PredccFlagBits': ('CCReg', 'uqw', 'X86ISA::CCREG_ZAPS', None,
60, None, None,
-'''(((ext & (X86ISA::PFBit | X86ISA::AFBit |
- X86ISA::ZFBit | X86ISA::SFBit)) !=
- (X86ISA::PFBit | X86ISA::AFBit |
- X86ISA::ZFBit | X86ISA::SFBit)) &&
-((ext & (X86ISA::PFBit | X86ISA::AFBit |
- X86ISA::ZFBit | X86ISA::SFBit)) != 0))''',
-'''((ext & (X86ISA::PFBit | X86ISA::AFBit |
-X86ISA::ZFBit | X86ISA::SFBit)) != 0)'''),
-'PredcfofBits': ('CCReg', 'uqw', '(X86ISA::CCREG_CFOF)', None,
-61, None, None, '''(((ext & X86ISA::CFBit) == 0 ||
-(ext & X86ISA::OFBit) == 0) &&
-((ext & (X86ISA::CFBit | X86ISA::OFBit)) != 0))''',
-'((ext & (X86ISA::CFBit | X86ISA::OFBit)) != 0)'),
-'PreddfBit': ('CCReg', 'uqw', '(X86ISA::CCREG_DF)', None,
-62, None, None, '(false)', '((ext & X86ISA::DFBit) != 0)'),
-'PredecfBit': ('CCReg', 'uqw', '(X86ISA::CCREG_ECF)', None,
-63, None, None, '(false)', '((ext & X86ISA::ECFBit) !=
0)'),
-'PredezfBit': ('CCReg', 'uqw', '(X86ISA::CCREG_EZF)', None,
-64, None, None, '(false)', '((ext & X86ISA::EZFBit) !=
0)'),
+'(ext & X86ISA::ccFlagMask) != X86ISA::ccFlagMask && '
+'(ext & X86ISA::ccFlagMask) != 0',
+'(ext & X86ISA::ccFlagMask) != 0'),
+'PredcfofBits': ('CCReg', 'uqw', 'X86ISA::CCREG_CFOF', None,
+61, None, None,
+'(ext & X86ISA::cfofMask) != X86ISA::cfofMask && '
+'(ext & X86ISA::cfofMask) != 0',
+'(ext & X86ISA::cfofMask) != 0'),
+'PreddfBit': ('CCReg', 'uqw', 'X86ISA::CCREG_DF', None,
+62, None, None, 'false', '(ext & X86ISA::DFBit) != 0'),
+'PredecfBit':('CCReg', 'uqw', 'X86ISA::CCREG_ECF', None,
+63, None, None, 'false', '(ext & X86ISA::ECFBit) != 0'),
+'PredezfBit':('CCReg', 'uqw', 'X86ISA::CCREG_EZF', None,
+64, None, None, 'false', '(ext & X86ISA::EZFBit) != 0'),
# These register should needs to be more protected so that later
# instructions don't map their indexes with an old value.
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49244
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: Iab5a5de04d0fd858414451531a357770ca9fde14
Gerrit-Change-Number: 49244
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black
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
[gem5-dev] Re: SCons SimObject() and m5.objects hierarchy
Ok, it sounds like listing out the contents of the SimObject files is not
going to be a problem then, and I'll plan on doing that.
*I* still like the idea of adding hierarchy (this is why python has
packages, c++ has namespaces, almost all modern languages have similar
features, etc), but fortunately that's an orthogonal issue to this and
isn't blocking. This just would make that easier to implement if we decide
to do that in the future.
Gabe
On Fri, Aug 13, 2021 at 10:21 AM Jason Lowe-Power
wrote:
> Hey Gabe,
>
> Some responses inline below.
>
> On Thu, Aug 12, 2021 at 9:37 PM Gabe Black via gem5-dev
> wrote:
>
>> Hey folks, here are some of my thoughts on the SimObject() mechanism in
>> SCons, and the structure of the m5.objects package.
>>
>> Right now, when you put a SimObject('foo.py') line in a SConscript, that
>> eventually leads to SCons importing foo.py (with some special machinery)
>> and discovering through python what SimObjects, etc, are declared in that
>> file. Those are collected into lists, and then each SimObject gets a
>> params/Foo.hh, params/Bar.hh, etc.
>>
>> Then as far as m5.objects, each file declared with SimObject() ends up as
>> a subpackage, with the stuff inside it available as you would expect, ie
>> m5.objects.Foo.Bar and m5.objects.Foo.Foo. Also, all the SimObjects are
>> artificially put under m5.objects so that:
>>
>> from m5.objects import *
>>
>> will get everything.
>>
>>
>> First, having a single SimObject() which can produce an arbitrary number
>> of actual SimObject subclasses, enums, etc, is problematic for SCons. SCons
>> only does one pass, where you have to tell it all of what files come from
>> what files, and then tell it to build. That means that SimObject() files
>> *must* be read in during the build setup phase, since we have to figure out
>> what files they would produce. It would be much more convenient to be able
>> to delay that and do that as part of the actual build. That would let us,
>> for example, build the marshal binary as a build product and not as part of
>> setting up the build process itself.
>>
>> Second, something that I've brought up in the past is that having no
>> hierarchy in m5.objects is not ideal. That leads to SimObjects having
>> namespace information built into their name like "X86TLB", "ARMTLB", etc.
>> This makes things more verbose, and hides structure. It also increases the
>> likelihood of name collisions since everything is jammed into the same
>> space.
>>
>
> I actually think the more verbose names are better. I really don't like
> that we allow name "collisions" in different namespaces in C++ right now.
> That confuses me/our users and also confuses IDEs.
>
>
>>
>>
>> To help solve both of those problems, I'd like to propose changing
>> SimObject() to have an __all__ type parameter which specifies what inside
>> it to export, and what to export it as. It would also need to encode what
>> type of entity something is as well, so we know if we need to define an
>> enum, a SimObject, etc. Perhaps something like this:
>>
>> SimObject('foo.py', ['objects.x86.TLB', 'objects.x86.LocalApic',
>> 'enum.MyFavoriteEnum'])
>>
>
> I like this idea. It will make other things like supporting documentation
> and typing easier as well. However, I think we should not make significant
> changes to the python module hierarchy.
>
>
>>
>> That would tell SCons what targets to set up for, and also how to
>> structure the m5.objects package.
>>
>> import m5.objects.x86
>>
>> Foo.tlb = x86.TLB()
>>
>> class Example(SimObject):
>> tlb = Param.x86.TLB(...)
>>
>>
>> This would be a major departure from how imports have worked. It would
>> mostly be mechanical when SimObjects are imported directly, but would make
>> "from m5.objects import *" impossible.\\
>>
>
> I think this is a huge downside. Essentially, all of the config files
> would have to be updated, and testing those is impossible in our current
> implementation. If we really think that deprecating `from m5.object import
> *` is going to provide large benefits for the community, then we are going
> to have to do it over *many* release cycles (at least 3, I would guess,
> maybe more).
>
>
>>
>> It would also probably also mean the params/ headers would be slightly
>> more complex, and would need to be something like:
>>
>> #include "params/x86/TLB.hh"
>>
>> instead of:
>>
>> #include "params/X86TLB.hh"
>>
>>
>> What do folks think?
>>
>> Gabe
>> ___
>> 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 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]: misc: Add some changes to .git-blame-ignore-revs.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/49243 ) Change subject: misc: Add some changes to .git-blame-ignore-revs. .. misc: Add some changes to .git-blame-ignore-revs. Change-Id: I64656b05526a58de914c73f4ca94cc52fa3bf324 --- M .git-blame-ignore-revs 1 file changed, 3 insertions(+), 0 deletions(-) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index b94899f..6dcf591 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -22,3 +22,6 @@ c3bd8eb1214cbebbc92c7958b80aa06913bce3ba 488ded0c8d9e43deef531ad174937982b41f8e4b 26e888965d08486aeed7ebb3ef934ceb1a38cd6f +451f731748dee4bb1b9506edbd34e2f015d656f2 +f201a95e2a1d3dbe54f4f55e375c1161ee7f4dc7 +a759336a088cce59fa55ba4d9af6e6e04a25e537 -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49243 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: I64656b05526a58de914c73f4ca94cc52fa3bf324 Gerrit-Change-Number: 49243 Gerrit-PatchSet: 1 Gerrit-Owner: Gabe Black 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
[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Fix style in utility.hh.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/49144 )
Change subject: arch-arm: Fix style in utility.hh.
..
arch-arm: Fix style in utility.hh.
Change-Id: I66262e63695680f5638ef057be05274445ba38ac
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49144
Maintainer: Bobby R. Bruce
Tested-by: kokoro
Reviewed-by: Gabe Black
---
M src/arch/arm/utility.cc
M src/arch/arm/utility.hh
2 files changed, 149 insertions(+), 173 deletions(-)
Approvals:
Gabe Black: Looks good to me, approved
Bobby R. Bruce: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/utility.cc b/src/arch/arm/utility.cc
index bd1af44..b716ba9 100644
--- a/src/arch/arm/utility.cc
+++ b/src/arch/arm/utility.cc
@@ -305,7 +305,7 @@
std::pair
ELUsingAArch32K(ThreadContext *tc, ExceptionLevel el)
{
-bool secure = isSecureBelowEL3(tc);
+bool secure = isSecureBelowEL3(tc);
return ELStateUsingAArch32K(tc, el, secure);
}
@@ -366,8 +366,8 @@
aarch32 = (cpsr.width == 1);
} else {
known = true;
-aarch32 = (aarch32_below_el3 && el != EL3)
- || (aarch32_at_el1 && (el == EL0 || el == EL1) );
+aarch32 = (aarch32_below_el3 && el != EL3) ||
+(aarch32_at_el1 && (el == EL0 || el == EL1) );
}
}
@@ -510,13 +510,13 @@
mcrMrc15TrapToHyp(const MiscRegIndex miscReg, ThreadContext *tc, uint32_t
iss,
ExceptionClass *ec)
{
-boolisRead;
-uint32_tcrm;
+bool isRead;
+uint32_t crm;
IntRegIndex rt;
-uint32_tcrn;
-uint32_topc1;
-uint32_topc2;
-booltrapToHype = false;
+uint32_t crn;
+uint32_t opc1;
+uint32_t opc2;
+bool trapToHype = false;
const CPSR cpsr = tc->readMiscReg(MISCREG_CPSR);
const HCR hcr = tc->readMiscReg(MISCREG_HCR);
@@ -531,8 +531,8 @@
trapToHype |= hdcr.tpm && (crn == 9) && (crm >= 12);
trapToHype |= hcr.tidcp && (
((crn == 9) && ((crm <= 2) || ((crm >= 5) && (crm <= 8 ||
-((crn == 10) && ((crm <= 1) || (crm == 4) || (crm == 8))) ||
-((crn == 11) && ((crm <= 8) || (crm == 15))) );
+((crn == 10) && ((crm <= 1) || (crm == 4) || (crm == 8))) ||
+((crn == 11) && ((crm <= 8) || (crm == 15;
if (!trapToHype) {
switch (unflattenMiscReg(miscReg)) {
@@ -667,13 +667,13 @@
mcrMrc14TrapToHyp(const MiscRegIndex miscReg, HCR hcr, CPSR cpsr, SCR scr,
HDCR hdcr, HSTR hstr, HCPTR hcptr, uint32_t iss)
{
-boolisRead;
-uint32_tcrm;
+bool isRead;
+uint32_t crm;
IntRegIndex rt;
-uint32_tcrn;
-uint32_topc1;
-uint32_topc2;
-booltrapToHype = false;
+uint32_t crn;
+uint32_t opc1;
+uint32_t opc2;
+bool trapToHype = false;
if (!inSecureState(scr, cpsr) && (cpsr.mode != MODE_HYP)) {
mcrMrcIssExtract(iss, isRead, crm, rt, crn, opc1, opc2);
@@ -727,13 +727,13 @@
mcrrMrrc15TrapToHyp(const MiscRegIndex miscReg, ThreadContext *tc,
uint32_t iss, ExceptionClass *ec)
{
-uint32_tcrm;
+uint32_t crm;
IntRegIndex rt;
-uint32_tcrn;
-uint32_topc1;
-uint32_topc2;
-boolisRead;
-booltrapToHype = false;
+uint32_t crn;
+uint32_t opc1;
+uint32_t opc2;
+bool isRead;
+bool trapToHype = false;
const CPSR cpsr = tc->readMiscReg(MISCREG_CPSR);
const HCR hcr = tc->readMiscReg(MISCREG_HCR);
@@ -745,7 +745,7 @@
// the moment because we only need one field, which overlaps with
the
// mcrmrc layout
mcrMrcIssExtract(iss, isRead, crm, rt, crn, opc1, opc2);
-trapToHype = ((uint32_t) hstr) & (1 << crm);
+trapToHype = ((uint32_t)hstr) & (1 << crm);
if (!trapToHype) {
switch (unflattenMiscReg(miscReg)) {
@@ -771,8 +771,9 @@
// CNTFRQ may be trapped only on reads
// CNTPCT and CNTVCT are read-only
if (MISCREG_CNTFRQ <= miscReg && miscReg <= MISCREG_CNTVCT
&&
-!isRead)
+!isRead) {
break;
+}
trapToHype = isGenericTimerHypTrap(miscReg, tc, ec);
break;
// No default action needed
@@ -989,8 +990,8 @@
{
const HCR hcr = tc->readMiscReg(MISCREG_HCR_EL2);
bool trap_cond_0 = condGenericTimerPhysEL1SystemAccessTrapEL2(miscReg,
tc);
-bool trap_cond_1 =
condGenericTimerCommonEL1SystemAccessTrapEL2(miscReg,
-tc);
+bool trap_cond_1 = condGenericTimerCommonEL1SystemAccessTrapEL2(
+miscReg, tc);
switch (miscReg) {
[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Stop printing a source operand for the monitor instruction.
Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/49225 ) Change subject: arch-x86: Stop printing a source operand for the monitor instruction. .. arch-x86: Stop printing a source operand for the monitor instruction. Like CPUID, all operands for this instruction are implicit and are not written out when putting it into assembly. Stop printing a source operand when generating disassembly in gem5. Change-Id: I95898afdd9101ad393b3aace99536db602752a64 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49225 Maintainer: Gabe Black Maintainer: Matt Sinclair Tested-by: kokoro Reviewed-by: Matt Sinclair --- M src/arch/x86/isa/formats/monitor_mwait.isa 1 file changed, 0 insertions(+), 2 deletions(-) Approvals: Matt Sinclair: Looks good to me, approved; Looks good to me, approved Gabe Black: Looks good to me, approved kokoro: Regressions pass diff --git a/src/arch/x86/isa/formats/monitor_mwait.isa b/src/arch/x86/isa/formats/monitor_mwait.isa index 49a0158..59baf3c 100644 --- a/src/arch/x86/isa/formats/monitor_mwait.isa +++ b/src/arch/x86/isa/formats/monitor_mwait.isa @@ -27,8 +27,6 @@ std::stringstream response; printMnemonic(response, mnemonic); -ccprintf(response, " "); -printReg(response, srcRegIdx(0), machInst.opSize); return response.str(); } }}; 1 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/+/49225 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: I95898afdd9101ad393b3aace99536db602752a64 Gerrit-Change-Number: 49225 Gerrit-PatchSet: 3 Gerrit-Owner: Gabe Black Gerrit-Reviewer: Gabe Black 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]: arch-x86: Stop printing a source register for CPUID.
Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/49224 ) Change subject: arch-x86: Stop printing a source register for CPUID. .. arch-x86: Stop printing a source register for CPUID. When disassembling the CPUID instruction, we were (for some reason) printing a source register after the mneomic. The registers CPUID uses are fixed, and it has no operands when written in assembly. Change-Id: I5aee7f7a93cb84977cfe1fe387c5184aa740bc0e Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49224 Maintainer: Gabe Black Maintainer: Matt Sinclair Tested-by: kokoro Reviewed-by: Matt Sinclair --- M src/arch/x86/isa/formats/cpuid.isa 1 file changed, 0 insertions(+), 2 deletions(-) Approvals: Matt Sinclair: Looks good to me, approved; Looks good to me, approved Gabe Black: Looks good to me, approved kokoro: Regressions pass diff --git a/src/arch/x86/isa/formats/cpuid.isa b/src/arch/x86/isa/formats/cpuid.isa index b68b19e..3a69e4c 100644 --- a/src/arch/x86/isa/formats/cpuid.isa +++ b/src/arch/x86/isa/formats/cpuid.isa @@ -60,8 +60,6 @@ std::stringstream response; printMnemonic(response, mnemonic); -ccprintf(response, " "); -printReg(response, srcRegIdx(0), machInst.opSize); return response.str(); } }}; 1 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/+/49224 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: I5aee7f7a93cb84977cfe1fe387c5184aa740bc0e Gerrit-Change-Number: 49224 Gerrit-PatchSet: 3 Gerrit-Owner: Gabe Black Gerrit-Reviewer: Gabe Black 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] Re: Running clang-format
Hey Matt, I forgot to include a couple of important pieces of information :). 1. There are places that we want to disable clang format. It would be impossible to find these places in one big formatting change. 2. The big change will cause huge headaches merging. The benefits of having a more compliant codebase are probably not worth those headaches. To answer your question, I envision only two small things changing once 49063 is merged. 1. When a reviewer says "that doesn't match the style" the patch submitter can run `clang-format ` and say "now it does". There will be nothing to argue about. 2. If someone notices a file that doesn't have the right style, rather than fixing a subset of the file, they can run `clang-format` and fix the whole thing at once without worrying about not following the style guide perfectly. We would *not* enforce the style across the board automatically (at least not for now). We can have a separate discussion on if/when to replace style.py with clang-format. My opinion is that we should do it the same way we run the current checker, which is we only check the lines that have changed. But this is getting ahead of what I'm currently proposing :). Jason On Fri, Aug 13, 2021 at 11:13 AM Matt Sinclair wrote: > What does this practically mean though, Jason? Does it mean now when > somebody makes the next change to a file foo (after 49063 is checked in), > that all of the formatting and spacing changes will be included in that > patch, since it would need to be run through the checker? Perhaps that's a > short-term problem, but it seems to me like that will lead to some > confusing patches where spacing is ~90% of what is being checked in. In > comparison, I'd rather have 49064 that makes all the spacing changes at > once, but doesn't pollute subsequent patches. Maybe there is a third > option I'm not getting though? > > Matt > > On Fri, Aug 13, 2021 at 12:45 PM Jason Lowe-Power via gem5-dev < > gem5-dev@gem5.org> wrote: > >> Hi all, >> >> I have an updated proposal. >> >> FYI: It looks like since the changes are marked as WIP they don't send >> emails when they are updated. >> >> For those of you who haven't seen the changesets, you can see the >> discussion (and some great ideas from others!) at the following links: >> >> Clang-format file: >> https://gem5-review.googlesource.com/c/public/gem5/+/49063 >> The giant changeset: >> https://gem5-review.googlesource.com/c/public/gem5/+/49064/ >> >> New proposal: >> --- >> >> Daniel had a great suggestion to apply this change "little by little" and >> only apply one formatting rule at a time. Unfortunately, I don't think >> clang-format supports this. However, this fact, combined with the number of >> places where gem5's current style is underspecified got me thinking. >> >> Instead of applying this format for the entire repo at once, I suggest we >> use the clang-format file as the definitive style guide for gem5. Rather >> than humans discussing an underspecified style format, whenever anything >> style related comes up, we can point to clang-format and say "that's the >> way we do it." In other words, let the machine decide. >> >> As previously mentioned, there's lots of places where our current style >> guide is under specified. Currently it states something like "If not >> specified here, we use the Google style". So, the clang-format file I >> created does the *exact same thing*. >> >> My goal is for the clang-format file to implement all of the differences >> from the Google style that we have, and then it falls back on Google for >> everything else. If you notice anywhere that this is wrong, please comment >> on the commit. >> >> So, to summarize: I propose committing 49036 and dropping 49064. Then, >> from now on, if anyone has a style comment on a file or updates the style >> in the file, rather than having a potentially flawed human make the >> changes, we can have a deterministic machine (clang-format, the program) >> make the changes. >> >> Any objections? :) >> >> Cheers, >> Jason >> >> >> On Thu, Aug 5, 2021 at 4:39 PM Jason Lowe-Power >> wrote: >> >>> Hi everyone, >>> >>> For fun(?), I have come up with a clang-format file that's pretty darn >>> close to our current style. >>> >>> However, the problem is that most of our files don't actually meet our >>> style guide! :'( So, running this clang-format generates an enormous >>> changeset. Skimming through the (100,000+) changes clang-format makes all >>> of them look "correct" to me. But I didn't check them all, just a few >>> spot-checks. >>> >>> I've put together a WIP commit as an example of what it looks like to >>> run clang-format on gem5. There are a few caveats, however. >>> >>> 1. BitUnion really confuses clang-format for two reasons: (1) We indent >>> the code inside the BitUntion macro. (2) there are no semicolons after the >>> macro calls for BitUnion and EndBitUntion. >>> 2. There are a few lines that go over 79 characters, but
[gem5-dev] Re: Running clang-format
Hi all,
I have an updated proposal.
FYI: It looks like since the changes are marked as WIP they don't send
emails when they are updated.
For those of you who haven't seen the changesets, you can see the
discussion (and some great ideas from others!) at the following links:
Clang-format file:
https://gem5-review.googlesource.com/c/public/gem5/+/49063
The giant changeset:
https://gem5-review.googlesource.com/c/public/gem5/+/49064/
New proposal:
---
Daniel had a great suggestion to apply this change "little by little" and
only apply one formatting rule at a time. Unfortunately, I don't think
clang-format supports this. However, this fact, combined with the number of
places where gem5's current style is underspecified got me thinking.
Instead of applying this format for the entire repo at once, I suggest we
use the clang-format file as the definitive style guide for gem5. Rather
than humans discussing an underspecified style format, whenever anything
style related comes up, we can point to clang-format and say "that's the
way we do it." In other words, let the machine decide.
As previously mentioned, there's lots of places where our current style
guide is under specified. Currently it states something like "If not
specified here, we use the Google style". So, the clang-format file I
created does the *exact same thing*.
My goal is for the clang-format file to implement all of the differences
from the Google style that we have, and then it falls back on Google for
everything else. If you notice anywhere that this is wrong, please comment
on the commit.
So, to summarize: I propose committing 49036 and dropping 49064. Then, from
now on, if anyone has a style comment on a file or updates the style in the
file, rather than having a potentially flawed human make the changes, we
can have a deterministic machine (clang-format, the program) make the
changes.
Any objections? :)
Cheers,
Jason
On Thu, Aug 5, 2021 at 4:39 PM Jason Lowe-Power wrote:
> Hi everyone,
>
> For fun(?), I have come up with a clang-format file that's pretty darn
> close to our current style.
>
> However, the problem is that most of our files don't actually meet our
> style guide! :'( So, running this clang-format generates an enormous
> changeset. Skimming through the (100,000+) changes clang-format makes all
> of them look "correct" to me. But I didn't check them all, just a few
> spot-checks.
>
> I've put together a WIP commit as an example of what it looks like to run
> clang-format on gem5. There are a few caveats, however.
>
> 1. BitUnion really confuses clang-format for two reasons: (1) We indent
> the code inside the BitUntion macro. (2) there are no semicolons after the
> macro calls for BitUnion and EndBitUntion.
> 2. There are a few lines that go over 79 characters, but they're easy to
> manually fix (it's only 4 files).
> 3. When I ran style.py on all files in src/ there were a huge number that
> had invalid sorting of includes. In at least one case, this causes compiler
> failures when I let style.py fix it.
> 4. I'm skipping sorting includes with clang-format right now. There is a
> way to specify the blocks that we define, but I don't want to dive into
> this right now. I'll create a Jira issue so we can come back to it.
>
> Finally, I'm using the following shell script to test things. This runs
> sed to tell clang-format to ignore the BitUnion blocks, runs clang-format,
> and then undoes the sed script.
>
> If this seems like something we want to continue with, then I'll commit
> these changes and we can add clang-format to the commit hooks.
>
> Clang-format file:
> https://gem5-review.googlesource.com/c/public/gem5/+/49063
> The giant changeset:
> https://gem5-review.googlesource.com/c/public/gem5/+/49064/
>
> The script I used (don't judge my sed ability :D):
> ```
> # Make clang-format ignore bitunions
> grep -r -l BitUnion src | xargs sed -i -e '/^ *BitUnion.*(.*)/i \
> // clang-format off' -e '/^ *EndBitUnion(.*)/a; \
> // clang-format on'
>
> # undo the change to the bitunion file
> git checkout src/base/bitunion.hh
>
> # Run clang format
> find src -name "*.cc" -or -name "*.hh" -exec clang-format -style=file -i
> {} \;
>
> # Remove clang-format off in BitUnion files
> grep -r -l BitUnion src | xargs sed -i -e '/^ *\/\/ clang-format.*$/d' -e
> '/^; *$/d'
> ```
>
> Cheers,
> Jason
>
___
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] Re: SCons SimObject() and m5.objects hierarchy
Hey Gabe,
Some responses inline below.
On Thu, Aug 12, 2021 at 9:37 PM Gabe Black via gem5-dev
wrote:
> Hey folks, here are some of my thoughts on the SimObject() mechanism in
> SCons, and the structure of the m5.objects package.
>
> Right now, when you put a SimObject('foo.py') line in a SConscript, that
> eventually leads to SCons importing foo.py (with some special machinery)
> and discovering through python what SimObjects, etc, are declared in that
> file. Those are collected into lists, and then each SimObject gets a
> params/Foo.hh, params/Bar.hh, etc.
>
> Then as far as m5.objects, each file declared with SimObject() ends up as
> a subpackage, with the stuff inside it available as you would expect, ie
> m5.objects.Foo.Bar and m5.objects.Foo.Foo. Also, all the SimObjects are
> artificially put under m5.objects so that:
>
> from m5.objects import *
>
> will get everything.
>
>
> First, having a single SimObject() which can produce an arbitrary number
> of actual SimObject subclasses, enums, etc, is problematic for SCons. SCons
> only does one pass, where you have to tell it all of what files come from
> what files, and then tell it to build. That means that SimObject() files
> *must* be read in during the build setup phase, since we have to figure out
> what files they would produce. It would be much more convenient to be able
> to delay that and do that as part of the actual build. That would let us,
> for example, build the marshal binary as a build product and not as part of
> setting up the build process itself.
>
> Second, something that I've brought up in the past is that having no
> hierarchy in m5.objects is not ideal. That leads to SimObjects having
> namespace information built into their name like "X86TLB", "ARMTLB", etc.
> This makes things more verbose, and hides structure. It also increases the
> likelihood of name collisions since everything is jammed into the same
> space.
>
I actually think the more verbose names are better. I really don't like
that we allow name "collisions" in different namespaces in C++ right now.
That confuses me/our users and also confuses IDEs.
>
>
> To help solve both of those problems, I'd like to propose changing
> SimObject() to have an __all__ type parameter which specifies what inside
> it to export, and what to export it as. It would also need to encode what
> type of entity something is as well, so we know if we need to define an
> enum, a SimObject, etc. Perhaps something like this:
>
> SimObject('foo.py', ['objects.x86.TLB', 'objects.x86.LocalApic',
> 'enum.MyFavoriteEnum'])
>
I like this idea. It will make other things like supporting documentation
and typing easier as well. However, I think we should not make significant
changes to the python module hierarchy.
>
> That would tell SCons what targets to set up for, and also how to
> structure the m5.objects package.
>
> import m5.objects.x86
>
> Foo.tlb = x86.TLB()
>
> class Example(SimObject):
> tlb = Param.x86.TLB(...)
>
>
> This would be a major departure from how imports have worked. It would
> mostly be mechanical when SimObjects are imported directly, but would make
> "from m5.objects import *" impossible.\\
>
I think this is a huge downside. Essentially, all of the config files would
have to be updated, and testing those is impossible in our current
implementation. If we really think that deprecating `from m5.object import
*` is going to provide large benefits for the community, then we are going
to have to do it over *many* release cycles (at least 3, I would guess,
maybe more).
>
> It would also probably also mean the params/ headers would be slightly
> more complex, and would need to be something like:
>
> #include "params/x86/TLB.hh"
>
> instead of:
>
> #include "params/X86TLB.hh"
>
>
> What do folks think?
>
> Gabe
> ___
> 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 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] Re: plan to eliminate the "zero register" concept at the CPU level
Unfortunately it looks like that mechanism I mentioned (for predicating reads/writes) makes some big assumptions which are not generally correct, they just happened to be correct in the situation that feature was initially used. I think I'm going to try to replace that mechanism with one which puts sentry indices into the source/dest arrays which other mechanisms will know to skip. I think I remember mentioning that in an email before? That new mechanism will both handle zero registers, and replace this predication mechanism with something less complicated and less broken. Gabe On Thu, Aug 12, 2021 at 6:21 PM Gabe Black wrote: > Hey folks. I was thinking about better ways to handle the "zero register" > in the CPUs. > > A plan I had a little while ago was something sort of like /dev/null, > where the zero register would be flattened to one thing if it was a source > which would always hold zero, and another thing as a destination which > would just collect junk. I was thinking about how to implement this, but it > occurred to me that this scheme would have unnecessary overhead. Even > though the source register would never be written to and would always have > the same value, it would have to actually take up a spot in the register > file, get plumbed through to instructions, etc, just to have a fixed value. > The destination register would also take up at least one slot, and on > systems which use renaming, may actually take up more slots if multiple > instructions that wrote to it were in flight at a time. > > Fortunately, there is a mechanism that was added for x86 for condition > code registers a good while ago which will help. The original idea was that > when there is a single "register" from an architectural view which is > actually made up of multiple real registers from an implementation point of > view, specifically the condition code portion of the flags register in x86, > you might read a register to build the original value of the composite > register, but then end up writing over all the bits that came from that > part. This mechanism lets you conditionalize whether you're actually going > to read from (or write to) a register, and leave it out of src/dest > register arrays and not actually call the accessors if not. > > So, my proposal is to use this mechanism to handle accesses to any zero > registers at the ISA description level, and remove all special handling > anywhere else. I expect this to be fairly painless and non-controversial, > but wanted to send this email to let people know what was going on just in > case, and also to provide context for the eventual reviews. I think I might > put this into a JIRA issue as well. > > Gabe > ___ 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]: arch-arm: Eliminate the INTREG_DUMMY integer register.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/49223 ) Change subject: arch-arm: Eliminate the INTREG_DUMMY integer register. .. arch-arm: Eliminate the INTREG_DUMMY integer register. This register index was for an integer register which was just something to return when the result should be thrown away. That's basically what the zero register is/could already be used for. Replace INTREG_DUMMY with INTREG_ZERO. Also, change the type used for an index in SVE memory instructions from IntRegIndex to RegIndex, since it's not actually storing an integer register index, and g++ gets upset when you try to compare one against a value which is out of range for that enum. Change-Id: Ibdc488a2b55162a5f9e3d355126f6c48a99272a9 --- M src/arch/arm/isa/insts/misc.isa M src/arch/arm/isa/templates/sve_mem.isa M src/arch/arm/regs/int.hh M src/arch/arm/utility.hh 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/arch/arm/isa/insts/misc.isa b/src/arch/arm/isa/insts/misc.isa index bbd183d..2add447 100644 --- a/src/arch/arm/isa/insts/misc.isa +++ b/src/arch/arm/isa/insts/misc.isa @@ -290,7 +290,7 @@ // you look at the generated C code you'll find that they are. // However this is safe as DecodedBankedIntReg (which is used // in operands.isa to get the index of DecodedBankedIntReg) -// will return INTREG_DUMMY if its not a valid integer +// will return INTREG_ZERO if its not a valid integer // register, so redirecting the write to somewhere we don't // care about. DecodedBankedIntReg = Op1; diff --git a/src/arch/arm/isa/templates/sve_mem.isa b/src/arch/arm/isa/templates/sve_mem.isa index 6a9ea7e..017ace8 100644 --- a/src/arch/arm/isa/templates/sve_mem.isa +++ b/src/arch/arm/isa/templates/sve_mem.isa @@ -890,7 +890,7 @@ typedef _Element Element; typedef _Element TPElem; -IntRegIndex dest; +RegIndex dest; IntRegIndex gp; IntRegIndex base; int64_t imm; @@ -904,7 +904,7 @@ public: %(class_name)s(const char* mnem, ExtMachInst machInst, -IntRegIndex _dest, IntRegIndex _gp, IntRegIndex _base, +RegIndex _dest, IntRegIndex _gp, IntRegIndex _base, int64_t _imm, uint8_t _numRegs, int _regIndex) : %(base_class)s(mnem, machInst, %(op_class)s), dest(_dest), gp(_gp), base(_base), imm(_imm), @@ -1167,7 +1167,7 @@ typedef _Element Element; typedef _Element TPElem; -IntRegIndex dest; +RegIndex dest; IntRegIndex gp; IntRegIndex base; IntRegIndex offset; @@ -1181,7 +1181,7 @@ public: %(class_name)s(const char* mnem, ExtMachInst machInst, -IntRegIndex _dest, IntRegIndex _gp, IntRegIndex _base, +RegIndex _dest, IntRegIndex _gp, IntRegIndex _base, IntRegIndex _offset, uint8_t _numRegs, int _regIndex) : %(base_class)s(mnem, machInst, %(op_class)s), dest(_dest), gp(_gp), base(_base), offset(_offset), diff --git a/src/arch/arm/regs/int.hh b/src/arch/arm/regs/int.hh index 30230e8..59f8a6d 100644 --- a/src/arch/arm/regs/int.hh +++ b/src/arch/arm/regs/int.hh @@ -127,7 +127,6 @@ INTREG_UREG0, INTREG_UREG1, INTREG_UREG2, -INTREG_DUMMY, // Dummy reg used to throw away int reg results INTREG_SP0, INTREG_SP1, diff --git a/src/arch/arm/utility.hh b/src/arch/arm/utility.hh index 9b2496e..bf6bae5 100644 --- a/src/arch/arm/utility.hh +++ b/src/arch/arm/utility.hh @@ -396,7 +396,7 @@ bool validReg; validReg = decodeMrsMsrBankedReg(sysM, r, isIntReg, regIdx, 0, 0, 0, false); -return (validReg && isIntReg) ? regIdx : INTREG_DUMMY; +return (validReg && isIntReg) ? regIdx : INTREG_ZERO; } /** -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49223 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: Ibdc488a2b55162a5f9e3d355126f6c48a99272a9 Gerrit-Change-Number: 49223 Gerrit-PatchSet: 1 Gerrit-Owner: Gabe Black 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
[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Stop printing a source operand for the monitor instruction.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/49225 ) Change subject: arch-x86: Stop printing a source operand for the monitor instruction. .. arch-x86: Stop printing a source operand for the monitor instruction. Like CPUID, all operands for this instruction are implicit and are not written out when putting it into assembly. Stop printing a source operand when generating disassembly in gem5. Change-Id: I95898afdd9101ad393b3aace99536db602752a64 --- M src/arch/x86/isa/formats/monitor_mwait.isa 1 file changed, 0 insertions(+), 2 deletions(-) diff --git a/src/arch/x86/isa/formats/monitor_mwait.isa b/src/arch/x86/isa/formats/monitor_mwait.isa index 49a0158..59baf3c 100644 --- a/src/arch/x86/isa/formats/monitor_mwait.isa +++ b/src/arch/x86/isa/formats/monitor_mwait.isa @@ -27,8 +27,6 @@ std::stringstream response; printMnemonic(response, mnemonic); -ccprintf(response, " "); -printReg(response, srcRegIdx(0), machInst.opSize); return response.str(); } }}; -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49225 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: I95898afdd9101ad393b3aace99536db602752a64 Gerrit-Change-Number: 49225 Gerrit-PatchSet: 1 Gerrit-Owner: Gabe Black 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
[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Stop printing a source register for CPUID.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/49224 ) Change subject: arch-x86: Stop printing a source register for CPUID. .. arch-x86: Stop printing a source register for CPUID. When disassembling the CPUID instruction, we were (for some reason) printing a source register after the mneomic. The registers CPUID uses are fixed, and it has no operands when written in assembly. Change-Id: I5aee7f7a93cb84977cfe1fe387c5184aa740bc0e --- M src/arch/x86/isa/formats/cpuid.isa 1 file changed, 0 insertions(+), 2 deletions(-) diff --git a/src/arch/x86/isa/formats/cpuid.isa b/src/arch/x86/isa/formats/cpuid.isa index b68b19e..3a69e4c 100644 --- a/src/arch/x86/isa/formats/cpuid.isa +++ b/src/arch/x86/isa/formats/cpuid.isa @@ -60,8 +60,6 @@ std::stringstream response; printMnemonic(response, mnemonic); -ccprintf(response, " "); -printReg(response, srcRegIdx(0), machInst.opSize); return response.str(); } }}; -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49224 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: I5aee7f7a93cb84977cfe1fe387c5184aa740bc0e Gerrit-Change-Number: 49224 Gerrit-PatchSet: 1 Gerrit-Owner: Gabe Black 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
