[gem5-dev] Change in gem5/gem5[develop]: scons: Fix checking the --install-hooks option in the "git" tool.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/50628 ) Change subject: scons: Fix checking the --install-hooks option in the "git" tool. .. scons: Fix checking the --install-hooks option in the "git" tool. The option should be checked with 'install_hooks' and not 'install-hooks', even though the option is '--install-hooks' to the user. This bug was due to an incorrect review suggestion from me on this original change: https://gem5-review.googlesource.com/c/public/gem5/+/50410 JIRA: https://gem5.atlassian.net/browse/GEM5-1091 Change-Id: Ifb4526307bea015de1fe73d3d685477be711b7cb --- M site_scons/site_tools/git.py 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site_scons/site_tools/git.py b/site_scons/site_tools/git.py index e95c62f..fe083b7 100644 --- a/site_scons/site_tools/git.py +++ b/site_scons/site_tools/git.py @@ -100,7 +100,7 @@ return print(git_style_message, end=' ') -if SCons.Script.GetOption('install-hooks'): +if SCons.Script.GetOption('install_hooks'): print("Installing revision control hooks automatically.") else: try: -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50628 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: Ifb4526307bea015de1fe73d3d685477be711b7cb Gerrit-Change-Number: 50628 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]: scons: Fix gem5 build fails when install-hooks is not set
Hoa Nguyen has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/50627 ) Change subject: scons: Fix gem5 build fails when install-hooks is not set .. scons: Fix gem5 build fails when install-hooks is not set For interactive build, gem5 build fails when the git-hooks is not available since scons would look for `install-hooks` option, which is not necessarily set for interactive build. JIRA: https://gem5.atlassian.net/browse/GEM5-1091 Signed-off-by: Hoa Nguyen Change-Id: Ife248117420eb470c02d6e7536c1065256ba71e0 --- M site_scons/site_tools/git.py 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/site_scons/site_tools/git.py b/site_scons/site_tools/git.py index e95c62f..3a23276 100644 --- a/site_scons/site_tools/git.py +++ b/site_scons/site_tools/git.py @@ -100,9 +100,10 @@ return print(git_style_message, end=' ') -if SCons.Script.GetOption('install-hooks'): -print("Installing revision control hooks automatically.") -else: +try: +if SCons.Script.GetOption('install-hooks'): +print("Installing revision control hooks automatically.") +except AttributeError: try: input() except: -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50627 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: Ife248117420eb470c02d6e7536c1065256ba71e0 Gerrit-Change-Number: 50627 Gerrit-PatchSet: 1 Gerrit-Owner: Hoa Nguyen 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: Ensure using namespace * doesn't leak from generated ISA files.
Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/49887 ) Change subject: arch: Ensure using namespace * doesn't leak from generated ISA files. .. arch: Ensure using namespace * doesn't leak from generated ISA files. Only use "using namespace" with the *ISAInst namespace, not the top level namespace. Also only using namespace *ISA, and not the gem5 namespace itself. The *ISAInst namespace is already in the gem5 namespace, and so will resolve names in it automatically. Change-Id: Iebf3c9519c65baba073d73744665f8c98880804c Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49887 Reviewed-by: Daniel Carvalho Maintainer: Gabe Black Tested-by: kokoro --- M src/arch/arm/isa/includes.isa M src/arch/mips/isa/includes.isa M src/arch/power/isa/includes.isa M src/arch/riscv/isa/includes.isa M src/arch/sparc/isa/includes.isa M src/arch/x86/isa/includes.isa 6 files changed, 45 insertions(+), 30 deletions(-) Approvals: Daniel Carvalho: Looks good to me, approved Gabe Black: Looks good to me, approved kokoro: Regressions pass diff --git a/src/arch/arm/isa/includes.isa b/src/arch/arm/isa/includes.isa index 3ec3e65..c01eb72 100644 --- a/src/arch/arm/isa/includes.isa +++ b/src/arch/arm/isa/includes.isa @@ -70,13 +70,10 @@ #include "mem/packet.hh" #include "sim/faults.hh" -namespace gem5 -{ -namespace ArmISAInst +namespace gem5::ArmISAInst { using namespace ArmISA; -} // namespace ArmISAInst -} // namespace gem5 +} // namespace gem5::ArmISAInst }}; @@ -94,8 +91,10 @@ #include "base/loader/symtab.hh" #include "cpu/thread_context.hh" -using namespace gem5; +namespace gem5::ArmISAInst +{ using namespace ArmISA; +} // namespace gem5::ArmISAInst }}; output exec {{ @@ -120,7 +119,9 @@ #include "sim/pseudo_inst.hh" #include "sim/sim_exit.hh" -using namespace gem5; +namespace gem5::ArmISAInst +{ using namespace ArmISA; +} // namespace gem5::ArmISAInst }}; diff --git a/src/arch/mips/isa/includes.isa b/src/arch/mips/isa/includes.isa index 6802810..df9a4ce 100644 --- a/src/arch/mips/isa/includes.isa +++ b/src/arch/mips/isa/includes.isa @@ -64,8 +64,10 @@ #include "mem/packet.hh" #include "sim/full_system.hh" -using namespace gem5; +namespace gem5::MipsISAInst +{ using namespace MipsISA; +} // namespace gem5::MipsISAInst }}; output exec {{ @@ -96,7 +98,8 @@ #include "sim/sim_events.hh" #include "sim/sim_exit.hh" -using namespace gem5; +namespace gem5::MipsISAInst +{ using namespace MipsISA; +} // namespace gem5::MipsISAInst }}; - diff --git a/src/arch/power/isa/includes.isa b/src/arch/power/isa/includes.isa index e9b950b..5ab9e89 100644 --- a/src/arch/power/isa/includes.isa +++ b/src/arch/power/isa/includes.isa @@ -46,8 +46,10 @@ #include "cpu/static_inst.hh" #include "mem/packet.hh" -using namespace gem5; +namespace gem5::PowerISAInst +{ using namespace PowerISA; +} // namespace gem5::PowerISAInst }}; output decoder {{ @@ -60,8 +62,10 @@ #include "base/cprintf.hh" #include "cpu/thread_context.hh" -using namespace gem5; +namespace gem5::PowerISAInst +{ using namespace PowerISA; +} // namespace gem5::PowerISAInst }}; output exec {{ @@ -78,7 +82,8 @@ #include "mem/packet_access.hh" #include "sim/sim_exit.hh" -using namespace gem5; +namespace gem5::PowerISAInst +{ using namespace PowerISA; +} // namespace gem5::PowerISAInst }}; - diff --git a/src/arch/riscv/isa/includes.isa b/src/arch/riscv/isa/includes.isa index 6ffa277..88837e5 100644 --- a/src/arch/riscv/isa/includes.isa +++ b/src/arch/riscv/isa/includes.isa @@ -74,8 +74,10 @@ #include "mem/request.hh" #include "sim/full_system.hh" -using namespace gem5; +namespace gem5::RiscvISAInst +{ using namespace RiscvISA; +} // namespace gem5::RiscvISAInst }}; output exec {{ @@ -106,6 +108,8 @@ #include "sim/sim_exit.hh" #include "sim/system.hh" -using namespace gem5; +namespace gem5::RiscvISAInst +{ using namespace RiscvISA; +} // namespace gem5::RiscvISAInst }}; diff --git a/src/arch/sparc/isa/includes.isa b/src/arch/sparc/isa/includes.isa index c561056..d35619d 100644 --- a/src/arch/sparc/isa/includes.isa +++ b/src/arch/sparc/isa/includes.isa @@ -66,8 +66,10 @@ #include "cpu/thread_context.hh" // for Jump::branchTarget() #include "mem/packet.hh" -using namespace gem5; +namespace gem5::SparcISAInst +{ using namespace SparcISA; +} // namespace gem5::SparcISAInst }}; output exec {{ @@ -87,7 +89,8 @@ #include "sim/pseudo_inst.hh" #include "sim/sim_exit.hh" -using namespace gem5; +namespace gem5::SparcISAInst +{ using namespace SparcISA; +} // namespace gem5::SparcISAInst }}; - diff --git a/src/arch/x86/isa/includes.isa b/src/arch/x86/isa/includes.isa index fe28516e..bb07405 100644 --- a/src/arch/x86/isa/includes.isa +++ b/src/arch/x86/isa/includes.isa @@ -69,14 +69,10 @@ #include "mem/packet.hh" #include "sim/faults.hh" -using namespace gem5; -using
[gem5-dev] Change in gem5/gem5[develop]: base-stats: Revert "base-stats: Use std vector in vector stats"
Meng Chen has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/50607 ) Change subject: base-stats: Revert "base-stats: Use std vector in vector stats" .. base-stats: Revert "base-stats: Use std vector in vector stats" The reverted commit mistaken `storage` as an element in a std::vector. `storage` is a vector wrapper itself originally. The commit produce duplicated entries and wrong statistics in stat.txt. This reverts commit 70194795c3f41cc3f1e361b3cac24f839d86dd67. Change-Id: I4350863a83671fc10cc02b5cb7d3b38e6cf4f565 --- M src/base/statistics.hh 1 file changed, 67 insertions(+), 37 deletions(-) diff --git a/src/base/statistics.hh b/src/base/statistics.hh index 7714ce4..e380a87 100644 --- a/src/base/statistics.hh +++ b/src/base/statistics.hh @@ -1,5 +1,4 @@ /* - * Copyright (c) 2020 Inria * Copyright (c) 2019-2020 Arm Limited * All rights reserved. * @@ -929,7 +928,8 @@ protected: /** The storage of this stat. */ -std::vector storage; +Storage *storage; +size_type _size; protected: /** @@ -937,22 +937,28 @@ * @param index The vector index to access. * @return The storage object at the given index. */ -Storage *data(off_type index) { return storage[index]; } +Storage *data(off_type index) { return [index]; } /** * Retrieve a const pointer to the storage. * @param index The vector index to access. * @return A const pointer to the storage object at the given index. */ -const Storage *data(off_type index) const { return storage[index]; } +const Storage *data(off_type index) const { return [index]; } void doInit(size_type s) { -fatal_if(s <= 0, "Storage size must be positive"); -fatal_if(check(), "Stat has already been initialized"); +assert(s > 0 && "size must be positive!"); +assert(!storage && "already initialized"); +_size = s; -storage.resize(s, new Storage(this->info()->getStorageParams())); +char *ptr = new char[_size * sizeof(Storage)]; +storage = reinterpret_cast(ptr); + +for (off_type i = 0; i < _size; ++i) +new ([i]) Storage(this->info()->storageParams); + this->setInit(); } @@ -993,7 +999,7 @@ /** * @return the number of elements in this vector. */ -size_type size() const { return storage.size(); } +size_type size() const { return _size; } bool zero() const @@ -1007,7 +1013,7 @@ bool check() const { -return size() > 0; +return storage != NULL; } public: @@ -1015,14 +1021,17 @@ const units::Base *unit, const char *desc) : DataWrapVec(parent, name, unit, desc), - storage() + storage(nullptr), _size(0) {} ~VectorBase() { -for (auto& stor : storage) { -delete stor; -} +if (!storage) +return; + +for (off_type i = 0; i < _size; ++i) +data(i)->~Storage(); +delete [] reinterpret_cast(storage); } /** @@ -1143,32 +1152,36 @@ protected: size_type x; size_type y; -std::vector storage; +size_type _size; +Storage *storage; protected: -Storage *data(off_type index) { return storage[index]; } -const Storage *data(off_type index) const { return storage[index]; } +Storage *data(off_type index) { return [index]; } +const Storage *data(off_type index) const { return [index]; } public: Vector2dBase(Group *parent, const char *name, const units::Base *unit, const char *desc) : DataWrapVec2d(parent, name, unit, desc), - x(0), y(0), storage() + x(0), y(0), _size(0), storage(nullptr) {} ~Vector2dBase() { -for (auto& stor : storage) { -delete stor; -} +if (!storage) +return; + +for (off_type i = 0; i < _size; ++i) +data(i)->~Storage(); +delete [] reinterpret_cast(storage); } Derived & init(size_type _x, size_type _y) { -fatal_if((_x <= 0) || (_y <= 0), "Storage sizes must be positive"); -fatal_if(check(), "Stat has already been initialized"); +assert(_x > 0 && _y > 0 && "sizes must be positive!"); +assert(!storage && "already initialized"); Derived = this->self(); Info *info = this->info(); @@ -1177,8 +1190,14 @@ y = _y; info->x = _x; info->y = _y; +_size = x * y; -storage.resize(x * y, new Storage(info->getStorageParams())); +char *ptr = new char[_size * sizeof(Storage)]; +storage = reinterpret_cast(ptr); + +for (off_type i = 0; i < _size; ++i) +new ([i]) Storage(info->storageParams); +
[gem5-dev] Change in gem5/gem5[develop]: base-stats: Modify the invoke of storageParam in statistics.hh
Meng Chen has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/50608 ) Change subject: base-stats: Modify the invoke of storageParam in statistics.hh .. base-stats: Modify the invoke of storageParam in statistics.hh The previous revert (40cd47c) brought `statistics.h` into an old version. Update the invoke with the newer interface. Change-Id: I99b15ba3f84e4419f04f5315c725854267538dd1 --- M src/base/statistics.hh 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/base/statistics.hh b/src/base/statistics.hh index e380a87..d980e17 100644 --- a/src/base/statistics.hh +++ b/src/base/statistics.hh @@ -957,7 +957,7 @@ storage = reinterpret_cast(ptr); for (off_type i = 0; i < _size; ++i) -new ([i]) Storage(this->info()->storageParams); +new ([i]) Storage(this->info()->getStorageParams()); this->setInit(); } @@ -1196,7 +1196,7 @@ storage = reinterpret_cast(ptr); for (off_type i = 0; i < _size; ++i) -new ([i]) Storage(info->storageParams); +new ([i]) Storage(info->getStorageParams()); this->setInit(); @@ -1413,7 +1413,7 @@ Info *info = this->info(); for (off_type i = 0; i < _size; ++i) -new ([i]) Storage(info->storageParams); +new ([i]) Storage(info->getStorageParams()); this->setInit(); } -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50608 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: I99b15ba3f84e4419f04f5315c725854267538dd1 Gerrit-Change-Number: 50608 Gerrit-PatchSet: 1 Gerrit-Owner: Meng Chen 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: Squash a long standing warning from pybind11.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/50587 ) Change subject: sim: Squash a long standing warning from pybind11. .. sim: Squash a long standing warning from pybind11. The module_ constructor which takes a module name and an optional docstring is deprecated. This change replaces it with the create_extension_module call that it would result in, which gets rid of the warning. Change-Id: I700b4afcf1e5e2548af18e2eb2a7b1214c989807 --- M src/sim/init.cc 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/sim/init.cc b/src/sim/init.cc index 0c69958..ed54742 100644 --- a/src/sim/init.cc +++ b/src/sim/init.cc @@ -193,7 +193,17 @@ { std::list pending; -py::module_ m_m5 = py::module_("_m5"); +// The PyModuleDef structure needs to live as long as the module it +// defines, so we'll leak it here so it lives forever. This is what +// pybind11 does internally in the module_ constructor we were using. We +// could theoretically keep track of the lifetime of the _m5 module +// somehow and clean this up when it goes away, but that doesn't seem +// worth the effort. The docs recommend statically allocating it, but that +// could be unsafe on the very slim chance this method is called more than +// once. +auto *py_mod_def = new py::module_::module_def; +py::module_ m_m5 = py::module_::create_extension_module( +"_m5", nullptr, py_mod_def); m_m5.attr("__package__") = py::cast("_m5"); pybind_init_core(m_m5); -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50587 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: I700b4afcf1e5e2548af18e2eb2a7b1214c989807 Gerrit-Change-Number: 50587 Gerrit-PatchSet: 1 Gerrit-Owner: Gabe Black Gerrit-MessageType: newchange ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: tests: Use --silent-redirect when running gem5 tests.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/50589 ) Change subject: tests: Use --silent-redirect when running gem5 tests. .. tests: Use --silent-redirect when running gem5 tests. The redirects are to temp files which are going to be deleted at the end of the test, and there's no reason to clutter up the output with nonsense paths to non-existent files. Change-Id: Id312b1f25f03d7c57318ca0c58902e4193bd1e91 --- M tests/gem5/suite.py 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/gem5/suite.py b/tests/gem5/suite.py index 3b0f1f8..cef3e7d 100644 --- a/tests/gem5/suite.py +++ b/tests/gem5/suite.py @@ -168,9 +168,10 @@ gem5 = fixtures[constants.gem5_binary_fixture_name].path command = [ gem5, -'-d', # Set redirect dir to tempdir. +'-d', # Set redirect dir to tempdir. tempdir, -'-re',# TODO: Change to const. Redirect stdout and stderr +'-re', # TODO: Change to const. Redirect stdout and stderr +'--silent-redirect', ] command.extend(_gem5_args) command.append(config) -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50589 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: Id312b1f25f03d7c57318ca0c58902e4193bd1e91 Gerrit-Change-Number: 50589 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]: python: Add a --silent-redirect option to gem5.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/50588 ) Change subject: python: Add a --silent-redirect option to gem5. .. python: Add a --silent-redirect option to gem5. This complements the --redirect-stdout and --redirect-stderr options and supresses the message about where those streams are being redirected which print to the original stdout. Usually this is very helpful since it lets you know where to look for simulator output. If you're running gem5 in an automated environment like our testing framework however, the file name is a random temp file which will be deleted as soon as the test is finished running. The --silent-redirect option can be used in these particular scenarios to, for example, avoid lots and lots of useless lines in the test output naming files that no longer exist. Change-Id: If56b61567b3d98abd9cc9d9e9d661ea561be46f8 --- M src/python/m5/main.py 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/python/m5/main.py b/src/python/m5/main.py index 92878af..f3be50f 100644 --- a/src/python/m5/main.py +++ b/src/python/m5/main.py @@ -87,6 +87,8 @@ help="Redirect stdout (& stderr, without -e) to file") option('-e', "--redirect-stderr", action="store_true", default=False, help="Redirect stderr to file") +option("--silent-redirect", action="store_true", default=False, +help="Suppress printing a message when redirecting stdout or stderr") option("--stdout-file", metavar="FILE", default="simout", help="Filename for -r redirection [Default: %default]") option("--stderr-file", metavar="FILE", default="simerr", @@ -247,14 +249,15 @@ stdout_file = os.path.join(options.outdir, options.stdout_file) stderr_file = os.path.join(options.outdir, options.stderr_file) -# Print redirection notices here before doing any redirection -if options.redirect_stdout and not options.redirect_stderr: -print("Redirecting stdout and stderr to", stdout_file) -else: -if options.redirect_stdout: -print("Redirecting stdout to", stdout_file) -if options.redirect_stderr: -print("Redirecting stderr to", stderr_file) +if not options.silent_redirect: +# Print redirection notices here before doing any redirection +if options.redirect_stdout and not options.redirect_stderr: +print("Redirecting stdout and stderr to", stdout_file) +else: +if options.redirect_stdout: +print("Redirecting stdout to", stdout_file) +if options.redirect_stderr: +print("Redirecting stderr to", stderr_file) # Now redirect stdout/stderr as desired if options.redirect_stdout: -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50588 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: If56b61567b3d98abd9cc9d9e9d661ea561be46f8 Gerrit-Change-Number: 50588 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