[gem5-dev] Change in gem5/gem5[develop]: base: Add a rename helper to SymbolTable
Yu-hsin Wang has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43105 ) Change subject: base: Add a rename helper to SymbolTable .. base: Add a rename helper to SymbolTable In some cases, we want to move and copy the symbol table to another address. However, the name of symbol table should be unique. This rename helper provides a way to modify the name of symbol. Developers can use it to solve the conflict with this helper. Change-Id: I4627e06da3a03da57009d613188be117c75750a0 --- M src/base/loader/symtab.hh 1 file changed, 11 insertions(+), 0 deletions(-) diff --git a/src/base/loader/symtab.hh b/src/base/loader/symtab.hh index 5610544..a74bb76 100644 --- a/src/base/loader/symtab.hh +++ b/src/base/loader/symtab.hh @@ -154,6 +154,17 @@ } SymbolTablePtr +rename(std::function func) const +{ +SymTabOp op = [func](SymbolTable &symtab, const Symbol &symbol) { +Symbol sym = symbol; +func(sym.name); +symtab.insert(sym); +}; +return operate(op); +} + +SymbolTablePtr globals() const { return filterByBinding(Symbol::Binding::Global); -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43105 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: I4627e06da3a03da57009d613188be117c75750a0 Gerrit-Change-Number: 43105 Gerrit-PatchSet: 1 Gerrit-Owner: Yu-hsin Wang 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]: cpu: Narrow the dependencies of the StaticInst class.
Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/42965 ) Change subject: cpu: Narrow the dependencies of the StaticInst class. .. cpu: Narrow the dependencies of the StaticInst class. Include only the minimal number of headers to make StaticInst subclasses easier to test with unit tests. Change-Id: Ie2e1c01b77d1776b366d29982e481d62b6a2b8cf Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42965 Maintainer: Gabe Black Tested-by: kokoro Reviewed-by: Daniel Carvalho --- M src/cpu/static_inst.hh 1 file changed, 2 insertions(+), 3 deletions(-) Approvals: Daniel Carvalho: Looks good to me, approved Gabe Black: Looks good to me, approved kokoro: Regressions pass diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh index ed45b58..ebc5529 100644 --- a/src/cpu/static_inst.hh +++ b/src/cpu/static_inst.hh @@ -43,19 +43,17 @@ #define __CPU_STATIC_INST_HH__ #include +#include #include #include -#include "arch/registers.hh" #include "arch/types.hh" #include "base/logging.hh" #include "base/refcnt.hh" -#include "base/types.hh" #include "config/the_isa.hh" #include "cpu/op_class.hh" #include "cpu/reg_class.hh" #include "cpu/static_inst_fwd.hh" -#include "cpu/thread_context.hh" #include "enums/StaticInstFlags.hh" #include "sim/byteswap.hh" @@ -63,6 +61,7 @@ class Packet; class ExecContext; +class ThreadContext; namespace Loader { 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42965 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: Ie2e1c01b77d1776b366d29982e481d62b6a2b8cf Gerrit-Change-Number: 42965 Gerrit-PatchSet: 3 Gerrit-Owner: Gabe Black Gerrit-Reviewer: Daniel Carvalho Gerrit-Reviewer: Gabe Black 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]: cpu: Fix transitive includes in the O3 rename map and debug faults.
Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/42964 ) Change subject: cpu: Fix transitive includes in the O3 rename map and debug faults. .. cpu: Fix transitive includes in the O3 rename map and debug faults. Change-Id: I22f80c24c3128e91fd039b8e3b689d7065440ad0 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42964 Maintainer: Gabe Black Tested-by: kokoro Reviewed-by: Daniel Carvalho --- M src/arch/generic/debugfaults.hh M src/cpu/o3/rename_map.hh 2 files changed, 2 insertions(+), 0 deletions(-) Approvals: Daniel Carvalho: Looks good to me, approved Gabe Black: Looks good to me, approved kokoro: Regressions pass diff --git a/src/arch/generic/debugfaults.hh b/src/arch/generic/debugfaults.hh index 7d62d3f..5c3982f 100644 --- a/src/arch/generic/debugfaults.hh +++ b/src/arch/generic/debugfaults.hh @@ -41,6 +41,7 @@ #include #include "base/logging.hh" +#include "cpu/thread_context.hh" #include "sim/faults.hh" namespace GenericISA diff --git a/src/cpu/o3/rename_map.hh b/src/cpu/o3/rename_map.hh index 5f779bd..46d5400 100644 --- a/src/cpu/o3/rename_map.hh +++ b/src/cpu/o3/rename_map.hh @@ -46,6 +46,7 @@ #include #include +#include "arch/generic/isa.hh" #include "arch/types.hh" #include "config/the_isa.hh" #include "cpu/o3/free_list.hh" 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42964 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: I22f80c24c3128e91fd039b8e3b689d7065440ad0 Gerrit-Change-Number: 42964 Gerrit-PatchSet: 4 Gerrit-Owner: Gabe Black Gerrit-Reviewer: Daniel Carvalho Gerrit-Reviewer: Gabe Black 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]: cpu: Fix some minor style issues in cpu/static_inst.hh.
Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/42963 ) Change subject: cpu: Fix some minor style issues in cpu/static_inst.hh. .. cpu: Fix some minor style issues in cpu/static_inst.hh. Also use default values for some members rather than setting them in the constructor explicitly. Change-Id: I0f75cca54f952542d1f37576bd752a8d6acb5561 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42963 Reviewed-by: Gabe Black Reviewed-by: Daniel Carvalho Maintainer: Gabe Black Tested-by: kokoro --- M src/cpu/static_inst.hh 1 file changed, 25 insertions(+), 28 deletions(-) Approvals: Daniel Carvalho: Looks good to me, approved Gabe Black: 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 0bd2e50..ed45b58 100644 --- a/src/cpu/static_inst.hh +++ b/src/cpu/static_inst.hh @@ -103,24 +103,24 @@ OpClass _opClass; /// See numSrcRegs(). -int8_t _numSrcRegs; +int8_t _numSrcRegs = 0; /// See numDestRegs(). -int8_t _numDestRegs; +int8_t _numDestRegs = 0; /// The following are used to track physical register usage /// for machines with separate int & FP reg files. //@{ -int8_t _numFPDestRegs; -int8_t _numIntDestRegs; -int8_t _numCCDestRegs; +int8_t _numFPDestRegs = 0; +int8_t _numIntDestRegs = 0; +int8_t _numCCDestRegs = 0; //@} /** To use in architectures with vector register file. */ /** @{ */ -int8_t _numVecDestRegs; -int8_t _numVecElemDestRegs; -int8_t _numVecPredDestRegs; +int8_t _numVecDestRegs = 0; +int8_t _numVecElemDestRegs = 0; +int8_t _numVecPredDestRegs = 0; /** @} */ public: @@ -226,7 +226,7 @@ void setFlag(Flags f) { flags[f] = true; } /// Operation class. Used to select appropriate function unit in issue. -OpClass opClass() const { return _opClass; } +OpClass opClass() const { return _opClass; } /// Return logical index (architectural reg num) of i'th destination reg. @@ -284,50 +284,47 @@ * String representation of disassembly (lazily evaluated via * disassemble()). */ -mutable std::string *cachedDisassembly; +mutable std::string *cachedDisassembly = nullptr; /** * Internal function to generate disassembly string. */ -virtual std::string -generateDisassembly(Addr pc, const Loader::SymbolTable *symtab) const = 0; +virtual std::string generateDisassembly( +Addr pc, const Loader::SymbolTable *symtab) const = 0; /// Constructor. /// It's important to initialize everything here to a sane /// default, since the decoder generally only overrides /// the fields that are meaningful for the particular /// instruction. -StaticInst(const char *_mnemonic, OpClass __opClass) -: _opClass(__opClass), - _numSrcRegs(0), _numDestRegs(0), _numFPDestRegs(0), - _numIntDestRegs(0), _numCCDestRegs(0), _numVecDestRegs(0), - _numVecElemDestRegs(0), _numVecPredDestRegs(0), - mnemonic(_mnemonic), cachedDisassembly(0) -{ } +StaticInst(const char *_mnemonic, OpClass op_class) +: _opClass(op_class), mnemonic(_mnemonic) +{} public: virtual ~StaticInst(); virtual Fault execute(ExecContext *xc, - Trace::InstRecord *traceData) const = 0; +Trace::InstRecord *traceData) const = 0; -virtual Fault initiateAcc(ExecContext *xc, - Trace::InstRecord *traceData) const +virtual Fault +initiateAcc(ExecContext *xc, Trace::InstRecord *traceData) const { panic("initiateAcc not defined!"); } -virtual Fault completeAcc(Packet *pkt, ExecContext *xc, - Trace::InstRecord *traceData) const +virtual Fault +completeAcc(Packet *pkt, ExecContext *xc, +Trace::InstRecord *trace_data) const { panic("completeAcc not defined!"); } -virtual void advancePC(TheISA::PCState &pcState) const = 0; +virtual void advancePC(TheISA::PCState &pc_state) const = 0; virtual TheISA::PCState -buildRetPC(const TheISA::PCState &curPC, -const TheISA::PCState &callPC) const +buildRetPC(const TheISA::PCState &cur_pc, +const TheISA::PCState &call_pc) const { panic("buildRetPC not defined!"); } @@ -359,7 +356,7 @@ * return the target address as well. */ bool hasBranchTarget(const TheISA::PCState &pc, ThreadContext *tc, - TheISA::PCState &tgt) const; +TheISA::PCState &tgt) const; /** * Return string representation of disassembled instruction. -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42963 To unsubscribe
[gem5-dev] Change in gem5/gem5[develop]: base: Move Named class to its own file
Daniel Carvalho has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/38743 ) Change subject: base: Move Named class to its own file .. base: Move Named class to its own file Named can be useful in instances where trace is not required. Because of that it has been moved to its own file. Named is intended for public inheritance; thus it should have a virtual destructor. Added a unit test for the Named class. Change-Id: I314e850b4fafd7804d919fd3fe6dec44822e1f48 Signed-off-by: Daniel R. Carvalho Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38743 Reviewed-by: Gabe Black Maintainer: Bobby R. Bruce Tested-by: kokoro --- A src/base/named.hh A src/base/named.test.cc M src/base/trace.hh M src/cpu/minor/buffers.hh M src/cpu/minor/decode.hh M src/cpu/minor/dyn_inst.hh M src/cpu/minor/execute.hh M src/cpu/minor/fetch1.hh M src/cpu/minor/fetch2.hh M src/cpu/minor/func_unit.cc M src/cpu/minor/lsq.hh M src/cpu/minor/scoreboard.hh M src/mem/mem_checker.hh 13 files changed, 95 insertions(+), 14 deletions(-) Approvals: Gabe Black: Looks good to me, approved Bobby R. Bruce: Looks good to me, approved kokoro: Regressions pass diff --git a/src/base/named.hh b/src/base/named.hh new file mode 100644 index 000..7e73310 --- /dev/null +++ b/src/base/named.hh @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2021 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. + */ + +#ifndef __BASE_NAMED_HH__ +#define __BASE_NAMED_HH__ + +#include + +/** Interface for things with names. This is useful when using DPRINTF. */ +class Named +{ + private: +const std::string _name; + + public: +Named(const std::string &name_) : _name(name_) { } +virtual ~Named() = default; + +const std::string &name() const { return _name; } +}; + +#endif // __BASE_NAMED_HH__ diff --git a/src/base/named.test.cc b/src/base/named.test.cc new file mode 100644 index 000..3cc6951 --- /dev/null +++ b/src/base/named.test.cc @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2021 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 + * (INCL
[gem5-dev] Change in gem5/gem5[develop]: misc: Add a copied symbol table for the kernel on physical address
Yu-hsin Wang has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43104 ) Change subject: misc: Add a copied symbol table for the kernel on physical address .. misc: Add a copied symbol table for the kernel on physical address In fs simulation, the kernel is loaded to physical address first and then it would relocate itself to virtual address. The address which using by kernel symbol table is virtual address. To debug the process before kernel relocated to virutal memory, we need another copy of symbol table for physical address. Change-Id: I38107ff94b301df1a5170dd98774df88cfb02298 --- M src/base/loader/symtab.hh M src/sim/kernel_workload.cc 2 files changed, 18 insertions(+), 0 deletions(-) diff --git a/src/base/loader/symtab.hh b/src/base/loader/symtab.hh index 5610544..a74bb76 100644 --- a/src/base/loader/symtab.hh +++ b/src/base/loader/symtab.hh @@ -154,6 +154,17 @@ } SymbolTablePtr +rename(std::function func) const +{ +SymTabOp op = [func](SymbolTable &symtab, const Symbol &symbol) { +Symbol sym = symbol; +func(sym.name); +symtab.insert(sym); +}; +return operate(op); +} + +SymbolTablePtr globals() const { return filterByBinding(Symbol::Binding::Global); diff --git a/src/sim/kernel_workload.cc b/src/sim/kernel_workload.cc index 434e317..1b9bbfa 100644 --- a/src/sim/kernel_workload.cc +++ b/src/sim/kernel_workload.cc @@ -61,6 +61,13 @@ }); kernelSymtab = kernelObj->symtab(); +auto relocateKernelSymtab = kernelSymtab.mask(_loadAddrMask) +->offset(_loadAddrOffset) +->rename([](std::string &name) { + name = "__gem5_relocate__" + name; +}); + +Loader::debugSymbolTable.insert(*relocateKernelSymtab); Loader::debugSymbolTable.insert(kernelSymtab); } -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43104 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: I38107ff94b301df1a5170dd98774df88cfb02298 Gerrit-Change-Number: 43104 Gerrit-PatchSet: 1 Gerrit-Owner: Yu-hsin Wang 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]: base: Add fatal tests to sat_counter
Daniel Carvalho has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/42763 ) Change subject: base: Add fatal tests to sat_counter .. base: Add fatal tests to sat_counter Add tests to make sure that the fatal conditions are triggered when met. This commit also moves shift-related death tests to their own DeathTests. Change-Id: Ia610636fe8cf636401e2b2ed623bf20b41147ea4 Signed-off-by: Daniel R. Carvalho Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42763 Reviewed-by: Gabe Black Maintainer: Bobby R. Bruce Tested-by: kokoro --- M src/base/sat_counter.hh M src/base/sat_counter.test.cc 2 files changed, 63 insertions(+), 7 deletions(-) Approvals: Gabe Black: Looks good to me, approved Bobby R. Bruce: Looks good to me, approved kokoro: Regressions pass diff --git a/src/base/sat_counter.hh b/src/base/sat_counter.hh index 4849c2a..cf60fdc 100644 --- a/src/base/sat_counter.hh +++ b/src/base/sat_counter.hh @@ -78,7 +78,7 @@ fatal_if(bits > 8*sizeof(T), "Number of bits exceeds counter size"); fatal_if(initial_val > maxVal, - "Saturating counter's Initial value exceeds max value."); + "Saturating counter's initial value exceeds max value."); } /** diff --git a/src/base/sat_counter.test.cc b/src/base/sat_counter.test.cc index 19f792e..02b6a87 100644 --- a/src/base/sat_counter.test.cc +++ b/src/base/sat_counter.test.cc @@ -31,9 +31,44 @@ #include +#include "base/gtest/logging.hh" #include "base/sat_counter.hh" /** + * Test that an error is triggered when the number of bits exceeds the + * counter's capacity. + */ +TEST(SatCounterDeathTest, BitCountExceeds) +{ +#ifdef NDEBUG +GTEST_SKIP() << "Skipping as assertions are " +"stripped out of fast builds"; +#endif + +gtestLogOutput.str(""); +EXPECT_ANY_THROW(SatCounter8 counter(9)); +ASSERT_NE(gtestLogOutput.str().find("Number of bits exceeds counter size"), +std::string::npos); +} + +/** + * Test that an error is triggered when the initial value is higher than the + * maximum possible value. + */ +TEST(SatCounterDeathTest, InitialValueExceeds) +{ +#ifdef NDEBUG +GTEST_SKIP() << "Skipping as assertions are " +"stripped out of fast builds"; +#endif + +gtestLogOutput.str(""); +EXPECT_ANY_THROW(SatCounter8 counter(7, 128)); +ASSERT_NE(gtestLogOutput.str().find("initial value exceeds max value"), +std::string::npos); +} + +/** * Test if the maximum value is indeed the maximum value reachable. */ TEST(SatCounterTest, MaximumValue) @@ -183,15 +218,36 @@ ASSERT_EQ(counter, value); counter >>= saturated_counter; ASSERT_EQ(counter, 0); +} -// Make sure the counters cannot be shifted by negative numbers, since -// that is undefined behaviour. As these tests depend on asserts failing, -// these tests are only functional if `TRACING_ON == 1`, when gem5 is -// compiled as `debug` or `opt`. -#if TRACING_ON +/** + * Make sure the counters cannot be right-shifted by negative numbers, since + * that is undefined behaviour + */ +TEST(SatCounterDeathTest, RightShiftNegative) +{ +#ifdef NDEBUG +GTEST_SKIP() << "Skipping as assertions are " +"stripped out of fast builds"; +#endif + +SatCounter8 counter(8); ASSERT_DEATH(counter >>= -1, ""); +} + +/** + * Make sure the counters cannot be left-shifted by negative numbers, since + * that is undefined behaviour + */ +TEST(SatCounterDeathTest, LeftShiftNegative) +{ +#ifdef NDEBUG +GTEST_SKIP() << "Skipping as assertions are " +"stripped out of fast builds"; +#endif + +SatCounter8 counter(8); ASSERT_DEATH(counter <<= -1, ""); -#endif } /** -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42763 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: Ia610636fe8cf636401e2b2ed623bf20b41147ea4 Gerrit-Change-Number: 42763 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Carvalho Gerrit-Reviewer: Bobby R. Bruce Gerrit-Reviewer: Daniel Carvalho Gerrit-Reviewer: Gabe Black 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]: dev: Clean up the VirtIO9PDiod::startDiod method.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43083 ) Change subject: dev: Clean up the VirtIO9PDiod::startDiod method. .. dev: Clean up the VirtIO9PDiod::startDiod method. Rather than print the errno, print the result of strerror so the user doesn't have to look up what each error code does. Don't duplicate descriptors needlessly which have been returned by pipe(). Use panic_if instead of if () panic. Check each call to a standard library function instead of calling multiple and then only knowing that one of them failed (but not which one). Close the far side of our pipes for both the gem5 process, and the process that will become diod. Remove some stray #undef-s which undefine macros that were never defined. Don't try to force the descriptors going to diod to be particular numbers. Slightly reduce nesting in the if which checks the results of fork. Drop unnecessary \n-s in calls to panic, inform, etc. Minor spacing related style fixes. Use nullptr instead of NULL. Change-Id: I48d93778a1e139ef624876a0b316486aac774d7f --- M src/dev/virtio/fs9p.cc 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/src/dev/virtio/fs9p.cc b/src/dev/virtio/fs9p.cc index 5263eed..705d961 100644 --- a/src/dev/virtio/fs9p.cc +++ b/src/dev/virtio/fs9p.cc @@ -47,6 +47,7 @@ #include #include +#include #include #include "base/callback.hh" @@ -336,32 +337,29 @@ const Params &p = dynamic_cast(params()); int pipe_rfd[2]; int pipe_wfd[2]; -const int DIOD_RFD = 3; -const int DIOD_WFD = 4; -const char *diod(p.diod.c_str()); +DPRINTF(VIO9P, "Using diod at %s.\n", p.diod); -DPRINTF(VIO9P, "Using diod at %s \n", p.diod.c_str()); - -if (pipe(pipe_rfd) == -1 || pipe(pipe_wfd) == -1) -panic("Failed to create DIOD pipes: %i\n", errno); +panic_if(pipe(pipe_rfd) == -1, "Failed to create DIOD read pipe: %s", +strerror(errno)); +panic_if(pipe(pipe_wfd) == -1, "Failed to create DIOD write pipe: %s", +strerror(errno)); fd_to_diod = pipe_rfd[1]; fd_from_diod = pipe_wfd[0]; // Create Unix domain socket int socket_id = socket(AF_UNIX, SOCK_STREAM, 0); -if (socket_id == -1) { -panic("Socket creation failed %i \n", errno); -} +panic_if(socket_id == -1, "Socket creation failed %i", errno); + // Bind the socket to a path which will not be read struct sockaddr_un socket_address; -memset(&socket_address, 0, sizeof(struct sockaddr_un)); +memset(&socket_address, 0, sizeof(socket_address)); socket_address.sun_family = AF_UNIX; const std::string socket_path = simout.resolve(p.socketPath); -fatal_if(!OutputDirectory::isAbsolute(socket_path), "Please make the" \ - " output directory an absolute path, else diod will fail!\n"); +fatal_if(!OutputDirectory::isAbsolute(socket_path), "Please make the" + " output directory an absolute path, else diod will fail!"); // Prevent overflow in strcpy fatal_if(sizeof(socket_address.sun_path) <= socket_path.length(), @@ -369,47 +367,44 @@ strncpy(socket_address.sun_path, socket_path.c_str(), sizeof(socket_address.sun_path) - 1); if (bind(socket_id, (struct sockaddr*) &socket_address, - sizeof(struct sockaddr_un)) == -1){ + sizeof(struct sockaddr_un)) == -1) { perror("Socket binding"); panic("Socket binding to %i failed - most likely the output dir" \ - " and hence unused socket already exists \n", socket_id); + " and hence unused socket already exists.", socket_id); } diod_pid = fork(); -if (diod_pid == -1) { -panic("Fork failed: %i\n", errno); -} else if (diod_pid == 0) { +panic_if(diod_pid == -1, "Fork failed: %s", strerror(errno)); + +if (diod_pid == 0) { // Create the socket which will later by used by the diod process close(STDIN_FILENO); -if (dup2(pipe_rfd[0], DIOD_RFD) == -1 || -dup2(pipe_wfd[1], DIOD_WFD) == -1) { +close(pipe_rfd[1]); +close(pipe_wfd[0]); -panic("Failed to setup read/write pipes: %i\n", - errno); -} +auto diod_rfd_s = csprintf("%d", pipe_rfd[0]); +auto diod_wfd_s = csprintf("%d", pipe_wfd[1]); // Start diod -execlp(diod, diod, +execlp(p.diod.c_str(), p.diod.c_str(), "-d", DTRACE(VIO9P) ? "1" : "0", // show debug output "-f", // start in foreground - "-r", "3", // setup read FD - "-w", "4", // setup write FD + "-r", diod_rfd_s.c_str(), // setup read FD + "-w", diod_wfd_s.c_str(), // setup write FD "-e", p.root.c_str(), // path to export "-n", // disable security "-S", // squ
[gem5-dev] Change in gem5/gem5[release-staging-v21-0]: misc: Add util-gem5art maintainer tag
Jason Lowe-Power has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43044 ) Change subject: misc: Add util-gem5art maintainer tag .. misc: Add util-gem5art maintainer tag Change-Id: I73bbb8cc511ec3008135d7fa0ac2fa1585b3b5ec Signed-off-by: Jason Lowe-Power --- M MAINTAINERS.yaml 1 file changed, 6 insertions(+), 0 deletions(-) diff --git a/MAINTAINERS.yaml b/MAINTAINERS.yaml index 068da7f..02084a9 100644 --- a/MAINTAINERS.yaml +++ b/MAINTAINERS.yaml @@ -303,6 +303,12 @@ maintainers: - Gabe Black +util-gem5art: + status: maintained + maintainers: +- Bobby Bruce +- Jason Lowe-Power + website: desc: >- The gem5-website repo which contains the gem5.org site -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43044 To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem5 Gerrit-Branch: release-staging-v21-0 Gerrit-Change-Id: I73bbb8cc511ec3008135d7fa0ac2fa1585b3b5ec Gerrit-Change-Number: 43044 Gerrit-PatchSet: 1 Gerrit-Owner: Jason Lowe-Power 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[release-staging-v21-0]: scons,mem-ruby: export need_partial_func_reads in SConstruct
Tiago Mück has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43043 ) Change subject: scons,mem-ruby: export need_partial_func_reads in SConstruct .. scons,mem-ruby: export need_partial_func_reads in SConstruct need_partial_func_reads should now be modified from protocol specific files (e.g. src/learning_gem5/part3/SConsopts) Change-Id: I38039aab6178a019d063d6124200050f2ed7b446 --- M SConstruct M src/mem/ruby/system/SConscript 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/SConstruct b/SConstruct index beaf9ae..fb3421c 100755 --- a/SConstruct +++ b/SConstruct @@ -764,6 +764,9 @@ Export('protocol_dirs') slicc_includes = [] Export('slicc_includes') +# list of protocols that require the partial functional read interface +need_partial_func_reads = [] +Export('need_partial_func_reads') # Walk the tree and execute all SConsopts scripts that wil add to the # above variables diff --git a/src/mem/ruby/system/SConscript b/src/mem/ruby/system/SConscript index a2708ae..e87cd24 100644 --- a/src/mem/ruby/system/SConscript +++ b/src/mem/ruby/system/SConscript @@ -45,9 +45,6 @@ env.Append(CPPDEFINES=['PROTOCOL_' + env['PROTOCOL']]) -# list of protocols that require the partial functional read interface -need_partial_func_reads = [] - if env['PROTOCOL'] in need_partial_func_reads: env.Append(CPPDEFINES=['PARTIAL_FUNC_READS']) -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43043 To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem5 Gerrit-Branch: release-staging-v21-0 Gerrit-Change-Id: I38039aab6178a019d063d6124200050f2ed7b446 Gerrit-Change-Number: 43043 Gerrit-PatchSet: 1 Gerrit-Owner: Tiago Mück 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]: python: debug, fix Mapping import
Adrian Herrera has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/43023 ) Change subject: python: debug, fix Mapping import .. python: debug, fix Mapping import Change "collections.Mapping" to "collections.abc.Mapping". "collections.Mapping" was an alias, it is deprecated starting from Python 3.3, and it will be removed in Python 3.10. Change-Id: Ic257e3c5206eb3d48d4eed85a93fac48bd3b8dc4 Signed-off-by: Adrian Herrera Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/43023 Reviewed-by: Jason Lowe-Power Maintainer: Jason Lowe-Power Tested-by: kokoro --- M src/python/m5/debug.py 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/python/m5/debug.py b/src/python/m5/debug.py index d808850..787a39e 100644 --- a/src/python/m5/debug.py +++ b/src/python/m5/debug.py @@ -24,7 +24,7 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -from collections import Mapping +from collections.abc import Mapping import _m5.debug from _m5.debug import SimpleFlag, CompoundFlag -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43023 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: Ic257e3c5206eb3d48d4eed85a93fac48bd3b8dc4 Gerrit-Change-Number: 43023 Gerrit-PatchSet: 3 Gerrit-Owner: Adrian Herrera Gerrit-Reviewer: Adrian Herrera Gerrit-Reviewer: Andreas Sandberg Gerrit-Reviewer: Jason Lowe-Power 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]: python: debug, fix Mapping import
Adrian Herrera has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43023 ) Change subject: python: debug, fix Mapping import .. python: debug, fix Mapping import Change "collections.Mapping" to "collections.abc.Mapping". "collections.Mapping" was an alias, it is deprecated starting from Python 3.3, and it will be removed in Python 3.10. Change-Id: Ic257e3c5206eb3d48d4eed85a93fac48bd3b8dc4 Signed-off-by: Adrian Herrera --- M src/python/m5/debug.py 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/m5/debug.py b/src/python/m5/debug.py index d808850..787a39e 100644 --- a/src/python/m5/debug.py +++ b/src/python/m5/debug.py @@ -24,7 +24,7 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -from collections import Mapping +from collections.abc import Mapping import _m5.debug from _m5.debug import SimpleFlag, CompoundFlag -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43023 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: Ic257e3c5206eb3d48d4eed85a93fac48bd3b8dc4 Gerrit-Change-Number: 43023 Gerrit-PatchSet: 1 Gerrit-Owner: Adrian Herrera 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]: base-stats: Fix null addStatGroup
Daniel Carvalho has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43012 ) Change subject: base-stats: Fix null addStatGroup .. base-stats: Fix null addStatGroup A group must be provided to be added to a stat group. Change-Id: I9da42fb12c2a8b258f9f45922a6fb6b7fd41a698 Signed-off-by: Daniel R. Carvalho --- M src/base/stats/group.cc M src/base/stats/group.test.cc 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/base/stats/group.cc b/src/base/stats/group.cc index 9e50c45..8f1f08f 100644 --- a/src/base/stats/group.cc +++ b/src/base/stats/group.cc @@ -110,6 +110,7 @@ void Group::addStatGroup(const char *name, Group *block) { +panic_if(!block, "Can't add null stat group %s", name); panic_if(block == this, "Stat group can't be added to itself"); panic_if(statGroups.find(name) != statGroups.end(), "Stats of the same group share the same name `%s`.\n", name); diff --git a/src/base/stats/group.test.cc b/src/base/stats/group.test.cc index 23e9a3b..40aa66c 100644 --- a/src/base/stats/group.test.cc +++ b/src/base/stats/group.test.cc @@ -77,7 +77,7 @@ } /** Test that a group cannot add a non-existent group. */ -TEST(StatsGroupDeathTest, DISABLED_AddNull) +TEST(StatsGroupDeathTest, AddNull) { Stats::Group root(nullptr); ASSERT_ANY_THROW(root.addStatGroup("Node1", nullptr)); -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43012 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: I9da42fb12c2a8b258f9f45922a6fb6b7fd41a698 Gerrit-Change-Number: 43012 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Carvalho 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]: WIP,base-stats: Add unit test for Stats::Info
Daniel Carvalho has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43009 ) Change subject: WIP,base-stats: Add unit test for Stats::Info .. WIP,base-stats: Add unit test for Stats::Info Add a unit test for stats/info. Change-Id: I8b169d34c1309b37ba79fa9cf6895547b7e97fc0 Signed-off-by: Daniel R. Carvalho --- M src/base/stats/SConscript M src/base/stats/info.cc M src/base/stats/info.hh A src/base/stats/info.test.cc 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/base/stats/SConscript b/src/base/stats/SConscript index 6981418..702b18b 100644 --- a/src/base/stats/SConscript +++ b/src/base/stats/SConscript @@ -40,6 +40,7 @@ else: Source('hdf5.cc') +GTest('info.test', 'info.test.cc', 'info.cc', '../debug.cc', '../str.cc') GTest('storage.test', 'storage.test.cc', '../debug.cc', '../str.cc', 'info.cc', 'storage.cc', '../../sim/cur_tick.cc') GTest('units.test', 'units.test.cc') diff --git a/src/base/stats/info.cc b/src/base/stats/info.cc index b2ad86b..b2b9b6a 100644 --- a/src/base/stats/info.cc +++ b/src/base/stats/info.cc @@ -42,8 +42,6 @@ #include "base/stats/info.hh" #include -#include -#include #include "base/cprintf.hh" #include "base/debug.hh" diff --git a/src/base/stats/info.hh b/src/base/stats/info.hh index 0568aa4..971a798 100644 --- a/src/base/stats/info.hh +++ b/src/base/stats/info.hh @@ -29,6 +29,11 @@ #ifndef __BASE_STATS_INFO_HH__ #define __BASE_STATS_INFO_HH__ +#include +#include +#include +#include + #include "base/flags.hh" #include "base/stats/types.hh" #include "base/stats/units.hh" diff --git a/src/base/stats/info.test.cc b/src/base/stats/info.test.cc new file mode 100644 index 000..d76a33c --- /dev/null +++ b/src/base/stats/info.test.cc @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2021 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 "base/stats/info.hh" + +TEST(StatsInfoTest, Construct) +{ +} -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43009 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: I8b169d34c1309b37ba79fa9cf6895547b7e97fc0 Gerrit-Change-Number: 43009 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Carvalho 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]: base-stats: Add unit test for Stats::Group
Daniel Carvalho has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43010 ) Change subject: base-stats: Add unit test for Stats::Group .. base-stats: Add unit test for Stats::Group Add a unit test for Stats::Group. Three bugs were found: groups are able to add themselves/null groups as their sub-groups, and one can create a cyclic dependency of sub-groups. The ADD_STAT macro is not being tested. Change-Id: I52326994b3f75e313024f872d214e8c45943f44d Signed-off-by: Daniel R. Carvalho --- M src/base/stats/SConscript A src/base/stats/group.test.cc 2 files changed, 659 insertions(+), 0 deletions(-) diff --git a/src/base/stats/SConscript b/src/base/stats/SConscript index 702b18b..d39543b 100644 --- a/src/base/stats/SConscript +++ b/src/base/stats/SConscript @@ -40,6 +40,8 @@ else: Source('hdf5.cc') +GTest('group.test', 'group.test.cc', 'group.cc', 'info.cc', +with_tag('gem5 trace')) GTest('info.test', 'info.test.cc', 'info.cc', '../debug.cc', '../str.cc') GTest('storage.test', 'storage.test.cc', '../debug.cc', '../str.cc', 'info.cc', 'storage.cc', '../../sim/cur_tick.cc') diff --git a/src/base/stats/group.test.cc b/src/base/stats/group.test.cc new file mode 100644 index 000..f563dba --- /dev/null +++ b/src/base/stats/group.test.cc @@ -0,0 +1,657 @@ +/* + * Copyright (c) 2021 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 "base/stats/group.hh" +#include "base/stats/info.hh" +#include "base/stats/output.hh" + +/** Test that the constructor without a parent doesn't do anything. */ +TEST(StatsGroupTest, ConstructNoParent) +{ +Stats::Group root(nullptr); +ASSERT_EQ(root.getStatGroups().size(), 0); +} + +/** Test adding a single stat group to a root node. */ +TEST(StatsGroupTest, AddGetSingleStatGroup) +{ +Stats::Group root(nullptr); +Stats::Group node1(nullptr); +root.addStatGroup("Node1", &node1); + +const auto root_map = root.getStatGroups(); +ASSERT_EQ(root_map.size(), 1); +ASSERT_NE(root_map.find("Node1"), root_map.end()); + +ASSERT_EQ(node1.getStatGroups().size(), 0); +} + +/** Test that group names are unique within a node's stat group. */ +TEST(StatsGroupDeathTest, AddUniqueNameStatGroup) +{ +Stats::Group root(nullptr); +Stats::Group node1(nullptr); +Stats::Group node2(nullptr); +root.addStatGroup("Node1", &node1); +ASSERT_ANY_THROW(root.addStatGroup("Node1", &node2)); +} + +/** Test that group names are not unique among two nodes' stat groups. */ +TEST(StatsGroupTest, AddNotUniqueNameAmongGroups) +{ +Stats::Group root(nullptr); +Stats::Group node1(nullptr); +Stats::Group node2(nullptr); +Stats::Group node1_1(nullptr); +root.addStatGroup("Node1", &node1); +node1.addStatGroup("Node1_1", &node1_1); +ASSERT_NO_THROW(node1.addStatGroup("Node1", &node2)); +} + +/** Test that a group cannot add a non-existent group. */ +TEST(StatsGroupDeathTest, DISABLED_AddNull) +{ +Stats::Group root(nullptr); +ASSERT_ANY_THROW(root.addStatGroup("Node1", nullptr)); +} + +/** Test that a group cannot add itself. */ +TEST(StatsGroupDeathTest, DISABLED_AddItself) +{ +Stats::Group root(nullptr); +ASSERT_ANY_THROW(root.addStatGroup("Node1", &root)); +} + +/** Test that a group cannot be added in a cycle. */ +TEST(StatsGroupDeathTest, DISABLED_AddCycle) +{ +Stats::Group
[gem5-dev] Change in gem5/gem5[develop]: base-stats: Add unit test for Stats::Units
Daniel Carvalho has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43007 ) Change subject: base-stats: Add unit test for Stats::Units .. base-stats: Add unit test for Stats::Units Add a unit test for base/stats/unit.hh. Change-Id: I87f729928bb99c7f4d657bca0307955be933d3ed Signed-off-by: Daniel R. Carvalho --- M src/base/stats/SConscript A src/base/stats/units.test.cc 2 files changed, 136 insertions(+), 0 deletions(-) diff --git a/src/base/stats/SConscript b/src/base/stats/SConscript index 8f037ba..6981418 100644 --- a/src/base/stats/SConscript +++ b/src/base/stats/SConscript @@ -42,3 +42,4 @@ GTest('storage.test', 'storage.test.cc', '../debug.cc', '../str.cc', 'info.cc', 'storage.cc', '../../sim/cur_tick.cc') +GTest('units.test', 'units.test.cc') diff --git a/src/base/stats/units.test.cc b/src/base/stats/units.test.cc new file mode 100644 index 000..e95177d --- /dev/null +++ b/src/base/stats/units.test.cc @@ -0,0 +1,135 @@ +/* + * Copyright (c) 2021 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 "base/stats/units.hh" + +TEST(StatsUnitsTest, Cycle) +{ +Stats::Units::Cycle *unit = UNIT_CYCLE; +ASSERT_EQ(unit->getUnitString(), Stats::Units::Cycle::toString()); +} + +TEST(StatsUnitsTest, Tick) +{ +Stats::Units::Tick *unit = UNIT_TICK; +ASSERT_EQ(unit->getUnitString(), Stats::Units::Tick::toString()); +} + +TEST(StatsUnitsTest, Second) +{ +Stats::Units::Second *unit = UNIT_SECOND; +ASSERT_EQ(unit->getUnitString(), Stats::Units::Second::toString()); +} + +TEST(StatsUnitsTest, Bit) +{ +Stats::Units::Bit *unit = UNIT_BIT; +ASSERT_EQ(unit->getUnitString(), Stats::Units::Bit::toString()); +} + +TEST(StatsUnitsTest, Byte) +{ +Stats::Units::Byte *unit = UNIT_BYTE; +ASSERT_EQ(unit->getUnitString(), Stats::Units::Byte::toString()); +} + +TEST(StatsUnitsTest, Watt) +{ +Stats::Units::Watt *unit = UNIT_WATT; +ASSERT_EQ(unit->getUnitString(), Stats::Units::Watt::toString()); +} + +TEST(StatsUnitsTest, Joule) +{ +Stats::Units::Joule *unit = UNIT_JOULE; +ASSERT_EQ(unit->getUnitString(), Stats::Units::Joule::toString()); +} + +TEST(StatsUnitsTest, Volt) +{ +Stats::Units::Volt *unit = UNIT_VOLT; +ASSERT_EQ(unit->getUnitString(), Stats::Units::Volt::toString()); +} + +TEST(StatsUnitsTest, DegreeCelsius) +{ +Stats::Units::DegreeCelsius *unit = UNIT_CELSIUS; +ASSERT_EQ(unit->getUnitString(), Stats::Units::DegreeCelsius::toString()); +} + +TEST(StatsUnitsTest, Count) +{ +Stats::Units::Count *unit = UNIT_COUNT; +ASSERT_EQ(unit->getUnitString(), Stats::Units::Count::toString()); +} + +TEST(StatsUnitsTest, Rate1) +{ +Stats::Units::Rate *unit = +UNIT_RATE(Stats::Units::Count, Stats::Units::Count); +ASSERT_EQ(unit->getUnitString(), "(Count/Count)"); +ASSERT_EQ(unit->getUnitString(), (Stats::Units::Rate +Stats::Units::Count>::toString())); +} + +TEST(StatsUnitsTest, Rate2) +{ +Stats::Units::Rate *unit = +UNIT_RATE(Stats::Units::Tick, Stats::Units::Second); +ASSERT_EQ(unit->getUnitString(), "(Tick/Second)"); +ASSERT_EQ(unit->getUnitString(), (Stats::Units::Rate +Stats::Units::Second>::toString())); +} + +TEST(StatsUnitsTest, RateOfRates) +{ +typedef Stats::Units::Rate +BitPerSecond; +typedef Stats::Units::Rate +CountPerCycle; +
[gem5-dev] Change in gem5/gem5[develop]: base-stats: Fix Watt Unit
Daniel Carvalho has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43006 ) Change subject: base-stats: Fix Watt Unit .. base-stats: Fix Watt Unit Watt had two implementations. Since having the unit printed as Watt is more relevant than as Joule/Second, keep the class. Change-Id: Ic9ae755115e2eca94492f3d5b11245db9fe42bb6 Signed-off-by: Daniel R. Carvalho --- M src/base/stats/units.hh 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/base/stats/units.hh b/src/base/stats/units.hh index 5b785b4..b2dfefa 100644 --- a/src/base/stats/units.hh +++ b/src/base/stats/units.hh @@ -47,10 +47,9 @@ #define UNIT_RATE(T1, T2) Stats::Units::Rate::get() #define UNIT_RATIO Stats::Units::Ratio::get() #define UNIT_COUNT Stats::Units::Count::get() +#define UNIT_WATT Stats::Units::Watt::get() #define UNIT_UNSPECIFIED Stats::Units::Unspecified::get() -#define UNIT_WATT UNIT_RATE(Stats::Units::Joule, Stats::Units::Second) - namespace Stats { /** -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43006 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: Ic9ae755115e2eca94492f3d5b11245db9fe42bb6 Gerrit-Change-Number: 43006 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Carvalho 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]: base-stats: Remove SimObject dependency from stats group
Daniel Carvalho has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43005 ) Change subject: base-stats: Remove SimObject dependency from stats group .. base-stats: Remove SimObject dependency from stats group Remove this circular dependency by casting to Named instead of SimObject. Change-Id: I1054562750ba30d3ad4f9e2fbe238fd6fe2d4cb0 Signed-off-by: Daniel R. Carvalho --- M src/base/stats/group.cc 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/base/stats/group.cc b/src/base/stats/group.cc index de546cd..1852c0c 100644 --- a/src/base/stats/group.cc +++ b/src/base/stats/group.cc @@ -38,10 +38,10 @@ #include "base/stats/group.hh" #include "base/logging.hh" +#include "base/named.hh" #include "base/stats/info.hh" #include "base/trace.hh" #include "debug/Stats.hh" -#include "sim/sim_object.hh" namespace Stats { @@ -67,10 +67,9 @@ for (auto &g : statGroups) { if (DTRACE(Stats)) { -M5_VAR_USED const SimObject *so = -dynamic_cast(this); +M5_VAR_USED const Named *named = dynamic_cast*>(this); DPRINTF(Stats, "%s: regStats in group %s\n", -so ? so->name() : "?", +named ? named->name() : "?", g.first); } g.second->regStats(); -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43005 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: I1054562750ba30d3ad4f9e2fbe238fd6fe2d4cb0 Gerrit-Change-Number: 43005 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Carvalho 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: Remove SimObject dependency from Drainable
Daniel Carvalho has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43004 ) Change subject: sim: Remove SimObject dependency from Drainable .. sim: Remove SimObject dependency from Drainable Remove this circular dependency by casting to Named instead of SimObject. Change-Id: Ia064f7ab1a693586b6bd0045f431512ca3c78801 Signed-off-by: Daniel R. Carvalho --- M src/sim/drain.cc 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sim/drain.cc b/src/sim/drain.cc index 3006768..f883c94 100644 --- a/src/sim/drain.cc +++ b/src/sim/drain.cc @@ -40,10 +40,10 @@ #include #include "base/logging.hh" +#include "base/named.hh" #include "base/trace.hh" #include "debug/Drain.hh" #include "sim/sim_exit.hh" -#include "sim/sim_object.hh" DrainManager DrainManager::_instance; @@ -71,7 +71,7 @@ for (auto *obj : _allDrainable) { DrainState status = obj->dmDrain(); if (DTRACE(Drain) && status != DrainState::Drained) { -SimObject *temp = dynamic_cast(obj); +Named *temp = dynamic_cast(obj); if (temp) DPRINTF(Drain, "Failed to drain %s\n", temp->name()); } -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43004 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: Ia064f7ab1a693586b6bd0045f431512ca3c78801 Gerrit-Change-Number: 43004 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Carvalho 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]: base-stats: Make Rate's compilation smarter
Daniel Carvalho has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43008 ) Change subject: base-stats: Make Rate's compilation smarter .. base-stats: Make Rate's compilation smarter A Rate should be supplied units of different types. Rates between units of the same type are Ratios, and should be declared as such. An exception is applied to Count and Unspecified, since those units represent unknown underlying units. Change-Id: I36ab7c73b239ccc86d866c5b38e14fd765bbbd0f Signed-off-by: Daniel R. Carvalho --- M src/base/stats/units.hh 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/src/base/stats/units.hh b/src/base/stats/units.hh index b2dfefa..87e6025 100644 --- a/src/base/stats/units.hh +++ b/src/base/stats/units.hh @@ -272,38 +272,6 @@ std::string getUnitString() const override { return Count::toString(); } }; -template -class Rate : public Base -{ - static_assert(std::is_base_of::value, -"Rate(T1,T2) must have T1 and T2 derived from" -"Stats::Units::Base"); - static_assert(std::is_base_of::value, -"Rate(T1,T2) must have T1 and T2 derived from" -"Stats::Units::Base"); - private: -Rate() {} - public: -Rate(Rate const&) = delete; -void operator=(Rate const&) = delete; -static Rate* -get() -{ -static Rate instance; -return &instance; -} -static std::string -toString() -{ -return csprintf("(%s/%s)", T1::toString(), T2::toString()); -} -std::string -getUnitString() const override -{ -return Rate::toString(); -} -}; - class Ratio : public Base { private: @@ -342,6 +310,41 @@ } }; +template +class Rate : public Base +{ +static_assert(std::is_base_of::value, +"Rate(T1,T2) must have T1 and T2 derived from Stats::Units::Base"); +static_assert(std::is_base_of::value, +"Rate(T1,T2) must have T1 and T2 derived from Stats::Units::Base"); +static_assert(!std::is_same::value || +std::is_same::value || std::is_sameUnspecified>::value, +"Rate(T1,T2) must have T1 and T2 of different types; " +"otherwise, it would be a Ratio"); + + private: +Rate() {} + public: +Rate(Rate const&) = delete; +void operator=(Rate const&) = delete; +static Rate* +get() +{ +static Rate instance; +return &instance; +} +static std::string +toString() +{ +return csprintf("(%s/%s)", T1::toString(), T2::toString()); +} +std::string +getUnitString() const override +{ +return Rate::toString(); +} +}; + } // namespace Units } // namespace Stats -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43008 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: I36ab7c73b239ccc86d866c5b38e14fd765bbbd0f Gerrit-Change-Number: 43008 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Carvalho 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]: base-stats: Fix self addition bug in addStatGroup
Daniel Carvalho has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43011 ) Change subject: base-stats: Fix self addition bug in addStatGroup .. base-stats: Fix self addition bug in addStatGroup A group cannot be added to itself; otherwise, it would create a cyclic dependency. Change-Id: I2e42f84814c675e8d5318cfda2d99b1b394758aa Signed-off-by: Daniel R. Carvalho --- M src/base/stats/group.cc M src/base/stats/group.test.cc 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/base/stats/group.cc b/src/base/stats/group.cc index 1852c0c..9e50c45 100644 --- a/src/base/stats/group.cc +++ b/src/base/stats/group.cc @@ -110,6 +110,7 @@ void Group::addStatGroup(const char *name, Group *block) { +panic_if(block == this, "Stat group can't be added to itself"); panic_if(statGroups.find(name) != statGroups.end(), "Stats of the same group share the same name `%s`.\n", name); diff --git a/src/base/stats/group.test.cc b/src/base/stats/group.test.cc index f563dba..23e9a3b 100644 --- a/src/base/stats/group.test.cc +++ b/src/base/stats/group.test.cc @@ -84,7 +84,7 @@ } /** Test that a group cannot add itself. */ -TEST(StatsGroupDeathTest, DISABLED_AddItself) +TEST(StatsGroupDeathTest, AddItself) { Stats::Group root(nullptr); ASSERT_ANY_THROW(root.addStatGroup("Node1", &root)); -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/43011 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: I2e42f84814c675e8d5318cfda2d99b1b394758aa Gerrit-Change-Number: 43011 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Carvalho 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 unit test for sim/port
Daniel Carvalho has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/43003 ) Change subject: sim: Add unit test for sim/port .. sim: Add unit test for sim/port Add a unit test for sim/port. The fact that binding a port does not automatically bind its peer is error-prone, and should likely be revisited in the future. Change-Id: Iee91fc9bfa80a527d9a1902529722833b061dec9 Signed-off-by: Daniel R. Carvalho --- M src/sim/SConscript M src/sim/port.hh A src/sim/port.test.cc 3 files changed, 242 insertions(+), 0 deletions(-) diff --git a/src/sim/SConscript b/src/sim/SConscript index 7db0262..0a0e5e4 100644 --- a/src/sim/SConscript +++ b/src/sim/SConscript @@ -87,6 +87,7 @@ GTest('byteswap.test', 'byteswap.test.cc', '../base/types.cc') GTest('guest_abi.test', 'guest_abi.test.cc') +GTest('port.test', 'port.test.cc', 'port.cc') GTest('proxy_ptr.test', 'proxy_ptr.test.cc') GTest('serialize.test', 'serialize.test.cc', 'serialize.cc', '../base/inifile.cc', with_tag('gem5 trace')) diff --git a/src/sim/port.hh b/src/sim/port.hh index 85472d0..f265666 100644 --- a/src/sim/port.hh +++ b/src/sim/port.hh @@ -46,6 +46,8 @@ #ifndef __SIM_PORT_HH__ #define __SIM_PORT_HH__ +#include +#include #include #include "base/types.hh" diff --git a/src/sim/port.test.cc b/src/sim/port.test.cc new file mode 100644 index 000..2543673 --- /dev/null +++ b/src/sim/port.test.cc @@ -0,0 +1,239 @@ +/* + * Copyright 2021 Daniel R. Carvalho + * + * 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 +#include + +#include "base/gtest/logging.hh" +#include "sim/port.hh" + +class TestPort : public Port +{ + public: +TestPort(PortID _id) : Port("TestPort", _id) {} +}; + +/** Test getting the port id. */ +TEST(PortTest, GetId) +{ +TestPort port(0), port2(2); +ASSERT_EQ(port.getId(), 0); +ASSERT_EQ(port2.getId(), 2); +} + +/** Test connecting one of the ports. */ +TEST(PortTest, OneSidedConnection) +{ +TestPort port(0), port2(2); + +// Both ports start unconnected +ASSERT_FALSE(port.isConnected()); +ASSERT_FALSE(port2.isConnected()); + +// Bind one of the ports to the other +port.bind(port2); + +// Binding a single side does not bind both ports automatically +ASSERT_TRUE(port.isConnected()); +ASSERT_EQ(&port.getPeer(), &port2); +ASSERT_FALSE(port2.isConnected()); + +// Unbind the port +port.unbind(); +ASSERT_FALSE(port.isConnected()); +ASSERT_FALSE(port2.isConnected()); +} + +/** Test connecting both ports. */ +TEST(PortTest, TwoSidedConnection) +{ +TestPort port(0), port2(2); + +// Both ports start unconnected +ASSERT_FALSE(port.isConnected()); +ASSERT_FALSE(port2.isConnected()); + +// Bind the ports +port.bind(port2); +port2.bind(port); +ASSERT_TRUE(port.isConnected()); +ASSERT_EQ(&port.getPeer(), &port2); +ASSERT_TRUE(port2.isConnected()); +ASSERT_EQ(&port2.getPeer(), &port); + +// Unbinding one port does not automatically unbind the other +port.unbind(); +ASSERT_FALSE(port.isConnected()); +ASSERT_TRUE(port2.isConnected()); +ASSERT_EQ(&port2.getPeer(), &port); + +// Finish unbinding +port2.unbind(); +ASSERT_FALSE(port.isConnected()); +ASSERT_FALSE(port2.isConnected()); +} + +/** Test that manually overwriting a bind is possible. */ +TEST(PortTest, OverwriteConnection) +{ +TestPort port(0)
[gem5-dev] Change in gem5/gem5[release-staging-v21-0]: dev-arm: Fix SMMUv3BaseCache Stats
Giacomo Travaglini has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42983 ) Change subject: dev-arm: Fix SMMUv3BaseCache Stats .. dev-arm: Fix SMMUv3BaseCache Stats After [1] the SMMUv3BaseCache stats are undistinguible within each other. With this patch we are adding a string to their constructor so that we can distinguish between an IPA, Config etc cache stat [1]: https://gem5-review.googlesource.com/c/public/gem5/+/36415 Signed-off-by: Giacomo Travaglini Change-Id: Iaa84ed948cf2a4c36ea4fcda589676b9bbeed6fd --- M src/dev/arm/smmu_v3_caches.cc M src/dev/arm/smmu_v3_caches.hh M src/dev/arm/smmu_v3_deviceifc.cc 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/dev/arm/smmu_v3_caches.cc b/src/dev/arm/smmu_v3_caches.cc index 8bea3ea..59f5aca 100644 --- a/src/dev/arm/smmu_v3_caches.cc +++ b/src/dev/arm/smmu_v3_caches.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2018-2019 ARM Limited + * Copyright (c) 2014, 2018-2019, 2021 Arm Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -59,12 +59,12 @@ */ SMMUv3BaseCache::SMMUv3BaseCache(const std::string &policy_name, uint32_t seed, - Stats::Group *parent) : -replacementPolicy(decodePolicyName(policy_name)), + Stats::Group *parent, const std::string &name) + : replacementPolicy(decodePolicyName(policy_name)), nextToReplace(0), random(seed), useStamp(0), -baseCacheStats(parent) +baseCacheStats(parent, name) {} int @@ -82,8 +82,9 @@ } SMMUv3BaseCache:: -SMMUv3BaseCacheStats::SMMUv3BaseCacheStats(Stats::Group *parent) -: Stats::Group(parent), +SMMUv3BaseCacheStats::SMMUv3BaseCacheStats( +Stats::Group *parent, const std::string &name) +: Stats::Group(parent, name.c_str()), ADD_STAT(averageLookups, UNIT_RATE(Stats::Units::Count, Stats::Units::Second), "Average number lookups per second"), @@ -144,9 +145,10 @@ */ SMMUTLB::SMMUTLB(unsigned numEntries, unsigned _associativity, - const std::string &policy, Stats::Group *parent) + const std::string &policy, Stats::Group *parent, + const std::string &name) : -SMMUv3BaseCache(policy, SMMUTLB_SEED, parent), +SMMUv3BaseCache(policy, SMMUTLB_SEED, parent, name), associativity(_associativity) { if (associativity == 0) @@ -426,7 +428,7 @@ ARMArchTLB::ARMArchTLB(unsigned numEntries, unsigned _associativity, const std::string &policy, Stats::Group *parent) : -SMMUv3BaseCache(policy, ARMARCHTLB_SEED, parent), +SMMUv3BaseCache(policy, ARMARCHTLB_SEED, parent, "tlb"), associativity(_associativity) { if (associativity == 0) @@ -625,7 +627,7 @@ IPACache::IPACache(unsigned numEntries, unsigned _associativity, const std::string &policy, Stats::Group *parent) : -SMMUv3BaseCache(policy, IPACACHE_SEED, parent), +SMMUv3BaseCache(policy, IPACACHE_SEED, parent, "ipa"), associativity(_associativity) { if (associativity == 0) @@ -805,7 +807,7 @@ ConfigCache::ConfigCache(unsigned numEntries, unsigned _associativity, const std::string &policy, Stats::Group *parent) : -SMMUv3BaseCache(policy, CONFIGCACHE_SEED, parent), +SMMUv3BaseCache(policy, CONFIGCACHE_SEED, parent, "cfg"), associativity(_associativity) { if (associativity == 0) @@ -969,7 +971,7 @@ WalkCache::WalkCache(const std::array &_sizes, unsigned _associativity, const std::string &policy, Stats::Group *parent) : -SMMUv3BaseCache(policy, WALKCACHE_SEED, parent), +SMMUv3BaseCache(policy, WALKCACHE_SEED, parent, "walk"), walkCacheStats(&(SMMUv3BaseCache::baseCacheStats)), associativity(_associativity), sizes() @@ -1226,7 +1228,7 @@ WalkCache:: WalkCacheStats::WalkCacheStats(Stats::Group *parent) -: Stats::Group(parent, "WalkCache") +: Stats::Group(parent) { using namespace Stats; diff --git a/src/dev/arm/smmu_v3_caches.hh b/src/dev/arm/smmu_v3_caches.hh index 640710f..dee09f2 100644 --- a/src/dev/arm/smmu_v3_caches.hh +++ b/src/dev/arm/smmu_v3_caches.hh @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2018-2019 ARM Limited + * Copyright (c) 2014, 2018-2019, 2021 Arm Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -67,7 +67,7 @@ struct SMMUv3BaseCacheStats : public Stats::Group { -SMMUv3BaseCacheStats(Stats::Group *parent); +SMMUv3BaseCacheStats(Stats::Group *parent, const std::string &name); Stats::Formula averageLookups; Stats::Scalar totalLookups; @@ -87,7 +87,7 @@ public: SMMUv3BaseCache(const std::string &policy_name, uint32_t seed
[gem5-dev] Change in gem5/gem5[release-staging-v21-0]: dev-arm: Remove unused SMMUv3 WalkCache variables
Giacomo Travaglini has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42985 ) Change subject: dev-arm: Remove unused SMMUv3 WalkCache variables .. dev-arm: Remove unused SMMUv3 WalkCache variables Those were grouped within the stats data structures but were not actually stats Signed-off-by: Giacomo Travaglini Change-Id: I01bbbada423825ded04a033c0709108e2980ec70 --- M src/dev/arm/smmu_v3_caches.cc M src/dev/arm/smmu_v3_caches.hh 2 files changed, 0 insertions(+), 6 deletions(-) diff --git a/src/dev/arm/smmu_v3_caches.cc b/src/dev/arm/smmu_v3_caches.cc index b09fb31..069e070 100644 --- a/src/dev/arm/smmu_v3_caches.cc +++ b/src/dev/arm/smmu_v3_caches.cc @@ -1043,10 +1043,8 @@ if (result == NULL) baseCacheStats.totalMisses++; -walkCacheStats.lookupsByStageLevel[stage-1][level]++; walkCacheStats.totalLookupsByStageLevel[stage-1][level]++; if (result == NULL) { -walkCacheStats.missesByStageLevel[stage-1][level]++; walkCacheStats.totalMissesByStageLevel[stage-1][level]++; } } @@ -1079,7 +1077,6 @@ } baseCacheStats.totalUpdates++; -walkCacheStats.updatesByStageLevel[incoming.stage-1][incoming.level]++; walkCacheStats .totalUpdatesByStageLevel[incoming.stage-1][incoming.level]++; } diff --git a/src/dev/arm/smmu_v3_caches.hh b/src/dev/arm/smmu_v3_caches.hh index 07c6242..a6faf6a 100644 --- a/src/dev/arm/smmu_v3_caches.hh +++ b/src/dev/arm/smmu_v3_caches.hh @@ -327,15 +327,12 @@ WalkCacheStats(Stats::Group *parent); ~WalkCacheStats(); -unsigned int lookupsByStageLevel[2][WALK_CACHE_LEVELS]; std::vector averageLookupsByStageLevel; Stats::Vector2d totalLookupsByStageLevel; -unsigned int missesByStageLevel[2][WALK_CACHE_LEVELS]; std::vector averageMissesByStageLevel; Stats::Vector2d totalMissesByStageLevel; -unsigned int updatesByStageLevel[2][WALK_CACHE_LEVELS]; std::vector averageUpdatesByStageLevel; Stats::Vector2d totalUpdatesByStageLevel; -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42985 To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem5 Gerrit-Branch: release-staging-v21-0 Gerrit-Change-Id: I01bbbada423825ded04a033c0709108e2980ec70 Gerrit-Change-Number: 42985 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[release-staging-v21-0]: dev-arm: Fix WalkCache stats
Giacomo Travaglini has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42984 ) Change subject: dev-arm: Fix WalkCache stats .. dev-arm: Fix WalkCache stats The WalkCache stats are wrongly using the legacy framework. With this patch we are registering those to the hierarchical structure. As we need to pass the Stats::Group parent at construction time, we are replacing 2d arrays with Vector2d for count stats and using a flat vector of pointers for the Formula stats Signed-off-by: Giacomo Travaglini Change-Id: I8992bc262a376e4e81a4d608c11dff6902e0a01d --- M src/dev/arm/smmu_v3_caches.cc M src/dev/arm/smmu_v3_caches.hh 2 files changed, 88 insertions(+), 50 deletions(-) diff --git a/src/dev/arm/smmu_v3_caches.cc b/src/dev/arm/smmu_v3_caches.cc index 59f5aca..b09fb31 100644 --- a/src/dev/arm/smmu_v3_caches.cc +++ b/src/dev/arm/smmu_v3_caches.cc @@ -1228,68 +1228,105 @@ WalkCache:: WalkCacheStats::WalkCacheStats(Stats::Group *parent) -: Stats::Group(parent) +: Stats::Group(parent), + ADD_STAT(totalLookupsByStageLevel, UNIT_COUNT, + "Total number of lookups"), + ADD_STAT(totalMissesByStageLevel, UNIT_COUNT, + "Total number of misses"), + ADD_STAT(totalUpdatesByStageLevel, UNIT_COUNT, + "Total number of updates"), + ADD_STAT(insertionsByStageLevel, UNIT_COUNT, + "Number of insertions (not replacements)") { using namespace Stats; +totalLookupsByStageLevel +.init(2, WALK_CACHE_LEVELS) +.flags(pdf); +totalMissesByStageLevel +.init(2, WALK_CACHE_LEVELS) +.flags(pdf); +totalUpdatesByStageLevel +.init(2, WALK_CACHE_LEVELS) +.flags(pdf); +insertionsByStageLevel +.init(2, WALK_CACHE_LEVELS) +.flags(pdf); + for (int s = 0; s < 2; s++) { +totalLookupsByStageLevel.subname(s, csprintf("S%d", s + 1)); +totalMissesByStageLevel.subname(s, csprintf("S%d", s + 1)); +totalUpdatesByStageLevel.subname(s, csprintf("S%d", s + 1)); +insertionsByStageLevel.subname(s, csprintf("S%d", s + 1)); + for (int l = 0; l < WALK_CACHE_LEVELS; l++) { -averageLookupsByStageLevel[s][l] -.name(csprintf("averageLookupsS%dL%d", s+1, l)) -.desc("Average number lookups per second") -.flags(pdf); +totalLookupsByStageLevel.ysubname(l, csprintf("L%d", l)); +totalMissesByStageLevel.ysubname(l, csprintf("L%d", l)); +totalUpdatesByStageLevel.ysubname(l, csprintf("L%d", l)); +insertionsByStageLevel.ysubname(l, csprintf("L%d", l)); -totalLookupsByStageLevel[s][l] -.name(csprintf("totalLookupsS%dL%d", s+1, l)) -.desc("Total number of lookups") -.flags(pdf); +auto avg_lookup = new Stats::Formula( +this, +csprintf("averageLookups_S%dL%d", s+1, l).c_str(), +UNIT_RATE(Stats::Units::Count, Stats::Units::Second), +"Average number lookups per second"); +avg_lookup->flags(pdf); +averageLookupsByStageLevel.push_back(avg_lookup); -averageLookupsByStageLevel[s][l] = +*avg_lookup = totalLookupsByStageLevel[s][l] / simSeconds; +auto avg_misses = new Stats::Formula( +this, +csprintf("averageMisses_S%dL%d", s+1, l).c_str(), +UNIT_RATE(Stats::Units::Count, Stats::Units::Second), +"Average number misses per second"); +avg_misses->flags(pdf); +averageMissesByStageLevel.push_back(avg_misses); -averageMissesByStageLevel[s][l] -.name(csprintf("averageMissesS%dL%d", s+1, l)) -.desc("Average number misses per second") -.flags(pdf); - -totalMissesByStageLevel[s][l] -.name(csprintf("totalMissesS%dL%d", s+1, l)) -.desc("Total number of misses") -.flags(pdf); - -averageMissesByStageLevel[s][l] = +*avg_misses = totalMissesByStageLevel[s][l] / simSeconds; +auto avg_updates = new Stats::Formula( +this, +csprintf("averageUpdates_S%dL%d", s+1, l).c_str(), +UNIT_RATE(Stats::Units::Count, Stats::Units::Second), +"Average number updates per second"); +avg_updates->flags(pdf); +averageUpdatesByStageLevel.push_back(avg_updates); -averageUpdatesByStageLevel[s][l] -.name(csprintf("averageUpdatesS%dL%d", s+1, l)) -.desc("Average number updates per second") -.flags(pdf); - -totalUpdatesByStageLevel[s][l] -.name(csprintf("totalUpdatesS%dL%d",
[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Build source picking into the operands.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42971 ) Change subject: arch-x86: Build source picking into the operands. .. arch-x86: Build source picking into the operands. The parser is able to add extra code to generate the operand value, but we were explicitly adding that code ourselves. That added complexity particularly to the reg microops, but made other types a little more complex as well. Change-Id: Idae36c27662c79302ea9b2831062c8c067ba174c --- M src/arch/x86/isa/microops/ldstop.isa M src/arch/x86/isa/microops/mediaop.isa M src/arch/x86/isa/microops/regop.isa M src/arch/x86/isa/operands.isa 4 files changed, 109 insertions(+), 119 deletions(-) diff --git a/src/arch/x86/isa/microops/ldstop.isa b/src/arch/x86/isa/microops/ldstop.isa index 0bda652..4fb953a 100644 --- a/src/arch/x86/isa/microops/ldstop.isa +++ b/src/arch/x86/isa/microops/ldstop.isa @@ -662,10 +662,10 @@ microopClasses[name] = StoreOp -defineMicroStoreOp('St', 'Mem = pick(Data, data, dataSize);') -defineMicroStoreOp('Stis', 'Mem = pick(Data, data, dataSize);', +defineMicroStoreOp('St', 'Mem = PData;') +defineMicroStoreOp('Stis', 'Mem = PData;', implicitStack=True) -defineMicroStoreOp('Stul', 'Mem = pick(Data, data, dataSize);', +defineMicroStoreOp('Stul', 'Mem = PData;', mem_flags="Request::LOCKED_RMW") defineMicroStoreOp('Stfp', code='Mem = FpData_uqw;', is_float=True) diff --git a/src/arch/x86/isa/microops/mediaop.isa b/src/arch/x86/isa/microops/mediaop.isa index 17c609b..3e8 100644 --- a/src/arch/x86/isa/microops/mediaop.isa +++ b/src/arch/x86/isa/microops/mediaop.isa @@ -253,11 +253,10 @@ if (bits(dest, 0) && (ext & 0x1)) offset -= items; if (offset >= 0 && offset < items) { -uint64_t srcReg1 = pick(SrcReg1, src1, srcSize); FpDestReg_uqw = insertBits(FpDestReg_uqw, (offset + 1) * destSize * 8 - 1, -(offset + 0) * destSize * 8, srcReg1); +(offset + 0) * destSize * 8, PMSrcReg1); } else { FpDestReg_uqw = FpDestReg_uqw; } diff --git a/src/arch/x86/isa/microops/regop.isa b/src/arch/x86/isa/microops/regop.isa index 1861b0c..15d5e60 100644 --- a/src/arch/x86/isa/microops/regop.isa +++ b/src/arch/x86/isa/microops/regop.isa @@ -209,7 +209,8 @@ typeQual = "" if match.group("typeQual"): typeQual = match.group("typeQual") -src2_name = "%spsrc2%s" % (match.group("prefix"), typeQual) +src2_name = "%sPSrcReg2%s" % (match.group("prefix").upper(), +typeQual) self.buildCppClasses(name, Name, suffix, matcher.sub(src2_name, code), matcher.sub(src2_name, big_code), @@ -218,7 +219,7 @@ matcher.sub(src2_name, else_code), matcher.sub(src2_name, cond_control_flag_init), op_class, operand_types) -imm_name = "%simm8" % match.group("prefix") +imm_name = '(int8_t)imm8' if match.group("prefix") else 'imm8' self.buildCppClasses(name + "i", Name, suffix + "Imm", matcher.sub(imm_name, code), matcher.sub(imm_name, big_code), @@ -238,41 +239,13 @@ operand_types) suffix = "Flags" + suffix -# If psrc1 or psrc2 is used, we need to actually insert code to -# compute it. -for (big, all) in ((False, allCode), (True, allBigCode)): -prefix = "" -for (rex, decl) in ( -("(?- "uint64_t psrc1 = pick(SrcReg1, src1, dataSize);"), -("(?- "uint64_t psrc2 = pick(SrcReg2, src2, dataSize);"), -("(?' -# If imm8 shows up in the code, use the immediate templates, if -# not, hopefully the register ones will be correct. -templates = normalTemplates -matcher = re.compile("(?- PredezfBit, ext & ~mask, result, psrc1, op2); +PredezfBit, ext & ~mask, result, PSrcReg1, op2); PredezfBit = newFlags & EZFBit; PreddfBit = newFlags & DFBit; PredccFlagBits = newFlags & ccFlagMask; @@ -443,7 +416,7 @@ flag_code = ''' uint64_t newFlags = genFlags(PredccFlagBits | PredcfofBits | PreddfBit | PredecfBit | PredezfBit, -ext, result, psrc1, op2); +
[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Number operands automatically.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42973 ) Change subject: arch-x86: Number operands automatically. .. arch-x86: Number operands automatically. Keeping the sequence of ids for operands unique and in the right order is done manually now, and is a pain to maintain. This change builds updating that index into the functions which generate most of the entries for the operand table. Change-Id: I1ee05ce2702f2534d509ec2204c40124d7a30559 --- M src/arch/x86/isa/operands.isa 1 file changed, 124 insertions(+), 109 deletions(-) diff --git a/src/arch/x86/isa/operands.isa b/src/arch/x86/isa/operands.isa index 2bf253e..926e417 100644 --- a/src/arch/x86/isa/operands.isa +++ b/src/arch/x86/isa/operands.isa @@ -54,89 +54,106 @@ }}; let {{ -def intReg(idx, id): -return ('IntReg', 'uqw', idx, 'IsInteger', id) -def pickedReg(idx, id, size='dataSize'): -return ('IntReg', 'uqw', idx, 'IsInteger', id, +operandIdx = 0 +def intReg(idx): +global operandIdx +operandIdx += 1 +return ('IntReg', 'uqw', idx, 'IsInteger', operandIdx) +def pickedReg(idx, size='dataSize'): +global operandIdx +operandIdx += 1 +return ('IntReg', 'uqw', idx, 'IsInteger', operandIdx, 'pick(xc->readIntRegOperand(this, %(op_idx)s), ' '%(reg_idx)s, ' + size + ')') -def signedPickedReg(idx, id, size='dataSize'): -return ('IntReg', 'uqw', idx, 'IsInteger', id, +def signedPickedReg(idx, size='dataSize'): +global operandIdx +operandIdx += 1 +return ('IntReg', 'uqw', idx, 'IsInteger', operandIdx, 'signedPick(xc->readIntRegOperand(this, %(op_idx)s), ' '%(reg_idx)s, ' + size + ')') -def floatReg(idx, id): -return ('FloatReg', 'df', idx, 'IsFloating', id) -def ccReg(idx, id): -return ('CCReg', 'uqw', idx, None, id) -def controlReg(idx, id, ctype = 'uqw'): +def floatReg(idx): +global operandIdx +operandIdx += 1 +return ('FloatReg', 'df', idx, 'IsFloating', operandIdx) +def ccReg(idx, predRead=None, predWrite=None): +global operandIdx +operandIdx += 1 +return ('CCReg', 'uqw', idx, None, operandIdx, None, None, +predRead, predWrite) +def controlReg(idx, ctype = 'uqw'): +global operandIdx +operandIdx += 1 return ('ControlReg', ctype, idx, (None, None, ['IsSerializeAfter', 'IsSerializing', 'IsNonSpeculative']), -id) -def squashCheckReg(idx, id, check, ctype = 'uqw'): +operandIdx) +def squashCheckReg(idx, check, ctype='uqw'): +global operandIdx +operandIdx += 1 return ('ControlReg', ctype, idx, (None, None, ['((%s) ? ' % check+ \ 'IsSquashAfter : IsSerializeAfter)', 'IsSerializing', 'IsNonSpeculative']), -id) -def squashCReg(idx, id, ctype = 'uqw'): -return squashCheckReg(idx, id, 'true', ctype) -def squashCSReg(idx, id, ctype = 'uqw'): -return squashCheckReg(idx, id, 'dest == X86ISA::SEGMENT_REG_CS', ctype) -def squashCR0Reg(idx, id, ctype = 'uqw'): -return squashCheckReg(idx, id, 'dest == 0', ctype) +operandIdx) +def squashCReg(idx, ctype='uqw'): +return squashCheckReg(idx, 'true', ctype) +def squashCSReg(idx, ctype='uqw'): +return squashCheckReg(idx, 'dest == X86ISA::SEGMENT_REG_CS', ctype) +def squashCR0Reg(idx, ctype='uqw'): +return squashCheckReg(idx, 'dest == 0', ctype) +def pcState(idx, ctype='uqw'): +global operandIdx +operandIdx += 1 +return ('PCState', ctype, idx, (None, None, 'IsControl'), operandIdx) }}; def operands {{ -'SrcReg1': intReg('src1', 1), -'PSrcReg1': pickedReg('src1', 1), -'PMSrcReg1': pickedReg('src1', 1, 'srcSize'), -'SPSrcReg1': signedPickedReg('src1', 1), -'SrcReg2': intReg('src2', 2), -'PSrcReg2': pickedReg('src2', 2), -'SPSrcReg2': signedPickedReg('src2', 2), -'Index': intReg('index', 3), -'Base': intReg('base', 4), -'DestReg': intReg('dest', 5), -'Data': intReg('data', 6), -'PData': pickedReg('data', 6), -'DataLow': intReg('dataLow', 6), -'DataHi':intReg('dataHi', 6), -'ProdLow': intReg('X86ISA::INTREG_PRODLOW', 7), -'ProdHi':intReg('X86ISA::INTREG_PRODHI', 8), -'Quotient': intReg('X86ISA::INTREG_QUOTIENT', 9), -'Remainder': intReg('X86ISA::INTR
[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Create some infrastructure for x86 microop operands.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42969 ) Change subject: arch-x86: Create some infrastructure for x86 microop operands. .. arch-x86: Create some infrastructure for x86 microop operands. These are currently only used in the reg, float and media ops, but should be expanded to the other types eventually. This is part of an attempt to use common machinery for the microops instead of having each type recreate the mechanisms it needs locally. Change-Id: I67f521b6b161310ba50a30ac4c73305cc75a63d5 --- M src/arch/x86/isa/microops/base.isa M src/arch/x86/isa/microops/fpop.isa M src/arch/x86/isa/microops/mediaop.isa M src/arch/x86/isa/microops/regop.isa 4 files changed, 251 insertions(+), 106 deletions(-) diff --git a/src/arch/x86/isa/microops/base.isa b/src/arch/x86/isa/microops/base.isa index 035ee9d..24c61be 100644 --- a/src/arch/x86/isa/microops/base.isa +++ b/src/arch/x86/isa/microops/base.isa @@ -48,6 +48,156 @@ // let {{ +class Operand(object): +@classmethod +def isDual(cls): +return False + +@classmethod +def cxx_class(cls): +return 'X86ISA::' + cls.flavor + cls.idx_name + 'Op' + +class FlavorlessOperand(Operand): +@classmethod +def cxx_class(cls): +return 'X86ISA::' + cls.op_type + +def __init__(self, it): +self.value = next(it) + +def ctor_args(self): +return str(self.value) + +class UpcOp(FlavorlessOperand): +op_type = 'Upc' +class FaultOp(FlavorlessOperand): +op_type = 'Fault' +class AddrOp(FlavorlessOperand): +op_type = 'Addr' + +def __init__(self, it): +self.segment = next(it) +[self.scale, self.index, self.base] = next(it) +self.disp = next(it) + +def ctor_args(self): +args = [self.scale, self.index, self.base, self.disp, self.segment] +return ', '.join(map(str, args)) + +class ImmOp(object): +idx_name = '' + +def __init__(self, it): +self.value = next(it) +super(ImmOp, self).__init__() + +def ctor_args(self): +return str(self.value) + +class Imm8Op(ImmOp, Operand): +flavor = 'Imm8' +class Imm64Op(ImmOp, Operand): +flavor = 'Imm64' + +class DestOp(object): +idx_name = 'Dest' +class Src1Op(object): +idx_name = 'Src1' +class Src2Op(object): +idx_name = 'Src2' + +class RegisterOp(object): +def __init__(self, it): +self.idx = next(it) +super(RegisterOp, self).__init__() + +def ctor_args(self): +return str(self.idx) + +class FoldedOp(RegisterOp): +flavor = 'Folded' +class DbgOp(RegisterOp): +flavor = 'Dbg' +class CrOp(RegisterOp): +flavor = 'Cr' +class SegOp(RegisterOp): +flavor = 'Seg' +class MiscOp(RegisterOp): +flavor = 'Misc' +class FloatOp(RegisterOp): +flavor = 'Float' +class IntOp(RegisterOp): +flavor = 'Int' +class DataOp(RegisterOp): +idx_name = 'Data' +class DataHiOp(RegisterOp): +idx_name = 'DataHi' +class DataLowOp(RegisterOp): +idx_name = 'DataLow' + +class FoldedDataOp(FoldedOp, DataOp, Operand): +pass +class FloatDataOp(FloatOp, DataOp, Operand): +pass +class FoldedDataHiOp(FoldedOp, DataHiOp, Operand): +pass +class FoldedDataLowOp(FoldedOp, DataLowOp, Operand): +pass + +class FoldedDestOp(FoldedOp, DestOp, Operand): +pass +class DbgDestOp(DbgOp, DestOp, Operand): +pass +class CrDestOp(CrOp, DestOp, Operand): +pass +class SegDestOp(SegOp, DestOp, Operand): +pass +class MiscDestOp(MiscOp, DestOp, Operand): +pass +class FloatDestOp(FloatOp, DestOp, Operand): +pass +class IntDestOp(IntOp, DestOp, Operand): +pass + +class FoldedSrc1Op(FoldedOp, Src1Op, Operand): +pass +class DbgSrc1Op(DbgOp, Src1Op, Operand): +pass +class CrSrc1Op(CrOp, Src1Op, Operand): +pass +class SegSrc1Op(SegOp, Src1Op, Operand): +pass +class MiscSrc1Op(MiscOp, Src1Op, Operand): +pass +class FloatSrc1Op(FloatOp, Src1Op, Operand): +pass +class IntSrc1Op(IntOp, Src1Op, Operand): +pass + +class FoldedSrc2Op(FoldedOp, Src2Op, Operand): +pass +class DbgSrc2Op(DbgOp, Src2Op, Operand): +pass +class CrSrc2Op(CrOp, Src2Op, Operand): +pass +class SegSrc2Op(SegOp, Src2Op, Operand): +pass +class MiscSrc2Op(MiscOp, Src2Op, Operand): +pass +class FloatSrc2Op(FloatOp, Src2Op, Operand): +pass +
[gem5-dev] Change in gem5/gem5[develop]: arch: Don't zero initialize operands in the ISA parser.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42972 ) Change subject: arch: Don't zero initialize operands in the ISA parser. .. arch: Don't zero initialize operands in the ISA parser. If the operand is a source, it will be initialized by generated code. If it's a destination and it's read before it's used or not used at all then that's a bug which needs to be fixed. Change-Id: I53a6f7068613da670b7a665d25ad112cdb51205b --- M src/arch/isa_parser/operand_types.py 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/arch/isa_parser/operand_types.py b/src/arch/isa_parser/operand_types.py index bed58d7..0ea2f2d 100755 --- a/src/arch/isa_parser/operand_types.py +++ b/src/arch/isa_parser/operand_types.py @@ -165,9 +165,7 @@ return my_flags def makeDecl(self): -# Note that initializations in the declarations are solely -# to avoid 'uninitialized variable' errors from the compiler. -return self.ctype + ' ' + self.base_name + ' = 0;\n'; +return self.ctype + ' ' + self.base_name + ';\n'; class IntRegOperand(Operand): -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42972 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: I53a6f7068613da670b7a665d25ad112cdb51205b Gerrit-Change-Number: 42972 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: Set %(op_idx)s properly when predicated operands are present.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42970 ) Change subject: arch: Set %(op_idx)s properly when predicated operands are present. .. arch: Set %(op_idx)s properly when predicated operands are present. This should be either the fixed index if there are no predicated operands (ie operands which may not be used), and an auto incrementing index otherwise. This is still not bulletproof since the auto incrementing index is just code which ++-es the index, and so the index will be different and incremented each time that value is substituted in. Also, the mixture of predicated operands and the vector operands is broken and will not generate compilable code, but I'm not going to try to fix that here. Change-Id: I1ceae519649762e54eaa019610e51bb8c21d28d6 --- M src/arch/isa_parser/operand_types.py 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/arch/isa_parser/operand_types.py b/src/arch/isa_parser/operand_types.py index 10e763b..bed58d7 100755 --- a/src/arch/isa_parser/operand_types.py +++ b/src/arch/isa_parser/operand_types.py @@ -47,24 +47,26 @@ src_reg_constructor = '\n\tsetSrcRegIdx(_numSrcRegs++, RegId(%s, %s));' dst_reg_constructor = '\n\tsetDestRegIdx(_numDestRegs++, RegId(%s, %s));' -def buildReadCode(self, func = None): +def buildReadCode(self, predRead, func=None): subst_dict = {"name": self.base_name, "func": func, "reg_idx": self.reg_spec, "ctype": self.ctype} if hasattr(self, 'src_reg_idx'): -subst_dict['op_idx'] = self.src_reg_idx +subst_dict['op_idx'] = \ +'_sourceIndex++' if predRead else str(self.src_reg_idx) code = self.read_code % subst_dict return '%s = %s;\n' % (self.base_name, code) -def buildWriteCode(self, func = None): +def buildWriteCode(self, predWrite, func=None): subst_dict = {"name": self.base_name, "func": func, "reg_idx": self.reg_spec, "ctype": self.ctype, "final_val": self.base_name} if hasattr(self, 'dest_reg_idx'): -subst_dict['op_idx'] = self.dest_reg_idx +subst_dict['op_idx'] = \ +'_destIndex++' if predWrite else str(self.dest_reg_idx) code = self.write_code % subst_dict return ''' { @@ -200,7 +202,7 @@ if (self.ctype == 'float' or self.ctype == 'double'): error('Attempt to read integer register as FP') if self.read_code != None: -return self.buildReadCode('readIntRegOperand') +return self.buildReadCode(predRead, 'readIntRegOperand') int_reg_val = '' if predRead: @@ -217,7 +219,7 @@ if (self.ctype == 'float' or self.ctype == 'double'): error('Attempt to write integer register as FP') if self.write_code != None: -return self.buildWriteCode('setIntRegOperand') +return self.buildWriteCode(predWrite, 'setIntRegOperand') if predWrite: wp = 'true' @@ -264,7 +266,7 @@ def makeRead(self, predRead): if self.read_code != None: -return self.buildReadCode('readFloatRegOperandBits') +return self.buildReadCode(predRead, 'readFloatRegOperandBits') if predRead: rindex = '_sourceIndex++' @@ -280,7 +282,7 @@ def makeWrite(self, predWrite): if self.write_code != None: -return self.buildWriteCode('setFloatRegOperandBits') +return self.buildWriteCode(predWrite, 'setFloatRegOperandBits') if predWrite: wp = '_destIndex++' @@ -369,7 +371,7 @@ def makeReadW(self, predWrite): func = 'getWritableVecRegOperand' if self.read_code != None: -return self.buildReadCode(func) +return self.buildReadCode(predWrite, func) if predWrite: rindex = '_destIndex++' @@ -407,7 +409,7 @@ def makeRead(self, predRead): func = 'readVecRegOperand' if self.read_code != None: -return self.buildReadCode(func) +return self.buildReadCode(predRead, func) if predRead: rindex = '_sourceIndex++' @@ -437,7 +439,7 @@ def makeWrite(self, predWrite): func = 'setVecRegOperand' if self.write_code != None: -return self.buildWriteCode(func) +return self.buildWriteCode(predWrite, func) wb = ''' if (traceData) { @@ -537,7 +539,7 @@ def makeRead(self, predRead): func = 'readVecPredRegOperand' if self.read_code != None: -return self.buildReadCode(func) +return self.buildReadCode(predRead, func) if p
[gem5-dev] Change in gem5/gem5[develop]: arch: Make it possible for force operands to be sources or dests.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42974 ) Change subject: arch: Make it possible for force operands to be sources or dests. .. arch: Make it possible for force operands to be sources or dests. Add either Src or Dest as a suffix to force an operand to be purely a source or destination. This helps avoid situations where a calculated value in a destination operand is used to find additional values. The parser would assume the destination is a source, even when it's not. Also, in the future, this will make it possible to keep track of all inputs and outputs from instructions automatically for tracing purposes. Change-Id: Ife805e8bc312f1dd0e4aebccb37a6165389d2edc --- M src/arch/isa_parser/operand_types.py 1 file changed, 71 insertions(+), 0 deletions(-) diff --git a/src/arch/isa_parser/operand_types.py b/src/arch/isa_parser/operand_types.py index 0ea2f2d..2d9889f 100755 --- a/src/arch/isa_parser/operand_types.py +++ b/src/arch/isa_parser/operand_types.py @@ -44,6 +44,9 @@ derived classes encapsulates the traits of a particular operand type (e.g., "32-bit integer register").''' +IsForcedSource = False +IsForcedDest = False + src_reg_constructor = '\n\tsetSrcRegIdx(_numSrcRegs++, RegId(%s, %s));' dst_reg_constructor = '\n\tsetDestRegIdx(_numDestRegs++, RegId(%s, %s));' @@ -97,6 +100,15 @@ # by predicated register reads/writes. Hence, we forward the flags # that indicate whether or not predication is in use. def finalize(self, predRead, predWrite): +if self.IsForcedSource: +assert not self.IsForcedDest +self.is_src = True +self.is_dest = False +elif self.IsForcedDest: +assert not self.IsForcedSource +self.is_src = False +self.is_dest = True + self.flags = self.getFlags() self.constructor = self.makeConstructor(predRead, predWrite) self.op_decl = self.makeDecl() @@ -167,6 +179,11 @@ def makeDecl(self): return self.ctype + ' ' + self.base_name + ';\n'; +class SrcOperand(object): +IsForcedSource = True + +class DestOperand(object): +IsForcedDest = True class IntRegOperand(Operand): reg_class = 'IntRegClass' @@ -240,6 +257,12 @@ return wb +class IntRegSrcOperand(SrcOperand, IntRegOperand): +pass + +class IntRegDestOperand(DestOperand, IntRegOperand): +pass + class FloatRegOperand(Operand): reg_class = 'FloatRegClass' @@ -303,6 +326,12 @@ }''' % (self.ctype, self.base_name, wp) return wb +class FloatRegSrcOperand(SrcOperand, FloatRegOperand): +pass + +class FloatRegDestOperand(DestOperand, FloatRegOperand): +pass + class VecRegOperand(Operand): reg_class = 'VecRegClass' @@ -451,6 +480,12 @@ if self.is_dest: self.op_rd = self.makeReadW(predWrite) + self.op_rd +class VecRegSrcOperand(SrcOperand, VecRegOperand): +pass + +class VecRegDestOperand(DestOperand, VecRegOperand): +pass + class VecElemOperand(Operand): reg_class = 'VecElemClass' @@ -505,6 +540,12 @@ return c_write +class VecElemSrcOperand(SrcOperand, VecElemOperand): +pass + +class VecElemDestOperand(DestOperand, VecElemOperand): +pass + class VecPredRegOperand(Operand): reg_class = 'VecPredRegClass' @@ -587,6 +628,12 @@ if self.is_dest: self.op_rd = self.makeReadW(predWrite) + self.op_rd +class VecPredSrcOperand(SrcOperand, VecPredRegOperand): +pass + +class VecPredDestOperand(DestOperand, VecPredRegOperand): +pass + class CCRegOperand(Operand): reg_class = 'CCRegClass' @@ -659,6 +706,12 @@ return wb +class CCRegSrcOperand(SrcOperand, CCRegOperand): +pass + +class CCRegDestOperand(DestOperand, CCRegOperand): +pass + class ControlRegOperand(Operand): reg_class = 'MiscRegClass' @@ -713,6 +766,12 @@ return wb +class ControlRegSrcOperand(SrcOperand, ControlRegOperand): +pass + +class ControlRegDestOperand(DestOperand, ControlRegOperand): +pass + class MemOperand(Operand): def isMem(self): return 1 @@ -734,6 +793,12 @@ return self.buildWriteCode(predWrite) return '' +class MemSrcOperand(SrcOperand, MemOperand): +pass + +class MemDestOperand(DestOperand, MemOperand): +pass + class PCStateOperand(Operand): def makeConstructor(self, predRead, predWrite): return '' @@ -766,3 +831,9 @@ def isPCState(self): return 1 + +class PCStateSrcOperand(SrcOperand, PCStateOperand): +pass + +class PCStateDestOperand(DestOperand, PCStateOperand): +pass -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42974 To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem
[gem5-dev] Change in gem5/gem5[develop]: arch,cpu,sim: Move the null and nop StaticInstPtrs to their own files.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42968 ) Change subject: arch,cpu,sim: Move the null and nop StaticInstPtrs to their own files. .. arch,cpu,sim: Move the null and nop StaticInstPtrs to their own files. The nullStaticInstPtr was low overhead, but the nopStaticInstPtr needed an actual StaticInst implementation it could point to, and that brought with it some (minor) additional dependencies. Specifically, the implementation of advancePC needs the definition of TheISA::PCState, while all other signatures/impementations in StaticInst are already passing around that type by reference or could be made to, reducing dependencies further. Change-Id: I9ac6a6e5a3106858ea1fc727648f61dc39738a59 --- M src/arch/arm/faults.hh M src/arch/generic/debugfaults.hh M src/arch/mips/faults.hh M src/arch/power/insts/branch.cc M src/arch/riscv/faults.hh M src/arch/sparc/faults.hh M src/arch/x86/faults.hh M src/cpu/SConscript M src/cpu/checker/cpu_impl.hh M src/cpu/minor/dyn_inst.cc M src/cpu/minor/fetch2.cc A src/cpu/nop_static_inst.cc A src/cpu/nop_static_inst.hh A src/cpu/null_static_inst.cc A src/cpu/null_static_inst.hh M src/cpu/o3/commit_impl.hh M src/cpu/o3/fetch_impl.hh M src/cpu/pred/tage_base.hh M src/cpu/simple/base.cc M src/cpu/static_inst.cc M src/cpu/static_inst.hh M src/sim/faults.hh 22 files changed, 249 insertions(+), 107 deletions(-) diff --git a/src/arch/arm/faults.hh b/src/arch/arm/faults.hh index b911136..66eb745 100644 --- a/src/arch/arm/faults.hh +++ b/src/arch/arm/faults.hh @@ -46,6 +46,7 @@ #include "arch/arm/pagetable.hh" #include "arch/arm/types.hh" #include "base/logging.hh" +#include "cpu/null_static_inst.hh" #include "sim/faults.hh" #include "sim/full_system.hh" @@ -221,9 +222,9 @@ MiscRegIndex getFaultAddrReg64() const; void invoke(ThreadContext *tc, const StaticInstPtr &inst = -StaticInst::nullStaticInstPtr) override; +nullStaticInstPtr) override; void invoke64(ThreadContext *tc, const StaticInstPtr &inst = - StaticInst::nullStaticInstPtr); + nullStaticInstPtr); void update(ThreadContext *tc); bool isResetSPSR(){ return bStep; } @@ -295,7 +296,7 @@ public: void invoke(ThreadContext *tc, const StaticInstPtr &inst = -StaticInst::nullStaticInstPtr) override; +nullStaticInstPtr) override; }; class UndefinedInstruction : public ArmFaultVals @@ -323,7 +324,7 @@ {} void invoke(ThreadContext *tc, const StaticInstPtr &inst = -StaticInst::nullStaticInstPtr) override; +nullStaticInstPtr) override; bool routeToHyp(ThreadContext *tc) const override; ExceptionClass ec(ThreadContext *tc) const override; uint32_t iss() const override; @@ -344,7 +345,7 @@ } void invoke(ThreadContext *tc, const StaticInstPtr &inst = -StaticInst::nullStaticInstPtr) override; +nullStaticInstPtr) override; bool routeToHyp(ThreadContext *tc) const override; ExceptionClass ec(ThreadContext *tc) const override; uint32_t iss() const override; @@ -361,7 +362,7 @@ } void invoke(ThreadContext *tc, const StaticInstPtr &inst = -StaticInst::nullStaticInstPtr) override; +nullStaticInstPtr) override; ExceptionClass ec(ThreadContext *tc) const override; uint32_t iss() const override; uint32_t vectorCatchFlag() const override { return 0x0400; } @@ -467,7 +468,7 @@ bool getFaultVAddr(Addr &va) const override; void invoke(ThreadContext *tc, const StaticInstPtr &inst = -StaticInst::nullStaticInstPtr) override; +nullStaticInstPtr) override; FSR getFsr(ThreadContext *tc) const override; uint8_t getFaultStatusCode(ThreadContext *tc) const; @@ -590,7 +591,7 @@ PCAlignmentFault(Addr _faultPC) : faultPC(_faultPC) {} void invoke(ThreadContext *tc, const StaticInstPtr &inst = -StaticInst::nullStaticInstPtr) override; +nullStaticInstPtr) override; bool routeToHyp(ThreadContext *tc) const override; }; @@ -608,7 +609,7 @@ public: SystemError(); void invoke(ThreadContext *tc, const StaticInstPtr &inst = -StaticInst::nullStaticInstPtr) override; +nullStaticInstPtr) override; bool routeToMonitor(ThreadContext *tc) const override; bool routeToHyp(ThreadContext *tc) const override; }; @@ -629,7 +630,7 @@ Addr vAddr; public: void invoke(ThreadContext *tc, const StaticInstPtr &inst = -StaticInst::nullStaticInstPtr) override; +nullStaticInstPtr) override; HardwareBreakpoint(Addr _vaddr, uint32_t _iss); bool routeToHyp(ThreadContext *tc) const override; ExceptionClass ec(ThreadContext *tc) co
[gem5-dev] Change in gem5/gem5[develop]: cpu: Fix some minor style issues in cpu/static_inst.hh.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42963 ) Change subject: cpu: Fix some minor style issues in cpu/static_inst.hh. .. cpu: Fix some minor style issues in cpu/static_inst.hh. Also use default values for some members rather than setting them in the constructor explicitly. Change-Id: I0f75cca54f952542d1f37576bd752a8d6acb5561 --- M src/cpu/static_inst.hh 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh index 0bd2e50..41c3dcc 100644 --- a/src/cpu/static_inst.hh +++ b/src/cpu/static_inst.hh @@ -103,24 +103,24 @@ OpClass _opClass; /// See numSrcRegs(). -int8_t _numSrcRegs; +int8_t _numSrcRegs = 0; /// See numDestRegs(). -int8_t _numDestRegs; +int8_t _numDestRegs = 0; /// The following are used to track physical register usage /// for machines with separate int & FP reg files. //@{ -int8_t _numFPDestRegs; -int8_t _numIntDestRegs; -int8_t _numCCDestRegs; +int8_t _numFPDestRegs = 0; +int8_t _numIntDestRegs = 0; +int8_t _numCCDestRegs = 0; //@} /** To use in architectures with vector register file. */ /** @{ */ -int8_t _numVecDestRegs; -int8_t _numVecElemDestRegs; -int8_t _numVecPredDestRegs; +int8_t _numVecDestRegs = 0; +int8_t _numVecElemDestRegs = 0; +int8_t _numVecPredDestRegs = 0; /** @} */ public: @@ -226,7 +226,7 @@ void setFlag(Flags f) { flags[f] = true; } /// Operation class. Used to select appropriate function unit in issue. -OpClass opClass() const { return _opClass; } +OpClass opClass() const { return _opClass; } /// Return logical index (architectural reg num) of i'th destination reg. @@ -284,50 +284,47 @@ * String representation of disassembly (lazily evaluated via * disassemble()). */ -mutable std::string *cachedDisassembly; +mutable std::string *cachedDisassembly = nullptr; /** * Internal function to generate disassembly string. */ -virtual std::string -generateDisassembly(Addr pc, const Loader::SymbolTable *symtab) const = 0; +virtual std::string generateDisassembly( +Addr pc, const Loader::SymbolTable *symtab) const = 0; /// Constructor. /// It's important to initialize everything here to a sane /// default, since the decoder generally only overrides /// the fields that are meaningful for the particular /// instruction. -StaticInst(const char *_mnemonic, OpClass __opClass) -: _opClass(__opClass), - _numSrcRegs(0), _numDestRegs(0), _numFPDestRegs(0), - _numIntDestRegs(0), _numCCDestRegs(0), _numVecDestRegs(0), - _numVecElemDestRegs(0), _numVecPredDestRegs(0), - mnemonic(_mnemonic), cachedDisassembly(0) -{ } +StaticInst(const char *_mnemonic, OpClass op_class) +: _opClass(op_class), mnemonic(_mnemonic) +{} public: virtual ~StaticInst(); virtual Fault execute(ExecContext *xc, - Trace::InstRecord *traceData) const = 0; +Trace::InstRecord *traceData) const = 0; -virtual Fault initiateAcc(ExecContext *xc, - Trace::InstRecord *traceData) const +virtual Fault +initiateAcc(ExecContext *xc, Trace::InstRecord *traceData) const { panic("initiateAcc not defined!"); } -virtual Fault completeAcc(Packet *pkt, ExecContext *xc, - Trace::InstRecord *traceData) const +virtual Fault +completeAcc(Packet *pkt, ExecContext *xc, +Trace::InstRecord *trace_data) const { panic("completeAcc not defined!"); } -virtual void advancePC(TheISA::PCState &pcState) const = 0; +virtual void advancePC(TheISA::PCState &pc_state) const = 0; virtual TheISA::PCState -buildRetPC(const TheISA::PCState &curPC, -const TheISA::PCState &callPC) const +buildRetPC(const TheISA::PCState &cur_pc, +const TheISA::PCState &call_pc) const { panic("buildRetPC not defined!"); } @@ -359,7 +356,7 @@ * return the target address as well. */ bool hasBranchTarget(const TheISA::PCState &pc, ThreadContext *tc, - TheISA::PCState &tgt) const; +TheISA::PCState &tgt) const; /** * Return string representation of disassembled instruction. @@ -369,7 +366,7 @@ * should not be cached, this function should be overridden directly. */ virtual const std::string &disassemble(Addr pc, -const Loader::SymbolTable *symtab=nullptr) const; +const Loader::SymbolTable *symtab=nullptr) const; /** * Print a separator separated list of this instruction's set flag -- To view, visit https://gem5-review.googl
[gem5-dev] Change in gem5/gem5[develop]: cpu: Use a unique_ptr to manage cached disassembly.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42967 ) Change subject: cpu: Use a unique_ptr to manage cached disassembly. .. cpu: Use a unique_ptr to manage cached disassembly. This is a fairly minor improvement, but avoids having to manually keep track of that string pointer. Change-Id: Ic3d4ddd9445920a110b36ab0cd64ff2289cf0139 --- M src/arch/mips/isa/formats/branch.isa M src/cpu/static_inst.cc M src/cpu/static_inst.hh 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/arch/mips/isa/formats/branch.isa b/src/arch/mips/isa/formats/branch.isa index 09bab47..7ab2e0b 100644 --- a/src/arch/mips/isa/formats/branch.isa +++ b/src/arch/mips/isa/formats/branch.isa @@ -153,14 +153,11 @@ PCDependentDisassembly::disassemble( Addr pc, const Loader::SymbolTable *symtab) const { -if (!cachedDisassembly || -pc != cachedPC || symtab != cachedSymtab) -{ -if (cachedDisassembly) -delete cachedDisassembly; +if (!cachedDisassembly || pc != cachedPC || symtab != cachedSymtab) { +if (!cachedDisassembly) +cachedDisassembly = std::make_unique(); -cachedDisassembly = -new std::string(generateDisassembly(pc, symtab)); +*cachedDisassembly = generateDisassembly(pc, symtab); cachedPC = pc; cachedSymtab = symtab; } diff --git a/src/cpu/static_inst.cc b/src/cpu/static_inst.cc index 05cd360..c42b1d2 100644 --- a/src/cpu/static_inst.cc +++ b/src/cpu/static_inst.cc @@ -64,12 +64,6 @@ StaticInstPtr StaticInst::nullStaticInstPtr; StaticInstPtr StaticInst::nopStaticInstPtr = new NopStaticInst; -StaticInst::~StaticInst() -{ -if (cachedDisassembly) -delete cachedDisassembly; -} - bool StaticInst::hasBranchTarget(const TheISA::PCState &pc, ThreadContext *tc, TheISA::PCState &tgt) const @@ -111,8 +105,10 @@ const std::string & StaticInst::disassemble(Addr pc, const Loader::SymbolTable *symtab) const { -if (!cachedDisassembly) -cachedDisassembly = new std::string(generateDisassembly(pc, symtab)); +if (!cachedDisassembly) { +cachedDisassembly = +std::make_unique(generateDisassembly(pc, symtab)); +} return *cachedDisassembly; } diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh index b6bd17b..5fbf5d7 100644 --- a/src/cpu/static_inst.hh +++ b/src/cpu/static_inst.hh @@ -283,7 +283,7 @@ * String representation of disassembly (lazily evaluated via * disassemble()). */ -mutable std::string *cachedDisassembly = nullptr; +mutable std::unique_ptr cachedDisassembly; /** * Internal function to generate disassembly string. @@ -301,7 +301,7 @@ {} public: -virtual ~StaticInst(); +virtual ~StaticInst() {}; virtual Fault execute(ExecContext *xc, Trace::InstRecord *traceData) const = 0; -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42967 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: Ic3d4ddd9445920a110b36ab0cd64ff2289cf0139 Gerrit-Change-Number: 42967 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]: cpu: Narrow the dependencies of the StaticInst class.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42965 ) Change subject: cpu: Narrow the dependencies of the StaticInst class. .. cpu: Narrow the dependencies of the StaticInst class. Include only the minimal number of headers to make StaticInst subclasses easier to test with unit tests. Change-Id: Ie2e1c01b77d1776b366d29982e481d62b6a2b8cf --- M src/cpu/static_inst.hh 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh index 41c3dcc..b6bd17b 100644 --- a/src/cpu/static_inst.hh +++ b/src/cpu/static_inst.hh @@ -43,19 +43,17 @@ #define __CPU_STATIC_INST_HH__ #include +#include #include #include -#include "arch/registers.hh" #include "arch/types.hh" #include "base/logging.hh" #include "base/refcnt.hh" -#include "base/types.hh" #include "config/the_isa.hh" #include "cpu/op_class.hh" #include "cpu/reg_class.hh" #include "cpu/static_inst_fwd.hh" -#include "cpu/thread_context.hh" #include "enums/StaticInstFlags.hh" #include "sim/byteswap.hh" @@ -63,6 +61,7 @@ class Packet; class ExecContext; +class ThreadContext; namespace Loader { -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42965 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: Ie2e1c01b77d1776b366d29982e481d62b6a2b8cf Gerrit-Change-Number: 42965 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]: cpu: Fix transitive includes in the O3 rename map and debug faults.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42964 ) Change subject: cpu: Fix transitive includes in the O3 rename map and debug faults. .. cpu: Fix transitive includes in the O3 rename map and debug faults. Change-Id: I22f80c24c3128e91fd039b8e3b689d7065440ad0 --- M src/arch/generic/debugfaults.hh M src/cpu/o3/rename_map.hh 2 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/arch/generic/debugfaults.hh b/src/arch/generic/debugfaults.hh index 7d62d3f..5c3982f 100644 --- a/src/arch/generic/debugfaults.hh +++ b/src/arch/generic/debugfaults.hh @@ -41,6 +41,7 @@ #include #include "base/logging.hh" +#include "cpu/thread_context.hh" #include "sim/faults.hh" namespace GenericISA diff --git a/src/cpu/o3/rename_map.hh b/src/cpu/o3/rename_map.hh index 5f779bd..46d5400 100644 --- a/src/cpu/o3/rename_map.hh +++ b/src/cpu/o3/rename_map.hh @@ -46,6 +46,7 @@ #include #include +#include "arch/generic/isa.hh" #include "arch/types.hh" #include "config/the_isa.hh" #include "cpu/o3/free_list.hh" -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42964 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: I22f80c24c3128e91fd039b8e3b689d7065440ad0 Gerrit-Change-Number: 42964 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,base,cpu: Move some type aliases into base/types.hh.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/42966 ) Change subject: arch,base,cpu: Move some type aliases into base/types.hh. .. arch,base,cpu: Move some type aliases into base/types.hh. The arch/generic/types.hh header includes some more complicated types which in turn bring in more dependencies, adding baggage when other code only needs the simple RegIndex or ElemIndex types. Also the RegVal type alias is already in base/types.hh. It doesn't really make sense to have RegVal in one header and RegIndex in another. Change-Id: I1360652598b5fa59e0632b1ee0e0535ace2ba563 --- M src/arch/generic/types.hh M src/base/types.hh M src/cpu/reg_class.hh 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/arch/generic/types.hh b/src/arch/generic/types.hh index 76df835..0f1e837 100644 --- a/src/arch/generic/types.hh +++ b/src/arch/generic/types.hh @@ -42,21 +42,11 @@ #define __ARCH_GENERIC_TYPES_HH__ #include -#include #include "base/trace.hh" #include "base/types.hh" #include "sim/serialize.hh" -// Logical register index type. -typedef uint16_t RegIndex; - -/** Logical vector register elem index type. */ -using ElemIndex = uint16_t; - -/** ElemIndex value that indicates that the register is not a vector. */ -#define ILLEGAL_ELEM_INDEX std::numeric_limits::max() - namespace GenericISA { diff --git a/src/base/types.hh b/src/base/types.hh index 5e9a207..5f4c741 100644 --- a/src/base/types.hh +++ b/src/base/types.hh @@ -38,6 +38,7 @@ #include #include +#include #include #include #include @@ -166,7 +167,17 @@ const Addr MaxAddr = (Addr)-1; -typedef uint64_t RegVal; +using RegVal = uint64_t; + +// Logical register index type. +using RegIndex = uint16_t; + +/** Logical vector register elem index type. */ +using ElemIndex = uint16_t; + +/** ElemIndex value that indicates that the register is not a vector. */ +static const ElemIndex IllegalElemIndex = +std::numeric_limits::max(); static inline uint32_t floatToBits32(float val) diff --git a/src/cpu/reg_class.hh b/src/cpu/reg_class.hh index 3496874..b537352 100644 --- a/src/cpu/reg_class.hh +++ b/src/cpu/reg_class.hh @@ -44,8 +44,8 @@ #include #include -#include "arch/generic/types.hh" #include "arch/registers.hh" +#include "base/types.hh" #include "config/the_isa.hh" /** Enumerate the classes of registers. */ @@ -99,13 +99,13 @@ RegId() : RegId(IntRegClass, 0) {} RegId(RegClass reg_class, RegIndex reg_idx) -: RegId(reg_class, reg_idx, ILLEGAL_ELEM_INDEX) {} +: RegId(reg_class, reg_idx, IllegalElemIndex) {} explicit RegId(RegClass reg_class, RegIndex reg_idx, ElemIndex elem_idx) : regClass(reg_class), regIdx(reg_idx), elemIdx(elem_idx), numPinnedWrites(0) { -if (elemIdx == ILLEGAL_ELEM_INDEX) { +if (elemIdx == IllegalElemIndex) { panic_if(regClass == VecElemClass, "Creating vector physical index w/o element index"); } else { -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42966 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: I1360652598b5fa59e0632b1ee0e0535ace2ba563 Gerrit-Change-Number: 42966 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