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

Change subject: util: Further consolidate the Args interface in the m5 utility.
......................................................................

util: Further consolidate the Args interface in the m5 utility.

Create static methods to convert any string to an integer or to pack it
into an array of integers. Create non-static methdos named pop() to
pop() the first element and simultaneously convert it. If the conversion
fails, the argument is not popped.

Change-Id: I55d98b3971e7abb7b6206d06ec7fcf046e828d29
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/27553
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
Reviewed-by: Daniel Carvalho <oda...@yahoo.com.br>
Maintainer: Jason Lowe-Power <power...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M util/m5/src/args.cc
M util/m5/src/args.hh
M util/m5/src/call_type/addr.cc
M util/m5/src/commands.cc
4 files changed, 150 insertions(+), 53 deletions(-)

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



diff --git a/util/m5/src/args.cc b/util/m5/src/args.cc
index b6bd1c8..1caad43 100644
--- a/util/m5/src/args.cc
+++ b/util/m5/src/args.cc
@@ -38,39 +38,20 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */

-#include <cinttypes>
-#include <cstdlib>
 #include <cstring>

 #include "args.hh"

 bool
-parse_int_args(Args &args, uint64_t ints[], int len)
-{
-// On 32 bit platforms we need to use strtoull to do the conversion
-#ifdef __LP64__
-#define strto64 strtoul
-#else
-#define strto64 strtoull
-#endif
-    for (int i = 0; i < len; ++i)
-        ints[i] = strto64(args.pop("0").c_str(), NULL, 0);
-
-#undef strto64
-    return true;
-}
-
-bool
-pack_arg_into_regs(Args &args, uint64_t regs[], int num_regs)
+Args::pack(const std::string &str, uint64_t regs[], int num_regs)
 {
     const size_t RegSize = sizeof(regs[0]);
     const size_t MaxLen = num_regs * RegSize;
-    const std::string &sarg = args.pop();
-    const char *arg = sarg.c_str();
+    const char *arg = str.c_str();

     memset(regs, 0, MaxLen);

-    size_t len = sarg.size();
+    size_t len = str.size();

     if (len > MaxLen)
         return false;
diff --git a/util/m5/src/args.hh b/util/m5/src/args.hh
index 1afd2f2..b541533 100644
--- a/util/m5/src/args.hh
+++ b/util/m5/src/args.hh
@@ -41,9 +41,9 @@
 #ifndef __ARGS_HH__
 #define __ARGS_HH__

-#include <cstddef>
 #include <cstdint>
 #include <initializer_list>
+#include <stdexcept>
 #include <string>
 #include <vector>

@@ -61,19 +61,115 @@
     }
     Args(std::initializer_list<std::string> strings) : args(strings) {}

+    /*
+     * Attempt to convert str into an integer.
+     *
+     * Return whether that succeeded.
+     */
+    static bool
+    stoi(const std::string &str, uint64_t &val)
+    {
+        try {
+            val = std::stoi(str, nullptr, 0);
+            return true;
+        } catch (const std::invalid_argument &e) {
+            return false;
+        } catch (const std::out_of_range &e) {
+            return false;
+        }
+    }
+
+    /*
+     * Attempt to convert str into an integer.
+     *
+     * Return whether that suceeded. If not, val will be set to def.
+     */
+    static bool
+    stoi(const std::string &str, uint64_t &val, uint64_t def)
+    {
+        val = def;
+        return stoi(str, val);
+    }
+
+    /*
+ * Attempt to pack str as a sequence of bytes in to regs in little endian
+     * byte order.
+     *
+     * Return whether that succeeded.
+     */
+ static bool pack(const std::string &str, uint64_t regs[], int num_regs);
+
+    /*
+     * If there are any arguments in the list, remove and return the first
+     * one. If not, return def instead.
+     */
     const std::string &
-    pop(const std::string &def = "") {
+    pop(const std::string &def = "")
+    {
         if (!size())
             return def;
         return args[offset++];
     }

+    /*
+     * If there are any arguments in the list, attempt to convert the first
+ * one to an integer. If successful, remove it and store its value in val.
+     *
+     * Return whether that succeeded.
+     */
+    bool
+    pop(uint64_t &val)
+    {
+        if (!size() || !stoi(args[offset], val)) {
+            return false;
+        } else {
+            offset++;
+            return true;
+        }
+    }
+
+    /*
+     * If there are any arguments in the list, attempt to convert the first
+ * one to an integer. If successful, remove it and store its value in val.
+     * If there are no arguments or the conversion failed, set val to def.
+     *
+ * Return true if there were no arguments, or there were and the conversion
+     * succeeded.
+     */
+    bool
+    pop(uint64_t &val, uint64_t def)
+    {
+        val = def;
+        if (!size())
+            return true;
+        if (stoi(args[offset], val)) {
+            offset++;
+            return true;
+        }
+        return false;
+    }
+
+    /*
+ * If there are any arguments in the list, attempt to pack the first one + * as a sequence of bytes into the integers in regs in little endian byte
+     * order.
+     *
+     * Return whether that succeeded.
+     */
+    bool
+    pop(uint64_t regs[], int num_regs)
+    {
+        if (!size() || !pack(args[offset], regs, num_regs)) {
+            return false;
+        } else {
+            offset++;
+            return true;
+        }
+    }
+
     size_t size() { return args.size() - offset; }

const std::string &operator [] (size_t idx) { return args[offset + idx]; }
 };

-bool parse_int_args(Args &args, uint64_t ints[], int len);
-bool pack_arg_into_regs(Args &args, uint64_t regs[], int num_regs);
-
 #endif // __ARGS_HH__
diff --git a/util/m5/src/call_type/addr.cc b/util/m5/src/call_type/addr.cc
index 752d831..f8e39c9 100644
--- a/util/m5/src/call_type/addr.cc
+++ b/util/m5/src/call_type/addr.cc
@@ -97,8 +97,7 @@
             if (arg[0] != '=')
                 usage();
             // Attempt to extract an address after the '='.
-            Args temp_args({ arg.substr(1) });
-            if (!parse_int_args(temp_args, &addr_override, 1))
+            if (!args.stoi(arg.substr(1), addr_override))
                 usage();
             // If we found an address, use it to override m5op_addr.
             m5op_addr = addr_override;
@@ -106,7 +105,7 @@
         }
// If an address override wasn't part of the first argument, check if
         // it's the second argument. If not, then there's no override.
-        if (args.size() && parse_int_args(args, &addr_override, 1)) {
+        if (args.pop(addr_override)) {
             m5op_addr = addr_override;
             return true;
         }
diff --git a/util/m5/src/commands.cc b/util/m5/src/commands.cc
index d721899..b59633b 100644
--- a/util/m5/src/commands.cc
+++ b/util/m5/src/commands.cc
@@ -108,49 +108,63 @@
     if (args.size() > 1)
         usage();

-    uint64_t ints[1];
-    if (!parse_int_args(args, ints, 1))
+    uint64_t ns_delay;
+    if (!args.pop(ns_delay, 0))
         usage();
-    (*dt.m5_exit)(ints[0]);
+
+    (*dt.m5_exit)(ns_delay);
 }

 static void
 do_fail(const DispatchTable &dt, Args &args)
 {
-    if (args.size() < 1 || args.size() > 2)
+    if (args.size() > 2)
         usage();

-    uint64_t ints[2] = { 0, 0 };
-    if (!parse_int_args(args, ints, args.size()))
+    uint64_t ns_delay, code;
+    if (!args.pop(code) || !args.pop(ns_delay, 0))
         usage();
-    (*dt.m5_fail)(ints[1], ints[0]);
+
+    (*dt.m5_fail)(ns_delay, code);
 }

 static void
 do_reset_stats(const DispatchTable &dt, Args &args)
 {
-    uint64_t ints[2];
-    if (!parse_int_args(args, ints, 2))
+    if (args.size() > 2)
         usage();
-    (*dt.m5_reset_stats)(ints[0], ints[1]);
+
+    uint64_t ns_delay, ns_period;
+    if (!args.pop(ns_delay, 0) || !args.pop(ns_period, 0))
+        usage();
+
+    (*dt.m5_reset_stats)(ns_delay, ns_period);
 }

 static void
 do_dump_stats(const DispatchTable &dt, Args &args)
 {
-    uint64_t ints[2];
-    if (!parse_int_args(args, ints, 2))
+    if (args.size() > 2)
         usage();
-    (*dt.m5_dump_stats)(ints[0], ints[1]);
+
+    uint64_t ns_delay, ns_period;
+    if (!args.pop(ns_delay, 0) || !args.pop(ns_period, 0))
+        usage();
+
+    (*dt.m5_dump_stats)(ns_delay, ns_period);
 }

 static void
 do_dump_reset_stats(const DispatchTable &dt, Args &args)
 {
-    uint64_t ints[2];
-    if (!parse_int_args(args, ints, 2))
+    if (args.size() > 2)
         usage();
-    (*dt.m5_dump_reset_stats)(ints[0], ints[1]);
+
+    uint64_t ns_delay, ns_period;
+    if (!args.pop(ns_delay, 0) || !args.pop(ns_period, 0))
+        usage();
+
+    (*dt.m5_dump_reset_stats)(ns_delay, ns_period);
 }

 static void
@@ -165,7 +179,7 @@
 static void
 do_write_file(const DispatchTable &dt, Args &args)
 {
-    if (args.size() != 1 && args.size() != 2)
+    if (args.size() < 1 || args.size() > 2)
         usage();

     const std::string &filename = args.pop();
@@ -177,10 +191,14 @@
 static void
 do_checkpoint(const DispatchTable &dt, Args &args)
 {
-    uint64_t ints[2];
-    if (!parse_int_args(args, ints, 2))
+    if (args.size() > 2)
         usage();
-    (*dt.m5_checkpoint)(ints[0], ints[1]);
+
+    uint64_t ns_delay, ns_period;
+    if (!args.pop(ns_delay, 0) || !args.pop(ns_period, 0))
+        usage();
+
+    (*dt.m5_checkpoint)(ns_delay, ns_period);
 }

 static void
@@ -189,8 +207,11 @@
     if (args.size() != 2)
         usage();

-    uint64_t addr = strtoul(args.pop().c_str(), NULL, 0);
+    uint64_t addr;
+    if (!args.pop(addr))
+        usage();
     const std::string &symbol = args.pop();
+
     (*dt.m5_add_symbol)(addr, symbol.c_str());
 }

@@ -207,11 +228,11 @@
 static void
 do_initparam(const DispatchTable &dt, Args &args)
 {
-    if (args.size() > 1)
+    if (args.size() != 1)
         usage();

     uint64_t key_str[2];
-    if (!pack_arg_into_regs(args, key_str, 2))
+    if (!args.pop(key_str, 2))
         usage();
     uint64_t val = (*dt.m5_init_param)(key_str[0], key_str[1]);
     std::cout << val;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/27553
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: I55d98b3971e7abb7b6206d06ec7fcf046e828d29
Gerrit-Change-Number: 27553
Gerrit-PatchSet: 20
Gerrit-Owner: Gabe Black <gabebl...@google.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Earl Ou <shunhsin...@google.com>
Gerrit-Reviewer: Gabe Black <gabebl...@google.com>
Gerrit-Reviewer: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: Yu-hsin Wang <yuhsi...@google.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