[gem5-dev] Change in gem5/gem5[develop]: tests: Stop using memcmp in the circlebuf test.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/38895 )
Change subject: tests: Stop using memcmp in the circlebuf test.
..
tests: Stop using memcmp in the circlebuf test.
Comparing arrays with memcmp is fairly easy to do and will correctly
identify when a value is incorrect, but gtest doesn't know what
comparison actually happened and can't print any diagnostic information
to help the person running the test determine what went wrong.
Unfortunately, gtest is also slightly too smart and also not smart
enough to make it easy to compare the contents of sub-arrays with each
other. The thing you're checking the value of *must* be an array with a
well defined size (not a pointer), and the size *must* exactly match the
number of elements it expects to find.
One fairly clean way to get around this problem would be to use the new
std::span type introduced in c++20 which lets you refer to a sub-section
of another container in place, adjusting indexing, sizing, etc as
needed. Unfortunately since we only require up to c++-14 currently we
can't use that type.
Instead, we can create vectors which hold copies of the required data.
This is suboptimal since it means we're copying around data which
doesn't really need to be copied, but it means that the templates in
gtest will get a type they can handle, and the sizes will match like it
expects them to. Since the number of checks/copies is still small, the
overhead should be trivial in practice.
A helper function, subArr, has been added to help keep things fairly
clutter free.
Change-Id: I9f88c583a6a742346b177dba7cae791824b65942
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38895
Maintainer: Gabe Black
Tested-by: kokoro
Reviewed-by: Daniel Carvalho
---
M src/base/circlebuf.test.cc
1 file changed, 22 insertions(+), 8 deletions(-)
Approvals:
Daniel Carvalho: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/base/circlebuf.test.cc b/src/base/circlebuf.test.cc
index a2babc5..6b81b61 100644
--- a/src/base/circlebuf.test.cc
+++ b/src/base/circlebuf.test.cc
@@ -35,15 +35,29 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
+#include
#include
+#include
+
#include "base/circlebuf.hh"
+using testing::ElementsAreArray;
+
const char data[] = {
0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
};
+// A better way to implement this would be with std::span, but that is only
+// available starting in c++20.
+template
+std::vector
+subArr(T *arr, int size, int offset=0)
+{
+return std::vector(arr + offset, arr + offset + size);
+}
+
// Basic non-overflow functionality
TEST(CircleBufTest, BasicReadWriteNoOverflow)
{
@@ -54,14 +68,14 @@
buf.write(data, 8);
EXPECT_EQ(buf.size(), 8);
buf.peek(foo, 8);
-EXPECT_EQ(memcmp(foo, data, 8), 0);
+EXPECT_THAT(subArr(foo, 8), ElementsAreArray(data, 8));
// Read 2
buf.read(foo, 2);
-EXPECT_EQ(memcmp(foo, data, 2), 0);
+EXPECT_THAT(subArr(foo, 2), ElementsAreArray(data, 2));
EXPECT_EQ(buf.size(), 6);
buf.read(foo, 6);
-EXPECT_EQ(memcmp(foo, data + 2, 6), 0);
+EXPECT_THAT(subArr(foo, 6), ElementsAreArray(data + 2, 6));
EXPECT_EQ(buf.size(), 0);
}
@@ -74,7 +88,7 @@
buf.write(data, 16);
EXPECT_EQ(buf.size(), 8);
buf.peek(foo, 8);
-EXPECT_EQ(memcmp(data + 8, foo, 8), 0);
+EXPECT_THAT(subArr(foo, 8), ElementsAreArray(data + 8, 8));
}
@@ -89,8 +103,8 @@
buf.write(data + 8, 6);
EXPECT_EQ(buf.size(), 8);
buf.peek(foo, 8);
-EXPECT_EQ(memcmp(data + 4, foo, 2), 0);
-EXPECT_EQ(memcmp(data + 8, foo + 2, 6), 0);
+EXPECT_THAT(subArr(foo, 2), ElementsAreArray(data + 4, 2));
+EXPECT_THAT(subArr(foo, 6, 2), ElementsAreArray(data + 8, 6));
}
// Pointer wrap around
@@ -110,9 +124,9 @@
// Normalized: _start == 2, _stop = 4
buf.read(foo + 4, 6);
EXPECT_EQ(buf.size(), 2);
-EXPECT_EQ(memcmp(data, foo, 10), 0);
+EXPECT_THAT(subArr(foo, 10), ElementsAreArray(data, 10));
// Normalized: _start == 4, _stop = 4
buf.read(foo + 10, 2);
EXPECT_EQ(buf.size(), 0);
-EXPECT_EQ(memcmp(data, foo, 12), 0);
+EXPECT_THAT(subArr(foo, 12), ElementsAreArray(data, 12));
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38895
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: I9f88c583a6a742346b177dba7cae791824b65942
Gerrit-Change-Number: 38895
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black
Gerrit-Reviewer: Bobby R. Bruce
Gerrit-Reviewer: Daniel Carvalho
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: kokoro
Gerrit-MessageType: merged
___
gem5-d
[gem5-dev] Change in gem5/gem5[develop]: arch,cpu: Move TheISA::copyRegs to TheISA::ISA::copyRegsFrom.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39336 )
Change subject: arch,cpu: Move TheISA::copyRegs to
TheISA::ISA::copyRegsFrom.
..
arch,cpu: Move TheISA::copyRegs to TheISA::ISA::copyRegsFrom.
This eliminates the last externally used function in arch/utility.hh.
Change-Id: I7f402b0303e2758762e19d69f3bed37262cc9289
---
M src/arch/arm/isa.cc
M src/arch/arm/isa.hh
M src/arch/arm/linux/linux.hh
M src/arch/arm/utility.cc
M src/arch/arm/utility.hh
M src/arch/generic/isa.hh
M src/arch/mips/isa.cc
M src/arch/mips/isa.hh
M src/arch/mips/utility.cc
M src/arch/power/SConscript
M src/arch/power/isa.cc
M src/arch/power/isa.hh
M src/arch/power/pagetable.hh
D src/arch/power/utility.cc
M src/arch/power/utility.hh
M src/arch/riscv/isa.cc
M src/arch/riscv/isa.hh
M src/arch/riscv/linux/linux.hh
M src/arch/riscv/utility.hh
M src/arch/sparc/SConscript
M src/arch/sparc/isa.cc
M src/arch/sparc/isa.hh
M src/arch/sparc/linux/linux.hh
D src/arch/sparc/utility.cc
M src/arch/sparc/utility.hh
M src/arch/x86/isa.cc
M src/arch/x86/isa.hh
M src/arch/x86/linux/linux.hh
M src/arch/x86/utility.cc
M src/arch/x86/utility.hh
M src/cpu/o3/thread_context_impl.hh
M src/cpu/simple_thread.cc
32 files changed, 344 insertions(+), 440 deletions(-)
diff --git a/src/arch/arm/isa.cc b/src/arch/arm/isa.cc
index f4fabc1..db00b69 100644
--- a/src/arch/arm/isa.cc
+++ b/src/arch/arm/isa.cc
@@ -481,6 +481,52 @@
setupThreadContext();
}
+static void
+copyVecRegs(ThreadContext *src, ThreadContext *dest)
+{
+auto src_mode = RenameMode::mode(src->pcState());
+
+// The way vector registers are copied (VecReg vs VecElem) is relevant
+// in the O3 model only.
+if (src_mode == Enums::Full) {
+for (auto idx = 0; idx < NumVecRegs; idx++)
+dest->setVecRegFlat(idx, src->readVecRegFlat(idx));
+} else {
+for (auto idx = 0; idx < NumVecRegs; idx++)
+for (auto elem_idx = 0; elem_idx < NumVecElemPerVecReg;
elem_idx++)
+dest->setVecElemFlat(
+idx, elem_idx, src->readVecElemFlat(idx, elem_idx));
+}
+}
+
+void
+ISA::copyRegsFrom(ThreadContext *src)
+{
+for (int i = 0; i < NumIntRegs; i++)
+tc->setIntRegFlat(i, src->readIntRegFlat(i));
+
+for (int i = 0; i < NumFloatRegs; i++)
+tc->setFloatRegFlat(i, src->readFloatRegFlat(i));
+
+for (int i = 0; i < NumCCRegs; i++)
+tc->setCCReg(i, src->readCCReg(i));
+
+for (int i = 0; i < NumMiscRegs; i++)
+tc->setMiscRegNoEffect(i, src->readMiscRegNoEffect(i));
+
+copyVecRegs(src, tc);
+
+// setMiscReg "with effect" will set the misc register mapping
correctly.
+// e.g. updateRegMap(val)
+tc->setMiscReg(MISCREG_CPSR, src->readMiscRegNoEffect(MISCREG_CPSR));
+
+// Copy over the PC State
+tc->pcState(src->pcState());
+
+// Invalidate the tlb misc register cache
+static_cast(tc->getMMUPtr())->invalidateMiscReg();
+}
+
RegVal
ISA::readMiscRegNoEffect(int misc_reg) const
{
diff --git a/src/arch/arm/isa.hh b/src/arch/arm/isa.hh
index d350378..ff885a8 100644
--- a/src/arch/arm/isa.hh
+++ b/src/arch/arm/isa.hh
@@ -900,6 +900,8 @@
CPSR cpsr = miscRegs[MISCREG_CPSR];
return ArmISA::inUserMode(cpsr);
}
+
+void copyRegsFrom(ThreadContext *src) override;
};
}
diff --git a/src/arch/arm/linux/linux.hh b/src/arch/arm/linux/linux.hh
index ad773cd..9b824fc 100644
--- a/src/arch/arm/linux/linux.hh
+++ b/src/arch/arm/linux/linux.hh
@@ -57,7 +57,7 @@
ThreadContext *ptc, ThreadContext *ctc,
uint64_t stack, uint64_t tls)
{
-ArmISA::copyRegs(ptc, ctc);
+ctc->getIsaPtr()->copyRegsFrom(ptc);
if (flags & TGT_CLONE_SETTLS) {
/* TPIDR_EL0 is architecturally mapped to TPIDRURW, so
diff --git a/src/arch/arm/utility.cc b/src/arch/arm/utility.cc
index 31408f6..688ee8d 100644
--- a/src/arch/arm/utility.cc
+++ b/src/arch/arm/utility.cc
@@ -53,52 +53,6 @@
namespace ArmISA
{
-static void
-copyVecRegs(ThreadContext *src, ThreadContext *dest)
-{
-auto src_mode = RenameMode::mode(src->pcState());
-
-// The way vector registers are copied (VecReg vs VecElem) is relevant
-// in the O3 model only.
-if (src_mode == Enums::Full) {
-for (auto idx = 0; idx < NumVecRegs; idx++)
-dest->setVecRegFlat(idx, src->readVecRegFlat(idx));
-} else {
-for (auto idx = 0; idx < NumVecRegs; idx++)
-for (auto elem_idx = 0; elem_idx < NumVecElemPerVecReg;
elem_idx++)
-dest->setVecElemFlat(
-idx, elem_idx, src->readVecElemFlat(idx, elem_idx));
-}
-}
-
-void
-copyRegs(ThreadContext *src, ThreadContext *dest)
-{
-for (int i = 0; i < NumIntRegs; i++)
-dest->setIntRegFlat(i, src->readIntRegFlat(i));
-
-for (int i = 0; i < NumFloatRegs; i++
[gem5-dev] Change in gem5/gem5[develop]: arch,cpu,kern,sim: Eliminate the utility.hh switching header.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39337 )
Change subject: arch,cpu,kern,sim: Eliminate the utility.hh switching
header.
..
arch,cpu,kern,sim: Eliminate the utility.hh switching header.
This header is no longer used. Remove the places where it's included,
and stop generating it. Also eliminate the now empty SPARC and Power
versions of the header.
Change-Id: I6ee66d39bc0218d1d9b9b7db3b350134ef03251d
---
M src/arch/SConscript
M src/arch/power/isa/includes.isa
M src/arch/power/pagetable.hh
M src/arch/power/tlb.cc
M src/arch/power/tlb.hh
D src/arch/power/utility.hh
M src/arch/sparc/linux/linux.hh
D src/arch/sparc/utility.hh
M src/cpu/base_dyn_inst.hh
M src/cpu/exetrace.cc
M src/cpu/kvm/base.cc
M src/cpu/minor/cpu.cc
M src/cpu/minor/fetch2.cc
M src/cpu/o3/commit_impl.hh
M src/cpu/o3/fetch.hh
M src/cpu/o3/fetch_impl.hh
M src/cpu/o3/iew_impl.hh
M src/cpu/pred/bpred_unit.cc
M src/cpu/simple/atomic.cc
M src/cpu/simple/timing.cc
M src/cpu/simple_thread.cc
M src/kern/freebsd/events.cc
M src/kern/linux/events.cc
M src/sim/syscall_emul.cc
M src/sim/syscall_emul.hh
M src/sim/system.cc
26 files changed, 0 insertions(+), 91 deletions(-)
diff --git a/src/arch/SConscript b/src/arch/SConscript
index b97881d..5e27d4e 100644
--- a/src/arch/SConscript
+++ b/src/arch/SConscript
@@ -64,7 +64,6 @@
registers.hh
remote_gdb.hh
types.hh
-utility.hh
'''),
env.subst('${TARGET_ISA}'))
diff --git a/src/arch/power/isa/includes.isa
b/src/arch/power/isa/includes.isa
index ac7756c..f3e0f7f 100644
--- a/src/arch/power/isa/includes.isa
+++ b/src/arch/power/isa/includes.isa
@@ -59,7 +59,6 @@
#include "arch/power/decoder.hh"
#include "arch/power/faults.hh"
#include "arch/power/isa_traits.hh"
-#include "arch/power/utility.hh"
#include "base/loader/symtab.hh"
#include "base/cprintf.hh"
#include "cpu/thread_context.hh"
@@ -76,7 +75,6 @@
#include "arch/generic/memhelpers.hh"
#include "arch/power/faults.hh"
#include "arch/power/isa_traits.hh"
-#include "arch/power/utility.hh"
#include "base/condcodes.hh"
#include "cpu/base.hh"
#include "cpu/exetrace.hh"
diff --git a/src/arch/power/pagetable.hh b/src/arch/power/pagetable.hh
index 1f0ec4c..f193987 100644
--- a/src/arch/power/pagetable.hh
+++ b/src/arch/power/pagetable.hh
@@ -33,7 +33,6 @@
#define __ARCH_POWER_PAGETABLE_H__
#include "arch/power/isa_traits.hh"
-#include "arch/power/utility.hh"
#include "sim/serialize.hh"
namespace PowerISA
diff --git a/src/arch/power/tlb.cc b/src/arch/power/tlb.cc
index dd3b50a..f67baa4 100644
--- a/src/arch/power/tlb.cc
+++ b/src/arch/power/tlb.cc
@@ -36,7 +36,6 @@
#include "arch/power/faults.hh"
#include "arch/power/pagetable.hh"
-#include "arch/power/utility.hh"
#include "base/inifile.hh"
#include "base/str.hh"
#include "base/trace.hh"
diff --git a/src/arch/power/tlb.hh b/src/arch/power/tlb.hh
index a9653e6..7dcc3c0 100644
--- a/src/arch/power/tlb.hh
+++ b/src/arch/power/tlb.hh
@@ -37,7 +37,6 @@
#include "arch/generic/tlb.hh"
#include "arch/power/isa_traits.hh"
#include "arch/power/pagetable.hh"
-#include "arch/power/utility.hh"
#include "base/statistics.hh"
#include "mem/request.hh"
#include "params/PowerTLB.hh"
diff --git a/src/arch/power/utility.hh b/src/arch/power/utility.hh
deleted file mode 100644
index dfe3d06..000
--- a/src/arch/power/utility.hh
+++ /dev/null
@@ -1,34 +0,0 @@
-/*
- * Copyright (c) 2003-2005 The Regents of The University of Michigan
- * Copyright (c) 2007-2008 The Florida State University
- * Copyright (c) 2009 The University of Edinburgh
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met: redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer;
- * redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution;
- * neither the name of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUS
[gem5-dev] Change in gem5/gem5[develop]: arch,cpu: Collapse away TheISA::advancePC.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39335 )
Change subject: arch,cpu: Collapse away TheISA::advancePC.
..
arch,cpu: Collapse away TheISA::advancePC.
In most ISAs except MIPS and Power, this was implemented as
inst->advancePC(). It works just fine to call this function all the
time, but the idea had originally been that for ISAs which could simply
advance the PC using the PC itself, they could save the virtual function
call. Since the only ISAs which could skip the call were MIPS and Power,
and neither is at the point where that level of performance tuning
matters, this function can be collapsed with little downside.
If this turns out to be a performance bottleneck in the future, the way
the PC is managed could be revisited to see if we can factor out this
trip to the instruction object in the first place.
Change-Id: I533d1ad316e5c936466c529b7f1238a9ab87bd1c
---
M src/arch/arm/utility.hh
M src/arch/mips/utility.hh
M src/arch/power/utility.hh
M src/arch/riscv/faults.cc
M src/arch/riscv/utility.hh
M src/arch/sparc/utility.hh
M src/arch/x86/utility.hh
M src/cpu/base_dyn_inst.hh
M src/cpu/checker/cpu_impl.hh
M src/cpu/minor/execute.cc
M src/cpu/minor/fetch2.cc
M src/cpu/o3/commit_impl.hh
M src/cpu/o3/fetch_impl.hh
M src/cpu/o3/iew_impl.hh
M src/cpu/pred/bpred_unit.cc
M src/cpu/simple/base.cc
16 files changed, 13 insertions(+), 54 deletions(-)
diff --git a/src/arch/arm/utility.hh b/src/arch/arm/utility.hh
index 853643b..52c6ba0 100644
--- a/src/arch/arm/utility.hh
+++ b/src/arch/arm/utility.hh
@@ -374,12 +374,6 @@
bool SPAlignmentCheckEnabled(ThreadContext* tc);
-inline void
-advancePC(PCState &pc, const StaticInstPtr &inst)
-{
-inst->advancePC(pc);
-}
-
Addr truncPage(Addr addr);
Addr roundPage(Addr addr);
diff --git a/src/arch/mips/utility.hh b/src/arch/mips/utility.hh
index 1804680..f86e93a 100644
--- a/src/arch/mips/utility.hh
+++ b/src/arch/mips/utility.hh
@@ -72,14 +72,6 @@
return (addr + PageBytes - 1) & ~(PageBytes - 1);
}
-void copyRegs(ThreadContext *src, ThreadContext *dest);
-
-inline void
-advancePC(PCState &pc, const StaticInstPtr &inst)
-{
-pc.advance();
-}
-
};
diff --git a/src/arch/power/utility.hh b/src/arch/power/utility.hh
index 03df7fb..13183f1 100644
--- a/src/arch/power/utility.hh
+++ b/src/arch/power/utility.hh
@@ -39,12 +39,6 @@
void copyRegs(ThreadContext *src, ThreadContext *dest);
-inline void
-advancePC(PCState &pc, const StaticInstPtr &inst)
-{
-pc.advance();
-}
-
} // namespace PowerISA
diff --git a/src/arch/riscv/faults.cc b/src/arch/riscv/faults.cc
index 5ac2a3c..1c14f0b 100644
--- a/src/arch/riscv/faults.cc
+++ b/src/arch/riscv/faults.cc
@@ -140,7 +140,7 @@
pcState.set(addr);
} else {
invokeSE(tc, inst);
-advancePC(pcState, inst);
+inst->advancePC(pcState);
}
tc->pcState(pcState);
}
diff --git a/src/arch/riscv/utility.hh b/src/arch/riscv/utility.hh
index 3bbbd71..f033b2b 100644
--- a/src/arch/riscv/utility.hh
+++ b/src/arch/riscv/utility.hh
@@ -142,12 +142,6 @@
}
}
-inline void
-advancePC(PCState &pc, const StaticInstPtr &inst)
-{
-inst->advancePC(pc);
-}
-
} // namespace RiscvISA
#endif // __ARCH_RISCV_UTILITY_HH__
diff --git a/src/arch/sparc/utility.hh b/src/arch/sparc/utility.hh
index 792d7c4..78e5e25 100644
--- a/src/arch/sparc/utility.hh
+++ b/src/arch/sparc/utility.hh
@@ -43,12 +43,6 @@
void copyRegs(ThreadContext *src, ThreadContext *dest);
-inline void
-advancePC(PCState &pc, const StaticInstPtr &inst)
-{
-inst->advancePC(pc);
-}
-
} // namespace SparcISA
#endif
diff --git a/src/arch/x86/utility.hh b/src/arch/x86/utility.hh
index 9cd4e94..a572637 100644
--- a/src/arch/x86/utility.hh
+++ b/src/arch/x86/utility.hh
@@ -46,12 +46,6 @@
{
void copyRegs(ThreadContext *src, ThreadContext *dest);
-inline void
-advancePC(PCState &pc, const StaticInstPtr &inst)
-{
-inst->advancePC(pc);
-}
-
/**
* Reconstruct the rflags register from the internal gem5 register
* state.
diff --git a/src/cpu/base_dyn_inst.hh b/src/cpu/base_dyn_inst.hh
index 0b8a272..61230fc 100644
--- a/src/cpu/base_dyn_inst.hh
+++ b/src/cpu/base_dyn_inst.hh
@@ -504,7 +504,7 @@
mispredicted()
{
TheISA::PCState tempPC = pc;
-TheISA::advancePC(tempPC, staticInst);
+staticInst->advancePC(tempPC);
return !(tempPC == predPC);
}
diff --git a/src/cpu/checker/cpu_impl.hh b/src/cpu/checker/cpu_impl.hh
index 70dc451..8839903 100644
--- a/src/cpu/checker/cpu_impl.hh
+++ b/src/cpu/checker/cpu_impl.hh
@@ -75,7 +75,7 @@
if (curStaticInst->isLastMicroop())
curMacroStaticInst = StaticInst::nullStaticInstPtr;
TheISA::PCState pcState = thread->pcState();
-TheISA::advancePC(pcState, curStaticInst);
+curStaticInst->advancePC(
[gem5-dev] Change in gem5/gem5[develop]: base: Rename Flags::update as Flags::replace
Daniel Carvalho has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/38737 )
Change subject: base: Rename Flags::update as Flags::replace
..
base: Rename Flags::update as Flags::replace
The function name `update` is too generic. Given that
the expected functionality is to replace the selected
flag bits with the bits provided as parameters, rename
it as `replace`.
Change-Id: Ic7613ae09ecf9b92e31103b4e928192c07e9b640
Signed-off-by: Daniel R. Carvalho
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38737
Tested-by: kokoro
Reviewed-by: Jason Lowe-Power
Maintainer: Bobby R. Bruce
---
M src/base/flags.hh
M src/base/flags.test.cc
2 files changed, 4 insertions(+), 4 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Bobby R. Bruce: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/base/flags.hh b/src/base/flags.hh
index 6e82210..0544380 100644
--- a/src/base/flags.hh
+++ b/src/base/flags.hh
@@ -135,7 +135,7 @@
* @param mask Mask used to determine which bits are replaced.
*/
void
-update(Type flags, Type mask)
+replace(Type flags, Type mask)
{
_flags = (_flags & ~mask) | (flags & mask);
}
diff --git a/src/base/flags.test.cc b/src/base/flags.test.cc
index c45a7a8..08031b9 100644
--- a/src/base/flags.test.cc
+++ b/src/base/flags.test.cc
@@ -192,11 +192,11 @@
}
/**
- * Test updating a masked selection of bits. This means that bits of the
+ * Test replacing a masked selection of bits. This means that bits of the
* original value that match the mask will be replaced by the bits of
* the new value that match the mask.
*/
-TEST(FlagsTest, UpdateOverlapping)
+TEST(FlagsTest, ReplaceOverlapping)
{
const uint32_t value_a = (1 << 4) | (1 << 5) | (1 << 6);
const uint32_t value_b = (1 << 3) | (1 << 5) | (1 << 9);
@@ -207,6 +207,6 @@
// (1 << 10) is not set in both values, so it remains not set
const uint32_t result = (1 << 5) | (1 << 6) | (1 << 9);
Flags flags(value_a);
-flags.update(value_b, mask);
+flags.replace(value_b, mask);
ASSERT_EQ(result, uint32_t(flags));
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38737
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: Ic7613ae09ecf9b92e31103b4e928192c07e9b640
Gerrit-Change-Number: 38737
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Carvalho
Gerrit-Reviewer: Bobby R. Bruce
Gerrit-Reviewer: Daniel Carvalho
Gerrit-Reviewer: Jason Lowe-Power
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]: base: Add unit tests for flags.hh
Daniel Carvalho has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/38714 )
Change subject: base: Add unit tests for flags.hh
..
base: Add unit tests for flags.hh
Add a unit test for flags.hh.
Change-Id: I58135b5f2fc016c30e1b4535f3daf46a77e99aff
Signed-off-by: Daniel R. Carvalho
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38714
Reviewed-by: Bobby R. Bruce
Tested-by: kokoro
Maintainer: Bobby R. Bruce
---
M src/base/SConscript
A src/base/flags.test.cc
2 files changed, 246 insertions(+), 0 deletions(-)
Approvals:
Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/base/SConscript b/src/base/SConscript
index b3d9506..3937314 100644
--- a/src/base/SConscript
+++ b/src/base/SConscript
@@ -47,6 +47,7 @@
Source('pngwriter.cc')
Source('fiber.cc')
GTest('fiber.test', 'fiber.test.cc', 'fiber.cc')
+GTest('flags.test', 'flags.test.cc')
GTest('coroutine.test', 'coroutine.test.cc', 'fiber.cc')
Source('framebuffer.cc')
Source('hostinfo.cc')
diff --git a/src/base/flags.test.cc b/src/base/flags.test.cc
new file mode 100644
index 000..297abc8
--- /dev/null
+++ b/src/base/flags.test.cc
@@ -0,0 +1,245 @@
+/*
+ * Copyright (c) 2020 Daniel R. Carvalho
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met: redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer;
+ * redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution;
+ * neither the name of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include
+
+#include
+#include
+
+#include "base/flags.hh"
+
+/** Test default zero-initialized constructor. */
+TEST(FlagsTest, ConstructorZero)
+{
+const Flags flags;
+ASSERT_EQ(uint32_t(0), uint32_t(flags));
+}
+
+/** Test constructor with a single-bit initial value. */
+TEST(FlagsTest, ConstructorSingle)
+{
+const uint32_t value = (1 << 3);
+const Flags flags(value);
+ASSERT_EQ(value, uint32_t(flags));
+}
+
+/** Test constructor with an initial multi-bit value. */
+TEST(FlagsTest, ConstructorMulti)
+{
+const uint32_t value = (1 << 3) | (1 << 5) | (1 << 9);
+const Flags flags(value);
+ASSERT_EQ(value, uint32_t(flags));
+}
+
+/** Test assignment of variable of underlying type. */
+TEST(FlagsTest, TypeAssignment)
+{
+const uint32_t value = (1 << 3) | (1 << 5) | (1 << 9);
+Flags flags;
+flags = value;
+ASSERT_EQ(value, uint32_t(flags));
+}
+
+/**
+ * Test assignment of variable of underlying type, overwriting an initial
+ * value.
+ */
+TEST(FlagsTest, TypeAssignmentOverwrite)
+{
+const uint32_t init_value = (1 << 5) | (1 << 6) ;
+const uint32_t value = (1 << 3) | (1 << 5) | (1 << 9);
+Flags flags(init_value);
+flags = value;
+ASSERT_EQ(value, uint32_t(flags));
+}
+
+/** Test assignment of other Flags. */
+TEST(FlagsTest, FlagsAssignment)
+{
+const uint32_t value = (1 << 3) | (1 << 5) | (1 << 9);
+Flags flags_a;
+Flags flags_b(value);
+flags_a = flags_b;
+ASSERT_EQ(uint32_t(flags_a), uint32_t(flags_b));
+}
+
+/** Test assignment of other Flags, overwriting an initial value. */
+TEST(FlagsTest, FlagsAssignmentOverwrite)
+{
+const uint32_t init_value = (1 << 5) | (1 << 6);
+const uint32_t value = (1 << 3) | (1 << 5) | (1 << 9);
+Flags flags_a(init_value);
+Flags flags_b(value);
+flags_a = flags_b;
+ASSERT_EQ(uint32_t(flags_a), uint32_t(flags_b));
+}
+
+/** Test isSet for any bit set. */
+TEST(FlagsTest, IsSetAny)
+{
+const uint32_t value = (1 << 3);
+const Flags flags_a;
+const Flags flags_b(value);
+ASSERT_FALSE(flags_a.isSet());
+
[gem5-dev] Change in gem5/gem5[develop]: base: Remove dubious/unused Flags functions
Daniel Carvalho has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/38736 )
Change subject: base: Remove dubious/unused Flags functions
..
base: Remove dubious/unused Flags functions
The functions isSet(), noneSet(), and allSet() assume that
all bits of the underlying container have a corresponding
flag. This is typically not true, and the likelihood of
incorrectly using these functions is high.
Fortunately these functions are not being used anywhere,
and can be safely removed.
Alternatively, a mask could be provided on construction to
determine which bits of the underlying container correspond
to a flag bit, so that the proper bits are checked in these
functions.
Change-Id: Ia7cbfd0726943506a3f04dc417e67a0b57cdbf95
Signed-off-by: Daniel R. Carvalho
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38736
Maintainer: Bobby R. Bruce
Tested-by: kokoro
Reviewed-by: Jason Lowe-Power
Reviewed-by: Bobby R. Bruce
---
M src/base/flags.hh
M src/base/flags.test.cc
2 files changed, 1 insertion(+), 55 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/base/flags.hh b/src/base/flags.hh
index 170abc5..6e82210 100644
--- a/src/base/flags.hh
+++ b/src/base/flags.hh
@@ -72,13 +72,6 @@
}
/**
- * Verifies whether any bit in the flags is set.
- *
- * @return True if any flag bit is set; false otherwise.
- */
-bool isSet() const { return _flags; }
-
-/**
* Verifies whether any bit matching the given mask is set.
*
* @param mask The mask containing the bits to verify.
@@ -87,13 +80,6 @@
bool isSet(Type mask) const { return (_flags & mask); }
/**
- * Verifies whether all bits in the flags are set.
- *
- * @return True if all flag bits are set; false otherwise.
- */
-bool allSet() const { return !(~_flags); }
-
-/**
* Verifies whether no bits matching the given mask are set.
*
* @param mask The mask containing the bits to verify.
@@ -102,13 +88,6 @@
bool allSet(Type mask) const { return (_flags & mask) == mask; }
/**
- * Verifies whether no bits in the flags are set.
- *
- * @return True if all flag bits are cleared; false otherwise.
- */
-bool noneSet() const { return _flags == 0; }
-
-/**
* Verifies whether no bits matching the given mask are set.
*
* @param mask The mask containing the bits to verify.
diff --git a/src/base/flags.test.cc b/src/base/flags.test.cc
index 297abc8..c45a7a8 100644
--- a/src/base/flags.test.cc
+++ b/src/base/flags.test.cc
@@ -99,16 +99,6 @@
ASSERT_EQ(uint32_t(flags_a), uint32_t(flags_b));
}
-/** Test isSet for any bit set. */
-TEST(FlagsTest, IsSetAny)
-{
-const uint32_t value = (1 << 3);
-const Flags flags_a;
-const Flags flags_b(value);
-ASSERT_FALSE(flags_a.isSet());
-ASSERT_TRUE(flags_b.isSet());
-}
-
/** Test isSet for multiple bits set. */
TEST(FlagsTest, IsSetValue)
{
@@ -131,19 +121,6 @@
ASSERT_FALSE(flags.isSet(value_c));
}
-/** Test if all bits are set with allSet. */
-TEST(FlagsTest, AllSet)
-{
-const uint32_t value_b = (1 << 5) | (1 << 6);
-const uint32_t value_c = std::numeric_limits::max();
-const Flags flags_a;
-const Flags flags_b(value_b);
-const Flags flags_c(value_c);
-ASSERT_FALSE(flags_a.allSet());
-ASSERT_FALSE(flags_b.allSet());
-ASSERT_TRUE(flags_c.allSet());
-}
-
/** Test allSet comparing against another flag. */
TEST(FlagsTest, AllSetMatch)
{
@@ -154,16 +131,6 @@
ASSERT_FALSE(flags.allSet(value_b));
}
-/** Test if no bits are set with noneSet. */
-TEST(FlagsTest, NoneSet)
-{
-const uint32_t value_b = (1 << 5) | (1 << 6);
-const Flags flags_a;
-const Flags flags_b(value_b);
-ASSERT_TRUE(flags_a.noneSet());
-ASSERT_FALSE(flags_b.noneSet());
-}
-
/** Test noneSet comparing against another flag. */
TEST(FlagsTest, NoneSetMatch)
{
@@ -182,7 +149,7 @@
const uint32_t value = (1 << 5) | (1 << 6);
Flags flags(value);
flags.clear();
-ASSERT_TRUE(flags.noneSet());
+ASSERT_EQ(0, uint32_t(flags));
}
/** Test clearing specific bits. */
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38736
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: Ia7cbfd0726943506a3f04dc417e67a0b57cdbf95
Gerrit-Change-Number: 38736
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Carvalho
Gerrit-Reviewer: Bobby R. Bruce
Gerrit-Reviewer: Daniel Carvalho
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: kokoro
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To
[gem5-dev] Change in gem5/gem5[develop]: arch: Fix the code that computes MaxMiscDestReg.
Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/38383 ) Change subject: arch: Fix the code that computes MaxMiscDestReg. .. arch: Fix the code that computes MaxMiscDestReg. This code was simplified a little while ago, and the wrong variable name was used in that computation accidentally. Fortunately the "wrong" value would be too large, and so nothing bad would happen except a pair of arrays would be overly large in the O3 instruction class. Change-Id: I9694f1a8c79a62a172ef63bdd2f98fa0ace06acd Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38383 Reviewed-by: Giacomo Travaglini Maintainer: Giacomo Travaglini Tested-by: kokoro --- M src/arch/isa_parser/operand_list.py 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/arch/isa_parser/operand_list.py b/src/arch/isa_parser/operand_list.py index 12a23d9..cea3ae4 100755 --- a/src/arch/isa_parser/operand_list.py +++ b/src/arch/isa_parser/operand_list.py @@ -135,7 +135,7 @@ parser.maxInstSrcRegs = max(parser.maxInstSrcRegs, self.numSrcRegs) parser.maxInstDestRegs = max(parser.maxInstDestRegs, self.numDestRegs) -parser.maxMiscDestRegs = max(parser.maxInstDestRegs, +parser.maxMiscDestRegs = max(parser.maxMiscDestRegs, self.numMiscDestRegs) # now make a final pass to finalize op_desc fields that may depend -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38383 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: I9694f1a8c79a62a172ef63bdd2f98fa0ace06acd Gerrit-Change-Number: 38383 Gerrit-PatchSet: 4 Gerrit-Owner: Gabe Black Gerrit-Reviewer: Andreas Sandberg Gerrit-Reviewer: Gabe Black Gerrit-Reviewer: Giacomo Travaglini 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]: cpu: Remove the default, globally sized index arrays from StaticInst.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/38382 )
Change subject: cpu: Remove the default, globally sized index arrays from
StaticInst.
..
cpu: Remove the default, globally sized index arrays from StaticInst.
All ISAs now allocate their own arrays in all their instructions, making
these arrays unnecessary.
Change-Id: Ie2bc5d7a2903e07703809505cdaaf6c578f5
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38382
Reviewed-by: Giacomo Travaglini
Maintainer: Giacomo Travaglini
Tested-by: kokoro
---
M src/cpu/static_inst.hh
1 file changed, 1 insertion(+), 12 deletions(-)
Approvals:
Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh
index e5d9753..25a49b3 100644
--- a/src/cpu/static_inst.hh
+++ b/src/cpu/static_inst.hh
@@ -88,20 +88,13 @@
/// Binary extended machine instruction type.
typedef TheISA::ExtMachInst ExtMachInst;
-enum {
-MaxInstSrcRegs = TheISA::MaxInstSrcRegs,//< Max source regs
-MaxInstDestRegs = TheISA::MaxInstDestRegs //< Max dest regs
-};
-
using RegIdArrayPtr = RegId (StaticInst:: *)[];
private:
/// See srcRegIdx().
-RegId _srcRegIdx[MaxInstSrcRegs];
RegIdArrayPtr _srcRegIdxPtr = nullptr;
/// See destRegIdx().
-RegId _destRegIdx[MaxInstDestRegs];
RegIdArrayPtr _destRegIdxPtr = nullptr;
protected:
@@ -309,11 +302,7 @@
/// the fields that are meaningful for the particular
/// instruction.
StaticInst(const char *_mnemonic, ExtMachInst _machInst, OpClass
__opClass)
-: _srcRegIdxPtr(
-reinterpret_cast(&StaticInst::_srcRegIdx)),
- _destRegIdxPtr(
-reinterpret_cast(&StaticInst::_destRegIdx)),
- _opClass(__opClass),
+: _opClass(__opClass),
_numSrcRegs(0), _numDestRegs(0), _numFPDestRegs(0),
_numIntDestRegs(0), _numCCDestRegs(0), _numVecDestRegs(0),
_numVecElemDestRegs(0), _numVecPredDestRegs(0),
machInst(_machInst),
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38382
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: Ie2bc5d7a2903e07703809505cdaaf6c578f5
Gerrit-Change-Number: 38382
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Giacomo Travaglini
Gerrit-Reviewer: Jason Lowe-Power
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]: arm: Use local src and dest reg index arrays.
Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/38381 ) Change subject: arm: Use local src and dest reg index arrays. .. arm: Use local src and dest reg index arrays. Change-Id: I7512c0abfaa2148b31b51fa43c3789bdca179105 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38381 Reviewed-by: Giacomo Travaglini Maintainer: Giacomo Travaglini Tested-by: kokoro --- A src/arch/arm/isa/arminstobjparams.isa M src/arch/arm/isa/formats/basic.isa M src/arch/arm/isa/formats/pred.isa M src/arch/arm/isa/formats/pseudo.isa M src/arch/arm/isa/insts/aarch64.isa M src/arch/arm/isa/insts/branch.isa M src/arch/arm/isa/insts/branch64.isa M src/arch/arm/isa/insts/crypto.isa M src/arch/arm/isa/insts/crypto64.isa M src/arch/arm/isa/insts/data.isa M src/arch/arm/isa/insts/data64.isa M src/arch/arm/isa/insts/div.isa M src/arch/arm/isa/insts/fp.isa M src/arch/arm/isa/insts/fp64.isa M src/arch/arm/isa/insts/m5ops.isa M src/arch/arm/isa/insts/macromem.isa M src/arch/arm/isa/insts/mem.isa M src/arch/arm/isa/insts/misc.isa M src/arch/arm/isa/insts/misc64.isa M src/arch/arm/isa/insts/mult.isa M src/arch/arm/isa/insts/neon.isa M src/arch/arm/isa/insts/neon64.isa M src/arch/arm/isa/insts/neon64_mem.isa M src/arch/arm/isa/insts/pauth.isa M src/arch/arm/isa/insts/sve.isa M src/arch/arm/isa/insts/sve_mem.isa M src/arch/arm/isa/main.isa M src/arch/arm/isa/templates/basic.isa M src/arch/arm/isa/templates/branch.isa M src/arch/arm/isa/templates/branch64.isa M src/arch/arm/isa/templates/data64.isa M src/arch/arm/isa/templates/macromem.isa M src/arch/arm/isa/templates/mem.isa M src/arch/arm/isa/templates/mem64.isa M src/arch/arm/isa/templates/misc.isa M src/arch/arm/isa/templates/misc64.isa M src/arch/arm/isa/templates/mult.isa M src/arch/arm/isa/templates/neon.isa M src/arch/arm/isa/templates/neon64.isa M src/arch/arm/isa/templates/pred.isa M src/arch/arm/isa/templates/semihost.isa M src/arch/arm/isa/templates/sve.isa M src/arch/arm/isa/templates/sve_mem.isa M src/arch/arm/isa/templates/vfp.isa M src/arch/arm/isa/templates/vfp64.isa 45 files changed, 2,076 insertions(+), 1,351 deletions(-) Approvals: Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38381 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: I7512c0abfaa2148b31b51fa43c3789bdca179105 Gerrit-Change-Number: 38381 Gerrit-PatchSet: 4 Gerrit-Owner: Gabe Black Gerrit-Reviewer: Andreas Sandberg Gerrit-Reviewer: Gabe Black Gerrit-Reviewer: Giacomo Travaglini 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: Wrap InstObjParams with a class and not a function.
Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/39275 ) Change subject: arch: Wrap InstObjParams with a class and not a function. .. arch: Wrap InstObjParams with a class and not a function. When parsing an ISA description, the InstObjParams class needs to have a reference to the current parser. It does that by exposing a wrapper to the description rather than the actual InstObjParams class. That wrapper injects an additional argument into the InstObjParams constructor. Originally, the wrapper which injectect the additional argument was a function which masqueraded as a class. That made it impossible to subclass InstObjParams. Instead, this change replaces that function wrapper with a class wrapper, and injects the extra argument in the __init__ method. This preserves the fact that the InstObjParams name refers to a class, and allows any sort of interaction that's normally allowed with a class like subclassing. Change-Id: I550ea2e60eadac3c7c0b9afa7d71f4607b49a5d2 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39275 Reviewed-by: Giacomo Travaglini Maintainer: Giacomo Travaglini Tested-by: kokoro --- M src/arch/isa_parser/isa_parser.py 1 file changed, 6 insertions(+), 5 deletions(-) Approvals: Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/arch/isa_parser/isa_parser.py b/src/arch/isa_parser/isa_parser.py index 9ca813e..a0b9f74 100755 --- a/src/arch/isa_parser/isa_parser.py +++ b/src/arch/isa_parser/isa_parser.py @@ -1436,11 +1436,12 @@ # END OF GRAMMAR RULES def updateExportContext(self): - -# create a continuation that allows us to grab the current parser -def wrapInstObjParams(*args): -return InstObjParams(self, *args) -self.exportContext['InstObjParams'] = wrapInstObjParams +# Create a wrapper class that allows us to grab the current parser. +class InstObjParamsWrapper(InstObjParams): +def __init__(iop, *args, **kwargs): +super(InstObjParamsWrapper, iop).__init__( +self, *args, **kwargs) +self.exportContext['InstObjParams'] = InstObjParamsWrapper self.exportContext.update(self.templateMap) def defFormat(self, id, params, code, lineno): -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39275 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: I550ea2e60eadac3c7c0b9afa7d71f4607b49a5d2 Gerrit-Change-Number: 39275 Gerrit-PatchSet: 2 Gerrit-Owner: Gabe Black Gerrit-Reviewer: Andreas Sandberg Gerrit-Reviewer: Gabe Black Gerrit-Reviewer: Giacomo Travaglini 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-arm: dtb_addr is already encoding the loadAddrOffset
Giacomo Travaglini has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/39216 )
Change subject: arch-arm: dtb_addr is already encoding the loadAddrOffset
..
arch-arm: dtb_addr is already encoding the loadAddrOffset
This fixes a bug in AArch32 where the dtb_address is
adding the loadAddrOffset twice to the dtb base address
after
https://gem5-review.googlesource.com/c/public/gem5/+/35076
Change-Id: Ia8bd35a02d998c54fbc3a889739c9abbeb506d96
Signed-off-by: Giacomo Travaglini
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39216
Reviewed-by: Ciro Santilli
Tested-by: kokoro
---
M src/arch/arm/freebsd/fs_workload.cc
M src/arch/arm/linux/fs_workload.cc
2 files changed, 5 insertions(+), 5 deletions(-)
Approvals:
Ciro Santilli: Looks good to me, approved
Giacomo Travaglini: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/freebsd/fs_workload.cc
b/src/arch/arm/freebsd/fs_workload.cc
index 8eb3c70..cc0151b 100644
--- a/src/arch/arm/freebsd/fs_workload.cc
+++ b/src/arch/arm/freebsd/fs_workload.cc
@@ -95,7 +95,7 @@
// Kernel supports flattened device tree and dtb file specified.
// Using Device Tree Blob to describe system configuration.
inform("Loading DTB file: %s at address %#x\n", params().dtb_filename,
-params().dtb_addr + _loadAddrOffset);
+params().dtb_addr);
auto *dtb_file = new ::Loader::DtbFile(params().dtb_filename);
@@ -108,7 +108,7 @@
bootReleaseAddr = ra & ~ULL(0x7F);
dtb_file->buildImage().
-offset(params().dtb_addr + _loadAddrOffset).
+offset(params().dtb_addr).
write(system->physProxy);
delete dtb_file;
@@ -116,7 +116,7 @@
for (auto *tc: system->threads) {
tc->setIntReg(0, 0);
tc->setIntReg(1, params().machine_type);
-tc->setIntReg(2, params().dtb_addr + _loadAddrOffset);
+tc->setIntReg(2, params().dtb_addr);
}
}
diff --git a/src/arch/arm/linux/fs_workload.cc
b/src/arch/arm/linux/fs_workload.cc
index f05fd6f..b296b68 100644
--- a/src/arch/arm/linux/fs_workload.cc
+++ b/src/arch/arm/linux/fs_workload.cc
@@ -151,7 +151,7 @@
DPRINTF(Loader, "Boot atags was %d bytes in total\n", size << 2);
DDUMP(Loader, boot_data, size << 2);
-system->physProxy.writeBlob(params().dtb_addr + _loadAddrOffset,
+system->physProxy.writeBlob(params().dtb_addr,
boot_data, size << 2);
delete[] boot_data;
@@ -170,7 +170,7 @@
for (auto *tc: system->threads) {
tc->setIntReg(0, 0);
tc->setIntReg(1, params().machine_type);
-tc->setIntReg(2, params().dtb_addr + _loadAddrOffset);
+tc->setIntReg(2, params().dtb_addr);
}
}
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39216
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: Ia8bd35a02d998c54fbc3a889739c9abbeb506d96
Gerrit-Change-Number: 39216
Gerrit-PatchSet: 2
Gerrit-Owner: Giacomo Travaglini
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Ciro Santilli
Gerrit-Reviewer: Giacomo Travaglini
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: Nightly stuck
Thanks Giacomo, I've jumped in and added timeouts to the Nightly builds and the compiler checks. The Nightly builds will now fail (instead of just "abort", which apparently is a silent failure) after 23 hours. Most of the time the nightly builds complete within 12ish hours (though this does vary a lot depending on how much compilation needs done). The Compiler Tests typically take 3 to 4 days. I've added a timeout at 6 days. I'm probably being a bit liberal with these timeouts, but I don't want to keep going back and extending them as we extend the tests. This doesn't stop us having checks at individual tests (i.e., we may want checks to ensure certain executions complete in a certain time). Kind regards, Bobby. -- Dr. Bobby R. Bruce Room 2235, Kemper Hall, UC Davis Davis, CA, 95616 web: https://www.bobbybruce.net Please consider making a submission to GI@ICSE '21: http://geneticimprovementofsoftware.com/gi2021icse.html On Mon, Jan 18, 2021 at 10:57 AM Jason Lowe-Power via gem5-dev < gem5-dev@gem5.org> wrote: > Thanks, Giacomo! We may be a bit slow to respond (it's a holiday here in > the states today and Bobby is off the next couple of days). > > I don't think we want to do a global timeout since we have some tests that > take days. The compiler test takes about 3 days, IIRC. I think per-test > timeout is going to be required. > > Cheers, > Jason > > On Mon, Jan 18, 2021 at 9:43 AM Giacomo Travaglini via gem5-dev < > gem5-dev@gem5.org> wrote: > >> Hi all, >> >> This has been fixed and it's going to be merged today. >> >> This made me realize we might want to introduce a timeout for our >> regressions. A global, Jenkins level one is easy to add and doesn't require >> any code modifications. >> >> However a per-test timeout is probably the best approach. In this way we >> can: >> >> a) Understand which regression is deadlocking from the output log >> b) Killing the failing test earlier thus saving computational costs: >> >> e.g: I will kill the test after 6 minutes if it is supposed to complete >> in a minute, rather than having a global abort set up after 24 hours. >> >> How to do this is up do debate. This falls within the typical wall-clock >> time vs cpu time issue; setting up a timeout for wall-clock time is >> straightforward with python subprocess, but cpu time is what we would >> really care about... >> Though if we make sure our timing out margin is relatively high >> (loosening the safety net) we can probably get around the intrinsic >> difference between the two metrics and just use wall-clock time. >> >> Does anybody have any thoughts on this? >> >> Giacomo >> >> > -Original Message- >> > From: Giacomo Travaglini >> > Sent: 18 January 2021 10:22 >> > To: gem5 Developer List >> > Subject: Nightly stuck >> > >> > Hi devs, >> > >> > It seems like the following Nightly run #191 is stuck: >> > >> > https://jenkins.gem5.org/job/Nightly/191/ >> > >> > I am running those tests locally to understand if it is a tool problem >> or broken >> > long regressions are currently broken Will keep you posted >> > >> > Kind Regards >> > >> > Giacomo >> IMPORTANT NOTICE: The contents of this email and any attachments are >> confidential and may also be privileged. If you are not the intended >> recipient, please notify the sender immediately and do not disclose the >> contents to any other person, use it for any purpose, or store or copy the >> information in any medium. Thank you. >> ___ >> 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 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: Nightly stuck
Thanks, Giacomo! We may be a bit slow to respond (it's a holiday here in the states today and Bobby is off the next couple of days). I don't think we want to do a global timeout since we have some tests that take days. The compiler test takes about 3 days, IIRC. I think per-test timeout is going to be required. Cheers, Jason On Mon, Jan 18, 2021 at 9:43 AM Giacomo Travaglini via gem5-dev < gem5-dev@gem5.org> wrote: > Hi all, > > This has been fixed and it's going to be merged today. > > This made me realize we might want to introduce a timeout for our > regressions. A global, Jenkins level one is easy to add and doesn't require > any code modifications. > > However a per-test timeout is probably the best approach. In this way we > can: > > a) Understand which regression is deadlocking from the output log > b) Killing the failing test earlier thus saving computational costs: > > e.g: I will kill the test after 6 minutes if it is supposed to complete in > a minute, rather than having a global abort set up after 24 hours. > > How to do this is up do debate. This falls within the typical wall-clock > time vs cpu time issue; setting up a timeout for wall-clock time is > straightforward with python subprocess, but cpu time is what we would > really care about... > Though if we make sure our timing out margin is relatively high (loosening > the safety net) we can probably get around the intrinsic difference between > the two metrics and just use wall-clock time. > > Does anybody have any thoughts on this? > > Giacomo > > > -Original Message- > > From: Giacomo Travaglini > > Sent: 18 January 2021 10:22 > > To: gem5 Developer List > > Subject: Nightly stuck > > > > Hi devs, > > > > It seems like the following Nightly run #191 is stuck: > > > > https://jenkins.gem5.org/job/Nightly/191/ > > > > I am running those tests locally to understand if it is a tool problem > or broken > > long regressions are currently broken Will keep you posted > > > > Kind Regards > > > > Giacomo > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > ___ > 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: Nightly stuck
Hi all, This has been fixed and it's going to be merged today. This made me realize we might want to introduce a timeout for our regressions. A global, Jenkins level one is easy to add and doesn't require any code modifications. However a per-test timeout is probably the best approach. In this way we can: a) Understand which regression is deadlocking from the output log b) Killing the failing test earlier thus saving computational costs: e.g: I will kill the test after 6 minutes if it is supposed to complete in a minute, rather than having a global abort set up after 24 hours. How to do this is up do debate. This falls within the typical wall-clock time vs cpu time issue; setting up a timeout for wall-clock time is straightforward with python subprocess, but cpu time is what we would really care about... Though if we make sure our timing out margin is relatively high (loosening the safety net) we can probably get around the intrinsic difference between the two metrics and just use wall-clock time. Does anybody have any thoughts on this? Giacomo > -Original Message- > From: Giacomo Travaglini > Sent: 18 January 2021 10:22 > To: gem5 Developer List > Subject: Nightly stuck > > Hi devs, > > It seems like the following Nightly run #191 is stuck: > > https://jenkins.gem5.org/job/Nightly/191/ > > I am running those tests locally to understand if it is a tool problem or > broken > long regressions are currently broken Will keep you posted > > Kind Regards > > Giacomo IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ___ 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: Re-join debug flags headers
Thanks for bringing this up, Daniel! I don't have a strong opinion, personally. I see how this makes things a little easier for developers. No longer would we have to teach people that to add a new debug flag you both declare it in the SConscript file and include the named header file. Given that I've worked with gem5 for a long time, I don't see that as much overhead, though. This would increase the compile time (though, hopefully only rarely). And, we recently found from the gem5 survey that people don't care that much about the compile time (more info coming soon). I think the biggest downside would be for those that use gem5 in teaching. Students in architecture classes are the most likely to have underpowered computers building gem5, and they are also likely to be adding debug flags. Cheers, Jason On Fri, Jan 15, 2021 at 5:47 PM Daniel Carvalho via gem5-dev < gem5-dev@gem5.org> wrote: > Hello all, > > I've uploaded a commit that re-joins the debug header files into a single > one: https://gem5-review.googlesource.com/c/public/gem5/+/39255. This > means that instead of having to include a different header for each and > every debug flag used in a file, only one is needed. For example, in > src/arch/arm/kvm/arm_cpu.cc > > #include "debug/Kvm.hh" > #include "debug/KvmContext.hh" > #include "debug/KvmInt.hh" > > would be replaced by > > #include "debug/flags.hh" > > Finally, since most of the code using these flags will use DPRINTF or a > similar macro, this include could be added to base/trace.hh, so that most > of the times it would be included transitively. > > The advantage of this change is that debugging becomes easier/faster: this > file would behave as a library of supported flags and very few header > files would suffice for all debugging needs. The disadvantage is that > every time a commit introduces or removes a debug flag debug/flags.hh will > be recompiled, which will cause an almost complete build. Luckily, debug > flags are not frequently modified. > > Performance-wise both approaches are similar. > > What are your opinions in this matter? > > Cheers, > Daniel > ___ > 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]: arch-arm: dtb_addr is already encoding the loadAddrOffset
Giacomo Travaglini has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39216 )
Change subject: arch-arm: dtb_addr is already encoding the loadAddrOffset
..
arch-arm: dtb_addr is already encoding the loadAddrOffset
This fixes a bug in AArch32 where the dtb_address is
adding the loadAddrOffset twice to the dtb base address
after
https://gem5-review.googlesource.com/c/public/gem5/+/35076
Change-Id: Ia8bd35a02d998c54fbc3a889739c9abbeb506d96
Signed-off-by: Giacomo Travaglini
---
M src/arch/arm/freebsd/fs_workload.cc
M src/arch/arm/linux/fs_workload.cc
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/arch/arm/freebsd/fs_workload.cc
b/src/arch/arm/freebsd/fs_workload.cc
index 8eb3c70..cc0151b 100644
--- a/src/arch/arm/freebsd/fs_workload.cc
+++ b/src/arch/arm/freebsd/fs_workload.cc
@@ -95,7 +95,7 @@
// Kernel supports flattened device tree and dtb file specified.
// Using Device Tree Blob to describe system configuration.
inform("Loading DTB file: %s at address %#x\n", params().dtb_filename,
-params().dtb_addr + _loadAddrOffset);
+params().dtb_addr);
auto *dtb_file = new ::Loader::DtbFile(params().dtb_filename);
@@ -108,7 +108,7 @@
bootReleaseAddr = ra & ~ULL(0x7F);
dtb_file->buildImage().
-offset(params().dtb_addr + _loadAddrOffset).
+offset(params().dtb_addr).
write(system->physProxy);
delete dtb_file;
@@ -116,7 +116,7 @@
for (auto *tc: system->threads) {
tc->setIntReg(0, 0);
tc->setIntReg(1, params().machine_type);
-tc->setIntReg(2, params().dtb_addr + _loadAddrOffset);
+tc->setIntReg(2, params().dtb_addr);
}
}
diff --git a/src/arch/arm/linux/fs_workload.cc
b/src/arch/arm/linux/fs_workload.cc
index f05fd6f..b296b68 100644
--- a/src/arch/arm/linux/fs_workload.cc
+++ b/src/arch/arm/linux/fs_workload.cc
@@ -151,7 +151,7 @@
DPRINTF(Loader, "Boot atags was %d bytes in total\n", size << 2);
DDUMP(Loader, boot_data, size << 2);
-system->physProxy.writeBlob(params().dtb_addr + _loadAddrOffset,
+system->physProxy.writeBlob(params().dtb_addr,
boot_data, size << 2);
delete[] boot_data;
@@ -170,7 +170,7 @@
for (auto *tc: system->threads) {
tc->setIntReg(0, 0);
tc->setIntReg(1, params().machine_type);
-tc->setIntReg(2, params().dtb_addr + _loadAddrOffset);
+tc->setIntReg(2, params().dtb_addr);
}
}
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39216
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: Ia8bd35a02d998c54fbc3a889739c9abbeb506d96
Gerrit-Change-Number: 39216
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini
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]: sim: Add units to src/sim
Hoa Nguyen has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39296 )
Change subject: sim: Add units to src/sim
..
sim: Add units to src/sim
Change-Id: I5fa147aa1319d62be1790bbd74fd097ac566f808
Signed-off-by: Hoa Nguyen
---
M src/sim/clock_domain.cc
M src/sim/power/power_model.cc
M src/sim/power/thermal_domain.cc
M src/sim/power_domain.cc
M src/sim/power_state.cc
M src/sim/process.cc
M src/sim/root.cc
M src/sim/voltage_domain.cc
M src/sim/workload.hh
9 files changed, 46 insertions(+), 31 deletions(-)
diff --git a/src/sim/clock_domain.cc b/src/sim/clock_domain.cc
index 61d5654..adb29a5 100644
--- a/src/sim/clock_domain.cc
+++ b/src/sim/clock_domain.cc
@@ -51,7 +51,7 @@
ClockDomain::ClockDomainStats::ClockDomainStats(ClockDomain &cd)
: Stats::Group(&cd),
-ADD_STAT(clock, "Clock period in ticks")
+ADD_STAT_WITH_UNIT(clock, UNIT_TICK, "Clock period in ticks")
{
// Expose the current clock period as a stat for observability in
// the dumps
diff --git a/src/sim/power/power_model.cc b/src/sim/power/power_model.cc
index fbc67d3..a355eb3 100644
--- a/src/sim/power/power_model.cc
+++ b/src/sim/power/power_model.cc
@@ -45,8 +45,10 @@
PowerModelState::PowerModelState(const Params &p)
: SimObject(p), _temp(0), clocked_object(NULL),
- ADD_STAT(dynamicPower, "Dynamic power for this object (Watts)"),
- ADD_STAT(staticPower, "Static power for this object (Watts)")
+ ADD_STAT_WITH_UNIT(dynamicPower, UNIT_WATT,
+ "Dynamic power for this object (Watts)"),
+ ADD_STAT_WITH_UNIT(staticPower, UNIT_WATT,
+ "Static power for this object (Watts)")
{
dynamicPower
.method(this, &PowerModelState::getDynamicPower);
@@ -57,8 +59,10 @@
PowerModel::PowerModel(const Params &p)
: SimObject(p), states_pm(p.pm), subsystem(p.subsystem),
clocked_object(NULL), power_model_type(p.pm_type),
- ADD_STAT(dynamicPower, "Dynamic power for this power state"),
- ADD_STAT(staticPower, "Static power for this power state")
+ ADD_STAT_WITH_UNIT(dynamicPower, UNIT_WATT,
+ "Dynamic power for this power state"),
+ ADD_STAT_WITH_UNIT(staticPower, UNIT_WATT,
+ "Static power for this power state")
{
panic_if(subsystem == NULL,
"Subsystem is NULL! This is not acceptable for a
PowerModel!\n");
diff --git a/src/sim/power/thermal_domain.cc
b/src/sim/power/thermal_domain.cc
index dabf2fe..7ca8e6a 100644
--- a/src/sim/power/thermal_domain.cc
+++ b/src/sim/power/thermal_domain.cc
@@ -51,7 +51,8 @@
ThermalDomain::ThermalDomain(const Params &p)
: SimObject(p), _initTemperature(p.initial_temperature),
node(NULL), subsystem(NULL),
-ADD_STAT(currentTemp, "Temperature in centigrade degrees")
+ADD_STAT_WITH_UNIT(currentTemp, UNIT_CENTIGRADE,
+ "Temperature in centigrade degrees")
{
currentTemp
.method(this, &ThermalDomain::currentTemperature);
diff --git a/src/sim/power_domain.cc b/src/sim/power_domain.cc
index c6e0d35..0a5ca33 100644
--- a/src/sim/power_domain.cc
+++ b/src/sim/power_domain.cc
@@ -243,11 +243,12 @@
PowerDomain::PowerDomainStats::PowerDomainStats(PowerDomain &pd)
: Stats::Group(&pd),
-ADD_STAT(numLeaderCalls,
- "Number of calls by leaders to change power domain state"),
-ADD_STAT(numLeaderCallsChangingState,
- "Number of calls by leader to change power domain state "
- "actually resulting in a power state change")
+ADD_STAT_WITH_UNIT(numLeaderCalls, UNIT_COUNT,
+ "Number of calls by leaders to change power domain "
+ "state"),
+ADD_STAT_WITH_UNIT(numLeaderCallsChangingState, UNIT_COUNT,
+ "Number of calls by leader to change power domain "
+ "state actually resulting in a power state change")
{
}
diff --git a/src/sim/power_state.cc b/src/sim/power_state.cc
index a11ed43..c571180 100644
--- a/src/sim/power_state.cc
+++ b/src/sim/power_state.cc
@@ -217,14 +217,14 @@
PowerState::PowerStateStats::PowerStateStats(PowerState &co)
: Stats::Group(&co),
powerState(co),
-ADD_STAT(numTransitions,
- "Number of power state transitions"),
-ADD_STAT(numPwrMatchStateTransitions,
- "Number of power state transitions due match request"),
-ADD_STAT(ticksClkGated,
- "Distribution of time spent in the clock gated state"),
-ADD_STAT(pwrStateResidencyTicks,
- "Cumulative time (in ticks) in various power states")
+ADD_STAT_WITH_UNIT(numTransitions, UNIT_COUNT,
+ "Number of power state transitions"),
+ADD_STAT_WITH_UNIT(numPwrMatchStateTransitions, UNIT_COUNT,
+ "Number of power state transitions due match
request"),
+AD
[gem5-dev] Nightly stuck
Hi devs, It seems like the following Nightly run #191 is stuck: https://jenkins.gem5.org/job/Nightly/191/ I am running those tests locally to understand if it is a tool problem or broken long regressions are currently broken Will keep you posted Kind Regards Giacomo IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ___ 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: Remove copyMiscRegs from utility.hh.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39325 )
Change subject: arch: Remove copyMiscRegs from utility.hh.
..
arch: Remove copyMiscRegs from utility.hh.
This function is occasionally used internally in copyRegs, but is not
used by anything else and doesn't need to be publically exposed in the
header file.
Change-Id: Id02a77e7dd19c6c089a408bfe0099466822c523d
---
M src/arch/arm/utility.hh
M src/arch/mips/utility.cc
M src/arch/mips/utility.hh
M src/arch/power/utility.cc
M src/arch/power/utility.hh
M src/arch/sparc/utility.hh
M src/arch/x86/utility.hh
7 files changed, 5 insertions(+), 22 deletions(-)
diff --git a/src/arch/arm/utility.hh b/src/arch/arm/utility.hh
index 302967b..853643b 100644
--- a/src/arch/arm/utility.hh
+++ b/src/arch/arm/utility.hh
@@ -86,12 +86,6 @@
void copyRegs(ThreadContext *src, ThreadContext *dest);
-static inline void
-copyMiscRegs(ThreadContext *src, ThreadContext *dest)
-{
-panic("Copy Misc. Regs Not Implemented Yet\n");
-}
-
/** Send an event (SEV) to a specific PE if there isn't
* already a pending event */
void sendEvent(ThreadContext *tc);
diff --git a/src/arch/mips/utility.cc b/src/arch/mips/utility.cc
index cf8515f..7f98122 100644
--- a/src/arch/mips/utility.cc
+++ b/src/arch/mips/utility.cc
@@ -228,10 +228,4 @@
dest->pcState(src->pcState());
}
-void
-copyMiscRegs(ThreadContext *src, ThreadContext *dest)
-{
-panic("Copy Misc. Regs Not Implemented Yet\n");
-}
-
} // namespace MipsISA
diff --git a/src/arch/mips/utility.hh b/src/arch/mips/utility.hh
index eb7f74a..1804680 100644
--- a/src/arch/mips/utility.hh
+++ b/src/arch/mips/utility.hh
@@ -73,7 +73,6 @@
}
void copyRegs(ThreadContext *src, ThreadContext *dest);
-void copyMiscRegs(ThreadContext *src, ThreadContext *dest);
inline void
advancePC(PCState &pc, const StaticInstPtr &inst)
diff --git a/src/arch/power/utility.cc b/src/arch/power/utility.cc
index bed0be9..9003b13 100644
--- a/src/arch/power/utility.cc
+++ b/src/arch/power/utility.cc
@@ -35,6 +35,11 @@
namespace PowerISA {
void
+copyMiscRegs(ThreadContext *stc, ThreadContext *dest)
+{
+}
+
+void
copyRegs(ThreadContext *src, ThreadContext *dest)
{
// First loop through the integer registers.
diff --git a/src/arch/power/utility.hh b/src/arch/power/utility.hh
index 7c892d4..03df7fb 100644
--- a/src/arch/power/utility.hh
+++ b/src/arch/power/utility.hh
@@ -39,11 +39,6 @@
void copyRegs(ThreadContext *src, ThreadContext *dest);
-static inline void
-copyMiscRegs(ThreadContext *src, ThreadContext *dest)
-{
-}
-
inline void
advancePC(PCState &pc, const StaticInstPtr &inst)
{
diff --git a/src/arch/sparc/utility.hh b/src/arch/sparc/utility.hh
index 2bf8af0..792d7c4 100644
--- a/src/arch/sparc/utility.hh
+++ b/src/arch/sparc/utility.hh
@@ -43,8 +43,6 @@
void copyRegs(ThreadContext *src, ThreadContext *dest);
-void copyMiscRegs(ThreadContext *src, ThreadContext *dest);
-
inline void
advancePC(PCState &pc, const StaticInstPtr &inst)
{
diff --git a/src/arch/x86/utility.hh b/src/arch/x86/utility.hh
index a7c2dfc..9cd4e94 100644
--- a/src/arch/x86/utility.hh
+++ b/src/arch/x86/utility.hh
@@ -46,8 +46,6 @@
{
void copyRegs(ThreadContext *src, ThreadContext *dest);
-void copyMiscRegs(ThreadContext *src, ThreadContext *dest);
-
inline void
advancePC(PCState &pc, const StaticInstPtr &inst)
{
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39325
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: Id02a77e7dd19c6c089a408bfe0099466822c523d
Gerrit-Change-Number: 39325
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,cpu: Move buildRetPC into the StaticInst class.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39324 )
Change subject: arch,cpu: Move buildRetPC into the StaticInst class.
..
arch,cpu: Move buildRetPC into the StaticInst class.
This was an inline function defined for each ISA, but really it makes
more sense for it to be defined by the instruction classes. The actual
return address for any given instruction can best be calculated when you
know what that instruction actually does, and also the instructions will
know about ISA level PC management.
Change-Id: I2c5203aefa90f2f26ecd94e82b925c6b552e33d3
---
M src/arch/arm/insts/static_inst.hh
M src/arch/arm/utility.hh
M src/arch/mips/isa/base.isa
M src/arch/mips/utility.hh
M src/arch/power/insts/static_inst.hh
M src/arch/power/utility.hh
M src/arch/riscv/insts/static_inst.hh
M src/arch/riscv/utility.hh
M src/arch/sparc/insts/static_inst.hh
M src/arch/sparc/utility.hh
M src/arch/x86/insts/static_inst.hh
M src/arch/x86/utility.hh
M src/cpu/pred/bpred_unit.cc
M src/cpu/static_inst.hh
14 files changed, 60 insertions(+), 53 deletions(-)
diff --git a/src/arch/arm/insts/static_inst.hh
b/src/arch/arm/insts/static_inst.hh
index e101d93..084342b 100644
--- a/src/arch/arm/insts/static_inst.hh
+++ b/src/arch/arm/insts/static_inst.hh
@@ -197,6 +197,15 @@
pcState.advance();
}
+
+PCState
+buildRetPC(const PCState &curPC, const PCState &callPC) const override
+{
+PCState retPC = callPC;
+retPC.uEnd();
+return retPC;
+}
+
std::string generateDisassembly(
Addr pc, const Loader::SymbolTable *symtab) const override;
diff --git a/src/arch/arm/utility.hh b/src/arch/arm/utility.hh
index bd043df..302967b 100644
--- a/src/arch/arm/utility.hh
+++ b/src/arch/arm/utility.hh
@@ -55,14 +55,6 @@
namespace ArmISA {
-inline PCState
-buildRetPC(const PCState &curPC, const PCState &callPC)
-{
-PCState retPC = callPC;
-retPC.uEnd();
-return retPC;
-}
-
inline bool
testPredicate(uint32_t nz, uint32_t c, uint32_t v, ConditionCode code)
{
diff --git a/src/arch/mips/isa/base.isa b/src/arch/mips/isa/base.isa
index cdd950c..3ebc9e2 100644
--- a/src/arch/mips/isa/base.isa
+++ b/src/arch/mips/isa/base.isa
@@ -63,6 +63,15 @@
pc.advance();
}
+PCState
+buildRetPC(const PCState &curPC, const PCState &callPC) const
override
+{
+PCState ret = callPC;
+ret.advance();
+ret.pc(curPC.npc());
+return ret;
+}
+
size_t
asBytes(void *buf, size_t max_size) override
{
diff --git a/src/arch/mips/utility.hh b/src/arch/mips/utility.hh
index 6fb211d..eb7f74a 100644
--- a/src/arch/mips/utility.hh
+++ b/src/arch/mips/utility.hh
@@ -40,15 +40,6 @@
namespace MipsISA {
-inline PCState
-buildRetPC(const PCState &curPC, const PCState &callPC)
-{
-PCState ret = callPC;
-ret.advance();
-ret.pc(curPC.npc());
-return ret;
-}
-
//
// Floating Point Utility Functions
diff --git a/src/arch/power/insts/static_inst.hh
b/src/arch/power/insts/static_inst.hh
index dc53bc1..11a2ad7 100644
--- a/src/arch/power/insts/static_inst.hh
+++ b/src/arch/power/insts/static_inst.hh
@@ -68,6 +68,14 @@
pcState.advance();
}
+PCState
+buildRetPC(const PCState &curPC, const PCState &callPC) const override
+{
+PCState retPC = callPC;
+retPC.advance();
+return retPC;
+}
+
size_t
asBytes(void *buf, size_t max_size) override
{
diff --git a/src/arch/power/utility.hh b/src/arch/power/utility.hh
index bdb201d..7c892d4 100644
--- a/src/arch/power/utility.hh
+++ b/src/arch/power/utility.hh
@@ -37,14 +37,6 @@
namespace PowerISA {
-inline PCState
-buildRetPC(const PCState &curPC, const PCState &callPC)
-{
-PCState retPC = callPC;
-retPC.advance();
-return retPC;
-}
-
void copyRegs(ThreadContext *src, ThreadContext *dest);
static inline void
diff --git a/src/arch/riscv/insts/static_inst.hh
b/src/arch/riscv/insts/static_inst.hh
index 149e847..712bb2a 100644
--- a/src/arch/riscv/insts/static_inst.hh
+++ b/src/arch/riscv/insts/static_inst.hh
@@ -51,6 +51,15 @@
public:
void advancePC(PCState &pc) const override { pc.advance(); }
+PCState
+buildRetPC(const PCState &curPC, const PCState &callPC) const override
+{
+PCState retPC = callPC;
+retPC.advance();
+retPC.pc(curPC.npc());
+return retPC;
+}
+
size_t
asBytes(void *buf, size_t size) override
{
diff --git a/src/arch/riscv/utility.hh b/src/arch/riscv/utility.hh
index 32cf046..3bbbd71 100644
--- a/src/arch/riscv/utility.hh
+++ b/src/arch/riscv/utility.hh
@@ -98,15 +98,6 @@
&& (reinterpret_cast(val)&0x0004ULL);
}
-inline PCState
-buildRetPC(
[gem5-dev] Change in gem5/gem5[develop]: sim: Eliminate the generic PseudoInstABI.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39319 )
Change subject: sim: Eliminate the generic PseudoInstABI.
..
sim: Eliminate the generic PseudoInstABI.
Calls to gem5 ops are now handled by locally defined ABIs in each of the
ISAs that support them.
Change-Id: I30aac7b49fa8dc8e18aa7724338d1fd2adacda90
---
M src/sim/pseudo_inst.hh
1 file changed, 3 insertions(+), 24 deletions(-)
diff --git a/src/sim/pseudo_inst.hh b/src/sim/pseudo_inst.hh
index d244adb..b0b65c6 100644
--- a/src/sim/pseudo_inst.hh
+++ b/src/sim/pseudo_inst.hh
@@ -45,35 +45,14 @@
class ThreadContext;
-#include "arch/utility.hh"
#include "base/bitfield.hh"
+#include "base/logging.hh"
+#include "base/trace.hh"
#include "base/types.hh" // For Tick and Addr data types.
+#include "cpu/thread_context.hh"
#include "debug/PseudoInst.hh"
#include "sim/guest_abi.hh"
-struct PseudoInstABI
-{
-using State = int;
-};
-
-namespace GuestABI
-{
-
-template <>
-struct Argument
-{
-static uint64_t
-get(ThreadContext *tc, PseudoInstABI::State &state)
-{
-uint64_t result =
-TheISA::getArgument(tc, state, sizeof(uint64_t), false);
-state++;
-return result;
-}
-};
-
-} // namespace GuestABI
-
namespace PseudoInst
{
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39319
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: I30aac7b49fa8dc8e18aa7724338d1fd2adacda90
Gerrit-Change-Number: 39319
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]: arm: Export the mostly generic syscall ABI.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39315 )
Change subject: arm: Export the mostly generic syscall ABI.
..
arm: Export the mostly generic syscall ABI.
This ABI is also applicable for gem5 ops. Rather than have the gem5 ops
use the syscall ABI, this change exports the syscall ABI and renames it
the "reg" ABI, or in other words an ABI which only uses registers. The
SE workload class then just creates a local name for the "reg" ABI so it
can continue to use it for system calls.
Change-Id: Ifaa38a94d6f0d49b8a2e515e02ce94472a499a00
---
M src/arch/arm/SConscript
R src/arch/arm/reg_abi.cc
A src/arch/arm/reg_abi.hh
M src/arch/arm/se_workload.hh
4 files changed, 83 insertions(+), 45 deletions(-)
diff --git a/src/arch/arm/SConscript b/src/arch/arm/SConscript
index 31e83a7..1d6799e 100644
--- a/src/arch/arm/SConscript
+++ b/src/arch/arm/SConscript
@@ -89,11 +89,11 @@
Source('process.cc')
Source('qarma.cc')
Source('remote_gdb.cc')
+Source('reg_abi.cc')
Source('semihosting.cc')
Source('system.cc')
Source('table_walker.cc')
Source('self_debug.cc')
-Source('se_workload.cc')
Source('stage2_mmu.cc')
Source('stage2_lookup.cc')
Source('tlb.cc')
diff --git a/src/arch/arm/se_workload.cc b/src/arch/arm/reg_abi.cc
similarity index 87%
rename from src/arch/arm/se_workload.cc
rename to src/arch/arm/reg_abi.cc
index 72abb8d..ba1511c 100644
--- a/src/arch/arm/se_workload.cc
+++ b/src/arch/arm/reg_abi.cc
@@ -25,17 +25,12 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-#include "arch/arm/se_workload.hh"
+#include "arch/arm/reg_abi.hh"
namespace ArmISA
{
-const std::vector SEWorkload::SyscallABI32::ArgumentRegs = {
-0, 1, 2, 3, 4, 5, 6
-};
-
-const std::vector SEWorkload::SyscallABI64::ArgumentRegs = {
-0, 1, 2, 3, 4, 5, 6
-};
+const std::vector RegABI32::ArgumentRegs = {0, 1, 2, 3, 4, 5, 6};
+const std::vector RegABI64::ArgumentRegs = {0, 1, 2, 3, 4, 5, 6};
} // namespace ArmISA
diff --git a/src/arch/arm/reg_abi.hh b/src/arch/arm/reg_abi.hh
new file mode 100644
index 000..eb87eff
--- /dev/null
+++ b/src/arch/arm/reg_abi.hh
@@ -0,0 +1,76 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met: redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer;
+ * redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution;
+ * neither the name of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __ARCH_ARM_REG_ABI_HH__
+#define __ARCH_ARM_REG_ABI_HH__
+
+#include
+
+#include "base/logging.hh"
+#include "sim/syscall_abi.hh"
+
+namespace ArmISA
+{
+
+struct RegABI32 : public GenericSyscallABI32
+{
+static const std::vector ArgumentRegs;
+};
+
+struct RegABI64 : public GenericSyscallABI64
+{
+static const std::vector ArgumentRegs;
+};
+
+} // namespace ArmISA
+
+namespace GuestABI
+{
+
+template
+struct Argument::value &&
+ABI::template IsWide::value>>
+{
+static Arg
+get(ThreadContext *tc, typename ABI::State &state)
+{
+// 64 bit arguments are passed starting in an even register.
+if (state % 2)
+state++;
+panic_if(state + 1 >= ABI::ArgumentRegs.size(),
+"Ran out of syscall argument registers.");
+auto low = ABI::ArgumentRegs[state++];
+auto high = ABI::ArgumentRegs[state++];
+return (Arg)ABI::mergeRegs(tc, low, high);
+}
+};
+
+} // namespace GuestABI
+
+#endif // __ARCH_ARM_GEM5_OP_HH__
diff --git a/src/arch/arm/se_workload.hh b/src/arch/arm/se_workload.hh
index cad350e..8538edd 100644
--- a/src/arch/a
[gem5-dev] Change in gem5/gem5[develop]: riscv: Export the system call ABI for use in gem5 ops.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39318 )
Change subject: riscv: Export the system call ABI for use in gem5 ops.
..
riscv: Export the system call ABI for use in gem5 ops.
This ABI is effectively used by both the gem5 ops and system calls, in
system calls because it only relies on registers, and in gem5 ops by
inheritance.
Even though these ABIs happen to be the same and were initially defined
to be the same, this change creates a root "reg" ABI which will act as a
root for both so that there isn't an implication that changes to one
should be changes to both.
Change-Id: I8726d8628503be2ad7616a71cc48b66f13e7d955
---
M src/arch/riscv/SConscript
M src/arch/riscv/isa/formats/m5ops.isa
M src/arch/riscv/isa/includes.isa
R src/arch/riscv/reg_abi.cc
C src/arch/riscv/reg_abi.hh
M src/arch/riscv/registers.hh
M src/arch/riscv/se_workload.hh
7 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/src/arch/riscv/SConscript b/src/arch/riscv/SConscript
index 0179fbc..472264f 100644
--- a/src/arch/riscv/SConscript
+++ b/src/arch/riscv/SConscript
@@ -51,8 +51,8 @@
Source('process.cc')
Source('pagetable.cc')
Source('pagetable_walker.cc')
+Source('reg_abi.cc')
Source('remote_gdb.cc')
-Source('se_workload.cc')
Source('tlb.cc')
Source('linux/se_workload.cc')
diff --git a/src/arch/riscv/isa/formats/m5ops.isa
b/src/arch/riscv/isa/formats/m5ops.isa
index 11834f6..986438b 100644
--- a/src/arch/riscv/isa/formats/m5ops.isa
+++ b/src/arch/riscv/isa/formats/m5ops.isa
@@ -36,12 +36,11 @@
def format M5Op() {{
-iop = InstObjParams(name, Name, 'PseudoOp',
-'uint64_t result;\n'
-'PseudoInst::pseudoInst('
-'xc->tcBase(), M5FUNC, result);\n'
-'a0 = result',
-['IsNonSpeculative', 'IsSerializeAfter'])
+iop = InstObjParams(name, Name, 'PseudoOp', '''
+uint64_t result;
+PseudoInst::pseudoInst(xc->tcBase(), M5FUNC, result);
+a0 = result''',
+['IsNonSpeculative', 'IsSerializeAfter'])
header_output = BasicDeclare.subst(iop)
decoder_output = BasicConstructor.subst(iop)
decode_block = BasicDecode.subst(iop)
diff --git a/src/arch/riscv/isa/includes.isa
b/src/arch/riscv/isa/includes.isa
index 009f143..d1caf7a 100644
--- a/src/arch/riscv/isa/includes.isa
+++ b/src/arch/riscv/isa/includes.isa
@@ -82,6 +82,7 @@
#include "arch/generic/memhelpers.hh"
#include "arch/riscv/faults.hh"
#include "arch/riscv/mmu.hh"
+#include "arch/riscv/reg_abi.hh"
#include "arch/riscv/registers.hh"
#include "arch/riscv/utility.hh"
#include "base/condcodes.hh"
diff --git a/src/arch/riscv/se_workload.cc b/src/arch/riscv/reg_abi.cc
similarity index 91%
rename from src/arch/riscv/se_workload.cc
rename to src/arch/riscv/reg_abi.cc
index ce4679c..25aee6f 100644
--- a/src/arch/riscv/se_workload.cc
+++ b/src/arch/riscv/reg_abi.cc
@@ -25,13 +25,11 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-#include "arch/riscv/se_workload.hh"
+#include "arch/riscv/reg_abi.hh"
namespace RiscvISA
{
-const std::vector SEWorkload::SyscallABI::ArgumentRegs = {
-10, 11, 12, 13, 14, 15, 16
-};
+const std::vector RegABI64::ArgumentRegs = {10, 11, 12, 13, 14, 15,
16};
} // namespace RiscvISA
diff --git a/src/arch/riscv/se_workload.cc b/src/arch/riscv/reg_abi.hh
similarity index 82%
copy from src/arch/riscv/se_workload.cc
copy to src/arch/riscv/reg_abi.hh
index ce4679c..492c117 100644
--- a/src/arch/riscv/se_workload.cc
+++ b/src/arch/riscv/reg_abi.hh
@@ -25,13 +25,22 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-#include "arch/riscv/se_workload.hh"
+#ifndef __ARCH_RISCV_REG_ABI_HH__
+#define __ARCH_RISCV_REG_ABI_HH__
+
+#include
+
+#include "sim/syscall_abi.hh"
namespace RiscvISA
{
-const std::vector SEWorkload::SyscallABI::ArgumentRegs = {
-10, 11, 12, 13, 14, 15, 16
+//FIXME RISCV needs to handle 64 bit arguments in its 32 bit ISA.
+struct RegABI64 : public GenericSyscallABI64
+{
+static const std::vector ArgumentRegs;
};
} // namespace RiscvISA
+
+#endif // __ARCH_RISCV_REG_ABI_HH__
diff --git a/src/arch/riscv/registers.hh b/src/arch/riscv/registers.hh
index 84a1924..9721635 100644
--- a/src/arch/riscv/registers.hh
+++ b/src/arch/riscv/registers.hh
@@ -93,11 +93,8 @@
const int ZeroReg = 0;
const int ReturnAddrReg = 1;
const int StackPointerReg = 2;
-const int GlobalPointerReg = 3;
const int ThreadPointerReg = 4;
-const int FramePointerReg = 8;
const int ReturnValueReg = 10;
-const std::vector ReturnValueRegs = {10, 11};
const std::vector ArgumentRegs = {10, 11, 12, 13, 14, 15, 16, 17};
const int AMOTempReg = 32;
diff --git a/src/arch/riscv/se_workload.hh b/src/arch/riscv/se_workload.hh
index e0be5a1.
[gem5-dev] Change in gem5/gem5[develop]: arch: Eliminate the getArgument function.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39321 )
Change subject: arch: Eliminate the getArgument function.
..
arch: Eliminate the getArgument function.
This ISA switched function is no longer used. Eliminate it, and reduce
the number of functions used in the switched utility.hh header by one.
Change-Id: Ic6020c5fa6d976d9dbf1e9f517809acf9b0b7cd8
---
M src/arch/arm/utility.cc
M src/arch/arm/utility.hh
M src/arch/mips/utility.cc
M src/arch/mips/utility.hh
M src/arch/power/utility.cc
M src/arch/power/utility.hh
M src/arch/riscv/utility.hh
M src/arch/sparc/utility.cc
M src/arch/sparc/utility.hh
M src/arch/x86/utility.cc
M src/arch/x86/utility.hh
11 files changed, 2 insertions(+), 141 deletions(-)
diff --git a/src/arch/arm/utility.cc b/src/arch/arm/utility.cc
index 93e0a78..31408f6 100644
--- a/src/arch/arm/utility.cc
+++ b/src/arch/arm/utility.cc
@@ -53,64 +53,6 @@
namespace ArmISA
{
-uint64_t
-getArgument(ThreadContext *tc, int &number, uint16_t size, bool fp)
-{
-panic_if(!FullSystem,
-"getArgument() only implemented for full system mode.");
-
-panic_if(fp, "getArgument(): Floating point arguments not
implemented");
-
-if (inAArch64(tc)) {
-if (size == (uint16_t)(-1))
-size = sizeof(uint64_t);
-
-if (number < 8 /*NumArgumentRegs64*/) {
- return tc->readIntReg(number);
-} else {
-panic("getArgument(): No support reading stack args for
AArch64\n");
-}
-} else {
-if (size == (uint16_t)(-1))
-size = sizeof(uint32_t);
-
-if (number < NumArgumentRegs) {
-// If the argument is 64 bits, it must be in an even regiser
-// number. Increment the number here if it isn't even.
-if (size == sizeof(uint64_t)) {
-if ((number % 2) != 0)
-number++;
-// Read the two halves of the data. Number is inc here to
-// get the second half of the 64 bit reg.
-uint64_t tmp;
-tmp = tc->readIntReg(number++);
-tmp |= tc->readIntReg(number) << 32;
-return tmp;
-} else {
- return tc->readIntReg(number);
-}
-} else {
-Addr sp = tc->readIntReg(StackPointerReg);
-PortProxy &vp = tc->getVirtProxy();
-uint64_t arg;
-if (size == sizeof(uint64_t)) {
-// If the argument is even it must be aligned
-if ((number % 2) != 0)
-number++;
-arg = vp.read(sp +
-(number-NumArgumentRegs) * sizeof(uint32_t));
-// since two 32 bit args == 1 64 bit arg, increment number
-number++;
-} else {
-arg = vp.read(sp +
- (number-NumArgumentRegs) *
sizeof(uint32_t));
-}
-return arg;
-}
-}
-panic("getArgument() should always return\n");
-}
-
static void
copyVecRegs(ThreadContext *src, ThreadContext *dest)
{
diff --git a/src/arch/arm/utility.hh b/src/arch/arm/utility.hh
index 6ec6403..fcaefbb 100644
--- a/src/arch/arm/utility.hh
+++ b/src/arch/arm/utility.hh
@@ -400,8 +400,6 @@
bool SPAlignmentCheckEnabled(ThreadContext* tc);
-uint64_t getArgument(ThreadContext *tc, int &number, uint16_t size, bool
fp);
-
inline void
advancePC(PCState &pc, const StaticInstPtr &inst)
{
diff --git a/src/arch/mips/utility.cc b/src/arch/mips/utility.cc
index 930c36b..cf8515f 100644
--- a/src/arch/mips/utility.cc
+++ b/src/arch/mips/utility.cc
@@ -44,12 +44,6 @@
namespace MipsISA {
uint64_t
-getArgument(ThreadContext *tc, int &number, uint16_t size, bool fp)
-{
-panic("getArgument() not implemented\n");
-}
-
-uint64_t
fpConvert(ConvertType cvt_type, double fp_val)
{
diff --git a/src/arch/mips/utility.hh b/src/arch/mips/utility.hh
index c156c82..a0d222e 100644
--- a/src/arch/mips/utility.hh
+++ b/src/arch/mips/utility.hh
@@ -49,8 +49,6 @@
return ret;
}
-uint64_t getArgument(ThreadContext *tc, int &number, uint16_t size, bool
fp);
-
//
// Floating Point Utility Functions
diff --git a/src/arch/power/utility.cc b/src/arch/power/utility.cc
index da4748d..bed0be9 100644
--- a/src/arch/power/utility.cc
+++ b/src/arch/power/utility.cc
@@ -55,11 +55,4 @@
dest->pcState(src->pcState());
}
-uint64_t
-getArgument(ThreadContext *tc, int &number, uint16_t size, bool fp)
-{
-panic("getArgument not implemented for POWER.\n");
-return 0;
-}
-
} // namespace PowerISA
diff --git a/src/arch/power/utility.hh b/src/arch/power/utility.hh
index ba28f07..80b98c6 100644
--- a/src/arch/power/utility.hh
+++ b/src/arch/power/utility.hh
@@ -58,8 +58,6 @@
[gem5-dev] Change in gem5/gem5[develop]: arch,cpu: Move getExecutingAsid to the ISA class.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39322 )
Change subject: arch,cpu: Move getExecutingAsid to the ISA class.
..
arch,cpu: Move getExecutingAsid to the ISA class.
This function was switched based on the ISA, and returned 0 on
everything except SPARC and ARM. It was used only when tracing
instruction execution with --debug-flags=Exec.
Change-Id: I70c274cb76fb229d0e2bc606ba41f458ed18ab81
---
M src/arch/arm/isa.hh
M src/arch/arm/utility.hh
M src/arch/generic/isa.hh
M src/arch/mips/utility.hh
M src/arch/power/utility.hh
M src/arch/riscv/utility.hh
M src/arch/sparc/isa.hh
M src/arch/sparc/utility.hh
M src/arch/x86/utility.hh
M src/cpu/exetrace.cc
10 files changed, 14 insertions(+), 38 deletions(-)
diff --git a/src/arch/arm/isa.hh b/src/arch/arm/isa.hh
index dce5e37..97b41cc 100644
--- a/src/arch/arm/isa.hh
+++ b/src/arch/arm/isa.hh
@@ -886,6 +886,12 @@
const Params ¶ms() const;
ISA(const Params &p);
+
+uint64_t
+getExecutingAsid() const override
+{
+return readMiscRegNoEffect(MISCREG_CONTEXTIDR);
+}
};
}
diff --git a/src/arch/arm/utility.hh b/src/arch/arm/utility.hh
index fcaefbb..e255b1c 100644
--- a/src/arch/arm/utility.hh
+++ b/src/arch/arm/utility.hh
@@ -409,12 +409,6 @@
Addr truncPage(Addr addr);
Addr roundPage(Addr addr);
-inline uint64_t
-getExecutingAsid(ThreadContext *tc)
-{
-return tc->readMiscReg(MISCREG_CONTEXTIDR);
-}
-
// Decodes the register index to access based on the fields used in a MSR
// or MRS instruction
bool
diff --git a/src/arch/generic/isa.hh b/src/arch/generic/isa.hh
index df07763..9ea2d9f 100644
--- a/src/arch/generic/isa.hh
+++ b/src/arch/generic/isa.hh
@@ -57,6 +57,8 @@
{}
virtual void setThreadContext(ThreadContext *_tc) { tc = _tc; }
+
+virtual uint64_t getExecutingAsid() const { return 0; }
};
#endif // __ARCH_GENERIC_ISA_HH__
diff --git a/src/arch/mips/utility.hh b/src/arch/mips/utility.hh
index a0d222e..0cb9349 100644
--- a/src/arch/mips/utility.hh
+++ b/src/arch/mips/utility.hh
@@ -106,12 +106,6 @@
pc.advance();
}
-inline uint64_t
-getExecutingAsid(ThreadContext *tc)
-{
-return 0;
-}
-
};
diff --git a/src/arch/power/utility.hh b/src/arch/power/utility.hh
index 80b98c6..9092a23 100644
--- a/src/arch/power/utility.hh
+++ b/src/arch/power/utility.hh
@@ -64,12 +64,6 @@
return 0;
}
-inline uint64_t
-getExecutingAsid(ThreadContext *tc)
-{
-return 0;
-}
-
} // namespace PowerISA
diff --git a/src/arch/riscv/utility.hh b/src/arch/riscv/utility.hh
index 1f82d2b..c2f4ac8 100644
--- a/src/arch/riscv/utility.hh
+++ b/src/arch/riscv/utility.hh
@@ -163,12 +163,6 @@
return true;
}
-inline uint64_t
-getExecutingAsid(ThreadContext *tc)
-{
-return 0;
-}
-
} // namespace RiscvISA
#endif // __ARCH_RISCV_UTILITY_HH__
diff --git a/src/arch/sparc/isa.hh b/src/arch/sparc/isa.hh
index 2881384..21143dd 100644
--- a/src/arch/sparc/isa.hh
+++ b/src/arch/sparc/isa.hh
@@ -215,6 +215,11 @@
int flattenCCIndex(int reg) const { return reg; }
int flattenMiscIndex(int reg) const { return reg; }
+uint64_t
+getExecutingAsid() const override
+{
+return readMiscRegNoEffect(MISCREG_MMU_P_CONTEXT);
+}
typedef SparcISAParams Params;
const Params ¶ms() const;
diff --git a/src/arch/sparc/utility.hh b/src/arch/sparc/utility.hh
index 8a638e1..18b5164 100644
--- a/src/arch/sparc/utility.hh
+++ b/src/arch/sparc/utility.hh
@@ -68,12 +68,6 @@
inst->advancePC(pc);
}
-inline uint64_t
-getExecutingAsid(ThreadContext *tc)
-{
-return tc->readMiscRegNoEffect(MISCREG_MMU_P_CONTEXT);
-}
-
} // namespace SparcISA
#endif
diff --git a/src/arch/x86/utility.hh b/src/arch/x86/utility.hh
index 4ae8102..1ff7b16 100644
--- a/src/arch/x86/utility.hh
+++ b/src/arch/x86/utility.hh
@@ -74,13 +74,6 @@
inst->advancePC(pc);
}
-inline uint64_t
-getExecutingAsid(ThreadContext *tc)
-{
-return 0;
-}
-
-
/**
* Reconstruct the rflags register from the internal gem5 register
* state.
diff --git a/src/cpu/exetrace.cc b/src/cpu/exetrace.cc
index 4980c91..17c877e 100644
--- a/src/cpu/exetrace.cc
+++ b/src/cpu/exetrace.cc
@@ -70,7 +70,7 @@
}
if (Debug::ExecAsid)
-outs << "A" << dec << TheISA::getExecutingAsid(thread) << " ";
+outs << "A" << dec << thread->getIsaPtr()->getExecutingAsid()
<< " ";
if (Debug::ExecThread)
outs << "T" << thread->threadId() << " : ";
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39322
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: I70c274cb76fb229d0e2bc606ba41f458ed18ab81
Gerrit-Change-Number: 39322
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black
G
[gem5-dev] Change in gem5/gem5[develop]: arm: Use the "reg" ABI for gem5 ops.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39316 )
Change subject: arm: Use the "reg" ABI for gem5 ops.
..
arm: Use the "reg" ABI for gem5 ops.
The generic PseudoInstABI just calls back into the ISA specific
getArgument function, and that adds a lot of handling for cases that
aren't used and, besides those, basically just boils down to the "reg"
ABI anyway.
Change-Id: I57e738631dbccbf89cba3a6ca62b1f954b39e959
---
M src/arch/arm/isa/includes.isa
M src/arch/arm/isa/insts/m5ops.isa
M src/arch/arm/tlb.cc
3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/src/arch/arm/isa/includes.isa b/src/arch/arm/isa/includes.isa
index 13b47c8..6af382a 100644
--- a/src/arch/arm/isa/includes.isa
+++ b/src/arch/arm/isa/includes.isa
@@ -105,6 +105,7 @@
#include "arch/arm/htm.hh"
#include "arch/arm/isa_traits.hh"
#include "arch/arm/pauth_helpers.hh"
+#include "arch/arm/reg_abi.hh"
#include "arch/arm/semihosting.hh"
#include "arch/arm/utility.hh"
#include "arch/generic/memhelpers.hh"
diff --git a/src/arch/arm/isa/insts/m5ops.isa
b/src/arch/arm/isa/insts/m5ops.isa
index 48d533d..f3a8eeb 100644
--- a/src/arch/arm/isa/insts/m5ops.isa
+++ b/src/arch/arm/isa/insts/m5ops.isa
@@ -38,8 +38,13 @@
let {{
gem5OpCode = '''
uint64_t ret;
-bool recognized = PseudoInst::pseudoInst(
-xc->tcBase(), bits(machInst, 23, 16), ret);
+bool recognized;
+int func = bits(machInst, 23, 16);
+auto *tc = xc->tcBase();
+if (inAArch64(tc))
+recognized = PseudoInst::pseudoInst(tc, func, ret);
+else
+recognized = PseudoInst::pseudoInst(tc, func, ret);
if (!recognized)
fault = std::make_shared(machInst, true);
'''
diff --git a/src/arch/arm/tlb.cc b/src/arch/arm/tlb.cc
index 5d2ed90..91c7088 100644
--- a/src/arch/arm/tlb.cc
+++ b/src/arch/arm/tlb.cc
@@ -47,6 +47,7 @@
#include "arch/arm/faults.hh"
#include "arch/arm/isa.hh"
#include "arch/arm/pagetable.hh"
+#include "arch/arm/reg_abi.hh"
#include "arch/arm/self_debug.hh"
#include "arch/arm/stage2_lookup.hh"
#include "arch/arm/stage2_mmu.hh"
@@ -146,9 +147,14 @@
[func, mode](ThreadContext *tc, PacketPtr pkt) -> Cycles
{
uint64_t ret;
-PseudoInst::pseudoInst(tc, func, ret);
+if (inAArch64(tc))
+PseudoInst::pseudoInst(tc, func, ret);
+else
+PseudoInst::pseudoInst(tc, func, ret);
+
if (mode == Read)
pkt->setLE(ret);
+
return Cycles(1);
}
);
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39316
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: I57e738631dbccbf89cba3a6ca62b1f954b39e959
Gerrit-Change-Number: 39316
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,cpu: Move the inUserMode function to the ISA object.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39323 )
Change subject: arch,cpu: Move the inUserMode function to the ISA object.
..
arch,cpu: Move the inUserMode function to the ISA object.
This function is used when tracing execution with --debug-flags=Exec.
The data used by the function (now method) is stored in the ISA object,
and so that's a logical place to move it.
Change-Id: I624f9365124679343e988cabfb4e1929225b439a
---
M src/arch/arm/fastmodel/iris/isa.hh
M src/arch/arm/isa.hh
M src/arch/arm/utility.hh
M src/arch/generic/isa.hh
M src/arch/mips/isa.hh
M src/arch/mips/utility.hh
M src/arch/power/isa.hh
M src/arch/power/utility.hh
M src/arch/riscv/isa.hh
M src/arch/riscv/utility.hh
M src/arch/sparc/isa.hh
M src/arch/sparc/utility.hh
M src/arch/x86/isa.hh
M src/arch/x86/utility.hh
M src/cpu/exetrace.cc
15 files changed, 66 insertions(+), 65 deletions(-)
diff --git a/src/arch/arm/fastmodel/iris/isa.hh
b/src/arch/arm/fastmodel/iris/isa.hh
index d9646df..a7ae7b5 100644
--- a/src/arch/arm/fastmodel/iris/isa.hh
+++ b/src/arch/arm/fastmodel/iris/isa.hh
@@ -28,6 +28,7 @@
#ifndef __ARCH_ARM_FASTMODEL_IRIS_ISA_HH__
#define __ARCH_ARM_FASTMODEL_IRIS_ISA_HH__
+#include "arch/arm/utility.hh"
#include "arch/generic/isa.hh"
namespace Iris
@@ -39,6 +40,13 @@
ISA(const Params &p) : BaseISA(p) {}
void serialize(CheckpointOut &cp) const;
+
+bool
+inUserMode() const override
+{
+CPSR cpsr = tc->readMiscRegNoEffect(MISCREG_CPSR);
+return ::inUserMode(cpsr);
+}
};
} // namespace Iris
diff --git a/src/arch/arm/isa.hh b/src/arch/arm/isa.hh
index 97b41cc..d350378 100644
--- a/src/arch/arm/isa.hh
+++ b/src/arch/arm/isa.hh
@@ -48,6 +48,7 @@
#include "arch/arm/system.hh"
#include "arch/arm/tlb.hh"
#include "arch/arm/types.hh"
+#include "arch/arm/utility.hh"
#include "arch/generic/isa.hh"
#include "arch/generic/traits.hh"
#include "debug/Checkpoint.hh"
@@ -892,6 +893,13 @@
{
return readMiscRegNoEffect(MISCREG_CONTEXTIDR);
}
+
+bool
+inUserMode() const override
+{
+CPSR cpsr = miscRegs[MISCREG_CPSR];
+return ArmISA::inUserMode(cpsr);
+}
};
}
diff --git a/src/arch/arm/utility.hh b/src/arch/arm/utility.hh
index e255b1c..bd043df 100644
--- a/src/arch/arm/utility.hh
+++ b/src/arch/arm/utility.hh
@@ -111,23 +111,11 @@
}
static inline bool
-inUserMode(ThreadContext *tc)
-{
-return inUserMode(tc->readMiscRegNoEffect(MISCREG_CPSR));
-}
-
-static inline bool
inPrivilegedMode(CPSR cpsr)
{
return !inUserMode(cpsr);
}
-static inline bool
-inPrivilegedMode(ThreadContext *tc)
-{
-return !inUserMode(tc);
-}
-
bool isSecure(ThreadContext *tc);
bool inAArch64(ThreadContext *tc);
diff --git a/src/arch/generic/isa.hh b/src/arch/generic/isa.hh
index 9ea2d9f..12c58ad 100644
--- a/src/arch/generic/isa.hh
+++ b/src/arch/generic/isa.hh
@@ -59,6 +59,7 @@
virtual void setThreadContext(ThreadContext *_tc) { tc = _tc; }
virtual uint64_t getExecutingAsid() const { return 0; }
+virtual bool inUserMode() const = 0;
};
#endif // __ARCH_GENERIC_ISA_HH__
diff --git a/src/arch/mips/isa.hh b/src/arch/mips/isa.hh
index 1b81046..6242c62 100644
--- a/src/arch/mips/isa.hh
+++ b/src/arch/mips/isa.hh
@@ -142,6 +142,26 @@
// dummy
int flattenCCIndex(int reg) const { return reg; }
int flattenMiscIndex(int reg) const { return reg; }
+
+bool
+inUserMode() const override
+{
+RegVal Stat = readMiscRegNoEffect(MISCREG_STATUS);
+RegVal Dbg = readMiscRegNoEffect(MISCREG_DEBUG);
+
+if (// EXL, ERL or CU0 set, CP0 accessible
+(Stat & 0x1006) == 0 &&
+// DM bit set, CP0 accessible
+(Dbg & 0x4000) == 0 &&
+// KSU = 0, kernel mode is base mode
+(Stat & 0x0018) != 0) {
+// Unable to use Status_CU0, etc directly,
+// using bitfields & masks.
+return true;
+} else {
+return false;
+}
+}
};
}
diff --git a/src/arch/mips/utility.hh b/src/arch/mips/utility.hh
index 0cb9349..6fb211d 100644
--- a/src/arch/mips/utility.hh
+++ b/src/arch/mips/utility.hh
@@ -65,22 +65,6 @@
bool isQnan(void *val_ptr, int size);
bool isSnan(void *val_ptr, int size);
-static inline bool
-inUserMode(ThreadContext *tc)
-{
-RegVal Stat = tc->readMiscReg(MISCREG_STATUS);
-RegVal Dbg = tc->readMiscReg(MISCREG_DEBUG);
-
-if ((Stat & 0x1006) == 0 && // EXL, ERL or CU0 set, CP0 accessible
-(Dbg & 0x4000) == 0 && // DM bit set, CP0 accessible
-(Stat & 0x0018) != 0) { // KSU = 0, kernel mode is base mode
-// Unable to use Status_CU0, etc directly, using bitfields & masks
[gem5-dev] Change in gem5/gem5[develop]: arm,kern: Stop using the getArgument function for kernel events.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39320 )
Change subject: arm,kern: Stop using the getArgument function for kernel
events.
..
arm,kern: Stop using the getArgument function for kernel events.
This change plumbs through an ABI to use with the GuestABI mechanism so
that the ISA switched getArgument function is no longer used.
Change-Id: I0d394dcfd7ad6fa745b6ef2aa62973167108f0c3
---
M src/arch/arm/freebsd/fs_workload.cc
M src/arch/arm/fs_workload.hh
M src/arch/arm/linux/fs_workload.cc
M src/kern/freebsd/events.cc
M src/kern/freebsd/events.hh
M src/kern/linux/events.cc
M src/kern/linux/events.hh
7 files changed, 64 insertions(+), 37 deletions(-)
diff --git a/src/arch/arm/freebsd/fs_workload.cc
b/src/arch/arm/freebsd/fs_workload.cc
index 8eb3c70..9638b3c 100644
--- a/src/arch/arm/freebsd/fs_workload.cc
+++ b/src/arch/arm/freebsd/fs_workload.cc
@@ -68,8 +68,7 @@
"oops_exit", "Kernel oops in guest");
}
-skipUDelay = addKernelFuncEvent>(
-"DELAY", "DELAY", 1000, 0);
+skipUDelay = addSkipFunc("DELAY", "DELAY", 1000, 0);
}
void
diff --git a/src/arch/arm/fs_workload.hh b/src/arch/arm/fs_workload.hh
index 4461670..d7f1738 100644
--- a/src/arch/arm/fs_workload.hh
+++ b/src/arch/arm/fs_workload.hh
@@ -44,6 +44,8 @@
#include
#include
+#include "arch/arm/aapcs32.hh"
+#include "arch/arm/aapcs64.hh"
#include "kern/linux/events.hh"
#include "params/ArmFsWorkload.hh"
#include "sim/kernel_workload.hh"
@@ -86,6 +88,34 @@
*/
Loader::ObjectFile *getBootLoader(Loader::ObjectFile *const obj);
+template class FuncEvent,
+ typename... Args>
+PCEvent *
+addSkipFunc(Args... args)
+{
+if (getArch() == Loader::Arm64) {
+return addKernelFuncEvent>(
+std::forward(args)...);
+} else {
+return addKernelFuncEvent>(
+std::forward(args)...);
+}
+}
+
+template class FuncEvent,
+ typename... Args>
+PCEvent *
+addSkipFuncOrPanic(Args... args)
+{
+if (getArch() == Loader::Arm64) {
+return addKernelFuncEventOrPanic>(
+std::forward(args)...);
+} else {
+return addKernelFuncEventOrPanic>(
+std::forward(args)...);
+}
+}
+
public:
typedef ArmFsWorkloadParams Params;
const Params &
diff --git a/src/arch/arm/linux/fs_workload.cc
b/src/arch/arm/linux/fs_workload.cc
index f05fd6f..5ada99a 100644
--- a/src/arch/arm/linux/fs_workload.cc
+++ b/src/arch/arm/linux/fs_workload.cc
@@ -230,28 +230,23 @@
// With ARM udelay() is #defined to __udelay
// newer kernels use __loop_udelay and __loop_const_udelay symbols
-skipUDelay = addKernelFuncEvent>(
+skipUDelay = addSkipFunc(
"__loop_udelay", "__udelay", 1000, 0);
-if (!skipUDelay)
-skipUDelay = addKernelFuncEventOrPanic>(
- "__udelay", "__udelay", 1000, 0);
+if (!skipUDelay) {
+skipUDelay = addSkipFuncOrPanic(
+"__udelay", "__udelay", 1000, 0);
+}
// constant arguments to udelay() have some precomputation done ahead
of
// time. Constant comes from code.
-skipConstUDelay = addKernelFuncEvent>(
+skipConstUDelay = addSkipFunc(
"__loop_const_udelay", "__const_udelay", 1000, 107374);
if (!skipConstUDelay) {
-skipConstUDelay = addKernelFuncEventOrPanic>(
+skipConstUDelay = addSkipFuncOrPanic(
"__const_udelay", "__const_udelay", 1000, 107374);
}
-if (getArch() == Loader::Arm64) {
-debugPrintk = addKernelFuncEvent<
-DebugPrintk>("dprintk");
-} else {
-debugPrintk = addKernelFuncEvent<
-DebugPrintk>("dprintk");
-}
+debugPrintk = addSkipFunc("dprintk");
}
void
diff --git a/src/kern/freebsd/events.cc b/src/kern/freebsd/events.cc
index 0c4c613..e6b66fa 100644
--- a/src/kern/freebsd/events.cc
+++ b/src/kern/freebsd/events.cc
@@ -45,13 +45,8 @@
{
void
-onUDelay(ThreadContext *tc, uint64_t div, uint64_t mul)
+onUDelay(ThreadContext *tc, uint64_t div, uint64_t mul, uint64_t time)
{
-int arg_num = 0;
-
-// Get the time in native size
-uint64_t time = TheISA::getArgument(tc, arg_num, (uint16_t)-1, false);
-
// convert parameter to ns
if (div)
time /= div;
diff --git a/src/kern/freebsd/events.hh b/src/kern/freebsd/events.hh
index 79894b6..ebf6128 100644
--- a/src/kern/freebsd/events.hh
+++ b/src/kern/freebsd/events.hh
@@ -34,18 +34,19 @@
#define __KERN_FREEBSD_EVENTS_HH__
#include "kern/system_events.hh"
+#include "sim/guest_abi.hh"
namespace FreeBSD
{
-void onUDelay(ThreadContext *tc, uint64_t div, uint64_t mul);
+void onUDelay(ThreadContext *tc, uint64_t div, uint64_t mul, uint64_t
time);
/** A class to skip udelay() an
[gem5-dev] Change in gem5/gem5[develop]: riscv: Get rid of some unused constants.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39317 )
Change subject: riscv: Get rid of some unused constants.
..
riscv: Get rid of some unused constants.
Change-Id: I464e86dc6bfcd333a0bee32e56d9dcaa6fdf682d
---
M src/arch/riscv/registers.hh
1 file changed, 0 insertions(+), 2 deletions(-)
diff --git a/src/arch/riscv/registers.hh b/src/arch/riscv/registers.hh
index ed8b916..84a1924 100644
--- a/src/arch/riscv/registers.hh
+++ b/src/arch/riscv/registers.hh
@@ -101,8 +101,6 @@
const std::vector ArgumentRegs = {10, 11, 12, 13, 14, 15, 16, 17};
const int AMOTempReg = 32;
-const int SyscallPseudoReturnReg = 10;
-const std::vector SyscallArgumentRegs = {10, 11, 12, 13, 14, 15, 16};
const int SyscallNumReg = 17;
const std::vector IntRegNames = {
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39317
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: I464e86dc6bfcd333a0bee32e56d9dcaa6fdf682d
Gerrit-Change-Number: 39317
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]: tests: Fix syntax error in cpu_tests/test.py
Giacomo Travaglini has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39295 )
Change subject: tests: Fix syntax error in cpu_tests/test.py
..
tests: Fix syntax error in cpu_tests/test.py
The testsuite was not loaded with the following error:
Exception thrown while loading
/scratch/jactra01/GEM/external/reviews/gem5/tests/gem5/cpu_tests/test.py
Signed-off-by: Giacomo Travaglini
Change-Id: I1e88b8957bb24471e1bb6113ffc7c78886b6ed70
---
M tests/gem5/cpu_tests/test.py
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/gem5/cpu_tests/test.py b/tests/gem5/cpu_tests/test.py
index 393ff26..ee56400 100644
--- a/tests/gem5/cpu_tests/test.py
+++ b/tests/gem5/cpu_tests/test.py
@@ -60,9 +60,9 @@
base_url = config.resource_url + '/gem5/cpu_tests/benchmarks/bin/'
isa_url = {
-constants.gcn3_x86_tag : base_url + "x86"
-constants.arm_tag : base_url + "arm"
-constants.riscv_tag : base_url + "riscv"
+constants.gcn3_x86_tag : base_url + "x86",
+constants.arm_tag : base_url + "arm",
+constants.riscv_tag : base_url + "riscv",
}
for isa in valid_isas:
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39295
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: I1e88b8957bb24471e1bb6113ffc7c78886b6ed70
Gerrit-Change-Number: 39295
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini
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]: mem: Add Units to mem stats
Hoa Nguyen has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39276 )
Change subject: mem: Add Units to mem stats
..
mem: Add Units to mem stats
Change-Id: Iab214b5d08eb1accc2b35af0c3aed7d30df5b5f3
Signed-off-by: Hoa Nguyen
---
M src/mem/abstract_mem.cc
M src/mem/cache/base.cc
M src/mem/cache/tags/base.cc
M src/mem/coherent_xbar.cc
M src/mem/ruby/slicc_interface/AbstractController.cc
M src/mem/ruby/structures/RubyPrefetcher.cc
M src/mem/ruby/system/HTMSequencer.cc
7 files changed, 148 insertions(+), 85 deletions(-)
diff --git a/src/mem/abstract_mem.cc b/src/mem/abstract_mem.cc
index 1238f17..0a5709a 100644
--- a/src/mem/abstract_mem.cc
+++ b/src/mem/abstract_mem.cc
@@ -114,19 +114,31 @@
AbstractMemory::MemStats::MemStats(AbstractMemory &_mem)
: Stats::Group(&_mem), mem(_mem),
-ADD_STAT(bytesRead, "Number of bytes read from this memory"),
-ADD_STAT(bytesInstRead,
- "Number of instructions bytes read from this memory"),
-ADD_STAT(bytesWritten, "Number of bytes written to this memory"),
-ADD_STAT(numReads, "Number of read requests responded to by this
memory"),
-ADD_STAT(numWrites,
- "Number of write requests responded to by this memory"),
-ADD_STAT(numOther, "Number of other requests responded to by this
memory"),
-ADD_STAT(bwRead, "Total read bandwidth from this memory (bytes/s)"),
-ADD_STAT(bwInstRead,
- "Instruction read bandwidth from this memory (bytes/s)"),
-ADD_STAT(bwWrite, "Write bandwidth from this memory (bytes/s)"),
-ADD_STAT(bwTotal, "Total bandwidth to/from this memory (bytes/s)")
+ADD_STAT_WITH_UNIT(bytesRead, UNIT_BYTE,
+ "Number of bytes read from this memory"),
+ADD_STAT_WITH_UNIT(bytesInstRead, UNIT_BYTE,
+ "Number of instructions bytes read from this
memory"),
+ADD_STAT_WITH_UNIT(bytesWritten, UNIT_BYTE,
+ "Number of bytes written to this memory"),
+ADD_STAT_WITH_UNIT(numReads, UNIT_COUNT,
+ "Number of read requests responded to by this
memory"),
+ADD_STAT_WITH_UNIT(numWrites, UNIT_COUNT,
+ "Number of write requests responded to by this
memory"),
+ADD_STAT_WITH_UNIT(numOther, UNIT_COUNT,
+ "Number of other requests responded to by this
memory"),
+ADD_STAT_WITH_UNIT(bwRead,
+ UNIT_RATE(Stats::Units::Byte, Stats::Units::Second),
+ "Total read bandwidth from this memory (bytes/s)"),
+ADD_STAT_WITH_UNIT(bwInstRead,
+ UNIT_RATE(Stats::Units::Byte, Stats::Units::Second),
+ "Instruction read bandwidth from this memory "
+ "(bytes/s)"),
+ADD_STAT_WITH_UNIT(bwWrite,
+ UNIT_RATE(Stats::Units::Byte, Stats::Units::Second),
+ "Write bandwidth from this memory (bytes/s)"),
+ADD_STAT_WITH_UNIT(bwTotal,
+ UNIT_RATE(Stats::Units::Byte, Stats::Units::Second),
+ "Total bandwidth to/from this memory (bytes/s)")
{
}
diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc
index cf1a9e8..6bed42c 100644
--- a/src/mem/cache/base.cc
+++ b/src/mem/cache/base.cc
@@ -2108,44 +2108,74 @@
BaseCache::CacheStats::CacheStats(BaseCache &c)
: Stats::Group(&c), cache(c),
-ADD_STAT(demandHits, "number of demand (read+write) hits"),
-ADD_STAT(overallHits, "number of overall hits"),
-ADD_STAT(demandMisses, "number of demand (read+write) misses"),
-ADD_STAT(overallMisses, "number of overall misses"),
-ADD_STAT(demandMissLatency, "number of demand (read+write) miss
cycles"),
-ADD_STAT(overallMissLatency, "number of overall miss cycles"),
-ADD_STAT(demandAccesses, "number of demand (read+write) accesses"),
-ADD_STAT(overallAccesses, "number of overall (read+write) accesses"),
-ADD_STAT(demandMissRate, "miss rate for demand accesses"),
-ADD_STAT(overallMissRate, "miss rate for overall accesses"),
-ADD_STAT(demandAvgMissLatency, "average overall miss latency"),
-ADD_STAT(overallAvgMissLatency, "average overall miss latency"),
-ADD_STAT(blocked_cycles, "number of cycles access was blocked"),
-ADD_STAT(blocked_causes, "number of cycles access was blocked"),
-ADD_STAT(avg_blocked,"average number of cycles each access was
blocked"),
-ADD_STAT(unusedPrefetches,
- "number of HardPF blocks evicted w/o reference"),
-ADD_STAT(writebacks, "number of writebacks"),
-ADD_STAT(demandMshrHits, "number of demand (read+write) MSHR hits"),
-ADD_STAT(overallMshrHits, "number of overall MSHR hits"),
-ADD_STAT(demandMshrMisses, "number of demand (read+write) MSHR
misses"),
-ADD_STAT(overallMshrMisses, "number of overall MSHR misses"),
-ADD_STAT(overallMsh
