power: Add error checking to MathExprPowerModel

MathExprPower model currently doesn't print any useful error messages
if an expression fails to evaluate. To add insult to injury, the model
only detects a failure when dumping stats and not at
initialization. This change adds a verification step in startup() that
ensures that all of the referenced stats actually exist.

Change-Id: I8f71c73341578d5882c8d93e482f5383fbda5f1d
Signed-off-by: Andreas Sandberg <andreas.sandberg@arm.com>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Reviewed-by: Sascha Bischoff <sascha.bischoff@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/2661
Reviewed-by: Jason Lowe-Power <jason@lowepower.com>
Maintainer: Jason Lowe-Power <jason@lowepower.com>

2 files changed, 78 insertions, 18 deletions

diff --git a/src/sim/power/mathexpr_powermodel.cc b/src/sim/power/mathexpr_powermodel.cc
index 7bd218659..13d225ee3 100644
--- a/src/sim/power/mathexpr_powermodel.cc
+++ b/src/sim/power/mathexpr_powermodel.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016 ARM Limited
+ * Copyright (c) 2016-2017 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -46,7 +46,7 @@
 #include "sim/sim_object.hh"
 
 MathExprPowerModel::MathExprPowerModel(const Params *p)
-    : PowerModelState(p), dyn_expr(p->dyn), st_expr(p->st)
+    : PowerModelState(p), dyn_expr(p->dyn), st_expr(p->st), failed(false)
 {
     // Calculate the name of the object we belong to
     std::vector<std::string> path;
@@ -65,9 +65,51 @@ MathExprPowerModel::startup()
     for (auto & i: Stats::statsList())
         if (i->name.find(basename) == 0)
             stats_map[i->name.substr(basename.size())] = i;
+
+    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(
+        std::bind(&MathExprPowerModel::getStatValue,
+                  this, std::placeholders::_1)
+        );
+
+    return value;
+}
+
+
+double
 MathExprPowerModel::getStatValue(const std::string &name) const
 {
     using namespace Stats;
@@ -77,12 +119,19 @@ MathExprPowerModel::getStatValue(const std::string &name) const
         return _temp;
 
     // Try to cast the stat, only these are supported right now
-    Info *info = stats_map.at(name);
+    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 Info *info = it->second;
 
-    ScalarInfo *si = dynamic_cast<ScalarInfo*>(info);
+    auto si = dynamic_cast<const ScalarInfo *>(info);
     if (si)
         return si->value();
-    FormulaInfo *fi = dynamic_cast<FormulaInfo*>(info);
+    auto fi = dynamic_cast<const FormulaInfo *>(info);
     if (fi)
         return fi->total();
 
diff --git a/src/sim/power/mathexpr_powermodel.hh b/src/sim/power/mathexpr_powermodel.hh
index 5c121c7b6..b4d0254d8 100644
--- a/src/sim/power/mathexpr_powermodel.hh
+++ b/src/sim/power/mathexpr_powermodel.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016 ARM Limited
+ * Copyright (c) 2016-2017 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -66,24 +66,14 @@ class MathExprPowerModel : public PowerModelState
      *
      * @return Power (Watts) consumed by this object (dynamic component)
      */
-    double getDynamicPower() const {
-        return dyn_expr.eval(
-            std::bind(&MathExprPowerModel::getStatValue,
-                this, std::placeholders::_1)
-        );
-    }
+    double getDynamicPower() const { return eval(dyn_expr); }
 
     /**
      * Get the static power consumption.
      *
      * @return Power (Watts) consumed by this object (static component)
      */
-    double getStaticPower() const {
-        return st_expr.eval(
-            std::bind(&MathExprPowerModel::getStatValue,
-                this, std::placeholders::_1)
-        );
-    }
+    double getStaticPower() const { return eval(st_expr); }
 
     /**
      * Get the value for a variable (maps to a stat)
@@ -99,6 +89,23 @@ class MathExprPowerModel : public PowerModelState
     void regStats();
 
   private:
+    /**
+     * Evaluate an expression in the context of this object, fatal if
+     * evaluation fails.
+     *
+     * @param expr Expression to evaluate
+     * @return Value of expression.
+     */
+    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;
@@ -108,6 +115,10 @@ class MathExprPowerModel : public PowerModelState
 
     // 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;
 };
 
 #endif