[gem5-dev] Re: deprecating warn_once
Hi Gabe,
Is there a use case for GEM5_ONCE() other than warn_once()?
Thanks,
Steve
On Mon, Jul 26, 2021 at 6:06 PM Gabe Black via gem5-dev
wrote:
> Hi folks. I'm continuing to research how to turn warn, panic, etc, into
> regular functions instead of macros, and one particularly sticky problem is
> how to ensure something like warn_once only happens once.
>
> Right now, the macro warn_once expands to something like this:
>
> do {
> static bool once = false;
> if (!once) {
> warn(...);
> once = true;
> }
> } while (0)
>
> So, a static bool is declared which guards the warn. The problem with this
> is that it requires declaring a static bool at the call sight, which as you
> can imagine is hard to do without macros.
>
> As far as how it *might* be done, if we can extract the location of the
> call (file name, source line, column offset), then we could possibly use a
> template holding a static bool.
>
> template <${source location info}>
> warn_once(...)
> {
> static bool once = false;
>
> }
>
> There are a few problems with this approach. First, the source location
> would have to be broken down into individual primitive types, like an int
> for the line number, and individual characters for the file name string,
> since you can't use a struct as a non-type template parameter until c++20.
> This *might* be possible using fancy template tricks, but it would be a bit
> ugly and may gum up the compiler, slowing builds.
>
> Second, if the column information is not unique (I think the standard is
> not very specific about what it maps to), then the "once" will apply to
> more than one thing. This would be particularly true if a macro whose
> contents all share the same source location had multiple warn_once-s in it.
>
> I did a check with grep, and warn_once shows up in all of gem5 about 80
> times, so while it's used, it's not used extensively.
>
> What I would like to propose is that instead of having warn_once(...), we
> add a new macro called GEM5_ONCE which would be defined something like the
> following:
>
> #define GEM5_ONCE(statement) do { \
> static [[maybe_unused]] bool _once = ([](){ statement; }(), true); \
> while (0)
>
> Then when you want to warn once (or anything else once), you'd write it
> like this:
>
> GEM5_ONCE(warn("blah blah"));
>
> This is *slightly* more verbose, but warn_once is only used 80 times in
> the whole code base. Also the macro is namespaced now, which is a nice
> improvement.
>
> Gabe
> ___
> gem5-dev mailing list -- gem5-dev@gem5.org
> To unsubscribe send an email to gem5-dev-le...@gem5.org
> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: base: Extend gem5_assert to subsume chatty_assert.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48606 )
Change subject: base: Extend gem5_assert to subsume chatty_assert.
..
base: Extend gem5_assert to subsume chatty_assert.
If given more than just its condition, gem5_assert will assume it
should act like chatty_assert.
Because we have our own custom assert now anyway, we can fold the
behavior of both into one macro and make life easier for users.
Deprecate chatty_assert.
Change-Id: I43497b5333802a265c0ad096681f64ab6f0424b1
---
M src/base/logging.hh
M src/base/logging.test.cc
2 files changed, 32 insertions(+), 33 deletions(-)
diff --git a/src/base/logging.hh b/src/base/logging.hh
index 758a9cb..88e1109 100644
--- a/src/base/logging.hh
+++ b/src/base/logging.hh
@@ -43,6 +43,7 @@
#include
#include
+#include
#include
#include "base/compiler.hh"
@@ -288,26 +289,20 @@
#define NDEBUG_DEFINED 0
#endif
-/**
- * The chatty assert macro will function like a normal assert, but will
allow
- * the specification of additional, helpful material to aid debugging why
the
- * assertion actually failed. chatty_assert will not actually check its
- * condition for fast builds, but the condition must still be valid code.
- *
- * @param cond Condition that is checked; if false -> assert
- * @param ... Printf-based format string with arguments, extends printout.
- *
- * \def chatty_assert(cond, ...)
- *
- * @ingroup api_logger
- */
-#define chatty_assert(cond, ...) \
-do { \
-if (GEM5_UNLIKELY(!NDEBUG_DEFINED && !static_cast(cond))) \
-panic("assert(" # cond ") failed: %s", \
-::gem5::csprintf(__VA_ARGS__)); \
-} while (0)
-/** @} */ // end of api_logger
+template
+inline const std::string
+gem5AssertMessage(const std::string , const std::tuple )
+{
+return cond + ": " + std::apply(
+&::gem5::csprintf, args);
+}
+
+template <>
+inline const std::string
+gem5AssertMessage<>(const std::string , const std::tuple<> )
+{
+return cond;
+}
/**
* The assert macro will function like a normal assert, but will use panic
@@ -316,17 +311,26 @@
* condition in fast builds, but it must still be valid code.
*
* @param cond Condition that is checked; if false -> panic
+ * @param ... Printf-based format string with arguments, extends printout.
*
- * \def gem5_assert(cond)
+ * \def gem5_assert(cond, ...)
*
* @ingroup api_logger
*/
-#define gem5_assert(cond) \
+#define gem5_assert(cond, ...) \
do { \
-if (GEM5_UNLIKELY(!NDEBUG_DEFINED && !static_cast(cond))) \
-panic("assert(" # cond ") failed"); \
+if (GEM5_UNLIKELY(!NDEBUG_DEFINED && !static_cast(cond))) { \
+panic(::gem5::gem5AssertMessage("assert(" #cond ") failed", \
+std::make_tuple(__VA_ARGS__))); \
+} \
} while (0)
/** @} */ // end of api_logger
+#define chatty_assert(...) \
+do { \
+gem5_assert(__VA_ARGS__); \
+GEM5_DEPRECATED_MACRO(chatty_assert, {}, "Please use
gem5_assert()"); \
+} while(0)
+
} // namespace gem5
#endif // __BASE_LOGGING_HH__
diff --git a/src/base/logging.test.cc b/src/base/logging.test.cc
index 0cbe3d0..fb201b6 100644
--- a/src/base/logging.test.cc
+++ b/src/base/logging.test.cc
@@ -543,21 +543,16 @@
"fatal: fatal condition true occurred: message\nMemory Usage:"));
}
-/** Test macro chatty_assert. */
-TEST(LoggingDeathTest, ChattyAssert)
+/** Test macro gem5_assert. */
+TEST(LoggingDeathTest, Gem5Assert)
{
#ifdef NDEBUG
GTEST_SKIP() << "Skipping as assertions are "
"stripped out of fast builds";
#endif
-chatty_assert(true, "message\n");
-ASSERT_DEATH(chatty_assert(false, "message\n"), ::testing::HasSubstr(
+gem5_assert(true, "message\n");
+ASSERT_DEATH(gem5_assert(false, "message\n"), ::testing::HasSubstr(
"panic: assert(false) failed: message\nMemory Usage:"));
-}
-
-/** Test macro gem5_assert. */
-TEST(LoggingDeathTest, GEM5Assert)
-{
gem5_assert(true);
ASSERT_DEATH(gem5_assert(false), ::testing::HasSubstr(
"panic: assert(false) failed\nMemory Usage:"));
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48606
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: I43497b5333802a265c0ad096681f64ab6f0424b1
Gerrit-Change-Number: 48606
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: base: Always compile the condition of chatty_assert.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48605 )
Change subject: base: Always compile the condition of chatty_assert.
..
base: Always compile the condition of chatty_assert.
The condition must always be valid code and will always exist to
satisfy the compiler as far as what variables are used, etc, but it
will only actually be evaluated if NDEBUG is not set.
Change-Id: Ia5a6273c95f2e7bf1b7443751fed38c62e73b351
---
M src/base/logging.hh
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/src/base/logging.hh b/src/base/logging.hh
index 56396f8..2dd5aad 100644
--- a/src/base/logging.hh
+++ b/src/base/logging.hh
@@ -282,11 +282,17 @@
} while (0)
/** @} */ // end of api_logger
+#ifdef NDEBUG
+#define NDEBUG_DEFINED 1
+#else
+#define NDEBUG_DEFINED 0
+#endif
+
/**
* The chatty assert macro will function like a normal assert, but will
allow
* the specification of additional, helpful material to aid debugging why
the
- * assertion actually failed. Like the normal assertion, the chatty_assert
- * will not be active in fast builds.
+ * assertion actually failed. chatty_assert will not actually check its
+ * condition for fast builds, but the condition must still be valid code.
*
* @param cond Condition that is checked; if false -> assert
* @param ... Printf-based format string with arguments, extends printout.
@@ -295,16 +301,12 @@
*
* @ingroup api_logger
*/
-#ifdef NDEBUG
-#define chatty_assert(cond, ...)
-#else //!NDEBUG
-#define chatty_assert(cond, ...)\
-do {\
-if (GEM5_UNLIKELY(!(cond))) \
+#define chatty_assert(cond, ...) \
+do { \
+if (GEM5_UNLIKELY(!NDEBUG_DEFINED && !static_cast(cond))) \
panic("assert(" # cond ") failed: %s", \
-::gem5::csprintf(__VA_ARGS__)); \
+::gem5::csprintf(__VA_ARGS__)); \
} while (0)
-#endif // NDEBUG
/** @} */ // end of api_logger
} // namespace gem5
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48605
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: Ia5a6273c95f2e7bf1b7443751fed38c62e73b351
Gerrit-Change-Number: 48605
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]: mem,sim: Replace the deprecated chatty_assert with gem5_assert.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48607 )
Change subject: mem,sim: Replace the deprecated chatty_assert with
gem5_assert.
..
mem,sim: Replace the deprecated chatty_assert with gem5_assert.
The new gem5_assert macro now does the job of both regular asserts, and
chatty_assert, except that its condition must always be valid code. It
is still not evaluated if NDEBUG is set, though.
Change-Id: I7c9435311746b2e02fe7335bce6ba618bf9fd4eb
---
M src/mem/cache/base.cc
M src/mem/cache/cache.cc
M src/mem/mem_checker.cc
M src/sim/voltage_domain.cc
M src/sim/voltage_domain.hh
5 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc
index 5f7d81b..4e532e0 100644
--- a/src/mem/cache/base.cc
+++ b/src/mem/cache/base.cc
@@ -1156,9 +1156,9 @@
// sanity check
assert(pkt->isRequest());
-chatty_assert(!(isReadOnly && pkt->isWrite()),
- "Should never see a write in a read-only cache %s\n",
- name());
+gem5_assert(!(isReadOnly && pkt->isWrite()),
+"Should never see a write in a read-only cache %s\n",
+name());
// Access block in the tags
Cycles tag_latency(0);
@@ -1501,8 +1501,8 @@
// owners copy
blk->setCoherenceBits(CacheBlk::DirtyBit);
-chatty_assert(!isReadOnly, "Should never see dirty snoop
response "
- "in read-only cache %s\n", name());
+gem5_assert(!isReadOnly, "Should never see dirty snoop
response "
+"in read-only cache %s\n", name());
}
}
@@ -1615,8 +1615,8 @@
PacketPtr
BaseCache::writebackBlk(CacheBlk *blk)
{
-chatty_assert(!isReadOnly || writebackClean,
- "Writeback from read-only cache");
+gem5_assert(!isReadOnly || writebackClean,
+"Writeback from read-only cache");
assert(blk && blk->isValid() &&
(blk->isSet(CacheBlk::DirtyBit) || writebackClean));
diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index 7ea8a8c..29e52db 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -165,9 +165,9 @@
if (pkt->req->isUncacheable()) {
assert(pkt->isRequest());
-chatty_assert(!(isReadOnly && pkt->isWrite()),
- "Should never see a write in a read-only cache %s\n",
- name());
+gem5_assert(!(isReadOnly && pkt->isWrite()),
+"Should never see a write in a read-only cache %s\n",
+name());
DPRINTF(Cache, "%s for %s\n", __func__, pkt->print());
@@ -1104,7 +1104,7 @@
// xbar.
respond = blk->isSet(CacheBlk::DirtyBit) && pkt->needsResponse();
-chatty_assert(!(isReadOnly && blk->isSet(CacheBlk::DirtyBit)),
+gem5_assert(!(isReadOnly && blk->isSet(CacheBlk::DirtyBit)),
"Should never have a dirty block in a read-only cache %s\n",
name());
}
diff --git a/src/mem/mem_checker.cc b/src/mem/mem_checker.cc
index d8a3ee9..b331c84 100644
--- a/src/mem/mem_checker.cc
+++ b/src/mem/mem_checker.cc
@@ -53,7 +53,7 @@
// Initialize a fresh write cluster
start = _start;
}
-chatty_assert(start <= _start, "WriteClusters must filled in order!");
+gem5_assert(start <= _start, "WriteClusters must filled in order!");
++numIncomplete;
diff --git a/src/sim/voltage_domain.cc b/src/sim/voltage_domain.cc
index b8debaa..297c23f 100644
--- a/src/sim/voltage_domain.cc
+++ b/src/sim/voltage_domain.cc
@@ -64,9 +64,9 @@
void
VoltageDomain::perfLevel(PerfLevel perf_level)
{
-chatty_assert(perf_level < voltageOpPoints.size(),
- "DVFS: Requested voltage ID %d is outside the known "\
- "range for domain %s.\n", perf_level, name());
+gem5_assert(perf_level < voltageOpPoints.size(),
+"DVFS: Requested voltage ID %d is outside the known "\
+"range for domain %s.\n", perf_level, name());
if (perf_level == _perfLevel) {
// Silently ignore identical overwrites
@@ -88,11 +88,12 @@
// Find the highest requested performance level and update the voltage
// domain with it
PerfLevel perf_max = (PerfLevel)-1;
-for (auto dit = srcClockChildren.begin(); dit !=
srcClockChildren.end(); ++dit) {
+for (auto dit = srcClockChildren.begin(); dit !=
srcClockChildren.end();
+++dit) {
SrcClockDomain* d = *dit;
-chatty_assert(d->voltageDomain() == this, "DVFS: Clock domain %s "\
- "(id: %d) should not be registered with voltage
domain "\
- "%s\n", d->name(), d->domainID(), name());
+gem5_assert(d->voltageDomain() == this, "DVFS: Clock domain %s "\
+"(id: %d) should
[gem5-dev] Change in gem5/gem5[develop]: base: socket: Add SOCK_CLOEXEC
Nicolas Boichat has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48623 )
Change subject: base: socket: Add SOCK_CLOEXEC
..
base: socket: Add SOCK_CLOEXEC
Prevents any forked process (e.g. diod) from holding on the sockets
if gem5 crashes.
This flag is only supported on Linux, so we stub it out on other
platforms.
Bug: 194336737
Test: ls -l /proc/$DIOD_PID/fd shows a lot less fds.
Change-Id: I6bc195ed3bd638ab28ff7690f43afce030fa28c7
---
M src/base/socket.cc
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/base/socket.cc b/src/base/socket.cc
index fa61ea4..3510737 100644
--- a/src/base/socket.cc
+++ b/src/base/socket.cc
@@ -43,9 +43,23 @@
#include "base/types.hh"
#include "sim/byteswap.hh"
+// Stub out GNU/Linux extensions for SOCK_CLOEXEC.
+#ifndef _GNU_SOURCE
+static inline int
+accept4(int sockfd, struct sockaddr *addr, socklen_t *addrlen, int flags)
+{
+if (flags)
+panic("accept4 stub only accepts flags == 0.");
+return accept(sockfd, addr, addrlen);
+}
+#endif
+
+#ifndef SOCK_CLOEXEC
+#define SOCK_CLOEXEC 0
+#endif
+
namespace gem5
{
-
bool ListenSocket::listeningDisabled = false;
bool ListenSocket::anyListening = false;
@@ -104,7 +118,7 @@
// only create socket if not already created by a previous call
if (fd == -1) {
-fd = ::socket(PF_INET, SOCK_STREAM, 0);
+fd = ::socket(PF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
if (fd < 0)
panic("Can't create socket:%s !", strerror(errno));
}
@@ -150,7 +164,7 @@
{
struct sockaddr_in sockaddr;
socklen_t slen = sizeof (sockaddr);
-int sfd = ::accept(fd, (struct sockaddr *), );
+int sfd = ::accept4(fd, (struct sockaddr *), ,
SOCK_CLOEXEC);
if (sfd != -1 && nodelay) {
int i = 1;
if (::setsockopt(sfd, IPPROTO_TCP, TCP_NODELAY, (char *),
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48623
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: I6bc195ed3bd638ab28ff7690f43afce030fa28c7
Gerrit-Change-Number: 48623
Gerrit-PatchSet: 1
Gerrit-Owner: Nicolas Boichat
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]: fastmodel: add iris readMem and writeMem function
Yu-hsin Wang has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/45268 )
Change subject: fastmodel: add iris readMem and writeMem function
..
fastmodel: add iris readMem and writeMem function
Iris memory API allows us to access the memory inside the core, for
example the tightly coupled memory (TCM). If we access a memory address
which is not in the CPU, it also fire a request to memory system.
Change-Id: I5925214534a10e3a55b780c3d4ed06e7559aafe0
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/45268
Reviewed-by: Gabe Black
Maintainer: Gabe Black
Tested-by: kokoro
---
M src/arch/arm/fastmodel/iris/thread_context.cc
M src/arch/arm/fastmodel/iris/thread_context.hh
2 files changed, 25 insertions(+), 0 deletions(-)
Approvals:
Gabe Black: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/fastmodel/iris/thread_context.cc
b/src/arch/arm/fastmodel/iris/thread_context.cc
index e2b32f4..716ffad 100644
--- a/src/arch/arm/fastmodel/iris/thread_context.cc
+++ b/src/arch/arm/fastmodel/iris/thread_context.cc
@@ -39,11 +39,15 @@
#include "arch/arm/fastmodel/iris/thread_context.hh"
+#include
+#include
#include
+#include
#include "arch/arm/fastmodel/iris/cpu.hh"
#include "arch/arm/system.hh"
#include "arch/arm/utility.hh"
+#include "base/logging.hh"
#include "iris/detail/IrisCppAdapter.h"
#include "iris/detail/IrisObjects.h"
#include "mem/se_translating_port_proxy.hh"
@@ -420,6 +424,25 @@
return true;
}
+void
+ThreadContext::readMem(Addr addr, void *p, size_t size)
+{
+iris::r0master::MemoryReadResult r;
+auto err = call().memory_read(_instId, r, 0, addr, 1, size);
+panic_if(err != iris::r0master::E_ok, "readMem failed.");
+std::memcpy(p, r.data.data(), size);
+}
+
+void
+ThreadContext::writeMem(Addr addr, const void *p, size_t size)
+{
+std::vector data((size + 7) / 8);
+std::memcpy(data.data(), p, size);
+iris::MemoryWriteResult r;
+auto err = call().memory_write(_instId, r, 0, addr, 1, size, data);
+panic_if(err != iris::r0master::E_ok, "writeMem failed.");
+}
+
bool
ThreadContext::translateAddress(Addr , iris::MemorySpaceId p_space,
Addr vaddr, iris::MemorySpaceId v_space)
diff --git a/src/arch/arm/fastmodel/iris/thread_context.hh
b/src/arch/arm/fastmodel/iris/thread_context.hh
index eadd089..d16b480 100644
--- a/src/arch/arm/fastmodel/iris/thread_context.hh
+++ b/src/arch/arm/fastmodel/iris/thread_context.hh
@@ -162,6 +162,8 @@
iris::IrisCppAdapter () const { return client.irisCall(); }
iris::IrisCppAdapter () const { return
client.irisCallNoThrow(); }
+void readMem(Addr addr, void *p, size_t size);
+void writeMem(Addr addr, const void *p, size_t size);
bool translateAddress(Addr , iris::MemorySpaceId p_space,
Addr vaddr, iris::MemorySpaceId v_space);
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/45268
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: I5925214534a10e3a55b780c3d4ed06e7559aafe0
Gerrit-Change-Number: 45268
Gerrit-PatchSet: 10
Gerrit-Owner: Yu-hsin Wang
Gerrit-Reviewer: Earl Ou
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Philip Metzler
Gerrit-Reviewer: Yu-hsin Wang
Gerrit-Reviewer: kokoro
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] deprecating warn_once
Hi folks. I'm continuing to research how to turn warn, panic, etc, into
regular functions instead of macros, and one particularly sticky problem is
how to ensure something like warn_once only happens once.
Right now, the macro warn_once expands to something like this:
do {
static bool once = false;
if (!once) {
warn(...);
once = true;
}
} while (0)
So, a static bool is declared which guards the warn. The problem with this
is that it requires declaring a static bool at the call sight, which as you
can imagine is hard to do without macros.
As far as how it *might* be done, if we can extract the location of the
call (file name, source line, column offset), then we could possibly use a
template holding a static bool.
template <${source location info}>
warn_once(...)
{
static bool once = false;
}
There are a few problems with this approach. First, the source location
would have to be broken down into individual primitive types, like an int
for the line number, and individual characters for the file name string,
since you can't use a struct as a non-type template parameter until c++20.
This *might* be possible using fancy template tricks, but it would be a bit
ugly and may gum up the compiler, slowing builds.
Second, if the column information is not unique (I think the standard is
not very specific about what it maps to), then the "once" will apply to
more than one thing. This would be particularly true if a macro whose
contents all share the same source location had multiple warn_once-s in it.
I did a check with grep, and warn_once shows up in all of gem5 about 80
times, so while it's used, it's not used extensively.
What I would like to propose is that instead of having warn_once(...), we
add a new macro called GEM5_ONCE which would be defined something like the
following:
#define GEM5_ONCE(statement) do { \
static [[maybe_unused]] bool _once = ([](){ statement; }(), true); \
while (0)
Then when you want to warn once (or anything else once), you'd write it
like this:
GEM5_ONCE(warn("blah blah"));
This is *slightly* more verbose, but warn_once is only used 80 times in the
whole code base. Also the macro is namespaced now, which is a nice
improvement.
Gabe
___
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]: dev: Drop include of drm/drm.h in kfd_ioctl.h.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/48603 ) Change subject: dev: Drop include of drm/drm.h in kfd_ioctl.h. .. dev: Drop include of drm/drm.h in kfd_ioctl.h. I don't have this header on one of the machines I build on, so this is breaking the build for me. Removing this include seems to make the build succeed, implying that it's not actually necessary. I looked at the file it's used in and didn't see anything that looked like it came from this header file. Change-Id: If4a29063d6d0d25904183cab78c9713ff1f8daa6 --- M src/dev/hsa/kfd_ioctl.h 1 file changed, 0 insertions(+), 1 deletion(-) diff --git a/src/dev/hsa/kfd_ioctl.h b/src/dev/hsa/kfd_ioctl.h index e1286dc..c953787 100644 --- a/src/dev/hsa/kfd_ioctl.h +++ b/src/dev/hsa/kfd_ioctl.h @@ -24,7 +24,6 @@ #define KFD_IOCTL_H_INCLUDED #include -#include #include #include -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48603 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: If4a29063d6d0d25904183cab78c9713ff1f8daa6 Gerrit-Change-Number: 48603 Gerrit-PatchSet: 1 Gerrit-Owner: Gabe Black Gerrit-MessageType: newchange ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: arch, arch-arm: Make BaseMMU translate methods virtual
Giacomo Travaglini has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/48140 ) Change subject: arch, arch-arm: Make BaseMMU translate methods virtual .. arch, arch-arm: 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 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48140 Reviewed-by: Jason Lowe-Power Reviewed-by: Andreas Sandberg Maintainer: Jason Lowe-Power Maintainer: Andreas Sandberg Tested-by: kokoro --- M src/arch/arm/mmu.hh M src/arch/generic/mmu.hh 2 files changed, 14 insertions(+), 8 deletions(-) Approvals: Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved Andreas Sandberg: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/arch/arm/mmu.hh b/src/arch/arm/mmu.hh index a129831..14816d7 100644 --- a/src/arch/arm/mmu.hh +++ b/src/arch/arm/mmu.hh @@ -90,6 +90,7 @@ void init() override; +using BaseMMU::translateFunctional; bool translateFunctional(ThreadContext *tc, Addr vaddr, Addr ); Fault translateFunctional(const RequestPtr , ThreadContext *tc, @@ -103,6 +104,7 @@ Fault translateAtomic(const RequestPtr , ThreadContext *tc, BaseMMU::Mode mode, bool stage2); +using BaseMMU::translateTiming; void translateTiming(const RequestPtr , ThreadContext *tc, BaseMMU::Translation *translation, BaseMMU::Mode mode, bool stage2); 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: 3 Gerrit-Owner: Giacomo Travaglini Gerrit-Reviewer: Andreas Sandberg Gerrit-Reviewer: Gabe Black Gerrit-Reviewer: Giacomo Travaglini Gerrit-Reviewer: Jason Lowe-Power Gerrit-Reviewer: kokoro Gerrit-MessageType: merged ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: misc: Merge branch 'release-staging-v21-1' into develop
Bobby R. Bruce has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/48563 ) Change subject: misc: Merge branch 'release-staging-v21-1' into develop .. misc: Merge branch 'release-staging-v21-1' into develop Change-Id: I6ba57d7f70be70ae43fab396780d18623679a59a --- 1 file changed, 0 insertions(+), 0 deletions(-) Approvals: Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved Matt Sinclair: Looks good to me, approved kokoro: Regressions pass -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48563 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: I6ba57d7f70be70ae43fab396780d18623679a59a Gerrit-Change-Number: 48563 Gerrit-PatchSet: 1 Gerrit-Owner: Bobby R. Bruce Gerrit-Reviewer: Bobby R. Bruce Gerrit-Reviewer: Matt Sinclair Gerrit-Reviewer: kokoro Gerrit-MessageType: merged ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: util: Fix gerrit bot commit subject parser
Hoa Nguyen has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48586 )
Change subject: util: Fix gerrit bot commit subject parser
..
util: Fix gerrit bot commit subject parser
Currently, if the commit subject does have tags, the parser will
return the list of tags as a NoneType object, which will be
iterated later. This caused the gerrit bot to fail.
This change lets the parser return the list of tags as an empty
list rather than a NoneType object. Also, a commit subject without
a semicolon `:` will be assumed as having no tags and the whole
subject is the commit message.
Signed-off-by: Hoa Nguyen
Change-Id: Ie8c90e14bb85c4ce1c583121d02e75aa87db7811
---
M util/gerrit-bot/util.py
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/util/gerrit-bot/util.py b/util/gerrit-bot/util.py
index 31b9fcc..d836690 100644
--- a/util/gerrit-bot/util.py
+++ b/util/gerrit-bot/util.py
@@ -30,9 +30,10 @@
parsed_subject = subject.split(":", maxsplit = 1)
# If the subject does not have a colon, it either does not have tags
-# or does not have a message
+# or does not have a message. In this case, we assume that the subject
+# is the commit message.
if len(parsed_subject) <= 1:
-return None, None
+return [], parsed_subject[0]
tags = [ tag.strip() for tag in parsed_subject[0].split(",") ]
message = parsed_subject[1]
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48586
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: Ie8c90e14bb85c4ce1c583121d02e75aa87db7811
Gerrit-Change-Number: 48586
Gerrit-PatchSet: 1
Gerrit-Owner: Hoa Nguyen
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]: sim-se: Properly handle a clone with the VFORK flag
Matt Sinclair has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/48346 )
Change subject: sim-se: Properly handle a clone with the VFORK flag
..
sim-se: Properly handle a clone with the VFORK flag
When clone is called with the VFORK flag, the calling process is
suspended until the child process either exits, or calls execve.
This patch adds in a new variable to Process, which is used to store the
context of the calling process if this process is created through a
clone with VFORK set.
This patch also adds the required support in clone to suspend the
calling thread, and in exitImpl and execveFunc to wake up the calling
thread when the child thread calls either of those functions
Change-Id: I85af67544ea1d5df7102dcff1331b5a6f6f4fa7c
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48346
Tested-by: kokoro
Reviewed-by: Bobby R. Bruce
Reviewed-by: Matt Sinclair
Maintainer: Matt Sinclair
---
M src/sim/process.cc
M src/sim/process.hh
M src/sim/syscall_emul.cc
M src/sim/syscall_emul.hh
4 files changed, 34 insertions(+), 0 deletions(-)
Approvals:
Matt Sinclair: Looks good to me, but someone else must approve; Looks
good to me, approved
Bobby R. Bruce: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/sim/process.cc b/src/sim/process.cc
index 207c275..272fc9f 100644
--- a/src/sim/process.cc
+++ b/src/sim/process.cc
@@ -175,6 +175,9 @@
#ifndef CLONE_THREAD
#define CLONE_THREAD 0
#endif
+#ifndef CLONE_VFORK
+#define CLONE_VFORK 0
+#endif
if (CLONE_VM & flags) {
/**
* Share the process memory address space between the new process
@@ -249,6 +252,10 @@
np->exitGroup = exitGroup;
}
+if (CLONE_VFORK & flags) {
+np->vforkContexts.push_back(otc->contextId());
+}
+
np->argv.insert(np->argv.end(), argv.begin(), argv.end());
np->envp.insert(np->envp.end(), envp.begin(), envp.end());
}
diff --git a/src/sim/process.hh b/src/sim/process.hh
index 632ba90..34768a0 100644
--- a/src/sim/process.hh
+++ b/src/sim/process.hh
@@ -284,6 +284,9 @@
// Process was forked with SIGCHLD set.
bool *sigchld;
+// Contexts to wake up when this thread exits or calls execve
+std::vector vforkContexts;
+
// Track how many system calls are executed
statistics::Scalar numSyscalls;
};
diff --git a/src/sim/syscall_emul.cc b/src/sim/syscall_emul.cc
index 147cb39..713bec4 100644
--- a/src/sim/syscall_emul.cc
+++ b/src/sim/syscall_emul.cc
@@ -193,6 +193,16 @@
}
}
+/**
+ * If we were a thread created by a clone with vfork set, wake up
+ * the thread that created us
+ */
+if (!p->vforkContexts.empty()) {
+ThreadContext *vtc = sys->threads[p->vforkContexts.front()];
+assert(vtc->status() == ThreadContext::Suspended);
+vtc->activate();
+}
+
tc->halt();
/**
diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh
index 09be700..8695638 100644
--- a/src/sim/syscall_emul.hh
+++ b/src/sim/syscall_emul.hh
@@ -1521,6 +1521,10 @@
ctc->pcState(cpc);
ctc->activate();
+if (flags & OS::TGT_CLONE_VFORK) {
+tc->suspend();
+}
+
return cp->pid();
}
@@ -1998,6 +2002,16 @@
};
/**
+ * If we were a thread created by a clone with vfork set, wake up
+ * the thread that created us
+ */
+if (!p->vforkContexts.empty()) {
+ThreadContext *vtc = p->system->threads[p->vforkContexts.front()];
+assert(vtc->status() == ThreadContext::Suspended);
+vtc->activate();
+}
+
+/**
* Note that ProcessParams is generated by swig and there are no other
* examples of how to create anything but this default constructor. The
* fields are manually initialized instead of passing parameters to the
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48346
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: I85af67544ea1d5df7102dcff1331b5a6f6f4fa7c
Gerrit-Change-Number: 48346
Gerrit-PatchSet: 3
Gerrit-Owner: Kyle Roarty
Gerrit-Reviewer: Bobby R. Bruce
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: Matt Sinclair
Gerrit-Reviewer: Matthew Poremba
Gerrit-Reviewer: kokoro
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[release-staging-v21-1]: sim-se: Fix execve syscall
Matt Sinclair has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/48345 )
Change subject: sim-se: Fix execve syscall
..
sim-se: Fix execve syscall
There were three things preventing execve from working
Firstly, the entrypoint for the new program wasn't correct. This was
fixed by calling Process::init, which adds a bias to the entrypoint.
Secondly, the uname string wasn't being copied over. This meant when the
new executable tried to run, it would think the kernel was too old to
run on, and would error out. This was fixed by copying over the uname
string (the `release` string in Process) when creating the new process.
Additionally, this patch also ensures we copy over the uname string in
the clone implementation, as otherwise a cloned thread that called
execve would crash.
Finally, we choose to not delete the new ProcessParams or the old
Process. This is done both because it matches what is done in cloneFunc,
but also because deleting the old process results in a segfault later
on.
Change-Id: I4ca201da689e9e37671b4cb477dc76fa12eecf69
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48345
Reviewed-by: Matt Sinclair
Reviewed-by: Bobby R. Bruce
Maintainer: Matt Sinclair
Tested-by: kokoro
---
M src/sim/syscall_emul.hh
1 file changed, 6 insertions(+), 2 deletions(-)
Approvals:
Matt Sinclair: Looks good to me, but someone else must approve; Looks
good to me, approved
Bobby R. Bruce: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh
index aa02fd6..09be700 100644
--- a/src/sim/syscall_emul.hh
+++ b/src/sim/syscall_emul.hh
@@ -1452,6 +1452,7 @@
pp->euid = p->euid();
pp->gid = p->gid();
pp->egid = p->egid();
+pp->release = p->release;
/* Find the first free PID that's less than the maximum */
std::set const& pids = p->system->PIDs;
@@ -2017,6 +2018,7 @@
pp->errout.assign("cerr");
pp->cwd.assign(p->tgtCwd);
pp->system = p->system;
+pp->release = p->release;
/**
* Prevent process object creation with identical PIDs (which will trip
* a fatal check in Process constructor). The execve call is supposed
to
@@ -2027,7 +2029,9 @@
*/
p->system->PIDs.erase(p->pid());
Process *new_p = pp->create();
-delete pp;
+// TODO: there is no way to know when the Process SimObject is done
with
+// the params pointer. Both the params pointer (pp) and the process
+// pointer (p) are normally managed in python and are never cleaned up.
/**
* Work through the file descriptor array and close any files marked
@@ -2042,10 +2046,10 @@
*new_p->sigchld = true;
-delete p;
tc->clearArchRegs();
tc->setProcessPtr(new_p);
new_p->assignThreadContext(tc->contextId());
+new_p->init();
new_p->initState();
tc->activate();
TheISA::PCState pcState = tc->pcState();
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48345
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: I4ca201da689e9e37671b4cb477dc76fa12eecf69
Gerrit-Change-Number: 48345
Gerrit-PatchSet: 3
Gerrit-Owner: Kyle Roarty
Gerrit-Reviewer: Bobby R. Bruce
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: Matt Sinclair
Gerrit-Reviewer: Matthew Poremba
Gerrit-Reviewer: kokoro
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[release-staging-v21-1]: arch-gcn3: Validate if scalar sources are scalar gprs
Matt Sinclair has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/48344 )
Change subject: arch-gcn3: Validate if scalar sources are scalar gprs
..
arch-gcn3: Validate if scalar sources are scalar gprs
Scalar sources can either be a general-purpose register or a constant
register that holds a single value.
If we don't check for if the register is a general-purpose register,
it's possible that we get a constant register, which then causes all of
the register mapping code to break, as the constant registers aren't
supposed to be mapped like the general-purpose registers are.
This fix adds an isScalarReg check to the instruction encodings that
were missing it.
Change-Id: I3d7d5393aa324737301c3269cc227b60e8a159e4
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48344
Tested-by: kokoro
Reviewed-by: Matt Sinclair
Reviewed-by: Bobby R. Bruce
Reviewed-by: Matthew Poremba
Maintainer: Matt Sinclair
---
M src/arch/amdgpu/gcn3/insts/op_encodings.cc
1 file changed, 6 insertions(+), 6 deletions(-)
Approvals:
Matthew Poremba: Looks good to me, approved
Matt Sinclair: Looks good to me, but someone else must approve; Looks
good to me, approved
Bobby R. Bruce: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/amdgpu/gcn3/insts/op_encodings.cc
b/src/arch/amdgpu/gcn3/insts/op_encodings.cc
index cbbb767..cf20a2e 100644
--- a/src/arch/amdgpu/gcn3/insts/op_encodings.cc
+++ b/src/arch/amdgpu/gcn3/insts/op_encodings.cc
@@ -1277,12 +1277,12 @@
reg = extData.SRSRC;
srcOps.emplace_back(reg, getOperandSize(opNum), true,
- true, false, false);
+ isScalarReg(reg), false, false);
opNum++;
reg = extData.SOFFSET;
srcOps.emplace_back(reg, getOperandSize(opNum), true,
- true, false, false);
+ isScalarReg(reg), false, false);
opNum++;
}
@@ -1368,12 +1368,12 @@
reg = extData.SRSRC;
srcOps.emplace_back(reg, getOperandSize(opNum), true,
- true, false, false);
+ isScalarReg(reg), false, false);
opNum++;
reg = extData.SOFFSET;
srcOps.emplace_back(reg, getOperandSize(opNum), true,
- true, false, false);
+ isScalarReg(reg), false, false);
opNum++;
// extData.VDATA moves in the reg list depending on the instruction
@@ -1441,13 +1441,13 @@
reg = extData.SRSRC;
srcOps.emplace_back(reg, getOperandSize(opNum), true,
- true, false, false);
+ isScalarReg(reg), false, false);
opNum++;
if (getNumOperands() == 4) {
reg = extData.SSAMP;
srcOps.emplace_back(reg, getOperandSize(opNum), true,
- true, false, false);
+ isScalarReg(reg), false, false);
opNum++;
}
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the
submitted one.
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48344
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: I3d7d5393aa324737301c3269cc227b60e8a159e4
Gerrit-Change-Number: 48344
Gerrit-PatchSet: 3
Gerrit-Owner: Kyle Roarty
Gerrit-Reviewer: Alex Dutu
Gerrit-Reviewer: Bobby R. Bruce
Gerrit-Reviewer: Matt Sinclair
Gerrit-Reviewer: Matthew Poremba
Gerrit-Reviewer: kokoro
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[release-staging-v21-1]: arch-gcn3: Implement LDS accesses in Flat instructions
Matt Sinclair has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/48343 )
Change subject: arch-gcn3: Implement LDS accesses in Flat instructions
..
arch-gcn3: Implement LDS accesses in Flat instructions
Add support for LDS accesses by allowing Flat instructions to dispatch
into the local memory pipeline if the requested address is in the group
aperture.
This requires implementing LDS accesses in the Flat initMemRead/Write
functions, in a similar fashion to the DS functions of the same name.
Because we now can potentially dispatch to the local memory pipeline,
this change also adds a check to regain any tokens we requested as a
flat instruction.
Change-Id: Id26191f7ee43291a5e5ca5f39af06af981ec23ab
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48343
Reviewed-by: Matt Sinclair
Reviewed-by: Matthew Poremba
Maintainer: Matt Sinclair
Tested-by: kokoro
---
M src/arch/amdgpu/gcn3/insts/instructions.cc
M src/arch/amdgpu/gcn3/insts/op_encodings.hh
M src/gpu-compute/gpu_dyn_inst.cc
M src/gpu-compute/local_memory_pipeline.cc
4 files changed, 184 insertions(+), 32 deletions(-)
Approvals:
Matthew Poremba: Looks good to me, approved
Matt Sinclair: Looks good to me, but someone else must approve; Looks
good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/amdgpu/gcn3/insts/instructions.cc
b/src/arch/amdgpu/gcn3/insts/instructions.cc
index 79af7ac..65d008b 100644
--- a/src/arch/amdgpu/gcn3/insts/instructions.cc
+++ b/src/arch/amdgpu/gcn3/insts/instructions.cc
@@ -36314,7 +36314,7 @@
gpuDynInst->computeUnit()->globalMemoryPipe.
issueRequest(gpuDynInst);
} else {
-fatal("Non global flat instructions not implemented yet.\n");
+fatal("Unsupported scope for flat instruction.\n");
}
}
@@ -36363,7 +36363,7 @@
gpuDynInst->computeUnit()->globalMemoryPipe.
issueRequest(gpuDynInst);
} else {
-fatal("Non global flat instructions not implemented yet.\n");
+fatal("Unsupported scope for flat instruction.\n");
}
}
void
@@ -39384,8 +39384,11 @@
if (gpuDynInst->executedAs() == enums::SC_GLOBAL) {
gpuDynInst->computeUnit()->globalMemoryPipe
.issueRequest(gpuDynInst);
+} else if (gpuDynInst->executedAs() == enums::SC_GROUP) {
+gpuDynInst->computeUnit()->localMemoryPipe
+.issueRequest(gpuDynInst);
} else {
-fatal("Non global flat instructions not implemented yet.\n");
+fatal("Unsupported scope for flat instruction.\n");
}
} // execute
@@ -39448,8 +39451,11 @@
if (gpuDynInst->executedAs() == enums::SC_GLOBAL) {
gpuDynInst->computeUnit()->globalMemoryPipe
.issueRequest(gpuDynInst);
+} else if (gpuDynInst->executedAs() == enums::SC_GROUP) {
+gpuDynInst->computeUnit()->localMemoryPipe
+.issueRequest(gpuDynInst);
} else {
-fatal("Non global flat instructions not implemented yet.\n");
+fatal("Unsupported scope for flat instruction.\n");
}
}
@@ -39511,8 +39517,11 @@
if (gpuDynInst->executedAs() == enums::SC_GLOBAL) {
gpuDynInst->computeUnit()->globalMemoryPipe
.issueRequest(gpuDynInst);
+} else if (gpuDynInst->executedAs() == enums::SC_GROUP) {
+gpuDynInst->computeUnit()->localMemoryPipe
+.issueRequest(gpuDynInst);
} else {
-fatal("Non global flat instructions not implemented yet.\n");
+fatal("Unsupported scope for flat instruction.\n");
}
}
@@ -39603,8 +39612,11 @@
if (gpuDynInst->executedAs() == enums::SC_GLOBAL) {
gpuDynInst->computeUnit()->globalMemoryPipe
.issueRequest(gpuDynInst);
+} else if (gpuDynInst->executedAs() == enums::SC_GROUP) {
+gpuDynInst->computeUnit()->localMemoryPipe
+.issueRequest(gpuDynInst);
} else {
-fatal("Non global flat instructions not implemented yet.\n");
+fatal("Unsupported scope for flat instruction.\n");
}
}
@@ -39667,8 +39679,11 @@
if (gpuDynInst->executedAs() == enums::SC_GLOBAL) {
gpuDynInst->computeUnit()->globalMemoryPipe
.issueRequest(gpuDynInst);
+} else if (gpuDynInst->executedAs() == enums::SC_GROUP) {
+gpuDynInst->computeUnit()->localMemoryPipe
+.issueRequest(gpuDynInst);
} else {
-fatal("Non global flat instructions not implemented yet.\n");
+fatal("Unsupported scope for flat instruction.\n");
}
}
@@ -39731,8 +39746,11 @@
if (gpuDynInst->executedAs() == enums::SC_GLOBAL) {
[gem5-dev] Change in gem5/gem5[develop]: misc: Merge branch 'release-staging-v21-1' into develop
Bobby R. Bruce has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/48563 ) Change subject: misc: Merge branch 'release-staging-v21-1' into develop .. misc: Merge branch 'release-staging-v21-1' into develop Change-Id: I6ba57d7f70be70ae43fab396780d18623679a59a --- 1 file changed, 0 insertions(+), 0 deletions(-) -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48563 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: I6ba57d7f70be70ae43fab396780d18623679a59a Gerrit-Change-Number: 48563 Gerrit-PatchSet: 1 Gerrit-Owner: Bobby R. Bruce Gerrit-MessageType: newchange ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: Merge remote-tracking branch 'origin/release-staging-v21-1' into develop
Quentin Forcioli has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48543 )
Change subject: Merge remote-tracking branch 'origin/release-staging-v21-1'
into develop
..
Merge remote-tracking branch 'origin/release-staging-v21-1' into develop
Change-Id: Ie21c284906366928621e0f7f2044406499d14df3
---
M ext/git-commit-msg
2 files changed, 53 insertions(+), 166 deletions(-)
diff --git a/ext/git-commit-msg b/ext/git-commit-msg
index db83fd7..d46afd2 100755
--- a/ext/git-commit-msg
+++ b/ext/git-commit-msg
@@ -1,5 +1,5 @@
#!/bin/sh
-# From Gerrit Code Review 2.13.5-2617-gba50ae91fd
+# From Gerrit Code Review 3.4.0-1311-gb54da87ee1
#
# Part of Gerrit Code Review (https://www.gerritcodereview.com/)
#
@@ -16,179 +16,66 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
-#
-unset GREP_OPTIONS
+set -u
-CHANGE_ID_AFTER="Bug|Depends-On|Issue|Test|Feature|Fixes|Fixed"
-MSG="$1"
+# avoid [[ which is not POSIX sh.
+if test "$#" != 1 ; then
+ echo "$0 requires an argument."
+ exit 1
+fi
-# Check for, and add if missing, a unique Change-Id
-#
-add_ChangeId() {
- clean_message=`sed -e '
- /^diff --git .*/{
- s///
- q
- }
- /^Signed-off-by:/d
- /^#/d
- ' "$MSG" | git stripspace`
- if test -z "$clean_message"
- then
- return
- fi
+if test ! -f "$1" ; then
+ echo "file does not exist: $1"
+ exit 1
+fi
- # Do not add Change-Id to temp commits
- if echo "$clean_message" | head -1 | grep -q '^\(fixup\|squash\)!'
- then
- return
- fi
+# Do not create a change id if requested
+if test "false" = "$(git config --bool --get gerrit.createChangeId)" ; then
+ exit 0
+fi
- if test "false" = "`git config --bool --get gerrit.createChangeId`"
- then
- return
- fi
+if git rev-parse --verify HEAD >/dev/null 2>&1; then
+ refhash="$(git rev-parse HEAD)"
+else
+ refhash="$(git hash-object -t tree /dev/null)"
+fi
- # Does Change-Id: already exist? if so, exit (no change).
- if grep -i '^Change-Id:' "$MSG" >/dev/null
- then
- return
- fi
+random=$({ git var GIT_COMMITTER_IDENT ; echo "$refhash" ; cat "$1"; } |
git hash-object --stdin)
+dest="$1.tmp.${random}"
- id=`_gen_ChangeId`
- T="$MSG.tmp.$$"
- AWK=awk
- if [ -x /usr/xpg4/bin/awk ]; then
- # Solaris AWK is just too broken
- AWK=/usr/xpg4/bin/awk
- fi
+trap 'rm -f "${dest}"' EXIT
- # Get core.commentChar from git config or use default symbol
- commentChar=`git config --get core.commentChar`
- commentChar=${commentChar:-#}
+if ! git stripspace --strip-comments < "$1" > "${dest}" ; then
+ echo "cannot strip comments from $1"
+ exit 1
+fi
- # How this works:
- # - parse the commit message as (textLine+ blankLine*)*
- # - assume textLine+ to be a footer until proven otherwise
- # - exception: the first block is not footer (as it is the title)
- # - read textLine+ into a variable
- # - then count blankLines
- # - once the next textLine appears, print textLine+ blankLine* as these
- # aren't footer
- # - in END, the last textLine+ block is available for footer parsing
- $AWK '
- BEGIN {
- # while we start with the assumption that textLine+
- # is a footer, the first block is not.
- isFooter = 0
- footerComment = 0
- blankLines = 0
- }
+if test ! -s "${dest}" ; then
+ echo "file is empty: $1"
+ exit 1
+fi
- # Skip lines starting with commentChar without any spaces before it.
- /^'"$commentChar"'/ { next }
+reviewurl="$(git config --get gerrit.reviewUrl)"
+if test -n "${reviewurl}" ; then
+ if ! git interpret-trailers --parse < "$1" | grep
-q '^Link:.*/id/I[0-9a-f]\{40\}$' ; then
+if ! git interpret-trailers \
+ --trailer "Link: ${reviewurl%/}/id/I${random}" < "$1"
"${dest}" ; then
+ echo "cannot insert link footer in $1"
+ exit 1
+fi
+ fi
+else
+ # Avoid the --in-place option which only appeared in Git 2.8
+ # Avoid the --if-exists option which only appeared in Git 2.15
+ if ! git -c trailer.ifexists=doNothing interpret-trailers \
+--trailer "Change-Id: I${random}" < "$1" > "${dest}" ; then
+echo "cannot insert change-id line in $1"
+exit 1
+ fi
+fi
- # Skip the line starting with the diff command and everything after it,
- # up to the end of the file, assuming it is only patch data.
- # If more than one line before the diff was empty, strip all but one.
- /^diff --git
[gem5-dev] Change in gem5/gem5[develop]: arch-arm,dev-arm: Running OPTEE to boot on gem5
Quentin Forcioli has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/48544 )
Change subject: arch-arm,dev-arm: Running OPTEE to boot on gem5
..
arch-arm,dev-arm: Running OPTEE to boot on gem5
The OPTEE I boot is built using ARM "Running Trusted Firmware-A on gem5"
tutorial from ARM
(https://community.arm.com/developer/research/b/articles/posts/running-trusted-firmware-a-on-gem5)
and OPTEE repositories
(https://optee.readthedocs.io/en/latest/building/gits/index.html).
-Changed IRQ/FIQ behavior: taking into account that FIQ and IRQ can
transform into each other when they belong to group 1.
-Change El2S behavior: prevent a disabled EL2S to trap anything from EL1S
-ARM Foundation platform: add devices that are need to boot OPTEE
(respecting ARM specifications)
Change-Id: Ifd3ed698f766e5aef7f4af0bfb43094be188f446
---
M src/arch/arm/insts/misc64.cc
M src/arch/arm/interrupts.cc
M src/arch/arm/interrupts.hh
M src/arch/arm/isa.hh
M src/dev/arm/RealView.py
M src/dev/arm/gic_v3_cpu_interface.hh
6 files changed, 47 insertions(+), 8 deletions(-)
diff --git a/src/arch/arm/insts/misc64.cc b/src/arch/arm/insts/misc64.cc
index 0f301b0..88810ab 100644
--- a/src/arch/arm/insts/misc64.cc
+++ b/src/arch/arm/insts/misc64.cc
@@ -191,6 +191,9 @@
const HDCR mdcr = tc->readMiscReg(MISCREG_MDCR_EL3);
bool trap_to_hyp = false;
+//don't trap to EL2 from EL1s if EL2S is not enabled
+if (isSecure(tc)!= IsSecureEL2Enabled(tc))
+ return false;
switch (misc_reg) {
case MISCREG_IMPDEF_UNIMPL:
diff --git a/src/arch/arm/interrupts.cc b/src/arch/arm/interrupts.cc
index b0f18df..7010e6b 100644
--- a/src/arch/arm/interrupts.cc
+++ b/src/arch/arm/interrupts.cc
@@ -36,8 +36,9 @@
*/
#include "arch/arm/interrupts.hh"
-
+#include "arch/arm/isa.hh"
#include "arch/arm/system.hh"
+#include "dev/arm/gic_v3_cpu_interface.hh"
namespace gem5
{
@@ -159,4 +160,16 @@
(mask != INT_MASK_P);
}
+void ArmISA::Interrupts::checkForFiqIrqMutation() const
+{
+//TODO check if pending IRQ is from G1S or G1NS
+ISA* isa=static_cast(tc->getIsaPtr());
+if (isa->haveGICv3CpuIfc()) {
+Gicv3CPUInterface& interf=dynamic_cast(
+isa->getGICv3CPUInterface()
+);
+interf.update();
+}
+}
+
} // namespace gem5
diff --git a/src/arch/arm/interrupts.hh b/src/arch/arm/interrupts.hh
index 2f99d6e..2bd3dc6 100644
--- a/src/arch/arm/interrupts.hh
+++ b/src/arch/arm/interrupts.hh
@@ -128,6 +128,10 @@
};
bool takeInt(InterruptTypes int_type) const;
+//this function check for FIQ or IRQ that changed nature of interrupt
+//because of EL change
+//(G1NS et G1S IRQ are FIQ if not in the right security setting)
+void checkForFiqIrqMutation() const;
bool
checkInterrupts() const override
@@ -164,9 +168,15 @@
bool take_irq = takeInt(INT_IRQ);
bool take_fiq = takeInt(INT_FIQ);
bool take_ea = takeInt(INT_ABT);
-
-return ((interrupts[INT_IRQ] && take_irq) ||
-(interrupts[INT_FIQ] && take_fiq) ||
+bool is_irq =(interrupts[INT_IRQ] && take_irq);
+bool is_fiq =(interrupts[INT_FIQ] && take_fiq);
+if (is_irq || is_fiq) {
+checkForFiqIrqMutation();
+is_irq =(interrupts[INT_IRQ] && take_irq);
+is_fiq =(interrupts[INT_FIQ] && take_fiq);
+}
+return (is_irq ||
+is_fiq ||
(interrupts[INT_ABT] && take_ea)||
((interrupts[INT_VIRT_IRQ] || hcr.vi) && allowVIrq) ||
((interrupts[INT_VIRT_FIQ] || hcr.vf) && allowVFiq) ||
diff --git a/src/arch/arm/isa.hh b/src/arch/arm/isa.hh
index cc6e54d..5c4ccf2 100644
--- a/src/arch/arm/isa.hh
+++ b/src/arch/arm/isa.hh
@@ -499,7 +499,6 @@
}
BaseISADevice ();
-BaseISADevice ();
private:
void assert32() { assert(((CPSR)readMiscReg(MISCREG_CPSR)).width);
}
@@ -889,6 +888,8 @@
return gicv3CpuInterface != nullptr;
}
+BaseISADevice ();
+
enums::VecRegRenameMode
initVecRegRenameMode() const override
{
diff --git a/src/dev/arm/RealView.py b/src/dev/arm/RealView.py
index 919ff63..91fd4da 100644
--- a/src/dev/arm/RealView.py
+++ b/src/dev/arm/RealView.py
@@ -1430,10 +1430,19 @@
pci_pio_base=0x5000,
pci_mem_base=0x40,
int_policy="ARM_PCI_INT_DEV", int_base=100, int_count=4)
+#flash1 have to be a SimpleMemory to keep
+#support for u-boot vexpress_aemv8a configuration
+flash1 = SimpleMemory(range=AddrRange(0x0c00, 0x1000),
+ conf_table_reported=False)
+#TODO: preventing access from unsecure world to the
