[gem5-dev] Change in gem5/gem5[develop]: base: Add a link to documentation in the remote GDB header file.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: base: Add a link to documentation in the remote GDB header  
file.

..

base: Add a link to documentation in the remote GDB header file.

Change-Id: I34bf4d24e58e6dfc8e8d1220a158c90fd0935e47
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/44608
Reviewed-by: Daniel Carvalho 
Reviewed-by: Boris Shingarov 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/base/remote_gdb.hh
1 file changed, 7 insertions(+), 0 deletions(-)

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



diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index cfd6b3d..211bf2e 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -58,6 +58,13 @@
 #include "cpu/pc_event.hh"
 #include "sim/eventq.hh"

+/*
+ * This file implements a client for the GDB remote serial protocol as
+ * described in this official documentation:
+ *
+ * https://sourceware.org/gdb/current/onlinedocs/gdb/Remote-Protocol.html
+ */
+
 class System;
 class ThreadContext;




1 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/+/44608
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: I34bf4d24e58e6dfc8e8d1220a158c90fd0935e47
Gerrit-Change-Number: 44608
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Boris Shingarov 
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: Streamline the "send" method of the BaseRemoteGDB class.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: base: Streamline the "send" method of the BaseRemoteGDB  
class.

..

base: Streamline the "send" method of the BaseRemoteGDB class.

The existing send method takes a const char *, but frequently the class
wants to use it to send a std::string, also frequently after generating
that string with csprintf. Rather than force each call sight to add a
.c_str() and call csprintf, this change adds helpers which will accept a
std::string and call c_str for you, or accept a format const char * and
arguments and call csprintf for you (and then call .c_str() on the
result).

Change-Id: Ifcef5e09f6469322c6040374209972528c80fb25
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/44607
Reviewed-by: Daniel Carvalho 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 16 insertions(+), 7 deletions(-)

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 3fc5240..16373e6 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -466,7 +466,7 @@
 active = true;
 } else {
 // Tell remote host that an exception has occurred.
-send(csprintf("S%02x", type).c_str());
+send("S%02x", type);
 }

 // Stick frame regs into our reg cache.
@@ -506,7 +506,7 @@
 } catch (Unsupported ) {
 send("");
 } catch (CmdError ) {
-send(e.error.c_str());
+send(e.error);
 } catch (...) {
 panic("Unrecognzied GDB exception.");
 }
@@ -837,7 +837,7 @@
 bool
 BaseRemoteGDB::cmdSignal(GdbCommand::Context )
 {
-send(csprintf("S%02x", ctx.type).c_str());
+send("S%02x", ctx.type);
 return true;
 }

@@ -986,7 +986,7 @@
 void
 BaseRemoteGDB::queryC(QuerySetCommand::Context )
 {
-send(csprintf("QC%x", encodeThreadId(tc->contextId())).c_str());
+send("QC%x", encodeThreadId(tc->contextId()));
 }

 void
@@ -999,7 +999,7 @@
 oss << "PacketSize=1024";
 for (const auto& feature : availableFeatures())
 oss << ';' << feature;
-send(oss.str().c_str());
+send(oss.str());
 }

 void
@@ -1040,13 +1040,13 @@

 std::string encoded;
 encodeXferResponse(content, encoded, offset, length);
-send(encoded.c_str());
+send(encoded);
 }

 void
 BaseRemoteGDB::queryFThreadInfo(QuerySetCommand::Context )
 {
-send(csprintf("m%x", encodeThreadId(tc->contextId())).c_str());
+send("m%x", encodeThreadId(tc->contextId()));
 }

 void
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index 0d67b09..cfd6b3d 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -51,6 +51,7 @@
 #include 

 #include "arch/types.hh"
+#include "base/cprintf.hh"
 #include "base/pollevent.hh"
 #include "base/socket.hh"
 #include "base/types.hh"
@@ -203,6 +204,14 @@

 void recv(std::vector );
 void send(const char *data);
+void send(const std::string ) { send(data.c_str()); }
+
+template 
+void
+send(const char *format, const Args &...args)
+{
+send(csprintf(format, args...));
+}

 /*
  * Simulator side debugger state.



1 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/+/44607
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: Ifcef5e09f6469322c6040374209972528c80fb25
Gerrit-Change-Number: 44607
Gerrit-PatchSet: 3
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: Collapse unused size parameter from "as" VecPredReg method.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: arch: Collapse unused size parameter from "as" VecPredReg  
method.

..

arch: Collapse unused size parameter from "as" VecPredReg method.

Change-Id: Ibdaf38b2e2d8f37ef76d6b8874ac3620982e78a2
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42000
Reviewed-by: Giacomo Travaglini 
Maintainer: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/arch/generic/vec_pred_reg.hh
1 file changed, 14 insertions(+), 21 deletions(-)

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



diff --git a/src/arch/generic/vec_pred_reg.hh  
b/src/arch/generic/vec_pred_reg.hh

index 3e248b4..54228a1 100644
--- a/src/arch/generic/vec_pred_reg.hh
+++ b/src/arch/generic/vec_pred_reg.hh
@@ -336,33 +336,26 @@

 /// Create a view of this container.
 ///
-/// If NumElems is provided, the size of the container is  
bounds-checked,

-/// otherwise the size is inferred from the container size.
 /// @tparam VecElem Type of the vector elements.
-/// @tparam NumElems Number of vector elements making up the view.
 /// @{
-template -  size_t NumElems = (Packed ? NumBits : NumBits /  
sizeof(VecElem))>

-VecPredRegT as() const
+template 
+VecPredRegT
+as() const
 {
-static_assert((Packed && NumElems <= NumBits) ||
-  (!Packed &&
-   NumBits % sizeof(VecElem) == 0 &&
-   sizeof(VecElem) * NumElems <= NumBits),
-  "Container size incompatible with view size");
-return VecPredRegT(*this);
+static_assert(NumBits % sizeof(VecElem) == 0,
+"Container size incompatible with view size.");
+return VecPredRegTtrue>(

+*this);
 }

-template -  size_t NumElems = (Packed ? NumBits : NumBits /  
sizeof(VecElem))>

-VecPredRegT as()
+template 
+VecPredRegT
+as()
 {
-static_assert((Packed && NumElems <= NumBits) ||
-  (!Packed &&
-   NumBits % sizeof(VecElem) == 0 &&
-   sizeof(VecElem) * NumElems <= NumBits),
-  "Container size incompatible with view size");
-return VecPredRegT(*this);
+static_assert(NumBits % sizeof(VecElem) == 0,
+"Container size incompatible with view size.");
+return VecPredRegTfalse>(

+*this);
 }
 /// @}
 };

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42000
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: Ibdaf38b2e2d8f37ef76d6b8874ac3620982e78a2
Gerrit-Change-Number: 42000
Gerrit-PatchSet: 10
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
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]: sim: Don't needlessly recreate ISA types in InstRecord.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: sim: Don't needlessly recreate ISA types in InstRecord.
..

sim: Don't needlessly recreate ISA types in InstRecord.

The ISAs already define fully realized types. We don't need to
separately track what parameters they used and then feed them into the
same templates again elsewhere.

Change-Id: Iac18bb9374ff684259c6aa00036eac4d1026dcfc
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42002
Reviewed-by: Giacomo Travaglini 
Maintainer: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/sim/insttracer.hh
1 file changed, 6 insertions(+), 9 deletions(-)

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



diff --git a/src/sim/insttracer.hh b/src/sim/insttracer.hh
index cf16790..079e803 100644
--- a/src/sim/insttracer.hh
+++ b/src/sim/insttracer.hh
@@ -95,9 +95,8 @@
 {
 uint64_t as_int;
 double as_double;
-::VecRegContainer* as_vec;
-::VecPredRegContainer* as_pred;
+TheISA::VecRegContainer* as_vec;
+TheISA::VecPredRegContainer* as_pred;
 } data;

 /** @defgroup fetch_seq
@@ -204,18 +203,16 @@
 void setData(double d) { data.as_double = d; data_status = DataDouble;  
}


 void
-setData(::VecRegContainer& d)
+setData(TheISA::VecRegContainer& d)
 {
-data.as_vec = new ::VecRegContainer(d);
+data.as_vec = new TheISA::VecRegContainer(d);
 data_status = DataVec;
 }

 void
-setData(::VecPredRegContainer& d)
+setData(TheISA::VecPredRegContainer& d)
 {
-data.as_pred = new ::VecPredRegContainer<
-TheISA::VecPredRegSizeBits,  
TheISA::VecPredRegHasPackedRepr>(d);

+data.as_pred = new TheISA::VecPredRegContainer(d);
 data_status = DataVecPred;
 }


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42002
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: Iac18bb9374ff684259c6aa00036eac4d1026dcfc
Gerrit-Change-Number: 42002
Gerrit-PatchSet: 10
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
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

[gem5-dev] Change in gem5/gem5[develop]: arch,cpu: Separate printing and serialization of VecPredReg.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: arch,cpu: Separate printing and serialization of VecPredReg.
..

arch,cpu: Separate printing and serialization of VecPredReg.

This is equivalent to what was done with VecReg recently.

Change-Id: I8e28c9796bf5cabd35a6bf5b89e55efcf9324d92
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/41999
Reviewed-by: Gabe Black 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/arch/generic/vec_pred_reg.hh
M src/cpu/o3/regfile.hh
M src/cpu/simple_thread.hh
3 files changed, 37 insertions(+), 29 deletions(-)

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



diff --git a/src/arch/generic/vec_pred_reg.hh  
b/src/arch/generic/vec_pred_reg.hh

index d156ba0..3e248b4 100644
--- a/src/arch/generic/vec_pred_reg.hh
+++ b/src/arch/generic/vec_pred_reg.hh
@@ -42,6 +42,7 @@

 #include "arch/generic/vec_reg.hh"
 #include "base/cprintf.hh"
+#include "sim/serialize_handlers.hh"

 template 
 class VecPredRegContainer;
@@ -152,18 +153,13 @@
 friend std::ostream&
 operator<<(std::ostream& os, const MyClass& p)
 {
-// 0-sized is not allowed
-os << '[' << p.container[0];
-for (int i = 0; i < p.NUM_BITS; ++i) {
-os << " " << (p.container[i] ? 1 : 0);
-}
-os << ']';
+// Size must be greater than 0.
+for (int i = 0; i < NUM_BITS; i++)
+ccprintf(os, "%s%d", i ? " " : "[", (int)p.container[i]);
+ccprintf(os, "]");
 return os;
 }

-/// Returns a string representation of the register content.
-const std::string print() const { return csprintf("%s", *this); }
-
 /// Returns true if the first active element of the register is true.
 /// @param mask Input mask used to filter the predicates to be tested.
 /// @param actual_num_elems Actual number of vector elements  
considered for

@@ -326,18 +322,18 @@
 }
 }

-/// Returns a string representation of the register content.
-const std::string print() const { return csprintf("%s", *this); }
-
 friend std::ostream&
-operator<<(std::ostream& os, const MyClass& v)
+operator<<(std::ostream& os, const MyClass& p)
 {
-for (auto b: v.container) {
-os << csprintf("%d", b);
-}
+// Size must be greater than 0.
+for (int i = 0; i < NumBits; i++)
+ccprintf(os, "%s%d", i ? " " : "[", (int)p.container[i]);
+ccprintf(os, "]");
 return os;
 }

+friend ShowParam>;
+
 /// Create a view of this container.
 ///
 /// If NumElems is provided, the size of the container is  
bounds-checked,

@@ -371,17 +367,29 @@
 /// @}
 };

-/// Helper functions used for serialization/de-serialization
 template 
-inline bool
-to_number(const std::string& value, VecPredRegContainer&  
p)

+struct ParseParam>
 {
-int i = 0;
-for (const auto& c: value) {
-p[i] = (c == '1');
+static bool
+parse(const std::string , VecPredRegContainer  
)

+{
+int i = 0;
+for (const auto& c: s)
+value[i++] = (c == '1');
+return true;
 }
-return true;
-}
+};
+
+template 
+struct ShowParam>
+{
+static void
+show(std::ostream , const VecPredRegContainer  
)

+{
+for (auto b: value.container)
+ccprintf(os, "%d", b);
+}
+};

 /// Dummy type aliases and constants for architectures that do not  
implement

 /// vector predicate registers.
diff --git a/src/cpu/o3/regfile.hh b/src/cpu/o3/regfile.hh
index d30f577..8b45d61 100644
--- a/src/cpu/o3/regfile.hh
+++ b/src/cpu/o3/regfile.hh
@@ -239,7 +239,7 @@

 DPRINTF(IEW, "RegFile: Access to predicate register %i, has "
 "data %s\n", int(phys_reg->index()),
-vecPredRegFile[phys_reg->index()].print());
+vecPredRegFile[phys_reg->index()]);

 return vecPredRegFile[phys_reg->index()];
 }
@@ -323,7 +323,7 @@
 assert(phys_reg->isVecPredPhysReg());

 DPRINTF(IEW, "RegFile: Setting predicate register %i to %s\n",
-int(phys_reg->index()), val.print());
+int(phys_reg->index()), val);

 vecPredRegFile[phys_reg->index()] = val;
 }
diff --git a/src/cpu/simple_thread.hh b/src/cpu/simple_thread.hh
index e192ff1..83820ef 100644
--- a/src/cpu/simple_thread.hh
+++ b/src/cpu/simple_thread.hh
@@ -330,7 +330,7 @@
 const TheISA::VecPredRegContainer& regVal =
 readVecPredRegFlat(flatIndex);
 DPRINTF(VecPredRegs, "Reading predicate reg %d (%d) as %s.\n",
-reg.index(), flatIndex, regVal.print());
+reg.index(), flatIndex, regVal);
 return regVal;
 }

@@ -343,7 +343,7 @@
 getWritableVecPredRegFlat(flatIndex);
 

[gem5-dev] Change in gem5/gem5[develop]: cpu: Use the built in << for VecReg and VecPredReg in ExeTrace.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: cpu: Use the built in << for VecReg and VecPredReg in  
ExeTrace.

..

cpu: Use the built in << for VecReg and VecPredReg in ExeTrace.

There's no reason to reimplement printing code when VecReg and
VecPredReg types already know how to print themselves.

Change-Id: I092c28143de286d765312122b81ce865a5184091
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42001
Tested-by: kokoro 
Reviewed-by: Giacomo Travaglini 
Maintainer: Giacomo Travaglini 
---
M src/cpu/exetrace.cc
1 file changed, 2 insertions(+), 23 deletions(-)

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



diff --git a/src/cpu/exetrace.cc b/src/cpu/exetrace.cc
index be86161..f209bd4 100644
--- a/src/cpu/exetrace.cc
+++ b/src/cpu/exetrace.cc
@@ -115,31 +115,10 @@
 if (Debug::ExecResult && data_status != DataInvalid) {
 switch (data_status) {
   case DataVec:
-{
-ccprintf(outs, " D=0x[");
-auto dv = data.as_vec->as();
-for (int i = TheISA::VecRegSizeBytes / 4 - 1; i >= 0;
- i--) {
-ccprintf(outs, "%08x", dv[i]);
-if (i != 0) {
-ccprintf(outs, "_");
-}
-}
-ccprintf(outs, "]");
-}
+ccprintf(outs, " D=%s", *data.as_vec);
 break;
   case DataVecPred:
-{
-ccprintf(outs, " D=0b[");
-auto pv = data.as_pred->as();
-for (int i = TheISA::VecPredRegSizeBits - 1; i >= 0;  
i--) {

-ccprintf(outs, pv[i] ? "1" : "0");
-if (i != 0 && i % 4 == 0) {
-ccprintf(outs, "_");
-}
-}
-ccprintf(outs, "]");
-}
+ccprintf(outs, " D=%s", *data.as_pred);
 break;
   default:
 ccprintf(outs, " D=%#018x", data.as_int);



8 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/+/42001
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: I092c28143de286d765312122b81ce865a5184091
Gerrit-Change-Number: 42001
Gerrit-PatchSet: 10
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
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]: arch,mem: Use szext instead of sext as appropriate.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: arch,mem: Use szext instead of sext as appropriate.
..

arch,mem: Use szext instead of sext as appropriate.

When the value being passed to sext needs to be masked first, szext can
be used instead without the masking.

Change-Id: I98c99ad2731216fe8ccf1253f5ac3891fe03b1de
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42386
Reviewed-by: Daniel Carvalho 
Reviewed-by: Bobby R. Bruce 
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
---
M src/arch/amdgpu/gcn3/insts/instructions.cc
M src/arch/arm/insts/vfp.cc
M src/arch/mips/isa/decoder.isa
M src/arch/sparc/insts/blockmem.hh
M src/arch/sparc/insts/branch.hh
M src/arch/sparc/insts/integer.hh
M src/arch/sparc/insts/mem.hh
M src/arch/sparc/isa/decoder.isa
M src/arch/sparc/tlb.cc
M src/mem/cache/compressors/dictionary_compressor.hh
M src/mem/cache/compressors/fpc.hh
11 files changed, 31 insertions(+), 41 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, approved
  Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/amdgpu/gcn3/insts/instructions.cc  
b/src/arch/amdgpu/gcn3/insts/instructions.cc

index ad04a4a..8cadff7 100644
--- a/src/arch/amdgpu/gcn3/insts/instructions.cc
+++ b/src/arch/amdgpu/gcn3/insts/instructions.cc
@@ -5728,8 +5728,7 @@

 for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) {
 if (wf->execMask(lane)) {
-vdst[lane] = sext<24>(bits(src0[lane], 23, 0))
-* sext<24>(bits(src1[lane], 23, 0));
+vdst[lane] = szext<24>(src0[lane]) * szext<24>(src1[lane]);
 }
 }

@@ -5760,10 +5759,8 @@

 for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) {
 if (wf->execMask(lane)) {
-VecElemI64 tmp_src0
-= (VecElemI64)sext<24>(bits(src0[lane], 23, 0));
-VecElemI64 tmp_src1
-= (VecElemI64)sext<24>(bits(src1[lane], 23, 0));
+VecElemI64 tmp_src0 = (VecElemI64)szext<24>(src0[lane]);
+VecElemI64 tmp_src1 = (VecElemI64)szext<24>(src1[lane]);

 vdst[lane] = (VecElemI32)((tmp_src0 * tmp_src1) >> 32);
 }
@@ -23563,8 +23560,7 @@

 for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) {
 if (wf->execMask(lane)) {
-vdst[lane] = sext<24>(bits(src0[lane], 23, 0))
-* sext<24>(bits(src1[lane], 23, 0));
+vdst[lane] = szext<24>(src0[lane]) * szext<24>(src1[lane]);
 }
 }

@@ -23605,10 +23601,8 @@

 for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) {
 if (wf->execMask(lane)) {
-VecElemI64 tmp_src0
-= (VecElemI64)sext<24>(bits(src0[lane], 23, 0));
-VecElemI64 tmp_src1
-= (VecElemI64)sext<24>(bits(src1[lane], 23, 0));
+VecElemI64 tmp_src0 = (VecElemI64)szext<24>(src0[lane]);
+VecElemI64 tmp_src1 = (VecElemI64)szext<24>(src1[lane]);

 vdst[lane] = (VecElemI32)((tmp_src0 * tmp_src1) >> 32);
 }
@@ -27785,8 +27779,8 @@

 for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) {
 if (wf->execMask(lane)) {
-vdst[lane] = sext<24>(bits(src0[lane], 23, 0))
-* sext<24>(bits(src1[lane], 23, 0)) + src2[lane];
+vdst[lane] = szext<24>(src0[lane])
+* szext<24>(src1[lane]) + src2[lane];
 }
 }

diff --git a/src/arch/arm/insts/vfp.cc b/src/arch/arm/insts/vfp.cc
index c39b94a..4729039 100644
--- a/src/arch/arm/insts/vfp.cc
+++ b/src/arch/arm/insts/vfp.cc
@@ -692,9 +692,9 @@
 {
 fesetround(FeRoundNearest);
 if (width == 16)
-val = sext<16>(val & mask(16));
+val = szext<16>(val);
 else if (width == 32)
-val = sext<32>(val & mask(32));
+val = szext<32>(val);
 else if (width != 64)
 panic("Unsupported width %d", width);

@@ -731,9 +731,9 @@
 {
 fesetround(FeRoundNearest);
 if (width == 16)
-val = sext<16>(val & mask(16));
+val = szext<16>(val);
 else if (width == 32)
-val = sext<32>(val & mask(32));
+val = szext<32>(val);
 else if (width != 64)
 panic("Unsupported width %d", width);

diff --git a/src/arch/mips/isa/decoder.isa b/src/arch/mips/isa/decoder.isa
index ea2b43a..2e56a04 100644
--- a/src/arch/mips/isa/decoder.isa
+++ b/src/arch/mips/isa/decoder.isa
@@ -2445,12 +2445,11 @@
 }
 }});
 0x3: shilov({{
-if ((int64_t)sext<6>(Rs_sw<5:0>) <  
0) {

+if 

[gem5-dev] Change in gem5/gem5[develop]: arch-sparc: Fix some bit manipulation bugs.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: arch-sparc: Fix some bit manipulation bugs.
..

arch-sparc: Fix some bit manipulation bugs.

Several sext<> calls had an off by one sign bit index, which is really
the size of the number which is being sign extended. Also, two calls in
arch/sparc/tlb.cc seemed to assume that the argument to that function
was modified in place, where really the new value is returned
separately. Move the call to sext so its return value is used and not
thrown away.

Change-Id: I86cb81ad243558e1a0d33def7f3eebe6973d6800
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42603
Reviewed-by: Daniel Carvalho 
Reviewed-by: Bobby R. Bruce 
Reviewed-by: Boris Shingarov 
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
---
M src/arch/sparc/insts/integer.hh
M src/arch/sparc/tlb.cc
2 files changed, 3 insertions(+), 5 deletions(-)

Approvals:
  Boris Shingarov: Looks good to me, but someone else must approve
  Daniel Carvalho: Looks good to me, approved
  Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/sparc/insts/integer.hh  
b/src/arch/sparc/insts/integer.hh

index 1438cfd..a38bebe 100644
--- a/src/arch/sparc/insts/integer.hh
+++ b/src/arch/sparc/insts/integer.hh
@@ -94,7 +94,7 @@
 {
   protected:
 IntOpImm11(const char *mnem, ExtMachInst _machInst, OpClass  
__opClass) :
-IntOpImm(mnem, _machInst, __opClass, sext<10>(bits(_machInst, 10,  
0)))
+IntOpImm(mnem, _machInst, __opClass, sext<11>(bits(_machInst, 10,  
0)))

 {}
 };

diff --git a/src/arch/sparc/tlb.cc b/src/arch/sparc/tlb.cc
index 380b365..167ff03 100644
--- a/src/arch/sparc/tlb.cc
+++ b/src/arch/sparc/tlb.cc
@@ -1238,8 +1238,7 @@
 itb->sfsr = data;
 break;
   case 0x30:
-sext<59>(bits(data, 59,0));
-itb->tag_access = data;
+itb->tag_access = sext<60>(bits(data, 59,0));
 break;
   default:
 goto doMmuWriteError;
@@ -1315,8 +1314,7 @@
 sfsr = data;
 break;
   case 0x30:
-sext<59>(bits(data, 59,0));
-tag_access = data;
+tag_access = sext<60>(bits(data, 59,0));
 break;
   case 0x80:
 tc->setMiscReg(MISCREG_MMU_PART_ID, data);

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42603
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: I86cb81ad243558e1a0d33def7f3eebe6973d6800
Gerrit-Change-Number: 42603
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Boris Shingarov 
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-sparc: Wrap overly long lines in the decoder definition.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: arch-sparc: Wrap overly long lines in the decoder  
definition.

..

arch-sparc: Wrap overly long lines in the decoder definition.

Change-Id: I194f4367bd889fa6639046675b2f565a4e03bc4d
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42383
Reviewed-by: Daniel Carvalho 
Reviewed-by: Bobby R. Bruce 
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
---
M src/arch/sparc/isa/decoder.isa
1 file changed, 72 insertions(+), 34 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, approved
  Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/sparc/isa/decoder.isa b/src/arch/sparc/isa/decoder.isa
index 3fc472b..fb1d889 100644
--- a/src/arch/sparc/isa/decoder.isa
+++ b/src/arch/sparc/isa/decoder.isa
@@ -215,7 +215,8 @@
 Y = resTemp<63:32>;}});
 0x1B: IntOpCcRes::smulcc({{
 int64_t resTemp;
-Rd = resTemp = sext<32>(Rs1_sdw<31:0>) *  
sext<32>(Rs2_or_imm13<31:0>);

+Rd = resTemp = sext<32>(Rs1_sdw<31:0>) *
+   sext<32>(Rs2_or_imm13<31:0>);
 Y = resTemp<63:32>;}});
 0x1C: subccc({{
 int64_t res, op1 = Rs1, op2 = Rs2_or_imm13;
@@ -247,13 +248,15 @@
 if (val2 == 0) {
 fault = std::make_shared();
 } else {
-Rd = (int64_t)((Y << 32) | Rs1_sdw<31:0>) / val2;
-overflow = ((int64_t)Rd >=  
std::numeric_limits::max());
-underflow = ((int64_t)Rd <=  
std::numeric_limits::min());

+Rd_sdw = ((Y_sdw << 32) | Rs1_sdw<31:0>) / val2;
+overflow =
+(Rd_sdw >=  
std::numeric_limits::max());

+underflow =
+(Rd_sdw <=  
std::numeric_limits::min());

 if (overflow)
-Rd = 0x7FFF;
+Rd_sdw = 0x7FFF;
 else if (underflow)
-Rd = 0x8000ULL;
+Rd_sdw = 0x8000ULL;
 }
 }}, iv={{overflow || underflow}});
 0x20: taddcc({{
@@ -395,10 +398,12 @@
 }
 0x2B: BasicOperate::flushw({{
 if (NWindows - 2 - Cansave != 0) {
-if (Otherwin)
-fault =  
std::make_shared(4*Wstate<5:3>);

-else
-fault =  
std::make_shared(4*Wstate<2:0>);

+if (Otherwin) {
+fault = std::make_shared(4 *  
Wstate<5:3>);

+} else {
+fault = std::make_shared(
+4 * Wstate<2:0>);
+}
 }
 }});
 0x2C: decode MOVCC3
@@ -471,7 +476,9 @@
 // 0x04-0x05 should cause an illegal instruction exception
 0x06: NoPriv::wrfprs({{Fprs = Rs1 ^ Rs2_or_imm13;}});
 // 0x07-0x0E should cause an illegal instruction exception
-0x0F: Trap::softreset({{fault =  
std::make_shared();}});

+0x0F: Trap::softreset({{
+fault = std::make_shared();
+}});
 0x10: Priv::wrpcr({{Pcr = Rs1 ^ Rs2_or_imm13;}});
 0x11: Priv::wrpic({{Pic = Rs1 ^ Rs2_or_imm13;}},  
{{Pcr<0:>}});

 // 0x12 should cause an illegal instruction exception
@@ -480,8 +487,12 @@
 return std::make_shared();
 Gsr = Rs1 ^ Rs2_or_imm13;
 }});
-0x14: Priv::wrsoftint_set({{SoftintSet = Rs1 ^  
Rs2_or_imm13;}});
-0x15: Priv::wrsoftint_clr({{SoftintClr = Rs1 ^  
Rs2_or_imm13;}});

+0x14: Priv::wrsoftint_set({{
+SoftintSet = Rs1 ^ Rs2_or_imm13;
+}});
+0x15: Priv::wrsoftint_clr({{
+SoftintClr = Rs1 ^ Rs2_or_imm13;
+}});
 0x16: Priv::wrsoftint({{Softint = Rs1 ^ Rs2_or_imm13;}});
 0x17: Priv::wrtick_cmpr({{TickCmpr = Rs1 ^  
Rs2_or_imm13;}});

 0x18: NoPriv::wrstick({{
@@ -541,7 +552,9 @@
 0x08: Priv::wrprpil({{Pil = Rs1 ^ Rs2_or_imm13;}});
 0x09: Priv::wrprcwp({{Cwp = Rs1 ^ Rs2_or_imm13;}});
 0x0A: Priv::wrprcansave({{Cansave = Rs1 ^ Rs2_or_imm13;}});
-0x0B: Priv::wrprcanrestore({{Canrestore = Rs1 ^  
Rs2_or_imm13;}});

+0x0B: Priv::wrprcanrestore({{
+  

[gem5-dev] Change in gem5/gem5[develop]: base: Improve handling of thread IDs for remote GDB.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: base: Improve handling of thread IDs for remote GDB.
..

base: Improve handling of thread IDs for remote GDB.

The remote GDB protocol encode thread IDs as positive integers, where 0
is a special value which indicates "pick any thread", and -1 is a
special value which indicates all threads.

The previous implementation would look like it worked handling the
special value -1 (for instance) because it see the '-' of "-1", stop
looking for digits, and return the default value 0.

This new implementation handles -1 as a special case, and will report an
error if no digits were found otherwise.

Also this change adds an encodeThreadId method to convert a ContextID
into a GDB thread ID by adding one to avoid the special value 0.

Change-Id: Iec54fbd9563d20a56011f48d50d69111ed1467b8
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/44606
Maintainer: Bobby R. Bruce 
Reviewed-by: Daniel Carvalho 
Tested-by: kokoro 
---
M src/base/remote_gdb.cc
1 file changed, 44 insertions(+), 3 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, approved
  Bobby R. Bruce: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 9278eb0..3fc5240 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -283,6 +283,43 @@
 return r;
 }

+bool
+parseThreadId(const char **srcp, bool , bool , ContextID )
+{
+all = any = false;
+tid = 0;
+const char *src = *srcp;
+if (*src == '-') {
+// This could be the start of -1, which means all threads.
+src++;
+if (*src++ != '1')
+return false;
+*srcp += 2;
+all = true;
+return true;
+}
+tid = hex2i(srcp);
+// If *srcp still points to src, no characters were consumed and no  
thread
+// id was found. Without this check, we can't tell the difference  
between

+// zero and a parsing error.
+if (*srcp == src)
+return false;
+
+if (tid == 0)
+any = true;
+
+tid--;
+
+return true;
+}
+
+int
+encodeThreadId(ContextID id)
+{
+// Thread ID 0 is reserved and means "pick any thread".
+return id + 1;
+}
+
 enum GdbBreakpointType
 {
 GdbSoftBp = '0',
@@ -864,8 +901,12 @@
 BaseRemoteGDB::cmdSetThread(GdbCommand::Context )
 {
 const char *p = ctx.data + 1; // Ignore the subcommand byte.
-if (hex2i() != tc->contextId())
+ContextID tid = 0;
+bool all, any;
+if (!parseThreadId(, all, any, tid))
 throw CmdError("E01");
+if (any || all || tid != tc->contextId())
+throw CmdError("E02");
 send("OK");
 return true;
 }
@@ -945,7 +986,7 @@
 void
 BaseRemoteGDB::queryC(QuerySetCommand::Context )
 {
-send(csprintf("QC%x", tc->contextId()).c_str());
+send(csprintf("QC%x", encodeThreadId(tc->contextId())).c_str());
 }

 void
@@ -1005,7 +1046,7 @@
 void
 BaseRemoteGDB::queryFThreadInfo(QuerySetCommand::Context )
 {
-send(csprintf("m%x", tc->contextId()).c_str());
+send(csprintf("m%x", encodeThreadId(tc->contextId())).c_str());
 }

 void

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44606
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: Iec54fbd9563d20a56011f48d50d69111ed1467b8
Gerrit-Change-Number: 44606
Gerrit-PatchSet: 2
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]: sim: Minor cleanup of the System class.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: sim: Minor cleanup of the System class.
..

sim: Minor cleanup of the System class.

Fix style, move constant initialization out of the constructor and into
the class body, and minorly streamline an if condition.

Change-Id: I8ef42dcb8336ece58578cbd1f33937103b42989f
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/44605
Reviewed-by: Bobby R. Bruce 
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
---
M src/sim/system.cc
M src/sim/system.hh
2 files changed, 32 insertions(+), 27 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/sim/system.cc b/src/sim/system.cc
index e3924d7..0c42256 100644
--- a/src/sim/system.cc
+++ b/src/sim/system.cc
@@ -204,21 +204,16 @@
 System::System(const Params )
 : SimObject(p), _systemPort("system_port", this),
   multiThread(p.multi_thread),
-  pagePtr(0),
   init_param(p.init_param),
   physProxy(_systemPort, p.cache_line_size),
   workload(p.workload),
 #if USE_KVM
   kvmVM(p.kvm_vm),
-#else
-  kvmVM(nullptr),
 #endif
   physmem(name() + ".physmem", p.memories, p.mmap_using_noreserve,
   p.shared_backstore),
   memoryMode(p.mem_mode),
   _cacheLineSize(p.cache_line_size),
-  workItemsBegin(0),
-  workItemsEnd(0),
   numWorkIds(p.num_work_ids),
   thermalModel(p.thermal_model),
   _m5opRange(p.m5ops_base ?
@@ -239,9 +234,10 @@
 #endif

 // check if the cache line size is a value known to work
-if (!(_cacheLineSize == 16 || _cacheLineSize == 32 ||
-  _cacheLineSize == 64 || _cacheLineSize == 128))
+if (_cacheLineSize != 16 && _cacheLineSize != 32 &&
+_cacheLineSize != 64 && _cacheLineSize != 128) {
 warn_once("Cache line size is neither 16, 32, 64 nor 128  
bytes.\n");

+}

 // Get the generic system requestor IDs
 M5_VAR_USED RequestorID tmp_id;
diff --git a/src/sim/system.hh b/src/sim/system.hh
index f0eac6a..480b523 100644
--- a/src/sim/system.hh
+++ b/src/sim/system.hh
@@ -89,10 +89,18 @@
 SystemPort(const std::string &_name, SimObject *_owner)
 : RequestPort(_name, _owner)
 { }
-bool recvTimingResp(PacketPtr pkt) override
-{ panic("SystemPort does not receive timing!\n"); return false; }
-void recvReqRetry() override
-{ panic("SystemPort does not expect retry!\n"); }
+
+bool
+recvTimingResp(PacketPtr pkt) override
+{
+panic("SystemPort does not receive timing!");
+}
+
+void
+recvReqRetry() override
+{
+panic("SystemPort does not expect retry!");
+}
 };

 std::list liveEvents;
@@ -250,7 +258,9 @@
  * CPUs. SimObjects are expected to use Port::sendAtomic() and
  * Port::recvAtomic() when accessing memory in this mode.
  */
-bool isAtomicMode() const {
+bool
+isAtomicMode() const
+{
 return memoryMode == Enums::atomic ||
 memoryMode == Enums::atomic_noncaching;
 }
@@ -261,9 +271,7 @@
  * SimObjects are expected to use Port::sendTiming() and
  * Port::recvTiming() when accessing memory in this mode.
  */
-bool isTimingMode() const {
-return memoryMode == Enums::timing;
-}
+bool isTimingMode() const { return memoryMode == Enums::timing; }

 /**
  * Should caches be bypassed?
@@ -271,7 +279,9 @@
  * Some CPUs need to bypass caches to allow direct memory
  * accesses, which is required for hardware virtualization.
  */
-bool bypassCaches() const {
+bool
+bypassCaches() const
+{
 return memoryMode == Enums::atomic_noncaching;
 }
 /** @} */
@@ -310,7 +320,7 @@
 bool schedule(PCEvent *event) override;
 bool remove(PCEvent *event) override;

-Addr pagePtr;
+Addr pagePtr = 0;

 uint64_t init_param;

@@ -326,9 +336,7 @@
  * Get a pointer to the Kernel Virtual Machine (KVM) SimObject,
  * if present.
  */
-KvmVM* getKvmVM() {
-return kvmVM;
-}
+KvmVM *getKvmVM() { return kvmVM; }

 /** Verify gem5 configuration will support KVM emulation */
 bool validKvmEnvironment() const;
@@ -402,7 +410,7 @@

   protected:

-KvmVM *const kvmVM;
+KvmVM *const kvmVM = nullptr;

 PhysicalMemory physmem;

@@ -410,8 +418,8 @@

 const unsigned int _cacheLineSize;

-uint64_t workItemsBegin;
-uint64_t workItemsEnd;
+uint64_t workItemsBegin = 0;
+uint64_t workItemsEnd = 0;
 uint32_t numWorkIds;

 /** This array is a per-system list of all devices capable of issuing a
@@ -465,7 +473,7 @@
  * @return the requestor's ID.
  */
 RequestorID getRequestorId(const SimObject* requestor,
-

[gem5-dev] Re: error from new test_convert.py?

[Bobby Bruce via gem5-dev]

My quick fix for this is here:
https://gem5-review.googlesource.com/c/public/gem5/+/44625
--
Dr. Bobby R. Bruce
Room 2235,
Kemper Hall, UC Davis
Davis,
CA, 95616

web: https://www.bobbybruce.net


On Mon, Apr 19, 2021 at 7:20 AM Bobby Bruce  wrote:

> I think I've got a solution to this (in my head at least), I'll try to
> push a patch today.
>
> I like having all the tests runnable in the `test` directory with
> `./main.py`, so it irks me this is a bit of a special case (not that we
> don't already have a few special cases, most notable the GTests). I think I
> can work this in a way to be runnable within the `tests` directory and work
> as a presubmit test.
> --
> Dr. Bobby R. Bruce
> Room 2235,
> Kemper Hall, UC Davis
> Davis,
> CA, 95616
>
> web: https://www.bobbybruce.net
>
>
> On Mon, Apr 19, 2021 at 6:55 AM Giacomo Travaglini <
> giacomo.travagl...@arm.com> wrote:
>
>> Yes,
>>
>> This is why I usually run using tests/gem5 as root directory
>>
>> ./tests/main.py [...] \
>> tests/gem5 <- root directory
>>
>> Testlib is basically trying to load every python file it finds on its
>> walk.
>> We should definitely add python unit tests to our presubmit runs IMHO.
>> The simplest way is via unittest Test Discovery [1]
>>
>> Kind Regards
>>
>> Giacomo
>>
>> [1]:
>> https://docs.python.org/3/library/unittest.html#unittest-test-discovery
>>
>> > -Original Message-
>> > From: Bobby Bruce via gem5-dev 
>> > Sent: 19 April 2021 13:43
>> > To: gem5 Developer List 
>> > Cc: Gabe Black ; Bobby Bruce
>> > 
>> > Subject: [gem5-dev] Re: error from new test_convert.py?
>> >
>> > The exception being thrown is "No module named 'm5'".  m5 is not in the
>> > path, but could easily be added.
>> >
>> > From looking at https://gem5-
>> > review.googlesource.com/c/public/gem5/+/39377, this was never intended
>> > to be run via `main.py`, though "test_convert.py" has the "test" prefix
>> so our
>> > TestLib framework tries to run it anyway.
>> >
>> > My questions are: when were these tests meant to be run? Should they be
>> > incorporated into the TestLib (i.e. those tests run via `main.py`) to
>> be run as a
>> > Presubmit, Nightly, or Weekly test? This would be easy to do (I think I
>> could
>> > incorporate this relatively quickly).
>> >
>> > Kind regards,
>> > Bobby
>> >
>> > --
>> >
>> > Dr. Bobby R. Bruce
>> > Room 2235,
>> > Kemper Hall, UC Davis
>> > Davis,
>> > CA, 95616
>> >
>> >
>> > web: https://www.bobbybruce.net
>> >
>> >
>> >
>> > On Sun, Apr 18, 2021 at 9:35 PM Gabe Black via gem5-dev > > d...@gem5.org  > wrote:
>> >
>> >
>> > Hi gem5 devs, specifically Andreas. I was just running the regressions
>> > on a change I'm working on, and I noticed this message:
>> >
>> > Exception thrown while loading
>> > "/home/gblack/gem5/work/tests/pyunit/util/test_convert.py"
>> > Ignoring all tests in this file.
>> >
>> > FYI, it looks like these tests are not runnng...
>> > ___
>> > gem5-dev mailing list -- gem5-dev@gem5.org > > d...@gem5.org>
>> > To unsubscribe send an email to gem5-dev-le...@gem5.org
>> > 
>> > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>>
>> IMPORTANT NOTICE: The contents of this email and any attachments are
>> confidential and may also be privileged. If you are not the intended
>> recipient, please notify the sender immediately and do not disclose the
>> contents to any other person, use it for any purpose, or store or copy the
>> information in any medium. Thank you.
>>
>
___
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]: python,tests: Update pyunit tests to run in TestLib

[Bobby R. Bruce (Gerrit) via gem5-dev]

Bobby R. Bruce has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/44625 )



Change subject: python,tests: Update pyunit tests to run in TestLib
..

python,tests: Update pyunit tests to run in TestLib

Previously the pyunit tests needed run in the gem5 root, this change
allows them to run as part of the quick TestLib tests (thereby having
them run as part of the presubmit checks).

`pyunit/util/test_convert.py` has been renamed
`pyunit/util/convert-test.py` as TestLib attempts to parse any Python
file with the "test" prefix.

Example usage:

```
./main.py run --uid  
SuiteUID:tests/pyunit/test_run.py:pyunit-convert-check-NULL-x86_64-opt

```

Discussed briefly in email thread:
https://www.mail-archive.com/gem5-dev@gem5.org/msg38563.html

Change-Id: Id566d44fcb5d8c599eb1a90bca56793158a201e6
---
A tests/pyunit/test_run.py
R tests/pyunit/util/convert-check.py
2 files changed, 49 insertions(+), 0 deletions(-)



diff --git a/tests/pyunit/test_run.py b/tests/pyunit/test_run.py
new file mode 100644
index 000..6dfe7e1
--- /dev/null
+++ b/tests/pyunit/test_run.py
@@ -0,0 +1,49 @@
+# Copyright (c) 2021 The Regents of the University of California
+# All Rights Reserved.
+#
+# 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.
+
+import os
+
+from testlib.configuration import constants
+from testlib.helper import joinpath
+from gem5.suite import *
+
+# A map of test names to their configs. This is where the name of the test  
is

+# set.
+test_configs = {
+"pyunit-convert-check" :
+str(joinpath(os.getcwd(), "util", "convert_check.py")),
+}
+
+for name in test_configs:
+gem5_verify_config(
+name=name,
+config=test_configs[name],
+verifiers=(),
+config_args=[],
+valid_isas=(constants.null_tag,),
+length = constants.quick_tag,
+)
+
diff --git a/tests/pyunit/util/test_convert.py  
b/tests/pyunit/util/convert-check.py

similarity index 100%
rename from tests/pyunit/util/test_convert.py
rename to tests/pyunit/util/convert-check.py

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44625
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: Id566d44fcb5d8c599eb1a90bca56793158a201e6
Gerrit-Change-Number: 44625
Gerrit-PatchSet: 1
Gerrit-Owner: Bobby R. Bruce 
Gerrit-MessageType: newchange
___
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] Re: gem5 namespace

[Jason Lowe-Power via gem5-dev]

>
> I still favor namespace gem5 - it'd be the "external external" API, i.e.
> we probably wouldn't be using it within src/ that much, and it would be
> used by other simulators...


This is why I'm strongly in favor of lowercase gem5. When external projects
link to gem5 (which is *going* to happen), I think it's much better to use
the normative gem5 spelling. It would be very confusing for people to use
Gem5 in the code but gem5 in documentation/papers.

```
class MyExternalObj: public gem5::SimObject
{
};
```

Jason



On Mon, Apr 19, 2021 at 6:40 AM Bobby Bruce via gem5-dev 
wrote:

> Nothing gets software devs as engaged as when discussing naming
> conventions :).
>
> I vote for "gem5" (lowercase) namespace, with all caps MACROS, but my sole
> reason for this is I've grown to flinch whenever I see "Gem5", which I
> admit isn't the best argument.
>
> I echo Daniel's comment that I care more about having a rule and moving
> past this than what that rule should be.
>
> --
> Dr. Bobby R. Bruce
> Room 2235,
> Kemper Hall, UC Davis
> Davis,
> CA, 95616
>
> web: https://www.bobbybruce.net
>
>
> On Sun, Apr 18, 2021 at 3:44 PM Daniel Carvalho via gem5-dev <
> gem5-dev@gem5.org> wrote:
>
>> Overall, there are way more uses of "gem5" than "Gem5" in the codebase,
>> and most of the instances that break the identity rule could be easily
>> fixed; however, there are cases that generate inconvenience: classes
>> starting with lowercase, and situations where gem5 is in the middle of the
>> var name (like "haveGem5Extension") make the code harder to
>> read/understand. In this sense, the uppercase version generates better
>> code.
>>
>>
>> I still favor namespace gem5 - it'd be the "external external" API, i.e.
>> we probably wouldn't be using it within src/ that much, and it would be
>> used by other simulators and within our SystemC bridge (more on that later)
>> - however, since we already have some exceptions, it wouldn't be the end
>> of the world having it start with a capital letter.
>>
>>
>> In the end, I personally do not care about which approach is taken, but
>> the decision taken must be taken as a community. Therefore, it would be
>> nice if we could have participation from other contributors to make the
>> final decision less susceptible to changes/complaints in the future.
>>
>>
>> Regarding when to use it:
>> IMHO (and not thoroughly thought out), all .cc and .hh and objects within
>> src/ should be subject to the namespace. Objects declared there are
>> declared and maintained by gem5. Because of that there would probably be
>> very few instances of namespace resolution within src/, so we should keep
>> avoiding "using namespace" and being verbose about it. Finally, we also
>> probably want to encourage users to define their objects inside the gem5
>> namespace to make it less unlikely that they will give up on uploading
>> their contributions due to the different styles, and the laziness to adapt
>> code. This means that disturbance in user code would be minimal: they would
>> simply add "namespace (G|g)em5 {" in the beginning and "} // namespace
>> (G|g)em5" at the end, instead of multiple "(G)|gem5::" instances.
>>
>>
>> Regards,
>>
>> Daniel
>> For the record, if the namespaces you found using snake_case start with
>> sc_, those are for systemc and are mandated by that standard. The one
>> exception, sc_gem5, is one I made up which is closely associated with the
>> other systemc namespaces and is named similarly to them for consistency.
>>
>> Gabe
>>
>> On Thu, Apr 15, 2021 at 1:51 AM Giacomo Travaglini <
>> giacomo.travagl...@arm.com> wrote:
>>
>> My vote goes to 1 and A as well
>>
>> My sole argument is consistency; in general I'd rather start a namespace
>> with a lowercase. So that when we have something
>> like a scope resolution we know we are dealing with a namespace and not a
>> class. But that's off-topic.
>>
>> Namespace names are anyway not covered by our coding style, so it's
>> probably worth adding an entry.
>>
>> https://www.gem5.org/documentation/general_docs/development/coding_style/
>>
>> From a quick grep I can see most of our namespaces follow the PascalCase
>> type, though there are some namespaces using snake_case convention.
>>
>> Giacomo
>>
>> > -Original Message-
>> > From: Gabe Black via gem5-dev 
>> > Sent: 15 April 2021 09:03
>> > To: gem5 Developer List 
>> > Cc: Gabe Black 
>> > Subject: [gem5-dev] Re: gem5 namespace
>> >
>> > My vote is for 1 and A.
>> >
>> > We have style rules for a reason, and that is because not following them
>> > causes technical problems like name collisions, and makes it less
>> obvious
>> > when reading code what things are and what they're doing. It's a bit
>> > hypocritical to say that we should follow style rules and completely
>> ignore
>> > the aesthetic rule when capitalizing GEM5_, but then say that the
>> aesthetic
>> > rule should win when dealing with the namespace.
>> >
>> > This is further inconsistent with the 

[gem5-dev] Re: error from new test_convert.py?

[Bobby Bruce via gem5-dev]

I think I've got a solution to this (in my head at least), I'll try to push
a patch today.

I like having all the tests runnable in the `test` directory with
`./main.py`, so it irks me this is a bit of a special case (not that we
don't already have a few special cases, most notable the GTests). I think I
can work this in a way to be runnable within the `tests` directory and work
as a presubmit test.
--
Dr. Bobby R. Bruce
Room 2235,
Kemper Hall, UC Davis
Davis,
CA, 95616

web: https://www.bobbybruce.net


On Mon, Apr 19, 2021 at 6:55 AM Giacomo Travaglini <
giacomo.travagl...@arm.com> wrote:

> Yes,
>
> This is why I usually run using tests/gem5 as root directory
>
> ./tests/main.py [...] \
> tests/gem5 <- root directory
>
> Testlib is basically trying to load every python file it finds on its walk.
> We should definitely add python unit tests to our presubmit runs IMHO. The
> simplest way is via unittest Test Discovery [1]
>
> Kind Regards
>
> Giacomo
>
> [1]:
> https://docs.python.org/3/library/unittest.html#unittest-test-discovery
>
> > -Original Message-
> > From: Bobby Bruce via gem5-dev 
> > Sent: 19 April 2021 13:43
> > To: gem5 Developer List 
> > Cc: Gabe Black ; Bobby Bruce
> > 
> > Subject: [gem5-dev] Re: error from new test_convert.py?
> >
> > The exception being thrown is "No module named 'm5'".  m5 is not in the
> > path, but could easily be added.
> >
> > From looking at https://gem5-
> > review.googlesource.com/c/public/gem5/+/39377, this was never intended
> > to be run via `main.py`, though "test_convert.py" has the "test" prefix
> so our
> > TestLib framework tries to run it anyway.
> >
> > My questions are: when were these tests meant to be run? Should they be
> > incorporated into the TestLib (i.e. those tests run via `main.py`) to be
> run as a
> > Presubmit, Nightly, or Weekly test? This would be easy to do (I think I
> could
> > incorporate this relatively quickly).
> >
> > Kind regards,
> > Bobby
> >
> > --
> >
> > Dr. Bobby R. Bruce
> > Room 2235,
> > Kemper Hall, UC Davis
> > Davis,
> > CA, 95616
> >
> >
> > web: https://www.bobbybruce.net
> >
> >
> >
> > On Sun, Apr 18, 2021 at 9:35 PM Gabe Black via gem5-dev  > d...@gem5.org  > wrote:
> >
> >
> > Hi gem5 devs, specifically Andreas. I was just running the regressions
> > on a change I'm working on, and I noticed this message:
> >
> > Exception thrown while loading
> > "/home/gblack/gem5/work/tests/pyunit/util/test_convert.py"
> > Ignoring all tests in this file.
> >
> > FYI, it looks like these tests are not runnng...
> > ___
> > gem5-dev mailing list -- gem5-dev@gem5.org  > d...@gem5.org>
> > To unsubscribe send an email to gem5-dev-le...@gem5.org
> > 
> > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>
___
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] Re: error from new test_convert.py?

[Giacomo Travaglini via gem5-dev]

Yes,

This is why I usually run using tests/gem5 as root directory

./tests/main.py [...] \
tests/gem5 <- root directory

Testlib is basically trying to load every python file it finds on its walk.
We should definitely add python unit tests to our presubmit runs IMHO. The 
simplest way is via unittest Test Discovery [1]

Kind Regards

Giacomo

[1]: https://docs.python.org/3/library/unittest.html#unittest-test-discovery

> -Original Message-
> From: Bobby Bruce via gem5-dev 
> Sent: 19 April 2021 13:43
> To: gem5 Developer List 
> Cc: Gabe Black ; Bobby Bruce
> 
> Subject: [gem5-dev] Re: error from new test_convert.py?
>
> The exception being thrown is "No module named 'm5'".  m5 is not in the
> path, but could easily be added.
>
> From looking at https://gem5-
> review.googlesource.com/c/public/gem5/+/39377, this was never intended
> to be run via `main.py`, though "test_convert.py" has the "test" prefix so our
> TestLib framework tries to run it anyway.
>
> My questions are: when were these tests meant to be run? Should they be
> incorporated into the TestLib (i.e. those tests run via `main.py`) to be run 
> as a
> Presubmit, Nightly, or Weekly test? This would be easy to do (I think I could
> incorporate this relatively quickly).
>
> Kind regards,
> Bobby
>
> --
>
> Dr. Bobby R. Bruce
> Room 2235,
> Kemper Hall, UC Davis
> Davis,
> CA, 95616
>
>
> web: https://www.bobbybruce.net
>
>
>
> On Sun, Apr 18, 2021 at 9:35 PM Gabe Black via gem5-dev  d...@gem5.org  > wrote:
>
>
> Hi gem5 devs, specifically Andreas. I was just running the regressions
> on a change I'm working on, and I noticed this message:
>
> Exception thrown while loading
> "/home/gblack/gem5/work/tests/pyunit/util/test_convert.py"
> Ignoring all tests in this file.
>
> FYI, it looks like these tests are not runnng...
> ___
> gem5-dev mailing list -- gem5-dev@gem5.org  d...@gem5.org>
> To unsubscribe send an email to gem5-dev-le...@gem5.org
> 
> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
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]: scons: Allow declaring dependencies of tags

[Daniel Carvalho (Gerrit) via gem5-dev]

Daniel Carvalho has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/43587 )


Change subject: scons: Allow declaring dependencies of tags
..

scons: Allow declaring dependencies of tags

It was quite incovenient to declare tags. tags either
had to be added to the Source declaration of included
files, or using (with_tag()) would need to be added
with all the indirect sub-tags. For example,

Assume:
- B.cc refers to symbols in A.cc
- C.cc refers to symbols in B.cc (i.e., it needs A transitively)
- D.cc refers to symbols in A.cc and C.cc (i.e., it needs B trans.)

So either their SConscript would be:
  Source('A.cc', add_tags=['B', 'D'])
  Source('B.cc', add_tags='B')
  Source('C.cc', add_tags='C')
  Source('D.cc', add_tags='D')

  GTest('A.test', 'A.test.cc', 'a.cc')
  GTest('B.test', 'B.test.cc', with_tag('B'))
  GTest('C.test', 'C.test.cc', with_any_tags('B', 'C'))
  GTest('D.test', 'D.test.cc', with_any_tags('B', 'C', 'D'))

or:
  Source('A.cc', add_tags=['B', 'C', 'D'])
  Source('B.cc', add_tags=['B', 'C', 'D'])
  Source('C.cc', add_tags=['C', 'D'])
  Source('D.cc', add_tags='D')

  GTest('A.test', 'A.test.cc', 'a.cc')
  GTest('B.test', 'B.test.cc', with_tag('B'))
  GTest('C.test', 'C.test.cc', with_tag('C'))
  GTest('D.test', 'D.test.cc', with_tag('D'))

This change makes it simpler. The tag should be added
only to the files directly included by the functionality
being tagged. Using the same example:

  Source('A.cc', add_tags=['B', 'D'])
  Source('B.cc', add_tags='B')
  Source('C.cc', add_tags='C')
  Source('D.cc', add_tags='D')

  env.TagImplies('B', 'A')
  env.TagImplies('C', 'B')
  env.TagImplies('D', ['A', 'C'])

  GTest('A.test', 'A.test.cc', 'a.cc')
  GTest('B.test', 'B.test.cc', with_tag('B'))
  GTest('C.test', 'C.test.cc', with_tag('C'))
  GTest('D.test', 'D.test.cc', with_tag('D'))

This also means that when a file no longer refers to
symbols from other file, or when it starts refering to
symbols from another file, one only needs to change
the dependencies of the tag directly being modified,
not the tags that rely on (imply) them. That is, on
the previous example, if C stops refering to symbols
from B, then tags that imply C do not need to be
modified:

  env.TagImplies('B','A')
  env.TagImplies('D', ['A', 'C'])

Change-Id: I5be07b01864f8d5df83f59002dfd2f01c73d5e09
Signed-off-by: Daniel R. Carvalho 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/43587
Reviewed-by: Gabe Black 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/SConscript
1 file changed, 97 insertions(+), 20 deletions(-)

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



diff --git a/src/SConscript b/src/SConscript
index 31fce0c..831f5c6 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -71,17 +71,91 @@
 # When specifying a source file of some type, a set of tags can be
 # specified for that file.

+def tag_implies(env, tag, tag_list):
+'''
+Associates a tag X to a list of tags which are implied by X.
+
+For example, assume:
+- Each file .cc is tagged with the tag "Tag ".
+- B.cc refers to symbols from A.cc
+- C.cc refers to symbols from B.cc
+- D.cc refers to symbols from A.cc and C.cc
+
+Then:
+- "Tag A" is implied by "Tag B"
+- "Tag B" is implied by "Tag C"
+- "Tag A" is transitively implied by "Tag C" (from "Tag B")
+- "Tag A" and "Tag C" are implied by "Tag D"
+- "Tag B" is transitively implied by "Tag D" (from "Tag C")
+- "Tag A" is transitively implied by "Tag D" (from transitive "Tag B")
+
+All of these implications are simply declared as:
+env.TagImplies("Tag B", "Tag A")
+env.TagImplies("Tag C", "Tag B")
+env.TagImplies("Tag D", ["Tag A", "Tag C"])
+
+So that any use of a tag will automatically include its transitive tags
+after being resolved.
+'''
+
+env.SetDefault(_tag_implies={})
+implications = env['_tag_implies']
+
+if isinstance(tag_list, str):
+tag_list = frozenset([tag_list])
+if not isinstance(tag_list, frozenset):
+tag_list = frozenset(tag_list)
+if tag in implications:
+implications[tag] |= tag_list
+else:
+implications[tag] = tag_list
+
+# Check if any of the tags on which the new tag depends on already
+# has a list of implications. If so, add the list to the new tag's
+# implications
+for t in tag_list:
+if t in implications:
+implications[tag] |= implications[t]
+
+# Check if another tag depends on this tag. If so, add this tag's
+# implications to that tag.
+for t,implied in implications.items():
+if tag in implied:
+implications[t] |= implications[tag]
+
+env.AddMethod(tag_implies, 'TagImplies')
+
+def resolve_tags(env, tags):
+'''
+Returns the complete set of tags implied (dependencies) by the
+  

[gem5-dev] Re: gem5 namespace

[Bobby Bruce via gem5-dev]

Nothing gets software devs as engaged as when discussing naming conventions
:).

I vote for "gem5" (lowercase) namespace, with all caps MACROS, but my sole
reason for this is I've grown to flinch whenever I see "Gem5", which I
admit isn't the best argument.

I echo Daniel's comment that I care more about having a rule and moving
past this than what that rule should be.

--
Dr. Bobby R. Bruce
Room 2235,
Kemper Hall, UC Davis
Davis,
CA, 95616

web: https://www.bobbybruce.net


On Sun, Apr 18, 2021 at 3:44 PM Daniel Carvalho via gem5-dev <
gem5-dev@gem5.org> wrote:

> Overall, there are way more uses of "gem5" than "Gem5" in the codebase,
> and most of the instances that break the identity rule could be easily
> fixed; however, there are cases that generate inconvenience: classes
> starting with lowercase, and situations where gem5 is in the middle of the
> var name (like "haveGem5Extension") make the code harder to
> read/understand. In this sense, the uppercase version generates better
> code.
>
>
> I still favor namespace gem5 - it'd be the "external external" API, i.e.
> we probably wouldn't be using it within src/ that much, and it would be
> used by other simulators and within our SystemC bridge (more on that later)
> - however, since we already have some exceptions, it wouldn't be the end
> of the world having it start with a capital letter.
>
>
> In the end, I personally do not care about which approach is taken, but
> the decision taken must be taken as a community. Therefore, it would be
> nice if we could have participation from other contributors to make the
> final decision less susceptible to changes/complaints in the future.
>
>
> Regarding when to use it:
> IMHO (and not thoroughly thought out), all .cc and .hh and objects within
> src/ should be subject to the namespace. Objects declared there are
> declared and maintained by gem5. Because of that there would probably be
> very few instances of namespace resolution within src/, so we should keep
> avoiding "using namespace" and being verbose about it. Finally, we also
> probably want to encourage users to define their objects inside the gem5
> namespace to make it less unlikely that they will give up on uploading
> their contributions due to the different styles, and the laziness to adapt
> code. This means that disturbance in user code would be minimal: they would
> simply add "namespace (G|g)em5 {" in the beginning and "} // namespace
> (G|g)em5" at the end, instead of multiple "(G)|gem5::" instances.
>
>
> Regards,
>
> Daniel
> For the record, if the namespaces you found using snake_case start with
> sc_, those are for systemc and are mandated by that standard. The one
> exception, sc_gem5, is one I made up which is closely associated with the
> other systemc namespaces and is named similarly to them for consistency.
>
> Gabe
>
> On Thu, Apr 15, 2021 at 1:51 AM Giacomo Travaglini <
> giacomo.travagl...@arm.com> wrote:
>
> My vote goes to 1 and A as well
>
> My sole argument is consistency; in general I'd rather start a namespace
> with a lowercase. So that when we have something
> like a scope resolution we know we are dealing with a namespace and not a
> class. But that's off-topic.
>
> Namespace names are anyway not covered by our coding style, so it's
> probably worth adding an entry.
>
> https://www.gem5.org/documentation/general_docs/development/coding_style/
>
> From a quick grep I can see most of our namespaces follow the PascalCase
> type, though there are some namespaces using snake_case convention.
>
> Giacomo
>
> > -Original Message-
> > From: Gabe Black via gem5-dev 
> > Sent: 15 April 2021 09:03
> > To: gem5 Developer List 
> > Cc: Gabe Black 
> > Subject: [gem5-dev] Re: gem5 namespace
> >
> > My vote is for 1 and A.
> >
> > We have style rules for a reason, and that is because not following them
> > causes technical problems like name collisions, and makes it less obvious
> > when reading code what things are and what they're doing. It's a bit
> > hypocritical to say that we should follow style rules and completely
> ignore
> > the aesthetic rule when capitalizing GEM5_, but then say that the
> aesthetic
> > rule should win when dealing with the namespace.
> >
> > This is further inconsistent with the Gem5Internal namespace, the Gem5
> > class in SCons, the Gem5Op instruction format used for ARM, and the
> > Gem5Imm constant used for ARM semihosting. It would also cause a
> collision
> > with any variable called gem5, a completely legal and reasonable name to
> use,
> > *especially* in code outside of gem5 which might be using it to refer to
> > something related to gem5 which it is interacting with.
> >
> > There are no other instances where we let superficial aesthetic
> conventions
> > like this overrule technical considerations. We don't add _tm to the end
> of
> > trademarked names, we don't call AtomicSimpleCPU the atomic simple CPU
> > since that's not a valid class name, and a hundred other examples of
> 

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

Hi Boris,

On 14/04/21 11:43 pm, Boris Shingarov wrote:
> Hi Sandipan,
> 
> I notice some of the commits (which were, if not blocking reviewing other 
> commits, but at least making comprehension of the whole body of commits 
> harder 
> for me) are ready for merge, for example
> https://gem5-review.googlesource.com/c/public/gem5/+/42943 
> 
> 
> Do you want to start merging (what Gerrit calls "submit") them?
> 

Sure. Sorry I could not respond earlier as I was on vacation but going forward,
I'll keep that in mind. Thanks for merging a few of the initial patches.


- Sandipan
___
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] Re: error from new test_convert.py?

[Bobby Bruce via gem5-dev]

The exception being thrown is "No module named 'm5'".  m5 is not in the
path, but could easily be added.

>From looking at https://gem5-review.googlesource.com/c/public/gem5/+/39377,
this was never intended to be run via `main.py`, though "test_convert.py"
has the "test" prefix so our TestLib framework tries to run it anyway.

My questions are: when were these tests meant to be run? Should they be
incorporated into the TestLib (i.e. those tests run via `main.py`) to be
run as a Presubmit, Nightly, or Weekly test? This would be easy to do (I
think I could incorporate this relatively quickly).

Kind regards,
Bobby
--
Dr. Bobby R. Bruce
Room 2235,
Kemper Hall, UC Davis
Davis,
CA, 95616

web: https://www.bobbybruce.net


On Sun, Apr 18, 2021 at 9:35 PM Gabe Black via gem5-dev 
wrote:

> Hi gem5 devs, specifically Andreas. I was just running the regressions on
> a change I'm working on, and I noticed this message:
>
> Exception thrown while loading
> "/home/gblack/gem5/work/tests/pyunit/util/test_convert.py"
> Ignoring all tests in this file.
>
> FYI, it looks like these tests are not runnng...
> ___
> 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 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] Gem5 CPU Model & ISA Addition

[Vignesh Manjunath via gem5-dev]

Hello Team

Good day!!!

I am a researcher at Pro2Future, GmbH under Technical University of Graz, 
Austria.

With an amazing platform like Gem5, I would like to use the framework to study 
the instruction behavior in the CPU based on a custom ISA and Pipeline model.

As a part of the same, I would like to know about the Gem5 architecture behind 
creating a pipeline model of a CPU, adding custom ISA to Gem5 to study the CPU 
behavior.

It would be great if I could learn about the Gem5 modules and I would use the 
same to add custom ISA to Gem5

Please let me know, if any such reference I can use or if I could get any 
support for the same.

Thank you in advance!!
Best Regards,
Vignesh Manjunath, MTech
Researcher (Area 4.1 - Cognitive Products)
Ph: +43-6889060774
Pro2Future GmbH
Altenberger Straße 69, 4040 Linz, Österreich

Pro2Future GmbH - Standort Graz:
Inffeldgasse 25F/1.OG, 8010 Graz

vignesh.manjun...@pro2future.at

___
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]: sim,cpu: Move the remote GDB stub into the workload.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: sim,cpu: Move the remote GDB stub into the workload.
..

sim,cpu: Move the remote GDB stub into the workload.

There are two user visible effects of this change. First, all of the
threads for a particular workload are moved under a single GDB instance.
The GDB session can see all the threads at once, and can let you move
between them as you want.

Second, since there is a GDB instance per workload and not per CPU, the
wait_for_gdb parameter was moved to the workload.

Change-Id: I510410c3cbb56e445b0fbb1def94c769d3a7b2e3
---
M src/cpu/BaseCPU.py
M src/cpu/base.cc
M src/cpu/base.hh
M src/sim/Workload.py
M src/sim/system.cc
M src/sim/system.hh
M src/sim/workload.cc
M src/sim/workload.hh
8 files changed, 42 insertions(+), 45 deletions(-)



diff --git a/src/cpu/BaseCPU.py b/src/cpu/BaseCPU.py
index cb43419..bd702e2 100644
--- a/src/cpu/BaseCPU.py
+++ b/src/cpu/BaseCPU.py
@@ -146,9 +146,6 @@
 do_statistics_insts = Param.Bool(True,
 "enable statistics pseudo instructions")

-wait_for_remote_gdb = Param.Bool(False,
-"Wait for a remote GDB connection");
-
 workload = VectorParam.Process([], "processes to run")

 mmu = Param.BaseMMU(ArchMMU(), "CPU memory management unit")
diff --git a/src/cpu/base.cc b/src/cpu/base.cc
index c40c001..3eaa1e8 100644
--- a/src/cpu/base.cc
+++ b/src/cpu/base.cc
@@ -724,12 +724,6 @@
 }
 }

-bool
-BaseCPU::waitForRemoteGDB() const
-{
-return params().wait_for_remote_gdb;
-}
-

 BaseCPU::GlobalStats::GlobalStats(::Stats::Group *parent)
 : ::Stats::Group(parent),
diff --git a/src/cpu/base.hh b/src/cpu/base.hh
index 6d324da..e7b2ccc 100644
--- a/src/cpu/base.hh
+++ b/src/cpu/base.hh
@@ -623,8 +623,6 @@
 return [tid];
 }

-bool waitForRemoteGDB() const;
-
 Cycles syscallRetryLatency;

   // Enables CPU to enter power gating on a configurable cycle count
diff --git a/src/sim/Workload.py b/src/sim/Workload.py
index 4f1b1b5..91c1c55 100644
--- a/src/sim/Workload.py
+++ b/src/sim/Workload.py
@@ -33,6 +33,9 @@
 cxx_header = "sim/workload.hh"
 abstract = True

+wait_for_remote_gdb = Param.Bool(False,
+"Wait for a remote GDB connection");
+
 class KernelWorkload(Workload):
 type = 'KernelWorkload'
 cxx_header = "sim/kernel_workload.hh"
diff --git a/src/sim/system.cc b/src/sim/system.cc
index 9fd312c..1d39f36 100644
--- a/src/sim/system.cc
+++ b/src/sim/system.cc
@@ -43,7 +43,6 @@

 #include 

-#include "arch/remote_gdb.hh"
 #include "base/compiler.hh"
 #include "base/loader/object_file.hh"
 #include "base/loader/symtab.hh"
@@ -124,13 +123,6 @@
 // been reallocated.
 t.resumeEvent = new EventFunctionWrapper(
 [this, id](){ thread(id).resume(); }, sys->name());
-#   if THE_ISA != NULL_ISA
-int port = getRemoteGDBPort();
-if (port) {
-t.gdb = new TheISA::RemoteGDB(sys, tc, port + id);
-t.gdb->listen();
-}
-#   endif
 }

 void
@@ -138,8 +130,6 @@
 {
 auto  = thread(id);
 panic_if(!t.context, "Can't replace a context which doesn't exist.");
-if (t.gdb)
-t.gdb->replaceThreadContext(tc);
 #   if THE_ISA != NULL_ISA
 if (t.resumeEvent->scheduled()) {
 Tick when = t.resumeEvent->when();
@@ -262,26 +252,6 @@
 delete workItemStats[j];
 }

-void
-System::startup()
-{
-SimObject::startup();
-
-// Now that we're about to start simulation, wait for GDB connections  
if

-// requested.
-#if THE_ISA != NULL_ISA
-for (int i = 0; i < threads.size(); i++) {
-auto *gdb = threads.thread(i).gdb;
-auto *cpu = threads[i]->getCpuPtr();
-if (gdb && cpu->waitForRemoteGDB()) {
-inform("%s: Waiting for a remote GDB connection on port %d.",
-   cpu->name(), gdb->port());
-gdb->connect();
-}
-}
-#endif
-}
-
 Port &
 System::getPort(const std::string _name, PortID idx)
 {
diff --git a/src/sim/system.hh b/src/sim/system.hh
index 6613217..37acf2c 100644
--- a/src/sim/system.hh
+++ b/src/sim/system.hh
@@ -118,7 +118,6 @@
 {
 ThreadContext *context = nullptr;
 bool active = false;
-BaseRemoteGDB *gdb = nullptr;
 Event *resumeEvent = nullptr;

 void resume();
@@ -230,8 +229,6 @@
 const_iterator end() const { return const_iterator(*this, size());  
}

 };

-void startup() override;
-
 /**
  * Get a reference to the system port that can be used by
  * non-structural simulation objects like processes or threads, or
diff --git a/src/sim/workload.cc b/src/sim/workload.cc
index c7dfcd4..989305b 100644
--- a/src/sim/workload.cc
+++ b/src/sim/workload.cc
@@ -27,7 +27,9 @@

 #include "sim/workload.hh"

+#include "arch/remote_gdb.hh"
 #include "cpu/thread_context.hh"
+#include 

[gem5-dev] Change in gem5/gem5[develop]: arch,base,sim: Construct the GDB stub with no threads.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: arch,base,sim: Construct the GDB stub with no threads.
..

arch,base,sim: Construct the GDB stub with no threads.

By moving the installation of even the first ThreadContext out of the
constructor, it's possible to construct the stub separately. We can then
move the code that creates the stub out of the base class and into
architecture specific sub-classes.

Change-Id: I0dfd53a3135ebc98ec49acf81d83e58830bc365c
---
M src/arch/arm/remote_gdb.cc
M src/arch/arm/remote_gdb.hh
M src/arch/mips/remote_gdb.cc
M src/arch/mips/remote_gdb.hh
M src/arch/power/remote_gdb.cc
M src/arch/power/remote_gdb.hh
M src/arch/riscv/remote_gdb.cc
M src/arch/riscv/remote_gdb.hh
M src/arch/sparc/remote_gdb.cc
M src/arch/sparc/remote_gdb.hh
M src/arch/x86/remote_gdb.cc
M src/arch/x86/remote_gdb.hh
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
M src/sim/system.cc
M src/sim/workload.cc
M src/sim/workload.hh
17 files changed, 44 insertions(+), 33 deletions(-)



diff --git a/src/arch/arm/remote_gdb.cc b/src/arch/arm/remote_gdb.cc
index 96344a9..c0c1e0f 100644
--- a/src/arch/arm/remote_gdb.cc
+++ b/src/arch/arm/remote_gdb.cc
@@ -184,8 +184,8 @@
mmu->translateFunctional(req, tc, BaseTLB::Execute) == NoFault;
 }

-RemoteGDB::RemoteGDB(System *_system, ThreadContext *tc, int _port)
-: BaseRemoteGDB(_system, tc, _port), regCache32(this), regCache64(this)
+RemoteGDB::RemoteGDB(System *_system, int _port)
+: BaseRemoteGDB(_system, _port), regCache32(this), regCache64(this)
 {
 }

diff --git a/src/arch/arm/remote_gdb.hh b/src/arch/arm/remote_gdb.hh
index 112056a..84859da 100644
--- a/src/arch/arm/remote_gdb.hh
+++ b/src/arch/arm/remote_gdb.hh
@@ -115,7 +115,7 @@
 AArch64GdbRegCache regCache64;

   public:
-RemoteGDB(System *_system, ThreadContext *tc, int _port);
+RemoteGDB(System *_system, int _port);
 BaseGdbRegCache *gdbRegs();
 std::vector
 availableFeatures() const
diff --git a/src/arch/mips/remote_gdb.cc b/src/arch/mips/remote_gdb.cc
index 30efb36..6f869fc 100644
--- a/src/arch/mips/remote_gdb.cc
+++ b/src/arch/mips/remote_gdb.cc
@@ -148,8 +148,8 @@

 using namespace MipsISA;

-RemoteGDB::RemoteGDB(System *_system, ThreadContext *tc, int _port)
-: BaseRemoteGDB(_system, tc, _port), regCache(this)
+RemoteGDB::RemoteGDB(System *_system, int _port)
+: BaseRemoteGDB(_system, _port), regCache(this)
 {
 }

diff --git a/src/arch/mips/remote_gdb.hh b/src/arch/mips/remote_gdb.hh
index a8e7c1f..8fb54f7 100644
--- a/src/arch/mips/remote_gdb.hh
+++ b/src/arch/mips/remote_gdb.hh
@@ -77,7 +77,7 @@
 MipsGdbRegCache regCache;

   public:
-RemoteGDB(System *_system, ThreadContext *tc, int _port);
+RemoteGDB(System *_system, int _port);
 BaseGdbRegCache *gdbRegs();
 std::vector
 availableFeatures() const
diff --git a/src/arch/power/remote_gdb.cc b/src/arch/power/remote_gdb.cc
index ce9976f..860a233 100644
--- a/src/arch/power/remote_gdb.cc
+++ b/src/arch/power/remote_gdb.cc
@@ -145,8 +145,8 @@

 using namespace PowerISA;

-RemoteGDB::RemoteGDB(System *_system, ThreadContext *tc, int _port)
-: BaseRemoteGDB(_system, tc, _port), regCache(this)
+RemoteGDB::RemoteGDB(System *_system, int _port)
+: BaseRemoteGDB(_system, _port), regCache(this)
 {
 }

diff --git a/src/arch/power/remote_gdb.hh b/src/arch/power/remote_gdb.hh
index 4f46ca5..ab5037e 100644
--- a/src/arch/power/remote_gdb.hh
+++ b/src/arch/power/remote_gdb.hh
@@ -76,7 +76,7 @@
 PowerGdbRegCache regCache;

   public:
-RemoteGDB(System *_system, ThreadContext *tc, int _port);
+RemoteGDB(System *_system, int _port);
 BaseGdbRegCache *gdbRegs();
 std::vector
 availableFeatures() const
diff --git a/src/arch/riscv/remote_gdb.cc b/src/arch/riscv/remote_gdb.cc
index da78957..b339892 100644
--- a/src/arch/riscv/remote_gdb.cc
+++ b/src/arch/riscv/remote_gdb.cc
@@ -150,8 +150,8 @@

 using namespace RiscvISA;

-RemoteGDB::RemoteGDB(System *_system, ThreadContext *tc, int _port)
-: BaseRemoteGDB(_system, tc, _port), regCache(this)
+RemoteGDB::RemoteGDB(System *_system, int _port)
+: BaseRemoteGDB(_system, _port), regCache(this)
 {
 }

diff --git a/src/arch/riscv/remote_gdb.hh b/src/arch/riscv/remote_gdb.hh
index fe6503b..f1ff6c1 100644
--- a/src/arch/riscv/remote_gdb.hh
+++ b/src/arch/riscv/remote_gdb.hh
@@ -141,7 +141,7 @@
 RiscvGdbRegCache regCache;

   public:
-RemoteGDB(System *_system, ThreadContext *tc, int _port);
+RemoteGDB(System *_system, int _port);
 BaseGdbRegCache *gdbRegs() override;
 /**
  * Informs GDB remote serial protocol that XML features are supported
diff --git a/src/arch/sparc/remote_gdb.cc b/src/arch/sparc/remote_gdb.cc
index 1a1042d..53d2250 100644
--- a/src/arch/sparc/remote_gdb.cc
+++ b/src/arch/sparc/remote_gdb.cc
@@ -144,8 +144,8 @@

 using 

[gem5-dev] Change in gem5/gem5[develop]: arch,base,sim: Move GDB stub creation into the arch Workloads.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: arch,base,sim: Move GDB stub creation into the arch  
Workloads.

..

arch,base,sim: Move GDB stub creation into the arch Workloads.

These classes know what flavor of GDB stub they want, so there's no need
for TheISA:: to multiplex.

Change-Id: Ia428fe391719a3320d865421fc59352a17875bcf
---
M src/arch/arm/fs_workload.hh
M src/arch/arm/se_workload.hh
M src/arch/mips/se_workload.hh
M src/arch/power/se_workload.hh
M src/arch/riscv/bare_metal/fs_workload.hh
M src/arch/riscv/linux/fs_workload.hh
M src/arch/riscv/se_workload.hh
M src/arch/sparc/fs_workload.hh
M src/arch/sparc/se_workload.hh
M src/arch/x86/fs_workload.hh
M src/arch/x86/linux/se_workload.hh
M src/base/remote_gdb.hh
M src/sim/workload.cc
M src/sim/workload.hh
14 files changed, 104 insertions(+), 14 deletions(-)



diff --git a/src/arch/arm/fs_workload.hh b/src/arch/arm/fs_workload.hh
index d14c8bc..c85f867 100644
--- a/src/arch/arm/fs_workload.hh
+++ b/src/arch/arm/fs_workload.hh
@@ -46,6 +46,7 @@

 #include "arch/arm/aapcs32.hh"
 #include "arch/arm/aapcs64.hh"
+#include "arch/arm/remote_gdb.hh"
 #include "kern/linux/events.hh"
 #include "params/ArmFsWorkload.hh"
 #include "sim/kernel_workload.hh"
@@ -143,6 +144,13 @@

 void initState() override;

+void
+setSystem(System *sys) override
+{
+KernelWorkload::setSystem(sys);
+gdb = BaseRemoteGDB::build(system);
+}
+
 Addr
 fixFuncEventAddr(Addr addr) const override
 {
diff --git a/src/arch/arm/se_workload.hh b/src/arch/arm/se_workload.hh
index 70a85b0..b14f949 100644
--- a/src/arch/arm/se_workload.hh
+++ b/src/arch/arm/se_workload.hh
@@ -29,6 +29,7 @@
 #define __ARCH_ARM_SE_WORKLOAD_HH__

 #include "arch/arm/reg_abi.hh"
+#include "arch/arm/remote_gdb.hh"
 #include "params/ArmSEWorkload.hh"
 #include "sim/se_workload.hh"

@@ -42,6 +43,13 @@

 SEWorkload(const Params ) : ::SEWorkload(p) {}

+void
+setSystem(System *sys) override
+{
+::SEWorkload::setSystem(sys);
+gdb = BaseRemoteGDB::build(system);
+}
+
 ::Loader::Arch getArch() const override { return ::Loader::Arm64; }

 using SyscallABI32 = RegABI32;
diff --git a/src/arch/mips/se_workload.hh b/src/arch/mips/se_workload.hh
index 63eba3c..e1dd03f 100644
--- a/src/arch/mips/se_workload.hh
+++ b/src/arch/mips/se_workload.hh
@@ -29,6 +29,7 @@
 #define __ARCH_MIPS_SE_WORKLOAD_HH__

 #include "arch/mips/regs/int.hh"
+#include "arch/mips/remote_gdb.hh"
 #include "params/MipsSEWorkload.hh"
 #include "sim/se_workload.hh"
 #include "sim/syscall_abi.hh"
@@ -44,6 +45,13 @@

 SEWorkload(const Params ) : ::SEWorkload(p) {}

+void
+setSystem(System *sys) override
+{
+::SEWorkload::setSystem(sys);
+gdb = BaseRemoteGDB::build(system);
+}
+
 ::Loader::Arch getArch() const override { return ::Loader::Mips; }

 struct SyscallABI : public GenericSyscallABI64
diff --git a/src/arch/power/se_workload.hh b/src/arch/power/se_workload.hh
index ac79151..2305538 100644
--- a/src/arch/power/se_workload.hh
+++ b/src/arch/power/se_workload.hh
@@ -30,6 +30,7 @@

 #include "arch/power/regs/int.hh"
 #include "arch/power/regs/misc.hh"
+#include "arch/power/remote_gdb.hh"
 #include "params/PowerSEWorkload.hh"
 #include "sim/se_workload.hh"
 #include "sim/syscall_abi.hh"
@@ -44,6 +45,13 @@
 using Params = PowerSEWorkloadParams;
 SEWorkload(const Params ) : ::SEWorkload(p) {}

+void
+setSystem(System *sys) override
+{
+::SEWorkload::setSystem(sys);
+gdb = BaseRemoteGDB::build(system);
+}
+
 ::Loader::Arch getArch() const override { return ::Loader::Power; }

 struct SyscallABI : public GenericSyscallABI64
diff --git a/src/arch/riscv/bare_metal/fs_workload.hh  
b/src/arch/riscv/bare_metal/fs_workload.hh

index aa86cca..d7545f2 100644
--- a/src/arch/riscv/bare_metal/fs_workload.hh
+++ b/src/arch/riscv/bare_metal/fs_workload.hh
@@ -29,6 +29,7 @@
 #ifndef __ARCH_RISCV_BARE_METAL_SYSTEM_HH__
 #define __ARCH_RISCV_BARE_METAL_SYSTEM_HH__

+#include "arch/riscv/remote_gdb.hh"
 #include "params/RiscvBareMetal.hh"
 #include "sim/workload.hh"

@@ -52,6 +53,13 @@

 void initState() override;

+void
+setSystem(System *sys) override
+{
+Workload::setSystem(sys);
+gdb = BaseRemoteGDB::build(system);
+}
+
 Loader::Arch getArch() const override { return bootloader->getArch(); }
 const Loader::SymbolTable &
 symtab(ThreadContext *tc) override
diff --git a/src/arch/riscv/linux/fs_workload.hh  
b/src/arch/riscv/linux/fs_workload.hh

index 84b0e1d..d891349 100644
--- a/src/arch/riscv/linux/fs_workload.hh
+++ b/src/arch/riscv/linux/fs_workload.hh
@@ -29,6 +29,7 @@
 #ifndef __ARCH_RISCV_LINUX_SYSTEM_HH__
 #define __ARCH_RISCV_LINUX_SYSTEM_HH__

+#include "arch/riscv/remote_gdb.hh"
 

[gem5-dev] Change in gem5/gem5[develop]: sim: Track ThreadContext-s in the Workload object.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: sim: Track ThreadContext-s in the Workload object.
..

sim: Track ThreadContext-s in the Workload object.

Change-Id: I00bf9fa36d3993f55d41e50196ad8a89a3d506c4
---
M src/sim/SConscript
M src/sim/system.cc
A src/sim/workload.cc
M src/sim/workload.hh
4 files changed, 75 insertions(+), 0 deletions(-)



diff --git a/src/sim/SConscript b/src/sim/SConscript
index fe18d24..fb87bfe 100644
--- a/src/sim/SConscript
+++ b/src/sim/SConscript
@@ -83,6 +83,7 @@
 Source('power_state.cc')
 Source('power_domain.cc')
 Source('stats.cc')
+Source('workload.cc')

 GTest('byteswap.test', 'byteswap.test.cc', '../base/types.cc')
 GTest('guest_abi.test', 'guest_abi.test.cc')
diff --git a/src/sim/system.cc b/src/sim/system.cc
index 7a3e248..9fd312c 100644
--- a/src/sim/system.cc
+++ b/src/sim/system.cc
@@ -301,6 +301,9 @@
 {
 threads.insert(tc, assigned);

+if (workload)
+workload->registerThreadContext(tc);
+
 for (auto *e: liveEvents)
 tc->schedule(e);
 }
@@ -331,6 +334,9 @@
 auto *otc = threads[context_id];
 threads.replace(tc, context_id);

+if (workload)
+workload->replaceThreadContext(tc);
+
 for (auto *e: liveEvents) {
 otc->remove(e);
 tc->schedule(e);
diff --git a/src/sim/workload.cc b/src/sim/workload.cc
new file mode 100644
index 000..c7dfcd4
--- /dev/null
+++ b/src/sim/workload.cc
@@ -0,0 +1,60 @@
+/*
+ * Copyright 2021 Google Inc.
+ *
+ * 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.
+ */
+
+#include "sim/workload.hh"
+
+#include "cpu/thread_context.hh"
+
+void
+Workload::registerThreadContext(ThreadContext *tc)
+{
+std::set::iterator it;
+bool success;
+std::tie(it, success) = threads.insert(tc);
+panic_if(!success, "Failed to add thread context %d.",
+tc->contextId());
+}
+
+void
+Workload::replaceThreadContext(ThreadContext *tc)
+{
+ContextID id = tc->contextId();
+
+for (auto *old: threads) {
+if (old->contextId() != id)
+continue;
+threads.erase(old);
+
+std::set::iterator it;
+bool success;
+std::tie(it, success) = threads.insert(tc);
+panic_if(!success,
+"Failed to insert replacement thread context %d.", id);
+return;
+}
+panic("Replacement thread context %d doesn't match any known id.", id);
+}
diff --git a/src/sim/workload.hh b/src/sim/workload.hh
index 3458af7..f3b55ad 100644
--- a/src/sim/workload.hh
+++ b/src/sim/workload.hh
@@ -28,6 +28,9 @@
 #ifndef __SIM_WORKLOAD_HH__
 #define __SIM_WORKLOAD_HH__

+#include 
+#include 
+
 #include "base/loader/object_file.hh"
 #include "base/loader/symtab.hh"
 #include "params/Workload.hh"
@@ -63,6 +66,8 @@
 {}
 } stats;

+std::set threads;
+
   public:
 Workload(const WorkloadParams ) : SimObject(params), stats(this)
 {}
@@ -72,6 +77,9 @@

 System *system = nullptr;

+virtual void registerThreadContext(ThreadContext *tc);
+virtual void replaceThreadContext(ThreadContext *tc);
+
 virtual Addr getEntry() const = 0;
 virtual Loader::Arch getArch() const = 0;


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44616
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: 

[gem5-dev] Change in gem5/gem5[develop]: arch: Eliminate the now unused remote_gdb.hh switching header.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: arch: Eliminate the now unused remote_gdb.hh switching  
header.

..

arch: Eliminate the now unused remote_gdb.hh switching header.

Change-Id: Ideb04254a65670350d319c2273539d8bc9e48d70
---
M src/arch/SConscript
1 file changed, 0 insertions(+), 1 deletion(-)



diff --git a/src/arch/SConscript b/src/arch/SConscript
index 73c6afb..952bac6 100644
--- a/src/arch/SConscript
+++ b/src/arch/SConscript
@@ -62,7 +62,6 @@
 locked_mem.hh
 page_size.hh
 registers.hh
-remote_gdb.hh
 types.hh
 '''),
 env.subst('${TARGET_ISA}'))

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44620
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: Ideb04254a65670350d319c2273539d8bc9e48d70
Gerrit-Change-Number: 44620
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
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,sim: Set ThreadContext's ContextID right away.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: cpu,sim: Set ThreadContext's ContextID right away.
..

cpu,sim: Set ThreadContext's ContextID right away.

The code which registers thread contexts originally returned the ID that
it had chosen, and let the CPU actually set the ID in the object itself.
That opened a window where calling contextId() on the ThreadContext
would return the wrong answer.

Instead, we can just set the ID immediately after it's decided. This
also localizes that logic and removes plumbing for the ID between that
decision and where it's actually applied.

Change-Id: I31ad84c3f9bf6f5b6f72457ca640ea929b24f6a0
---
M src/cpu/base.cc
M src/sim/system.cc
M src/sim/system.hh
3 files changed, 9 insertions(+), 11 deletions(-)



diff --git a/src/cpu/base.cc b/src/cpu/base.cc
index f98837c..c40c001 100644
--- a/src/cpu/base.cc
+++ b/src/cpu/base.cc
@@ -431,9 +431,9 @@
 ThreadContext *tc = threadContexts[tid];

 if (system->multiThread) {
-tc->setContextId(system->registerThreadContext(tc));
+system->registerThreadContext(tc);
 } else {
-tc->setContextId(system->registerThreadContext(tc, _cpuId));
+system->registerThreadContext(tc, _cpuId);
 }

 if (!FullSystem)
diff --git a/src/sim/system.cc b/src/sim/system.cc
index 0c42256..7a3e248 100644
--- a/src/sim/system.cc
+++ b/src/sim/system.cc
@@ -98,7 +98,7 @@
 workload->recordQuiesce();
 }

-ContextID
+void
 System::Threads::insert(ThreadContext *tc, ContextID id)
 {
 if (id == InvalidContextID) {
@@ -108,6 +108,8 @@
 }
 }

+tc->setContextId(id);
+
 if (id >= size())
 threads.resize(id + 1);

@@ -129,8 +131,6 @@
 t.gdb->listen();
 }
 #   endif
-
-return id;
 }

 void
@@ -296,15 +296,13 @@
 memoryMode = mode;
 }

-ContextID
+void
 System::registerThreadContext(ThreadContext *tc, ContextID assigned)
 {
-ContextID id = threads.insert(tc, assigned);
+threads.insert(tc, assigned);

 for (auto *e: liveEvents)
 tc->schedule(e);
-
-return id;
 }

 bool
diff --git a/src/sim/system.hh b/src/sim/system.hh
index 480b523..6613217 100644
--- a/src/sim/system.hh
+++ b/src/sim/system.hh
@@ -142,7 +142,7 @@
 return threads[id];
 }

-ContextID insert(ThreadContext *tc, ContextID id=InvalidContextID);
+void insert(ThreadContext *tc, ContextID id=InvalidContextID);
 void replace(ThreadContext *tc, ContextID id);

 friend class System;
@@ -586,7 +586,7 @@
 /// @return Starting address of first page
 Addr allocPhysPages(int npages);

-ContextID registerThreadContext(
+void registerThreadContext(
 ThreadContext *tc, ContextID assigned=InvalidContextID);
 void replaceThreadContext(ThreadContext *tc, ContextID context_id);


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44615
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: I31ad84c3f9bf6f5b6f72457ca640ea929b24f6a0
Gerrit-Change-Number: 44615
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
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: When switching remote GDB threads, re-align on an inst boundary.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: base: When switching remote GDB threads, re-align on an  
inst boundary.

..

base: When switching remote GDB threads, re-align on an inst boundary.

Change-Id: I199542d6e815f05b25a92d1c5ce0f6b5436dc16a
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 12 insertions(+), 4 deletions(-)



diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 96b09be..69e28ee 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -351,8 +351,7 @@

 BaseRemoteGDB::BaseRemoteGDB(System *_system, ThreadContext *c, int  
_port) :

 connectEvent(nullptr), dataEvent(nullptr), _port(_port), fd(-1),
-active(false), attached(false), sys(_system),
-trapEvent(this), singleStepEvent(*this)
+sys(_system), trapEvent(this), singleStepEvent(*this)
 {
 addThreadContext(c);
 }
@@ -498,6 +497,10 @@
  */
 if (!active) {
 active = true;
+} else if (threadSwitching) {
+threadSwitching = false;
+// Tell GDB the thread switch has completed.
+send("OK");
 } else {
 // Tell remote host that an exception has occurred.
 send("S%02x", type);
@@ -965,6 +968,10 @@
 if (!any && tid != tc->contextId()) {
 if (!selectThreadContext(tid))
 throw CmdError("E04");
+// Line up on an instruction boundary in the new thread.
+threadSwitching = true;
+scheduleInstCommitEvent(, 0);
+return false;
 }
 } else {
 throw CmdError("E05");
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index 93a6026..3371a95 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -225,8 +225,9 @@
 /*
  * Simulator side debugger state.
  */
-bool active;
-bool attached;
+bool active = false;
+bool attached = false;
+bool threadSwitching = false;

 System *sys;


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44613
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: I199542d6e815f05b25a92d1c5ce0f6b5436dc16a
Gerrit-Change-Number: 44613
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
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: Improve handling of thread IDs for remote GDB.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: base: Improve handling of thread IDs for remote GDB.
..

base: Improve handling of thread IDs for remote GDB.

The remote GDB protocol encode thread IDs as positive integers, where 0
is a special value which indicates "pick any thread", and -1 is a
special value which indicates all threads.

The previous implementation would look like it worked handling the
special value -1 (for instance) because it see the '-' of "-1", stop
looking for digits, and return the default value 0.

This new implementation handles -1 as a special case, and will report an
error if no digits were found otherwise.

Also this change adds an encodeThreadId method to convert a ContextID
into a GDB thread ID by adding one to avoid the special value 0.

Change-Id: Iec54fbd9563d20a56011f48d50d69111ed1467b8
---
M src/base/remote_gdb.cc
1 file changed, 44 insertions(+), 3 deletions(-)



diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 9278eb0..3fc5240 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -283,6 +283,43 @@
 return r;
 }

+bool
+parseThreadId(const char **srcp, bool , bool , ContextID )
+{
+all = any = false;
+tid = 0;
+const char *src = *srcp;
+if (*src == '-') {
+// This could be the start of -1, which means all threads.
+src++;
+if (*src++ != '1')
+return false;
+*srcp += 2;
+all = true;
+return true;
+}
+tid = hex2i(srcp);
+// If *srcp still points to src, no characters were consumed and no  
thread
+// id was found. Without this check, we can't tell the difference  
between

+// zero and a parsing error.
+if (*srcp == src)
+return false;
+
+if (tid == 0)
+any = true;
+
+tid--;
+
+return true;
+}
+
+int
+encodeThreadId(ContextID id)
+{
+// Thread ID 0 is reserved and means "pick any thread".
+return id + 1;
+}
+
 enum GdbBreakpointType
 {
 GdbSoftBp = '0',
@@ -864,8 +901,12 @@
 BaseRemoteGDB::cmdSetThread(GdbCommand::Context )
 {
 const char *p = ctx.data + 1; // Ignore the subcommand byte.
-if (hex2i() != tc->contextId())
+ContextID tid = 0;
+bool all, any;
+if (!parseThreadId(, all, any, tid))
 throw CmdError("E01");
+if (any || all || tid != tc->contextId())
+throw CmdError("E02");
 send("OK");
 return true;
 }
@@ -945,7 +986,7 @@
 void
 BaseRemoteGDB::queryC(QuerySetCommand::Context )
 {
-send(csprintf("QC%x", tc->contextId()).c_str());
+send(csprintf("QC%x", encodeThreadId(tc->contextId())).c_str());
 }

 void
@@ -1005,7 +1046,7 @@
 void
 BaseRemoteGDB::queryFThreadInfo(QuerySetCommand::Context )
 {
-send(csprintf("m%x", tc->contextId()).c_str());
+send(csprintf("m%x", encodeThreadId(tc->contextId())).c_str());
 }

 void

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44606
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: Iec54fbd9563d20a56011f48d50d69111ed1467b8
Gerrit-Change-Number: 44606
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
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: Streamline the "send" method of the BaseRemoteGDB class.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: base: Streamline the "send" method of the BaseRemoteGDB  
class.

..

base: Streamline the "send" method of the BaseRemoteGDB class.

The existing send method takes a const char *, but frequently the class
wants to use it to send a std::string, also frequently after generating
that string with csprintf. Rather than force each call sight to add a
.c_str() and call csprintf, this change adds helpers which will accept a
std::string and call c_str for you, or accept a format const char * and
arguments and call csprintf for you (and then call .c_str() on the
result).

Change-Id: Ifcef5e09f6469322c6040374209972528c80fb25
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 16 insertions(+), 7 deletions(-)



diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 3fc5240..16373e6 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -466,7 +466,7 @@
 active = true;
 } else {
 // Tell remote host that an exception has occurred.
-send(csprintf("S%02x", type).c_str());
+send("S%02x", type);
 }

 // Stick frame regs into our reg cache.
@@ -506,7 +506,7 @@
 } catch (Unsupported ) {
 send("");
 } catch (CmdError ) {
-send(e.error.c_str());
+send(e.error);
 } catch (...) {
 panic("Unrecognzied GDB exception.");
 }
@@ -837,7 +837,7 @@
 bool
 BaseRemoteGDB::cmdSignal(GdbCommand::Context )
 {
-send(csprintf("S%02x", ctx.type).c_str());
+send("S%02x", ctx.type);
 return true;
 }

@@ -986,7 +986,7 @@
 void
 BaseRemoteGDB::queryC(QuerySetCommand::Context )
 {
-send(csprintf("QC%x", encodeThreadId(tc->contextId())).c_str());
+send("QC%x", encodeThreadId(tc->contextId()));
 }

 void
@@ -999,7 +999,7 @@
 oss << "PacketSize=1024";
 for (const auto& feature : availableFeatures())
 oss << ';' << feature;
-send(oss.str().c_str());
+send(oss.str());
 }

 void
@@ -1040,13 +1040,13 @@

 std::string encoded;
 encodeXferResponse(content, encoded, offset, length);
-send(encoded.c_str());
+send(encoded);
 }

 void
 BaseRemoteGDB::queryFThreadInfo(QuerySetCommand::Context )
 {
-send(csprintf("m%x", encodeThreadId(tc->contextId())).c_str());
+send("m%x", encodeThreadId(tc->contextId()));
 }

 void
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index 0d67b09..cfd6b3d 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -51,6 +51,7 @@
 #include 

 #include "arch/types.hh"
+#include "base/cprintf.hh"
 #include "base/pollevent.hh"
 #include "base/socket.hh"
 #include "base/types.hh"
@@ -203,6 +204,14 @@

 void recv(std::vector );
 void send(const char *data);
+void send(const std::string ) { send(data.c_str()); }
+
+template 
+void
+send(const char *format, const Args &...args)
+{
+send(csprintf(format, args...));
+}

 /*
  * Simulator side debugger state.

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44607
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: Ifcef5e09f6469322c6040374209972528c80fb25
Gerrit-Change-Number: 44607
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
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-x86: If possible, use the workload to pick GDB arch.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: arch-x86: If possible, use the workload to pick GDB arch.
..

arch-x86: If possible, use the workload to pick GDB arch.

When using remote GDB to debug an x86 simulated system within gem5, the
stub within gem5 needs to decide what arch the GDB instance expects.
That determines what format the blob of data with register values should
be.

Previously, gem5 would make that decision based on the current mode of
the target thread context. If the target was currently executing in 64
bit mode, that would imply that GDB was expecting 64 bit registers. If
not, then it was probably trying to debug a 32 bit program and would
expect 32 bit registers.

That works in many circumstances, but won't work if, for instance, a CPU
has not yet been initialized and is not running in its final, typical
mode, or if it's dipped into another mode to, for instance, run a user
mode program which is 32 bit under a 64 bit kernel.

This change modifies the GDB stub to first try to use the workload
object to determine what arch the GDB instance is most likely to assume.
This is a reasonably accurate representation for the arch GDB expects,
even though there isn't a direct, enforced link. It would be best if GDB
could just tell us what it expected, but I wasn't able to find any way
to get it to do that.

In most (all?) cases where someone would be using GDB to debug the guest
there will be a workload, and that workload will have a well defined
architecture. Since that isn't technically required though, this change
will still fall back to the old detection mechanism if it can't tell
from the workload, or if there is no workload in the first place.

Later revisions of the GDB interface may tie the remote GDB stub to the
workload object itself, in which case it *will* be possible to assume
that a workload object exists, and the workload object will be able to
explicitly select what GDB stub to use based on what it's running. In
the mean time, this seems like a fairly robust approximation of that.

Change-Id: I5059d48c28380e2fee5629d832acf95063a1a27a
---
M src/arch/x86/remote_gdb.cc
1 file changed, 17 insertions(+), 0 deletions(-)



diff --git a/src/arch/x86/remote_gdb.cc b/src/arch/x86/remote_gdb.cc
index c2d622e..39d467b 100644
--- a/src/arch/x86/remote_gdb.cc
+++ b/src/arch/x86/remote_gdb.cc
@@ -49,6 +49,7 @@
 #include "arch/x86/process.hh"
 #include "arch/x86/regs/int.hh"
 #include "arch/x86/regs/misc.hh"
+#include "base/loader/object_file.hh"
 #include "base/remote_gdb.hh"
 #include "base/socket.hh"
 #include "base/trace.hh"
@@ -57,6 +58,7 @@
 #include "debug/GDBAcc.hh"
 #include "mem/page_table.hh"
 #include "sim/full_system.hh"
+#include "sim/workload.hh"

 using namespace X86ISA;

@@ -91,6 +93,21 @@
 BaseGdbRegCache*
 RemoteGDB::gdbRegs()
 {
+// First, try to figure out which type of register cache to return  
based

+// on the architecture reported by the workload.
+if (system()->workload) {
+auto arch = system()->workload->getArch();
+if (arch == Loader::X86_64) {
+return 
+} else if (arch == Loader::I386) {
+return 
+} else if (arch != Loader::UnknownArch) {
+panic("Unrecognized workload arch %s.",
+Loader::archToString(arch));
+}
+}
+
+// If that didn't work, decide based on the current mode of the  
context.

 HandyM5Reg m5reg = context()->readMiscRegNoEffect(MISCREG_M5_REG);
 if (m5reg.submode == SixtyFourBitMode)
 return 

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44614
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: I5059d48c28380e2fee5629d832acf95063a1a27a
Gerrit-Change-Number: 44614
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
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: Don't wait for an inactive CPU for remote GDB.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: base: Don't wait for an inactive CPU for remote GDB.
..

base: Don't wait for an inactive CPU for remote GDB.

When connecting to a thread, the remote GDB stub will try to wait for an
instruction boundary before proceeding. Since the CPU the thread context
is attached to may be inactive, it may not get around to reaching an
instruction boundary, and the event may not happen for an indefinite
period of time.

Instead, assume that inactive CPUs are already at instruction
boundaries, and trigger the event manually.

Change-Id: I9a67a49f9a52bdf9b1f0b88a1d173aa2bdfb5a16
---
M src/base/remote_gdb.cc
1 file changed, 10 insertions(+), 3 deletions(-)



diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 20b26b9..96b09be 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -784,9 +784,16 @@
 void
 BaseRemoteGDB::scheduleInstCommitEvent(Event *ev, int delta)
 {
-// Here "ticks" aren't simulator ticks which measure time, they're
-// instructions committed by the CPU.
-tc->scheduleInstCountEvent(ev, tc->getCurrentInstCount() + delta);
+if (delta == 0 && tc->status() != ThreadContext::Active) {
+// If delta is zero, we're just trying to wait for an instruction
+// boundary. If the CPU is not active, assume we're already at a
+// boundary without waiting for the CPU to eventually wake up.
+ev->process();
+} else {
+// Here "ticks" aren't simulator ticks which measure time, they're
+// instructions committed by the CPU.
+tc->scheduleInstCountEvent(ev, tc->getCurrentInstCount() + delta);
+}
 }

 void

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44612
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: I9a67a49f9a52bdf9b1f0b88a1d173aa2bdfb5a16
Gerrit-Change-Number: 44612
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
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: Fill out the 'H' thread setting command in remote GDB.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: base: Fill out the 'H' thread setting command in remote GDB.
..

base: Fill out the 'H' thread setting command in remote GDB.

Distinguish between the 'g' and 'c' subcommands. 'c' sets what thread(s)
should be continued or single stepped when those commands are used, and
'g' sets what thread(s) should be used for anything else. Also, insist
that all threads are used for continuing or single stepping.

Still complain if we're asked to switch threads, since we only have one
and we can't change to anything else.

Change-Id: Ia15c055baba48f75fc29ef369567535b0aa2c76b
---
M src/base/remote_gdb.cc
1 file changed, 24 insertions(+), 4 deletions(-)



diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 16373e6..d074c96 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -900,13 +900,33 @@
 bool
 BaseRemoteGDB::cmdSetThread(GdbCommand::Context )
 {
-const char *p = ctx.data + 1; // Ignore the subcommand byte.
-ContextID tid = 0;
+const char *p = ctx.data;
+char subcommand = *p++;
+int tid = 0;
 bool all, any;
 if (!parseThreadId(, all, any, tid))
 throw CmdError("E01");
-if (any || all || tid != tc->contextId())
-throw CmdError("E02");
+
+if (subcommand == 'c') {
+// We can only single step or continue all threads at once, since  
we

+// stop time itself and not individual threads.
+if (!all)
+throw CmdError("E02");
+} else if (subcommand == 'g') {
+// We don't currently support reading registers, memory, etc, from  
all

+// threads at once. GDB may never ask for this, but if it does we
+// should complain.
+if (all)
+throw CmdError("E03");
+// If GDB cares which thread we're using and wants a different  
one, we

+// don't support that right now.
+if (!any && tid != tc->contextId())
+throw CmdError("E04");
+// Since we weren't asked to do anything, we succeeded.
+} else {
+throw CmdError("E05");
+}
+
 send("OK");
 return true;
 }

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44609
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: Ia15c055baba48f75fc29ef369567535b0aa2c76b
Gerrit-Change-Number: 44609
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
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: Check the context ID when replacing a ThreadContext in GDB.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has uploaded this change for review. (  
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
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 9 insertions(+), 1 deletion(-)



diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index d074c96..6f5841b 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -436,6 +436,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);


--
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: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
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: Add a link to documentation in the remote GDB header file.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: base: Add a link to documentation in the remote GDB header  
file.

..

base: Add a link to documentation in the remote GDB header file.

Change-Id: I34bf4d24e58e6dfc8e8d1220a158c90fd0935e47
---
M src/base/remote_gdb.hh
1 file changed, 7 insertions(+), 0 deletions(-)



diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index cfd6b3d..211bf2e 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -58,6 +58,13 @@
 #include "cpu/pc_event.hh"
 #include "sim/eventq.hh"

+/*
+ * This file implements a client for the GDB remote serial protocol as
+ * described in this official documentation:
+ *
+ * https://sourceware.org/gdb/current/onlinedocs/gdb/Remote-Protocol.html
+ */
+
 class System;
 class ThreadContext;


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44608
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: I34bf4d24e58e6dfc8e8d1220a158c90fd0935e47
Gerrit-Change-Number: 44608
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
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]: sim: Minor cleanup of the System class.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: sim: Minor cleanup of the System class.
..

sim: Minor cleanup of the System class.

Fix style, move constant initialization out of the constructor and into
the class body, and minorly streamline an if condition.

Change-Id: I8ef42dcb8336ece58578cbd1f33937103b42989f
---
M src/sim/system.cc
M src/sim/system.hh
2 files changed, 32 insertions(+), 27 deletions(-)



diff --git a/src/sim/system.cc b/src/sim/system.cc
index e3924d7..0c42256 100644
--- a/src/sim/system.cc
+++ b/src/sim/system.cc
@@ -204,21 +204,16 @@
 System::System(const Params )
 : SimObject(p), _systemPort("system_port", this),
   multiThread(p.multi_thread),
-  pagePtr(0),
   init_param(p.init_param),
   physProxy(_systemPort, p.cache_line_size),
   workload(p.workload),
 #if USE_KVM
   kvmVM(p.kvm_vm),
-#else
-  kvmVM(nullptr),
 #endif
   physmem(name() + ".physmem", p.memories, p.mmap_using_noreserve,
   p.shared_backstore),
   memoryMode(p.mem_mode),
   _cacheLineSize(p.cache_line_size),
-  workItemsBegin(0),
-  workItemsEnd(0),
   numWorkIds(p.num_work_ids),
   thermalModel(p.thermal_model),
   _m5opRange(p.m5ops_base ?
@@ -239,9 +234,10 @@
 #endif

 // check if the cache line size is a value known to work
-if (!(_cacheLineSize == 16 || _cacheLineSize == 32 ||
-  _cacheLineSize == 64 || _cacheLineSize == 128))
+if (_cacheLineSize != 16 && _cacheLineSize != 32 &&
+_cacheLineSize != 64 && _cacheLineSize != 128) {
 warn_once("Cache line size is neither 16, 32, 64 nor 128  
bytes.\n");

+}

 // Get the generic system requestor IDs
 M5_VAR_USED RequestorID tmp_id;
diff --git a/src/sim/system.hh b/src/sim/system.hh
index f0eac6a..480b523 100644
--- a/src/sim/system.hh
+++ b/src/sim/system.hh
@@ -89,10 +89,18 @@
 SystemPort(const std::string &_name, SimObject *_owner)
 : RequestPort(_name, _owner)
 { }
-bool recvTimingResp(PacketPtr pkt) override
-{ panic("SystemPort does not receive timing!\n"); return false; }
-void recvReqRetry() override
-{ panic("SystemPort does not expect retry!\n"); }
+
+bool
+recvTimingResp(PacketPtr pkt) override
+{
+panic("SystemPort does not receive timing!");
+}
+
+void
+recvReqRetry() override
+{
+panic("SystemPort does not expect retry!");
+}
 };

 std::list liveEvents;
@@ -250,7 +258,9 @@
  * CPUs. SimObjects are expected to use Port::sendAtomic() and
  * Port::recvAtomic() when accessing memory in this mode.
  */
-bool isAtomicMode() const {
+bool
+isAtomicMode() const
+{
 return memoryMode == Enums::atomic ||
 memoryMode == Enums::atomic_noncaching;
 }
@@ -261,9 +271,7 @@
  * SimObjects are expected to use Port::sendTiming() and
  * Port::recvTiming() when accessing memory in this mode.
  */
-bool isTimingMode() const {
-return memoryMode == Enums::timing;
-}
+bool isTimingMode() const { return memoryMode == Enums::timing; }

 /**
  * Should caches be bypassed?
@@ -271,7 +279,9 @@
  * Some CPUs need to bypass caches to allow direct memory
  * accesses, which is required for hardware virtualization.
  */
-bool bypassCaches() const {
+bool
+bypassCaches() const
+{
 return memoryMode == Enums::atomic_noncaching;
 }
 /** @} */
@@ -310,7 +320,7 @@
 bool schedule(PCEvent *event) override;
 bool remove(PCEvent *event) override;

-Addr pagePtr;
+Addr pagePtr = 0;

 uint64_t init_param;

@@ -326,9 +336,7 @@
  * Get a pointer to the Kernel Virtual Machine (KVM) SimObject,
  * if present.
  */
-KvmVM* getKvmVM() {
-return kvmVM;
-}
+KvmVM *getKvmVM() { return kvmVM; }

 /** Verify gem5 configuration will support KVM emulation */
 bool validKvmEnvironment() const;
@@ -402,7 +410,7 @@

   protected:

-KvmVM *const kvmVM;
+KvmVM *const kvmVM = nullptr;

 PhysicalMemory physmem;

@@ -410,8 +418,8 @@

 const unsigned int _cacheLineSize;

-uint64_t workItemsBegin;
-uint64_t workItemsEnd;
+uint64_t workItemsBegin = 0;
+uint64_t workItemsEnd = 0;
 uint32_t numWorkIds;

 /** This array is a per-system list of all devices capable of issuing a
@@ -465,7 +473,7 @@
  * @return the requestor's ID.
  */
 RequestorID getRequestorId(const SimObject* requestor,
- std::string subrequestor = std::string());
+ std::string subrequestor={});

 /**
  * Registers a GLOBAL RequestorID, which is a RequestorID not related
@@ -544,9 +552,10 @@
 return threads.numActive();
  

[gem5-dev] Change in gem5/gem5[develop]: base: Make the BaseRemoteGDB class able to handle multiple TCs.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: base: Make the BaseRemoteGDB class able to handle multiple  
TCs.

..

base: Make the BaseRemoteGDB class able to handle multiple TCs.

Only one is set up corrent, the one passed in from the constructor.
Others can be added with addThreadContext.

The inconsistency of adding one ThreadContext through the constructor
and others through addThreadContext isn't great, but this way we can
ensure that there is always at least one ThreadContext. I'm not sure
what the GDB stub should do if there aren't any threads. I don't think
that the protocol can actually handle that, judging from the
documentation I can find.

Change-Id: I9160c3701ce78dcbbe99de1a6fe2a13e7e69404e
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 53 insertions(+), 13 deletions(-)



diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 6f5841b..20b26b9 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -132,6 +132,7 @@
 #include 
 #include 

+#include 
 #include 
 #include 
 #include 
@@ -350,9 +351,10 @@

 BaseRemoteGDB::BaseRemoteGDB(System *_system, ThreadContext *c, int  
_port) :

 connectEvent(nullptr), dataEvent(nullptr), _port(_port), fd(-1),
-active(false), attached(false), sys(_system), tc(c),
+active(false), attached(false), sys(_system),
 trapEvent(this), singleStepEvent(*this)
 {
+addThreadContext(c);
 }

 BaseRemoteGDB::~BaseRemoteGDB()
@@ -437,11 +439,35 @@
 }

 void
+BaseRemoteGDB::addThreadContext(ThreadContext *_tc)
+{
+threads[_tc->contextId()] = _tc;
+// If no ThreadContext is current selected, select this one.
+if (!tc)
+assert(selectThreadContext(_tc->contextId()));
+}
+
+void
 BaseRemoteGDB::replaceThreadContext(ThreadContext *_tc)
 {
-ContextID id = _tc->contextId();
-panic_if(id != tc->contextId(), "No context with ID %d found.", id);
-tc = _tc;
+auto it = threads.find(_tc->contextId());
+panic_if(it == threads.end(), "No context with ID %d found.",
+_tc->contextId());
+it->second = _tc;
+}
+
+bool
+BaseRemoteGDB::selectThreadContext(ContextID id)
+{
+auto it = threads.find(id);
+if (it == threads.end())
+return false;
+
+tc = it->second;
+// Update the register cache for the new thread context, if there is  
one.

+if (regCachePtr)
+regCachePtr->getRegs(tc);
+return true;
 }

 // This function does all command processing for interfacing to a
@@ -926,11 +952,13 @@
 // should complain.
 if (all)
 throw CmdError("E03");
-// If GDB cares which thread we're using and wants a different  
one, we

-// don't support that right now.
-if (!any && tid != tc->contextId())
-throw CmdError("E04");
-// Since we weren't asked to do anything, we succeeded.
+
+// If GDB doesn't care which thread we're using, keep using the
+// current one, otherwise switch.
+if (!any && tid != tc->contextId()) {
+if (!selectThreadContext(tid))
+throw CmdError("E04");
+}
 } else {
 throw CmdError("E05");
 }
@@ -1074,13 +1102,19 @@
 void
 BaseRemoteGDB::queryFThreadInfo(QuerySetCommand::Context )
 {
-send("m%x", encodeThreadId(tc->contextId()));
+threadInfoIdx = 0;
+querySThreadInfo(ctx);
 }

 void
 BaseRemoteGDB::querySThreadInfo(QuerySetCommand::Context )
 {
-send("l");
+if (threadInfoIdx >= threads.size()) {
+threadInfoIdx = 0;
+send("l");
+} else {
+send("m%x", encodeThreadId(threads[threadInfoIdx++]->contextId()));
+}
 }

 bool
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index ba274e6..93a6026 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -163,7 +163,9 @@
 void detach();
 bool isAttached() { return attached; }

+void addThreadContext(ThreadContext *_tc);
 void replaceThreadContext(ThreadContext *_tc);
+bool selectThreadContext(ContextID id);

 bool trap(int type);

@@ -227,9 +229,11 @@
 bool attached;

 System *sys;
-ThreadContext *tc;

-BaseGdbRegCache *regCachePtr;
+std::map threads;
+ThreadContext *tc = nullptr;
+
+BaseGdbRegCache *regCachePtr = nullptr;

 class TrapEvent : public Event
 {
@@ -340,6 +344,8 @@
 void queryC(QuerySetCommand::Context );
 void querySupported(QuerySetCommand::Context );
 void queryXfer(QuerySetCommand::Context );
+
+size_t threadInfoIdx = 0;
 void queryFThreadInfo(QuerySetCommand::Context );
 void querySThreadInfo(QuerySetCommand::Context );


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

[gem5-dev] Change in gem5/gem5[develop]: util: Remove unused package import

[Giacomo Travaglini (Gerrit) via gem5-dev]

Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/44511 )


Change subject: util: Remove unused package import
..

util: Remove unused package import

Change-Id: I67a03cffad11d4b26a193cc2c5ccb4cd30159a48
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/44511
Reviewed-by: Jason Lowe-Power 
Reviewed-by: Gabe Black 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M util/tlm/conf/tlm_elastic_slave.py
M util/tlm/examples/tlm_elastic_slave_with_l2.py
2 files changed, 0 insertions(+), 2 deletions(-)

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



diff --git a/util/tlm/conf/tlm_elastic_slave.py  
b/util/tlm/conf/tlm_elastic_slave.py

index 01f6dfb..d9bbceb 100644
--- a/util/tlm/conf/tlm_elastic_slave.py
+++ b/util/tlm/conf/tlm_elastic_slave.py
@@ -29,7 +29,6 @@
 # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

 import m5
-import optparse

 from m5.objects import *
 from m5.util import addToPath, fatal
diff --git a/util/tlm/examples/tlm_elastic_slave_with_l2.py  
b/util/tlm/examples/tlm_elastic_slave_with_l2.py

index 2f7bc95..ff2bbde 100644
--- a/util/tlm/examples/tlm_elastic_slave_with_l2.py
+++ b/util/tlm/examples/tlm_elastic_slave_with_l2.py
@@ -29,7 +29,6 @@
 # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

 import m5
-import optparse

 from m5.objects import *
 from m5.util import addToPath, fatal

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44511
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: I67a03cffad11d4b26a193cc2c5ccb4cd30159a48
Gerrit-Change-Number: 44511
Gerrit-PatchSet: 2
Gerrit-Owner: Giacomo Travaglini 
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]: util: Fix cpt_upgrader format string

[Giacomo Travaglini (Gerrit) via gem5-dev]

Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/44510 )


Change subject: util: Fix cpt_upgrader format string
..

util: Fix cpt_upgrader format string

Change-Id: I9d15316f199b20976420c35d2c79dd13cc9db9ee
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/44510
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M util/cpt_upgrader.py
1 file changed, 1 insertion(+), 1 deletion(-)

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



diff --git a/util/cpt_upgrader.py b/util/cpt_upgrader.py
index f1d7eb5..cf46b58 100755
--- a/util/cpt_upgrader.py
+++ b/util/cpt_upgrader.py
@@ -126,7 +126,7 @@
 sys.exit(1)
 Upgrader.untag_set.add(self.tag)
 else:
-print("Error: no upgrader or downgrader method for".format(
+print("Error: no upgrader or downgrader method for {}".format(
 self.tag))
 sys.exit(1)


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44510
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: I9d15316f199b20976420c35d2c79dd13cc9db9ee
Gerrit-Change-Number: 44510
Gerrit-PatchSet: 2
Gerrit-Owner: Giacomo Travaglini 
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]: configs: Remove Ruby on ARM warning

[Giacomo Travaglini (Gerrit) via gem5-dev]

Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/44509 )


Change subject: configs: Remove Ruby on ARM warning
..

configs: Remove Ruby on ARM warning

The warning was discouraging using Ruby on ARM

Change-Id: I7ccb44cdc1d32ff1258f09bc4d0d4892a758a6ef
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/44509
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M configs/common/FSConfig.py
1 file changed, 0 insertions(+), 2 deletions(-)

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



diff --git a/configs/common/FSConfig.py b/configs/common/FSConfig.py
index 22b1ee1..92f6798 100644
--- a/configs/common/FSConfig.py
+++ b/configs/common/FSConfig.py
@@ -345,8 +345,6 @@
 fatal("The MI_example protocol cannot implement Load/Store "
   "Exclusive operations. Multicore ARM systems configured "
   "with the MI_example protocol will not work properly.")
-warn("You are trying to use Ruby on ARM, which is not working "
- "properly yet.")

 return self


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44509
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: I7ccb44cdc1d32ff1258f09bc4d0d4892a758a6ef
Gerrit-Change-Number: 44509
Gerrit-PatchSet: 2
Gerrit-Owner: Giacomo Travaglini 
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