From 3c86175d08ea1b19d7509a0ea0d3f7c31ed7be6b Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Sun, 24 Mar 2024 15:56:54 -0700 Subject: [PATCH 01/25] stdlib: Rename BaseScalarVector -> Vector This isn't a true Base class, it's just a Vector. In gem5 all Vectors are Scalar Vectors. This change simplfies the naming. Change-Id: Ib8881d854ab18de6acbf0fb200db2de6a43621a7 --- src/python/m5/ext/pystats/statistic.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/python/m5/ext/pystats/statistic.py b/src/python/m5/ext/pystats/statistic.py index 8daec11606..f11fc94a29 100644 --- a/src/python/m5/ext/pystats/statistic.py +++ b/src/python/m5/ext/pystats/statistic.py @@ -85,9 +85,9 @@ class Scalar(Statistic): self.datatype = datatype -class BaseScalarVector(Statistic): +class Vector(Statistic): """ - An abstract base class for classes containing a vector of Scalar values. + An Python statistics which representing a vector of Scalar values. """ value: List[Union[int, float]] @@ -127,7 +127,7 @@ class BaseScalarVector(Statistic): return sum(self.value) -class Distribution(BaseScalarVector): +class Distribution(Vector): """ A statistic type that stores information relating to distributions. Each distribution has a number of bins (>=1) @@ -182,7 +182,7 @@ class Distribution(BaseScalarVector): assert self.num_bins >= 1 -class Accumulator(BaseScalarVector): +class Accumulator(Vector): """ A statistical type representing an accumulator. """ From 252dbe9c72f12d05f7d476f97d7fb97f73d0080c Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Tue, 26 Mar 2024 01:45:00 -0700 Subject: [PATCH 02/25] stdlib: Add tests for PyStats's Vector and fix bugs The big thing missing from the Vector stats was that each position in the vector could have it's own unique id (a str, float, or int) and each position in the vector can have its own description. Therefore, to add this the Vector is represented as a dictionary mapping the unique ID to a Pystat Scaler (whcih can have it's own unique description. Change-Id: I3a8634f43298f6491300cf5a4f9d25dee8101808 --- .../m5/ext/pystats/serializable_stat.py | 5 + src/python/m5/ext/pystats/statistic.py | 41 ++++-- src/python/m5/stats/gem5stats.py | 37 +++-- src/test_objects/SConscript | 6 +- src/test_objects/StatTester.py | 18 +++ src/test_objects/stat_tester.cc | 33 +++++ src/test_objects/stat_tester.hh | 22 +++ .../gem5/stats/configs/pystat_vector_check.py | 137 ++++++++++++++++++ .../stats/configs/simstat_output_check.py | 49 +++---- tests/gem5/stats/test_simstats_output.py | 77 +++++++++- 10 files changed, 365 insertions(+), 60 deletions(-) create mode 100644 tests/gem5/stats/configs/pystat_vector_check.py diff --git a/src/python/m5/ext/pystats/serializable_stat.py b/src/python/m5/ext/pystats/serializable_stat.py index faa70a5022..e4eb861d3f 100644 --- a/src/python/m5/ext/pystats/serializable_stat.py +++ b/src/python/m5/ext/pystats/serializable_stat.py @@ -85,6 +85,11 @@ class SerializableStat: return value elif isinstance(value, datetime): return value.replace(microsecond=0).isoformat() + elif isinstance(value, Dict): + d = {} + for k, v in value.items(): + d[self.__process_json_value(k)] = self.__process_json_value(v) + return d elif isinstance(value, list): return [self.__process_json_value(v) for v in value] elif isinstance(value, StorageType): diff --git a/src/python/m5/ext/pystats/statistic.py b/src/python/m5/ext/pystats/statistic.py index f11fc94a29..3fb4fc11f6 100644 --- a/src/python/m5/ext/pystats/statistic.py +++ b/src/python/m5/ext/pystats/statistic.py @@ -27,8 +27,8 @@ from abc import ABC from typing import ( Any, + Dict, Iterable, - List, Optional, Union, ) @@ -90,41 +90,56 @@ class Vector(Statistic): An Python statistics which representing a vector of Scalar values. """ - value: List[Union[int, float]] - def __init__( self, - value: Iterable[Union[int, float]], + value: Dict[Union[str, int, float], Scalar], type: Optional[str] = None, description: Optional[str] = None, ): super().__init__( - value=list(value), + value=value, type=type, description=description, ) + def __getitem__(self, index: Union[int, str, float]) -> Scalar: + assert self.value != None + # In the case of string, we cast strings to integers of floats if they + # are numeric. This avoids users having to cast strings to integers. + if isinstance(index, str): + if index.isindex(): + index = int(index) + elif index.isnumeric(): + index = float(index) + return self.value[index] + + def size(self) -> int: + """ + Returns the size of the vector. + + :returns: The size of the vector. + """ + assert self.value != None + return len(self.value) + def mean(self) -> float: """ Returns the mean of the value vector. - :returns: The mean value across all bins. + :returns: The mean value across all values in the vector. """ assert self.value != None - assert isinstance(self.value, List) - from statistics import mean as statistics_mean - - return statistics_mean(self.value) + return self.count() / self.size() def count(self) -> float: """ - Returns the count across all the bins. + Returns the count (sum) of all values in the vector. - :returns: The sum of all bin values. + :returns: The sum of all vector values. """ assert self.value != None - return sum(self.value) + return sum(float(self.value[key]) for key in self.values) class Distribution(Vector): diff --git a/src/python/m5/stats/gem5stats.py b/src/python/m5/stats/gem5stats.py index 8b619a00b0..5e0d57ec15 100644 --- a/src/python/m5/stats/gem5stats.py +++ b/src/python/m5/stats/gem5stats.py @@ -183,29 +183,44 @@ def __get_distribution(statistic: _m5.stats.DistInfo) -> Distribution: def __get_vector(statistic: _m5.stats.VectorInfo) -> Vector: - to_add = dict() + vec: Dict[Union[str, int, float], Scalar] = {} + for index in range(statistic.size): # All the values in a Vector are Scalar values value = statistic.value[index] - unit = statistic.unit - description = statistic.subdescs[index] - # ScalarInfo uses the C++ `double`. - datatype = StorageType["f64"] + assert isinstance(value, float) or isinstance(value, int) # Sometimes elements within a vector are defined by their name. Other # times they have no name. When a name is not available, we name the # stat the index value. - if str(statistic.subnames[index]): - index_string = str(statistic.subnames[index]) + if len(statistic.subnames) > index and statistic.subnames[index]: + index_subname = str(statistic.subnames[index]) + if index_subname.isdigit(): + index_subname = int(index_subname) + elif index_subname.isnumeric(): + index_subname = float(index_subname) else: - index_string = str(index) + index_subname = index - to_add[index_string] = Scalar( - value=value, unit=unit, description=description, datatype=datatype + index_subdesc = None + if len(statistic.subdescs) > index and statistic.subdescs[index]: + index_subdesc = str(statistic.subdescs[index]) + else: + index_subdesc = statistic.desc + + vec[index_subname] = Scalar( + value=value, + unit=statistic.unit, + description=index_subdesc, + datatype=StorageType["f64"], ) - return Vector(scalar_map=to_add) + return Vector( + vec, + type="Vector", + description=statistic.desc, + ) def _prepare_stats(group: _m5.stats.Group): diff --git a/src/test_objects/SConscript b/src/test_objects/SConscript index 6d1d1ec2d1..840a920af2 100644 --- a/src/test_objects/SConscript +++ b/src/test_objects/SConscript @@ -27,5 +27,9 @@ Import ("*") if env['CONF']['USE_TEST_OBJECTS']: - SimObject('StatTester.py', sim_objects=['StatTester', 'ScalarStatTester']) + SimObject('StatTester.py', sim_objects=[ + 'StatTester', + 'ScalarStatTester', + 'VectorStatTester', + ]) Source('stat_tester.cc') diff --git a/src/test_objects/StatTester.py b/src/test_objects/StatTester.py index a413b47764..9664ab92d5 100644 --- a/src/test_objects/StatTester.py +++ b/src/test_objects/StatTester.py @@ -44,3 +44,21 @@ class ScalarStatTester(StatTester): cxx_class = "gem5::ScalarStatTester" value = Param.Float("The scalar stat's value.") + + +class VectorStatTester(StatTester): + type = "VectorStatTester" + cxx_header = "test_objects/stat_tester.hh" + cxx_class = "gem5::VectorStatTester" + + values = VectorParam.Float("The vector stat's values.") + subnames = VectorParam.String( + [], + "The vector stat's subnames. If position is empty, index int is " + "used instead.", + ) + subdescs = VectorParam.String( + [], + "The vector stat's subdescriptions. If empty, the subdescriptions " + "are not used.", + ) diff --git a/src/test_objects/stat_tester.cc b/src/test_objects/stat_tester.cc index bf90993287..5a52d6991f 100644 --- a/src/test_objects/stat_tester.cc +++ b/src/test_objects/stat_tester.cc @@ -51,4 +51,37 @@ ScalarStatTester::ScalarStatTesterStats::ScalarStatTesterStats( { } +void +VectorStatTester::setStats() +{ + for (int i = 0; i < params.values.size(); i++) + { + stats.vector[i] = (params.values[i]); + } +} + +VectorStatTester::VectorStatTesterStats::VectorStatTesterStats( + statistics::Group *parent, + const VectorStatTesterParams ¶ms +) : statistics::Group(parent), + vector(this, + params.name.c_str(), + statistics::units::Count::get(), + params.description.c_str() + ) +{ + vector.init(params.values.size()); + for (int i = 0; i < params.values.size(); i++) + { + if (params.subnames.size() > i) { + vector.subname(i, params.subnames[i]); + } else { + vector.subname(i, std::to_string(i)); + } + if (params.subdescs.size() > i) { + vector.subdesc(i, params.subdescs[i]); + } + } +} + } // namespace gem5 diff --git a/src/test_objects/stat_tester.hh b/src/test_objects/stat_tester.hh index d9e3371997..d2099583f6 100644 --- a/src/test_objects/stat_tester.hh +++ b/src/test_objects/stat_tester.hh @@ -32,6 +32,7 @@ #include "base/statistics.hh" #include "params/ScalarStatTester.hh" #include "params/StatTester.hh" +#include "params/VectorStatTester.hh" #include "sim/sim_object.hh" namespace gem5 @@ -114,6 +115,27 @@ class ScalarStatTester : public StatTester } stats; }; +class VectorStatTester : public StatTester +{ + private: + VectorStatTesterParams params; + + public: + VectorStatTester(const VectorStatTesterParams &p) : + StatTester(p), params(p), stats(this, p) {} + + protected: + void setStats() override; + struct VectorStatTesterStats : public statistics::Group + { + VectorStatTesterStats( + statistics::Group *parent, + const VectorStatTesterParams ¶ms + ); + statistics::Vector vector; + } stats; +}; + } // namespace gem5 #endif // __STAT_TESTER_HH__ diff --git a/tests/gem5/stats/configs/pystat_vector_check.py b/tests/gem5/stats/configs/pystat_vector_check.py new file mode 100644 index 0000000000..65f847b28c --- /dev/null +++ b/tests/gem5/stats/configs/pystat_vector_check.py @@ -0,0 +1,137 @@ +# Copyright (c) 2024 The Regents of the University of California +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer; +# redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution; +# neither the name of the copyright holders nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import argparse +import sys + +import m5 +from m5.objects import ( + Root, + VectorStatTester, +) +from m5.stats.gem5stats import get_simstat + +"""This script is used for checking that the Vector statistics set in " +the simulation are correctly parsed through to the python Pystats. +""" + +parser = argparse.ArgumentParser( + description="Tests the output of a Vector PyStat." +) + +parser.add_argument( + "value", + help="Comma delimited list representing the vector.", + type=lambda s: [float(item) for item in s.split(",")], +) + +parser.add_argument( + "--name", + type=str, + default="vector", + required=False, + help="Name of the vector statistic.", +) + +parser.add_argument( + "--description", + type=str, + default="", + required=False, + help="Description of the vector statistic.", +) + +parser.add_argument( + "--subnames", + help="Comma delimited list representing the vector subnames.", + type=str, +) + +parser.add_argument( + "--subdescs", + help="Comma delimited list representing the vector subdescs", + type=str, +) + +args = parser.parse_args() + +stat_tester = VectorStatTester() +stat_tester.name = args.name +stat_tester.description = args.description +stat_tester.values = args.value + +stat_tester.subnames = [] +if args.subnames: + stat_tester.subnames = [str(item) for item in args.subnames.split(",")] + +stat_tester.subdescs = [] +if args.subdescs: + stat_tester.subdescs = [str(item) for item in args.subdescs.split(",")] + +value_dict = {} +for i in range(len(args.value)): + i_name = i + description = args.description + if stat_tester.subnames and i < len(stat_tester.subnames): + i_name = stat_tester.subnames[i] + if stat_tester.subdescs and i < len(stat_tester.subdescs): + description = stat_tester.subdescs[i] + + value_dict[i_name] = { + "value": args.value[i], + "type": "Scalar", + "unit": "Count", + "description": description, + "datatype": "f64", + } + +expected_output = { + "type": "Group", + "time_conversion": None, + args.name: { + "value": value_dict, + "type": "Vector", + "description": args.description, + }, +} + +root = Root(full_system=False, system=stat_tester) + +m5.instantiate() +m5.simulate() + +simstats = get_simstat(stat_tester) +output = simstats.to_json()["system"] + +if output != expected_output: + print("Output statistics do not match expected:", file=sys.stderr) + print("", file=sys.stderr) + print("Expected:", file=sys.stderr) + print(expected_output, file=sys.stderr) + print("", file=sys.stderr) + print("Actual:", file=sys.stderr) + print(output, file=sys.stderr) + sys.exit(1) diff --git a/tests/gem5/stats/configs/simstat_output_check.py b/tests/gem5/stats/configs/simstat_output_check.py index 4cbe2d0ca7..f794e63c48 100644 --- a/tests/gem5/stats/configs/simstat_output_check.py +++ b/tests/gem5/stats/configs/simstat_output_check.py @@ -26,7 +26,6 @@ import argparse import sys -from typing import Union import m5 from m5.objects import ( @@ -42,24 +41,18 @@ against the expected JSON output and that produced by the SimStats module. """ parser = argparse.ArgumentParser(description="Tests the output of a SimStat.") -subparsers = parser.add_subparsers( - dest="statistic", help="SimStats statistic to test", required=True -) -scalar_parser = subparsers.add_parser( - "scalar", help="Test a scalar statistic." -) -scalar_parser.add_argument( +parser.add_argument( "value", type=float, help="The value of the scalar statistic." ) -scalar_parser.add_argument( +parser.add_argument( "--name", type=str, default="scalar", required=False, help="The name of the scalar statistic.", ) -scalar_parser.add_argument( +parser.add_argument( "--description", type=str, default="", @@ -69,27 +62,21 @@ scalar_parser.add_argument( args = parser.parse_args() -expected_output = None -stat_tester = None -if args.statistic == "scalar": - stat_tester = ScalarStatTester() - stat_tester.name = args.name - stat_tester.description = args.description - stat_tester.value = args.value - expected_output = { - "type": "Group", - "time_conversion": None, - args.name: { - "value": args.value, - "type": "Scalar", - "unit": "Count", - "description": args.description, - "datatype": "f64", - }, - } - -assert stat_tester is not None -assert expected_output is not None +stat_tester = ScalarStatTester() +stat_tester.name = args.name +stat_tester.description = args.description +stat_tester.value = args.value +expected_output = { + "type": "Group", + "time_conversion": None, + args.name: { + "value": args.value, + "type": "Scalar", + "unit": "Count", + "description": args.description, + "datatype": "f64", + }, +} root = Root(full_system=False, system=stat_tester) diff --git a/tests/gem5/stats/test_simstats_output.py b/tests/gem5/stats/test_simstats_output.py index c7393ebf2b..7d3752f0bc 100644 --- a/tests/gem5/stats/test_simstats_output.py +++ b/tests/gem5/stats/test_simstats_output.py @@ -39,7 +39,6 @@ gem5_verify_config( "simstat_output_check.py", ), config_args=[ - "scalar", "42", "--name", "scalar_test", @@ -63,7 +62,6 @@ gem5_verify_config( "simstat_output_check.py", ), config_args=[ - "scalar", "0", ], valid_isas=(constants.all_compiled_tag,), @@ -83,7 +81,6 @@ gem5_verify_config( "simstat_output_check.py", ), config_args=[ - "scalar", "-245", ], valid_isas=(constants.all_compiled_tag,), @@ -103,7 +100,6 @@ gem5_verify_config( "simstat_output_check.py", ), config_args=[ - "scalar", "42.869", "--name", "float_test", @@ -113,3 +109,76 @@ gem5_verify_config( valid_isas=(constants.all_compiled_tag,), length=constants.quick_tag, ) + +gem5_verify_config( + name="pystat_vector_test", + fixtures=(), + verifiers=[], + config=joinpath( + config.base_dir, + "tests", + "gem5", + "stats", + "configs", + "pystat_vector_check.py", + ), + config_args=[ + "2.0,4,5.9,2.3,-8,0,0.0,-8.9", + "--name", + "vector_stat", + "--description", + "A vector statistic with a float value", + ], + valid_isas=(constants.all_compiled_tag,), + length=constants.quick_tag, +) + +gem5_verify_config( + name="pystat_vector_with_subnames_test", + fixtures=(), + verifiers=[], + config=joinpath( + config.base_dir, + "tests", + "gem5", + "stats", + "configs", + "pystat_vector_check.py", + ), + config_args=[ + "2.0,4,3", + "--name", + "vector_stat", + "--description", + "A vector statistic with a float value", + "--subnames", + "first,second,third", + ], + valid_isas=(constants.all_compiled_tag,), + length=constants.quick_tag, +) + +gem5_verify_config( + name="pystat_vector_with_subdescs_test", + fixtures=(), + verifiers=[], + config=joinpath( + config.base_dir, + "tests", + "gem5", + "stats", + "configs", + "pystat_vector_check.py", + ), + config_args=[ + "2.0,4,3", + "--name", + "vector_stat", + "--description", + "A vector statistic with a float value", + "--subdescs", + "first,second", + ], + valid_isas=(constants.all_compiled_tag,), + length=constants.quick_tag, +) From 940e1d20637c950f984f77ab6c1048c522632dea Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Wed, 27 Mar 2024 10:01:21 -0700 Subject: [PATCH 03/25] stdlib: Fix PyStats Distribution to be vector of Scalars As Distribution inherits from Vector, it should be constructed with a Dictionary of scalars (in our implementation, a dictionary mapping the vector position's unique id for each bin and the value of that bin). Change-Id: Ie603c248e5db4b6dd7f71cc453eebd78793f69a3 --- src/python/m5/ext/pystats/statistic.py | 2 +- src/python/m5/stats/gem5stats.py | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/python/m5/ext/pystats/statistic.py b/src/python/m5/ext/pystats/statistic.py index 3fb4fc11f6..ebe24fbbca 100644 --- a/src/python/m5/ext/pystats/statistic.py +++ b/src/python/m5/ext/pystats/statistic.py @@ -164,7 +164,7 @@ class Distribution(Vector): def __init__( self, - value: Iterable[int], + value: Dict[Union[int, float], Scalar], min: Union[float, int], max: Union[float, int], num_bins: int, diff --git a/src/python/m5/stats/gem5stats.py b/src/python/m5/stats/gem5stats.py index 5e0d57ec15..bd17994a59 100644 --- a/src/python/m5/stats/gem5stats.py +++ b/src/python/m5/stats/gem5stats.py @@ -167,8 +167,16 @@ def __get_distribution(statistic: _m5.stats.DistInfo) -> Distribution: overflow = statistic.overflow logs = statistic.logs + parsed_values = {} + for index in range(len(value)): + parsed_values[index] = Scalar( + value=value[index], + unit=statistic.unit, + datatype=StorageType["f64"], + ) + return Distribution( - value=value, + value=parsed_values, min=min, max=max, num_bins=num_bins, From a3af819d827722ceccff29d418644ce4c5c0e383 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Wed, 27 Mar 2024 10:06:38 -0700 Subject: [PATCH 04/25] stdlib: Remove PyStats Accumulator This appears to have no equivalent type in the CPP stats and was never utilized in PyStats. Change-Id: Ia9afc83b4159eb1ab2c6f44ec0ad86cd73f2a4f8 --- src/python/m5/ext/pystats/jsonloader.py | 5 ---- src/python/m5/ext/pystats/statistic.py | 34 ------------------------- 2 files changed, 39 deletions(-) diff --git a/src/python/m5/ext/pystats/jsonloader.py b/src/python/m5/ext/pystats/jsonloader.py index 260b978174..dfd9da38de 100644 --- a/src/python/m5/ext/pystats/jsonloader.py +++ b/src/python/m5/ext/pystats/jsonloader.py @@ -37,7 +37,6 @@ from .group import ( ) from .simstat import SimStat from .statistic import ( - Accumulator, Distribution, Scalar, Statistic, @@ -74,10 +73,6 @@ class JsonLoader(json.JSONDecoder): d.pop("type", None) return Distribution(**d) - elif d["type"] == "Accumulator": - d.pop("type", None) - return Accumulator(**d) - elif d["type"] == "Group": return Group(**d) diff --git a/src/python/m5/ext/pystats/statistic.py b/src/python/m5/ext/pystats/statistic.py index ebe24fbbca..a2ac1cc337 100644 --- a/src/python/m5/ext/pystats/statistic.py +++ b/src/python/m5/ext/pystats/statistic.py @@ -195,37 +195,3 @@ class Distribution(Vector): # These check some basic conditions of a distribution. assert self.bin_size >= 0 assert self.num_bins >= 1 - - -class Accumulator(Vector): - """ - A statistical type representing an accumulator. - """ - - _count: int - min: Union[int, float] - max: Union[int, float] - sum_squared: Optional[int] - - def __init__( - self, - value: Iterable[Union[int, float]], - count: int, - min: Union[int, float], - max: Union[int, float], - sum_squared: Optional[int] = None, - description: Optional[str] = None, - ): - super().__init__( - value=value, - type="Accumulator", - description=description, - ) - - self._count = count - self.min = min - self.max = max - self.sum_squared = sum_squared - - def count(self) -> int: - return self._count From 6ae3692057dd66982c0c9585bb83fb63c4dcefe6 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Sun, 24 Mar 2024 15:57:30 -0700 Subject: [PATCH 05/25] stdlib: Add Vector2d to PyStats Change-Id: Icb2f691abf88ef4bac8d277e421329edb000209b --- src/python/m5/ext/pystats/statistic.py | 58 ++++++ src/python/m5/stats/gem5stats.py | 39 +++- src/python/pybind11/stats.cc | 12 ++ src/test_objects/SConscript | 1 + src/test_objects/StatTester.py | 25 +++ src/test_objects/stat_tester.cc | 49 +++++ src/test_objects/stat_tester.hh | 23 +++ .../stats/configs/pystat_vector2d_check.py | 172 ++++++++++++++++++ tests/gem5/stats/test_simstats_output.py | 30 +++ 9 files changed, 408 insertions(+), 1 deletion(-) create mode 100644 tests/gem5/stats/configs/pystat_vector2d_check.py diff --git a/src/python/m5/ext/pystats/statistic.py b/src/python/m5/ext/pystats/statistic.py index a2ac1cc337..b5141f462d 100644 --- a/src/python/m5/ext/pystats/statistic.py +++ b/src/python/m5/ext/pystats/statistic.py @@ -142,6 +142,64 @@ class Vector(Statistic): return sum(float(self.value[key]) for key in self.values) +class Vector2d(Statistic): + """ + A 2D vector of scalar values. + """ + + value: Dict[Union[str, int, float], Vector] + + def __init__( + self, + value: Dict[Union[str, int, float], Vector], + type: Optional[str] = None, + description: Optional[str] = None, + ): + assert ( + len({vector.size() for vector in value.values()}) == 1 + ), "All the Vectors in the 2d Vector are not of equal length." + + super().__init__( + value=value, + type=type, + description=description, + ) + + def x_size(self) -> int: + """Returns the number of elements in the x dimension.""" + assert self.value is not None + return len(self.value) + + def y_size(self) -> int: + """Returns the number of elements in the y dimension.""" + assert self.value is not None + return len(self.value[0]) + + def size(self) -> int: + """Returns the total number of elements.""" + return self.x_size() * self.y_size() + + def total(self) -> int: + """The total (sum) of all the entries in the 2d vector/""" + assert self.value is not None + total = 0 + for vector in self.value.values(): + for scalar in vector.values(): + total += scalar.value + return total + + def __getitem__(self, index: Union[str, int, float]) -> Vector: + assert self.value is not None + # In the case of string, we cast strings to integers of floats if they + # are numeric. This avoids users having to cast strings to integers. + if isinstance(index, str): + if index.isindex(): + index = int(index) + elif index.isnumeric(): + index = float(index) + return self.value[index] + + class Distribution(Vector): """ A statistic type that stores information relating to distributions. Each diff --git a/src/python/m5/stats/gem5stats.py b/src/python/m5/stats/gem5stats.py index bd17994a59..b4d5db9792 100644 --- a/src/python/m5/stats/gem5stats.py +++ b/src/python/m5/stats/gem5stats.py @@ -138,6 +138,8 @@ def __get_statistic(statistic: _m5.stats.Info) -> Optional[Statistic]: pass elif isinstance(statistic, _m5.stats.VectorInfo): return __get_vector(statistic) + elif isinstance(statistic, _m5.stats.Vector2dInfo): + return __get_vector2d(statistic) return None @@ -193,7 +195,6 @@ def __get_distribution(statistic: _m5.stats.DistInfo) -> Distribution: def __get_vector(statistic: _m5.stats.VectorInfo) -> Vector: vec: Dict[Union[str, int, float], Scalar] = {} - for index in range(statistic.size): # All the values in a Vector are Scalar values value = statistic.value[index] @@ -231,6 +232,42 @@ def __get_vector(statistic: _m5.stats.VectorInfo) -> Vector: ) +def __get_vector2d(statistic: _m5.stats.Vector2dInfo) -> Vector2d: + # All the values in a 2D Vector are Scalar values + description = statistic.desc + x_size = statistic.x_size + y_size = statistic.y_size + + vector_rep: Dict[Union[str, int, float], Vector] = {} + for x_index in range(x_size): + x_index_string = x_index + if x_index in statistic.subnames: + x_index_string = str(statistic.subnames[x_index]) + + x_desc = description + if x_index in statistic.subdescs: + x_desc = str(statistic.subdescs[x_index]) + x_vec: Dict[str, Scalar] = {} + for y_index in range(y_size): + y_index_val = y_index + if y_index in statistic.ysubnames: + y_index_val = str(statistic.subnames[y_index]) + + x_vec[y_index_val] = Scalar( + value=statistic.value[x_index * y_size + y_index], + unit=statistic.unit, + datatype=StorageType["f64"], + ) + + vector_rep[x_index_string] = Vector( + x_vec, + type="Vector", + description=x_desc, + ) + + return Vector2d(value=vector_rep, type="Vector2d", description=description) + + def _prepare_stats(group: _m5.stats.Group): """ Prepares the statistics for dumping. diff --git a/src/python/pybind11/stats.cc b/src/python/pybind11/stats.cc index fee8c52882..494b9eb876 100644 --- a/src/python/pybind11/stats.cc +++ b/src/python/pybind11/stats.cc @@ -76,6 +76,7 @@ cast_stat_info(const statistics::Info *info) */ TRY_CAST(statistics::FormulaInfo); TRY_CAST(statistics::VectorInfo); + TRY_CAST(statistics::Vector2dInfo); TRY_CAST(statistics::DistInfo); return py::cast(info); @@ -183,6 +184,17 @@ pybind_init_stats(py::module_ &m_native) [](const statistics::VectorInfo &info) { return info.total(); }) ; + py::class_>( + m, "Vector2dInfo") + .def_readonly("x_size", &statistics::Vector2dInfo::x) + .def_readonly("y_size", &statistics::Vector2dInfo::y) + .def_readonly("subnames", &statistics::Vector2dInfo::subnames) + .def_readonly("subdescs", &statistics::Vector2dInfo::subdescs) + .def_readonly("ysubnames", &statistics::Vector2dInfo::y_subnames) + .def_readonly("value", &statistics::Vector2dInfo::cvec) + ; + py::class_>( m, "FormulaInfo") diff --git a/src/test_objects/SConscript b/src/test_objects/SConscript index 840a920af2..e20c288fa7 100644 --- a/src/test_objects/SConscript +++ b/src/test_objects/SConscript @@ -31,5 +31,6 @@ if env['CONF']['USE_TEST_OBJECTS']: 'StatTester', 'ScalarStatTester', 'VectorStatTester', + 'Vector2dStatTester', ]) Source('stat_tester.cc') diff --git a/src/test_objects/StatTester.py b/src/test_objects/StatTester.py index 9664ab92d5..bdfb6a4494 100644 --- a/src/test_objects/StatTester.py +++ b/src/test_objects/StatTester.py @@ -62,3 +62,28 @@ class VectorStatTester(StatTester): "The vector stat's subdescriptions. If empty, the subdescriptions " "are not used.", ) + + +class Vector2dStatTester(StatTester): + type = "Vector2dStatTester" + cxx_header = "test_objects/stat_tester.hh" + cxx_class = "gem5::Vector2dStatTester" + + x_size = Param.Int("The number of elements in the x dimension.") + y_size = Param.Int("The number of elements in the y dimension.") + + values = VectorParam.Float("The vector stat's values, flattened.") + subnames = VectorParam.String( + [], + "The vector stat's subnames. If position is empty, index int is " + "used instead.", + ) + subdescs = VectorParam.String( + [], + "The vector stat's subdescriptions. If empty, the subdescriptions " + "are not used.", + ) + ysubnames = VectorParam.String( + [], + "The vector stat's y subdescriptions. If empty, the subdescriptions ", + ) diff --git a/src/test_objects/stat_tester.cc b/src/test_objects/stat_tester.cc index 5a52d6991f..335d0f120d 100644 --- a/src/test_objects/stat_tester.cc +++ b/src/test_objects/stat_tester.cc @@ -84,4 +84,53 @@ VectorStatTester::VectorStatTesterStats::VectorStatTesterStats( } } +void +Vector2dStatTester::setStats() +{ + for (int i = 0; i < params.x_size; i++) + { + for (int j = 0; j < params.y_size; j++) + { + stats.vector2d[i][j] = (params.values[j + i * params.y_size]); + } + } +} + +Vector2dStatTester::Vector2dStatTesterStats::Vector2dStatTesterStats( + statistics::Group *parent, + const Vector2dStatTesterParams ¶ms +) : statistics::Group(parent), + vector2d(this, + params.name.c_str(), + statistics::units::Count::get(), + params.description.c_str() + ) +{ + vector2d.init(params.x_size, params.y_size); + + assert(params.x_size * params.y_size == params.values.size()); + + for (int i = 0; i < params.x_size; i++) + { + if (params.subnames.size() > i) { + vector2d.subname(i, params.subnames[i]); + } else { + vector2d.subname(i, std::to_string(i)); + } + if (params.subdescs.size() > i) { + vector2d.subdesc(i, params.subdescs[i]); + } + } + for (int j = 0; j < params.y_size; j++) + { + if (params.ysubnames.size() > j) { + vector2d.ysubname(j, params.ysubnames[j]); + } else { + vector2d.ysubname(j, std::to_string(j)); + } + + } + +} + } // namespace gem5 diff --git a/src/test_objects/stat_tester.hh b/src/test_objects/stat_tester.hh index d2099583f6..ef5ebf09dc 100644 --- a/src/test_objects/stat_tester.hh +++ b/src/test_objects/stat_tester.hh @@ -32,6 +32,7 @@ #include "base/statistics.hh" #include "params/ScalarStatTester.hh" #include "params/StatTester.hh" +#include "params/Vector2dStatTester.hh" #include "params/VectorStatTester.hh" #include "sim/sim_object.hh" @@ -136,6 +137,28 @@ class VectorStatTester : public StatTester } stats; }; +class Vector2dStatTester : public StatTester +{ + private: + Vector2dStatTesterParams params; + + public: + Vector2dStatTester(const Vector2dStatTesterParams &p) : + StatTester(p), params(p), stats(this, p) {} + + protected: + void setStats() override; + struct Vector2dStatTesterStats : public statistics::Group + { + Vector2dStatTesterStats( + statistics::Group *parent, + const Vector2dStatTesterParams ¶ms + ); + statistics::Vector2d vector2d; + } stats; +}; + + } // namespace gem5 #endif // __STAT_TESTER_HH__ diff --git a/tests/gem5/stats/configs/pystat_vector2d_check.py b/tests/gem5/stats/configs/pystat_vector2d_check.py new file mode 100644 index 0000000000..5e07e23164 --- /dev/null +++ b/tests/gem5/stats/configs/pystat_vector2d_check.py @@ -0,0 +1,172 @@ +# Copyright (c) 2024 The Regents of the University of California +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer; +# redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution; +# neither the name of the copyright holders nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import argparse +import sys + +import m5 +from m5.objects import ( + Root, + Vector2dStatTester, +) +from m5.stats.gem5stats import get_simstat + +"""This script is used for checking that Vector2d statistics set in the +simulation are correctly parsed through to the python Pystats. +""" + +parser = argparse.ArgumentParser( + description="Tests the output of a Vector2D Pystat." +) + +parser.add_argument( + "value", + help="Comma delimited list representing the 2d vector in a flattened " + "state.", + type=lambda s: [float(item) for item in s.split(",")], +) + +parser.add_argument( + "num_vectors", + help="The number of vectors in the vector of vectors", + type=int, +) + +parser.add_argument( + "--name", + type=str, + default="vector2d", + required=False, + help="Name of the vector statistic.", +) + +parser.add_argument( + "--description", + type=str, + default="", + required=False, + help="Description of the vector statistic.", +) + +parser.add_argument( + "--subnames", + help="delimited list representing the vector subnames", + type=str, +) + +parser.add_argument( + "--subdescs", + help="delimited list representing the vector subdescs", + type=str, +) + +parser.add_argument( + "--ysubnames", + help="delimited list representing the vector ysubnames", + type=str, +) + + +args = parser.parse_args() + +expected_output = None +stat_tester = None + +stat_tester = Vector2dStatTester() +stat_tester.name = args.name +stat_tester.description = args.description +stat_tester.subnames = [] +if args.subnames: + stat_tester.subnames = [str(item) for item in args.subnames.split(",")] + +stat_tester.subdescs = [] +if args.subdescs: + stat_tester.subdescs = [str(item) for item in args.subdescs.split(",")] + +stat_tester.ysubnames = [] +if args.ysubnames: + stat_tester.ysubnames = [str(item) for item in args.ysubnames.split(",")] + +assert ( + len(args.value) % args.num_vectors == 0 +), "The number of values is not divisable by the number of vectors." + +stat_tester.x_size = args.num_vectors +stat_tester.y_size = len(args.value) / args.num_vectors +stat_tester.values = args.value + +vectors = {} # The representation we expect output. +for x in range(args.num_vectors): + x_index = x if x not in stat_tester.subnames else stat_tester.subnames[x] + + vector = {} + for y in range(stat_tester.y_size): + to_add = args.value[ + int(y + (x * (len(args.value) / args.num_vectors))) + ] + vector[y] = { + "value": to_add, + "type": "Scalar", + "unit": "Count", + "description": None, + "datatype": "f64", + } + + vectors[x_index] = { + "type": "Vector", + "description": stat_tester.subdescs[x] + if x in stat_tester.subdescs + else stat_tester.description, + "value": vector, + } + +expected_output = { + "type": "Group", + "time_conversion": None, + args.name: { + "type": "Vector2d", + "value": vectors, + "description": args.description, + }, +} + +root = Root(full_system=False, system=stat_tester) + +m5.instantiate() +m5.simulate() + +simstats = get_simstat(stat_tester) +output = simstats.to_json()["system"] + +if output != expected_output: + print("Output statistics do not match expected:", file=sys.stderr) + print("", file=sys.stderr) + print("Expected:", file=sys.stderr) + print(expected_output, file=sys.stderr) + print("", file=sys.stderr) + print("Actual:", file=sys.stderr) + print(output, file=sys.stderr) + sys.exit(1) diff --git a/tests/gem5/stats/test_simstats_output.py b/tests/gem5/stats/test_simstats_output.py index 7d3752f0bc..5d47c6f6df 100644 --- a/tests/gem5/stats/test_simstats_output.py +++ b/tests/gem5/stats/test_simstats_output.py @@ -182,3 +182,33 @@ gem5_verify_config( valid_isas=(constants.all_compiled_tag,), length=constants.quick_tag, ) + +gem5_verify_config( + name="pystat_vector2d_test", + fixtures=(), + verifiers=[], + config=joinpath( + config.base_dir, + "tests", + "gem5", + "stats", + "configs", + "pystat_vector2d_check.py", + ), + config_args=[ + "2.4,4.3,3.7,-1.4,-2,4,0,0", + 2, + "--name", + "vector2d_stat", + "--description", + "A 2d vector statistic with", + "--subnames", + "decimals,integers", + "--subdescs", + "A random collection of decimals,A random collection of integers", + "--ysubnames", + "first,second,third,fourth", + ], + valid_isas=(constants.all_compiled_tag,), + length=constants.quick_tag, +) From b5e8804cd47ac2778741716911f3231501abea5e Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Sat, 30 Mar 2024 11:36:22 -0700 Subject: [PATCH 06/25] stdlib: Remove 'Vector' group subclass This was not used and easily confused with the other 'Vector' in PyStats. Change-Id: I9294bb0ae04db0537c87a5f50ce023fc83d587b8 --- src/python/m5/ext/pystats/group.py | 22 +--------------------- src/python/m5/ext/pystats/jsonloader.py | 5 +---- 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/src/python/m5/ext/pystats/group.py b/src/python/m5/ext/pystats/group.py index bc292c9d0b..5b2e760b32 100644 --- a/src/python/m5/ext/pystats/group.py +++ b/src/python/m5/ext/pystats/group.py @@ -27,16 +27,12 @@ from typing import ( Dict, List, - Mapping, Optional, Union, ) from .abstract_stat import AbstractStat -from .statistic import ( - Scalar, - Statistic, -) +from .statistic import Statistic from .timeconversion import TimeConversion @@ -66,19 +62,3 @@ class Group(AbstractStat): for key, value in kwargs.items(): setattr(self, key, value) - - -class Vector(Group): - """ - The Vector class is used to store vector information. However, in gem5 - Vectors, in practise, hold information that is more like a dictionary of - Scalar Values. This class may change, and may be merged into Group in - accordance to decisions made in relation to - https://gem5.atlassian.net/browse/GEM5-867. - """ - - def __init__(self, scalar_map: Mapping[str, Scalar]): - super().__init__(type="Vector", time_conversion=None, **scalar_map) - - def _repr_name(self) -> str: - return "Vector" diff --git a/src/python/m5/ext/pystats/jsonloader.py b/src/python/m5/ext/pystats/jsonloader.py index dfd9da38de..6c0f07585f 100644 --- a/src/python/m5/ext/pystats/jsonloader.py +++ b/src/python/m5/ext/pystats/jsonloader.py @@ -31,10 +31,7 @@ from typing import ( Union, ) -from .group import ( - Group, - Vector, -) +from .group import Group from .simstat import SimStat from .statistic import ( Distribution, From 178679cbfda1e0946a412e5694ff887c1611c764 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Wed, 27 Mar 2024 10:06:16 -0700 Subject: [PATCH 07/25] stdlib: Add SparseHist to PyStats This is inclusive of tests to ensure they have implemented correctly. Change-Id: I5c84d5ffdb7b914936cfd86ca012a7b141eeaf42 --- src/python/m5/ext/pystats/statistic.py | 28 +++++ src/python/m5/stats/gem5stats.py | 20 ++++ src/python/pybind11/stats.cc | 10 ++ src/test_objects/SConscript | 1 + src/test_objects/StatTester.py | 11 ++ src/test_objects/stat_tester.cc | 26 +++++ src/test_objects/stat_tester.hh | 22 ++++ .../stats/configs/pystat_sparse_dist_check.py | 110 ++++++++++++++++++ tests/gem5/stats/test_simstats_output.py | 23 ++++ 9 files changed, 251 insertions(+) create mode 100644 tests/gem5/stats/configs/pystat_sparse_dist_check.py diff --git a/src/python/m5/ext/pystats/statistic.py b/src/python/m5/ext/pystats/statistic.py index b5141f462d..b3a8d3aa12 100644 --- a/src/python/m5/ext/pystats/statistic.py +++ b/src/python/m5/ext/pystats/statistic.py @@ -253,3 +253,31 @@ class Distribution(Vector): # These check some basic conditions of a distribution. assert self.bin_size >= 0 assert self.num_bins >= 1 + + +class SparseHist(Vector): + """A Sparse Histogram of values. A sparse histogram simply counts the " + frequency of each value in a sample. Ergo, it is, ineffect an disctionary + of values mapped to their count""" + + def __init__( + self, + value: Dict[float, Scalar], + description: Optional[str] = None, + ): + super().__init__( + value=value, + type="SparseHist", + description=description, + ) + + def size(self) -> int: + """The number of unique sampled values.""" + return len(self.value) + + def count(self) -> int: + """ + Returns the total number of samples. + """ + assert self.value != None + return sum(self.value.values()) diff --git a/src/python/m5/stats/gem5stats.py b/src/python/m5/stats/gem5stats.py index b4d5db9792..98cd0b3df1 100644 --- a/src/python/m5/stats/gem5stats.py +++ b/src/python/m5/stats/gem5stats.py @@ -140,6 +140,8 @@ def __get_statistic(statistic: _m5.stats.Info) -> Optional[Statistic]: return __get_vector(statistic) elif isinstance(statistic, _m5.stats.Vector2dInfo): return __get_vector2d(statistic) + elif isinstance(statistic, _m5.stats.SparseHistInfo): + return __get_sparse_hist(statistic) return None @@ -268,6 +270,24 @@ def __get_vector2d(statistic: _m5.stats.Vector2dInfo) -> Vector2d: return Vector2d(value=vector_rep, type="Vector2d", description=description) +def __get_sparse_hist(statistic: _m5.stats.SparseHistInfo) -> SparseHist: + description = statistic.desc + value = statistic.values + + parsed_values = {} + for val in value: + parsed_values[val] = Scalar( + value=value[val], + unit=statistic.unit, + datatype=StorageType["f64"], + ) + + return SparseHist( + value=parsed_values, + description=description, + ) + + def _prepare_stats(group: _m5.stats.Group): """ Prepares the statistics for dumping. diff --git a/src/python/pybind11/stats.cc b/src/python/pybind11/stats.cc index 494b9eb876..5083d625bf 100644 --- a/src/python/pybind11/stats.cc +++ b/src/python/pybind11/stats.cc @@ -78,6 +78,7 @@ cast_stat_info(const statistics::Info *info) TRY_CAST(statistics::VectorInfo); TRY_CAST(statistics::Vector2dInfo); TRY_CAST(statistics::DistInfo); + TRY_CAST(statistics::SparseHistInfo); return py::cast(info); @@ -195,6 +196,15 @@ pybind_init_stats(py::module_ &m_native) .def_readonly("value", &statistics::Vector2dInfo::cvec) ; + py::class_>( + m, "SparseHistInfo") + .def_property_readonly("values", //A Dict[float, int] of sample & count + [](const statistics::SparseHistInfo &info) { + return info.data.cmap; + }) + ; + py::class_>( m, "FormulaInfo") diff --git a/src/test_objects/SConscript b/src/test_objects/SConscript index e20c288fa7..3bcd695f9f 100644 --- a/src/test_objects/SConscript +++ b/src/test_objects/SConscript @@ -32,5 +32,6 @@ if env['CONF']['USE_TEST_OBJECTS']: 'ScalarStatTester', 'VectorStatTester', 'Vector2dStatTester', + 'SparseHistStatTester', ]) Source('stat_tester.cc') diff --git a/src/test_objects/StatTester.py b/src/test_objects/StatTester.py index bdfb6a4494..ad34e411d7 100644 --- a/src/test_objects/StatTester.py +++ b/src/test_objects/StatTester.py @@ -87,3 +87,14 @@ class Vector2dStatTester(StatTester): [], "The vector stat's y subdescriptions. If empty, the subdescriptions ", ) + + +class SparseHistStatTester(StatTester): + type = "SparseHistStatTester" + cxx_header = "test_objects/stat_tester.hh" + cxx_class = "gem5::SparseHistStatTester" + + samples = VectorParam.Float( + "The sparse histogram's sampled values, to be inserted into the " + "histogram." + ) diff --git a/src/test_objects/stat_tester.cc b/src/test_objects/stat_tester.cc index 335d0f120d..7e80aa8721 100644 --- a/src/test_objects/stat_tester.cc +++ b/src/test_objects/stat_tester.cc @@ -28,6 +28,8 @@ #include "test_objects/stat_tester.hh" +#include + #include "base/stats/group.hh" namespace gem5 @@ -133,4 +135,28 @@ Vector2dStatTester::Vector2dStatTesterStats::Vector2dStatTesterStats( } +void +SparseHistStatTester::setStats() +{ + for (auto sample : params.samples) { + stats.sparse_histogram.sample(sample); + } +} + +SparseHistStatTester::SparseHistStatTesterStats::SparseHistStatTesterStats( + statistics::Group *parent, + const SparseHistStatTesterParams ¶ms +) : statistics::Group(parent), + sparse_histogram( + this, + params.name.c_str(), + statistics::units::Count::get(), + params.description.c_str() + ) +{ + sparse_histogram.init( + (std::set(params.samples.begin(), params.samples.end())).size() + ); +} + } // namespace gem5 diff --git a/src/test_objects/stat_tester.hh b/src/test_objects/stat_tester.hh index ef5ebf09dc..5fafbdc420 100644 --- a/src/test_objects/stat_tester.hh +++ b/src/test_objects/stat_tester.hh @@ -31,6 +31,7 @@ #include "base/statistics.hh" #include "params/ScalarStatTester.hh" +#include "params/SparseHistStatTester.hh" #include "params/StatTester.hh" #include "params/Vector2dStatTester.hh" #include "params/VectorStatTester.hh" @@ -158,6 +159,27 @@ class Vector2dStatTester : public StatTester } stats; }; +class SparseHistStatTester : public StatTester +{ + private: + SparseHistStatTesterParams params; + + public: + SparseHistStatTester(const SparseHistStatTesterParams &p) : + StatTester(p), params(p), stats(this, p) {} + + protected: + void setStats() override; + struct SparseHistStatTesterStats : public statistics::Group + { + SparseHistStatTesterStats( + statistics::Group *parent, + const SparseHistStatTesterParams ¶ms + ); + statistics::SparseHistogram sparse_histogram; + } stats; +}; + } // namespace gem5 diff --git a/tests/gem5/stats/configs/pystat_sparse_dist_check.py b/tests/gem5/stats/configs/pystat_sparse_dist_check.py new file mode 100644 index 0000000000..5603fbe467 --- /dev/null +++ b/tests/gem5/stats/configs/pystat_sparse_dist_check.py @@ -0,0 +1,110 @@ +# Copyright (c) 2024 The Regents of the University of California +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer; +# redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution; +# neither the name of the copyright holders nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import argparse +import sys + +import m5 +from m5.objects import ( + Root, + SparseHistStatTester, +) +from m5.stats.gem5stats import get_simstat + +parser = argparse.ArgumentParser( + description="Tests the output of a SparseHist Pystat." +) +parser.add_argument( + "samples", + help="delimited list representing the samples for the distributed " + "histogram.", + type=lambda s: [float(item) for item in s.split(",")], +) + +parser.add_argument( + "--name", + type=str, + default="sparse_hist", + required=False, + help="The name of the Sparse Histogram statistic.", +) + +parser.add_argument( + "--description", + type=str, + default=None, + required=False, + help="The description of the Sparse Histogram statistic.", +) + +args = parser.parse_args() + +stat_tester = SparseHistStatTester( + name=args.name, description=args.description, samples=args.samples +) + +root = Root(full_system=False, system=stat_tester) +m5.instantiate() +m5.simulate() + +simstats = get_simstat(root) +output = simstats.to_json()["system"] + +value_dict = {} +for sample in args.samples: + value_dict[sample] = ( + 1 if sample not in value_dict else value_dict[sample] + 1 + ) + +scaler_dict = {} +for key in value_dict: + scaler_dict[key] = { + "unit": "Count", + "type": "Scalar", + "description": None, + "value": value_dict[key], + "datatype": "f64", + } + +expected_output = { + "type": "Group", + "time_conversion": None, + args.name: { + "value": scaler_dict, + "type": "SparseHist", + "description": str(args.description), + }, +} + +if output != expected_output: + print("Output statistics do not match expected:", file=sys.stderr) + print("", file=sys.stderr) + print("Expected:", file=sys.stderr) + print(expected_output, file=sys.stderr) + print("", file=sys.stderr) + print("Actual:", file=sys.stderr) + print(output, file=sys.stderr) + sys.exit(1) diff --git a/tests/gem5/stats/test_simstats_output.py b/tests/gem5/stats/test_simstats_output.py index 5d47c6f6df..7d47245bdd 100644 --- a/tests/gem5/stats/test_simstats_output.py +++ b/tests/gem5/stats/test_simstats_output.py @@ -212,3 +212,26 @@ gem5_verify_config( valid_isas=(constants.all_compiled_tag,), length=constants.quick_tag, ) + +gem5_verify_config( + name="pystat-sparsehist-test", + fixtures=(), + verifiers=[], + config=joinpath( + config.base_dir, + "tests", + "gem5", + "stats", + "configs", + "pystat_sparse_dist_check.py", + ), + config_args=[ + "1.0,1,1.00,23,23,0.2,0.2,0.2,0.2,-1,-1.0,264", + "--name", + "sparsehist_stat", + "--description", + "A sparse histogram statistic.", + ], + valid_isas=(constants.all_compiled_tag,), + length=constants.quick_tag, +) From c0a1fa33fe89a9e97e2fd15c336357dda4c70e65 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Thu, 2 May 2024 04:55:58 -0700 Subject: [PATCH 08/25] stdlib: Improve PyStat support for SimObject Vectors Change-Id: Iba0c93ffa5c4b18acf75af82965c63a8881df189 --- src/python/m5/ext/pystats/abstract_stat.py | 3 + src/python/m5/ext/pystats/group.py | 60 ++++++++ src/python/m5/stats/gem5stats.py | 136 +++++++++++------- .../stats/configs/pystat_sparse_dist_check.py | 16 ++- .../stats/configs/pystat_vector2d_check.py | 13 +- .../gem5/stats/configs/pystat_vector_check.py | 13 +- .../configs/pystats_simobjectvector_check.py | 78 ++++++++++ .../stats/configs/simstat_output_check.py | 13 +- tests/gem5/stats/test_simstats_output.py | 17 +++ 9 files changed, 292 insertions(+), 57 deletions(-) create mode 100644 tests/gem5/stats/configs/pystats_simobjectvector_check.py diff --git a/src/python/m5/ext/pystats/abstract_stat.py b/src/python/m5/ext/pystats/abstract_stat.py index bae327fcf9..19932a1e29 100644 --- a/src/python/m5/ext/pystats/abstract_stat.py +++ b/src/python/m5/ext/pystats/abstract_stat.py @@ -99,3 +99,6 @@ class AbstractStat(SerializableStat): return self.children( lambda _name: re.match(pattern, _name), recursive=True ) + + def __getitem__(self, item: str): + return getattr(self, item) diff --git a/src/python/m5/ext/pystats/group.py b/src/python/m5/ext/pystats/group.py index 5b2e760b32..d37f39459a 100644 --- a/src/python/m5/ext/pystats/group.py +++ b/src/python/m5/ext/pystats/group.py @@ -25,6 +25,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. from typing import ( + Any, Dict, List, Optional, @@ -62,3 +63,62 @@ class Group(AbstractStat): for key, value in kwargs.items(): setattr(self, key, value) + + +class SimObjectGroup(Group): + """A group of statistics encapulated within a SimObject.""" + + def __init__(self, **kwargs: Dict[str, Union[Group, Statistic]]): + super().__init__(type="SimObject", **kwargs) + + +class SimObjectVectorGroup(Group): + """A Vector of SimObject objects. I.e., that which would be constructed + from something like `system.cpu = [DerivO3CPU(), TimingSimpleCPU()]`. + """ + + def __init__(self, value: List[AbstractStat], **kwargs: Dict[str, Any]): + assert isinstance(value, list), "Value must be a list" + super().__init__(type="SimObjectVector", value=value, **kwargs) + + def __getitem__(self, index: Union[int, str, float]) -> AbstractStat: + if not isinstance(index, int): + raise KeyError( + f"Index {index} not found in int. Cannot index Array with " + "non-int" + ) + return self.value[index] + + def __iter__(self): + return iter(self.value) + + def __len__(self): + return len(self.value) + + def get_all_stats_of_name(self, name: str) -> List[AbstractStat]: + """ + Get all the stats in the vector of that name. Useful for performing + operations on all the stats of the same name in a vector. + """ + to_return = [] + for stat in self.value: + if hasattr(stat, name): + to_return.append(getattr(stat, name)) + + # If the name is in the format "sim.bla.whatever", we are looking for + # the "bla.whatever" stats in the "sim" group. + # This is messy, but it works. + name_split = name.split(".") + if len(name_split) == 1: + return to_return + + if name_split[0] not in self: + return to_return + + to_return.extend( + self[name_split[0]].get_all_stats_of_name(".".join(name_split[1:])) + ) + return to_return + + def __getitem__(self, item: int): + return self.value[item] diff --git a/src/python/m5/stats/gem5stats.py b/src/python/m5/stats/gem5stats.py index 98cd0b3df1..144c365001 100644 --- a/src/python/m5/stats/gem5stats.py +++ b/src/python/m5/stats/gem5stats.py @@ -41,6 +41,7 @@ from m5.ext.pystats.simstat import * from m5.ext.pystats.statistic import * from m5.ext.pystats.storagetype import * from m5.objects import * +from m5.params import SimObjectVector import _m5.stats @@ -83,33 +84,6 @@ class JsonOutputVistor: simstat.dump(fp=fp, **self.json_args) -def get_stats_group(group: _m5.stats.Group) -> Group: - """ - Translates a gem5 Group object into a Python stats Group object. A Python - statistic Group object is a dictionary of labeled Statistic objects. Any - gem5 object passed to this will have its ``getStats()`` and ``getStatGroups`` - function called, and all the stats translated (inclusive of the stats - further down the hierarchy). - - :param group: The gem5 _m5.stats.Group object to be translated to be a Python - stats Group object. Typically this will be a gem5 SimObject. - - :returns: The stats group object translated from the input gem5 object. - """ - - stats_dict = {} - - for stat in group.getStats(): - statistic = __get_statistic(stat) - if statistic is not None: - stats_dict[stat.name] = statistic - - for key in group.getStatGroups(): - stats_dict[key] = get_stats_group(group.getStatGroups()[key]) - - return Group(**stats_dict) - - def __get_statistic(statistic: _m5.stats.Info) -> Optional[Statistic]: """ Translates a _m5.stats.Info object into a Statistic object, to process @@ -302,8 +276,84 @@ def _prepare_stats(group: _m5.stats.Group): _prepare_stats(child) +def _process_simobject_object(simobject: SimObject) -> SimObjectGroup: + """ + Processes the stats of a SimObject, and returns a dictionary of the stats + for the SimObject with PyStats objects when appropriate. + + :param simobject: The SimObject to process the stats for. + + :returns: A dictionary of the PyStats stats for the SimObject. + """ + + assert isinstance( + simobject, SimObject + ), "simobject param must be a SimObject." + + stats = ( + { + "name": simobject.get_name(), + } + if simobject.get_name() + else {} + ) + + for stat in simobject.getStats(): + val = __get_statistic(stat) + if val: + stats[stat.name] = val + + for name, child in simobject._children.items(): + to_add = _process_simobject_stats(child) + if to_add: + stats[name] = to_add + + for name, child in sorted(simobject.getStatGroups().items()): + # Note: We are using the name of the group to determine if we have + # already processed the group as a child simobject or a statistic. + # This is to avoid SimObjectVector's being processed twice. It is far + # from an ideal solution, but it works for now. + if not any( + re.compile(f"{to_match}" + r"\d*").search(name) + for to_match in stats.keys() + ): + stats[name] = Group(**_process_simobject_stats(child)) + + return SimObjectGroup(**stats) + + +def _process_simobject_stats( + simobject: Union[ + SimObject, SimObjectVector, List[Union[SimObject, SimObjectVector]] + ] +) -> Union[List[Dict], Dict]: + """ + Processes the stats of a SimObject, SimObjectVector, or List of either, and + returns a dictionary of the PySqtats for the SimObject. + + :param simobject: The SimObject to process the stats for. + + :returns: A dictionary of the stats for the SimObject. + """ + + if isinstance(simobject, SimObject): + return _process_simobject_object(simobject) + + if isinstance(simobject, Union[List, SimObjectVector]): + stats_list = [] + for obj in simobject: + stats_list.append(_process_simobject_stats(obj)) + return SimObjectVectorGroup(value=stats_list) + + return {} + + def get_simstat( - root: Union[SimObject, List[SimObject]], prepare_stats: bool = True + root: Union[ + Union[SimObject, SimObjectVector], + List[Union[SimObject, SimObjectVector]], + ], + prepare_stats: bool = True, ) -> SimStat: """ This function will return the SimStat object for a simulation given a @@ -323,7 +373,7 @@ def get_simstat( :Returns: The SimStat Object of the current simulation. """ - stats_map = {} + creation_time = datetime.now() time_converstion = None # TODO https://gem5.atlassian.net/browse/GEM5-846 final_tick = Root.getInstance().resolveStat("finalTick").value @@ -334,29 +384,19 @@ def get_simstat( if prepare_stats: _m5.stats.processDumpQueue() - for r in root: - if isinstance(r, Root): - # The Root is a special case, we jump directly into adding its - # constituent Groups. - if prepare_stats: - _prepare_stats(r) - for key in r.getStatGroups(): - stats_map[key] = get_stats_group(r.getStatGroups()[key]) - elif isinstance(r, SimObject): - if prepare_stats: - _prepare_stats(r) - stats_map[r.get_name()] = get_stats_group(r) + if prepare_stats: + if isinstance(root, list): + for obj in root: + _prepare_stats(obj) else: - raise TypeError( - "Object (" + str(r) + ") passed is not a " - "SimObject. " + __name__ + " only processes " - "SimObjects, or a list of SimObjects." - ) + _prepare_stats(root) + + stats = _process_simobject_stats(root).__dict__ + stats["name"] = root.get_name() if root.get_name() else "root" return SimStat( creation_time=creation_time, - time_conversion=time_converstion, simulated_begin_time=simulated_begin_time, simulated_end_time=simulated_end_time, - **stats_map, + **stats, ) diff --git a/tests/gem5/stats/configs/pystat_sparse_dist_check.py b/tests/gem5/stats/configs/pystat_sparse_dist_check.py index 5603fbe467..57afbd7ffa 100644 --- a/tests/gem5/stats/configs/pystat_sparse_dist_check.py +++ b/tests/gem5/stats/configs/pystat_sparse_dist_check.py @@ -70,8 +70,9 @@ root = Root(full_system=False, system=stat_tester) m5.instantiate() m5.simulate() -simstats = get_simstat(root) -output = simstats.to_json()["system"] +simstats = get_simstat(stat_tester) +output = simstats.to_json() + value_dict = {} for sample in args.samples: @@ -90,7 +91,8 @@ for key in value_dict: } expected_output = { - "type": "Group", + "type": "SimObject", + "name": "system", "time_conversion": None, args.name: { "value": scaler_dict, @@ -99,6 +101,14 @@ expected_output = { }, } +# Remove the time related fields from the outputs if they exist. +# `creation_time` is not deterministic, and `simulated_begin_time` and +# simulated_end_time are not under test here. +for field in ["creation_time", "simulated_begin_time", "simulated_end_time"]: + for map in [output, expected_output]: + if field in map: + del map[field] + if output != expected_output: print("Output statistics do not match expected:", file=sys.stderr) print("", file=sys.stderr) diff --git a/tests/gem5/stats/configs/pystat_vector2d_check.py b/tests/gem5/stats/configs/pystat_vector2d_check.py index 5e07e23164..539a3cb6be 100644 --- a/tests/gem5/stats/configs/pystat_vector2d_check.py +++ b/tests/gem5/stats/configs/pystat_vector2d_check.py @@ -144,7 +144,8 @@ for x in range(args.num_vectors): } expected_output = { - "type": "Group", + "type": "SimObject", + "name": "system", "time_conversion": None, args.name: { "type": "Vector2d", @@ -159,7 +160,15 @@ m5.instantiate() m5.simulate() simstats = get_simstat(stat_tester) -output = simstats.to_json()["system"] +output = simstats.to_json() + +# Remove the time related fields from the outputs if they exist. +# `creation_time` is not deterministic, and `simulated_begin_time` and +# simulated_end_time are not under test here. +for field in ["creation_time", "simulated_begin_time", "simulated_end_time"]: + for map in [output, expected_output]: + if field in map: + del map[field] if output != expected_output: print("Output statistics do not match expected:", file=sys.stderr) diff --git a/tests/gem5/stats/configs/pystat_vector_check.py b/tests/gem5/stats/configs/pystat_vector_check.py index 65f847b28c..1f8b18a577 100644 --- a/tests/gem5/stats/configs/pystat_vector_check.py +++ b/tests/gem5/stats/configs/pystat_vector_check.py @@ -109,7 +109,8 @@ for i in range(len(args.value)): } expected_output = { - "type": "Group", + "type": "SimObject", + "name": "system", "time_conversion": None, args.name: { "value": value_dict, @@ -124,7 +125,15 @@ m5.instantiate() m5.simulate() simstats = get_simstat(stat_tester) -output = simstats.to_json()["system"] +output = simstats.to_json() + +# Remove the time related fields from the outputs if they exist. +# `creation_time` is not deterministic, and `simulated_begin_time` and +# simulated_end_time are not under test here. +for field in ["creation_time", "simulated_begin_time", "simulated_end_time"]: + for map in [output, expected_output]: + if field in map: + del map[field] if output != expected_output: print("Output statistics do not match expected:", file=sys.stderr) diff --git a/tests/gem5/stats/configs/pystats_simobjectvector_check.py b/tests/gem5/stats/configs/pystats_simobjectvector_check.py new file mode 100644 index 0000000000..1393cf98f3 --- /dev/null +++ b/tests/gem5/stats/configs/pystats_simobjectvector_check.py @@ -0,0 +1,78 @@ +# Copyright (c) 2024 The Regents of the University of California +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer; +# redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution; +# neither the name of the copyright holders nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""This script is used for checking that SimObject Vectorsare +correctly parsed through to the gem5 PyStats.""" + +import m5 +from m5.objects import ( + Root, + ScalarStatTester, + VectorStatTester, +) +from m5.stats.gem5stats import get_simstat + +root = Root(full_system=False) +root.stat_testers = [ + ScalarStatTester(name="placeholder", value=11), + ScalarStatTester( + name="placeholder", value=22, description="Index 2 desc." + ), + ScalarStatTester(name="placeholder", value=33), + VectorStatTester( + name="index_4", + values=[44, 55, 66], + description="A SimStat Vector within a SimObject Vector.", + ), +] + +m5.instantiate() +m5.simulate() + +simstat = get_simstat(root) + +# 'stat_testers' is a list of SimObjects +assert hasattr(simstat, "stat_testers"), "No stat_testers attribute found." +assert len(simstat.stat_testers) == 4, "stat_testers list is not of length 3." + +# Accessable by index. +simobject = simstat.stat_testers[0] + +# We can directly access the statistic we're interested in and its "str" +# representation should be the same as the value we set. In this case "11.0". +assert ( + str(simobject.placeholder) == "11.0" +), "placeholder value is not 11.0 ()." + +# They can also be accessed like so: +# "other_stat" is a SimObject with a single stat called "stat". +str( + simstat["stat_testers"][3]["index_4"][0] +) == "44.0", 'simstat[3]["index_4"][0] value is not 44.' + +# We can also access other stats like type and description. +assert simstat.stat_testers[1].placeholder.description == "Index 2 desc." +assert simstat.stat_testers[1].placeholder.type == "Scalar" diff --git a/tests/gem5/stats/configs/simstat_output_check.py b/tests/gem5/stats/configs/simstat_output_check.py index f794e63c48..e5379c4e39 100644 --- a/tests/gem5/stats/configs/simstat_output_check.py +++ b/tests/gem5/stats/configs/simstat_output_check.py @@ -67,7 +67,8 @@ stat_tester.name = args.name stat_tester.description = args.description stat_tester.value = args.value expected_output = { - "type": "Group", + "type": "SimObject", + "name": "system", "time_conversion": None, args.name: { "value": args.value, @@ -84,7 +85,15 @@ m5.instantiate() m5.simulate() simstats = get_simstat(stat_tester) -output = simstats.to_json()["system"] +output = simstats.to_json() + +# Remove the time related fields from the outputs if they exist. +# `creation_time` is not deterministic, and `simulated_begin_time` and +# simulated_end_time are not under test here. +for field in ["creation_time", "simulated_begin_time", "simulated_end_time"]: + for map in [output, expected_output]: + if field in map: + del map[field] if output != expected_output: print("Output statistics do not match expected:", file=sys.stderr) diff --git a/tests/gem5/stats/test_simstats_output.py b/tests/gem5/stats/test_simstats_output.py index 7d47245bdd..da0c9bdca9 100644 --- a/tests/gem5/stats/test_simstats_output.py +++ b/tests/gem5/stats/test_simstats_output.py @@ -235,3 +235,20 @@ gem5_verify_config( valid_isas=(constants.all_compiled_tag,), length=constants.quick_tag, ) + +gem5_verify_config( + name="simstat-simobjectvector-test", + fixtures=(), + verifiers=[], + config=joinpath( + config.base_dir, + "tests", + "gem5", + "stats", + "configs", + "pystats_simobjectvector_check.py", + ), + config_args=[], + valid_isas=(constants.all_compiled_tag,), + length=constants.quick_tag, +) From 45b26ce465ec510f277bf86ec6085f188a52564e Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Thu, 2 May 2024 05:48:05 -0700 Subject: [PATCH 09/25] stdlib: Specialize scalar tests; use 'pystat', not 'simstat' 1. Thests here for the Scalar tasks are named appropriately. Not just generic "SimStats tess". 2. We remove 'simstat' terminology. The correct word is "Pystats". Change-Id: Idebc4e750f4be7f140ad6bff9c6772f580a24861 --- ...at_output_check.py => pystat_scalar_check.py} | 10 +++++----- ..._simstats_output.py => test_pystat_output.py} | 16 ++++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) rename tests/gem5/stats/configs/{simstat_output_check.py => pystat_scalar_check.py} (89%) rename tests/gem5/stats/{test_simstats_output.py => test_pystat_output.py} (95%) diff --git a/tests/gem5/stats/configs/simstat_output_check.py b/tests/gem5/stats/configs/pystat_scalar_check.py similarity index 89% rename from tests/gem5/stats/configs/simstat_output_check.py rename to tests/gem5/stats/configs/pystat_scalar_check.py index e5379c4e39..3c98f901df 100644 --- a/tests/gem5/stats/configs/simstat_output_check.py +++ b/tests/gem5/stats/configs/pystat_scalar_check.py @@ -34,13 +34,13 @@ from m5.objects import ( ) from m5.stats.gem5stats import get_simstat -"""This script is used for checking that statistics set in the simulation are -correctly parsed through to the python SimStats. This script sets the -statistics using the "StatTester" simobjects and ensures verifies correctness -against the expected JSON output and that produced by the SimStats module. +"""This script is used for checking that Scaler statistics set in the simulation are +correctly parsed through to the python Pystats. """ -parser = argparse.ArgumentParser(description="Tests the output of a SimStat.") +parser = argparse.ArgumentParser( + description="Tests the output of a Scaler Pystat." +) parser.add_argument( "value", type=float, help="The value of the scalar statistic." diff --git a/tests/gem5/stats/test_simstats_output.py b/tests/gem5/stats/test_pystat_output.py similarity index 95% rename from tests/gem5/stats/test_simstats_output.py rename to tests/gem5/stats/test_pystat_output.py index da0c9bdca9..619509dc56 100644 --- a/tests/gem5/stats/test_simstats_output.py +++ b/tests/gem5/stats/test_pystat_output.py @@ -27,7 +27,7 @@ from testlib import * gem5_verify_config( - name="simstat-scaler-int-test", + name="pystat-scaler-int-test", fixtures=(), verifiers=[], config=joinpath( @@ -36,7 +36,7 @@ gem5_verify_config( "gem5", "stats", "configs", - "simstat_output_check.py", + "pystat_scalar_check.py", ), config_args=[ "42", @@ -50,7 +50,7 @@ gem5_verify_config( ) gem5_verify_config( - name="simstat-scaler-int-zero-test", + name="pystat-scaler-int-zero-test", fixtures=(), verifiers=[], config=joinpath( @@ -59,7 +59,7 @@ gem5_verify_config( "gem5", "stats", "configs", - "simstat_output_check.py", + "pystat_scalar_check.py", ), config_args=[ "0", @@ -69,7 +69,7 @@ gem5_verify_config( ) gem5_verify_config( - name="simstat-scaler-int-negative-test", + name="pystat-scaler-int-negative-test", fixtures=(), verifiers=[], config=joinpath( @@ -78,7 +78,7 @@ gem5_verify_config( "gem5", "stats", "configs", - "simstat_output_check.py", + "pystat_scalar_check.py", ), config_args=[ "-245", @@ -88,7 +88,7 @@ gem5_verify_config( ) gem5_verify_config( - name="simstat-scaler-float-test", + name="pystat-scaler-float-test", fixtures=(), verifiers=[], config=joinpath( @@ -97,7 +97,7 @@ gem5_verify_config( "gem5", "stats", "configs", - "simstat_output_check.py", + "pystat_scalar_check.py", ), config_args=[ "42.869", From c509615ec919a8171d4198a3e8f7f735bf3ec8d3 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Thu, 16 May 2024 02:44:28 -0700 Subject: [PATCH 10/25] tests: Pretty print Dict when compating for PyStats Change-Id: I1d93453072d12aa2dd40066f364723de1225b4e0 --- tests/gem5/stats/configs/pystat_scalar_check.py | 5 +++-- tests/gem5/stats/configs/pystat_sparse_dist_check.py | 5 +++-- tests/gem5/stats/configs/pystat_vector2d_check.py | 5 +++-- tests/gem5/stats/configs/pystat_vector_check.py | 5 +++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/gem5/stats/configs/pystat_scalar_check.py b/tests/gem5/stats/configs/pystat_scalar_check.py index 3c98f901df..1fe61b7f62 100644 --- a/tests/gem5/stats/configs/pystat_scalar_check.py +++ b/tests/gem5/stats/configs/pystat_scalar_check.py @@ -25,6 +25,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import argparse +import json import sys import m5 @@ -99,8 +100,8 @@ if output != expected_output: print("Output statistics do not match expected:", file=sys.stderr) print("", file=sys.stderr) print("Expected:", file=sys.stderr) - print(expected_output, file=sys.stderr) + print(json.dumps(expected_output, indent=4), file=sys.stderr) print("", file=sys.stderr) print("Actual:", file=sys.stderr) - print(output, file=sys.stderr) + print(json.dumps(output, indent=4), file=sys.stderr) sys.exit(1) diff --git a/tests/gem5/stats/configs/pystat_sparse_dist_check.py b/tests/gem5/stats/configs/pystat_sparse_dist_check.py index 57afbd7ffa..2d7bc4b6c1 100644 --- a/tests/gem5/stats/configs/pystat_sparse_dist_check.py +++ b/tests/gem5/stats/configs/pystat_sparse_dist_check.py @@ -25,6 +25,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import argparse +import json import sys import m5 @@ -113,8 +114,8 @@ if output != expected_output: print("Output statistics do not match expected:", file=sys.stderr) print("", file=sys.stderr) print("Expected:", file=sys.stderr) - print(expected_output, file=sys.stderr) + print(json.dumps(expected_output, indent=4), file=sys.stderr) print("", file=sys.stderr) print("Actual:", file=sys.stderr) - print(output, file=sys.stderr) + print(json.dumps(output, indent=4), file=sys.stderr) sys.exit(1) diff --git a/tests/gem5/stats/configs/pystat_vector2d_check.py b/tests/gem5/stats/configs/pystat_vector2d_check.py index 539a3cb6be..617463e56f 100644 --- a/tests/gem5/stats/configs/pystat_vector2d_check.py +++ b/tests/gem5/stats/configs/pystat_vector2d_check.py @@ -25,6 +25,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import argparse +import json import sys import m5 @@ -174,8 +175,8 @@ if output != expected_output: print("Output statistics do not match expected:", file=sys.stderr) print("", file=sys.stderr) print("Expected:", file=sys.stderr) - print(expected_output, file=sys.stderr) + print(json.dumps(expected_output, indent=4), file=sys.stderr) print("", file=sys.stderr) print("Actual:", file=sys.stderr) - print(output, file=sys.stderr) + print(json.dumps(output, indent=4), file=sys.stderr) sys.exit(1) diff --git a/tests/gem5/stats/configs/pystat_vector_check.py b/tests/gem5/stats/configs/pystat_vector_check.py index 1f8b18a577..e708f0319f 100644 --- a/tests/gem5/stats/configs/pystat_vector_check.py +++ b/tests/gem5/stats/configs/pystat_vector_check.py @@ -25,6 +25,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import argparse +import json import sys import m5 @@ -139,8 +140,8 @@ if output != expected_output: print("Output statistics do not match expected:", file=sys.stderr) print("", file=sys.stderr) print("Expected:", file=sys.stderr) - print(expected_output, file=sys.stderr) + print(json.dumps(expected_output, indent=4), file=sys.stderr) print("", file=sys.stderr) print("Actual:", file=sys.stderr) - print(output, file=sys.stderr) + print(json.dumps(output, indent=4), file=sys.stderr) sys.exit(1) From 0f6bd24c952ed622634156f22d890c123497559f Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Thu, 23 May 2024 14:54:21 -0700 Subject: [PATCH 11/25] stdlib: Fix get_simstat to accept lists of SimObjects Change-Id: Iae12a0ac88f9646acb00e73d70f83b1e2ff94ac9 --- src/python/m5/stats/gem5stats.py | 41 +++++++++++++++++++------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/python/m5/stats/gem5stats.py b/src/python/m5/stats/gem5stats.py index 144c365001..3e84ed0a6e 100644 --- a/src/python/m5/stats/gem5stats.py +++ b/src/python/m5/stats/gem5stats.py @@ -373,30 +373,37 @@ def get_simstat( :Returns: The SimStat Object of the current simulation. """ + stats_map = {} + for r in root: + creation_time = datetime.now() + time_converstion = ( + None # TODO https://gem5.atlassian.net/browse/GEM5-846 + ) + final_tick = Root.getInstance().resolveStat("finalTick").value + sim_ticks = Root.getInstance().resolveStat("simTicks").value + simulated_begin_time = int(final_tick - sim_ticks) + simulated_end_time = int(final_tick) - creation_time = datetime.now() - time_converstion = None # TODO https://gem5.atlassian.net/browse/GEM5-846 - final_tick = Root.getInstance().resolveStat("finalTick").value - sim_ticks = Root.getInstance().resolveStat("simTicks").value - simulated_begin_time = int(final_tick - sim_ticks) - simulated_end_time = int(final_tick) + if prepare_stats: + _m5.stats.processDumpQueue() - if prepare_stats: - _m5.stats.processDumpQueue() + if prepare_stats: + if isinstance(r, list): + for obj in r: + _prepare_stats(obj) + else: + _prepare_stats(r) - if prepare_stats: - if isinstance(root, list): - for obj in root: - _prepare_stats(obj) - else: - _prepare_stats(root) + stats = _process_simobject_stats(r).__dict__ + stats["name"] = r.get_name() if r.get_name() else "root" + stats_map[stats["name"]] = stats - stats = _process_simobject_stats(root).__dict__ - stats["name"] = root.get_name() if root.get_name() else "root" + if len(stats_map) == 1: + stats_map = stats_map[next(iter(stats_map))] return SimStat( creation_time=creation_time, simulated_begin_time=simulated_begin_time, simulated_end_time=simulated_end_time, - **stats, + **stats_map, ) From 8f0ed4606184bb8de2647063508a40c9276f19ef Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Mon, 27 May 2024 08:35:21 -0700 Subject: [PATCH 12/25] stdlib: Move `_m5.stats.processDumpQueue` to call-once This commit addresses Jason's comment (https://github.com/gem5/gem5/pull/996#discussion_r1613870880) which highlighted putting the `_m5.stats.processDumpQueue` call in the iteration through the `root` object in `get_simstat` caused this function be potentially called many times when it only needs to be called once. This chance moved this call to just before this iteration and will tehrefore only be called once (if required) per `get_simstat` execution. Change-Id: I16908b6dee063a0df7877a19e215883963bfb081 --- src/python/m5/stats/gem5stats.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/python/m5/stats/gem5stats.py b/src/python/m5/stats/gem5stats.py index 3e84ed0a6e..5b031d9a73 100644 --- a/src/python/m5/stats/gem5stats.py +++ b/src/python/m5/stats/gem5stats.py @@ -373,6 +373,10 @@ def get_simstat( :Returns: The SimStat Object of the current simulation. """ + + if prepare_stats: + _m5.stats.processDumpQueue() + stats_map = {} for r in root: creation_time = datetime.now() @@ -384,9 +388,6 @@ def get_simstat( simulated_begin_time = int(final_tick - sim_ticks) simulated_end_time = int(final_tick) - if prepare_stats: - _m5.stats.processDumpQueue() - if prepare_stats: if isinstance(r, list): for obj in r: From 62c1b9f9deb418124e8c8843e2294c514331037f Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Wed, 29 May 2024 08:16:38 -0700 Subject: [PATCH 13/25] tests: move Pystat pyunit tests to their own dir Change-Id: Ifd3d88deebd4e72bdb8792405966d2e158e6366d --- tests/pyunit/{ => pystats}/pyunit_jsonserializable_check.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/pyunit/{ => pystats}/pyunit_jsonserializable_check.py (100%) diff --git a/tests/pyunit/pyunit_jsonserializable_check.py b/tests/pyunit/pystats/pyunit_jsonserializable_check.py similarity index 100% rename from tests/pyunit/pyunit_jsonserializable_check.py rename to tests/pyunit/pystats/pyunit_jsonserializable_check.py From 6d174c43e4c99c5eaffc1c99ff93d3f7adee0806 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Wed, 29 May 2024 08:22:49 -0700 Subject: [PATCH 14/25] stdlib: Expand and simplify PyStats __init__.py 1. Adds newly added PyStat classes to "__init__.py", ensuring they can all be accessed via a `m5.ext.pystats` import. 2. Simplifies the layout out "__init__.py" to just import all classes from all files. Change-Id: I43bfc5e7ff1aec837e661905304c6fb10b00c90e --- src/python/m5/ext/pystats/__init__.py | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/python/m5/ext/pystats/__init__.py b/src/python/m5/ext/pystats/__init__.py index 0d15ee1ad1..d8d2b07a60 100644 --- a/src/python/m5/ext/pystats/__init__.py +++ b/src/python/m5/ext/pystats/__init__.py @@ -24,22 +24,11 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -from .abstract_stat import AbstractStat -from .group import Group -from .jsonloader import JsonLoader -from .serializable_stat import SerializableStat -from .simstat import SimStat -from .statistic import Statistic -from .storagetype import StorageType -from .timeconversion import TimeConversion - -__all__ = [ - "AbstractStat", - "Group", - "SimStat", - "Statistic", - "TimeConversion", - "StorageType", - "SerializableStat", - "JsonLoader", -] +from .abstract_stat import * +from .group import * +from .jsonloader import * +from .serializable_stat import * +from .simstat import * +from .statistic import * +from .storagetype import * +from .timeconversion import * From 2d4a213046f3a70a1d84cb6e9fe680492bff7482 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Wed, 29 May 2024 23:37:44 -0700 Subject: [PATCH 15/25] stdlib: Make PyStat SimStat inherit from Group The SimStat Object is nothing more than a group of other SimStats and is therefore logically a group. With this, functionality can be shared more easily. Change-Id: I5dce23a02d5871e640b422654ca063e590b1429a --- src/python/m5/ext/pystats/group.py | 4 +--- src/python/m5/ext/pystats/simstat.py | 22 ++++++++-------------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/python/m5/ext/pystats/group.py b/src/python/m5/ext/pystats/group.py index d37f39459a..7bdc5523c9 100644 --- a/src/python/m5/ext/pystats/group.py +++ b/src/python/m5/ext/pystats/group.py @@ -54,9 +54,7 @@ class Group(AbstractStat): str, Union["Group", Statistic, List["Group"], List["Statistic"]] ], ): - if type is None: - self.type = "Group" - else: + if type: self.type = type self.time_conversion = time_conversion diff --git a/src/python/m5/ext/pystats/simstat.py b/src/python/m5/ext/pystats/simstat.py index 06858bb9ad..ec8b68623e 100644 --- a/src/python/m5/ext/pystats/simstat.py +++ b/src/python/m5/ext/pystats/simstat.py @@ -32,22 +32,16 @@ from typing import ( Union, ) -from .abstract_stat import AbstractStat from .group import Group from .statistic import Statistic from .timeconversion import TimeConversion -class SimStat(AbstractStat): +class SimStat(Group): """ Contains all the statistics for a given simulation. """ - creation_time: Optional[datetime] - time_conversion: Optional[TimeConversion] - simulated_begin_time: Optional[Union[int, float]] - simulated_end_time: Optional[Union[int, float]] - def __init__( self, creation_time: Optional[datetime] = None, @@ -56,10 +50,10 @@ class SimStat(AbstractStat): simulated_end_time: Optional[Union[int, float]] = None, **kwargs: Dict[str, Union[Group, Statistic, List[Group]]] ): - self.creation_time = creation_time - self.time_conversion = time_conversion - self.simulated_begin_time = simulated_begin_time - self.simulated_end_time = simulated_end_time - - for key, value in kwargs.items(): - setattr(self, key, value) + super().__init__( + creation_time=creation_time, + time_conversion=time_conversion, + simulated_begin_time=simulated_begin_time, + simulated_end_time=simulated_end_time, + **kwargs + ) From 7f0290985f440a958c57fefa9b7ebe50832f5d7a Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Wed, 29 May 2024 23:47:46 -0700 Subject: [PATCH 16/25] stdlib,tests: Add Pyunit tests to check Pyunit nav, fix bugs Bigs fixed of note: 1. The 'find' method has been fixed to work. This involved making 'children' a class implemented per-subclass as required. 2. The 'get_all_stats_of_name' method has been removed. This was not working at all correctly and is largely doing what 'find' does. 2. The functionality to get an element in a vector via an attribute call (i.e., self.vector1 == self.vector[1]) has been implemented this maintaining backwards compatibility with the regular Python stats. Change-Id: I31a4ccc723937018a3038dcdf491c82629ddbbb2 --- src/python/m5/ext/pystats/abstract_stat.py | 65 +++++-- src/python/m5/ext/pystats/group.py | 60 +++--- src/python/m5/ext/pystats/statistic.py | 86 ++++++++- tests/pyunit/pystats/pyunit_pystats.py | 208 +++++++++++++++++++++ 4 files changed, 374 insertions(+), 45 deletions(-) create mode 100644 tests/pyunit/pystats/pyunit_pystats.py diff --git a/src/python/m5/ext/pystats/abstract_stat.py b/src/python/m5/ext/pystats/abstract_stat.py index 19932a1e29..98adda314b 100644 --- a/src/python/m5/ext/pystats/abstract_stat.py +++ b/src/python/m5/ext/pystats/abstract_stat.py @@ -26,10 +26,12 @@ import re from typing import ( + Any, Callable, List, Optional, Pattern, + Tuple, Union, ) @@ -60,20 +62,11 @@ class AbstractStat(SerializableStat): If it returns ``True``, then the child is yielded. Otherwise, the child is skipped. If not provided then all children are returned. + + Note: This is method must be implemented in AbstractStat subclasses + which have children, otherwise it will return an empty list. """ - - to_return = [] - for attr in self.__dict__: - obj = getattr(self, attr) - if isinstance(obj, AbstractStat): - if (predicate and predicate(attr)) or not predicate: - to_return.append(obj) - if recursive: - to_return = to_return + obj.children( - predicate=predicate, recursive=True - ) - - return to_return + return [] def find(self, regex: Union[str, Pattern]) -> List["AbstractStat"]: """Find all stats that match the name, recursively through all the @@ -100,5 +93,51 @@ class AbstractStat(SerializableStat): lambda _name: re.match(pattern, _name), recursive=True ) + def _get_vector_item(self, item: str) -> Optional[Tuple[str, int, Any]]: + """It has been the case in gem5 that SimObject vectors are stored as + strings such as "cpu0" or "cpu1". This function splits the string into + the SimObject name and index, (e.g.: ["cpu", 0] and ["cpu", 1]) and + returns the item for that name and it's index. If the string cannot be + split into a SimObject name and index, or if the SimObject does not + exit at `Simobject[index]`, the function returns None. + """ + regex = re.compile("[0-9]+$") + match = regex.search(item) + if not match: + return None + + match_str = match.group() + + assert match_str.isdigit(), f"Regex match must be a digit: {match_str}" + vector_index = int(match_str) + vector_name = item[: (-1 * len(match_str))] + + if hasattr(self, vector_name): + vector = getattr(self, vector_name) + try: + vector_value = vector[vector_index] + return vector_name, vector_index, vector_value + except KeyError: + pass + return None + + def __iter__(self): + return iter(self.__dict__) + + def __getattr__(self, item: str) -> Any: + vector_item = self._get_vector_item(item) + if not vector_item: + return None + + assert ( + len(vector_item) == 3 + ), f"Vector item must have 3 elements: {vector_item}" + return vector_item[2] + def __getitem__(self, item: str): return getattr(self, item) + + def __contains__(self, item: Any) -> bool: + return ( + isinstance(item, str) and self._get_vector_item(item) + ) or hasattr(self, item) diff --git a/src/python/m5/ext/pystats/group.py b/src/python/m5/ext/pystats/group.py index 7bdc5523c9..d1808221e7 100644 --- a/src/python/m5/ext/pystats/group.py +++ b/src/python/m5/ext/pystats/group.py @@ -26,6 +26,7 @@ from typing import ( Any, + Callable, Dict, List, Optional, @@ -62,6 +63,23 @@ class Group(AbstractStat): for key, value in kwargs.items(): setattr(self, key, value) + def children( + self, + predicate: Optional[Callable[[str], bool]] = None, + recursive: bool = False, + ) -> List["AbstractStat"]: + to_return = [] + for attr in self.__dict__: + obj = getattr(self, attr) + if isinstance(obj, AbstractStat): + if (predicate and predicate(attr)) or not predicate: + to_return.append(obj) + if recursive: + to_return = to_return + obj.children( + predicate=predicate, recursive=True + ) + return to_return + class SimObjectGroup(Group): """A group of statistics encapulated within a SimObject.""" @@ -93,30 +111,22 @@ class SimObjectVectorGroup(Group): def __len__(self): return len(self.value) - def get_all_stats_of_name(self, name: str) -> List[AbstractStat]: - """ - Get all the stats in the vector of that name. Useful for performing - operations on all the stats of the same name in a vector. - """ - to_return = [] - for stat in self.value: - if hasattr(stat, name): - to_return.append(getattr(stat, name)) - - # If the name is in the format "sim.bla.whatever", we are looking for - # the "bla.whatever" stats in the "sim" group. - # This is messy, but it works. - name_split = name.split(".") - if len(name_split) == 1: - return to_return - - if name_split[0] not in self: - return to_return - - to_return.extend( - self[name_split[0]].get_all_stats_of_name(".".join(name_split[1:])) - ) - return to_return - def __getitem__(self, item: int): return self.value[item] + + def __contains__(self, item): + if isinstance(item, int): + return item >= 0 and item < len(self) + + def children( + self, + predicate: Optional[Callable[[str], bool]] = None, + recursive: bool = False, + ) -> List["AbstractStat"]: + to_return = [] + for child in self.value: + to_return = to_return + child.children( + predicate=predicate, recursive=recursive + ) + + return to_return diff --git a/src/python/m5/ext/pystats/statistic.py b/src/python/m5/ext/pystats/statistic.py index b3a8d3aa12..fb98ceb93e 100644 --- a/src/python/m5/ext/pystats/statistic.py +++ b/src/python/m5/ext/pystats/statistic.py @@ -27,8 +27,10 @@ from abc import ABC from typing import ( Any, + Callable, Dict, Iterable, + List, Optional, Union, ) @@ -102,16 +104,32 @@ class Vector(Statistic): description=description, ) - def __getitem__(self, index: Union[int, str, float]) -> Scalar: + def __getitem__(self, item: Union[int, str, float]) -> Scalar: assert self.value != None # In the case of string, we cast strings to integers of floats if they # are numeric. This avoids users having to cast strings to integers. - if isinstance(index, str): - if index.isindex(): - index = int(index) - elif index.isnumeric(): - index = float(index) - return self.value[index] + if isinstance(item, str): + if item.isdigit(): + item = int(item) + elif item.isnumeric(): + item = float(item) + return self.value[item] + + def __contains__(self, item) -> bool: + assert self.value != None + if isinstance(item, str): + if item.isdigit(): + item = int(item) + elif item.isnumeric(): + item = float(item) + return item in self.value + + def __iner__(self) -> None: + return iter(self.value) + + def __len__(self) -> int: + assert self.value != None + return len(self.value.values()) def size(self) -> int: """ @@ -141,6 +159,26 @@ class Vector(Statistic): assert self.value != None return sum(float(self.value[key]) for key in self.values) + def children( + self, + predicate: Optional[Callable[[str], bool]] = None, + recursive: bool = False, + ) -> List["AbstractStat"]: + to_return = [] + for attr in self.value.keys(): + obj = self.value[attr] + if isinstance(obj, AbstractStat): + if ( + isinstance(attr, str) + and (predicate and predicate(attr)) + or not predicate + ): + to_return.append(obj) + to_return = to_return + obj.children( + predicate=predicate, recursive=True + ) + return to_return + class Vector2d(Statistic): """ @@ -179,6 +217,12 @@ class Vector2d(Statistic): """Returns the total number of elements.""" return self.x_size() * self.y_size() + def __len__(self) -> int: + return self.x_size() + + def __iter__(self): + return iter(self.keys()) + def total(self) -> int: """The total (sum) of all the entries in the 2d vector/""" assert self.value is not None @@ -199,6 +243,34 @@ class Vector2d(Statistic): index = float(index) return self.value[index] + def children( + self, + predicate: Optional[Callable[[str], bool]] = None, + recursive: bool = False, + ) -> List["AbstractStat"]: + to_return = [] + for attr in self.value.keys(): + obj = self.value[attr] + if ( + isinstance(attr, str) + and (predicate and predicate(attr)) + or not predicate + ): + to_return.append(obj) + to_return = to_return + obj.children( + predicate=predicate, recursive=True + ) + return to_return + + def __contains__(self, item) -> bool: + assert self.value is not None + if isinstance(item, str): + if item.isdigit(): + item = int(item) + elif item.isnumeric(): + item = float(item) + return item in self.value + class Distribution(Vector): """ diff --git a/tests/pyunit/pystats/pyunit_pystats.py b/tests/pyunit/pystats/pyunit_pystats.py new file mode 100644 index 0000000000..70839c245b --- /dev/null +++ b/tests/pyunit/pystats/pyunit_pystats.py @@ -0,0 +1,208 @@ +# Copyright (c) 2024 The Regents of The University of California +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer; +# redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution; +# neither the name of the copyright holders nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import unittest +from datetime import datetime + +from m5.ext.pystats import ( + Distribution, + Scalar, + SimObjectGroup, + SimObjectVectorGroup, + SimStat, + SparseHist, + Vector, + Vector2d, +) + + +def _get_mock_simstat() -> SimStat: + """Used to create a mock SimStat for testing. + This SimStat is contains all simstat Statistic types and attempts to use + most of the different types of values that can be stored in a Statistic. + """ + simobject_vector_group = SimObjectVectorGroup( + value=[ + SimObjectGroup( + **{ + "vector2d": Vector2d( + value={ + 0: Vector( + value={ + "a": Scalar(value=1, description="one"), + "b": Scalar(value=2.0, description="two"), + "c": Scalar(value=-3, description="three"), + } + ), + 1: Vector( + value={ + 1: Scalar(value=4), + 0.2: Scalar(value=5.0), + 0.3: Scalar(value=6), + }, + description="vector 1", + ), + }, + description="vector 2d", + ), + } + ), + SimObjectGroup( + **{ + "distribution": Distribution( + value={ + 0: Scalar(1), + 1: Scalar(2), + 2: Scalar(3), + 3: Scalar(4), + 4: Scalar(5), + }, + min=0, + max=4, + num_bins=5, + bin_size=1, + ), + "sparse_hist": SparseHist( + value={ + 0.5: Scalar(4), + 0.51: Scalar(1), + 0.511: Scalar(4), + 5: Scalar(2), + }, + description="sparse hist", + ), + }, + ), + ], + ) + + return SimStat( + creation_time=datetime.fromisoformat("2021-01-01T00:00:00"), + time_conversion=None, + simulated_begin_time=123, + simulated_end_time=558644, + simobject_vector=simobject_vector_group, + ) + + +class NavigatingPyStatsTestCase(unittest.TestCase): + """A test case for navigating the PyStats data structure, primarily + on how to access children of a SimStat object, and the "find" methods to + search for a specific statistic. + """ + + def setUp(self) -> None: + """Overrides the setUp method to create a mock SimStat for testing. + Runs before each test method. + """ + self.failFast = True + self.simstat = _get_mock_simstat() + super().setUp() + + def test_simstat_index(self): + self.assertTrue("simobject_vector" in self.simstat) + self.assertIsInstance( + self.simstat["simobject_vector"], SimObjectVectorGroup + ) + + def test_simstat_attribute(self): + self.assertTrue(hasattr(self.simstat, "simobject_vector")) + self.assertIsInstance( + self.simstat.simobject_vector, SimObjectVectorGroup + ) + + def test_simobject_vector_attribute(self): + # To maintan compatibility with the old way of accessing the vector, + # the simobject vectors values can be accessed by attributes of that + # simoobject vector name and the index appended to it. + # E.g., `simstat.simobject_vector0 is the same + # is simstat.simobject_vector[0]`. In cases where there is already + # an attribute with the same name as the vector+index, the attribute + # will be returned. + self.assertEqual( + self.simstat.simobject_vector0, self.simstat.simobject_vector[0] + ) + + def test_simobject_vector_index(self): + self.assertTrue(self.simstat.simobject_vector[0], SimObjectGroup) + + def test_simobject_group_index(self): + self.assertTrue("vector2d" in self.simstat.simobject_vector[0]) + self.assertIsInstance( + self.simstat.simobject_vector[0]["vector2d"], Vector2d + ) + + def test_simobject_group_attribute(self): + self.assertTrue(hasattr(self.simstat.simobject_vector[0], "vector2d")) + self.assertIsInstance( + self.simstat.simobject_vector[0].vector2d, Vector2d + ) + + def test_vector2d_index(self): + self.assertEqual(2, len(self.simstat.simobject_vector[0]["vector2d"])) + self.assertTrue(0 in self.simstat.simobject_vector[0].vector2d) + self.assertIsInstance( + self.simstat.simobject_vector[0].vector2d[0], Vector + ) + + def test_vector_index_int(self): + self.assertEqual(3, len(self.simstat.simobject_vector[0].vector2d[1])) + self.assertTrue(1 in self.simstat.simobject_vector[0].vector2d[1]) + self.assertIsInstance( + self.simstat.simobject_vector[0].vector2d[1][1], Scalar + ) + + def test_vector_index_str(self): + self.assertEqual(3, len(self.simstat.simobject_vector[0].vector2d[0])) + self.assertTrue("a" in self.simstat.simobject_vector[0].vector2d[0]) + self.assertIsInstance( + self.simstat.simobject_vector[0].vector2d[0]["a"], Scalar + ) + + def test_vector_index_float(self): + self.assertEqual(3, len(self.simstat.simobject_vector[0].vector2d[1])) + self.assertTrue(0.2 in self.simstat.simobject_vector[0].vector2d[1]) + self.assertIsInstance( + self.simstat.simobject_vector[0].vector2d[1][0.2], Scalar + ) + + def test_distriibution_index(self): + self.assertTrue(0 in self.simstat.simobject_vector[1]["distribution"]) + self.assertIsInstance( + self.simstat.simobject_vector[1]["distribution"][0], Scalar + ) + + def test_sparse_hist_index(self): + self.assertTrue(0.5 in self.simstat.simobject_vector[1]["sparse_hist"]) + self.assertIsInstance( + self.simstat.simobject_vector[1]["sparse_hist"][0.5], Scalar + ) + + def test_pystat_find(self): + self.assertEqual( + self.simstat.find("sparse_hist"), + [self.simstat.simobject_vector[1]["sparse_hist"]], + ) From c0a64c4862358ca05c66d35a1056ff32abb4779f Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Thu, 30 May 2024 02:58:06 -0700 Subject: [PATCH 17/25] stdlib: Move SimStat specific varibale sets out of loop Change-Id: I6e1f4c01a52ae904e9a6c6582b5b413f94c1cb05 --- src/python/m5/stats/gem5stats.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/python/m5/stats/gem5stats.py b/src/python/m5/stats/gem5stats.py index 5b031d9a73..e6185d4a94 100644 --- a/src/python/m5/stats/gem5stats.py +++ b/src/python/m5/stats/gem5stats.py @@ -379,15 +379,6 @@ def get_simstat( stats_map = {} for r in root: - creation_time = datetime.now() - time_converstion = ( - None # TODO https://gem5.atlassian.net/browse/GEM5-846 - ) - final_tick = Root.getInstance().resolveStat("finalTick").value - sim_ticks = Root.getInstance().resolveStat("simTicks").value - simulated_begin_time = int(final_tick - sim_ticks) - simulated_end_time = int(final_tick) - if prepare_stats: if isinstance(r, list): for obj in r: @@ -402,6 +393,13 @@ def get_simstat( if len(stats_map) == 1: stats_map = stats_map[next(iter(stats_map))] + creation_time = datetime.now() + time_converstion = None # TODO https://gem5.atlassian.net/browse/GEM5-846 + final_tick = Root.getInstance().resolveStat("finalTick").value + sim_ticks = Root.getInstance().resolveStat("simTicks").value + simulated_begin_time = int(final_tick - sim_ticks) + simulated_end_time = int(final_tick) + return SimStat( creation_time=creation_time, simulated_begin_time=simulated_begin_time, From cf5e316a925bedeb362d01410549b7c1c178c729 Mon Sep 17 00:00:00 2001 From: Jarvis Jia Date: Mon, 10 Jun 2024 16:33:18 -0500 Subject: [PATCH 18/25] Change black format Change-Id: I3733b31baf187e0d3d38d971d9423a1b1afe2296 --- configs/example/apu_se.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/configs/example/apu_se.py b/configs/example/apu_se.py index d686ecb919..2d3a849df0 100644 --- a/configs/example/apu_se.py +++ b/configs/example/apu_se.py @@ -892,9 +892,9 @@ gpu_port_idx = gpu_port_idx - args.num_cp * 2 token_port_idx = 0 for i in range(len(system.ruby._cpu_ports)): if isinstance(system.ruby._cpu_ports[i], VIPERCoalescer): - system.cpu[shader_idx].CUs[token_port_idx].gmTokenPort = ( - system.ruby._cpu_ports[i].gmTokenPort - ) + system.cpu[shader_idx].CUs[ + token_port_idx + ].gmTokenPort = system.ruby._cpu_ports[i].gmTokenPort token_port_idx += 1 wavefront_size = args.wf_size From d198380489f764f9cd4da553584f2e9e421bd52a Mon Sep 17 00:00:00 2001 From: Harry Chiang Date: Wed, 12 Jun 2024 01:53:00 +0800 Subject: [PATCH 19/25] base: Fix uninitialized variable warning in symtab.test.cc (#1221) This warning is appeared when I add warning related flags to LINKFLAGS and turn on LTO to build unit tests. --- src/base/loader/symtab.test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/base/loader/symtab.test.cc b/src/base/loader/symtab.test.cc index 9a7b8c86b9..2de89aed75 100644 --- a/src/base/loader/symtab.test.cc +++ b/src/base/loader/symtab.test.cc @@ -720,7 +720,7 @@ TEST(LoaderSymtabTest, FindNearestRoundWithNext) EXPECT_TRUE(symtab.insert(symbols[0])); EXPECT_TRUE(symtab.insert(symbols[1])); - Addr next_addr; + Addr next_addr = 0; const auto it = symtab.findNearest(symbols[0].address() + 0x1, next_addr); ASSERT_NE(it, symtab.end()); ASSERT_PRED_FORMAT2(checkSymbol, *it, symbols[0]); @@ -741,7 +741,7 @@ TEST(LoaderSymtabTest, FindNearestRoundWithNextNonExistent) "symbol", 0x10}; EXPECT_TRUE(symtab.insert(symbol)); - Addr next_addr; + Addr next_addr = 0; const auto it = symtab.findNearest(symbol.address() + 0x1, next_addr); ASSERT_NE(it, symtab.end()); ASSERT_PRED_FORMAT2(checkSymbol, *it, symbol); From 8fc4d3f793dc7f1837b2507398c423665d8c9908 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Tue, 11 Jun 2024 15:42:53 -0700 Subject: [PATCH 20/25] misc,tests: Update daily test artifact actions to v4.0.0 Change-Id: I711fa36639e925ce958e0484a31ee6a4dde87dbe --- .github/workflows/daily-tests.yaml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/daily-tests.yaml b/.github/workflows/daily-tests.yaml index cf39474401..102ef0ca71 100644 --- a/.github/workflows/daily-tests.yaml +++ b/.github/workflows/daily-tests.yaml @@ -61,7 +61,7 @@ jobs: run: scons setconfig build/${{ matrix.image }} ${{ matrix.setconfig-option }} - name: Build gem5 run: scons build/${{ matrix.image }}/gem5.opt -j $(nproc) - - uses: actions/upload-artifact@v4 + - uses: actions/upload-artifact@v4.0.0 with: name: ${{ needs.name-artifacts.outputs.build-name }}${{ matrix.image }} path: '*build/${{ matrix.image }}/gem5.opt' @@ -78,7 +78,7 @@ jobs: needs: [build-gem5, name-artifacts] steps: - name: Merge gem5 build artifacts - uses: actions/upload-artifact/merge@v4 + uses: actions/upload-artifact/merge@v4.0.0 with: name: ${{needs.name-artifacts.outputs.build-name}} pattern: ${{needs.name-artifacts.outputs.build-name}}* @@ -125,7 +125,7 @@ jobs: # download all artifacts for each test. Thoguh this is inelegant, # it's simpler than figuring otu which long tests requires which # binary. - - uses: actions/download-artifact@v4 + - uses: actions/download-artifact@v4.0.0 with: name: ${{needs.name-artifacts.outputs.build-name}} # The upload/download GitHub actions do not preserve the executable @@ -138,7 +138,7 @@ jobs: run: ./main.py run gem5/${{ matrix.test-type }} --length=long --skip-build -vv -t $(nproc) - name: upload results if: success() || failure() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v4.0.0 env: MY_STEP_VAR: ${{ matrix.test-type }}_COMMIT.${{github.sha}}_RUN.${{github.run_id}}_ATTEMPT.${{github.run_attempt}} with: @@ -167,7 +167,7 @@ jobs: # Scheduled workflows run on the default branch by default. We # therefore need to explicitly checkout the develop branch. ref: develop - - uses: actions/download-artifact@v4 + - uses: actions/download-artifact@v4.0.0 with: name: ${{needs.name-artifacts.outputs.build-name}}ALL - run: chmod u+x build/ALL/gem5.opt @@ -177,7 +177,7 @@ jobs: --skip-build -vv - name: upload results if: success() || failure() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v4.0.0 env: MY_STEP_VAR: ${{ matrix.test-type }}_COMMIT.${{github.sha}}_RUN.${{github.run_id}}_ATTEMPT.${{github.run_attempt}} with: From 7e45ec0ff03be22864044067e87da249ea356868 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Tue, 11 Jun 2024 16:08:21 -0700 Subject: [PATCH 21/25] stdlib: Fix m5.ext.pystats __init__.py Addresses Jason's complaint that wildcare imports should be avoided, in accordance with PEP008: https://github.com/gem5/gem5/pull/996#discussion_r1621051601. Change-Id: I72266df43d3ec4ede3f45c3e34e2e05e1990bd6b --- src/python/m5/ext/pystats/__init__.py | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/python/m5/ext/pystats/__init__.py b/src/python/m5/ext/pystats/__init__.py index d8d2b07a60..ce51c77f33 100644 --- a/src/python/m5/ext/pystats/__init__.py +++ b/src/python/m5/ext/pystats/__init__.py @@ -24,11 +24,22 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -from .abstract_stat import * -from .group import * -from .jsonloader import * -from .serializable_stat import * -from .simstat import * -from .statistic import * -from .storagetype import * -from .timeconversion import * +from .abstract_stat import AbstractStat +from .group import ( + Group, + SimObjectGroup, + SimObjectVectorGroup, +) +from .jsonloader import JsonLoader +from .serializable_stat import SerializableStat +from .simstat import SimStat +from .statistic import ( + Distribution, + Scalar, + SparseHist, + Statistic, + Vector, + Vector2d, +) +from .storagetype import StorageType +from .timeconversion import TimeConversion From 943d1f1453dc5489e41f9b05e7d5ab8aef95a413 Mon Sep 17 00:00:00 2001 From: Vishnu Ramadas Date: Mon, 10 Jun 2024 22:16:27 -0500 Subject: [PATCH 22/25] mem-ruby: Fix deadlock in GPU_VIPER when issuing atomic requests When a compute unit issues several requests to the same line, the requests wait in the L2 if it is a writeback cache. If the line is invalid initially and the first request is atomic in nature, the L2 cache issues a request to main memory. On data return, the cache line transitions to M but doesn't wake up the other requests, resulting in a deadlock. This commit adds a wakeup call on data return for atomics and fixes potential deadlocks. Change-Id: I8200ce6e77da7c8b4db285c0cc8b8ca0dfa7d720 --- src/mem/ruby/protocol/GPU_VIPER-TCC.sm | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mem/ruby/protocol/GPU_VIPER-TCC.sm b/src/mem/ruby/protocol/GPU_VIPER-TCC.sm index f6ac25be36..9092222a4d 100644 --- a/src/mem/ruby/protocol/GPU_VIPER-TCC.sm +++ b/src/mem/ruby/protocol/GPU_VIPER-TCC.sm @@ -1299,6 +1299,7 @@ machine(MachineType:TCC, "TCC Cache") wardb_writeAtomicResponseDirtyBytes; pa_performAtomic; baplr_sendBypassedAtomicPerformedLocallyResponse; + wada_wakeUpAllDependentsAddr; dt_deallocateTBE; pr_popResponseQueue; } From 42b9a9666ee7b57afb8ae724d816c9a194228bd3 Mon Sep 17 00:00:00 2001 From: Vishnu Ramadas Date: Tue, 11 Jun 2024 20:35:03 -0500 Subject: [PATCH 23/25] mem-ruby: Add instSeqNum to atomic responses from GPU L2 caches This commit adds instSeqNum to the atomic responses in GPU_VIPER-TCC.sm. This will be useful when debugging issues related to GPU atomic transactions Change-Id: Ic05c8e1a1cb230abfca2759b51e5603304aadaa3 --- src/mem/ruby/protocol/GPU_VIPER-TCC.sm | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mem/ruby/protocol/GPU_VIPER-TCC.sm b/src/mem/ruby/protocol/GPU_VIPER-TCC.sm index 9092222a4d..da4318bcf9 100644 --- a/src/mem/ruby/protocol/GPU_VIPER-TCC.sm +++ b/src/mem/ruby/protocol/GPU_VIPER-TCC.sm @@ -606,6 +606,7 @@ machine(MachineType:TCC, "TCC Cache") out_msg.Destination.add(in_msg.Requestor); out_msg.Sender := machineID; out_msg.MessageSize := MessageSizeType:Response_Data; + out_msg.instSeqNum := in_msg.instSeqNum; out_msg.DataBlk := cache_entry.DataBlk; out_msg.isGLCSet := in_msg.isGLCSet; out_msg.isSLCSet := in_msg.isSLCSet; From 261490f23cfc3066784400e3746812f7e3e5b01e Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Wed, 12 Jun 2024 00:14:27 -0700 Subject: [PATCH 24/25] misc,tests: Revert merge version to 'v4' from 'v4.0.0' 'v4.0.0' wasn't working. The following error was occurred: ``` Can't find 'action.yml', 'action.yaml' or 'Dockerfile' for action 'actions/upload-artifact/merge@v4.0.0'. ``` Change-Id: I658b0fe292df029501fbc1286acb06f4014ae4e1 --- .github/workflows/daily-tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/daily-tests.yaml b/.github/workflows/daily-tests.yaml index 102ef0ca71..a45c39a183 100644 --- a/.github/workflows/daily-tests.yaml +++ b/.github/workflows/daily-tests.yaml @@ -78,7 +78,7 @@ jobs: needs: [build-gem5, name-artifacts] steps: - name: Merge gem5 build artifacts - uses: actions/upload-artifact/merge@v4.0.0 + uses: actions/upload-artifact/merge@v4 with: name: ${{needs.name-artifacts.outputs.build-name}} pattern: ${{needs.name-artifacts.outputs.build-name}}* From 74afea471d6f3763b726cf3459b401fa62b74b54 Mon Sep 17 00:00:00 2001 From: Harshil Patel Date: Wed, 12 Jun 2024 00:20:06 -0700 Subject: [PATCH 25/25] cpu: Revert "Don't change to suspend if the thread status is halted" (#1225) Reverts gem5/gem5#1039 --- src/cpu/o3/thread_context.cc | 4 +--- src/cpu/simple_thread.cc | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/cpu/o3/thread_context.cc b/src/cpu/o3/thread_context.cc index 2eac107c99..06210de07e 100644 --- a/src/cpu/o3/thread_context.cc +++ b/src/cpu/o3/thread_context.cc @@ -86,9 +86,7 @@ ThreadContext::suspend() DPRINTF(O3CPU, "Calling suspend on Thread Context %d\n", threadId()); - if (thread->status() == gem5::ThreadContext::Suspended || - thread->status() == gem5::ThreadContext::Halting || - thread->status() == gem5::ThreadContext::Halted) + if (thread->status() == gem5::ThreadContext::Suspended) return; if (cpu->isDraining()) { diff --git a/src/cpu/simple_thread.cc b/src/cpu/simple_thread.cc index 9e90ce45b8..c28359a4ed 100644 --- a/src/cpu/simple_thread.cc +++ b/src/cpu/simple_thread.cc @@ -143,8 +143,7 @@ SimpleThread::activate() void SimpleThread::suspend() { - if (status() == ThreadContext::Suspended || - status() == ThreadContext::Halted) + if (status() == ThreadContext::Suspended) return; lastActivate = curTick();