date:20211216

[gem5-dev] Change in gem5/gem5[develop]: misc: Merge branch 'release-staging-v21-2' into develop

2021-12-16 Thread Bobby Bruce (Gerrit) via gem5-dev

Bobby Bruce has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54383 )



Change subject: misc: Merge branch 'release-staging-v21-2' into develop
..

misc: Merge branch 'release-staging-v21-2' into develop

Change-Id: Icc03e585d87cf89ed844a0249c365cc296fa2d14
---
M ext/sst/gem5.cc
2 files changed, 30 insertions(+), 19 deletions(-)



diff --git a/ext/sst/gem5.cc b/ext/sst/gem5.cc
index abefb52..7af0eed 100644
--- a/ext/sst/gem5.cc
+++ b/ext/sst/gem5.cc
@@ -382,21 +382,6 @@
 // Initialize gem5 special signal handling.
 gem5::initSignals();

-<<< HEAD   (735f76 mem-cache: Added stats filtering both useful and  
spanPage pr)

-if (!Py_IsInitialized())
-py::initialize_interpreter(false, argc, _argv);
-
-auto importer = py::module_::import("importer");
-importer.attr("install")();
-
-try {
-py::module_::import("m5").attr("main")();
-} catch (py::error_already_set ) {
-if (!e.matches(PyExc_SystemExit)) {
-std::cerr << e.what();
-output.output(CALL_INFO, "Calling m5.main(...) failed.\n");
-}
-===
 if (!Py_IsInitialized()) {
 py::initialize_interpreter(true, argc, _argv);
 } else {
@@ -417,10 +402,7 @@
 // Fill it with our argvs.
 for (int i = 0; i < argc; i++)
 py_argv.append(_argv[i]);
->>> BRANCH (c7689b ext: Fix dumping stats error at the end of  
simulation)

 }
-<<< HEAD   (735f76 mem-cache: Added stats filtering both useful and  
spanPage pr)

-===

 auto importer = py::module_::import("importer");
 importer.attr("install")();
@@ -433,7 +415,6 @@
 output.output(CALL_INFO, "Calling m5.main(...) failed.\n");
 }
 }
->>> BRANCH (c7689b ext: Fix dumping stats error at the end of  
simulation)

 }

 void

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54383
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: Icc03e585d87cf89ed844a0249c365cc296fa2d14
Gerrit-Change-Number: 54383
Gerrit-PatchSet: 1
Gerrit-Owner: Bobby Bruce 
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-2]: sim,python: Use scoped_interpreter_guard's argc and argv ctor arguments.

2021-12-16 Thread Bobby Bruce (Gerrit) via gem5-dev

Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54309 )


Change subject: sim,python: Use scoped_interpreter_guard's argc and argv  
ctor arguments.

..

sim,python: Use scoped_interpreter_guard's argc and argv ctor arguments.

When pybind11's scoped_interpreter_guard is constructed, it can take
argc and argv parameters which it uses to intitialize the sys.argv list
in python. We can use that mechanism instead of setting that up manually
ourselves.

Change-Id: If34fac610d1f829aef313bb9ea4c9dfe6bc8fc0f
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54309
Reviewed-by: Andreas Sandberg 
Maintainer: Gabe Black 
Reviewed-by: Jason Lowe-Power 
Tested-by: kokoro 
---
M src/sim/main.cc
1 file changed, 20 insertions(+), 18 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Andreas Sandberg: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/sim/main.cc b/src/sim/main.cc
index c44531e..81a691d 100644
--- a/src/sim/main.cc
+++ b/src/sim/main.cc
@@ -60,28 +60,11 @@
 // It's probably not necessary, but is mostly harmless and might be  
useful.

 Py_SetProgramName(program.get());

-py::scoped_interpreter guard;
+py::scoped_interpreter guard(true, argc, argv);

 auto importer = py::module_::import("importer");
 importer.attr("install")();

-// Embedded python doesn't set up sys.argv, so we'll do that ourselves.
-py::list py_argv;
-auto sys = py::module::import("sys");
-if (py::hasattr(sys, "argv")) {
-// sys.argv already exists, so grab that.
-py_argv = sys.attr("argv");
-} else {
-// sys.argv doesn't exist, so create it.
-sys.add_object("argv", py_argv);
-}
-// Clear out argv just in case it has something in it.
-py_argv.attr("clear")();
-
-// Fill it with our argvs.
-for (int i = 0; i < argc; i++)
-py_argv.append(argv[i]);
-
 try {
 py::module_::import("m5").attr("main")();
 } catch (py::error_already_set ) {

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54309
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-2
Gerrit-Change-Id: If34fac610d1f829aef313bb9ea4c9dfe6bc8fc0f
Gerrit-Change-Number: 54309
Gerrit-PatchSet: 2
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Hoa Nguyen 
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[release-staging-v21-2]: systemc: Change how python initialization callbacks are handled.

2021-12-16 Thread Bobby Bruce (Gerrit) via gem5-dev

Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54325 )


Change subject: systemc: Change how python initialization callbacks are  
handled.

..

systemc: Change how python initialization callbacks are handled.

Because the python environment may already be up and running by the time
static initializers are run, specifically when gem5 is built as a
library and loaded with dlopen, we can't rely on all of the objects
declaring python initialization callbacks having been constructed by the
time the code which would execute them runs.

To address that problem, this change keeps track of whether the
initialization has already happened when a callback is installed, and if
so, runs the callback immediately.

The original implementation also had users install callbacks by
overriding a virtual function in the PythonInitFunc class, and then
statically allocating an instance of that subclass so its constructor
would be called at initialization time. Calling the function manually if
initialization has already happened won't work in that case, because you
can't call a virtual function from a constructor and get the behavior
you'd want.

Instead, this change makes the PythonInitFunc wrap the actual callback
which is outside of the structure itself. Because the callback is not a
virtual function of PythonInitFunc, we can call it in the constructor
without issue.

Also, the Callback type has to be a bare function pointer and not a
std::function<...> because the argument it takes is a pybind11::module_
reference. Pybind11 sets the visibility of all of its code to hidden to
improve binary size, but unfortunately that causes problems when
accepting one as an argument in a publically accessible lambda in g++.
clang doesn't raise a warning, but g++ does which breaks the build. We
could potentially disable this warning, but accepting a function pointer
instead works just as well, since captureless lambdas can be trivially
converted into function pointers, and they don't seem to upset g++.

Change-Id: I3fb321b577090df67c7be3be0e677c2c2055d446
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54325
Maintainer: Gabe Black 
Reviewed-by: Jason Lowe-Power 
Tested-by: kokoro 
---
M src/systemc/core/sc_time_python.cc
M src/systemc/core/sc_main_python.cc
M src/systemc/core/python.cc
M src/systemc/core/python.hh
M src/systemc/tlm_core/2/quantum/global_quantum_python.cc
5 files changed, 115 insertions(+), 71 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/systemc/core/python.cc b/src/systemc/core/python.cc
index 2283ae7..472a050 100644
--- a/src/systemc/core/python.cc
+++ b/src/systemc/core/python.cc
@@ -45,20 +45,31 @@
 return first;
 }

+bool python_initialized = false;
+
 void
 systemc_pybind(pybind11::module_ _internal)
 {
 pybind11::module_ m = m_internal.def_submodule("systemc");
 for (auto ptr = firstInitFunc(); ptr; ptr = ptr->next)
-ptr->run(m);
+ptr->callback(m);
+
+python_initialized = true;
 }
 gem5::EmbeddedPyBind embed_("systemc", _pybind);

 } // anonymous namespace

-PythonInitFunc::PythonInitFunc() : next(firstInitFunc())
+PythonInitFunc::PythonInitFunc(Callback run) :
+callback(run), next(firstInitFunc())
 {
 firstInitFunc() = this;
+
+// If the python was already initialized, run the callback immediately.
+if (python_initialized) {
+auto systemc_module = pybind11::module_::import("_m5.systemc");
+callback(systemc_module);
+}
 }

 } // namespace sc_gem5
diff --git a/src/systemc/core/python.hh b/src/systemc/core/python.hh
index 5a3f6ef..3c563db 100644
--- a/src/systemc/core/python.hh
+++ b/src/systemc/core/python.hh
@@ -35,11 +35,12 @@

 struct PythonInitFunc
 {
+using Callback = void(*)(pybind11::module_ );
+Callback callback;
+
 PythonInitFunc *next;

-PythonInitFunc();
-~PythonInitFunc() {}
-virtual void run(pybind11::module_ ) = 0;
+PythonInitFunc(Callback run);
 };

 } // namespace sc_gem5
diff --git a/src/systemc/core/sc_main_python.cc  
b/src/systemc/core/sc_main_python.cc

index 1697efe..8d2542e 100644
--- a/src/systemc/core/sc_main_python.cc
+++ b/src/systemc/core/sc_main_python.cc
@@ -92,15 +92,10 @@
 // Make our sc_main wrapper available in the internal _m5 python module  
under

 // the systemc submodule.

-struct InstallScMain : public ::sc_gem5::PythonInitFunc
-{
-void
-run(pybind11::module_ ) override
-{
-systemc.def("sc_main", _main);
-systemc.def("sc_main_result_code", _main_result_code);
-systemc.def("sc_main_result_str", _main_result_str);
-}
-} installScMain;
+::sc_gem5::PythonInitFunc installScMain([](pybind11::module_ ) {
+systemc.def("sc_main", _main);
+systemc.def("sc_main_result_code", _main_result_code);
+

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: ext: In sst, don't assume existing imports in python blobs.

2021-12-16 Thread Bobby Bruce (Gerrit) via gem5-dev

Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54308 )


Change subject: ext: In sst, don't assume existing imports in python blobs.
..

ext: In sst, don't assume existing imports in python blobs.

When executing blobs of python code in the gem5 sst integration, don't
assume previous blobs have already imported modules. Import them
explicitly instead.

Change-Id: I3aa008ffa092cf8ad82ad057eb73bc9de4bf77c5
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54308
Maintainer: Gabe Black 
Reviewed-by: Jason Lowe-Power 
Tested-by: kokoro 
---
M ext/sst/gem5.cc
1 file changed, 23 insertions(+), 1 deletion(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/ext/sst/gem5.cc b/ext/sst/gem5.cc
index 286f519..01ea3eb 100644
--- a/ext/sst/gem5.cc
+++ b/ext/sst/gem5.cc
@@ -194,6 +194,7 @@
 initPython(args.size(), [0]);

 const std::vector m5_instantiate_commands = {
+"import m5",
 "m5.instantiate()"
 };
 execPythonCommands(m5_instantiate_commands);
@@ -201,7 +202,10 @@
 // calling SimObject.startup()
 const std::vector simobject_setup_commands = {
 "import atexit",
-"import _m5",
+"import _m5.core",
+"import m5",
+"import m5.stats",
+"import m5.objects.Root",
 "root = m5.objects.Root.getInstance()",
 "for obj in root.descendants(): obj.startup()",
 "atexit.register(m5.stats.dump)",
@@ -258,6 +262,7 @@
 );
 // output gem5 stats
 const std::vector output_stats_commands = {
+"import m5.stats"
 "m5.stats.dump()"
 };
 execPythonCommands(output_stats_commands);

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54308
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-2
Gerrit-Change-Id: I3aa008ffa092cf8ad82ad057eb73bc9de4bf77c5
Gerrit-Change-Number: 54308
Gerrit-PatchSet: 2
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Hoa Nguyen 
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[release-staging-v21-2]: ext: In sst, set sys.argv up even when not initializing python.

2021-12-16 Thread Bobby Bruce (Gerrit) via gem5-dev

Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54310 )


Change subject: ext: In sst, set sys.argv up even when not initializing  
python.

..

ext: In sst, set sys.argv up even when not initializing python.

pybind11 gives us a simple way to set up sys.argv when initializing the
python interpreter, but we need to set that up even if the python
interpreter is already running. We need to do that manually, which we do
like gem5's main() used to.

Change-Id: If9b79a80e05158226d13f68bf06a2a98a36c2602
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54310
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M ext/sst/gem5.cc
1 file changed, 39 insertions(+), 2 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/ext/sst/gem5.cc b/ext/sst/gem5.cc
index 01ea3eb..8eab6281 100644
--- a/ext/sst/gem5.cc
+++ b/ext/sst/gem5.cc
@@ -382,8 +382,27 @@
 // Initialize gem5 special signal handling.
 gem5::initSignals();

-if (!Py_IsInitialized())
-py::initialize_interpreter(false, argc, _argv);
+if (!Py_IsInitialized()) {
+py::initialize_interpreter(true, argc, _argv);
+} else {
+// pybind doesn't provide a way to set sys.argv if not  
initializing the
+// interpreter, so we have to do that manually if it's already  
running.

+py::list py_argv;
+auto sys = py::module::import("sys");
+if (py::hasattr(sys, "argv")) {
+// sys.argv already exists, so grab that.
+py_argv = sys.attr("argv");
+} else {
+// sys.argv doesn't exist, so create it.
+sys.add_object("argv", py_argv);
+}
+// Clear out argv just in case it has something in it.
+py_argv.attr("clear")();
+
+// Fill it with our argvs.
+for (int i = 0; i < argc; i++)
+py_argv.append(_argv[i]);
+}

 auto importer = py::module_::import("importer");
 importer.attr("install")();

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54310
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-2
Gerrit-Change-Id: If9b79a80e05158226d13f68bf06a2a98a36c2602
Gerrit-Change-Number: 54310
Gerrit-PatchSet: 2
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Hoa Nguyen 
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[release-staging-v21-2]: ext: Stop using the uninitialized pythonMain in sst.

2021-12-16 Thread Bobby Bruce (Gerrit) via gem5-dev

Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54307 )


Change subject: ext: Stop using the uninitialized pythonMain in sst.
..

ext: Stop using the uninitialized pythonMain in sst.

Import the __main__ module when it's first used.

Change-Id: If800bd575398970faa8cb88072becd3d2b4218c0
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54307
Maintainer: Gabe Black 
Reviewed-by: Jason Lowe-Power 
Tested-by: kokoro 
---
M ext/sst/gem5.cc
M ext/sst/gem5.hh
2 files changed, 17 insertions(+), 2 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/ext/sst/gem5.cc b/ext/sst/gem5.cc
index 1d90bc5..286f519 100644
--- a/ext/sst/gem5.cc
+++ b/ext/sst/gem5.cc
@@ -355,7 +355,8 @@
 int
 gem5Component::execPythonCommands(const std::vector& commands)
 {
-PyObject *dict = PyModule_GetDict(pythonMain);
+static PyObject *dict =
+py::module_::import("__main__").attr("__dict__").ptr();

 PyObject *result;

diff --git a/ext/sst/gem5.hh b/ext/sst/gem5.hh
index 2712411..447c68c 100644
--- a/ext/sst/gem5.hh
+++ b/ext/sst/gem5.hh
@@ -104,7 +104,6 @@

   // stuff needed for gem5 sim
   public:
-PyObject *pythonMain;
 int execPythonCommands(const std::vector& commands);

   private:

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54307
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-2
Gerrit-Change-Id: If800bd575398970faa8cb88072becd3d2b4218c0
Gerrit-Change-Number: 54307
Gerrit-PatchSet: 2
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Hoa Nguyen 
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[release-staging-v21-2]: systemc: Update the testing framework to get it working again.

2021-12-16 Thread Bobby Bruce (Gerrit) via gem5-dev

Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54324 )


Change subject: systemc: Update the testing framework to get it working  
again.

..

systemc: Update the testing framework to get it working again.

The systemc testing framework is not used regularly, and had bit rot and
stopped working. This change updates it so that it runs again, and all
previously passing tests pass again.

These changes were mostly in the related SConscript now that top level
targets are built a little differently and that the gem5 shared library
is no longer stored in a special construction environment variable.
verify.py also needed to be updated since warn() and info() lines now
have file and line number information in them, throwing off pre diff
filtering of gem5 outputs.

Change-Id: Ifdcbd92eab8b9b2168c449bfbcebf52dbe1f016a
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54324
Maintainer: Gabe Black 
Reviewed-by: Jason Lowe-Power 
Tested-by: kokoro 
---
M src/systemc/tests/verify.py
M src/systemc/tests/SConscript
2 files changed, 35 insertions(+), 20 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/systemc/tests/SConscript b/src/systemc/tests/SConscript
index 7d544f2..fb916d2 100644
--- a/src/systemc/tests/SConscript
+++ b/src/systemc/tests/SConscript
@@ -63,7 +63,8 @@
 test_dir = Dir('.')
 class SystemCTestBin(Executable):
 def __init__(self, test):
-super().__init__(test.target, *test.sources)
+all_sources = test.sources + [with_tag('main')]
+super().__init__(test.target, *all_sources)
 self.reldir = test.reldir
 self.test_deps = test.deps

@@ -78,26 +79,16 @@

 env.Append(CPPPATH=test_dir.Dir('include'))

-shared_lib_path = env['SHARED_LIB'][0].abspath
-sl_dir, sl_base = os.path.split(shared_lib_path)
-env.Append(LIBPATH=[sl_dir], LIBS=[sl_base])
+env.Append(LIBPATH=['${BUILDDIR}'], LIBS=['gem5_${ENV_LABEL}'])
+env.AddLocalRPATH('${BUILDDIR}')
+
+env['OBJSUFFIX'] = '.sc' + env['OBJSUFFIX'][1:]
+env['SHOBJSUFFIX'] = '.sc' + env['OBJSUFFIX'][1:]

 super().declare_all(env)

 def declare(self, env):
-env = env.Clone()
-sources = list(self.sources)
-for f in self.filters:
-sources += Source.all.apply_filter(f)
-objs = self.srcs_to_objs(env, sources)
-objs = objs + env['MAIN_OBJS']
-relpath = os.path.relpath(
-env['SHARED_LIB'][0].dir.abspath,
-self.path(env).dir.abspath)
-env.Append(LINKFLAGS=Split('-z origin'))
-env.Append(RPATH=[
-env.Literal(os.path.join('\\$$ORIGIN', relpath))])
-test_bin = super().declare(env, objs)
+test_bin, _u = super().declare(env)
 test_dir = self.dir.Dir(self.reldir)
 for dep in self.test_deps:
 env.Depends(test_bin, test_dir.File(dep))
diff --git a/src/systemc/tests/verify.py b/src/systemc/tests/verify.py
index 54b4633..818855a 100755
--- a/src/systemc/tests/verify.py
+++ b/src/systemc/tests/verify.py
@@ -277,9 +277,9 @@
 test_filt = merge_filts(
 r'^/.*:\d+: ',
 r'^Global frequency set at \d* ticks per second\n',
-r'info: Entering event queue @ \d*\.  Starting simulation\.\.\.\n',
-r'warn: Ignoring request to set stack size\.\n',
-r'^warn: No dot file generated. Please install pydot ' +
+r'.*info: Entering event queue @ \d*\.  Starting  
simulation\.\.\.\n',

+r'.*warn: Ignoring request to set stack size\.\n',
+r'^.*warn: No dot file generated. Please install pydot ' +
 r'to generate the dot file and pdf.\n',
 info_filt(804),
 in_file_filt,

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54324
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-2
Gerrit-Change-Id: Ifdcbd92eab8b9b2168c449bfbcebf52dbe1f016a
Gerrit-Change-Number: 54324
Gerrit-PatchSet: 2
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Hoa Nguyen 
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[release-staging-v21-2]: ext: Fix dumping stats error at the end of simulation

2021-12-16 Thread Bobby Bruce (Gerrit) via gem5-dev

Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54343 )


Change subject: ext: Fix dumping stats error at the end of simulation
..

ext: Fix dumping stats error at the end of simulation

A missing comma causing two commands to be one, which resulted in
a Python interpreter syntax error.

Change-Id: Ieb0e3c620c175731084ff0e2040388b15364691e
Signed-off-by: Hoa Nguyen 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54343
Reviewed-by: Bobby Bruce 
Maintainer: Bobby Bruce 
Tested-by: kokoro 
---
M ext/sst/gem5.cc
1 file changed, 18 insertions(+), 1 deletion(-)

Approvals:
  Bobby Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/ext/sst/gem5.cc b/ext/sst/gem5.cc
index 8eab6281..7af0eed 100644
--- a/ext/sst/gem5.cc
+++ b/ext/sst/gem5.cc
@@ -262,7 +262,7 @@
 );
 // output gem5 stats
 const std::vector output_stats_commands = {
-"import m5.stats"
+"import m5.stats",
 "m5.stats.dump()"
 };
 execPythonCommands(output_stats_commands);

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54343
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-2
Gerrit-Change-Id: Ieb0e3c620c175731084ff0e2040388b15364691e
Gerrit-Change-Number: 54343
Gerrit-PatchSet: 2
Gerrit-Owner: Hoa Nguyen 
Gerrit-Reviewer: Bobby Bruce 
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[release-staging-v21-2]: systemc: Eliminate the unused PythonReadyFunc mechanism.

2021-12-16 Thread Bobby Bruce (Gerrit) via gem5-dev

Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54323 )


Change subject: systemc: Eliminate the unused PythonReadyFunc mechanism.
..

systemc: Eliminate the unused PythonReadyFunc mechanism.

Change-Id: I8892e4d209901454f2ab923aa3fa9932d7963274
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54323
Maintainer: Gabe Black 
Reviewed-by: Jason Lowe-Power 
Tested-by: kokoro 
---
M src/systemc/core/SystemC.py
M src/systemc/core/python.cc
M src/systemc/core/python.hh
3 files changed, 13 insertions(+), 37 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/systemc/core/SystemC.py b/src/systemc/core/SystemC.py
index 6828175..592b950 100644
--- a/src/systemc/core/SystemC.py
+++ b/src/systemc/core/SystemC.py
@@ -63,11 +63,3 @@
 @cxxMethod(return_value_policy="reference", cxx_name="gem5_getPort")
 def getPort(self, if_name, iex):
 return None
-
-try:
-import _m5
-except:
-pass
-else:
-import _m5.systemc
-_m5.systemc.python_ready()
diff --git a/src/systemc/core/python.cc b/src/systemc/core/python.cc
index 8f2d56b..2283ae7 100644
--- a/src/systemc/core/python.cc
+++ b/src/systemc/core/python.cc
@@ -38,13 +38,6 @@
 namespace
 {

-PythonReadyFunc *&
-firstReadyFunc()
-{
-static PythonReadyFunc *first = nullptr;
-return first;
-}
-
 PythonInitFunc *&
 firstInitFunc()
 {
@@ -53,17 +46,9 @@
 }

 void
-python_ready(pybind11::args args)
-{
-for (auto ptr = firstReadyFunc(); ptr; ptr = ptr->next)
-ptr->run();
-}
-
-void
 systemc_pybind(pybind11::module_ _internal)
 {
 pybind11::module_ m = m_internal.def_submodule("systemc");
-m.def("python_ready", _ready);
 for (auto ptr = firstInitFunc(); ptr; ptr = ptr->next)
 ptr->run(m);
 }
@@ -71,11 +56,6 @@

 } // anonymous namespace

-PythonReadyFunc::PythonReadyFunc() : next(firstReadyFunc())
-{
-firstReadyFunc() = this;
-}
-
 PythonInitFunc::PythonInitFunc() : next(firstInitFunc())
 {
 firstInitFunc() = this;
diff --git a/src/systemc/core/python.hh b/src/systemc/core/python.hh
index 61c9c80..5a3f6ef 100644
--- a/src/systemc/core/python.hh
+++ b/src/systemc/core/python.hh
@@ -33,15 +33,6 @@
 namespace sc_gem5
 {

-struct PythonReadyFunc
-{
-PythonReadyFunc *next;
-
-PythonReadyFunc();
-~PythonReadyFunc() {}
-virtual void run() = 0;
-};
-
 struct PythonInitFunc
 {
 PythonInitFunc *next;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54323
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-2
Gerrit-Change-Id: I8892e4d209901454f2ab923aa3fa9932d7963274
Gerrit-Change-Number: 54323
Gerrit-PatchSet: 2
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Hoa Nguyen 
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[release-staging-v21-2]: arch,sim-se: Handle syscall retry/suppression in the syscall desc.

2021-12-16 Thread Gabe Black (Gerrit) via gem5-dev

Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54204 )


Change subject: arch,sim-se: Handle syscall retry/suppression in the  
syscall desc.

..

arch,sim-se: Handle syscall retry/suppression in the syscall desc.

Rather than make each ISA include boilerplate to ignore a
SyscallReturn's value when it's marked as suppressed or needing a retry,
put that code into the SyscallDesc::doSyscall method instead.

That has two benefits. First, it removes a decent amount of code
duplication which is nice from a maintenance perspective. Second, it
puts the SyscallDesc in charge of figuring out what to do once a system
call implementation finishes. That will let it schedule a retry of the
system call for instance, without worrying about what the ISA is doing
with the SyscallReturn behind its back.

Jira Issue: https://gem5.atlassian.net/browse/GEM5-1123

Change-Id: I76760cba75fd23e6e3357f6169c0140bee3f01b6
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54204
Reviewed-by: Giacomo Travaglini 
Maintainer: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/arch/sparc/se_workload.hh
M src/arch/x86/linux/linux.hh
M src/arch/arm/freebsd/se_workload.hh
M src/arch/riscv/se_workload.hh
M src/arch/arm/linux/se_workload.hh
M src/arch/mips/se_workload.hh
M src/sim/syscall_desc.cc
M src/sim/syscall_desc.hh
M src/arch/power/se_workload.hh
9 files changed, 32 insertions(+), 25 deletions(-)

Approvals:
  Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/arch/arm/freebsd/se_workload.hh  
b/src/arch/arm/freebsd/se_workload.hh

index 8069bd2..4ec5090 100644
--- a/src/arch/arm/freebsd/se_workload.hh
+++ b/src/arch/arm/freebsd/se_workload.hh
@@ -82,9 +82,6 @@
 static void
 store(ThreadContext *tc, const SyscallReturn )
 {
-if (ret.suppressed() || ret.needsRetry())
-return;
-
 RegVal val;
 if (ret.successful()) {
 tc->setCCReg(ArmISA::CCREG_C, 0);
diff --git a/src/arch/arm/linux/se_workload.hh  
b/src/arch/arm/linux/se_workload.hh

index b22688f..f0af647 100644
--- a/src/arch/arm/linux/se_workload.hh
+++ b/src/arch/arm/linux/se_workload.hh
@@ -74,9 +74,6 @@
 static void
 store(ThreadContext *tc, const SyscallReturn )
 {
-if (ret.suppressed() || ret.needsRetry())
-return;
-
 tc->setIntReg(ArmISA::ReturnValueReg, ret.encodedValue());
 if (ret.count() > 1)
 tc->setIntReg(ArmISA::SyscallPseudoReturnReg, ret.value2());
diff --git a/src/arch/mips/se_workload.hh b/src/arch/mips/se_workload.hh
index 178093d..c10ceb0 100644
--- a/src/arch/mips/se_workload.hh
+++ b/src/arch/mips/se_workload.hh
@@ -77,9 +77,6 @@
 static void
 store(ThreadContext *tc, const SyscallReturn )
 {
-if (ret.suppressed() || ret.needsRetry())
-return;
-
 if (ret.successful()) {
 // no error
 tc->setIntReg(MipsISA::SyscallSuccessReg, 0);
diff --git a/src/arch/power/se_workload.hh b/src/arch/power/se_workload.hh
index 2b62795..c351d65 100644
--- a/src/arch/power/se_workload.hh
+++ b/src/arch/power/se_workload.hh
@@ -77,9 +77,6 @@
 static void
 store(ThreadContext *tc, const SyscallReturn )
 {
-if (ret.suppressed() || ret.needsRetry())
-return;
-
 PowerISA::Cr cr = tc->readIntReg(PowerISA::INTREG_CR);
 if (ret.successful()) {
 cr.cr0.so = 0;
diff --git a/src/arch/riscv/se_workload.hh b/src/arch/riscv/se_workload.hh
index 1f8a067..484803e 100644
--- a/src/arch/riscv/se_workload.hh
+++ b/src/arch/riscv/se_workload.hh
@@ -75,9 +75,6 @@
 static void
 store(ThreadContext *tc, const SyscallReturn )
 {
-if (ret.suppressed() || ret.needsRetry())
-return;
-
 if (ret.successful()) {
 // no error
 tc->setIntReg(RiscvISA::ReturnValueReg, ret.returnValue());
diff --git a/src/arch/sparc/se_workload.hh b/src/arch/sparc/se_workload.hh
index 6d034f7..18988fe 100644
--- a/src/arch/sparc/se_workload.hh
+++ b/src/arch/sparc/se_workload.hh
@@ -89,9 +89,6 @@
 static void
 store(ThreadContext *tc, const SyscallReturn )
 {
-if (ret.suppressed() || ret.needsRetry())
-return;
-
 // check for error condition.  SPARC syscall convention is to
 // indicate success/failure in reg the carry bit of the ccr
 // and put the return value itself in the standard return value  
reg.

diff --git a/src/arch/x86/linux/linux.hh b/src/arch/x86/linux/linux.hh
index cbf7358..928c580 100644
--- a/src/arch/x86/linux/linux.hh
+++ b/src/arch/x86/linux/linux.hh
@@ -86,9 +86,6 @@
 static void
 store(ThreadContext *tc, const SyscallReturn )
 {
-if (ret.suppressed() || ret.needsRetry())
-return;
-
 tc->setIntReg(X86ISA::INTREG_RAX,

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: sim-se: (Re)add support for retrying system calls.

2021-12-16 Thread Gabe Black (Gerrit) via gem5-dev

Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54205 )


Change subject: sim-se: (Re)add support for retrying system calls.
..

sim-se: (Re)add support for retrying system calls.

The previous incarnation of this support used faults to make the CPU
reexecute the system call instruction again and again to prevent
emulating/passing through blocking system calls from blocking gem5 as
a whole. That support was accidentally removed a while ago. This new
version suspends the thread context executing the system call, and
periodically wakes it up to retry using a periodically scheduled event.

Jira Issue: https://gem5.atlassian.net/browse/GEM5-1123

Change-Id: I155fa8205d7ea45e3d102216aeca6ee1979a522f
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54205
Reviewed-by: Giacomo Travaglini 
Maintainer: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/sim/syscall_desc.cc
M src/sim/syscall_desc.hh
2 files changed, 78 insertions(+), 4 deletions(-)

Approvals:
  Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/sim/syscall_desc.cc b/src/sim/syscall_desc.cc
index 7427f41..d293167 100644
--- a/src/sim/syscall_desc.cc
+++ b/src/sim/syscall_desc.cc
@@ -30,6 +30,7 @@
 #include "sim/syscall_desc.hh"

 #include "base/types.hh"
+#include "sim/eventq.hh"
 #include "sim/syscall_debug_macros.hh"

 namespace gem5
@@ -45,12 +46,58 @@
 SyscallReturn retval = executor(this, tc);

 if (retval.needsRetry()) {
-DPRINTF_SYSCALL(Base, "Needs retry.\n", name());
-} else if (retval.suppressed()) {
+// Suspend this ThreadContext while the syscall is pending.
+tc->suspend();
+
+DPRINTF_SYSCALL(Base, "%s needs retry.\n", name());
+setupRetry(tc);
+return;
+}
+
+handleReturn(tc, retval);
+}
+
+void
+SyscallDesc::retrySyscall(ThreadContext *tc)
+{
+DPRINTF_SYSCALL(Base, "Retrying %s...\n", dumper(name(), tc));
+
+SyscallReturn retval = executor(this, tc);
+
+if (retval.needsRetry()) {
+DPRINTF_SYSCALL(Base, "%s still needs retry.\n", name());
+setupRetry(tc);
+return;
+}
+
+// We're done retrying, so reactivate this ThreadContext.
+tc->activate();
+
+handleReturn(tc, retval);
+}
+
+void
+SyscallDesc::setupRetry(ThreadContext *tc)
+{
+// Create an event which will retry the system call later.
+auto retry = [this, tc]() { retrySyscall(tc); };
+auto *event = new EventFunctionWrapper(retry, name(), true);
+
+// Schedule it in about 100 CPU cycles. That will give other contexts
+// a chance to execute a bit of code before trying again.
+auto *cpu = tc->getCpuPtr();
+curEventQueue()->schedule(event,
+curTick() + cpu->cyclesToTicks(Cycles(100)));
+}
+
+void
+SyscallDesc::handleReturn(ThreadContext *tc, const SyscallReturn )
+{
+if (ret.suppressed()) {
 DPRINTF_SYSCALL(Base, "No return value.\n", name());
 } else {
-returnInto(tc, retval);
-DPRINTF_SYSCALL(Base, "Returned %d.\n", retval.encodedValue());
+returnInto(tc, ret);
+DPRINTF_SYSCALL(Base, "Returned %d.\n", ret.encodedValue());
 }
 }

diff --git a/src/sim/syscall_desc.hh b/src/sim/syscall_desc.hh
index 6ded904..e89cf49 100644
--- a/src/sim/syscall_desc.hh
+++ b/src/sim/syscall_desc.hh
@@ -95,11 +95,16 @@
 _name(name), _num(num), executor(exec), dumper(dump)
 {}

+void retrySyscall(ThreadContext *tc);
+
   private:
 /** System call name (e.g., open, mmap, clone, socket, etc.) */
 std::string _name;
 int _num;

+void setupRetry(ThreadContext *tc);
+void handleReturn(ThreadContext *tc, const SyscallReturn );
+
 /** Mechanism for ISAs to connect to the emul function definitions */
 Executor executor;
 Dumper dumper;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54205
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-2
Gerrit-Change-Id: I155fa8205d7ea45e3d102216aeca6ee1979a522f
Gerrit-Change-Number: 54205
Gerrit-PatchSet: 2
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: mem-ruby: Fix message stall time calculation

2021-12-16 Thread Daecheol You (Gerrit) via gem5-dev

Daecheol You has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54363 )



Change subject: mem-ruby: Fix message stall time calculation
..

mem-ruby: Fix message stall time calculation

Three changes below:
1. The m_stall_time was declared as statistics::Average, but
statistics::Average uses AvgStor as storage and this works as per-tick
average stat. In the case of m_stall_time, Scalar should be used to get
the calculation right.

2. The function used to get a enqueue time was changed since the getTime()
returns the time when the message was created.

3. Record the stall time only when the message is really dequeued
from the buffer (stall time is not evaluated when the message is moved
to stall map).

Change-Id: I090d19828b5c43f0843a8b735d3f00f312c436e9
---
M src/mem/ruby/network/MessageBuffer.cc
M src/mem/ruby/network/MessageBuffer.hh
2 files changed, 41 insertions(+), 5 deletions(-)



diff --git a/src/mem/ruby/network/MessageBuffer.cc  
b/src/mem/ruby/network/MessageBuffer.cc

index f4b2bb5..34f1a3c 100644
--- a/src/mem/ruby/network/MessageBuffer.cc
+++ b/src/mem/ruby/network/MessageBuffer.cc
@@ -66,10 +66,12 @@
 m_allow_zero_latency(p.allow_zero_latency),
 ADD_STAT(m_not_avail_count, "Number of times this buffer did not have "
 "N slots available"),
+ADD_STAT(m_msg_count, "Number of messages passed the buffer"),
 ADD_STAT(m_buf_msgs, "Average number of messages in buffer"),
-ADD_STAT(m_stall_time, "Average number of cycles messages are stalled  
in "

-   "this MB"),
+ADD_STAT(m_stall_time, "Total number of cycles messages are stalled  
in "

+   "this buffer"),
 ADD_STAT(m_stall_count, "Number of times messages were stalled"),
+ADD_STAT(m_avg_stall_time, "Average stall ticks per message"),
 ADD_STAT(m_occupancy, "Average occupancy of buffer capacity")
 {
 m_msg_counter = 0;
@@ -93,12 +95,18 @@
 m_not_avail_count
 .flags(statistics::nozero);

+m_msg_count
+.flags(statistics::nozero);
+
 m_buf_msgs
 .flags(statistics::nozero);

 m_stall_count
 .flags(statistics::nozero);

+m_avg_stall_time
+.flags(statistics::nozero | statistics::nonan);
+
 m_occupancy
 .flags(statistics::nozero);

@@ -110,6 +118,8 @@
 } else {
 m_occupancy = 0;
 }
+
+m_avg_stall_time = m_stall_time / m_msg_count;
 }

 unsigned int
@@ -288,8 +298,6 @@
 message->updateDelayedTicks(current_time);
 Tick delay = message->getDelayedTicks();

-m_stall_time = curTick() - message->getTime();
-
 // record previous size and time so the current buffer size isn't
 // adjusted until schd cycle
 if (m_time_last_time_pop < current_time) {
@@ -301,6 +309,10 @@
 pop_heap(m_prio_heap.begin(), m_prio_heap.end(),  
std::greater());

 m_prio_heap.pop_back();
 if (decrement_messages) {
+// Record how much time is passed since the message was enqueued
+m_stall_time += curTick() - message->getLastEnqueueTime();
+m_msg_count++;
+
 // If the message will be removed from the queue, decrement the
 // number of message in the queue.
 m_buf_msgs--;
diff --git a/src/mem/ruby/network/MessageBuffer.hh  
b/src/mem/ruby/network/MessageBuffer.hh

index ff2a4dd..7a48a1d 100644
--- a/src/mem/ruby/network/MessageBuffer.hh
+++ b/src/mem/ruby/network/MessageBuffer.hh
@@ -264,9 +264,11 @@

 // Count the # of times I didn't have N slots available
 statistics::Scalar m_not_avail_count;
+statistics::Scalar m_msg_count;
 statistics::Average m_buf_msgs;
-statistics::Average m_stall_time;
+statistics::Scalar m_stall_time;
 statistics::Scalar m_stall_count;
+statistics::Formula m_avg_stall_time;
 statistics::Formula m_occupancy;
 };


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54363
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: I090d19828b5c43f0843a8b735d3f00f312c436e9
Gerrit-Change-Number: 54363
Gerrit-PatchSet: 1
Gerrit-Owner: Daecheol You 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: mem-cache: Added stats filtering both useful and spanPage prefetch.

2021-12-16 Thread Huang Jiasen (Gerrit) via gem5-dev

Huang Jiasen has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54163 )


Change subject: mem-cache: Added stats filtering both useful and spanPage  
prefetch.

..

mem-cache: Added stats filtering both useful and spanPage prefetch.

Change-Id: I2570ee47f064ac999f2dcc813c9e39174a2ad8af
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54163
Reviewed-by: Daniel Carvalho 
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M src/mem/cache/prefetch/queued.cc
M src/mem/cache/prefetch/queued.hh
2 files changed, 22 insertions(+), 1 deletion(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
  Daniel Carvalho: Looks good to me, but someone else must approve
  kokoro: Regressions pass




diff --git a/src/mem/cache/prefetch/queued.cc  
b/src/mem/cache/prefetch/queued.cc

index 597c88a..da9cbf4 100644
--- a/src/mem/cache/prefetch/queued.cc
+++ b/src/mem/cache/prefetch/queued.cc
@@ -210,6 +210,10 @@

 if (!samePage(addr_prio.first, pfi.getAddr())) {
 statsQueued.pfSpanPage += 1;
+
+if (hasBeenPrefetched(pkt->getAddr(), pkt->isSecure())) {
+statsQueued.pfUsefulSpanPage += 1;
+}
 }

 bool can_cross_page = (tlb != nullptr);
@@ -272,7 +276,9 @@
 ADD_STAT(pfRemovedFull, statistics::units::Count::get(),
  "number of prefetches dropped due to prefetch queue size"),
 ADD_STAT(pfSpanPage, statistics::units::Count::get(),
- "number of prefetches that crossed the page")
+ "number of prefetches that crossed the page"),
+ADD_STAT(pfUsefulSpanPage, statistics::units::Count::get(),
+ "number of prefetches that is useful and crossed the page")
 {
 }

diff --git a/src/mem/cache/prefetch/queued.hh  
b/src/mem/cache/prefetch/queued.hh

index 1062630..c769b38 100644
--- a/src/mem/cache/prefetch/queued.hh
+++ b/src/mem/cache/prefetch/queued.hh
@@ -185,6 +185,7 @@
 statistics::Scalar pfRemovedDemand;
 statistics::Scalar pfRemovedFull;
 statistics::Scalar pfSpanPage;
+statistics::Scalar pfUsefulSpanPage;
 } statsQueued;
   public:
 using AddrPriority = std::pair;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54163
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: I2570ee47f064ac999f2dcc813c9e39174a2ad8af
Gerrit-Change-Number: 54163
Gerrit-PatchSet: 7
Gerrit-Owner: Huang Jiasen 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Huang Jiasen 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Nikos Nikoleris 
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[release-staging-v21-2]: ext: Fix dumping stats error at the end of simulation

2021-12-16 Thread Hoa Nguyen (Gerrit) via gem5-dev

Hoa Nguyen has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54343 )



Change subject: ext: Fix dumping stats error at the end of simulation
..

ext: Fix dumping stats error at the end of simulation

A missing comma causing two commands to be one, which resulted in
a Python interpreter syntax error.

Change-Id: Ieb0e3c620c175731084ff0e2040388b15364691e
Signed-off-by: Hoa Nguyen 
---
M ext/sst/gem5.cc
1 file changed, 14 insertions(+), 1 deletion(-)



diff --git a/ext/sst/gem5.cc b/ext/sst/gem5.cc
index 8eab6281..7af0eed 100644
--- a/ext/sst/gem5.cc
+++ b/ext/sst/gem5.cc
@@ -262,7 +262,7 @@
 );
 // output gem5 stats
 const std::vector output_stats_commands = {
-"import m5.stats"
+"import m5.stats",
 "m5.stats.dump()"
 };
 execPythonCommands(output_stats_commands);

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54343
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-2
Gerrit-Change-Id: Ieb0e3c620c175731084ff0e2040388b15364691e
Gerrit-Change-Number: 54343
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] Re: Broken SST due to python changes

2021-12-16 Thread Jason Lowe-Power via gem5-dev

Thanks, Gabe! We'll run the nightly tests and double check everything is
working as expected.

Jason

On Thu, Dec 16, 2021 at 6:49 AM Gabe Black  wrote:

> I just realized I turned off my computer since I'll be traveling, and my
> test run of sst can't finish if the computer is off :-P. I have not
> completed a run of your example command, but it starts and I have no reason
> to believe it won't finish. I would still suggest running it yourself (with
> my changes) to verify that things are fixed. I need to wake up to go to the
> airport in 4 hours, so I won't be able to do anything non-trivial with this
> until Friday at the earliest.
>
> Gabe
>
> On Thu, Dec 16, 2021 at 3:29 AM Gabe Black  wrote:
>
>> https://gem5-review.googlesource.com/c/public/gem5/+/54325/1
>>
>> On Wed, Dec 15, 2021 at 11:12 PM Gabe Black  wrote:
>>
>>> There will be a couple more patches coming, since the systemc subsystem
>>> has problems when python is already running when static initializers run. I
>>> had been building without systemc enabled to simplify things earlier, but I
>>> need to get it running too. That should be a fairly quick fix.
>>>
>>> Gabe
>>>
>>> On Wed, Dec 15, 2021 at 10:40 PM Hoa Nguyen 
>>> wrote:
>>>
 Hi Gabe,

 Thanks for uploading the changes. Typically it'll take less than an
 hour for RISC-V.

 Regards,
 Hoa Nguyen

 On Wed, Dec 15, 2021 at 10:38 PM Gabe Black 
 wrote:

> I have it running, although it's taking a while. How long should it
> take to finish? Should it finish immediately, 15 minutes, a few hours,
> tomorrow...? I'll upload my changes in the mean time.
>
> Gabe
>
> On Wed, Dec 15, 2021 at 9:13 PM Gabe Black 
> wrote:
>
>> Ok, I did/am doing a little more looking, and part of the problem
>> seems to be that some of the python blobs executed with executePython
>> assume prior blobs have done imports for them already. That was where the
>> modules or their contents not being found was coming from, and then after
>> that something about the error reporting was blowing up, but that wasn't
>> the primary problem.
>>
>> With that fixed, it's still upset, but now it's because "fatal: Need
>> to instantiate Root() before calling instantiate()". Also, gem5 is 
>> printing
>> the help blob which seems wrong, so maybe the command line arguments 
>> aren't
>> making it through like they should? It could be that no config script is
>> run, and that's why m5.instantiate() doesn't work.
>>
>> It's also a little weird that there's an ordering dependency between
>> the execPythonCommands invocations, considering the same set of those
>> should still be happening in the same order. I suppose I'm not explicitly
>> importing the m5 module into __main__.__dict__ to run m5.main(), so that
>> may be where that's coming from.
>>
>> Anyway, I'm still chopping away at this, but I think it's pretty
>> close.
>>
>> Gabe
>>
>> On Wed, Dec 15, 2021 at 8:29 PM Gabe Black 
>> wrote:
>>
>>> Also, I have to say, while I'm grateful that there was a push to get
>>> the SCons changes reviewed for the release, we wouldn't be in this 
>>> crunch
>>> if they had been reviewed a month ago. It would be best not to let 
>>> things
>>> bunch up and then make a herculean push right at the deadline, where
>>> problems have a very limited time to get resolved.
>>>
>>> Gabe
>>>
>>> On Wed, Dec 15, 2021 at 4:38 PM Gabe Black 
>>> wrote:
>>>
 That's actually very helpful, since I was pretty sure there was a
 memory problem but couldn't figure out where. I think there's a good 
 chance
 this will be an easy fix.

 Gabe

 On Wed, Dec 15, 2021, 8:59 AM Jason Lowe-Power 
 wrote:

> Ok, I was able to get a debugger to work and I dug in a bit more...
>
> The problem is that the new code does not initialize the
> __main__.py module. The original SST code had the following line:
> `pythonMain = PyImport_AddModule(PyCC("__main__"));`
>
> This was executed before `execPythonCommands`.
>
> In `execPythonCommands` we use the `pythonMain` variable. However,
> in the *new* code, that variable is NULL (or unitialized, actually).
>
> As far as I can tell, all of the new python changes didn't come
> with any documentation on how you are supposed to interact with 
> Python from
> gem5. So, there's no good way for us to update this code.
>
> Gabe, if you can update the `execPythonCommands` function to work
> however you expected it to, that would be great.
>
> Thanks,
> Jason
>
> On Wed, Dec 15, 2021 at 10:45 AM Jason Lowe-Power <
>

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: gdb support Thumb-2 ISA

2021-12-16 Thread Yu-hsin Wang (Gerrit) via gem5-dev

Yu-hsin Wang has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54124 )


Change subject: arch-arm: gdb support Thumb-2 ISA
..

arch-arm: gdb support Thumb-2 ISA

From the document*1, we should allow 2,3,4 in kind check function for
supporting all kinds of ARM breakpoint.

1.  
https://sourceware.org/gdb/current/onlinedocs/gdb/ARM-Breakpoint-Kinds.html


Change-Id: I82bcb88cfe6e80e7f17cd6bb68a26a45ace7b174
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54124
Reviewed-by: Giacomo Travaglini 
Maintainer: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/arch/arm/remote_gdb.cc
1 file changed, 40 insertions(+), 2 deletions(-)

Approvals:
  Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/arch/arm/remote_gdb.cc b/src/arch/arm/remote_gdb.cc
index 215c532..6e8923e 100644
--- a/src/arch/arm/remote_gdb.cc
+++ b/src/arch/arm/remote_gdb.cc
@@ -168,6 +168,20 @@

 using namespace ArmISA;

+namespace
+{
+
+//  
https://sourceware.org/gdb/current/onlinedocs/gdb/ARM-Breakpoint-Kinds.html

+enum class ArmBpKind
+{
+THUMB = 2,
+THUMB_2 = 3,
+ARM = 4,
+};
+
+} // namespace
+
+
 static bool
 tryTranslate(ThreadContext *tc, Addr addr)
 {
@@ -364,8 +378,14 @@
 bool
 RemoteGDB::checkBpKind(size_t kind)
 {
-// 2 for Thumb ISA, 4 for ARM ISA.
-return kind == 2 || kind == 4;
+switch (ArmBpKind(kind)) {
+  case ArmBpKind::THUMB:
+  case ArmBpKind::THUMB_2:
+  case ArmBpKind::ARM:
+return true;
+  default:
+return false;
+}
 }

 } // namespace gem5

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54124
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: I82bcb88cfe6e80e7f17cd6bb68a26a45ace7b174
Gerrit-Change-Number: 54124
Gerrit-PatchSet: 5
Gerrit-Owner: Yu-hsin Wang 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Earl Ou 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Philip Metzler 
Gerrit-Reviewer: Yu-hsin Wang 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: base: Correct checkBpLen naming with checkBpKind

2021-12-16 Thread Yu-hsin Wang (Gerrit) via gem5-dev

Yu-hsin Wang has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54123 )


Change subject: base: Correct checkBpLen naming with checkBpKind
..

base: Correct checkBpLen naming with checkBpKind

In gdb document*1, the second parameter of checkpoint command(Z0, Z1) is
named after kind. Although underlying implementation probably considers
it as length*2, it's still good to follow the name described in gdb
document for avoiding any confusion.

Refs:
1. https://sourceware.org/gdb/onlinedocs/gdb/Packets.html
2. https://github.com/bminor/binutils-gdb/blob/master/gdb/arch-utils.h#L41

Change-Id: Ib4b585613b8018970b16355f96cdff2ce9d5bae6
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54123
Reviewed-by: Daniel Carvalho 
Maintainer: Daniel Carvalho 
Tested-by: kokoro 
Reviewed-by: Giacomo Travaglini 
---
M src/arch/riscv/remote_gdb.hh
M src/arch/arm/remote_gdb.cc
M src/arch/arm/remote_gdb.hh
M src/arch/x86/remote_gdb.hh
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
6 files changed, 61 insertions(+), 36 deletions(-)

Approvals:
  Giacomo Travaglini: Looks good to me, approved
  Daniel Carvalho: Looks good to me, but someone else must approve; Looks  
good to me, approved

  kokoro: Regressions pass




diff --git a/src/arch/arm/remote_gdb.cc b/src/arch/arm/remote_gdb.cc
index 2efa82f..215c532 100644
--- a/src/arch/arm/remote_gdb.cc
+++ b/src/arch/arm/remote_gdb.cc
@@ -362,10 +362,10 @@
 }

 bool
-RemoteGDB::checkBpLen(size_t len)
+RemoteGDB::checkBpKind(size_t kind)
 {
 // 2 for Thumb ISA, 4 for ARM ISA.
-return len == 2 || len == 4;
+return kind == 2 || kind == 4;
 }

 } // namespace gem5
diff --git a/src/arch/arm/remote_gdb.hh b/src/arch/arm/remote_gdb.hh
index cff6d4a..8e512a4 100644
--- a/src/arch/arm/remote_gdb.hh
+++ b/src/arch/arm/remote_gdb.hh
@@ -120,7 +120,7 @@
   public:
 RemoteGDB(System *_system, int _port);
 BaseGdbRegCache *gdbRegs() override;
-bool checkBpLen(size_t len) override;
+bool checkBpKind(size_t kind) override;
 std::vector
 availableFeatures() const override
 {
diff --git a/src/arch/riscv/remote_gdb.hh b/src/arch/riscv/remote_gdb.hh
index 40fe821..753859f 100644
--- a/src/arch/riscv/remote_gdb.hh
+++ b/src/arch/riscv/remote_gdb.hh
@@ -56,7 +56,7 @@

 bool acc(Addr addr, size_t len) override;
 // A breakpoint will be 2 bytes if it is compressed and 4 if not
-bool checkBpLen(size_t len) override { return len == 2 || len == 4; }
+bool checkBpKind(size_t kind) override { return kind == 2 || kind ==  
4; }


 class RiscvGdbRegCache : public BaseGdbRegCache
 {
diff --git a/src/arch/x86/remote_gdb.hh b/src/arch/x86/remote_gdb.hh
index 62176a5..dfa9177 100644
--- a/src/arch/x86/remote_gdb.hh
+++ b/src/arch/x86/remote_gdb.hh
@@ -58,7 +58,7 @@
 {
   protected:
 bool acc(Addr addr, size_t len);
-bool checkBpLen(size_t len) { return len == 1; }
+bool checkBpKind(size_t kind) { return kind == 1; }
 class X86GdbRegCache : public BaseGdbRegCache
 {
   using BaseGdbRegCache::BaseGdbRegCache;
diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 1437b75..798d09f 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -784,28 +784,28 @@
 }

 void
-BaseRemoteGDB::insertSoftBreak(Addr addr, size_t len)
+BaseRemoteGDB::insertSoftBreak(Addr addr, size_t kind)
 {
-if (!checkBpLen(len))
-throw BadClient("Invalid breakpoint length\n");
+if (!checkBpKind(kind))
+throw BadClient("Invalid breakpoint kind.\n");

-return insertHardBreak(addr, len);
+return insertHardBreak(addr, kind);
 }

 void
-BaseRemoteGDB::removeSoftBreak(Addr addr, size_t len)
+BaseRemoteGDB::removeSoftBreak(Addr addr, size_t kind)
 {
-if (!checkBpLen(len))
-throw BadClient("Invalid breakpoint length.\n");
+if (!checkBpKind(kind))
+throw BadClient("Invalid breakpoint kind.\n");

-return removeHardBreak(addr, len);
+return removeHardBreak(addr, kind);
 }

 void
-BaseRemoteGDB::insertHardBreak(Addr addr, size_t len)
+BaseRemoteGDB::insertHardBreak(Addr addr, size_t kind)
 {
-if (!checkBpLen(len))
-throw BadClient("Invalid breakpoint length\n");
+if (!checkBpKind(kind))
+throw BadClient("Invalid breakpoint kind.\n");

 DPRINTF(GDBMisc, "Inserting hardware breakpoint at %#x\n", addr);

@@ -817,10 +817,10 @@
 }

 void
-BaseRemoteGDB::removeHardBreak(Addr addr, size_t len)
+BaseRemoteGDB::removeHardBreak(Addr addr, size_t kind)
 {
-if (!checkBpLen(len))
-throw BadClient("Invalid breakpoint length\n");
+if (!checkBpKind(kind))
+throw BadClient("Invalid breakpoint kind.\n");

 DPRINTF(GDBMisc, "Removing hardware breakpoint at %#x\n", addr);

@@ -917,7 +917,7 @@
 };

 bool
-BaseRemoteGDB::checkBpLen(size_t len)
+BaseRemoteGDB::checkBpKind(size_t kind)
 {
 return true;
 }
@@ -1302,17 +1302,17 @@

[gem5-dev] Re: Broken SST due to python changes

2021-12-16 Thread Gabe Black via gem5-dev

I just realized I turned off my computer since I'll be traveling, and my
test run of sst can't finish if the computer is off :-P. I have not
completed a run of your example command, but it starts and I have no reason
to believe it won't finish. I would still suggest running it yourself (with
my changes) to verify that things are fixed. I need to wake up to go to the
airport in 4 hours, so I won't be able to do anything non-trivial with this
until Friday at the earliest.

Gabe

On Thu, Dec 16, 2021 at 3:29 AM Gabe Black  wrote:

> https://gem5-review.googlesource.com/c/public/gem5/+/54325/1
>
> On Wed, Dec 15, 2021 at 11:12 PM Gabe Black  wrote:
>
>> There will be a couple more patches coming, since the systemc subsystem
>> has problems when python is already running when static initializers run. I
>> had been building without systemc enabled to simplify things earlier, but I
>> need to get it running too. That should be a fairly quick fix.
>>
>> Gabe
>>
>> On Wed, Dec 15, 2021 at 10:40 PM Hoa Nguyen 
>> wrote:
>>
>>> Hi Gabe,
>>>
>>> Thanks for uploading the changes. Typically it'll take less than an hour
>>> for RISC-V.
>>>
>>> Regards,
>>> Hoa Nguyen
>>>
>>> On Wed, Dec 15, 2021 at 10:38 PM Gabe Black 
>>> wrote:
>>>
 I have it running, although it's taking a while. How long should it
 take to finish? Should it finish immediately, 15 minutes, a few hours,
 tomorrow...? I'll upload my changes in the mean time.

 Gabe

 On Wed, Dec 15, 2021 at 9:13 PM Gabe Black 
 wrote:

> Ok, I did/am doing a little more looking, and part of the problem
> seems to be that some of the python blobs executed with executePython
> assume prior blobs have done imports for them already. That was where the
> modules or their contents not being found was coming from, and then after
> that something about the error reporting was blowing up, but that wasn't
> the primary problem.
>
> With that fixed, it's still upset, but now it's because "fatal: Need
> to instantiate Root() before calling instantiate()". Also, gem5 is 
> printing
> the help blob which seems wrong, so maybe the command line arguments 
> aren't
> making it through like they should? It could be that no config script is
> run, and that's why m5.instantiate() doesn't work.
>
> It's also a little weird that there's an ordering dependency between
> the execPythonCommands invocations, considering the same set of those
> should still be happening in the same order. I suppose I'm not explicitly
> importing the m5 module into __main__.__dict__ to run m5.main(), so that
> may be where that's coming from.
>
> Anyway, I'm still chopping away at this, but I think it's pretty close.
>
> Gabe
>
> On Wed, Dec 15, 2021 at 8:29 PM Gabe Black 
> wrote:
>
>> Also, I have to say, while I'm grateful that there was a push to get
>> the SCons changes reviewed for the release, we wouldn't be in this crunch
>> if they had been reviewed a month ago. It would be best not to let things
>> bunch up and then make a herculean push right at the deadline, where
>> problems have a very limited time to get resolved.
>>
>> Gabe
>>
>> On Wed, Dec 15, 2021 at 4:38 PM Gabe Black 
>> wrote:
>>
>>> That's actually very helpful, since I was pretty sure there was a
>>> memory problem but couldn't figure out where. I think there's a good 
>>> chance
>>> this will be an easy fix.
>>>
>>> Gabe
>>>
>>> On Wed, Dec 15, 2021, 8:59 AM Jason Lowe-Power 
>>> wrote:
>>>
 Ok, I was able to get a debugger to work and I dug in a bit more...

 The problem is that the new code does not initialize the
 __main__.py module. The original SST code had the following line:
 `pythonMain = PyImport_AddModule(PyCC("__main__"));`

 This was executed before `execPythonCommands`.

 In `execPythonCommands` we use the `pythonMain` variable. However,
 in the *new* code, that variable is NULL (or unitialized, actually).

 As far as I can tell, all of the new python changes didn't come
 with any documentation on how you are supposed to interact with Python 
 from
 gem5. So, there's no good way for us to update this code.

 Gabe, if you can update the `execPythonCommands` function to work
 however you expected it to, that would be great.

 Thanks,
 Jason

 On Wed, Dec 15, 2021 at 10:45 AM Jason Lowe-Power <
 ja...@lowepower.com> wrote:

> Here's my understanding of where we are on this:
>
> As of
> https://gem5.googlesource.com/public/gem5/+/cf7ce21848ea4aeee28737823e6e768f9a14ceaf
> SST was working. (Committed Dec. 7th)
>
> Then, this large relation chain was

[gem5-dev] Re: Broken SST due to python changes

2021-12-16 Thread Gabe Black via gem5-dev

https://gem5-review.googlesource.com/c/public/gem5/+/54325/1

On Wed, Dec 15, 2021 at 11:12 PM Gabe Black  wrote:

> There will be a couple more patches coming, since the systemc subsystem
> has problems when python is already running when static initializers run. I
> had been building without systemc enabled to simplify things earlier, but I
> need to get it running too. That should be a fairly quick fix.
>
> Gabe
>
> On Wed, Dec 15, 2021 at 10:40 PM Hoa Nguyen  wrote:
>
>> Hi Gabe,
>>
>> Thanks for uploading the changes. Typically it'll take less than an hour
>> for RISC-V.
>>
>> Regards,
>> Hoa Nguyen
>>
>> On Wed, Dec 15, 2021 at 10:38 PM Gabe Black  wrote:
>>
>>> I have it running, although it's taking a while. How long should it take
>>> to finish? Should it finish immediately, 15 minutes, a few hours,
>>> tomorrow...? I'll upload my changes in the mean time.
>>>
>>> Gabe
>>>
>>> On Wed, Dec 15, 2021 at 9:13 PM Gabe Black  wrote:
>>>
 Ok, I did/am doing a little more looking, and part of the problem seems
 to be that some of the python blobs executed with executePython assume
 prior blobs have done imports for them already. That was where the modules
 or their contents not being found was coming from, and then after that
 something about the error reporting was blowing up, but that wasn't the
 primary problem.

 With that fixed, it's still upset, but now it's because "fatal: Need to
 instantiate Root() before calling instantiate()". Also, gem5 is printing
 the help blob which seems wrong, so maybe the command line arguments aren't
 making it through like they should? It could be that no config script is
 run, and that's why m5.instantiate() doesn't work.

 It's also a little weird that there's an ordering dependency between
 the execPythonCommands invocations, considering the same set of those
 should still be happening in the same order. I suppose I'm not explicitly
 importing the m5 module into __main__.__dict__ to run m5.main(), so that
 may be where that's coming from.

 Anyway, I'm still chopping away at this, but I think it's pretty close.

 Gabe

 On Wed, Dec 15, 2021 at 8:29 PM Gabe Black 
 wrote:

> Also, I have to say, while I'm grateful that there was a push to get
> the SCons changes reviewed for the release, we wouldn't be in this crunch
> if they had been reviewed a month ago. It would be best not to let things
> bunch up and then make a herculean push right at the deadline, where
> problems have a very limited time to get resolved.
>
> Gabe
>
> On Wed, Dec 15, 2021 at 4:38 PM Gabe Black 
> wrote:
>
>> That's actually very helpful, since I was pretty sure there was a
>> memory problem but couldn't figure out where. I think there's a good 
>> chance
>> this will be an easy fix.
>>
>> Gabe
>>
>> On Wed, Dec 15, 2021, 8:59 AM Jason Lowe-Power 
>> wrote:
>>
>>> Ok, I was able to get a debugger to work and I dug in a bit more...
>>>
>>> The problem is that the new code does not initialize the __main__.py
>>> module. The original SST code had the following line:
>>> `pythonMain = PyImport_AddModule(PyCC("__main__"));`
>>>
>>> This was executed before `execPythonCommands`.
>>>
>>> In `execPythonCommands` we use the `pythonMain` variable. However,
>>> in the *new* code, that variable is NULL (or unitialized, actually).
>>>
>>> As far as I can tell, all of the new python changes didn't come with
>>> any documentation on how you are supposed to interact with Python from
>>> gem5. So, there's no good way for us to update this code.
>>>
>>> Gabe, if you can update the `execPythonCommands` function to work
>>> however you expected it to, that would be great.
>>>
>>> Thanks,
>>> Jason
>>>
>>> On Wed, Dec 15, 2021 at 10:45 AM Jason Lowe-Power <
>>> ja...@lowepower.com> wrote:
>>>
 Here's my understanding of where we are on this:

 As of
 https://gem5.googlesource.com/public/gem5/+/cf7ce21848ea4aeee28737823e6e768f9a14ceaf
 SST was working. (Committed Dec. 7th)

 Then, this large relation chain was pushed:
 https://gem5-review.googlesource.com/c/public/gem5/+/49425 ending
 with
 https://gem5.googlesource.com/public/gem5/+/4c1422e3ba780a5f152426e214b4be3d45b751e4.
 This relation chain broke SST. This was committed on Dec. 8th.

 We have two options: Either we fix this bug ASAP (as in within 6
 days so we can release on 12/22) or we revert the changesets that were
 committed on the 8th.

 SST is a *must have* feature of gem5 21.2. It is quite frustrating
 to see it broken after all of the hard work that Hoa and Giacomo put 
 into
 it.

 Let me know if

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: systemc: Eliminate the unused PythonReadyFunc mechanism.

2021-12-16 Thread Gabe Black (Gerrit) via gem5-dev

Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54323 )



Change subject: systemc: Eliminate the unused PythonReadyFunc mechanism.
..

systemc: Eliminate the unused PythonReadyFunc mechanism.

Change-Id: I8892e4d209901454f2ab923aa3fa9932d7963274
---
M src/systemc/core/SystemC.py
M src/systemc/core/python.cc
M src/systemc/core/python.hh
3 files changed, 9 insertions(+), 37 deletions(-)



diff --git a/src/systemc/core/SystemC.py b/src/systemc/core/SystemC.py
index 6828175..592b950 100644
--- a/src/systemc/core/SystemC.py
+++ b/src/systemc/core/SystemC.py
@@ -63,11 +63,3 @@
 @cxxMethod(return_value_policy="reference", cxx_name="gem5_getPort")
 def getPort(self, if_name, iex):
 return None
-
-try:
-import _m5
-except:
-pass
-else:
-import _m5.systemc
-_m5.systemc.python_ready()
diff --git a/src/systemc/core/python.cc b/src/systemc/core/python.cc
index 8f2d56b..2283ae7 100644
--- a/src/systemc/core/python.cc
+++ b/src/systemc/core/python.cc
@@ -38,13 +38,6 @@
 namespace
 {

-PythonReadyFunc *&
-firstReadyFunc()
-{
-static PythonReadyFunc *first = nullptr;
-return first;
-}
-
 PythonInitFunc *&
 firstInitFunc()
 {
@@ -53,17 +46,9 @@
 }

 void
-python_ready(pybind11::args args)
-{
-for (auto ptr = firstReadyFunc(); ptr; ptr = ptr->next)
-ptr->run();
-}
-
-void
 systemc_pybind(pybind11::module_ _internal)
 {
 pybind11::module_ m = m_internal.def_submodule("systemc");
-m.def("python_ready", _ready);
 for (auto ptr = firstInitFunc(); ptr; ptr = ptr->next)
 ptr->run(m);
 }
@@ -71,11 +56,6 @@

 } // anonymous namespace

-PythonReadyFunc::PythonReadyFunc() : next(firstReadyFunc())
-{
-firstReadyFunc() = this;
-}
-
 PythonInitFunc::PythonInitFunc() : next(firstInitFunc())
 {
 firstInitFunc() = this;
diff --git a/src/systemc/core/python.hh b/src/systemc/core/python.hh
index 61c9c80..5a3f6ef 100644
--- a/src/systemc/core/python.hh
+++ b/src/systemc/core/python.hh
@@ -33,15 +33,6 @@
 namespace sc_gem5
 {

-struct PythonReadyFunc
-{
-PythonReadyFunc *next;
-
-PythonReadyFunc();
-~PythonReadyFunc() {}
-virtual void run() = 0;
-};
-
 struct PythonInitFunc
 {
 PythonInitFunc *next;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54323
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-2
Gerrit-Change-Id: I8892e4d209901454f2ab923aa3fa9932d7963274
Gerrit-Change-Number: 54323
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[release-staging-v21-2]: systemc: Update the testing framework to get it working again.

2021-12-16 Thread Gabe Black (Gerrit) via gem5-dev

Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54324 )



Change subject: systemc: Update the testing framework to get it working  
again.

..

systemc: Update the testing framework to get it working again.

The systemc testing framework is not used regularly, and had bit rot and
stopped working. This change updates it so that it runs again, and all
previously passing tests pass again.

These changes were mostly in the related SConscript now that top level
targets are built a little differently and that the gem5 shared library
is no longer stored in a special construction environment variable.
verify.py also needed to be updated since warn() and info() lines now
have file and line number information in them, throwing off pre diff
filtering of gem5 outputs.

Change-Id: Ifdcbd92eab8b9b2168c449bfbcebf52dbe1f016a
---
M src/systemc/tests/verify.py
M src/systemc/tests/SConscript
2 files changed, 31 insertions(+), 20 deletions(-)



diff --git a/src/systemc/tests/SConscript b/src/systemc/tests/SConscript
index 7d544f2..fb916d2 100644
--- a/src/systemc/tests/SConscript
+++ b/src/systemc/tests/SConscript
@@ -63,7 +63,8 @@
 test_dir = Dir('.')
 class SystemCTestBin(Executable):
 def __init__(self, test):
-super().__init__(test.target, *test.sources)
+all_sources = test.sources + [with_tag('main')]
+super().__init__(test.target, *all_sources)
 self.reldir = test.reldir
 self.test_deps = test.deps

@@ -78,26 +79,16 @@

 env.Append(CPPPATH=test_dir.Dir('include'))

-shared_lib_path = env['SHARED_LIB'][0].abspath
-sl_dir, sl_base = os.path.split(shared_lib_path)
-env.Append(LIBPATH=[sl_dir], LIBS=[sl_base])
+env.Append(LIBPATH=['${BUILDDIR}'], LIBS=['gem5_${ENV_LABEL}'])
+env.AddLocalRPATH('${BUILDDIR}')
+
+env['OBJSUFFIX'] = '.sc' + env['OBJSUFFIX'][1:]
+env['SHOBJSUFFIX'] = '.sc' + env['OBJSUFFIX'][1:]

 super().declare_all(env)

 def declare(self, env):
-env = env.Clone()
-sources = list(self.sources)
-for f in self.filters:
-sources += Source.all.apply_filter(f)
-objs = self.srcs_to_objs(env, sources)
-objs = objs + env['MAIN_OBJS']
-relpath = os.path.relpath(
-env['SHARED_LIB'][0].dir.abspath,
-self.path(env).dir.abspath)
-env.Append(LINKFLAGS=Split('-z origin'))
-env.Append(RPATH=[
-env.Literal(os.path.join('\\$$ORIGIN', relpath))])
-test_bin = super().declare(env, objs)
+test_bin, _u = super().declare(env)
 test_dir = self.dir.Dir(self.reldir)
 for dep in self.test_deps:
 env.Depends(test_bin, test_dir.File(dep))
diff --git a/src/systemc/tests/verify.py b/src/systemc/tests/verify.py
index 54b4633..818855a 100755
--- a/src/systemc/tests/verify.py
+++ b/src/systemc/tests/verify.py
@@ -277,9 +277,9 @@
 test_filt = merge_filts(
 r'^/.*:\d+: ',
 r'^Global frequency set at \d* ticks per second\n',
-r'info: Entering event queue @ \d*\.  Starting simulation\.\.\.\n',
-r'warn: Ignoring request to set stack size\.\n',
-r'^warn: No dot file generated. Please install pydot ' +
+r'.*info: Entering event queue @ \d*\.  Starting  
simulation\.\.\.\n',

+r'.*warn: Ignoring request to set stack size\.\n',
+r'^.*warn: No dot file generated. Please install pydot ' +
 r'to generate the dot file and pdf.\n',
 info_filt(804),
 in_file_filt,

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54324
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-2
Gerrit-Change-Id: Ifdcbd92eab8b9b2168c449bfbcebf52dbe1f016a
Gerrit-Change-Number: 54324
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[release-staging-v21-2]: systemc: Change how python initialization callbacks are handled.

2021-12-16 Thread Gabe Black (Gerrit) via gem5-dev

Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54325 )



Change subject: systemc: Change how python initialization callbacks are  
handled.

..

systemc: Change how python initialization callbacks are handled.

Because the python environment may already be up and running by the time
static initializers are run, specifically when gem5 is built as a
library and loaded with dlopen, we can't rely on all of the objects
declaring python initialization callbacks having been constructed by the
time the code which would execute them runs.

To address that problem, this change keeps track of whether the
initialization has already happened when a callback is installed, and if
so, runs the callback immediately.

The original implementation also had users install callbacks by
overriding a virtual function in the PythonInitFunc class, and then
statically allocating an instance of that subclass so its constructor
would be called at initialization time. Calling the function manually if
initialization has already happened won't work in that case, because you
can't call a virtual function from a constructor and get the behavior
you'd want.

Instead, this change makes the PythonInitFunc wrap the actual callback
which is outside of the structure itself. Because the callback is not a
virtual function of PythonInitFunc, we can call it in the constructor
without issue.

Also, the Callback type has to be a bare function pointer and not a
std::function<...> because the argument it takes is a pybind11::module_
reference. Pybind11 sets the visibility of all of its code to hidden to
improve binary size, but unfortunately that causes problems when
accepting one as an argument in a publically accessible lambda in g++.
clang doesn't raise a warning, but g++ does which breaks the build. We
could potentially disable this warning, but accepting a function pointer
instead works just as well, since captureless lambdas can be trivially
converted into function pointers, and they don't seem to upset g++.

Change-Id: I3fb321b577090df67c7be3be0e677c2c2055d446
---
M src/systemc/core/sc_time_python.cc
M src/systemc/core/sc_main_python.cc
M src/systemc/core/python.cc
M src/systemc/core/python.hh
M src/systemc/tlm_core/2/quantum/global_quantum_python.cc
5 files changed, 111 insertions(+), 71 deletions(-)



diff --git a/src/systemc/core/python.cc b/src/systemc/core/python.cc
index 2283ae7..472a050 100644
--- a/src/systemc/core/python.cc
+++ b/src/systemc/core/python.cc
@@ -45,20 +45,31 @@
 return first;
 }

+bool python_initialized = false;
+
 void
 systemc_pybind(pybind11::module_ _internal)
 {
 pybind11::module_ m = m_internal.def_submodule("systemc");
 for (auto ptr = firstInitFunc(); ptr; ptr = ptr->next)
-ptr->run(m);
+ptr->callback(m);
+
+python_initialized = true;
 }
 gem5::EmbeddedPyBind embed_("systemc", _pybind);

 } // anonymous namespace

-PythonInitFunc::PythonInitFunc() : next(firstInitFunc())
+PythonInitFunc::PythonInitFunc(Callback run) :
+callback(run), next(firstInitFunc())
 {
 firstInitFunc() = this;
+
+// If the python was already initialized, run the callback immediately.
+if (python_initialized) {
+auto systemc_module = pybind11::module_::import("_m5.systemc");
+callback(systemc_module);
+}
 }

 } // namespace sc_gem5
diff --git a/src/systemc/core/python.hh b/src/systemc/core/python.hh
index 5a3f6ef..3c563db 100644
--- a/src/systemc/core/python.hh
+++ b/src/systemc/core/python.hh
@@ -35,11 +35,12 @@

 struct PythonInitFunc
 {
+using Callback = void(*)(pybind11::module_ );
+Callback callback;
+
 PythonInitFunc *next;

-PythonInitFunc();
-~PythonInitFunc() {}
-virtual void run(pybind11::module_ ) = 0;
+PythonInitFunc(Callback run);
 };

 } // namespace sc_gem5
diff --git a/src/systemc/core/sc_main_python.cc  
b/src/systemc/core/sc_main_python.cc

index 1697efe..8d2542e 100644
--- a/src/systemc/core/sc_main_python.cc
+++ b/src/systemc/core/sc_main_python.cc
@@ -92,15 +92,10 @@
 // Make our sc_main wrapper available in the internal _m5 python module  
under

 // the systemc submodule.

-struct InstallScMain : public ::sc_gem5::PythonInitFunc
-{
-void
-run(pybind11::module_ ) override
-{
-systemc.def("sc_main", _main);
-systemc.def("sc_main_result_code", _main_result_code);
-systemc.def("sc_main_result_str", _main_result_str);
-}
-} installScMain;
+::sc_gem5::PythonInitFunc installScMain([](pybind11::module_ ) {
+systemc.def("sc_main", _main);
+systemc.def("sc_main_result_code", _main_result_code);
+systemc.def("sc_main_result_str", _main_result_str);
+});

 } // anonymous namespace
diff --git a/src/systemc/core/sc_time_python.cc  
b/src/systemc/core/sc_time_python.cc

index 58fa65f..be383bc 100644
--- a/src/systemc/core/sc_time_python.cc
+++

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: configs: Remove unused WalkCache models

2021-12-16 Thread Giacomo Travaglini (Gerrit) via gem5-dev

Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54244 )


Change subject: configs: Remove unused WalkCache models
..

configs: Remove unused WalkCache models

Change-Id: Iebda966e72b484ee15cbc7cd62256a950b2a90f1
Signed-off-by: Giacomo Travaglini 
Reviewed-by: Richard Cooper 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54244
Reviewed-by: Andreas Sandberg 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M configs/common/cores/arm/ex5_big.py
M configs/common/cores/arm/HPI.py
M configs/common/cores/arm/ex5_LITTLE.py
M configs/common/cores/arm/O3_ARM_v7a.py
M configs/example/arm/devices.py
5 files changed, 16 insertions(+), 67 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved
  Richard Cooper: Looks good to me, approved
  Jason Lowe-Power: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/configs/common/cores/arm/HPI.py  
b/configs/common/cores/arm/HPI.py

index 624c40c..620c01e 100644
--- a/configs/common/cores/arm/HPI.py
+++ b/configs/common/cores/arm/HPI.py
@@ -1332,16 +1332,6 @@
 itb = ArmTLB(entry_type="instruction", size=256)
 dtb = ArmTLB(entry_type="data", size=256)

-class HPI_WalkCache(Cache):
-data_latency = 4
-tag_latency = 4
-response_latency = 4
-mshrs = 6
-tgts_per_mshr = 8
-size = '1kB'
-assoc = 8
-write_buffers = 16
-
 class HPI_BP(TournamentBP):
 localPredictorSize = 64
 localCtrBits = 2
@@ -1442,7 +1432,7 @@

 __all__ = [
 "HPI_BP",
-"HPI_ITB", "HPI_DTB", "HPI_WalkCache",
+"HPI_ITB", "HPI_DTB",
 "HPI_ICache", "HPI_DCache", "HPI_L2",
 "HPI",
 ]
diff --git a/configs/common/cores/arm/O3_ARM_v7a.py  
b/configs/common/cores/arm/O3_ARM_v7a.py

index a402e5f..8cacc65 100644
--- a/configs/common/cores/arm/O3_ARM_v7a.py
+++ b/configs/common/cores/arm/O3_ARM_v7a.py
@@ -169,21 +169,6 @@
 # Consider the L2 a victim cache also for clean lines
 writeback_clean = True

-# TLB Cache
-# Use a cache as a L2 TLB
-class O3_ARM_v7aWalkCache(Cache):
-tag_latency = 4
-data_latency = 4
-response_latency = 4
-mshrs = 6
-tgts_per_mshr = 8
-size = '1kB'
-assoc = 8
-write_buffers = 16
-is_read_only = True
-# Writeback clean lines as well
-writeback_clean = True
-
 # L2 Cache
 class O3_ARM_v7aL2(Cache):
 tag_latency = 12
diff --git a/configs/common/cores/arm/ex5_LITTLE.py  
b/configs/common/cores/arm/ex5_LITTLE.py

index b3f1ad5..bcbaa92 100644
--- a/configs/common/cores/arm/ex5_LITTLE.py
+++ b/configs/common/cores/arm/ex5_LITTLE.py
@@ -112,21 +112,6 @@
 assoc = 4
 write_buffers = 4

-# TLB Cache
-# Use a cache as a L2 TLB
-class WalkCache(Cache):
-tag_latency = 2
-data_latency = 2
-response_latency = 2
-mshrs = 6
-tgts_per_mshr = 8
-size = '1kB'
-assoc = 2
-write_buffers = 16
-is_read_only = True
-# Writeback clean lines as well
-writeback_clean = True
-
 # L2 Cache
 class L2(Cache):
 tag_latency = 9
diff --git a/configs/common/cores/arm/ex5_big.py  
b/configs/common/cores/arm/ex5_big.py

index c734c62..eb5f53f 100644
--- a/configs/common/cores/arm/ex5_big.py
+++ b/configs/common/cores/arm/ex5_big.py
@@ -164,21 +164,6 @@
 assoc = 2
 write_buffers = 16

-# TLB Cache
-# Use a cache as a L2 TLB
-class WalkCache(Cache):
-tag_latency = 4
-data_latency = 4
-response_latency = 4
-mshrs = 6
-tgts_per_mshr = 8
-size = '1kB'
-assoc = 8
-write_buffers = 16
-is_read_only = True
-# Writeback clean lines as well
-writeback_clean = True
-
 # L2 Cache
 class L2(Cache):
 tag_latency = 15
diff --git a/configs/example/arm/devices.py b/configs/example/arm/devices.py
index 5217b08..9122e7c 100644
--- a/configs/example/arm/devices.py
+++ b/configs/example/arm/devices.py
@@ -65,17 +65,6 @@
 write_buffers = 16


-class WalkCache(PageTableWalkerCache):
-tag_latency = 4
-data_latency = 4
-response_latency = 4
-mshrs = 6
-tgts_per_mshr = 8
-size = '1kB'
-assoc = 8
-write_buffers = 16
-
-
 class L2(L2Cache):
 tag_latency = 12
 data_latency = 12

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54244
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-2
Gerrit-Change-Id: Iebda966e72b484ee15cbc7cd62256a950b2a90f1
Gerrit-Change-Number: 54244
Gerrit-PatchSet: 2
Gerrit-Owner: Giacomo Travaglini 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Richard Cooper 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: configs: Stop using a PTW cache before L2 in Arm configs

2021-12-16 Thread Giacomo Travaglini (Gerrit) via gem5-dev

Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54243 )


Change subject: configs: Stop using a PTW cache before L2 in Arm configs
..

configs: Stop using a PTW cache before L2 in Arm configs

This implementation of a walk cache does not allow to skip walks as it
is a simple cache placed in front of the table walker.
It was meant to provide a faster retrieval of page table descriptors
than fetching them from L2 or memory.

This is not needed anymore for Arm as from [1] we implement
partial translation caching in Arm TLBs.

[1]: JIRA: https://gem5.atlassian.net/browse/GEM5-1108

Change-Id: I00d44a4e3961e15602bf4352f2f42ddccf2b746b
Signed-off-by: Giacomo Travaglini 
Reviewed-by: Richard Cooper 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54243
Reviewed-by: Andreas Sandberg 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M configs/example/arm/starter_se.py
M configs/example/arm/ruby_fs.py
M configs/example/arm/fs_bigLITTLE.py
M configs/example/arm/baremetal.py
M configs/example/arm/starter_fs.py
M configs/common/CacheConfig.py
M configs/example/arm/devices.py
7 files changed, 37 insertions(+), 23 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved
  Richard Cooper: Looks good to me, approved
  Jason Lowe-Power: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/configs/common/CacheConfig.py b/configs/common/CacheConfig.py
index 4979f7d..61c6a30 100644
--- a/configs/common/CacheConfig.py
+++ b/configs/common/CacheConfig.py
@@ -87,7 +87,7 @@
 dcache_class, icache_class, l2_cache_class, walk_cache_class = \
 core.O3_ARM_v7a_DCache, core.O3_ARM_v7a_ICache, \
 core.O3_ARM_v7aL2, \
-core.O3_ARM_v7aWalkCache
+None
 elif options.cpu_type == "HPI":
 try:
 import cores.arm.HPI as core
@@ -96,7 +96,7 @@
 sys.exit(1)

 dcache_class, icache_class, l2_cache_class, walk_cache_class = \
-core.HPI_DCache, core.HPI_ICache, core.HPI_L2,  
core.HPI_WalkCache

+core.HPI_DCache, core.HPI_ICache, core.HPI_L2, None
 else:
 dcache_class, icache_class, l2_cache_class, walk_cache_class = \
 L1_DCache, L1_ICache, L2Cache, None
diff --git a/configs/example/arm/baremetal.py  
b/configs/example/arm/baremetal.py

index 9655bb1..0944344 100644
--- a/configs/example/arm/baremetal.py
+++ b/configs/example/arm/baremetal.py
@@ -61,14 +61,12 @@
 # the cache class may be 'None' if the particular cache is not present.
 cpu_types = {

-"atomic" : ( AtomicSimpleCPU, None, None, None, None),
+"atomic" : ( AtomicSimpleCPU, None, None, None),
 "minor" : (MinorCPU,
devices.L1I, devices.L1D,
-   devices.WalkCache,
devices.L2),
 "hpi" : ( HPI.HPI,
   HPI.HPI_ICache, HPI.HPI_DCache,
-  HPI.HPI_WalkCache,
   HPI.HPI_L2)
 }

diff --git a/configs/example/arm/devices.py b/configs/example/arm/devices.py
index 73aea59..5217b08 100644
--- a/configs/example/arm/devices.py
+++ b/configs/example/arm/devices.py
@@ -106,12 +106,11 @@

 class CpuCluster(SubSystem):
 def __init__(self, system,  num_cpus, cpu_clock, cpu_voltage,
- cpu_type, l1i_type, l1d_type, wcache_type, l2_type):
+ cpu_type, l1i_type, l1d_type, l2_type):
 super(CpuCluster, self).__init__()
 self._cpu_type = cpu_type
 self._l1i_type = l1i_type
 self._l1d_type = l1d_type
-self._wcache_type = wcache_type
 self._l2_type = l2_type

 assert num_cpus > 0
@@ -140,9 +139,7 @@
 for cpu in self.cpus:
 l1i = None if self._l1i_type is None else self._l1i_type()
 l1d = None if self._l1d_type is None else self._l1d_type()
-iwc = None if self._wcache_type is None else  
self._wcache_type()
-dwc = None if self._wcache_type is None else  
self._wcache_type()

-cpu.addPrivateSplitL1Caches(l1i, l1d, iwc, dwc)
+cpu.addPrivateSplitL1Caches(l1i, l1d)

 def addL2(self, clk_domain):
 if self._l2_type is None:
diff --git a/configs/example/arm/fs_bigLITTLE.py  
b/configs/example/arm/fs_bigLITTLE.py

index c590fe5..3f8b0cf 100644
--- a/configs/example/arm/fs_bigLITTLE.py
+++ b/configs/example/arm/fs_bigLITTLE.py
@@ -79,7 +79,7 @@
 def __init__(self, system, num_cpus, cpu_clock,
  cpu_voltage="1.0V"):
 cpu_config = [ ObjectList.cpu_list.get("O3_ARM_v7a_3"),
-devices.L1I, devices.L1D, devices.WalkCache, devices.L2 ]
+devices.L1I, devices.L1D, devices.L2 ]
 super(BigCluster, self).__init__(system, num_cpus, cpu_clock,
  cpu_voltage, *cpu_config)

@@ -87,7 +87,7 @@
 def __init__(self, system, num_cpus, cpu_clock,

[gem5-dev] Change in gem5/gem5[develop]: misc: Merge branch 'release-staging-v21-2' into develop

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: sim,python: Use scoped_interpreter_guard's argc and argv ctor arguments.

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: systemc: Change how python initialization callbacks are handled.

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: ext: In sst, don't assume existing imports in python blobs.

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: ext: In sst, set sys.argv up even when not initializing python.

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: ext: Stop using the uninitialized pythonMain in sst.

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: systemc: Update the testing framework to get it working again.

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: ext: Fix dumping stats error at the end of simulation

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: systemc: Eliminate the unused PythonReadyFunc mechanism.

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: arch,sim-se: Handle syscall retry/suppression in the syscall desc.

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: sim-se: (Re)add support for retrying system calls.

[gem5-dev] Change in gem5/gem5[develop]: mem-ruby: Fix message stall time calculation

[gem5-dev] Change in gem5/gem5[develop]: mem-cache: Added stats filtering both useful and spanPage prefetch.

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: ext: Fix dumping stats error at the end of simulation

[gem5-dev] Re: Broken SST due to python changes

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: gdb support Thumb-2 ISA

[gem5-dev] Change in gem5/gem5[develop]: base: Correct checkBpLen naming with checkBpKind

[gem5-dev] Re: Broken SST due to python changes

[gem5-dev] Re: Broken SST due to python changes

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: systemc: Eliminate the unused PythonReadyFunc mechanism.

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: systemc: Update the testing framework to get it working again.

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: systemc: Change how python initialization callbacks are handled.

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: configs: Remove unused WalkCache models

[gem5-dev] Change in gem5/gem5[release-staging-v21-2]: configs: Stop using a PTW cache before L2 in Arm configs

24 matches

Site Navigation

Mail list logo

Footer information