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

Change subject: util: Further simplify the initParam implementation.
......................................................................

util: Further simplify the initParam implementation.

Take advantage of string comparisons when looking up what to do with a
given key.

Convert the key_str[12] registers from little endian to host endian.
This matches a corresponding change in the m5 utility to pack the
registers in little endian order, regardless of what the actual guest
endianness is.

Absorb the initparam_keys.hh header into sim/pseudo_inst.cc, and convert
its constants to c++ strings. The constants defined in it might be
useful to guest code calling into the m5 ops, but not for gem5 itself.
By merging them into the .cc file, we also don't have to do any tricks to
try to avoid them having multiple definitions.

Change-Id: I3a450ad7f9c4dca25f79c7835d7f9c167c02ae98
---
D src/sim/initparam_keys.hh
M src/sim/pseudo_inst.cc
2 files changed, 41 insertions(+), 87 deletions(-)



diff --git a/src/sim/initparam_keys.hh b/src/sim/initparam_keys.hh
deleted file mode 100644
index aace7d2..0000000
--- a/src/sim/initparam_keys.hh
+++ /dev/null
@@ -1,68 +0,0 @@
-/*
- * Copyright (c) 2015 ARM Limited
- * All rights reserved
- *
- * The license below extends only to copyright in the software and shall
- * not be construed as granting a license to any other intellectual
- * property including but not limited to intellectual property relating
- * to a hardware implementation of the functionality of the software
- * licensed hereunder.  You may use the software subject to the license
- * terms below provided that you ensure that this notice is replicated
- * unmodified and in its entirety in all distributions of the software,
- * modified or unmodified, in source code or in binary form.
- *
- * 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.
- */
-
-/* @file
- * Magic key definitions for the InitParam pseudo inst
- */
-#ifndef ___SIM_INITPARAM_KEYS_HH__
-#define ___SIM_INITPARAM_KEYS_HH__
-
-namespace PseudoInst {
-/**
- * Unique keys to retrieve various params by the initParam pseudo inst.
- *
- * @note Each key must be shorter than 16 characters (because we use
- * two 64-bit registers two pass in the key to the initparam function)
- */
-struct InitParamKey
-{
-    /**
-     *  The default key (empty string)
-     */
-    static constexpr const char *DEFAULT = "";
-    /**
-     *  Unique key for "rank" param (distributed gem5 runs)
-     */
-    static constexpr const char *DIST_RANK = "dist-rank";
-    /**
-     *  Unique key for "size" param (distributed gem5 runs)
-     */
-    static constexpr const char *DIST_SIZE = "dist-size";
-};
-} // namespace PseudoInst
-
-#endif
diff --git a/src/sim/pseudo_inst.cc b/src/sim/pseudo_inst.cc
index 28d032c..5f64f80 100644
--- a/src/sim/pseudo_inst.cc
+++ b/src/sim/pseudo_inst.cc
@@ -44,6 +44,7 @@
 #include <fcntl.h>
 #include <unistd.h>

+#include <array>
 #include <cerrno>
 #include <fstream>
 #include <string>
@@ -62,7 +63,6 @@
 #include "kern/kernel_stats.hh"
 #include "params/BaseCPU.hh"
 #include "sim/full_system.hh"
-#include "sim/initparam_keys.hh"
 #include "sim/process.hh"
 #include "sim/serialize.hh"
 #include "sim/sim_events.hh"
@@ -78,6 +78,30 @@
 namespace PseudoInst
 {

+/**
+ * Unique keys to retrieve various params by the initParam pseudo inst.
+ *
+ * @note Each key may be at most 16 characters (because we use
+ * two 64-bit registers to pass in the key to the initparam function).
+ */
+namespace InitParamKey
+{
+
+/**
+ *  The default key (empty string)
+ */
+const std::string DEFAULT = "";
+/**
+ *  Unique key for "rank" param (distributed gem5 runs)
+ */
+const std::string DIST_RANK = "dist-rank";
+/**
+ *  Unique key for "size" param (distributed gem5 runs)
+ */
+const std::string DIST_SIZE = "dist-size";
+
+} // namespace InitParamKey
+
 static inline void
 panicFsOnlyPseudoInst(const char *name)
 {
@@ -254,28 +278,26 @@
     }

// The key parameter string is passed in via two 64-bit registers. We copy - // out the characters from the 64-bit integer variables here and concatenate
-    // them in the key_str character buffer
+    // out the characters from the 64-bit integer variables here, and
+    // concatenate them in the key character buffer
     const int len = 2 * sizeof(uint64_t) + 1;
-    char key_str[len];
-    memset(key_str, '\0', len);
+    char key[len];
+    memset(key, '\0', len);

-    memcpy(key_str, (char *)&key_str1, sizeof(key_str1));
- memcpy(key_str + sizeof(uint64_t), (char *)&key_str2, sizeof(key_str2));
+    std::array<uint64_t, 2> key_regs = { key_str1, key_str2 };
+    key_regs = letoh(key_regs);
+    memcpy(key, key_regs.data(), sizeof(key_regs));

-    // Compare the key parameter with the known values to select the return
-    // value
-    uint64_t val;
-    if (strcmp(key_str, InitParamKey::DEFAULT) == 0) {
-        val = tc->getCpuPtr()->system->init_param;
-    } else if (strcmp(key_str, InitParamKey::DIST_RANK) == 0) {
-        val = DistIface::rankParam();
-    } else if (strcmp(key_str, InitParamKey::DIST_SIZE) == 0) {
-        val = DistIface::sizeParam();
-    } else {
+    // Check key parameter to figure out what to return.
+    const std::string key_str(key);
+    if (key == InitParamKey::DEFAULT)
+        return tc->getCpuPtr()->system->init_param;
+    else if (key == InitParamKey::DIST_RANK)
+        return DistIface::rankParam();
+    else if (key == InitParamKey::DIST_SIZE)
+        return DistIface::sizeParam();
+    else
panic("Unknown key for initparam pseudo instruction:\"%s\"", key_str);
-    }
-    return val;
 }



--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/27230
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: I3a450ad7f9c4dca25f79c7835d7f9c167c02ae98
Gerrit-Change-Number: 27230
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <gabebl...@google.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to