[gem5-dev] Change in gem5/gem5[develop]: base: Create wide multiply functions in intmath.hh.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: base: Create wide multiply functions in intmath.hh.
..

base: Create wide multiply functions in intmath.hh.

These implementations are from the x86 multiply microops. When
multipying two integers of a certain width together, these functions
will produce two values of the same size which hold the upper and lower
part of the multiplication result.

The version which works for 32 bit values and smaller just takes
advantage of 64 bit multiplication using standard types. The 64 bit
version needs to do more work since there isn't a built in standard
facility for doing those sorts of multiplications.

Change-Id: If7b3d3aa174dd13aae6f383772cbc5291181de5d
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42358
Reviewed-by: Gabe Black 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/base/intmath.hh
M src/base/intmath.test.cc
2 files changed, 211 insertions(+), 0 deletions(-)

Approvals:
  Gabe Black: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/base/intmath.hh b/src/base/intmath.hh
index 0852913..308c33f 100644
--- a/src/base/intmath.hh
+++ b/src/base/intmath.hh
@@ -109,6 +109,91 @@
 }

 /**
+ * @ingroup api_base_utils
+ */
+template 
+static constexpr std::enable_if_t
+mulUnsigned(std::make_unsigned_t , std::make_unsigned_t ,
+std::make_unsigned_t val_a, std::make_unsigned_t val_b)
+{
+uint64_t product = (uint64_t)val_a * (uint64_t)val_b;
+low = product;
+high = (product >> (sizeof(low) * 8));
+};
+
+/**
+ * @ingroup api_base_utils
+ */
+template 
+static constexpr std::enable_if_t
+mulSigned(std::make_signed_t , std::make_signed_t ,
+  std::make_signed_t val_a, std::make_signed_t val_b)
+{
+uint64_t product = (int64_t)val_a * (int64_t)val_b;
+low = product;
+high = (product >> (sizeof(low) * 8));
+};
+
+/**
+ * @ingroup api_base_utils
+ * Multiply two values with place value p.
+ *
+ *  (A * p + a) * (B * p + b) =
+ *  (A * B) * p^2 + (a * B + A * b) * p + (a * b)
+ *
+ *  low result = (a * B + A * b) * p + (a * b)
+ *  high result = (A * B) + carry out from low result.
+ *
+ * As long as p is at most half the capacity of the underlying type, no
+ * individual multiplication will overflow. We just have to carefully  
manage

+ * carries to avoid losing any during the addition steps.
+ */
+template 
+static constexpr std::enable_if_t
+mulUnsigned(std::make_unsigned_t , std::make_unsigned_t ,
+std::make_unsigned_t val_a, std::make_unsigned_t val_b)
+{
+low = val_a * val_b;
+
+uint64_t A = (uint32_t)(val_a >> 32);
+uint64_t a = (uint32_t)val_a;
+uint64_t B = (uint32_t)(val_b >> 32);
+uint64_t b = (uint32_t)val_b;
+
+uint64_t c1 = 0, c2 = 0; // Carry between place values.
+uint64_t ab = a * b, Ab = A * b, aB = a * B, AB = A * B;
+
+c1 = (uint32_t)(ab >> 32);
+
+// Be careful to avoid overflow.
+c2 = (c1 >> 1) + (Ab >> 1) + (aB >> 1);
+c2 += ((c1 & 0x1) + (Ab & 0x1) + (aB & 0x1)) >> 1;
+c2 >>= 31;
+
+high = AB + c2;
+}
+
+/**
+ * @ingroup api_base_utils
+ */
+template 
+static constexpr std::enable_if_t
+mulSigned(std::make_signed_t , std::make_signed_t ,
+  std::make_signed_t val_a, std::make_signed_t val_b)
+{
+uint64_t u_high = 0, u_low = 0;
+mulUnsigned(u_high, u_low, val_a, val_b);
+
+if (val_a < 0)
+u_high -= val_b;
+if (val_b < 0)
+u_high -= val_a;
+
+high = u_high;
+low = u_low;
+}
+
+/**
  * This function is used to align addresses in memory.
  *
  * @param val is the address to be aligned.
diff --git a/src/base/intmath.test.cc b/src/base/intmath.test.cc
index 3baa502..4eb8aaf 100644
--- a/src/base/intmath.test.cc
+++ b/src/base/intmath.test.cc
@@ -112,6 +112,132 @@
 EXPECT_EQ(46, divCeil(451, 10));
 }

+TEST(IntmathTest, mulUnsignedNarrow)
+{
+uint8_t a = 0xff;
+uint8_t b = 0x02;
+uint8_t hi;
+uint8_t low;
+mulUnsigned(hi, low, a, b);
+EXPECT_EQ(hi, 0x1);
+EXPECT_EQ(low, 0xfe);
+
+a = 14;
+b = 9;
+mulUnsigned(hi, low, a, b);
+EXPECT_EQ(hi, 0);
+EXPECT_EQ(low, 0x7e);
+
+a = 0;
+b = 0x55;
+mulUnsigned(hi, low, a, b);
+EXPECT_EQ(hi, 0);
+EXPECT_EQ(low, 0);
+}
+
+TEST(IntmathTest, mulSignedNarrow)
+{
+int8_t a = -0x80;
+int8_t b = -0x7f;
+int8_t hi;
+int8_t low;
+mulSigned(hi, low, a, b);
+EXPECT_EQ(hi, 0x3f);
+EXPECT_EQ(low, -0x80);
+
+a = 14;
+b = -9;
+mulSigned(hi, low, a, b);
+EXPECT_EQ(hi, -0x01);
+EXPECT_EQ(low, -0x7e);
+
+a = 0;
+b = -0x55;
+mulSigned(hi, low, a, b);
+EXPECT_EQ(hi, 0);
+EXPECT_EQ(low, 0);
+}
+
+TEST(IntmathTest, mulUnsignedMid)
+{
+uint32_t a = 0xULL;
+uint32_t b = 0x0002ULL;
+uint32_t hi;
+uint32_t low;
+

[gem5-dev] Change in gem5/gem5[develop]: base: Introduce versions of mul(Uns|S)igned which return two values.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: base: Introduce versions of mul(Uns|S)igned which return  
two values.

..

base: Introduce versions of mul(Uns|S)igned which return two values.

This makes code which needs to call lots of different sized multiplications.

Change-Id: Id0d28be4c304214171840e7916c2e90ecfcd3840
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42360
Maintainer: Gabe Black 
Tested-by: kokoro 
Reviewed-by: Daniel Carvalho 
---
M src/base/intmath.hh
M src/base/intmath.test.cc
2 files changed, 32 insertions(+), 0 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/base/intmath.hh b/src/base/intmath.hh
index 7acb889..4be4a3b 100644
--- a/src/base/intmath.hh
+++ b/src/base/intmath.hh
@@ -44,6 +44,7 @@
 #include 
 #include 
 #include 
+#include 

 #include "base/bitfield.hh"

@@ -223,6 +224,24 @@
 #endif
 }

+template 
+static constexpr std::pair,  
std::make_unsigned_t>

+mulUnsigned(std::make_unsigned_t val_a, std::make_unsigned_t val_b)
+{
+std::make_unsigned_t hi, low;
+mulUnsigned(hi, low, val_a, val_b);
+return {hi, low};
+};
+
+template 
+static constexpr std::pair, std::make_signed_t>
+mulSigned(std::make_signed_t val_a, std::make_signed_t val_b)
+{
+std::make_signed_t hi, low;
+mulSigned(hi, low, val_a, val_b);
+return {hi, low};
+};
+
 /**
  * This function is used to align addresses in memory.
  *
diff --git a/src/base/intmath.test.cc b/src/base/intmath.test.cc
index b7fb34a..e42a9a8 100644
--- a/src/base/intmath.test.cc
+++ b/src/base/intmath.test.cc
@@ -38,6 +38,7 @@
  */

 #include 
+#include 

 #include "base/intmath.hh"

@@ -220,6 +221,12 @@
 EXPECT_EQ(hi, 0x1);
 EXPECT_EQ(low, 0xfffe);

+hi = 0;
+low = 0;
+std::tie(hi, low) = mulUnsigned(a, b);
+EXPECT_EQ(hi, 0x1);
+EXPECT_EQ(low, 0xfffe);
+
 a = 0;
 b = 0x;
 mulUnsigned(hi, low, a, b);
@@ -243,6 +250,12 @@
 EXPECT_EQ(hi, 0x3fff);
 EXPECT_EQ(low, -0x8000);

+hi = 0;
+low = 0;
+std::tie(hi, low) = mulSigned(a, b);
+EXPECT_EQ(hi, 0x3fff);
+EXPECT_EQ(low, -0x8000);
+
 a = 0;
 b = -0x;
 mulSigned(hi, low, a, b);



7 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42360
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: Id0d28be4c304214171840e7916c2e90ecfcd3840
Gerrit-Change-Number: 42360
Gerrit-PatchSet: 9
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: kokoro 
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

[gem5-dev] Change in gem5/gem5[develop]: base: Teach gem5 how to use 128 bit types for multiplication.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: base: Teach gem5 how to use 128 bit types for  
multiplication.

..

base: Teach gem5 how to use 128 bit types for multiplication.

gcc provides __uint128_t and __int128_t types which represent 128 bit
wide unsigned and signed integers, respectively. We can detect that
extension and use it to perform wide multiplication which takes
advantage of the built in single multiply instruction on x86 hardware
without having to compute the value manually with 64 bit variables.

Since both gcc and clang should support this extension and the manual
version may not be exercised normally, this change also extends the
gtest for intmath so that it will explicitly run the manual versions of
these functions. On systems with the extension both versions will be
tested, and on other systems the manual version will be harmlessly
tested twice.

Change-Id: I32640679396584cd43bc91a3f7e649c6e6f94afa
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42359
Maintainer: Gabe Black 
Tested-by: kokoro 
Reviewed-by: Daniel Carvalho 
---
M src/base/intmath.hh
M src/base/intmath.test.cc
2 files changed, 47 insertions(+), 5 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/base/intmath.hh b/src/base/intmath.hh
index 308c33f..7acb889 100644
--- a/src/base/intmath.hh
+++ b/src/base/intmath.hh
@@ -135,7 +135,6 @@
 };

 /**
- * @ingroup api_base_utils
  * Multiply two values with place value p.
  *
  *  (A * p + a) * (B * p + b) =
@@ -150,8 +149,8 @@
  */
 template 
 static constexpr std::enable_if_t
-mulUnsigned(std::make_unsigned_t , std::make_unsigned_t ,
-std::make_unsigned_t val_a, std::make_unsigned_t val_b)
+mulUnsignedManual(std::make_unsigned_t , std::make_unsigned_t  
,
+  std::make_unsigned_t val_a, std::make_unsigned_t  
val_b)

 {
 low = val_a * val_b;

@@ -178,8 +177,22 @@
  */
 template 
 static constexpr std::enable_if_t
-mulSigned(std::make_signed_t , std::make_signed_t ,
-  std::make_signed_t val_a, std::make_signed_t val_b)
+mulUnsigned(std::make_unsigned_t , std::make_unsigned_t ,
+std::make_unsigned_t val_a, std::make_unsigned_t val_b)
+{
+#ifdef __SIZEOF_INT128__
+__uint128_t val = (__uint128_t)val_a * (__uint128_t)val_b;
+low = val;
+high = (val >> 64);
+#else
+mulUnsignedManual(high, low, val_a, val_b);
+#endif
+}
+
+template 
+static constexpr std::enable_if_t
+mulSignedManual(std::make_signed_t , std::make_signed_t ,
+std::make_signed_t val_a, std::make_signed_t val_b)
 {
 uint64_t u_high = 0, u_low = 0;
 mulUnsigned(u_high, u_low, val_a, val_b);
@@ -194,6 +207,23 @@
 }

 /**
+ * @ingroup api_base_utils
+ */
+template 
+static constexpr std::enable_if_t
+mulSigned(std::make_signed_t , std::make_signed_t ,
+  std::make_signed_t val_a, std::make_signed_t val_b)
+{
+#ifdef __SIZEOF_INT128__
+__int128_t val = (__int128_t)val_a * (__int128_t)val_b;
+low = val;
+high = (val >> 64);
+#else
+mulSignedManual(high, low, val_a, val_b);
+#endif
+}
+
+/**
  * This function is used to align addresses in memory.
  *
  * @param val is the address to be aligned.
diff --git a/src/base/intmath.test.cc b/src/base/intmath.test.cc
index 4eb8aaf..b7fb34a 100644
--- a/src/base/intmath.test.cc
+++ b/src/base/intmath.test.cc
@@ -214,6 +214,12 @@
 EXPECT_EQ(hi, 0x1);
 EXPECT_EQ(low, 0xfffe);

+hi = 0;
+low = 0;
+mulUnsignedManual(hi, low, a, b);
+EXPECT_EQ(hi, 0x1);
+EXPECT_EQ(low, 0xfffe);
+
 a = 0;
 b = 0x;
 mulUnsigned(hi, low, a, b);
@@ -231,6 +237,12 @@
 EXPECT_EQ(hi, 0x3fff);
 EXPECT_EQ(low, -0x8000);

+hi = 0;
+low = 0;
+mulSignedManual(hi, low, a, b);
+EXPECT_EQ(hi, 0x3fff);
+EXPECT_EQ(low, -0x8000);
+
 a = 0;
 b = -0x;
 mulSigned(hi, low, a, b);



7 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42359
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: I32640679396584cd43bc91a3f7e649c6e6f94afa
Gerrit-Change-Number: 42359
Gerrit-PatchSet: 9
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

[gem5-dev] Build failed in Jenkins: Compiler-Checks #61

[jenkins-no-reply--- via gem5-dev]

See 


Changes:

[alexandru.dutu] gpu-compute: Topology and driver changes for dGPU

[shingarov] arch-power: Refactor CR field generation

[shingarov] arch-power: Use 64-bit registers and operands

[Giacomo Travaglini] mem: CFI Flash Memory implementation

[Giacomo Travaglini] dev-arm: Turn flash1 into a CFI Flash Memory

[Giacomo Travaglini] tests: Arm FS regressions using new guest binaries

[Giacomo Travaglini] tests: Use MatchFileRegex vierifier for realview 
regressions

[Giacomo Travaglini] arch-arm: Move TLB stats update within the lookup method

[gabe.black] cpu: Merge the BaseDynInst and the BaseO3DynInst classes.

[gabe.black] cpu: Get rid of the unused eaSrcsReady method.

[gabe.black] cpu: De-templatize the BaseO3DynInst.

[gabe.black] arch-power: Fix power build.

[gabe.black] arch,cpu,sim: Move the null and nop StaticInstPtrs to their own 
files.

[gabe.black] scons: Stop piggy-backing on the default tool for default settings.

[gabe.black] scons: Move the "duplicate" setting into gem5_env_defaults.py.

[gabe.black] scons: Move MakeAction into gem5_scons.

[yuhsingw] systemc: Extend TlmBridges to 512 bits

[Giacomo Travaglini] configs: Remove Ruby on ARM warning

[Giacomo Travaglini] util: Fix cpt_upgrader format string

[Giacomo Travaglini] util: Remove unused package import

[odanrc] scons: Allow declaring dependencies of tags

[gabe.black] sim: Minor cleanup of the System class.

[gabe.black] base: Improve handling of thread IDs for remote GDB.

[gabe.black] arch-sparc: Wrap overly long lines in the decoder definition.

[gabe.black] arch-sparc: Fix some bit manipulation bugs.

[gabe.black] arch,mem: Use szext instead of sext as appropriate.

[gabe.black] sim: Don't needlessly recreate ISA types in InstRecord.

[gabe.black] arch,cpu: Separate printing and serialization of VecPredReg.

[gabe.black] arch: Collapse unused size parameter from "as" VecPredReg method.

[gabe.black] cpu: Use the built in << for VecReg and VecPredReg in ExeTrace.

[gabe.black] base: Streamline the "send" method of the BaseRemoteGDB class.

[gabe.black] base: Add a link to documentation in the remote GDB header file.

[gabe.black] cpu,sim: Set ThreadContext's ContextID right away.

[gabe.black] sim: Track ThreadContext-s in the Workload object.


--
Started by timer
Running as SYSTEM
Building in workspace 
Selected Git installation does not exist. Using Default
The recommended git tool is: NONE
No credentials specified
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url https://gem5.googlesource.com/public/gem5 # 
 > timeout=10
Fetching upstream changes from https://gem5.googlesource.com/public/gem5
 > git --version # timeout=10
 > git --version # 'git version 2.25.1'
 > git fetch --tags --force --progress -- 
 > https://gem5.googlesource.com/public/gem5 
 > +refs/heads/*:refs/remotes/origin/* # timeout=10
 > git rev-parse refs/remotes/origin/develop^{commit} # timeout=10
Checking out Revision 2f424f6a13557003743eb8d02bd093a1dd2b6542 
(refs/remotes/origin/develop)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f 2f424f6a13557003743eb8d02bd093a1dd2b6542 # timeout=10
Commit message: "sim: Track ThreadContext-s in the Workload object."
 > git rev-list --no-walk dabb0c8f45d810683427e2be9068ea0d64ee1e95 # timeout=10
[Compiler-Checks] $ /bin/sh -xe /tmp/jenkins5548733731142465391.sh
+ ./util/compiler-tests.sh -j 4
Starting build tests with 'gcc-version-10'...
'gcc-version-10' was found in the comprehensive tests. All ISAs will be built.
  * Building target 'POWER.opt' with 'gcc-version-10'...
Done.
  * Building target 'POWER.fast' with 'gcc-version-10'...
Done.
  * Building target 'ARM_MESI_Three_Level.opt' with 'gcc-version-10'...
Done.
  * Building target 'ARM_MESI_Three_Level.fast' with 'gcc-version-10'...
Done.
  * Building target 'NULL_MOESI_CMP_directory.opt' with 'gcc-version-10'...
Done.
  * Building target 'NULL_MOESI_CMP_directory.fast' with 'gcc-version-10'...
Done.
  * Building target 'X86.opt' with 'gcc-version-10'...
Done.
  * Building target 'X86.fast' with 'gcc-version-10'...
Done.
  * Building target 'RISCV.opt' with 'gcc-version-10'...
Done.
  * Building target 'RISCV.fast' with 'gcc-version-10'...
Done.
  * Building target 'X86_MOESI_AMD_Base.opt' with 'gcc-version-10'...
Done.
  * Building target 'X86_MOESI_AMD_Base.fast' with 'gcc-version-10'...
Done.
  * Building target 'MIPS.opt' with 'gcc-version-10'...
Done.
  * Building target 'MIPS.fast' with 'gcc-version-10'...
Done.
  * Building target 'NULL_MOESI_CMP_token.opt' with 'gcc-version-10'...
Done.
  * Building target 'NULL_MOESI_CMP_token.fast' with 'gcc-version-10'...
Done.
  * Building target 'NULL_MESI_Two_Level.opt' 

[gem5-dev] Change in gem5/gem5[develop]: base: Check the context ID when replacing a ThreadContext in GDB.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: base: Check the context ID when replacing a ThreadContext  
in GDB.

..

base: Check the context ID when replacing a ThreadContext in GDB.

This says *which* thread context you're replacing. Right now it's
implied that you're replacing the only thread context, but once we
support having multiple threads in the same GDB endpoint, that will no
longer be implied.

Change-Id: I5a789d12bbe195e019d5ccd8a005b5a6f16b9299
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/44610
Maintainer: Gabe Black 
Tested-by: kokoro 
Reviewed-by: Daniel Carvalho 
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 10 insertions(+), 1 deletion(-)

Approvals:
  Daniel Carvalho: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 16373e6..efda8e8 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -141,6 +141,7 @@

 #include "base/cprintf.hh"
 #include "base/intmath.hh"
+#include "base/logging.hh"
 #include "base/socket.hh"
 #include "base/trace.hh"
 #include "cpu/base.hh"
@@ -436,6 +437,14 @@
 DPRINTFN("remote gdb detached\n");
 }

+void
+BaseRemoteGDB::replaceThreadContext(ThreadContext *_tc)
+{
+ContextID id = _tc->contextId();
+panic_if(id != tc->contextId(), "No context with ID %d found.", id);
+tc = _tc;
+}
+
 // This function does all command processing for interfacing to a
 // remote gdb.  Note that the error codes are ignored by gdb at
 // present, but might eventually become meaningful. (XXX) It might
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index 211bf2e..ba274e6 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -163,7 +163,7 @@
 void detach();
 bool isAttached() { return attached; }

-void replaceThreadContext(ThreadContext *_tc) { tc = _tc; }
+void replaceThreadContext(ThreadContext *_tc);

 bool trap(int type);




3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44610
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: I5a789d12bbe195e019d5ccd8a005b5a6f16b9299
Gerrit-Change-Number: 44610
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: kokoro 
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

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Simplify the "mult" SIMD instructions with a BitUnion.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: arch-arm: Simplify the "mult" SIMD instructions with a  
BitUnion.

..

arch-arm: Simplify the "mult" SIMD instructions with a BitUnion.

These instructions go through a lot of effort to extract bitfields, sign
extend them, and cast things to an appropriate type/size.

Instead, we can define a BitUnion which has the appropriate ranges of
bits predefined, and take advantage of the fact that every bitfield
returns its value as either a uint64_t if it's unsigned, or an int64_t
if it's signed.

Also, stop setting resTemp if it's not going to be used to set condition
codes or used as an intermediate in calculating the destination
registers.

Change-Id: Ia511aa74c823fad48080de4fbf77791c0cb3309d
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42387
Reviewed-by: Giacomo Travaglini 
Maintainer: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/arch/arm/isa/insts/mult.isa
M src/arch/arm/isa/operands.isa
M src/arch/arm/regs/int.hh
3 files changed, 179 insertions(+), 256 deletions(-)

Approvals:
  Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/isa/insts/mult.isa  
b/src/arch/arm/isa/insts/mult.isa

index 263be9f..528aa66 100644
--- a/src/arch/arm/isa/insts/mult.isa
+++ b/src/arch/arm/isa/insts/mult.isa
@@ -127,260 +127,165 @@
 def buildMult4InstUnCc(mnem, code, flagType = "logic"):
 buildMultInst(mnem, False, True, 4, code, flagType)

-buildMult4Inst("mla", "Reg0 = resTemp = Reg1 * Reg2 + Reg3;")
-buildMult4InstUnCc("mls", "Reg0 = resTemp = Reg3 - Reg1 * Reg2;")
-buildMult3Inst("mul", "Reg0 = resTemp = Reg1 * Reg2;")
-buildMult4InstCc  ("smlabb", '''Reg0 = resTemp =
-sext<16>(bits(Reg1, 15, 0)) *
-sext<16>(bits(Reg2_sw, 15, 0)) +
-Reg3_sw;
- resTemp = bits(resTemp, 32) !=
-   bits(resTemp, 31);
- ''', "overflow")
-buildMult4InstCc  ("smlabt", '''Reg0 = resTemp =
-sext<16>(bits(Reg1, 15, 0)) *
-sext<16>(bits(Reg2_sw, 31, 16)) +
-Reg3_sw;
- resTemp = bits(resTemp, 32) !=
-   bits(resTemp, 31);
- ''', "overflow")
-buildMult4InstCc  ("smlatb", '''Reg0 = resTemp =
-sext<16>(bits(Reg1, 31, 16)) *
-sext<16>(bits(Reg2_sw, 15, 0)) +
-Reg3_sw;
- resTemp = bits(resTemp, 32) !=
-   bits(resTemp, 31);
- ''', "overflow")
-buildMult4InstCc  ("smlatt", '''Reg0 = resTemp =
-sext<16>(bits(Reg1, 31, 16)) *
-sext<16>(bits(Reg2_sw, 31, 16)) +
-Reg3_sw;
- resTemp = bits(resTemp, 32) !=
-   bits(resTemp, 31);
- ''', "overflow")
-buildMult4InstCc  ("smlad", '''Reg0 = resTemp =
-sext<16>(bits(Reg1, 31, 16)) *
-sext<16>(bits(Reg2, 31, 16)) +
-sext<16>(bits(Reg1, 15, 0)) *
-sext<16>(bits(Reg2, 15, 0)) +
-Reg3_sw;
-resTemp = bits(resTemp, 32) !=
-  bits(resTemp, 31);
-''', "overflow")
-buildMult4InstCc  ("smladx", '''Reg0 = resTemp =
- sext<16>(bits(Reg1, 31, 16)) *
- sext<16>(bits(Reg2, 15, 0)) +
- sext<16>(bits(Reg1, 15, 0)) *
- sext<16>(bits(Reg2, 31, 16)) +
- Reg3_sw;
-resTemp = bits(resTemp, 32) !=
-  bits(resTemp, 31);
- ''', "overflow")
-buildMult4Inst("smlal", '''resTemp = sext<32>(Reg2) *  
sext<32>(Reg3) +
-   (int64_t)((Reg1_ud << 32) |  
Reg0_ud);

-   Reg0_ud = (uint32_t)resTemp;
-   Reg1_ud = 

[gem5-dev] Change in gem5/gem5[develop]: sim: Add pool specific allocators to SE mode

[Alex Dutu (Gerrit) via gem5-dev]

Alex Dutu has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42215 )


Change subject: sim: Add pool specific allocators to SE mode
..

sim: Add pool specific allocators to SE mode

The System object supports multiple memory pools but there is currently
no way to specify which pool to allocate from in SE mode.  This patch
adds a optional poolID argument to the allocation functions.

Change-Id: I1c732f28905f3b3875adee5f2b0d9abb39a6c5d1
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42215
Reviewed-by: Matt Sinclair 
Maintainer: Matt Sinclair 
Tested-by: kokoro 
---
M src/sim/SConscript
A src/sim/mem_pool.cc
A src/sim/mem_pool.hh
M src/sim/system.cc
M src/sim/system.hh
5 files changed, 234 insertions(+), 27 deletions(-)

Approvals:
  Matt Sinclair: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/sim/SConscript b/src/sim/SConscript
index fb87bfe..700f702 100644
--- a/src/sim/SConscript
+++ b/src/sim/SConscript
@@ -84,6 +84,7 @@
 Source('power_domain.cc')
 Source('stats.cc')
 Source('workload.cc')
+Source('mem_pool.cc')

 GTest('byteswap.test', 'byteswap.test.cc', '../base/types.cc')
 GTest('guest_abi.test', 'guest_abi.test.cc')
diff --git a/src/sim/mem_pool.cc b/src/sim/mem_pool.cc
new file mode 100644
index 000..87132dc
--- /dev/null
+++ b/src/sim/mem_pool.cc
@@ -0,0 +1,116 @@
+/*
+ * Copyright (c) 2011-2021 Advanced Micro Devices, Inc.
+ * All rights reserved.
+ *
+ * For use for simulation and test purposes only
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are  
met:

+ *
+ * 1. Redistributions of source code must retain the above copyright  
notice,

+ * this list of conditions and the following disclaimer.
+ *
+ * 2. 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.
+ *
+ * 3. Neither the name of the copyright holder 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 HOLDER 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.
+ */
+
+#include "sim/mem_pool.hh"
+
+#include "sim/system.hh"
+
+MemPool::MemPool(System *system, Addr ptr, Addr limit)
+: sys(system), freePageNum(ptr >> sys->getPageShift()),
+_totalPages(limit >> sys->getPageShift())
+{
+}
+
+Counter
+MemPool::freePage() const
+{
+return freePageNum;
+}
+
+void
+MemPool::setFreePage(Counter value)
+{
+freePageNum = value;
+}
+
+Addr
+MemPool::freePageAddr() const
+{
+return freePageNum << sys->getPageShift();
+}
+
+Counter
+MemPool::totalPages() const
+{
+return _totalPages;
+}
+
+Counter
+MemPool::allocatedPages() const
+{
+return freePageNum;
+}
+
+Counter
+MemPool::freePages() const
+{
+return _totalPages - freePageNum;
+}
+
+Addr
+MemPool::allocatedBytes() const
+{
+return allocatedPages() << sys->getPageShift();
+}
+
+Addr
+MemPool::freeBytes() const
+{
+return freePages() << sys->getPageShift();
+}
+
+Addr
+MemPool::totalBytes() const
+{
+return totalPages() << sys->getPageShift();
+}
+
+Addr
+MemPool::allocate(Addr npages)
+{
+Addr return_addr = freePageAddr();
+freePageNum += npages;
+Addr next_return_addr = freePageAddr();
+
+if (sys->m5opRange().valid() &&
+next_return_addr >= sys->m5opRange().start()) {
+warn("Reached m5ops MMIO region\n");
+return_addr = sys->m5opRange().end();
+freePageNum = (return_addr >> sys->getPageShift()) + npages;
+}
+
+fatal_if((freePages() <= 0), "Out of memory, "
+ "please increase size of physical memory.");
+
+return return_addr;
+}
diff --git a/src/sim/mem_pool.hh b/src/sim/mem_pool.hh
new file mode 100644
index 000..2c99ebc
--- /dev/null
+++ b/src/sim/mem_pool.hh
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2011-2021 Advanced Micro Devices, Inc.
+ * All rights reserved.
+ *
+ * For use for simulation 

[gem5-dev] Re: Gem5-GCN3 Checkpointing Support

[Poremba, Matthew via gem5-dev]

[AMD Public Use]

Hi Paul,


Checkpoints (and similarly switchcpu) in GCN3 do not work once the GPU has 
launched a kernel because the drain functions are not implemented in the GPU 
model.  In general, there is a lot of state to be drained for the GPU.  If you 
drain/serialize between kernels it would probably be the easiest.  This has 
been low priority for us since we've found it sufficient to use KVM 
fast-forward to get to the first kernel and then switch to a timing CPU.

The use case for most users is to simulate single kernel or a handful of 
kernels at a time, so for that starting from the beginning of the application 
and exiting after a certain number of kernels is an unreasonable amount of time.

If the only concern is to avoid the time simulating the runtime initialization 
(before first kernel launch) there is a trick you can do to checkpoint or 
switchcpus (e.g., from KVM) before it launches.



MATTHEW POREMBA
MTS Silicon Design Engineer  |  AMD
AMD Research
O +(1) 425-586-6472  C +(1) 425-518-1014
--
2002 156th Ave NE, Suite 300, Bellevue, WA 98007
Facebook |  Twitter |  
amd.com
[cid:image001.png@01D73760.A70500B0]



From: Jason Lowe-Power via gem5-dev 
Sent: Thursday, April 22, 2021 8:34 AM
To: gem5-dev@gem5.org
Cc: Tschirhart, Paul K [US] (MS) ; Jason Lowe-Power 

Subject: [gem5-dev] Re: Gem5-GCN3 Checkpointing Support

[CAUTION: External Email]
Hi Paul,

I've included gem5-dev mailing list here. If you're not subscribed, I would 
suggest sending these kinds of questions there where the gem5-gcn/AMD 
developers will see things.

To answer your question, adding checkpointing should be relatively 
straightforward. The main changes are exactly as you described: saving the 
architectural state of the GPU threads. There are probably a few other pieces 
of GPU state that need to be saved too (e.g., the control processor, etc.). 
Hopefully one of the devs at AMD can reply with more details.

You'll have to add the serialize/unserialize functions, and you'll probably 
have to also implement a drain() function to flush out the current in-progress 
instructions. I imagine you'll at least want to finish the in-progress 
wavefronts, and you may want to wait until the currently scheduled workgroups 
are finished as well.

As far as Ruby goes, as long as the protocol you're using support 
checkpointing, it will "just work". To support checkpointing, I believe all the 
protocol needs to do is implement the flush RubyRequestMsg. I'm not sure if 
VIPER supports this or not.

Finally, the only other detail is whether or not the current implementation of 
SE mode fully supports checkpointing. Right now, this isn't something we 
regularly test, so it's possible that there are some details that are broken.

Hopefully the current devs at AMD will correct me where I'm wrong! Let us know 
on gem5-dev if there's any questions we can answer, etc. We would greatly 
appreciate this contribution!

Cheers,
Jason

On Wed, Apr 21, 2021 at 6:55 PM Tschirhart, Paul K [US] (MS) 
mailto:paul.tschirh...@ngc.com>> wrote:
Hello Professor Lowe-Power,

Thank you for your helpful replies to the emails from some other members of my 
group regarding various aspects of the Gem5 simulator.

I have been working with Gem5-GCN3 and I was wondering if you knew anything 
about the status of checkpointing support for that model. I saw in a post that 
adding checkpoints was something that was planned but I have not seen anything 
since.

Is there a significant technical challenge involved with expanding Gem5's 
checkpointing mechanism to support the GCN3 model or is this just a matter of 
writing the necessary serialize/unserialize functions? In other words, is this 
something that someone with experience making significant modifications to Gem5 
might be able to tackle by implementing an approach that is similar to the one 
used in the O3 model?

If the modifications should be mostly straightforward , do you know of anything 
that needs to be done besides adding the functions to serialize/unserialize 
threads in the GCN3 model? It seems like modifications might be required to 
support checkpointing for the VIPER protocol in Ruby but I don't see where I 
need to make the changes. Am I missing something either in Ruby or elsewhere in 
the simulator?

Thanks again for all of your help.

Paul



--
Jason Lowe-Power (he/him/his)
Assistant Professor, Computer Science Department
University of California, Davis
3049 Kemper Hall

[gem5-dev] Re: Gem5-GCN3 Checkpointing Support

[Jason Lowe-Power via gem5-dev]

Hi Paul,

I've included gem5-dev mailing list here. If you're not subscribed, I would
suggest sending these kinds of questions there where the gem5-gcn/AMD
developers will see things.

To answer your question, adding checkpointing should be relatively
straightforward. The main changes are exactly as you described: saving the
architectural state of the GPU threads. There are probably a few other
pieces of GPU state that need to be saved too (e.g., the control processor,
etc.). Hopefully one of the devs at AMD can reply with more details.

You'll have to add the serialize/unserialize functions, and you'll probably
have to also implement a drain() function to flush out the current
in-progress instructions. I imagine you'll at least want to finish the
in-progress wavefronts, and you may want to wait until the currently
scheduled workgroups are finished as well.

As far as Ruby goes, as long as the protocol you're using support
checkpointing, it will "just work". To support checkpointing, I believe all
the protocol needs to do is implement the flush RubyRequestMsg. I'm not
sure if VIPER supports this or not.

Finally, the only other detail is whether or not the current implementation
of SE mode fully supports checkpointing. Right now, this isn't something we
regularly test, so it's possible that there are some details that are
broken.

Hopefully the current devs at AMD will correct me where I'm wrong! Let us
know on gem5-dev if there's any questions we can answer, etc. We would
greatly appreciate this contribution!

Cheers,
Jason

On Wed, Apr 21, 2021 at 6:55 PM Tschirhart, Paul K [US] (MS) <
paul.tschirh...@ngc.com> wrote:

> Hello Professor Lowe-Power,
>
>
>
> Thank you for your helpful replies to the emails from some other members
> of my group regarding various aspects of the Gem5 simulator.
>
>
>
> I have been working with Gem5-GCN3 and I was wondering if you knew
> anything about the status of checkpointing support for that model. I saw in
> a post that adding checkpoints was something that was planned but I have
> not seen anything since.
>
>
>
> Is there a significant technical challenge involved with expanding Gem5’s
> checkpointing mechanism to support the GCN3 model or is this just a matter
> of writing the necessary serialize/unserialize functions? In other words,
> is this something that someone with experience making significant
> modifications to Gem5 might be able to tackle by implementing an approach
> that is similar to the one used in the O3 model?
>
>
>
> If the modifications should be mostly straightforward , do you know of
> anything that needs to be done besides adding the functions to
> serialize/unserialize threads in the GCN3 model? It seems like
> modifications might be required to support checkpointing for the VIPER
> protocol in Ruby but I don’t see where I need to make the changes. Am I
> missing something either in Ruby or elsewhere in the simulator?
>
>
>
> Thanks again for all of your help.
>
>
>
> Paul
>
>
>


-- 
Jason Lowe-Power (he/him/his)
Assistant Professor, Computer Science Department
University of California, Davis
3049 Kemper Hall
https://arch.cs.ucdavis.edu/
___
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

[gem5-dev] Change in gem5/gem5[develop]: cpu: Simplify the RegId class a little.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: cpu: Simplify the RegId class a little.
..

cpu: Simplify the RegId class a little.

Having const and non const reference accessors for the RegId index are
basically the same thing as just making the index value public but with
more complexity. Stop allowing updates through the accessor, and
simplify/fix the one location that was using that.

Also, there is no good reason to return an integer value by const
reference instead of returning it by value, since the value being passed
around (a pointer) is the same size, and just makes the value harder to
access.

Change-Id: I377ffc5878ef9bffa2ac53626a87c019a585ab1a
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42684
Reviewed-by: Giacomo Travaglini 
Maintainer: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/cpu/o3/cpu.cc
M src/cpu/reg_class.hh
2 files changed, 10 insertions(+), 17 deletions(-)

Approvals:
  Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/cpu/o3/cpu.cc b/src/cpu/o3/cpu.cc
index 04a9ee7..6b1ebee 100644
--- a/src/cpu/o3/cpu.cc
+++ b/src/cpu/o3/cpu.cc
@@ -773,29 +773,23 @@
 //Bind Int Regs to Rename Map
 const auto  = isa[tid]->regClasses();

-for (RegId reg_id(IntRegClass, 0);
-reg_id.index() < regClasses.at(IntRegClass).size();
-reg_id.index()++) {
+for (RegIndex idx = 0; idx < regClasses.at(IntRegClass).size(); idx++)  
{

 PhysRegIdPtr phys_reg = freeList.getIntReg();
-renameMap[tid].setEntry(reg_id, phys_reg);
+renameMap[tid].setEntry(RegId(IntRegClass, idx), phys_reg);
 scoreboard.setReg(phys_reg);
 }

 //Bind Float Regs to Rename Map
-for (RegId reg_id(FloatRegClass, 0);
-reg_id.index() < regClasses.at(FloatRegClass).size();
-reg_id.index()++) {
+for (RegIndex idx = 0; idx < regClasses.at(FloatRegClass).size();  
idx++) {

 PhysRegIdPtr phys_reg = freeList.getFloatReg();
-renameMap[tid].setEntry(reg_id, phys_reg);
+renameMap[tid].setEntry(RegId(FloatRegClass, idx), phys_reg);
 scoreboard.setReg(phys_reg);
 }

 //Bind condition-code Regs to Rename Map
-for (RegId reg_id(CCRegClass, 0);
-reg_id.index() < regClasses.at(CCRegClass).size();
-reg_id.index()++) {
+for (RegIndex idx = 0; idx < regClasses.at(CCRegClass).size(); idx++) {
 PhysRegIdPtr phys_reg = freeList.getCCReg();
-renameMap[tid].setEntry(reg_id, phys_reg);
+renameMap[tid].setEntry(RegId(CCRegClass, idx), phys_reg);
 scoreboard.setReg(phys_reg);
 }

diff --git a/src/cpu/reg_class.hh b/src/cpu/reg_class.hh
index 84b513c..37207a2 100644
--- a/src/cpu/reg_class.hh
+++ b/src/cpu/reg_class.hh
@@ -168,13 +168,12 @@

 /** Index accessors */
 /** @{ */
-const RegIndex& index() const { return regIdx; }
-RegIndex& index() { return regIdx; }
+RegIndex index() const { return regIdx; }

 /** Index flattening.
  * Required to be able to use a vector for the register mapping.
  */
-inline RegIndex
+RegIndex
 flatIndex() const
 {
 switch (regClass) {
@@ -194,9 +193,9 @@
 /** @} */

 /** Elem accessor */
-const RegIndex& elemIndex() const { return elemIdx; }
+RegIndex elemIndex() const { return elemIdx; }
 /** Class accessor */
-const RegClass& classValue() const { return regClass; }
+RegClass classValue() const { return regClass; }
 /** Return a const char* with the register class name. */
 const char* className() const { return regClassStrings[regClass]; }




10 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42684
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: I377ffc5878ef9bffa2ac53626a87c019a585ab1a
Gerrit-Change-Number: 42684
Gerrit-PatchSet: 12
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
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

[gem5-dev] Change in gem5/gem5[develop]: cpu: Eliminate the isZeroReg() helper in RegId.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: cpu: Eliminate the isZeroReg() helper in RegId.
..

cpu: Eliminate the isZeroReg() helper in RegId.

The isZeroReg() helper checked if the register was both an integer
register, and if it equaled TheISA::ZeroReg. This bakes in both the
assumption that any zero registers are integer (and that integer
registers are a thing), and also internalizes the plumbing which selects
what index is the zero register.

This change eliminates the isZeroReg helper and moves the logic inside
it into where it was called. In most cases, it was actually not
necessary to check if the register was integer since that was already
implied by context. This also brings the TheISA::ZeroReg constant out,
where it can be replaced by values plumbed in more generally than a
fixed, ISA specific constant.

Change-Id: I651762b6eb01fea83ec0b0076e8be9bf24b5b0da
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42683
Reviewed-by: Giacomo Travaglini 
Maintainer: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/cpu/minor/dyn_inst.cc
M src/cpu/minor/scoreboard.cc
M src/cpu/o3/probe/elastic_trace.cc
M src/cpu/o3/regfile.hh
M src/cpu/o3/rename_map.cc
M src/cpu/o3/scoreboard.hh
M src/cpu/reg_class.hh
7 files changed, 45 insertions(+), 57 deletions(-)

Approvals:
  Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/cpu/minor/dyn_inst.cc b/src/cpu/minor/dyn_inst.cc
index 1e34a88..ceac894 100644
--- a/src/cpu/minor/dyn_inst.cc
+++ b/src/cpu/minor/dyn_inst.cc
@@ -164,7 +164,7 @@
   static_cast(reg.elemIndex()) << ']';
 break;
   case IntRegClass:
-if (reg.isZeroReg()) {
+if (reg.index() == TheISA::ZeroReg) {
 os << 'z';
 } else {
 os << 'r' << static_cast(reg.index());
diff --git a/src/cpu/minor/scoreboard.cc b/src/cpu/minor/scoreboard.cc
index 0d943f1..5f8df87 100644
--- a/src/cpu/minor/scoreboard.cc
+++ b/src/cpu/minor/scoreboard.cc
@@ -50,41 +50,39 @@
 {
 bool ret = false;

-if (reg.isZeroReg()) {
-/* Don't bother with the zero register */
-ret = false;
-} else {
-switch (reg.classValue())
-{
-  case IntRegClass:
+switch (reg.classValue()) {
+  case IntRegClass:
+if (reg.index() == TheISA::ZeroReg) {
+/* Don't bother with the zero register */
+ret = false;
+} else {
 scoreboard_index = reg.index();
 ret = true;
-break;
-  case FloatRegClass:
-scoreboard_index = floatRegOffset + reg.index();
-ret = true;
-break;
-  case VecRegClass:
-  case VecElemClass:
-scoreboard_index = vecRegOffset + reg.index();
-ret = true;
-break;
-  case VecPredRegClass:
-scoreboard_index = vecPredRegOffset + reg.index();
-ret = true;
-break;
-  case CCRegClass:
-scoreboard_index = ccRegOffset + reg.index();
-ret = true;
-break;
-  case MiscRegClass:
-  /* Don't bother with Misc registers */
-ret = false;
-break;
-  default:
-panic("Unknown register class: %d",
-static_cast(reg.classValue()));
 }
+break;
+  case FloatRegClass:
+scoreboard_index = floatRegOffset + reg.index();
+ret = true;
+break;
+  case VecRegClass:
+  case VecElemClass:
+scoreboard_index = vecRegOffset + reg.index();
+ret = true;
+break;
+  case VecPredRegClass:
+scoreboard_index = vecPredRegOffset + reg.index();
+ret = true;
+break;
+  case CCRegClass:
+scoreboard_index = ccRegOffset + reg.index();
+ret = true;
+break;
+  case MiscRegClass:
+  /* Don't bother with Misc registers */
+ret = false;
+break;
+  default:
+panic("Unknown register class: %d", reg.classValue());
 }

 return ret;
diff --git a/src/cpu/o3/probe/elastic_trace.cc  
b/src/cpu/o3/probe/elastic_trace.cc

index ddea041..b48504a 100644
--- a/src/cpu/o3/probe/elastic_trace.cc
+++ b/src/cpu/o3/probe/elastic_trace.cc
@@ -241,7 +241,7 @@

 const RegId& src_reg = dyn_inst->srcRegIdx(src_idx);
 if (!src_reg.isMiscReg() &&
-!src_reg.isZeroReg()) {
+!(src_reg.isIntReg() && src_reg.index() ==  
TheISA::ZeroReg)) {

 // Get the physical register index of the i'th source register.
 PhysRegIdPtr phys_src_reg =  
dyn_inst->regs.renamedSrcIdx(src_idx);

 DPRINTFR(ElasticTrace, "[sn:%lli] Check map for src reg"
@@ -273,7 +273,8 @@
 // CC register and not a 

[gem5-dev] Change in gem5/gem5[develop]: sim: Trap into GDB instead of panicking on SEGV

[Boris Shingarov (Gerrit) via gem5-dev]

Boris Shingarov has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/44685 )


Change subject: sim: Trap into GDB instead of panicking on SEGV
..

sim: Trap into GDB instead of panicking on SEGV

When a segfault happens in the guest, report a SEGV trap to GDB (if
there is one attached) instead of bailing out immediately.

The obvious use-case for this, is the ability to debug guest crashes
in GDB in the standard manner.

The less-trivial use-case is for development of software in an
incomplete software stack (cf. Aarno-Engblom's "Virtual Platforms"
pp.105 et seq.)  One particular example is Ingalls-Miranda simulation of
JIT compilers, where the VM's address space may be split between the
simulated and the real machine: in this case, GDB traps facilitate the
transparent illusion of an unbroken address space.

Change-Id: I9072ed5f6474e05e9a99dc42ae5754be28121355
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/44685
Reviewed-by: Gabe Black 
Reviewed-by: Andreas Sandberg 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/arch/null/remote_gdb.hh
M src/sim/faults.cc
M src/sim/system.cc
M src/sim/system.hh
4 files changed, 21 insertions(+), 4 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved
  Gabe Black: Looks good to me, but someone else must approve; Looks good  
to me, approved

  kokoro: Regressions pass



diff --git a/src/arch/null/remote_gdb.hh b/src/arch/null/remote_gdb.hh
index 4df9cc8..c47ca9b 100644
--- a/src/arch/null/remote_gdb.hh
+++ b/src/arch/null/remote_gdb.hh
@@ -47,6 +47,7 @@

 bool breakpoint() { return false; }
 void replaceThreadContext(ThreadContext *tc) {}
+bool trap(int type) { return true; }

 virtual ~BaseRemoteGDB() {}
 };
diff --git a/src/sim/faults.cc b/src/sim/faults.cc
index 501b5d1..e4f4ae1 100644
--- a/src/sim/faults.cc
+++ b/src/sim/faults.cc
@@ -40,6 +40,8 @@

 #include "sim/faults.hh"

+#include 
+
 #include "arch/decoder.hh"
 #include "arch/locked_mem.hh"
 #include "base/logging.hh"
@@ -94,15 +96,16 @@
 Process *p = tc->getProcessPtr();
 handled = p->fixupFault(vaddr);
 }
-panic_if(!handled, "Page table fault when accessing virtual  
address %#x",

- vaddr);
-
+panic_if(!handled &&
+ !tc->getSystemPtr()->trapToGdb(SIGSEGV, tc->contextId()),
+ "Page table fault when accessing virtual address %#x\n",  
vaddr);

 }

 void
 GenericAlignmentFault::invoke(ThreadContext *tc, const StaticInstPtr )
 {
-panic("Alignment fault when accessing virtual address %#x\n", vaddr);
+panic_if(!tc->getSystemPtr()->trapToGdb(SIGSEGV, tc->contextId()),
+ "Alignment fault when accessing virtual address %#x\n",  
vaddr);

 }

 void GenericHtmFailureFault::invoke(ThreadContext *tc,
diff --git a/src/sim/system.cc b/src/sim/system.cc
index 9fd312c..8b23f90 100644
--- a/src/sim/system.cc
+++ b/src/sim/system.cc
@@ -495,6 +495,16 @@
 lastWorkItemStarted.erase(p);
 }

+bool
+System::trapToGdb(int signal, ContextID ctx_id) const
+{
+auto *gdb = threads.thread(ctx_id).gdb;
+if (!gdb)
+return false;
+gdb->trap(signal);
+return true;
+}
+
 void
 System::printSystems()
 {
diff --git a/src/sim/system.hh b/src/sim/system.hh
index 6613217..acdb316 100644
--- a/src/sim/system.hh
+++ b/src/sim/system.hh
@@ -561,6 +561,9 @@

 void workItemEnd(uint32_t tid, uint32_t workid);

+/* Returns whether we successfully trapped into GDB. */
+bool trapToGdb(int signal, ContextID ctx_id) const;
+
   protected:
 /**
  * Range for memory-mapped m5 pseudo ops. The range will be

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44685
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: I9072ed5f6474e05e9a99dc42ae5754be28121355
Gerrit-Change-Number: 44685
Gerrit-PatchSet: 4
Gerrit-Owner: Boris Shingarov 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Boris Shingarov 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
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