[gem5-dev] Re: DPRINTF_UNCONDITIONAL deprecation
Meant to reply to the list. On Thu, Jul 15, 2021 at 3:29 PM Gabe Black wrote: > Hi Giacomo. The DPRINTF_UNCONDITIONAL macro was used in one place, and > that I converted that place to a normal DPRINTF. Please see here for > explanation for that change, and justification for this type of change in > general: > > commit 07074f40e5a7e6000262a5d37f0be836f6ac92a9 > Author: Gabe Black > Date: Thu Apr 29 20:09:32 2021 -0700 > > There are several problems with DPRINTF_UNCONDITIONAL. First, while macros > don't take any run time overhead because they just turn into c++, and of > course you could have just typed that c++ to begin with, they do have > overhead because they make the code more complex and fragile, and obfuscate > what's actually going on. This isn't cost incurred (directly) by the > computer, but by the people who work on the code. This may eventually > translate into cost to the computer, because when code is harder to work > with, it's less likely to be completely correct or optimized. > > Second, the compiler is much better at figuring out and keeping track of > what code is reachable each time you build. Code paths change, new callers > are added, etc, and what may have been already guarded by a debug variable > may not any more. Also, the compiler can already merge together or drop > checks it knows are unnecessary, so I think we may frequently be dropping > something the compiler is already going to drop for us. I think > DPRINTF_UNCONDITIONAL may be a bit of premature optimization without > measurement to say that it's really necessary or even useful. > > I acknowledge that the lack of performance benefit is also an assumption > on my part though, and please let me know if you can measure a difference > in the scenarios you're thinking of. I still think the complexity and > fragility argument apply, but if the difference is substantial it might be > still be worth it. > > Gabe > > > On Thu, Jul 15, 2021 at 6:08 AM Giacomo Travaglini < > giacomo.travagl...@arm.com> wrote: > >> Hi Gabe, >> >> The DPRINTF_UNCONDITIONAL macro has been deprecated in develop [1] and >> the deprecation is going to be official in v21.1. >> As far as I understood, the reason why it has been deprecated is because >> it was not used in gem5. >> >> I am proposing to remove the deprecation. IMO that macro is useful when >> the Debug flag has already been checked outside of the >> Debug print. We recently merged a patch [2] that would have benefitted >> from the use of DPRINTF_UNCONDITIONAL. I am quite confident >> I can find some other parts of gem5 where the DPRINTF_UNCONDITIONAL would >> have been a better printing choice. >> I believe speed should still be a prime concern even if we are tracing, >> to make the debugging process faster and smoother for our users, especially >> As it comes with no cost (the definition of a macro) >> >> Let me know what you think about this, and if you don't have any >> objection I will push a revert of the patch you posted, and I will convert >> some DPRINT into a DPRINTF_UNCONDITIONAL >> >> Kind Regards >> >> Giacomo >> >> [1]: https://gem5-review.googlesource.com/c/public/gem5/+/44987 >> [2]: >> https://gem5-review.googlesource.com/c/public/gem5/+/47199/7/src/mem/cache/prefetch/queued.cc#134 >> 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]: base: Extract GDB command processing into separate function
Jan Vrany has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48181 )
Change subject: base: Extract GDB command processing into separate function
..
base: Extract GDB command processing into separate function
Change-Id: I1f090285f92752d6907044b9ee6ade1869a2cb9f
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 56 insertions(+), 42 deletions(-)
diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index b620b99..26aff5c 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -517,48 +517,7 @@
send("S%02x", signum);
}
-// Stick frame regs into our reg cache.
-regCachePtr = gdbRegs();
-regCachePtr->getRegs(tc);
-
-GdbCommand::Context cmd_ctx;
-cmd_ctx.type = signum;
-std::vector data;
-
-for (;;) {
-try {
-recv(data);
-if (data.size() == 1)
-throw BadClient();
-cmd_ctx.cmdByte = data[0];
-cmd_ctx.data = data.data() + 1;
-// One for sentinel, one for cmdByte.
-cmd_ctx.len = data.size() - 2;
-
-auto cmd_it = commandMap.find(cmd_ctx.cmdByte);
-if (cmd_it == commandMap.end()) {
-DPRINTF(GDBMisc, "Unknown command: %c(%#x)\n",
-cmd_ctx.cmdByte, cmd_ctx.cmdByte);
-throw Unsupported();
-}
-cmd_ctx.cmd = &(cmd_it->second);
-
-if (!(this->*(cmd_ctx.cmd->func))(cmd_ctx))
-break;
-
-} catch (BadClient ) {
-if (e.warning)
-warn(e.warning);
-detach();
-break;
-} catch (Unsupported ) {
-send("");
-} catch (CmdError ) {
-send(e.error);
-} catch (...) {
-panic("Unrecognzied GDB exception.");
-}
-}
+processCommands(signum);
}
void
@@ -679,6 +638,53 @@
} while ((c & 0x7f) == GDBBadP);
}
+void
+BaseRemoteGDB::processCommands(int signum)
+{
+// Stick frame regs into our reg cache.
+regCachePtr = gdbRegs();
+regCachePtr->getRegs(tc);
+
+GdbCommand::Context cmd_ctx;
+cmd_ctx.type = signum;
+std::vector data;
+
+for (;;) {
+try {
+recv(data);
+if (data.size() == 1)
+throw BadClient();
+cmd_ctx.cmdByte = data[0];
+cmd_ctx.data = data.data() + 1;
+// One for sentinel, one for cmdByte.
+cmd_ctx.len = data.size() - 2;
+
+auto cmd_it = commandMap.find(cmd_ctx.cmdByte);
+if (cmd_it == commandMap.end()) {
+DPRINTF(GDBMisc, "Unknown command: %c(%#x)\n",
+cmd_ctx.cmdByte, cmd_ctx.cmdByte);
+throw Unsupported();
+}
+cmd_ctx.cmd = &(cmd_it->second);
+
+if (!(this->*(cmd_ctx.cmd->func))(cmd_ctx))
+break;
+
+} catch (BadClient ) {
+if (e.warning)
+warn(e.warning);
+detach();
+break;
+} catch (Unsupported ) {
+send("");
+} catch (CmdError ) {
+send(e.error);
+} catch (...) {
+panic("Unrecognzied GDB exception.");
+}
+}
+}
+
// Read bytes from kernel address space for debugger.
bool
BaseRemoteGDB::read(Addr vaddr, size_t size, char *data)
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index 7d8305b..6f57ae0 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -238,6 +238,14 @@
}
/*
+ * Process commands from remote GDB. If simulation has been
+ * stopped because of some kind of fault (as segmentation violation,
+ * or SW trap), 'signum' is the signal value reported back to GDB
+ * in "S" packet ( this is done in trap() )
+ */
+void processCommands(int signum = 0);
+
+/*
* Simulator side debugger state.
*/
bool active = false;
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48181
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: I1f090285f92752d6907044b9ee6ade1869a2cb9f
Gerrit-Change-Number: 48181
Gerrit-PatchSet: 1
Gerrit-Owner: Jan Vrany
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: Do no listen for GDB when no `--wait-gdb` is specified
Jan Vrany has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48183 )
Change subject: base: Do no listen for GDB when no `--wait-gdb` is specified
..
base: Do no listen for GDB when no `--wait-gdb` is specified
Remote GDB listening socket was open as soon as first thread has been
added, regardless whether a `--wait-gdb` has been specified or not.
Moreover, a message that gem5 is listening to GDB connection has been
printed as soon as the listening socket was open, but gem5 was ready
to accept data much later at workflow startup time. When GDB tried to
connect before gem5 was ready, a timeout on GDB side may (and did) have
occurred, depending on how fast was the host machine.
This commit solves both problems by opening the listening socket to
workflow startup and doing so only if `--wait-gdb` is specified.
Change-Id: Ibe09c8ed4564474eefcb7211f1b63acbe24c9efb
---
M src/base/remote_gdb.cc
M src/sim/workload.cc
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index a6c569e..76541d9 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -373,6 +373,9 @@
void
BaseRemoteGDB::listen()
{
+panic_if(!tc, "Can't accept GDB connection without any threads.");
+panic_if(listener.islistening(), "Already accepting GDB connection.");
+
if (ListenSocket::allDisabled()) {
warn_once("Sockets disabled, not accepting gdb connections");
return;
@@ -385,17 +388,11 @@
connectEvent = new ConnectEvent(this, listener.getfd(), POLLIN);
pollQueue.schedule(connectEvent);
-
-ccprintf(std::cerr, "%d: %s: listening for remote gdb on port %d\n",
- curTick(), name(), _port);
}
void
BaseRemoteGDB::connect()
{
-panic_if(!listener.islistening(),
- "Can't accept GDB connections without any threads!");
-
int sfd = listener.accept(true);
if (sfd != -1) {
@@ -448,10 +445,6 @@
// If no ThreadContext is current selected, select this one.
if (!tc)
assert(selectThreadContext(_tc->contextId()));
-
-// Now that we have a thread, we can start listening.
-if (!listener.islistening())
-listen();
}
void
diff --git a/src/sim/workload.cc b/src/sim/workload.cc
index d208a58..7b33780 100644
--- a/src/sim/workload.cc
+++ b/src/sim/workload.cc
@@ -97,6 +97,7 @@
// Now that we're about to start simulation, wait for GDB connections
if
// requested.
if (gdb && waitForRemoteGDB) {
+gdb->listen();
inform("%s: Waiting for a remote GDB connection on port %d.",
name(),
gdb->port());
gdb->connect();
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48183
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: Ibe09c8ed4564474eefcb7211f1b63acbe24c9efb
Gerrit-Change-Number: 48183
Gerrit-PatchSet: 1
Gerrit-Owner: Jan Vrany
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: Remove unused ConnectEvent from BaseRemoteGDB
Jan Vrany has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48185 )
Change subject: base: Remove unused ConnectEvent from BaseRemoteGDB
..
base: Remove unused ConnectEvent from BaseRemoteGDB
There's no need for such event since connect() is called explicitly
in workflow's startup.
Change-Id: If0e9f344f0ef2a46df783041438e6c424ed94293
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 76541d9..a257916 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -354,13 +354,12 @@
}
BaseRemoteGDB::BaseRemoteGDB(System *_system, int _port) :
-connectEvent(nullptr), dataEvent(nullptr), _port(_port), fd(-1),
+dataEvent(nullptr), _port(_port), fd(-1),
sys(_system), trapEvent(this), singleStepEvent(*this)
{}
BaseRemoteGDB::~BaseRemoteGDB()
{
-delete connectEvent;
delete dataEvent;
}
@@ -385,9 +384,6 @@
DPRINTF(GDBMisc, "Can't bind port %d\n", _port);
_port++;
}
-
-connectEvent = new ConnectEvent(this, listener.getfd(), POLLIN);
-pollQueue.schedule(connectEvent);
}
void
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index dbf3774..962d2fb 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -191,7 +191,6 @@
* Connection to the external GDB.
*/
void incomingData(int revent);
-void connectWrapper(int revent) { connect(); }
template
class SocketEvent : public PollEvent
@@ -207,13 +206,10 @@
void process(int revent) { (gdb->*F)(revent); }
};
-typedef SocketEvent<::connectWrapper> ConnectEvent;
typedef SocketEvent<::incomingData> DataEvent;
-friend ConnectEvent;
friend DataEvent;
-ConnectEvent *connectEvent;
DataEvent *dataEvent;
ListenSocket listener;
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48185
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: If0e9f344f0ef2a46df783041438e6c424ed94293
Gerrit-Change-Number: 48185
Gerrit-PatchSet: 1
Gerrit-Owner: Jan Vrany
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: Change prototype of BaseRemoteGDB::trap()
Jan Vrany has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48180 )
Change subject: base: Change prototype of BaseRemoteGDB::trap()
..
base: Change prototype of BaseRemoteGDB::trap()
Change the return type of BaseRemoteGDB::trap() to void since the
return value was never used by any of the callers. Also change the
name of second parameter to signum since its value is reported back
to GDB in "S" packet.
Change-Id: I81acfac24ffe62e4ffae6b74bf33f1f07ada3ca7
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index e57ded7..b620b99 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -10,7 +10,7 @@
* unmodified and in its entirety in all distributions of the software,
* modified or unmodified, in source code or in binary form.
*
- * Copyright 2015 LabWare
+ * Copyright 2015, 2021 LabWare
* Copyright 2014 Google, Inc.
* Copyright (c) 2002-2005 The Regents of The University of Michigan
* All rights reserved.
@@ -481,15 +481,15 @@
// present, but might eventually become meaningful. (XXX) It might
// makes sense to use POSIX errno values, because that is what the
// gdb/remote.c functions want to return.
-bool
-BaseRemoteGDB::trap(ContextID id, int type)
+void
+BaseRemoteGDB::trap(ContextID id, int signum)
{
if (!attached)
-return false;
+return;
if (tc->contextId() != id) {
if (!selectThreadContext(id))
-return false;
+return;
}
DPRINTF(GDBMisc, "trap: PC=%s\n", tc->pcState());
@@ -514,7 +514,7 @@
send("OK");
} else {
// Tell remote host that an exception has occurred.
-send("S%02x", type);
+send("S%02x", signum);
}
// Stick frame regs into our reg cache.
@@ -522,7 +522,7 @@
regCachePtr->getRegs(tc);
GdbCommand::Context cmd_ctx;
-cmd_ctx.type = type;
+cmd_ctx.type = signum;
std::vector data;
for (;;) {
@@ -559,8 +559,6 @@
panic("Unrecognzied GDB exception.");
}
}
-
-return true;
}
void
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index 1089607..7d8305b 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -10,7 +10,7 @@
* unmodified and in its entirety in all distributions of the software,
* modified or unmodified, in source code or in binary form.
*
- * Copyright 2015 LabWare
+ * Copyright 2015, 2021 LabWare
* Copyright 2014 Google, Inc.
* Copyright (c) 2002-2005 The Regents of The University of Michigan
* All rights reserved.
@@ -171,7 +171,7 @@
void replaceThreadContext(ThreadContext *_tc);
bool selectThreadContext(ContextID id);
-bool trap(ContextID id, int type);
+void trap(ContextID id, int signum);
/** @} */ // end of api_remote_gdb
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48180
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: I81acfac24ffe62e4ffae6b74bf33f1f07ada3ca7
Gerrit-Change-Number: 48180
Gerrit-PatchSet: 1
Gerrit-Owner: Jan Vrany
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: Only trap to GDB if remote GDB is connected
Jan Vrany has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48184 )
Change subject: base: Only trap to GDB if remote GDB is connected
..
base: Only trap to GDB if remote GDB is connected
Change-Id: I3a82dc0f3e4f99dd1acfe99c1eb8caaae495e384
---
M src/sim/workload.cc
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/sim/workload.cc b/src/sim/workload.cc
index 7b33780..7be4106 100644
--- a/src/sim/workload.cc
+++ b/src/sim/workload.cc
@@ -80,7 +80,7 @@
Workload::trapToGdb(int signal, ContextID ctx_id)
{
# if THE_ISA != NULL_ISA
-if (gdb) {
+if (gdb && gdb->isAttached()) {
gdb->trap(ctx_id, signal);
return true;
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48184
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: I3a82dc0f3e4f99dd1acfe99c1eb8caaae495e384
Gerrit-Change-Number: 48184
Gerrit-PatchSet: 1
Gerrit-Owner: Jan Vrany
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: handle initial communication with GDB in `attach()`
Jan Vrany has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48182 )
Change subject: base: handle initial communication with GDB in `attach()`
..
base: handle initial communication with GDB in `attach()`
When remote GDB attaches to gem5, handle the initial communication
(`qSupported` and alike) right away instead of scheduling a `DataEvent`
and firing the simulation loop in hope that GDB will be quick enough to
send initial packets before instructions are dispatched.
This seems to fix the race described in 44612 [1]
[1]: https://gem5-review.googlesource.com/c/public/gem5/+/44612
Change-Id: I33b2922ba017205acabd51b6a8be3e6fb2d6409a
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 26aff5c..a6c569e 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -419,18 +419,19 @@
{
fd = f;
-dataEvent = new DataEvent(this, fd, POLLIN);
-pollQueue.schedule(dataEvent);
-
attached = true;
DPRINTFN("remote gdb attached\n");
+
+processCommands();
+
+dataEvent = new DataEvent(this, fd, POLLIN);
+pollQueue.schedule(dataEvent);
}
void
BaseRemoteGDB::detach()
{
attached = false;
-active = false;
clearSingleStep();
close(fd);
fd = -1;
@@ -496,19 +497,7 @@
clearSingleStep();
-/*
- * The first entry to this function is normally through
- * a breakpoint trap in kgdb_connect(), in which case we
- * must advance past the breakpoint because gdb will not.
- *
- * On the first entry here, we expect that gdb is not yet
- * listening to us, so just enter the interaction loop.
- * After the debugger is "active" (connected) it will be
- * waiting for a "signaled" message from us.
- */
-if (!active) {
-active = true;
-} else if (threadSwitching) {
+if (threadSwitching) {
threadSwitching = false;
// Tell GDB the thread switch has completed.
send("OK");
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index 6f57ae0..dbf3774 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -248,7 +248,6 @@
/*
* Simulator side debugger state.
*/
-bool active = false;
bool attached = false;
bool threadSwitching = false;
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48182
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: I33b2922ba017205acabd51b6a8be3e6fb2d6409a
Gerrit-Change-Number: 48182
Gerrit-PatchSet: 1
Gerrit-Owner: Jan Vrany
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] DPRINTF_UNCONDITIONAL deprecation
Hi Gabe, The DPRINTF_UNCONDITIONAL macro has been deprecated in develop [1] and the deprecation is going to be official in v21.1. As far as I understood, the reason why it has been deprecated is because it was not used in gem5. I am proposing to remove the deprecation. IMO that macro is useful when the Debug flag has already been checked outside of the Debug print. We recently merged a patch [2] that would have benefitted from the use of DPRINTF_UNCONDITIONAL. I am quite confident I can find some other parts of gem5 where the DPRINTF_UNCONDITIONAL would have been a better printing choice. I believe speed should still be a prime concern even if we are tracing, to make the debugging process faster and smoother for our users, especially As it comes with no cost (the definition of a macro) Let me know what you think about this, and if you don't have any objection I will push a revert of the patch you posted, and I will convert some DPRINT into a DPRINTF_UNCONDITIONAL Kind Regards Giacomo [1]: https://gem5-review.googlesource.com/c/public/gem5/+/44987 [2]: https://gem5-review.googlesource.com/c/public/gem5/+/47199/7/src/mem/cache/prefetch/queued.cc#134 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]: arch-arm: Add a shared L2 TLB to the default ArmMMU
Giacomo Travaglini has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48150 )
Change subject: arch-arm: Add a shared L2 TLB to the default ArmMMU
..
arch-arm: Add a shared L2 TLB to the default ArmMMU
JIRA: https://gem5.atlassian.net/browse/GEM5-790
Change-Id: I542c287a99c8b277afb4cd939c09521798dcf2f8
Signed-off-by: Giacomo Travaglini
---
M src/arch/arm/ArmMMU.py
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/arch/arm/ArmMMU.py b/src/arch/arm/ArmMMU.py
index 0112bc9..6b5 100644
--- a/src/arch/arm/ArmMMU.py
+++ b/src/arch/arm/ArmMMU.py
@@ -64,13 +64,17 @@
cxx_class = 'gem5::ArmISA::MMU'
cxx_header = 'arch/arm/mmu.hh'
-itb = ArmTLB(entry_type="instruction")
-dtb = ArmTLB(entry_type="data")
-
cxx_exports = [
PyBindMethod('wireComponents'),
]
+# L1 TLBs
+itb = ArmTLB(entry_type="instruction")
+dtb = ArmTLB(entry_type="data")
+
+# L2 TLBs
+l2_shared = ArmTLB(entry_type="unified", size=1280)
+
stage2_itb = Param.ArmTLB(
ArmStage2TLB(entry_type="instruction"),
"Stage 2 Instruction TLB")
@@ -91,6 +95,9 @@
sys = Param.System(Parent.any, "system object parameter")
def init(self):
+self.itb.setNextLevel(self.l2_shared.getCCObject())
+self.dtb.setNextLevel(self.l2_shared.getCCObject())
+
self.getCCObject().wireComponents()
super(ArmMMU, self).init()
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48150
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: I542c287a99c8b277afb4cd939c09521798dcf2f8
Gerrit-Change-Number: 48150
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini
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: Add a nextLevel pointer to BaseTLB
Giacomo Travaglini has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48145 )
Change subject: arch: Add a nextLevel pointer to BaseTLB
..
arch: Add a nextLevel pointer to BaseTLB
This is a step towards supporting multi-level TLBs:
Every TLB will have a pointer to the next level TLB in the
hierarchy.
We are not adding a next_level Param to the BaseTLB as it will
conflict with the SimObject hierarchy. Say we have
* L1 I-TLB
* L1 D-TLB
* L2 shared TLB (I+D)
l2 = BaseTLB()
itb = BaseTLB(next_level=l2)
dtb = BaseTLB(next_level=l2)
The L2 TLB would have two parents and this produces a warning
in gem5.
We are working around this by assigning the nextLevel pointer via
exported C++ methods
JIRA: https://gem5.atlassian.net/browse/GEM5-790
Change-Id: I398a17919564aad4b18efb8dace096965781ece1
Signed-off-by: Giacomo Travaglini
---
M src/arch/generic/BaseTLB.py
M src/arch/generic/tlb.hh
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/arch/generic/BaseTLB.py b/src/arch/generic/BaseTLB.py
index 4fe2338..3deaf57 100644
--- a/src/arch/generic/BaseTLB.py
+++ b/src/arch/generic/BaseTLB.py
@@ -26,7 +26,7 @@
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
from m5.params import *
-from m5.SimObject import SimObject
+from m5.SimObject import PyBindMethod, SimObject
class TypeTLB(ScopedEnum):
"""
@@ -46,6 +46,10 @@
cxx_header = "arch/generic/tlb.hh"
cxx_class = 'gem5::BaseTLB'
+cxx_exports = [
+PyBindMethod('setNextLevel'),
+]
+
# Ports to connect with other TLB levels
cpu_side_ports = VectorResponsePort("Ports closer to the CPU side")
slave = DeprecatedParam(cpu_side_ports,
diff --git a/src/arch/generic/tlb.hh b/src/arch/generic/tlb.hh
index 7e2ef44..40302dd 100644
--- a/src/arch/generic/tlb.hh
+++ b/src/arch/generic/tlb.hh
@@ -57,11 +57,13 @@
{
protected:
BaseTLB(const BaseTLBParams )
- : SimObject(p), _type(p.entry_type)
+ : SimObject(p), _type(p.entry_type), _nextLevel(nullptr)
{}
TypeTLB _type;
+BaseTLB *_nextLevel;
+
public:
virtual void demapPage(Addr vaddr, uint64_t asn) = 0;
@@ -119,6 +121,9 @@
void memInvalidate() { flushAll(); }
TypeTLB type() const { return _type; }
+
+BaseTLB* getNextLevel() const { return _nextLevel; }
+void setNextLevel(BaseTLB* next_level) { _nextLevel = next_level; }
};
} // namespace gem5
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48145
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: I398a17919564aad4b18efb8dace096965781ece1
Gerrit-Change-Number: 48145
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini
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: Make BaseMMU translate methods virtual
Giacomo Travaglini has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/48140 ) Change subject: arch: Make BaseMMU translate methods virtual .. arch: Make BaseMMU translate methods virtual As we are shifting towards making the MMU the main translating agent, we need to make those methods virtual to let all ISAs move their TLB::translate* methods to the MMU class JIRA: https://gem5.atlassian.net/browse/GEM5-790 Change-Id: I50c84784546e8148230d79efe4bf010d0e36d6ab Signed-off-by: Giacomo Travaglini --- M src/arch/generic/mmu.hh 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/arch/generic/mmu.hh b/src/arch/generic/mmu.hh index cc8b8b0..8bcb3a7 100644 --- a/src/arch/generic/mmu.hh +++ b/src/arch/generic/mmu.hh @@ -102,17 +102,21 @@ void demapPage(Addr vaddr, uint64_t asn); -Fault translateAtomic(const RequestPtr , ThreadContext *tc, - Mode mode); +virtual Fault +translateAtomic(const RequestPtr , ThreadContext *tc, +Mode mode); -void translateTiming(const RequestPtr , ThreadContext *tc, - Translation *translation, Mode mode); +virtual void +translateTiming(const RequestPtr , ThreadContext *tc, +Translation *translation, Mode mode); -Fault translateFunctional(const RequestPtr , ThreadContext *tc, - Mode mode); +virtual Fault +translateFunctional(const RequestPtr , ThreadContext *tc, +Mode mode); -Fault finalizePhysical(const RequestPtr , ThreadContext *tc, - Mode mode) const; +virtual Fault +finalizePhysical(const RequestPtr , ThreadContext *tc, + Mode mode) const; virtual void takeOverFrom(BaseMMU *old_mmu); -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48140 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: I50c84784546e8148230d79efe4bf010d0e36d6ab Gerrit-Change-Number: 48140 Gerrit-PatchSet: 1 Gerrit-Owner: Giacomo Travaglini 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-arm: Provide support for a multilevel-TLB in the ArmMMU
Giacomo Travaglini has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48149 )
Change subject: arch-arm: Provide support for a multilevel-TLB in the ArmMMU
..
arch-arm: Provide support for a multilevel-TLB in the ArmMMU
This is an initial implementation. It adapts the current MMU code
to account for extra levels of TLBs but it is still missing the
configurability we are looking for (to select the associativity
of the TLB and the replacement policy as an example)
JIRA: https://gem5.atlassian.net/browse/GEM5-790
Change-Id: I938ec38183337cd0e839bf3e3cd03594126128cd
Signed-off-by: Giacomo Travaglini
---
M src/arch/arm/mmu.cc
M src/arch/arm/mmu.hh
M src/arch/arm/tlb.cc
3 files changed, 65 insertions(+), 12 deletions(-)
diff --git a/src/arch/arm/mmu.cc b/src/arch/arm/mmu.cc
index 2ed38b5..e04df89 100644
--- a/src/arch/arm/mmu.cc
+++ b/src/arch/arm/mmu.cc
@@ -1307,6 +1307,22 @@
is_secure, tran_type, stage2 ? s2State : s1State);
}
+TlbEntry*
+MMU::lookup(Addr va, uint16_t asid, vmid_t vmid, bool hyp, bool secure,
+bool functional, bool ignore_asn, ExceptionLevel target_el,
+bool in_host, bool stage2, BaseMMU::Mode mode)
+{
+TLB *tlb = getTlb(mode, stage2);
+TlbEntry* te = nullptr;
+while (!te && tlb) {
+te = tlb->lookup(va, asid, vmid, hyp, secure, functional,
+ ignore_asn, target_el, in_host, mode);
+tlb = static_cast(tlb->getNextLevel());
+}
+
+return te;
+}
+
Fault
MMU::getTE(TlbEntry **te, const RequestPtr , ThreadContext *tc, Mode
mode,
Translation *translation, bool timing, bool functional,
@@ -1329,9 +1345,9 @@
vaddr = vaddr_tainted;
}
-auto tlb = getTlb(mode, state.isStage2);
-*te = tlb->lookup(vaddr, state.asid, state.vmid, state.isHyp,
is_secure,
- false, false, target_el, false, mode);
+*te = lookup(vaddr, state.asid, state.vmid, state.isHyp, is_secure,
false,
+ false, target_el, false, state.isStage2, mode);
+
if (*te == NULL) {
if (req->isPrefetch()) {
// if the request is a prefetch don't attempt to fill the TLB
or go
@@ -1359,10 +1375,8 @@
return fault;
}
-*te = tlb->lookup(vaddr, state.asid, state.vmid, state.isHyp,
is_secure,
- true, false, target_el, false, mode);
-if (!*te)
-tlb->printTlb();
+*te = lookup(vaddr, state.asid, state.vmid, state.isHyp, is_secure,
+ true, false, target_el, false, state.isStage2, mode);
assert(*te);
}
return NoFault;
diff --git a/src/arch/arm/mmu.hh b/src/arch/arm/mmu.hh
index b11e156..61d9c9e 100644
--- a/src/arch/arm/mmu.hh
+++ b/src/arch/arm/mmu.hh
@@ -267,8 +267,15 @@
void
flushStage1(const OP _op)
{
-iflush(tlbi_op);
-dflush(tlbi_op);
+for (auto tlb : instruction) {
+static_cast(tlb)->flush(tlbi_op);
+}
+for (auto tlb : data) {
+static_cast(tlb)->flush(tlbi_op);
+}
+for (auto tlb : unified) {
+static_cast(tlb)->flush(tlbi_op);
+}
}
template
@@ -283,14 +290,24 @@
void
iflush(const OP _op)
{
-getITBPtr()->flush(tlbi_op);
+for (auto tlb : instruction) {
+static_cast(tlb)->flush(tlbi_op);
+}
+for (auto tlb : unified) {
+static_cast(tlb)->flush(tlbi_op);
+}
}
template
void
dflush(const OP _op)
{
-getDTBPtr()->flush(tlbi_op);
+for (auto tlb : data) {
+static_cast(tlb)->flush(tlbi_op);
+}
+for (auto tlb : unified) {
+static_cast(tlb)->flush(tlbi_op);
+}
}
void
@@ -323,6 +340,24 @@
static ExceptionLevel tranTypeEL(CPSR cpsr, ArmTranslationType type);
public:
+/** Lookup an entry in the TLB
+ * @param vpn virtual address
+ * @param asn context id/address space id to use
+ * @param vmid The virtual machine ID used for stage 2 translation
+ * @param secure if the lookup is secure
+ * @param hyp if the lookup is done from hyp mode
+ * @param functional if the lookup should modify state
+ * @param ignore_asn if on lookup asn should be ignored
+ * @param target_el selecting the translation regime
+ * @param in_host if we are in host (EL2&0 regime)
+ * @param mode to differentiate between read/writes/fetches.
+ * @return pointer to TLB entry if it exists
+ */
+TlbEntry *lookup(Addr vpn, uint16_t asn, vmid_t vmid, bool hyp,
+ bool secure, bool functional,
+ bool ignore_asn, ExceptionLevel target_el,
+ bool in_host, bool stage2, BaseMMU::Mode mode);
+
Fault getTE(TlbEntry **te, const RequestPtr ,
[gem5-dev] Change in gem5/gem5[develop]: arch-arm, configs: Remove ArmITB/ArmDTB
Giacomo Travaglini has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48143 )
Change subject: arch-arm, configs: Remove ArmITB/ArmDTB
..
arch-arm, configs: Remove ArmITB/ArmDTB
Removing ArmITB and ArmDTB makes sense as it implies a fixed 2 TLBs
system; by using the generic ArmTLB class we open up to a more generic
configuration
This is also aligning to the other ISAs
Change-Id: Ifc5cf7c41484d4f45b14d1766833ad4c4f7e9e86
Signed-off-by: Giacomo Travaglini
---
M configs/common/cores/arm/HPI.py
M src/arch/arm/ArmMMU.py
M src/arch/arm/ArmTLB.py
3 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/configs/common/cores/arm/HPI.py
b/configs/common/cores/arm/HPI.py
index 68b3862..624c40c 100644
--- a/configs/common/cores/arm/HPI.py
+++ b/configs/common/cores/arm/HPI.py
@@ -1328,15 +1328,9 @@
HPI_MiscFU() # 6
]
-class HPI_DTB(ArmDTB):
-size = 256
-
-class HPI_ITB(ArmITB):
-size = 256
-
class HPI_MMU(ArmMMU):
-itb = HPI_ITB()
-dtb = HPI_DTB()
+itb = ArmTLB(entry_type="instruction", size=256)
+dtb = ArmTLB(entry_type="data", size=256)
class HPI_WalkCache(Cache):
data_latency = 4
diff --git a/src/arch/arm/ArmMMU.py b/src/arch/arm/ArmMMU.py
index 5017fa8..1596574 100644
--- a/src/arch/arm/ArmMMU.py
+++ b/src/arch/arm/ArmMMU.py
@@ -35,7 +35,7 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-from m5.objects.ArmTLB import ArmITB, ArmDTB, ArmStage2TLB
+from m5.objects.ArmTLB import ArmTLB, ArmStage2TLB
from m5.objects.BaseMMU import BaseMMU
from m5.objects.ClockedObject import ClockedObject
from m5.params import *
@@ -63,8 +63,9 @@
type = 'ArmMMU'
cxx_class = 'gem5::ArmISA::MMU'
cxx_header = 'arch/arm/mmu.hh'
-itb = ArmITB()
-dtb = ArmDTB()
+
+itb = ArmTLB(entry_type="instruction")
+dtb = ArmTLB(entry_type="data")
cxx_exports = [
PyBindMethod('wireComponents'),
diff --git a/src/arch/arm/ArmTLB.py b/src/arch/arm/ArmTLB.py
index 5c4cca0..e2d61e4 100644
--- a/src/arch/arm/ArmTLB.py
+++ b/src/arch/arm/ArmTLB.py
@@ -51,9 +51,3 @@
class ArmStage2TLB(ArmTLB):
size = 32
is_stage2 = True
-
-class ArmITB(ArmTLB):
-entry_type = "instruction"
-
-class ArmDTB(ArmTLB):
-entry_type = "data"
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48143
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: Ifc5cf7c41484d4f45b14d1766833ad4c4f7e9e86
Gerrit-Change-Number: 48143
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini
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: Provide an alternative view of the TLBs in the BaseMMU
Giacomo Travaglini has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48146 )
Change subject: arch: Provide an alternative view of the TLBs in the BaseMMU
..
arch: Provide an alternative view of the TLBs in the BaseMMU
It is possible from the MMU to traverse the entire hierarchy of
TLBs, starting from the DTB and ITB (generally speaking from the
first level) up to the last level via the nextLevel pointer. So
in theory no extra data should be stored in the BaseMMU.
This design makes some operations a bit more complex. For example
if we have a unified (I+D) L2, it will be pointed by both ITB and
DTB. If we want to invalidate all TLB entries, we should be
careful to not invalidate L2 twice, but if we simply follow the
next level pointer, we might do so. This is not a problem from
a functional perspective but alters the TLB statistics (a single
invalidation is recorded twice)
We then provide a different view of the set of TLBs in the system.
At the init phase we traverse the TLB hierarchy and we add every
TLB to the appropriate set. This makes invalidation (and any
operation targeting a specific kind of TLBs) easier.
JIRA: https://gem5.atlassian.net/browse/GEM5-790
Change-Id: Ieb833c2328e9daeaf50a32b79b970f77f3e874f7
Signed-off-by: Giacomo Travaglini
---
M src/arch/arm/ArmMMU.py
M src/arch/generic/BaseMMU.py
M src/arch/generic/mmu.cc
M src/arch/generic/mmu.hh
4 files changed, 82 insertions(+), 3 deletions(-)
diff --git a/src/arch/arm/ArmMMU.py b/src/arch/arm/ArmMMU.py
index 1596574..0112bc9 100644
--- a/src/arch/arm/ArmMMU.py
+++ b/src/arch/arm/ArmMMU.py
@@ -93,6 +93,8 @@
def init(self):
self.getCCObject().wireComponents()
+super(ArmMMU, self).init()
+
@classmethod
def walkerPorts(cls):
return ["mmu.itb_walker.port", "mmu.dtb_walker.port",
diff --git a/src/arch/generic/BaseMMU.py b/src/arch/generic/BaseMMU.py
index c9ea25a..9ee4c70 100644
--- a/src/arch/generic/BaseMMU.py
+++ b/src/arch/generic/BaseMMU.py
@@ -37,7 +37,7 @@
from m5.objects.BaseTLB import BaseTLB
from m5.params import *
-from m5.SimObject import SimObject
+from m5.SimObject import SimObject, PyBindMethod
class BaseMMU(SimObject):
type = 'BaseMMU'
@@ -45,9 +45,16 @@
cxx_header = "arch/generic/mmu.hh"
cxx_class = 'gem5::BaseMMU'
+cxx_exports = [
+PyBindMethod('initMMU'),
+]
+
itb = Param.BaseTLB("Instruction TLB")
dtb = Param.BaseTLB("Data TLB")
+def init(self):
+self.getCCObject().initMMU()
+
@classmethod
def walkerPorts(cls):
# This classmethod is used by the BaseCPU. It should return
diff --git a/src/arch/generic/mmu.cc b/src/arch/generic/mmu.cc
index 7aaec5b..5956293 100644
--- a/src/arch/generic/mmu.cc
+++ b/src/arch/generic/mmu.cc
@@ -48,10 +48,47 @@
{
void
+BaseMMU::initMMU()
+{
+auto traverse_hierarchy = [this](BaseTLB *starter) {
+for (BaseTLB *tlb = starter; tlb; tlb = tlb->getNextLevel()) {
+switch (tlb->type()) {
+ case TypeTLB::instruction:
+if (instruction.find(tlb) == instruction.end())
+instruction.insert(tlb);
+break;
+ case TypeTLB::data:
+if (data.find(tlb) == data.end())
+data.insert(tlb);
+break;
+ case TypeTLB::unified:
+if (unified.find(tlb) == unified.end())
+unified.insert(tlb);
+break;
+ default:
+panic("Invalid TLB type\n");
+}
+}
+};
+
+traverse_hierarchy(itb);
+traverse_hierarchy(dtb);
+}
+
+void
BaseMMU::flushAll()
{
-dtb->flushAll();
-itb->flushAll();
+for (auto tlb : instruction) {
+tlb->flushAll();
+}
+
+for (auto tlb : data) {
+tlb->flushAll();
+}
+
+for (auto tlb : unified) {
+tlb->flushAll();
+}
}
void
diff --git a/src/arch/generic/mmu.hh b/src/arch/generic/mmu.hh
index 8bcb3a7..4767278 100644
--- a/src/arch/generic/mmu.hh
+++ b/src/arch/generic/mmu.hh
@@ -38,6 +38,8 @@
#ifndef __ARCH_GENERIC_MMU_HH__
#define __ARCH_GENERIC_MMU_HH__
+#include
+
#include "params/BaseMMU.hh"
#include "mem/request.hh"
#include "sim/sim_object.hh"
@@ -98,6 +100,12 @@
}
public:
+/**
+ * Called at init time, this method is traversing the TLB hierarchy
+ * and pupulating the instruction/data/unified containers accordingly
+ */
+void initMMU();
+
virtual void flushAll();
void demapPage(Addr vaddr, uint64_t asn);
@@ -123,6 +131,31 @@
public:
BaseTLB* dtb;
BaseTLB* itb;
+
+ protected:
+/**
+ * It is possible from the MMU to traverse the entire hierarchy of
+ * TLBs, starting from the DTB and ITB (generally speaking from the
+ * first level) up to the last level via the nextLevel
[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Explicitly implement I/DTLBI Ops in TLB
Giacomo Travaglini has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48148 )
Change subject: arch-arm: Explicitly implement I/DTLBI Ops in TLB
..
arch-arm: Explicitly implement I/DTLBI Ops in TLB
At the moment the ITLBI and DTLBI operations where implicitly
implemented as generic TLBIs
e.g:
* DTLBIASID, ITLBIASID = TLBIASID
This was possible as a single TLB was either an instruction TLB
or a data TLB and no generic/shared TLB option was available.
In other words, an ITLBI Op to an ITB was invalidating all entries
as we were sure the ITB was not containing data entries.
With shared TLBs, this doesn't hold true and we need to explicitly
implement I/DTLBIs
JIRA: https://gem5.atlassian.net/browse/GEM5-790
Change-Id: I39a4add7674f6008dacaedfd1fd90560d264048e
Signed-off-by: Giacomo Travaglini
---
M src/arch/arm/tlb.cc
M src/arch/arm/tlb.hh
2 files changed, 147 insertions(+), 5 deletions(-)
diff --git a/src/arch/arm/tlb.cc b/src/arch/arm/tlb.cc
index dc0e155..d6f92fd 100644
--- a/src/arch/arm/tlb.cc
+++ b/src/arch/arm/tlb.cc
@@ -229,6 +229,56 @@
}
void
+TLB::flush(const ITLBIALL& tlbi_op)
+{
+DPRINTF(TLB, "Flushing all ITLB entries (%s lookup)\n",
+(tlbi_op.secureLookup ? "secure" : "non-secure"));
+int x = 0;
+TlbEntry *te;
+while (x < size) {
+te = [x];
+const bool el_match = te->checkELMatch(
+tlbi_op.targetEL, tlbi_op.inHost);
+if (te->type == TypeTLB::instruction && te->valid &&
+tlbi_op.secureLookup == !te->nstid &&
+(te->vmid == vmid || tlbi_op.el2Enabled) && el_match) {
+
+DPRINTF(TLB, " - %s\n", te->print());
+te->valid = false;
+stats.flushedEntries++;
+}
+++x;
+}
+
+stats.flushTlb++;
+}
+
+void
+TLB::flush(const DTLBIALL& tlbi_op)
+{
+DPRINTF(TLB, "Flushing all DTLB entries (%s lookup)\n",
+(tlbi_op.secureLookup ? "secure" : "non-secure"));
+int x = 0;
+TlbEntry *te;
+while (x < size) {
+te = [x];
+const bool el_match = te->checkELMatch(
+tlbi_op.targetEL, tlbi_op.inHost);
+if (te->type == TypeTLB::data && te->valid &&
+tlbi_op.secureLookup == !te->nstid &&
+(te->vmid == vmid || tlbi_op.el2Enabled) && el_match) {
+
+DPRINTF(TLB, " - %s\n", te->print());
+te->valid = false;
+stats.flushedEntries++;
+}
+++x;
+}
+
+stats.flushTlb++;
+}
+
+void
TLB::flush(const TLBIALLEL _op)
{
DPRINTF(TLB, "Flushing all TLB entries (%s lookup)\n",
@@ -307,7 +357,29 @@
"(%s lookup)\n", tlbi_op.addr, tlbi_op.asid,
(tlbi_op.secureLookup ? "secure" : "non-secure"));
_flushMva(tlbi_op.addr, tlbi_op.asid, tlbi_op.secureLookup, false,
-tlbi_op.targetEL, tlbi_op.inHost);
+tlbi_op.targetEL, tlbi_op.inHost, TypeTLB::unified);
+stats.flushTlbMvaAsid++;
+}
+
+void
+TLB::flush(const ITLBIMVA _op)
+{
+DPRINTF(TLB, "Flushing ITLB entries with mva: %#x, asid: %#x "
+"(%s lookup)\n", tlbi_op.addr, tlbi_op.asid,
+(tlbi_op.secureLookup ? "secure" : "non-secure"));
+_flushMva(tlbi_op.addr, tlbi_op.asid, tlbi_op.secureLookup, false,
+tlbi_op.targetEL, tlbi_op.inHost, TypeTLB::instruction);
+stats.flushTlbMvaAsid++;
+}
+
+void
+TLB::flush(const DTLBIMVA _op)
+{
+DPRINTF(TLB, "Flushing DTLB entries with mva: %#x, asid: %#x "
+"(%s lookup)\n", tlbi_op.addr, tlbi_op.asid,
+(tlbi_op.secureLookup ? "secure" : "non-secure"));
+_flushMva(tlbi_op.addr, tlbi_op.asid, tlbi_op.secureLookup, false,
+tlbi_op.targetEL, tlbi_op.inHost, TypeTLB::data);
stats.flushTlbMvaAsid++;
}
@@ -337,19 +409,72 @@
}
void
+TLB::flush(const ITLBIASID _op)
+{
+DPRINTF(TLB, "Flushing ITLB entries with asid: %#x (%s lookup)\n",
+tlbi_op.asid,
(tlbi_op.secureLookup ? "secure" : "non-secure"));
+
+int x = 0 ;
+TlbEntry *te;
+
+while (x < size) {
+te = [x];
+if (te->type == TypeTLB::instruction &&
+te->valid && te->asid == tlbi_op.asid &&
+tlbi_op.secureLookup == !te->nstid &&
+(te->vmid == vmid || tlbi_op.el2Enabled) &&
+te->checkELMatch(tlbi_op.targetEL, tlbi_op.inHost)) {
+
+te->valid = false;
+DPRINTF(TLB, " - %s\n", te->print());
+stats.flushedEntries++;
+}
+++x;
+}
+stats.flushTlbAsid++;
+}
+
+void
+TLB::flush(const DTLBIASID _op)
+{
+DPRINTF(TLB, "Flushing DTLB entries with asid: %#x (%s lookup)\n",
+tlbi_op.asid,
(tlbi_op.secureLookup ? "secure" : "non-secure"));
+
+int x = 0 ;
+TlbEntry *te;
+
+while (x < size) {
+te = [x];
+if (te->type == TypeTLB::data &&
+te->valid && te->asid ==
[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Remove unused parameter from TLB::insert
Giacomo Travaglini has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48144 )
Change subject: arch-arm: Remove unused parameter from TLB::insert
..
arch-arm: Remove unused parameter from TLB::insert
Change-Id: Iab395834fe8b3fabf4f5f666af1b8790af08182d
Signed-off-by: Giacomo Travaglini
---
M src/arch/arm/table_walker.cc
M src/arch/arm/tlb.cc
M src/arch/arm/tlb.hh
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/arch/arm/table_walker.cc b/src/arch/arm/table_walker.cc
index fdbd228..ec60a8b 100644
--- a/src/arch/arm/table_walker.cc
+++ b/src/arch/arm/table_walker.cc
@@ -2352,7 +2352,7 @@
descriptor.getRawData());
// Insert the entry into the TLB
-tlb->insert(currState->vaddr, te);
+tlb->insert(te);
if (!currState->timing) {
currState->tc = NULL;
currState->req = NULL;
diff --git a/src/arch/arm/tlb.cc b/src/arch/arm/tlb.cc
index 7aef0c0..dc0e155 100644
--- a/src/arch/arm/tlb.cc
+++ b/src/arch/arm/tlb.cc
@@ -143,7 +143,7 @@
// insert a new TLB entry
void
-TLB::insert(Addr addr, TlbEntry )
+TLB::insert(TlbEntry )
{
DPRINTF(TLB, "Inserting entry into TLB with pfn:%#x size:%#x vpn: %#x"
" asid:%d vmid:%d N:%d global:%d valid:%d nc:%d xn:%d"
diff --git a/src/arch/arm/tlb.hh b/src/arch/arm/tlb.hh
index 1c0549d..927ac23 100644
--- a/src/arch/arm/tlb.hh
+++ b/src/arch/arm/tlb.hh
@@ -180,7 +180,7 @@
void setVMID(vmid_t _vmid) { vmid = _vmid; }
-void insert(Addr vaddr, TlbEntry );
+void insert(TlbEntry );
/** Reset the entire TLB. Used for CPU switching to prevent stale
* translations after multiple switches
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48144
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: Iab395834fe8b3fabf4f5f666af1b8790af08182d
Gerrit-Change-Number: 48144
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini
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-arm: Distinguish Instruction from Data TLB entries
Giacomo Travaglini has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48147 )
Change subject: arch-arm: Distinguish Instruction from Data TLB entries
..
arch-arm: Distinguish Instruction from Data TLB entries
As we are going to support I+D (shared) TLBs we need to tag a TLB entry
as a data or instruction entry, so that I-TLBI and D-TLBI do not
over-invalidate entries
(The patch is also fixing the coding style of the TLB insertion
methods in the Table Walker)
JIRA: https://gem5.atlassian.net/browse/GEM5-790
Change-Id: I3d5880175fe6eda1b2f0edcbd0ea6a146d3c7a39
Signed-off-by: Giacomo Travaglini
---
M src/arch/arm/pagetable.hh
M src/arch/arm/table_walker.cc
2 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/src/arch/arm/pagetable.hh b/src/arch/arm/pagetable.hh
index 53780d7..c3d8738 100644
--- a/src/arch/arm/pagetable.hh
+++ b/src/arch/arm/pagetable.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010, 2012-2013 ARM Limited
+ * Copyright (c) 2010, 2012-2013, 2021 Arm Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -46,6 +46,7 @@
#include "arch/arm/page_size.hh"
#include "arch/arm/types.hh"
#include "arch/arm/utility.hh"
+#include "enums/TypeTLB.hh"
#include "sim/serialize.hh"
namespace gem5
@@ -136,6 +137,9 @@
bool nstid;
// Exception level on insert, AARCH64 EL0&1, AARCH32 -> el=1
ExceptionLevel el;
+// This is used to distinguish between instruction and data entries
+// in unified TLBs
+TypeTLB type;
// Type of memory
bool nonCacheable; // Can we wrap this in mtype?
@@ -156,7 +160,8 @@
innerAttrs(0), outerAttrs(0), ap(read_only ? 0x3 : 0), hap(0x3),
domain(DomainType::Client), mtype(MemoryType::StronglyOrdered),
longDescFormat(false), isHyp(false), global(false), valid(true),
- ns(true), nstid(true), el(EL0), nonCacheable(uncacheable),
+ ns(true), nstid(true), el(EL0), type(TypeTLB::unified),
+ nonCacheable(uncacheable),
shareable(false), outerShareable(false), xn(0), pxn(0)
{
// no restrictions by default, hap = 0x3
@@ -171,7 +176,8 @@
vmid(0), N(0), innerAttrs(0), outerAttrs(0), ap(0), hap(0x3),
domain(DomainType::Client), mtype(MemoryType::StronglyOrdered),
longDescFormat(false), isHyp(false), global(false), valid(false),
- ns(true), nstid(true), el(EL0), nonCacheable(false),
+ ns(true), nstid(true), el(EL0), type(TypeTLB::unified),
+ nonCacheable(false),
shareable(false), outerShareable(false), xn(0), pxn(0)
{
// no restrictions by default, hap = 0x3
@@ -317,6 +323,7 @@
SERIALIZE_SCALAR(valid);
SERIALIZE_SCALAR(ns);
SERIALIZE_SCALAR(nstid);
+SERIALIZE_ENUM(type);
SERIALIZE_SCALAR(nonCacheable);
SERIALIZE_ENUM(lookupLevel);
SERIALIZE_ENUM(mtype);
@@ -347,6 +354,7 @@
UNSERIALIZE_SCALAR(valid);
UNSERIALIZE_SCALAR(ns);
UNSERIALIZE_SCALAR(nstid);
+UNSERIALIZE_ENUM(type);
UNSERIALIZE_SCALAR(nonCacheable);
UNSERIALIZE_ENUM(lookupLevel);
UNSERIALIZE_ENUM(mtype);
diff --git a/src/arch/arm/table_walker.cc b/src/arch/arm/table_walker.cc
index ec60a8b..7171312 100644
--- a/src/arch/arm/table_walker.cc
+++ b/src/arch/arm/table_walker.cc
@@ -1449,16 +1449,16 @@
void
TableWalker::memAttrsLPAE(ThreadContext *tc, TlbEntry ,
-LongDescriptor )
+LongDescriptor _descriptor)
{
assert(_haveLPAE);
uint8_t attr;
-uint8_t sh = lDescriptor.sh();
+uint8_t sh = l_descriptor.sh();
// Different format and source of attributes if this is a stage 2
// translation
if (isStage2) {
-attr = lDescriptor.memAttr();
+attr = l_descriptor.memAttr();
uint8_t attr_3_2 = (attr >> 2) & 0x3;
uint8_t attr_1_0 = attr & 0x3;
@@ -1479,7 +1479,7 @@
te.nonCacheable = (attr_3_2 == 1) || (attr_1_0 == 1);
}
} else {
-uint8_t attrIndx = lDescriptor.attrIndx();
+uint8_t attrIndx = l_descriptor.attrIndx();
// LPAE always uses remapping of memory attributes, irrespective
of the
// value of SCTLR.TRE
@@ -1575,15 +1575,15 @@
void
TableWalker::memAttrsAArch64(ThreadContext *tc, TlbEntry ,
- LongDescriptor )
+ LongDescriptor _descriptor)
{
uint8_t attr;
uint8_t attr_hi;
uint8_t attr_lo;
-uint8_t sh = lDescriptor.sh();
+uint8_t sh = l_descriptor.sh();
if (isStage2) {
-attr = lDescriptor.memAttr();
+attr = l_descriptor.memAttr();
uint8_t attr_hi = (attr >> 2) & 0x3;
uint8_t attr_lo = attr & 0x3;
@@ -1607,7 +1607,7 @@
(attr_lo == 1) || (attr_lo == 2);
[gem5-dev] Change in gem5/gem5[develop]: arch: Add TypeTLB Param in BaseTLB
Giacomo Travaglini has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48142 )
Change subject: arch: Add TypeTLB Param in BaseTLB
..
arch: Add TypeTLB Param in BaseTLB
This patch is adding an enum Param in the BaseTLB to tag which kind of
translation entries the TLB is holding
* instruction: holding instruction entries
* data: holding data entries
* unified: holding instruction and data entries
JIRA: https://gem5.atlassian.net/browse/GEM5-790
Change-Id: I033f840652f354523f48e9eb78033ea759b5d0e0
Signed-off-by: Giacomo Travaglini
---
M src/arch/arm/ArmMMU.py
M src/arch/arm/ArmTLB.py
M src/arch/arm/fastmodel/iris/Iris.py
M src/arch/generic/BaseTLB.py
M src/arch/generic/tlb.hh
M src/arch/mips/MipsMMU.py
M src/arch/power/PowerMMU.py
M src/arch/riscv/RiscvMMU.py
M src/arch/sparc/SparcMMU.py
M src/arch/x86/X86MMU.py
10 files changed, 43 insertions(+), 17 deletions(-)
diff --git a/src/arch/arm/ArmMMU.py b/src/arch/arm/ArmMMU.py
index d7ed550..5017fa8 100644
--- a/src/arch/arm/ArmMMU.py
+++ b/src/arch/arm/ArmMMU.py
@@ -70,8 +70,12 @@
PyBindMethod('wireComponents'),
]
-stage2_itb = Param.ArmTLB(ArmStage2TLB(), "Stage 2 Instruction TLB")
-stage2_dtb = Param.ArmTLB(ArmStage2TLB(), "Stage 2 Data TLB")
+stage2_itb = Param.ArmTLB(
+ArmStage2TLB(entry_type="instruction"),
+"Stage 2 Instruction TLB")
+stage2_dtb = Param.ArmTLB(
+ArmStage2TLB(entry_type="data"),
+"Stage 2 Data TLB")
itb_walker = Param.ArmTableWalker(
ArmTableWalker(), "HW Table walker")
diff --git a/src/arch/arm/ArmTLB.py b/src/arch/arm/ArmTLB.py
index 4681d2e..5c4cca0 100644
--- a/src/arch/arm/ArmTLB.py
+++ b/src/arch/arm/ArmTLB.py
@@ -53,7 +53,7 @@
is_stage2 = True
class ArmITB(ArmTLB):
-pass
+entry_type = "instruction"
class ArmDTB(ArmTLB):
-pass
+entry_type = "data"
diff --git a/src/arch/arm/fastmodel/iris/Iris.py
b/src/arch/arm/fastmodel/iris/Iris.py
index 4bc6add..4979715 100644
--- a/src/arch/arm/fastmodel/iris/Iris.py
+++ b/src/arch/arm/fastmodel/iris/Iris.py
@@ -53,8 +53,8 @@
type = 'IrisMMU'
cxx_class = 'gem5::Iris::MMU'
cxx_header = 'arch/arm/fastmodel/iris/mmu.hh'
-itb = IrisTLB()
-dtb = IrisTLB()
+itb = IrisTLB(entry_type="instruction")
+dtb = IrisTLB(entry_type="data")
class IrisInterrupts(BaseInterrupts):
type = 'IrisInterrupts'
diff --git a/src/arch/generic/BaseTLB.py b/src/arch/generic/BaseTLB.py
index a090a06..4fe2338 100644
--- a/src/arch/generic/BaseTLB.py
+++ b/src/arch/generic/BaseTLB.py
@@ -28,6 +28,18 @@
from m5.params import *
from m5.SimObject import SimObject
+class TypeTLB(ScopedEnum):
+"""
+instruction: TLB contains instruction entries only
+data: TLB contains data entries only
+unified: TLB contains both instruction and data entries
+"""
+map = {
+'instruction' : 0x1,
+'data' : 0x2,
+'unified' : 0x3,
+}
+
class BaseTLB(SimObject):
type = 'BaseTLB'
abstract = True
@@ -41,3 +53,5 @@
mem_side_port = RequestPort("Port closer to memory side")
master = DeprecatedParam(mem_side_port,
'`master` is now called `mem_side_port`')
+
+entry_type = Param.TypeTLB("Instruction/Data/Unified TLB entries")
diff --git a/src/arch/generic/tlb.hh b/src/arch/generic/tlb.hh
index ac05726..7e2ef44 100644
--- a/src/arch/generic/tlb.hh
+++ b/src/arch/generic/tlb.hh
@@ -43,7 +43,9 @@
#include "arch/generic/mmu.hh"
#include "base/logging.hh"
+#include "enums/TypeTLB.hh"
#include "mem/request.hh"
+#include "params/BaseTLB.hh"
#include "sim/sim_object.hh"
namespace gem5
@@ -54,7 +56,11 @@
class BaseTLB : public SimObject
{
protected:
-BaseTLB(const Params ) : SimObject(p) {}
+BaseTLB(const BaseTLBParams )
+ : SimObject(p), _type(p.entry_type)
+{}
+
+TypeTLB _type;
public:
virtual void demapPage(Addr vaddr, uint64_t asn) = 0;
@@ -111,6 +117,8 @@
virtual Port* getTableWalkerPort() { return NULL; }
void memInvalidate() { flushAll(); }
+
+TypeTLB type() const { return _type; }
};
} // namespace gem5
diff --git a/src/arch/mips/MipsMMU.py b/src/arch/mips/MipsMMU.py
index 0de1002..ccaae24 100644
--- a/src/arch/mips/MipsMMU.py
+++ b/src/arch/mips/MipsMMU.py
@@ -42,5 +42,5 @@
type = 'MipsMMU'
cxx_class = 'gem5::MipsISA::MMU'
cxx_header = 'arch/mips/mmu.hh'
-itb = MipsTLB()
-dtb = MipsTLB()
+itb = MipsTLB(entry_type="instruction")
+dtb = MipsTLB(entry_type="data")
diff --git a/src/arch/power/PowerMMU.py b/src/arch/power/PowerMMU.py
index db9673d..aaf288a 100644
--- a/src/arch/power/PowerMMU.py
+++ b/src/arch/power/PowerMMU.py
@@ -42,5 +42,5 @@
type = 'PowerMMU'
cxx_class = 'gem5::PowerISA::MMU'
cxx_header = 'arch/power/mmu.hh'
-itb = PowerTLB()
-dtb = PowerTLB()
+itb =
[gem5-dev] Change in gem5/gem5[develop]: scons: Get rid of some unused or unnecessary PySource members.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48139 )
Change subject: scons: Get rid of some unused or unnecessary PySource
members.
..
scons: Get rid of some unused or unnecessary PySource members.
These were either not used at all, or were unnecessary since they, for
instance, were used to name a variable in an anonymous namespace where
the actual name is hidden and largely irrelevant.
Change-Id: I738960cf0d84017ccc133428aef56701c1d40b03
---
M src/SConscript
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/SConscript b/src/SConscript
index 0048288..77fd15e 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -349,10 +349,8 @@
class PySource(SourceFile):
'''Add a python source file to the named package'''
-invalid_sym_char = re.compile('[^A-z0-9_]')
modules = {}
tnodes = {}
-symnames = {}
def __init__(self, package, source, tags=None, add_tags=None):
'''specify the python package, the source file, and any tags'''
@@ -382,13 +380,10 @@
self.modpath = modpath
self.arcname = os.path.join(*arcpath)
self.abspath = abspath
-self.compiled = File(self.filename + 'c')
self.cpp = File(self.filename + '.cc')
-self.symname = PySource.invalid_sym_char.sub('_', modpath)
PySource.modules[modpath] = self
PySource.tnodes[self.tnode] = self
-PySource.symnames[self.symname] = self
class SimObject(PySource):
'''Add a SimObject python file as a python source object and add
@@ -1244,7 +1239,6 @@
compressed = zlib.compress(marshalled)
data = compressed
pysource = PySource.tnodes[source[1]]
-sym = pysource.symname
code = code_formatter()
code('''\
@@ -1256,13 +1250,13 @@
{
''')
-bytesToCppArray(code, 'data_{}'.format(sym), data)
+bytesToCppArray(code, 'embedded_module_data', data)
code('''
-EmbeddedPython embedded_${sym}(
+EmbeddedPython embedded_module_info(
${{c_str(pysource.arcname)}},
${{c_str(pysource.abspath)}},
${{c_str(pysource.modpath)}},
-data_${sym},
+embedded_module_data,
${{len(data)}},
${{len(marshalled)}});
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48139
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: I738960cf0d84017ccc133428aef56701c1d40b03
Gerrit-Change-Number: 48139
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]: scons: Turn the Blob method into a builder.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48136 )
Change subject: scons: Turn the Blob method into a builder.
..
scons: Turn the Blob method into a builder.
Build the blob .cc and .hh files in the same directory as the file
they're based off of. Move the GDB XML files into the arch directories
they go with.
Change-Id: I12fe48873312c3aba5910989d6e3049ebd5e5bbf
---
M SConstruct
M src/SConscript
M src/arch/arm/SConscript
A src/arch/arm/gdb-xml/SConscript
R src/arch/arm/gdb-xml/aarch64-core.xml
R src/arch/arm/gdb-xml/aarch64-fpu.xml
R src/arch/arm/gdb-xml/aarch64.xml
R src/arch/arm/gdb-xml/arm-core.xml
R src/arch/arm/gdb-xml/arm-vfpv3.xml
R src/arch/arm/gdb-xml/arm-with-neon.xml
M src/arch/arm/remote_gdb.cc
M src/arch/mips/SConscript
A src/arch/mips/gdb-xml/SConscript
R src/arch/mips/gdb-xml/mips.xml
M src/arch/mips/remote_gdb.cc
M src/arch/power/SConscript
A src/arch/power/gdb-xml/SConscript
R src/arch/power/gdb-xml/power-core.xml
R src/arch/power/gdb-xml/power-fpu.xml
R src/arch/power/gdb-xml/power64-core.xml
R src/arch/power/gdb-xml/powerpc-32.xml
R src/arch/power/gdb-xml/powerpc-64.xml
M src/arch/power/remote_gdb.cc
M src/arch/riscv/SConscript
A src/arch/riscv/gdb-xml/SConscript
R src/arch/riscv/gdb-xml/riscv-64bit-cpu.xml
R src/arch/riscv/gdb-xml/riscv-64bit-csr.xml
R src/arch/riscv/gdb-xml/riscv-64bit-fpu.xml
R src/arch/riscv/gdb-xml/riscv.xml
M src/arch/riscv/remote_gdb.cc
30 files changed, 243 insertions(+), 99 deletions(-)
diff --git a/SConstruct b/SConstruct
index 086b3ca..2b2e39c 100755
--- a/SConstruct
+++ b/SConstruct
@@ -615,9 +615,6 @@
main.SConscript(os.path.join(root, 'SConscript'),
variant_dir=os.path.join(build_root, build_dir))
-gdb_xml_dir = os.path.join(ext_dir, 'gdb-xml')
-Export('gdb_xml_dir')
-
#
diff --git a/src/SConscript b/src/SConscript
index 175b8cf..1272c06 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -286,71 +286,69 @@
code.dedent()
code('};')
-def blobToCpp(data, symbol, cpp_code, hpp_code, namespace):
-'''
-Convert bytes data into C++ .cpp and .hh uint8_t byte array
-code containing that binary data.
-
-:param data: binary data to be converted to C++
-:param symbol: name of the symbol
-:param cpp_code: append the generated cpp_code to this object
-:param hpp_code: append the generated hpp_code to this object
- Also include it in the .cpp file.
-:param namespace: namespace to put the symbol into.
-'''
-hpp_code('''\
-#include
-#include
-
-namespace ${namespace}
-{
-
-extern const std::size_t ${symbol}_len;
-extern const std::uint8_t ${symbol};
-
-}
-''')
-
-cpp_code('''\
-#include "blobs/${symbol}.hh"
-
-namespace ${namespace}
-{
-
-const std::size_t ${symbol}_len = ${{len(data)}};
-''')
-bytesToCode(cpp_code, symbol, data)
-cpp_code('}')
-
-def Blob(blob_path, symbol):
+def build_blob(target, source, env):
'''
Embed an arbitrary blob into the gem5 executable,
and make it accessible to C++ as a byte array.
'''
-blob_path = os.path.abspath(blob_path)
-blob_out_dir = os.path.join(env['BUILDDIR'], 'blobs')
-path_noext = os.path.join(blob_out_dir, symbol)
-cpp_path = path_noext + '.cc'
-hpp_path = path_noext + '.hh'
-def embedBlob(target, source, env):
-with open(str(source[0]), 'rb') as f:
-data = f.read()
-cpp_code = code_formatter()
-hpp_code = code_formatter()
-blobToCpp(data, symbol, cpp_code, hpp_code, namespace='Blobs')
-cpp_path = str(target[0])
-hpp_path = str(target[1])
-cpp_dir = os.path.split(cpp_path)[0]
-if not os.path.exists(cpp_dir):
-os.makedirs(cpp_dir)
-cpp_code.write(cpp_path)
-hpp_code.write(hpp_path)
-env.Command([cpp_path, hpp_path], blob_path,
-MakeAction(embedBlob, Transform("EMBED BLOB")))
-Source(cpp_path)
+
+with open(str(source[0]), 'rb') as f:
+data = f.read()
+symbol = str(source[1])
+cc, hh = target
+
+hh_code = code_formatter()
+hh_code('''\
+#include
+#include
+
+namespace gem5
+{
+namespace Blobs
+{
+
+extern const std::size_t ${symbol}_len;
+extern const std::uint8_t ${symbol}[];
+
+} // namespace Blobs
+} // namespace gem5
+''')
+hh_code.write(str(hh))
+
+include_path = os.path.relpath(hh.abspath, env['BUILDDIR'])
+
+cc_code = code_formatter()
+cc_code('''\
+#include "${include_path}"
+
+namespace gem5
+{
+namespace Blobs
+{
+
+const std::size_t ${symbol}_len = ${{len(data)}};
+''')
+bytesToCppArray(cc_code, symbol, data)
+cc_code('''
+} // namespace Blobs
+} // namespace gem5
+''')
+cc_code.write(str(cc))
+
+blob_action = MakeAction(build_blob, Transform("EMBED BLOB"))
+
+def
[gem5-dev] Change in gem5/gem5[develop]: scons: Use a different suffix for test object files.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/48137 ) Change subject: scons: Use a different suffix for test object files. .. scons: Use a different suffix for test object files. These files are built with a different command line, and so should be distinct build artifacts in the build directory. Change-Id: Iec9403ad73fbdbcb1cd268d68716c3aa85a80b24 --- M src/SConscript 1 file changed, 2 insertions(+), 0 deletions(-) diff --git a/src/SConscript b/src/SConscript index 1272c06..358f0b8 100644 --- a/src/SConscript +++ b/src/SConscript @@ -553,6 +553,8 @@ @classmethod def declare_all(cls, env): env = env.Clone() +env['OBJSUFFIX'] = 't' + env['OBJSUFFIX'] +env['SHOBJSUFFIX'] = 't' + env['SHOBJSUFFIX'] env.Append(LIBS=env['GTEST_LIBS']) env.Append(CPPFLAGS=env['GTEST_CPPFLAGS']) env['GTEST_LIB_SOURCES'] = Source.all.with_tag(env, 'gtest lib') -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48137 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: Iec9403ad73fbdbcb1cd268d68716c3aa85a80b24 Gerrit-Change-Number: 48137 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]: scons: Factor out the core of blobToCpp.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48135 )
Change subject: scons: Factor out the core of blobToCpp.
..
scons: Factor out the core of blobToCpp.
blobToCpp is called in two places, one which uses all its functionality,
and one which disables most of it. Instead, factor out the small core so
that it can be called directly by the call sight which uses only that
part, and blobToCpp itself.
This change also removes the ability to leave out a namespace or header
file code formatter, since the function is not called that way any more.
That simplifies blobToCpp significantly.
Change-Id: I63139311521bb4f9287fe41ff51e4e5301a18349
---
M src/SConscript
1 file changed, 38 insertions(+), 36 deletions(-)
diff --git a/src/SConscript b/src/SConscript
index 1750ed5..175b8cf 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -273,7 +273,20 @@
self.shared_objs[key] = env.SharedObject(self.tnode)
return self.shared_objs[key]
-def blobToCpp(data, symbol, cpp_code, hpp_code=None, namespace=None):
+def bytesToCppArray(code, symbol, data):
+'''
+Output an array of bytes to a code formatter as a c++ array
declaration.
+'''
+code('const std::uint8_t ${symbol}[] = {')
+code.indent()
+step = 16
+for i in range(0, len(data), step):
+x = array.array('B', data[i:i+step])
+code(''.join('%d,' % d for d in x))
+code.dedent()
+code('};')
+
+def blobToCpp(data, symbol, cpp_code, hpp_code, namespace):
'''
Convert bytes data into C++ .cpp and .hh uint8_t byte array
code containing that binary data.
@@ -282,41 +295,32 @@
:param symbol: name of the symbol
:param cpp_code: append the generated cpp_code to this object
:param hpp_code: append the generated hpp_code to this object
- If None, ignore it. Otherwise, also include it
- in the .cpp file.
-:param namespace: namespace to put the symbol into. If None,
- don't put the symbols into any namespace.
+ Also include it in the .cpp file.
+:param namespace: namespace to put the symbol into.
'''
-symbol_len_declaration = 'const std::size_t {}_len'.format(symbol)
-symbol_declaration = 'const std::uint8_t {}[]'.format(symbol)
-if hpp_code is not None:
-cpp_code('''\
-#include "blobs/{}.hh"
-'''.format(symbol))
-hpp_code('''\
+hpp_code('''\
#include
#include
+
+namespace ${namespace}
+{
+
+extern const std::size_t ${symbol}_len;
+extern const std::uint8_t ${symbol};
+
+}
''')
-if namespace is not None:
-hpp_code('namespace {} {{'.format(namespace))
-hpp_code('extern ' + symbol_len_declaration + ';')
-hpp_code('extern ' + symbol_declaration + ';')
-if namespace is not None:
-hpp_code('}')
-if namespace is not None:
-cpp_code('namespace {} {{'.format(namespace))
-if hpp_code is not None:
-cpp_code(symbol_len_declaration + ' = {};'.format(len(data)))
-cpp_code(symbol_declaration + ' = {')
-cpp_code.indent()
-step = 16
-for i in range(0, len(data), step):
-x = array.array('B', data[i:i+step])
-cpp_code(''.join('%d,' % d for d in x))
-cpp_code.dedent()
-cpp_code('};')
-if namespace is not None:
-cpp_code('}')
+
+cpp_code('''\
+#include "blobs/${symbol}.hh"
+
+namespace ${namespace}
+{
+
+const std::size_t ${symbol}_len = ${{len(data)}};
+''')
+bytesToCode(cpp_code, symbol, data)
+cpp_code('}')
def Blob(blob_path, symbol):
'''
@@ -1259,10 +1263,8 @@
{
''')
-blobToCpp(data, 'data_' + sym, code)
-code('''\
-
-
+bytesToCppArray(code, 'data_{}'.format(sym), data)
+code('''
EmbeddedPython embedded_${sym}(
${{c_str(pysource.arcname)}},
${{c_str(pysource.abspath)}},
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48135
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: I63139311521bb4f9287fe41ff51e4e5301a18349
Gerrit-Change-Number: 48135
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]: scons: Turn the ProtoBuf and GrpcProtoBuf classes into methods.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48134 )
Change subject: scons: Turn the ProtoBuf and GrpcProtoBuf classes into
methods.
..
scons: Turn the ProtoBuf and GrpcProtoBuf classes into methods.
These now instantiate Builders for the protobuf .cc and .h files of the
right flavors, and install the .cc files as sources.
Also, this sets up the builder for .cc files so that it will know how to
build those files from .proto using the protobuf compiler. This is
optional, but will make it possible to pass .proto files in place of .cc
files or even object files as sources for other builders, and for scons
to figure out what chain of rules to use to get the desired results.
This is a step towards handing over more responsibility to scons and
eliminating at least some of our bespoke build code.
Change-Id: I7188a0917e999ad9148f7079830b2c26bcd563db
---
M src/SConscript
1 file changed, 26 insertions(+), 47 deletions(-)
diff --git a/src/SConscript b/src/SConscript
index a6229a6..1750ed5 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -430,36 +430,25 @@
root, ext = os.path.splitext(source[0].get_abspath())
return [root + '.pb.cc', root + '.pb.h'], source
-env.Append(BUILDERS={'ProtoBufCC' : Builder(
-action=MakeAction('${PROTOC} --cpp_out ${BUILDDIR} '
- '--proto_path ${BUILDDIR} '
- '${SOURCE.get_abspath()}',
- Transform("PROTOC")),
-emitter=protoc_emitter
-)})
+protoc_action = MakeAction('${PROTOC} --cpp_out ${BUILDDIR} '
+'--proto_path ${BUILDDIR} ${SOURCE.get_abspath()}',
+Transform("PROTOC"))
+protobuf_builder = Builder(action=protoc_action, emitter=protoc_emitter,
+src_suffix='.proto')
+env.Append(BUILDERS={'ProtoBufCC' : protobuf_builder})
-class ProtoBuf(SourceFile):
+c_file, cxx_file = SCons.Tool.createCFileBuilders(env)
+cxx_file.add_action('.proto', protoc_action)
+cxx_file.add_emitter('.proto', protoc_emitter)
+
+def ProtoBuf(source, tags=None, add_tags=None):
'''Add a Protocol Buffer to build'''
-def __init__(self, source, tags=None, add_tags=None):
-'''Specify the source file, and any tags'''
-super(ProtoBuf, self).__init__(source, tags, add_tags)
+if not env['HAVE_PROTOC'] or not env['HAVE_PROTOBUF']:
+error('Got protobuf to build, but lacks support!')
-if not env['HAVE_PROTOC'] or not env['HAVE_PROTOBUF']:
-error('Got protobuf to build, but lacks support!')
-
-# Get the file name and the extension
-basename = os.path.basename(self.filename)
-modname, ext = os.path.splitext(basename)
-assert ext == '.proto'
-
-self.cc_file, self.hh_file = env.ProtoBufCC(source=source)
-
-# Add the C++ source file
-Source(self.cc_file, tags=self.tags,
-append={'CXXFLAGS': '-Wno-array-bounds'})
-
-
+'''Specify the source file, and any tags'''
+Source(source, tags, add_tags,
append={'CXXFLAGS': '-Wno-array-bounds'})
env['PROTOC_GRPC'] = distutils.spawn.find_executable('grpc_cpp_plugin')
if env['PROTOC_GRPC']:
@@ -469,34 +458,24 @@
root, ext = os.path.splitext(source[0].get_abspath())
return [root + '.grpc.pb.cc', root + '.grpc.pb.h'], source
+protoc_grpc_action=MakeAction('${PROTOC} --grpc_out ${BUILDDIR} '
+'--plugin=protoc-gen-grpc=${PROTOC_GRPC} --proto_path ${BUILDDIR} '
+'${SOURCE.get_abspath()}',
+Transform("PROTOC"))
+
env.Append(BUILDERS={'GrpcProtoBufCC' : Builder(
-action=MakeAction('${PROTOC} --grpc_out ${BUILDDIR} '
- '--plugin=protoc-gen-grpc=${PROTOC_GRPC} '
- '--proto_path ${BUILDDIR} '
- '${SOURCE.get_abspath()}',
- Transform("PROTOC")),
+action=protoc_grpc_action,
emitter=protoc_grpc_emitter
)})
-class GrpcProtoBuf(SourceFile):
+def GrpcProtoBuf(source, tags=None, add_tags=None):
'''Add a GRPC protocol buffer to the build'''
-def __init__(self, source, tags=None, add_tags=None):
-'''Specify the source file, and any tags'''
+if not env['PROTOC_GRPC']:
+error('No grpc_cpp_plugin found')
-super(GrpcProtoBuf, self).__init__(source, tags, add_tags)
-
-if not env['PROTOC_GRPC']:
-error('No grpc_cpp_plugin found')
-
-self.cc_file, self.hh_file = env.GrpcProtoBufCC(source=source)
-
-# We still need to build the normal protobuf code too.
-self.protobuf = ProtoBuf(source, tags=self.tags)
-
-# Add the C++ source file.
-Source(self.cc_file, tags=self.tags,
-append={'CXXFLAGS': '-Wno-array-bounds'})
+Source(env.GrpcProtoBufCC(source=source)[0], tags=tags,
[gem5-dev] Change in gem5/gem5[develop]: scons: Stop caching the first version of object files.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/48138 ) Change subject: scons: Stop caching the first version of object files. .. scons: Stop caching the first version of object files. Don't cache the first version requested of object files to use for subsequent requests. This was originally put in place to avoid an error when object files could be built with trivially different command lines, ie command lines which are technically different but not in a necessarily meaningful way, or less seriously a warning when the command lines were the same. The warning was disabled in an earlier change, and the error was avoided by using a different object file suffix when building unit tests. This helps avoid bugs if the object files actually *would* turn out to be different in a meaningful way based on the flags used, and simplifies the build. Change-Id: I6b90e6e36b13adb73e587bb8fc533984f764d95a --- M src/SConscript 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/SConscript b/src/SConscript index 358f0b8..0048288 100644 --- a/src/SConscript +++ b/src/SConscript @@ -256,22 +256,16 @@ base.all.append(self) def static(self, env): -key = (self.tnode, env['OBJSUFFIX']) if self.append: env = env.Clone() env.Append(**self.append) -if not key in self.static_objs: -self.static_objs[key] = env.StaticObject(self.tnode) -return self.static_objs[key] +return env.StaticObject(self.tnode) def shared(self, env): -key = (self.tnode, env['OBJSUFFIX']) if self.append: env = env.Clone() env.Append(**self.append) -if not key in self.shared_objs: -self.shared_objs[key] = env.SharedObject(self.tnode) -return self.shared_objs[key] +return env.SharedObject(self.tnode) def bytesToCppArray(code, symbol, data): ''' -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48138 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: I6b90e6e36b13adb73e587bb8fc533984f764d95a Gerrit-Change-Number: 48138 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]: scons: Disable the duplicate-environment warning by default.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48133 )
Change subject: scons: Disable the duplicate-environment warning by default.
..
scons: Disable the duplicate-environment warning by default.
Disable warnings when targets can be built with multiple environments
but with the same actions. This can happen intentionally if, for
instance, a generated source file is used to build object files in
different ways in different environments, but generating the source
file itself is exactly the same. This can be re-enabled from the
command line if desired.
Change-Id: I426ca3331267f026553c9a2bccc79494cace8109
---
M SConstruct
1 file changed, 7 insertions(+), 0 deletions(-)
diff --git a/SConstruct b/SConstruct
index ca1bc7b..086b3ca 100755
--- a/SConstruct
+++ b/SConstruct
@@ -132,6 +132,13 @@
from gem5_scons.builders import ConfigFile, AddLocalRPATH, SwitchingHeaders
from gem5_scons.util import compareVersions, readCommand
+# Disable warnings when targets can be built with multiple environments but
+# with the same actions. This can happen intentionally if, for instance, a
+# generated source file is used to build object files in different ways in
+# different environments, but generating the source file itself is exactly
the
+# same. This can be re-enabled from the command line if desired.
+SetOption('warn', 'no-duplicate-environment')
+
Export('MakeAction')
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48133
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: I426ca3331267f026553c9a2bccc79494cace8109
Gerrit-Change-Number: 48133
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]: scons: Delay evaluating the EXE_SUFFIX and ENV_LABEL values.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48132 )
Change subject: scons: Delay evaluating the EXE_SUFFIX and ENV_LABEL values.
..
scons: Delay evaluating the EXE_SUFFIX and ENV_LABEL values.
Delay evaluating these so that hopefully the same rules can be reused
between environments.
Change-Id: I4918d3a1a98f49ecad8ab6a8b89898c14316f6f5
---
M src/SConscript
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/SConscript b/src/SConscript
index 2186c75..a6229a6 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -533,7 +533,7 @@
self.dir = Dir('.')
def path(self, env):
-return self.dir.File(self.target + '.' + env['EXE_SUFFIX'])
+return self.dir.File(self.target + '.${EXE_SUFFIX}')
def srcs_to_objs(self, env, sources):
return list([ s.static(env) for s in sources ])
@@ -550,7 +550,7 @@
env['BIN_RPATH_PREFIX'] = os.path.relpath(
env['BUILDDIR'], self.path(env).dir.abspath)
-executable = env.Program(self.path(env), objs)[0]
+executable = env.Program(self.path(env).abspath, objs)[0]
if sys.platform == 'sunos5':
cmd = 'cp $SOURCE $TARGET; strip $TARGET'
@@ -576,7 +576,7 @@
env.Append(CPPFLAGS=env['GTEST_CPPFLAGS'])
env['GTEST_LIB_SOURCES'] = Source.all.with_tag(env, 'gtest lib')
env['GTEST_OUT_DIR'] = \
-Dir(env['BUILDDIR']).Dir('unittests.' + env['EXE_SUFFIX'])
+Dir(env['BUILDDIR']).Dir('unittests.${EXE_SUFFIX}')
return super(GTest, cls).declare_all(env)
def declare(self, env):
@@ -591,7 +591,7 @@
out_dir = env['GTEST_OUT_DIR']
xml_file = out_dir.Dir(str(self.dir)).File(self.target + '.xml')
-AlwaysBuild(env.Command(xml_file, binary,
+AlwaysBuild(env.Command(xml_file.abspath, binary,
"${SOURCES[0]} --gtest_output=xml:${TARGETS[0]}"))
return binary
@@ -1423,7 +1423,7 @@
env['SHARED_LIB'] = shared_lib
# Record some settings for building Executables.
-env['EXE_SUFFIX'] = env['ENV_LABEL']
+env['EXE_SUFFIX'] = '${ENV_LABEL}'
for cls in ExecutableMeta.all:
cls.declare_all(env)
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48132
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: I4918d3a1a98f49ecad8ab6a8b89898c14316f6f5
Gerrit-Change-Number: 48132
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]: scons: Build up different environments for different binaries.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48131 )
Change subject: scons: Build up different environments for different
binaries.
..
scons: Build up different environments for different binaries.
Rather than collect parameters describing what environments to use for
each binary, build up the environments themselves and skip the middle
man.
Change-Id: I57ff03a3522708396149b7d053dac014477859a7
---
M src/SConscript
1 file changed, 74 insertions(+), 95 deletions(-)
diff --git a/src/SConscript b/src/SConscript
index 5ee6cf0..2186c75 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -1319,108 +1319,46 @@
Gem5('gem5')
-# Function to create a new build environment as clone of current
-# environment 'env' with modified object suffix and optional stripped
-# binary. Additional keyword arguments are appended to corresponding
-# build environment vars.
-def makeEnv(env, label, objsfx, **kwargs):
-# SCons doesn't know to append a library suffix when there is a '.' in
the
-# name. Use '_' instead.
-libname = 'gem5_' + label
+env['SHOBJSUFFIX'] = '${OBJSUFFIX}s'
-new_env = env.Clone(OBJSUFFIX=objsfx, SHOBJSUFFIX=objsfx + 's')
-new_env.Append(**kwargs)
-
-lib_sources = Source.all.with_tag(new_env, 'gem5 lib')
-
-# Without Python, leave out all Python content from the library
-# builds. The option doesn't affect gem5 built as a program
-if GetOption('without_python'):
-lib_sources = lib_sources.without_tag(new_env, 'python')
-
-static_objs = list([ s.static(new_env) for s in lib_sources ])
-shared_objs = list([ s.shared(new_env) for s in lib_sources ])
-
-static_date = date_source.static(new_env)
-new_env.Depends(static_date, static_objs)
-static_objs.extend(static_date)
-
-shared_date = date_source.shared(new_env)
-new_env.Depends(shared_date, shared_objs)
-shared_objs.extend(shared_date)
-
-main_objs = [ s.static(new_env) for s in
-Source.all.with_tag(new_env, 'main') ]
-
-# First make a library of everything but main() so other programs can
-# link against m5.
-static_lib = new_env.StaticLibrary(libname, static_objs)
-shared_lib = new_env.SharedLibrary(libname, shared_objs)
-
-# Keep track of the object files generated so far so Executables can
-# include them.
-new_env['STATIC_OBJS'] = static_objs
-new_env['SHARED_OBJS'] = shared_objs
-new_env['MAIN_OBJS'] = main_objs
-
-new_env['STATIC_LIB'] = static_lib
-new_env['SHARED_LIB'] = shared_lib
-
-# Record some settings for building Executables.
-new_env['EXE_SUFFIX'] = label
-
-for cls in ExecutableMeta.all:
-cls.declare_all(new_env)
-
-class EnvParams(object):
-def __init__(self, label, obj, ccflags=None, cppdefines=None,
-ldflags=None):
-self.label = label
-self.obj = obj
-self.ccflags = ccflags if ccflags else []
-self.cppdefines = cppdefines if cppdefines else []
-self.ldflags = ldflags if ldflags else []
-
-# Start out with the compiler flags common to all compilers and linkers,
-# i.e. all compilers use -g for opt and -g -pg for prof, and all linkers
use
-# -pg for prof, and -lprofiler for perf.
-env_params = {
-'debug': EnvParams('debug', 'do',
cppdefines=['DEBUG', 'TRACING_ON=1']),
-'opt': EnvParams('opt', 'o', ccflags=['-g'],
cppdefines=['TRACING_ON=1']),
-'fast': EnvParams('fast', 'fo', cppdefines=['NDEBUG', 'TRACING_ON=0']),
-'prof': EnvParams('prof', 'po', ccflags=['-g', '-pg'],
- cppdefines=['NDEBUG', 'TRACING_ON=0'],
ldflags=['-pg']),
-# The -lprofile flag is surrounded by no-as-needed and as-needed as the
-# binutils linker is too clever and simply doesn't link to the library
-# otherwise.
-'perf': EnvParams('perf', 'gpo', ccflags=['-g'],
- cppdefines=['NDEBUG', 'TRACING_ON=0'],
- ldflags=['-Wl,--no-as-needed', '-lprofiler',
- '-Wl,--as-needed'])
+envs = {
+'debug': env.Clone(ENV_LABEL='debug', OBJSUFFIX='.do'),
+'opt': env.Clone(ENV_LABEL='opt', OBJSUFFIX='.o'),
+'fast': env.Clone(ENV_LABEL='fast', OBJSUFFIX='.fo'),
+'prof': env.Clone(ENV_LABEL='prof', OBJSUFFIX='.po'),
+'perf': env.Clone(ENV_LABEL='perf', OBJSUFFIX='.gpo')
}
+envs['debug'].Append(CPPDEFINES=['DEBUG', 'TRACING_ON=1'])
+envs['opt'].Append(CCFLAGS=['-g'], CPPDEFINES=['TRACING_ON=1'])
+envs['fast'].Append(CPPDEFINES=['NDEBUG', 'TRACING_ON=0'])
+envs['prof'].Append(CCFLAGS=['-g', '-pg'], LDFLAGS=['-pg'],
+CPPDEFINES=['NDEBUG', 'TRACING_ON=0'])
+envs['perf'].Append(CCFLAGS=['-g'], CPPDEFINES=['NDEBUG', 'TRACING_ON=0'],
+LDFLAGS=['-Wl,--no-as-needed', '-lprofiler', '-Wl,--as-needed'])
+
# For Link Time Optimization, the optimisation flags used to compile
#
[gem5-dev] Change in gem5/gem5[develop]: scons: Use a loop to build binary flavors.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48130 )
Change subject: scons: Use a loop to build binary flavors.
..
scons: Use a loop to build binary flavors.
Track structured data related to different binary flavors (opt, debug,
etc), using a class instead of various lists, etc. Also use a loop to
set up SCons environments to build these binaries instead of a spelled
out loop.
Change-Id: Ie35a914ab79342190e4cdc27a945a0fecd54a476
---
M src/SConscript
1 file changed, 53 insertions(+), 69 deletions(-)
diff --git a/src/SConscript b/src/SConscript
index 196b80c..5ee6cf0 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -1372,17 +1372,32 @@
for cls in ExecutableMeta.all:
cls.declare_all(new_env)
-# Start out with the compiler flags common to all compilers,
-# i.e. they all use -g for opt and -g -pg for prof
-ccflags = {'debug' : [], 'opt' : ['-g'], 'fast' : [], 'prof' :
['-g', '-pg'],
- 'perf' : ['-g']}
+class EnvParams(object):
+def __init__(self, label, obj, ccflags=None, cppdefines=None,
+ldflags=None):
+self.label = label
+self.obj = obj
+self.ccflags = ccflags if ccflags else []
+self.cppdefines = cppdefines if cppdefines else []
+self.ldflags = ldflags if ldflags else []
-# Start out with the linker flags common to all linkers, i.e. -pg for
-# prof, and -lprofiler for perf. The -lprofile flag is surrounded by
-# no-as-needed and as-needed as the binutils linker is too clever and
-# simply doesn't link to the library otherwise.
-ldflags = {'debug' : [], 'opt' : [], 'fast' : [], 'prof' : ['-pg'],
- 'perf' :
['-Wl,--no-as-needed', '-lprofiler', '-Wl,--as-needed']}
+# Start out with the compiler flags common to all compilers and linkers,
+# i.e. all compilers use -g for opt and -g -pg for prof, and all linkers
use
+# -pg for prof, and -lprofiler for perf.
+env_params = {
+'debug': EnvParams('debug', 'do',
cppdefines=['DEBUG', 'TRACING_ON=1']),
+'opt': EnvParams('opt', 'o', ccflags=['-g'],
cppdefines=['TRACING_ON=1']),
+'fast': EnvParams('fast', 'fo', cppdefines=['NDEBUG', 'TRACING_ON=0']),
+'prof': EnvParams('prof', 'po', ccflags=['-g', '-pg'],
+ cppdefines=['NDEBUG', 'TRACING_ON=0'],
ldflags=['-pg']),
+# The -lprofile flag is surrounded by no-as-needed and as-needed as the
+# binutils linker is too clever and simply doesn't link to the library
+# otherwise.
+'perf': EnvParams('perf', 'gpo', ccflags=['-g'],
+ cppdefines=['NDEBUG', 'TRACING_ON=0'],
+ ldflags=['-Wl,--no-as-needed', '-lprofiler',
+ '-Wl,--as-needed'])
+}
# For Link Time Optimization, the optimisation flags used to compile
# individual files are decoupled from those used at link time
@@ -1390,77 +1405,46 @@
# to also update the linker flags based on the target.
if env['GCC']:
if sys.platform == 'sunos5':
-ccflags['debug'] += ['-gstabs+']
+env_params['debug'].ccflags += ['-gstabs+']
else:
-ccflags['debug'] += ['-ggdb3']
-ldflags['debug'] += ['-O0']
+env_params['debug'].ccflags += ['-ggdb3']
+env_params['debug'].ldflags += ['-O0']
# opt, fast, prof and perf all share the same cc flags, also add
# the optimization to the ldflags as LTO defers the optimization
# to link time
for target in ['opt', 'fast', 'prof', 'perf']:
-ccflags[target] += ['-O3'] + env['LTO_CCFLAGS']
-ldflags[target] += ['-O3'] + env['LTO_LDFLAGS']
+env_params[target].ccflags += ['-O3'] + env['LTO_CCFLAGS']
+env_params[target].ldflags += ['-O3'] + env['LTO_LDFLAGS']
elif env['CLANG']:
-ccflags['debug'] += ['-g', '-O0']
+env_params['debug'].ccflags += ['-g', '-O0']
# opt, fast, prof and perf all share the same cc flags
for target in ['opt', 'fast', 'prof', 'perf']:
-ccflags[target] += ['-O3']
+env_params[target].ccflags += ['-O3']
else:
error('Unknown compiler, please fix compiler options')
-# To speed things up, we only instantiate the build environments we
-# need. We try to identify the needed environment for each target; if
-# we can't, we fall back on instantiating all the environments just to
-# be safe.
-target_types = {'debug', 'opt', 'fast', 'prof', 'perf'}
-obj2target = {'do': 'debug', 'o': 'opt', 'fo': 'fast', 'po': 'prof',
- 'gpo' : 'perf'}
+# To speed things up, we only instantiate the build environments we need.
We
+# try to identify the needed environment for each target; if we can't, we
fall
+# back on instantiating all the environments just to be safe.
-def identifyTarget(t):
-ext = t.split('.')[-1]
-if ext in target_types:
-return ext
-if ext in obj2target:
-return obj2target[ext]
-return 'all'
+# A
[gem5-dev] Change in gem5/gem5[develop]: scons: Use sets instead of lists to track needed target environments.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48129 )
Change subject: scons: Use sets instead of lists to track needed target
environments.
..
scons: Use sets instead of lists to track needed target environments.
This simple change intrinsically collapses away duplicates.
Change-Id: I697c21897a81c47cbf540caa49806413dce80dba
---
M src/SConscript
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/SConscript b/src/SConscript
index 7b62724..196b80c 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -1414,7 +1414,7 @@
# need. We try to identify the needed environment for each target; if
# we can't, we fall back on instantiating all the environments just to
# be safe.
-target_types = ['debug', 'opt', 'fast', 'prof', 'perf']
+target_types = {'debug', 'opt', 'fast', 'prof', 'perf'}
obj2target = {'do': 'debug', 'o': 'opt', 'fo': 'fast', 'po': 'prof',
'gpo' : 'perf'}
@@ -1426,9 +1426,9 @@
return obj2target[ext]
return 'all'
-needed_envs = [identifyTarget(target) for target in BUILD_TARGETS]
+needed_envs = {identifyTarget(target) for target in BUILD_TARGETS}
if 'all' in needed_envs:
-needed_envs += target_types
+needed_envs = target_types
# Debug binary
if 'debug' in needed_envs:
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48129
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: I697c21897a81c47cbf540caa49806413dce80dba
Gerrit-Change-Number: 48129
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]: scons: Make all Executables strip-able, and de-special case .fast.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48120 )
Change subject: scons: Make all Executables strip-able, and de-special
case .fast.
..
scons: Make all Executables strip-able, and de-special case .fast.
The build for .fast was set up to produce a stripped executable, and the
unstripped executable was instead named .fast.unstripped. I think the
assumption that a stripped executable is faster than an unstripped
executable is flawed, since the parts of the binary that are removed,
debug symbols, are not loaded into memory anyway, so while the program
is executing it shouldn't be any different or take up any additional
memory. This also made .fast a special case compared to the other build
types, like .opt, .debug, etc.
Instead, this change makes .fast unstripped like all the other binaries,
and also makes it possible to request a stripped version of *any* binary
the build can produce with a .stripped suffix.
Change-Id: I2de82e0951d9f41c30594f32fba50acdd14ed69c
---
M src/SConscript
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/src/SConscript b/src/SConscript
index 6a630da..a9d70bf 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -576,18 +576,16 @@
env['BIN_RPATH_PREFIX'] = os.path.relpath(
env['BUILDDIR'], self.path(env).dir.abspath)
-if env['STRIP_EXES']:
-stripped = self.path(env)
-unstripped = env.File(str(stripped) + '.unstripped')
-if sys.platform == 'sunos5':
-cmd = 'cp $SOURCE $TARGET; strip $TARGET'
-else:
-cmd = 'strip $SOURCE -o $TARGET'
-env.Program(unstripped, objs)
-return env.Command(stripped, unstripped,
- MakeAction(cmd, Transform("STRIP")))
+executable = env.Program(self.path(env), objs)[0]
+
+if sys.platform == 'sunos5':
+cmd = 'cp $SOURCE $TARGET; strip $TARGET'
else:
-return env.Program(self.path(env), objs)
+cmd = 'strip $SOURCE -o $TARGET'
+stripped = env.Command(str(executable) + '.stripped',
+executable, MakeAction(cmd, Transform("STRIP")))[0]
+
+return [executable, stripped]
class GTest(Executable):
'''Create a unit test based on the google test framework.'''
@@ -1351,7 +1349,7 @@
# environment 'env' with modified object suffix and optional stripped
# binary. Additional keyword arguments are appended to corresponding
# build environment vars.
-def makeEnv(env, label, objsfx, strip=False, **kwargs):
+def makeEnv(env, label, objsfx, **kwargs):
# SCons doesn't know to append a library suffix when there is a '.' in
the
# name. Use '_' instead.
libname = 'gem5_' + label
@@ -1397,7 +1395,6 @@
# Record some settings for building Executables.
new_env['EXE_SUFFIX'] = label
-new_env['STRIP_EXES'] = strip
for cls in ExecutableMeta.all:
cls.declare_all(new_env)
@@ -1476,7 +1473,7 @@
# "Fast" binary
if 'fast' in needed_envs:
-makeEnv(env, 'fast', '.fo', strip = True,
+makeEnv(env, 'fast', '.fo',
CCFLAGS = Split(ccflags['fast']),
CPPDEFINES = ['NDEBUG', 'TRACING_ON=0'],
LINKFLAGS = Split(ldflags['fast']))
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48120
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: I2de82e0951d9f41c30594f32fba50acdd14ed69c
Gerrit-Change-Number: 48120
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]: scons: Stop providing an "m5" hard link to the gem5 binary.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48119 )
Change subject: scons: Stop providing an "m5" hard link to the gem5 binary.
..
scons: Stop providing an "m5" hard link to the gem5 binary.
gem5 has been called gem5 for a long time, and the m5 binary has not
properly existed for a long time as well. Users have had a very long
time to move to the new binary name, so it should be safe to remove this
bit of legacy cruft.
Change-Id: I8a8ac14f29d25d48afa9db0d906ed4056ac8e961
---
M src/SConscript
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/SConscript b/src/SConscript
index d5a5b88..6a630da 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -1345,7 +1345,7 @@
# List of constructed environments to pass back to SConstruct
date_source = Source('base/date.cc', tags=[])
-gem5_binary = Gem5('gem5')
+Gem5('gem5')
# Function to create a new build environment as clone of current
# environment 'env' with modified object suffix and optional stripped
@@ -1355,7 +1355,6 @@
# SCons doesn't know to append a library suffix when there is a '.' in
the
# name. Use '_' instead.
libname = 'gem5_' + label
-secondary_exename = 'm5.' + label
new_env = env.Clone(OBJSUFFIX=objsfx, SHOBJSUFFIX=objsfx + 's')
new_env.Label = label
@@ -1403,11 +1402,6 @@
for cls in ExecutableMeta.all:
cls.declare_all(new_env)
-new_env.M5Binary = File(gem5_binary.path(new_env))
-
-new_env.Command(secondary_exename, new_env.M5Binary,
-MakeAction('ln $SOURCE $TARGET', Transform("HARDLINK")))
-
# Start out with the compiler flags common to all compilers,
# i.e. they all use -g for opt and -g -pg for prof
ccflags = {'debug' : [], 'opt' : ['-g'], 'fast' : [], 'prof' :
['-g', '-pg'],
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48119
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: I8a8ac14f29d25d48afa9db0d906ed4056ac8e961
Gerrit-Change-Number: 48119
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]: scons: Use Dir().Dir() and not os.path to extend CPPPATH.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/48127 ) Change subject: scons: Use Dir().Dir() and not os.path to extend CPPPATH. .. scons: Use Dir().Dir() and not os.path to extend CPPPATH. Since we're already working with Dir nodes, they can figure out appending to a path themselves without using os.path.join. Change-Id: Ib46946d7ec181dbbf443f957f23196eb0fd7f6b5 --- M src/SConscript 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/SConscript b/src/SConscript index a323828..13e8a2f 100644 --- a/src/SConscript +++ b/src/SConscript @@ -694,8 +694,7 @@ # Also add the corresponding build directory to pick up generated # include files. -env.Append(CPPPATH=Dir(os.path.join(env['BUILDDIR'], -extra_dir[prefix_len:]))) +env.Append(CPPPATH=Dir(env['BUILDDIR']).Dir(extra_dir[prefix_len:])) for root, dirs, files in os.walk(extra_dir, topdown=True): # if build lives in the extras directory, don't walk down it -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48127 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: Ib46946d7ec181dbbf443f957f23196eb0fd7f6b5 Gerrit-Change-Number: 48127 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]: scons: Get rid of the unused "dirname" property of SourceFile.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/48121 ) Change subject: scons: Get rid of the unused "dirname" property of SourceFile. .. scons: Get rid of the unused "dirname" property of SourceFile. Change-Id: I7c52f866542057b9f11ba96434f9c6f93ff0ea46 --- M src/SConscript 1 file changed, 0 insertions(+), 4 deletions(-) diff --git a/src/SConscript b/src/SConscript index a9d70bf..354b528 100644 --- a/src/SConscript +++ b/src/SConscript @@ -278,10 +278,6 @@ return str(self.tnode) @property -def dirname(self): -return dirname(self.filename) - -@property def basename(self): return basename(self.filename) -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48121 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: I7c52f866542057b9f11ba96434f9c6f93ff0ea46 Gerrit-Change-Number: 48121 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]: scons: Replace the extname property with os.path.splitext().
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48122 )
Change subject: scons: Replace the extname property with os.path.splitext().
..
scons: Replace the extname property with os.path.splitext().
This is almost exactly the same, except it leaves the "." on the
extension, and returns an empty string instead of None if there is no
extension.
Change-Id: Idb540771007f9f7ca8aafdb09512eb1219010237
---
M src/SConscript
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/src/SConscript b/src/SConscript
index 354b528..8c36374 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -43,6 +43,7 @@
import functools
import imp
import os
+import os.path
import re
import sys
import zlib
@@ -281,15 +282,6 @@
def basename(self):
return basename(self.filename)
-@property
-def extname(self):
-index = self.basename.rfind('.')
-if index <= 0:
-# dot files aren't extensions
-return self.basename, None
-
-return self.basename[:index], self.basename[index+1:]
-
def __lt__(self, other): return self.filename < other.filename
def __le__(self, other): return self.filename <= other.filename
def __gt__(self, other): return self.filename > other.filename
@@ -386,8 +378,8 @@
'''specify the python package, the source file, and any tags'''
super(PySource, self).__init__(source, tags, add_tags)
-modname,ext = self.extname
-assert ext == 'py'
+modname, ext = os.path.splitext(self.basename)
+assert ext == '.py'
if package:
path = package.split('.')
@@ -472,8 +464,8 @@
error('Got protobuf to build, but lacks support!')
# Get the file name and the extension
-modname,ext = self.extname
-assert ext == 'proto'
+modname, ext = os.path.splitext(self.basename)
+assert ext == '.proto'
self.cc_file, self.hh_file = env.ProtoBufCC(source=source)
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48122
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: Idb540771007f9f7ca8aafdb09512eb1219010237
Gerrit-Change-Number: 48122
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]: scons: Delete the comparison operators from SourceFile.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/48125 ) Change subject: scons: Delete the comparison operators from SourceFile. .. scons: Delete the comparison operators from SourceFile. These are apparently not used and can be deleted. Change-Id: I201d565d2e0207e0f43e9443ec03c2b695ab --- M src/SConscript 1 file changed, 0 insertions(+), 7 deletions(-) diff --git a/src/SConscript b/src/SConscript index 9e5471d..f20a0d6 100644 --- a/src/SConscript +++ b/src/SConscript @@ -275,13 +275,6 @@ self.shared_objs[key] = env.SharedObject(self.tnode) return self.shared_objs[key] -def __lt__(self, other): return self.filename < other.filename -def __le__(self, other): return self.filename <= other.filename -def __gt__(self, other): return self.filename > other.filename -def __ge__(self, other): return self.filename >= other.filename -def __eq__(self, other): return self.filename == other.filename -def __ne__(self, other): return self.filename != other.filename - def blobToCpp(data, symbol, cpp_code, hpp_code=None, namespace=None): ''' Convert bytes data into C++ .cpp and .hh uint8_t byte array -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48125 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: I201d565d2e0207e0f43e9443ec03c2b695ab Gerrit-Change-Number: 48125 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]: scons: Eliminate the SourceFile.basename property.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48124 )
Change subject: scons: Eliminate the SourceFile.basename property.
..
scons: Eliminate the SourceFile.basename property.
This value is used in only two places, and can be calculated in place to
avoid complexity.
Change-Id: I1e59b92521250b3f5a3e2cba599236ededf1761d
---
M src/SConscript
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/SConscript b/src/SConscript
index 3173a30..9e5471d 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -48,7 +48,7 @@
import sys
import zlib
-from os.path import basename, dirname, exists, isdir, isfile, join as
joinpath
+from os.path import dirname, exists, isdir, isfile, join as joinpath
import SCons
@@ -275,10 +275,6 @@
self.shared_objs[key] = env.SharedObject(self.tnode)
return self.shared_objs[key]
-@property
-def basename(self):
-return basename(self.filename)
-
def __lt__(self, other): return self.filename < other.filename
def __le__(self, other): return self.filename <= other.filename
def __gt__(self, other): return self.filename > other.filename
@@ -375,7 +371,8 @@
'''specify the python package, the source file, and any tags'''
super(PySource, self).__init__(source, tags, add_tags)
-modname, ext = os.path.splitext(self.basename)
+basename = os.path.basename(self.filename)
+modname, ext = os.path.splitext(basename)
assert ext == '.py'
if package:
@@ -388,7 +385,7 @@
modpath += [ modname ]
modpath = '.'.join(modpath)
-arcpath = path + [ self.basename ]
+arcpath = path + [ basename ]
abspath = self.snode.abspath
if not exists(abspath):
abspath = self.tnode.abspath
@@ -461,7 +458,8 @@
error('Got protobuf to build, but lacks support!')
# Get the file name and the extension
-modname, ext = os.path.splitext(self.basename)
+basename = os.path.basename(self.filename)
+modname, ext = os.path.splitext(basename)
assert ext == '.proto'
self.cc_file, self.hh_file = env.ProtoBufCC(source=source)
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48124
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: I1e59b92521250b3f5a3e2cba599236ededf1761d
Gerrit-Change-Number: 48124
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]: scons: Replace the SourceFile.filename property with attribute.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/48123 ) Change subject: scons: Replace the SourceFile.filename property with attribute. .. scons: Replace the SourceFile.filename property with attribute. The SourceFile.filename property dynamically calculated the str() conversion of self.tnode. Since self.tnode shouldn't be changed, it doesn't seem useful to calculate that value over and over, especially since it adds some extra indirection and magic to something that's really pretty simple. Change-Id: Ia0e1e8f4b0c019a026a08b5c2730d93c66de8190 --- M src/SConscript 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/SConscript b/src/SConscript index 8c36374..3173a30 100644 --- a/src/SConscript +++ b/src/SConscript @@ -250,6 +250,7 @@ tnode = File(source) self.tnode = tnode +self.filename = str(self.tnode) self.snode = tnode.srcnode() for base in type(self).__mro__: @@ -275,10 +276,6 @@ return self.shared_objs[key] @property -def filename(self): -return str(self.tnode) - -@property def basename(self): return basename(self.filename) -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48123 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: Ia0e1e8f4b0c019a026a08b5c2730d93c66de8190 Gerrit-Change-Number: 48123 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]: scons: Use the os.path prefix when using components of that module.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48126 )
Change subject: scons: Use the os.path prefix when using components of that
module.
..
scons: Use the os.path prefix when using components of that module.
That makes it obvious where the methods involved are coming from. Also
some of the imported names weren't being used.
Change-Id: I6ec75eef1e5ea9eae51e7df675e477dccb351bd1
---
M src/SConscript
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/src/SConscript b/src/SConscript
index f20a0d6..a323828 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -48,8 +48,6 @@
import sys
import zlib
-from os.path import dirname, exists, isdir, isfile, join as joinpath
-
import SCons
from gem5_scons import Transform, warning, error
@@ -327,7 +325,7 @@
'''
blob_path = os.path.abspath(blob_path)
blob_out_dir = os.path.join(env['BUILDDIR'], 'blobs')
-path_noext = joinpath(blob_out_dir, symbol)
+path_noext = os.path.join(blob_out_dir, symbol)
cpp_path = path_noext + '.cc'
hpp_path = path_noext + '.hh'
def embedBlob(target, source, env):
@@ -348,7 +346,7 @@
Source(cpp_path)
def GdbXml(xml_id, symbol):
-Blob(joinpath(gdb_xml_dir, xml_id), symbol)
+Blob(os.path.join(gdb_xml_dir, xml_id), symbol)
class Source(SourceFile):
pass
@@ -380,13 +378,13 @@
arcpath = path + [ basename ]
abspath = self.snode.abspath
-if not exists(abspath):
+if not os.path.exists(abspath):
abspath = self.tnode.abspath
self.package = package
self.modname = modname
self.modpath = modpath
-self.arcname = joinpath(*arcpath)
+self.arcname = os.path.join(*arcpath)
self.abspath = abspath
self.compiled = File(self.filename + 'c')
self.cpp = File(self.filename + '.cc')
@@ -688,15 +686,16 @@
continue
if 'SConscript' in files:
-build_dir = joinpath(env['BUILDDIR'], root[len(base_dir) + 1:])
-SConscript(joinpath(root, 'SConscript'), variant_dir=build_dir)
+build_dir = os.path.join(env['BUILDDIR'], root[len(base_dir) + 1:])
+SConscript(os.path.join(root, 'SConscript'), variant_dir=build_dir)
for extra_dir in extras_dir_list:
-prefix_len = len(dirname(extra_dir)) + 1
+prefix_len = len(os.path.dirname(extra_dir)) + 1
# Also add the corresponding build directory to pick up generated
# include files.
-env.Append(CPPPATH=Dir(joinpath(env['BUILDDIR'],
extra_dir[prefix_len:])))
+env.Append(CPPPATH=Dir(os.path.join(env['BUILDDIR'],
+extra_dir[prefix_len:])))
for root, dirs, files in os.walk(extra_dir, topdown=True):
# if build lives in the extras directory, don't walk down it
@@ -704,8 +703,8 @@
dirs.remove('build')
if 'SConscript' in files:
-build_dir = joinpath(env['BUILDDIR'], root[prefix_len:])
-SConscript(joinpath(root, 'SConscript'), variant_dir=build_dir)
+build_dir = os.path.join(env['BUILDDIR'], root[prefix_len:])
+SConscript(os.path.join(root, 'SConscript'),
variant_dir=build_dir)
for opt in export_vars:
env.ConfigFile(opt)
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48126
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: I6ec75eef1e5ea9eae51e7df675e477dccb351bd1
Gerrit-Change-Number: 48126
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]: scons: Remove the unused env.Label assignment in makeEnv.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/48128 ) Change subject: scons: Remove the unused env.Label assignment in makeEnv. .. scons: Remove the unused env.Label assignment in makeEnv. Change-Id: I33fe01bb0381061528c450bc5e8312b52882615e --- M src/SConscript 1 file changed, 0 insertions(+), 1 deletion(-) diff --git a/src/SConscript b/src/SConscript index 13e8a2f..7b62724 100644 --- a/src/SConscript +++ b/src/SConscript @@ -1329,7 +1329,6 @@ libname = 'gem5_' + label new_env = env.Clone(OBJSUFFIX=objsfx, SHOBJSUFFIX=objsfx + 's') -new_env.Label = label new_env.Append(**kwargs) lib_sources = Source.all.with_tag(new_env, 'gem5 lib') -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48128 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: I33fe01bb0381061528c450bc5e8312b52882615e Gerrit-Change-Number: 48128 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[release-staging-v21-1]: cpu: remove O3 dependency of CheckerCPU
Hoa Nguyen has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/48079 ) Change subject: cpu: remove O3 dependency of CheckerCPU .. cpu: remove O3 dependency of CheckerCPU Currently, compiling CheckerCPU uses the dyn_inst.hh header from O3CPU. However, including this header is not required and it causes gem5 failed to build when O3CPU is not part of CPU_MODELS. This change also involves moving the the dependency on src/cpu/o3/dyn_inst.hh to src/cpu/o3/cpu.cc and src/cpu/lsq_unit.cc, which previously includes src/cpu/o3/dyn_inst.hh implicitly through src/cpu/checker/cpu.hh. JIRA: https://gem5.atlassian.net/browse/GEM5-1025 Change-Id: I7664cd4b9591bf0a4635338fff576cb5f5cbfa10 Signed-off-by: Hoa Nguyen Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48079 Reviewed-by: Jason Lowe-Power Maintainer: Jason Lowe-Power Tested-by: kokoro --- M src/cpu/checker/cpu.hh M src/cpu/o3/cpu.cc M src/cpu/o3/lsq_unit.cc 3 files changed, 2 insertions(+), 1 deletion(-) Approvals: Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/cpu/checker/cpu.hh b/src/cpu/checker/cpu.hh index aebf522..a191ae4 100644 --- a/src/cpu/checker/cpu.hh +++ b/src/cpu/checker/cpu.hh @@ -51,7 +51,6 @@ #include "cpu/base.hh" #include "cpu/exec_context.hh" #include "cpu/inst_res.hh" -#include "cpu/o3/dyn_inst.hh" #include "cpu/pc_event.hh" #include "cpu/simple_thread.hh" #include "cpu/static_inst.hh" diff --git a/src/cpu/o3/cpu.cc b/src/cpu/o3/cpu.cc index fbdfcbd..e352236 100644 --- a/src/cpu/o3/cpu.cc +++ b/src/cpu/o3/cpu.cc @@ -46,6 +46,7 @@ #include "cpu/activity.hh" #include "cpu/checker/cpu.hh" #include "cpu/checker/thread_context.hh" +#include "cpu/o3/dyn_inst.hh" #include "cpu/o3/limits.hh" #include "cpu/o3/thread_context.hh" #include "cpu/simple_thread.hh" diff --git a/src/cpu/o3/lsq_unit.cc b/src/cpu/o3/lsq_unit.cc index 5394e4f..039184d 100644 --- a/src/cpu/o3/lsq_unit.cc +++ b/src/cpu/o3/lsq_unit.cc @@ -46,6 +46,7 @@ #include "base/str.hh" #include "config/the_isa.hh" #include "cpu/checker/cpu.hh" +#include "cpu/o3/dyn_inst.hh" #include "cpu/o3/limits.hh" #include "cpu/o3/lsq.hh" #include "debug/Activity.hh" -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48079 To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem5 Gerrit-Branch: release-staging-v21-1 Gerrit-Change-Id: I7664cd4b9591bf0a4635338fff576cb5f5cbfa10 Gerrit-Change-Number: 48079 Gerrit-PatchSet: 4 Gerrit-Owner: Hoa Nguyen Gerrit-Reviewer: Bobby R. Bruce Gerrit-Reviewer: Gabe Black Gerrit-Reviewer: Hoa Nguyen 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
