Nikos Nikoleris has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/27892 )

Change subject: sim-power: Fix the way the power model accesses stats
......................................................................

sim-power: Fix the way the power model accesses stats

With the introduction of StatGroups the organization of stats has
changed and the power modeling framework has been broken. This CL uses
the new function Stats::resolve to retrieve pointers to the necesary
stats and use them in the power estimation formulas.

Change-Id: Iedaa97eeddf51f7a0a1f222918715da309943be3
Signed-off-by: Nikos Nikoleris <nikos.nikole...@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/27892
Maintainer: Jason Lowe-Power <power...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
---
M src/sim/power/mathexpr_powermodel.cc
M src/sim/power/mathexpr_powermodel.hh
2 files changed, 23 insertions(+), 81 deletions(-)

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



diff --git a/src/sim/power/mathexpr_powermodel.cc b/src/sim/power/mathexpr_powermodel.cc
index 13af0fd..71131f5 100644
--- a/src/sim/power/mathexpr_powermodel.cc
+++ b/src/sim/power/mathexpr_powermodel.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016-2017 ARM Limited
+ * Copyright (c) 2016-2017, 2020 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -46,75 +46,39 @@
 #include "sim/sim_object.hh"

 MathExprPowerModel::MathExprPowerModel(const Params *p)
-    : PowerModelState(p), dyn_expr(p->dyn), st_expr(p->st), failed(false)
+    : PowerModelState(p), dyn_expr(p->dyn), st_expr(p->st)
 {
-    // Calculate the name of the object we belong to
-    std::vector<std::string> path;
-    tokenize(path, name(), '.', true);
-    // It's something like xyz.power_model.pm2
-    assert(path.size() > 2);
-    for (unsigned i = 0; i < path.size() - 2; i++)
-        basename += path[i] + ".";
 }

 void
 MathExprPowerModel::startup()
 {
-    // Create a map with stats and pointers for quick access
-    // Has to be done here, since we need access to the statsList
-    for (auto & i: Stats::statsList()) {
-        if (i->name.find(basename) == 0) {
-            // Add stats for this sim object and its child objects
-            stats_map[i->name.substr(basename.size())] = i;
-        } else if (i->name.find(".") == std::string::npos) {
-            // Add global stats (sim_seconds, for example)
-            stats_map[i->name] = i;
+    for (auto expr: {dyn_expr, st_expr}) {
+        std::vector<std::string> vars = expr.getVariables();
+
+        for (auto var: vars) {
+            // Automatic variables:
+ if (var == "temp" || var == "voltage" || var == "clock_period") {
+                continue;
+            }
+
+            auto *info = Stats::resolve(var);
+            fatal_if(!info, "Failed to evaluate %s in expression:\n%s\n",
+                     var, expr.toStr());
+            statsMap[var] = info;
         }
     }
-
-    tryEval(st_expr);
-    const bool st_failed = failed;
-
-    tryEval(dyn_expr);
-    const bool dyn_failed = failed;
-
-    if (st_failed || dyn_failed) {
-        const auto *p = dynamic_cast<const Params *>(params());
-        assert(p);
-
-        fatal("Failed to evaluate power expressions:\n%s%s%s\n",
-              st_failed ? p->st : "",
-              st_failed && dyn_failed ? "\n" : "",
-              dyn_failed ? p->dyn : "");
-    }
 }

 double
 MathExprPowerModel::eval(const MathExpr &expr) const
 {
-    const double value = tryEval(expr);
-
-    // This shouldn't happen unless something went wrong the equations
-    // were verified in startup().
-    panic_if(failed, "Failed to evaluate power expression '%s'\n",
-             expr.toStr());
-
-    return value;
-}
-
-double
-MathExprPowerModel::tryEval(const MathExpr &expr) const
-{
-    failed = false;
-    const double value = expr.eval(
+    return expr.eval(
         std::bind(&MathExprPowerModel::getStatValue,
                   this, std::placeholders::_1)
         );
-
-    return value;
 }

-
 double
 MathExprPowerModel::getStatValue(const std::string &name) const
 {
@@ -127,18 +91,13 @@
         return clocked_object->voltage();
     } else if (name=="clock_period") {
         return clocked_object->clockPeriod();
-    }
-
-    // Try to cast the stat, only these are supported right now
-    const auto it = stats_map.find(name);
-    if (it == stats_map.cend()) {
-        warn("Failed to find stat '%s'\n", name);
-        failed = true;
-        return 0;
     }

+    const auto it = statsMap.find(name);
+    assert(it != statsMap.cend());
     const Info *info = it->second;

+    // Try to cast the stat, only these are supported right now
     auto si = dynamic_cast<const ScalarInfo *>(info);
     if (si)
         return si->value();
diff --git a/src/sim/power/mathexpr_powermodel.hh b/src/sim/power/mathexpr_powermodel.hh
index d8bd239..1edb800 100644
--- a/src/sim/power/mathexpr_powermodel.hh
+++ b/src/sim/power/mathexpr_powermodel.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016-2017 ARM Limited
+ * Copyright (c) 2016-2017, 2020 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -82,9 +82,8 @@
      */
     double getStatValue(const std::string & name) const;

-    void startup();
-
-    void regStats();
+    void startup() override;
+    void regStats() override;

   private:
     /**
@@ -96,27 +95,11 @@
      */
     double eval(const MathExpr &expr) const;

-    /**
-     * Evaluate an expression in the context of this object, set
-     * failed if evaluation failed.
-     *
-     * @param expr Expression to evaluate
-     * @return Value of expression.
-     */
-    double tryEval(const MathExpr &expr) const;
-
     // Math expressions for dynamic and static power
     MathExpr dyn_expr, st_expr;

-    // Basename of the object in the gem5 stats hierachy
-    std::string basename;
-
     // Map that contains relevant stats for this power model
-    std::unordered_map<std::string, Stats::Info*> stats_map;
-
-    // Did the expression fail to evaluate (e.g., because a stat value
-    // can't be resolved)
-    mutable bool failed;
+    std::unordered_map<std::string, const Stats::Info*> statsMap;
 };

 #endif

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

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Iedaa97eeddf51f7a0a1f222918715da309943be3
Gerrit-Change-Number: 27892
Gerrit-PatchSet: 3
Gerrit-Owner: Nikos Nikoleris <nikos.nikole...@arm.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: Nikos Nikoleris <nikos.nikole...@arm.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to