[gem5-dev] Change in gem5/gem5[develop]: arch-riscv: fix incorrect interrupt checking logic

[Cui Jin (Gerrit) via gem5-dev]

Cui Jin has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/39035 )



Change subject: arch-riscv: fix incorrect interrupt checking logic
..

arch-riscv: fix incorrect interrupt checking logic

Whether global interrupt enabling or not is not simply decided by
xIE bit in mstatus, it also depends on current privilige level.
All level lower/higher than current should be disabled/enabled
regardless of the xIE bit. xIE bit is only control the enabling
of interrupt in current privilige level.

The fix is verified in FS.

Jira Issue: https://gem5.atlassian.net/browse/GEM5-883

Change-Id: I37f83ab77af2efbf1da9b81845828d322e49bf5f
---
M src/arch/riscv/interrupts.hh
1 file changed, 25 insertions(+), 6 deletions(-)



diff --git a/src/arch/riscv/interrupts.hh b/src/arch/riscv/interrupts.hh
index fba925e..e1460ab 100644
--- a/src/arch/riscv/interrupts.hh
+++ b/src/arch/riscv/interrupts.hh
@@ -72,12 +72,31 @@
 {
 INTERRUPT mask = 0;
 STATUS status = tc->readMiscReg(MISCREG_STATUS);
-if (status.mie)
-mask.mei = mask.mti = mask.msi = 1;
-if (status.sie)
-mask.sei = mask.sti = mask.ssi = 1;
-if (status.uie)
-mask.uei = mask.uti = mask.usi = 1;
+PrivilegeMode prv = (PrivilegeMode)tc->readMiscReg(MISCREG_PRV);
+switch (prv) {
+case PRV_U:
+mask.mei = mask.mti = mask.msi = 1;
+mask.sei = mask.sti = mask.ssi = 1;
+if (status.uie)
+mask.uei = mask.uti = mask.usi = 1;
+break;
+case PRV_S:
+mask.mei = mask.mti = mask.msi = 1;
+if (status.sie)
+mask.sei = mask.sti = mask.ssi = 1;
+mask.uei = mask.uti = mask.usi = 0;
+break;
+case PRV_M:
+if (status.mie)
+ mask.mei = mask.mti = mask.msi = 1;
+mask.sei = mask.sti = mask.ssi = 0;
+mask.uei = mask.uti = mask.usi = 0;
+break;
+default:
+panic("Unknown privilege mode %d.", prv);
+break;
+}
+
 return std::bitset(mask);
 }


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39035
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: I37f83ab77af2efbf1da9b81845828d322e49bf5f
Gerrit-Change-Number: 39035
Gerrit-PatchSet: 1
Gerrit-Owner: Cui Jin 
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 begin() and end() from CircleBuf.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: base: Remove begin() and end() from CircleBuf.
..

base: Remove begin() and end() from CircleBuf.

These functions return iterators which are inconsistent with the usage
model for this type. It should be accessed using the peek, push, and pop
methods and not iterators. If you need a class with iterators which is
oriented around accessing individual elements at a time, the
CircularQueue type is likely a better choice.

Change-Id: I9f37eab12e490b63d870d378a91f601dad353f25
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38998
Reviewed-by: Daniel Carvalho 
Reviewed-by: Andreas Sandberg 
Maintainer: Gabe Black 
Maintainer: Andreas Sandberg 
Tested-by: kokoro 
---
M src/base/circlebuf.hh
1 file changed, 0 insertions(+), 11 deletions(-)

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



diff --git a/src/base/circlebuf.hh b/src/base/circlebuf.hh
index 0f1cea6..bcfa91a 100644
--- a/src/base/circlebuf.hh
+++ b/src/base/circlebuf.hh
@@ -62,8 +62,6 @@

   public:
 using value_type = T;
-using iterator = typename std::vector::iterator;
-using const_iterator = typename std::vector::const_iterator;

 explicit CircleBuf(size_t size) : buffer(size), maxSize(size) {}

@@ -71,15 +69,6 @@
 size_t size() const { return used; }
 size_t capacity() const { return maxSize; }

-iterator begin() { return buffer.begin() + start % maxSize; }
-const_iterator begin() const { return buffer.begin() + start %  
maxSize; }

-iterator end() { return buffer.begin() + (start + used) % maxSize; }
-const_iterator
-end() const
-{
-return buffer.begin() + (start + used) % maxSize;
-}
-
 /**
  * Throw away any data in the buffer.
  */

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38998
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: I9f37eab12e490b63d870d378a91f601dad353f25
Gerrit-Change-Number: 38998
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] IWYU tool and include checking from scons

[Gabe Black via gem5-dev]

Hi folks. Daniel has submitted a big change which fixes up a bunch of
missing includes in files which were coincidentally getting the definitions
they needed indirectly from some other file. Way down the line in c++20 I
think the "modules" mechanism would be a great tool avoiding these sorts of
problems from the get go, but in the meantime it would be helpful if we had
a way to scan for these sorts of issues, instead of finding them when they
cause problems. These sorts of overly broad, leaky includes also likely
slow down builds by making the compiler process more text than it really
needs to, and also introduces more dependencies at the scons level than are
necessary.

I'm not really familiar with it, but after a little bit of Googling I found
a tool called iwyu (include what you use) which looks at includes and finds
places where includes are missing (transitive includes), and also includes
which are not being used. It looks like the way this tool is intended to be
used is to substitute it for the compiler, and then run it through, for
instance, make.

Rather than use it that way, I'm thinking we might be able to set up
additional scons rules which would build iwyu error reports based on Source
declarations in scons, alongside the normal build outputs. There may be
false positives in there somewhere, particularly from code we don't
control, and so we'd likely want to add in ways to flag exceptions.

If this sort of scons integration works out, it would be nice to make a
pass of this tool part of the presubmit checks, assuming it doesn't add
tons of time to the build.

What do folks think? I don't necessarily have a lot of time to work on this
myself, although I might give it a shot if I find some time. It could also
be something for someone wanting to cut their teeth on scons to help with,
and I could help give pointers and help with the high level design if
someone wanted to try it. If you do want to give it a try, at the very
least keep me in the loop so we don't have to redo things in a major way
when it comes time to check something in.

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: Use regular atomic accesses for DMA in bypass mode.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: dev: Use regular atomic accesses for DMA in bypass mode.
..

dev: Use regular atomic accesses for DMA in bypass mode.

These are now accelerated with backdoor accesses and should be at least
as fast as functional accesses. This removes a dependency on port
proxies, and also stops the HDLCD from using functional accesses.

Change-Id: I5e959288eb533d09cffa7b79938aa2f61e4aff7d
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38720
Reviewed-by: Andreas Sandberg 
Maintainer: Andreas Sandberg 
Tested-by: kokoro 
---
M src/dev/dma_device.cc
M src/dev/dma_device.hh
2 files changed, 10 insertions(+), 10 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/dev/dma_device.cc b/src/dev/dma_device.cc
index bbdae7d..9aebb7b 100644
--- a/src/dev/dma_device.cc
+++ b/src/dev/dma_device.cc
@@ -370,8 +370,8 @@
  unsigned max_pending,
  Request::Flags flags)
 : maxReqSize(max_req_size), fifoSize(size),
-  reqFlags(flags), port(_port), proxy(port, port.sys->cacheLineSize()),
-  cacheLineSize(port.sys->cacheLineSize()), buffer(size)
+  reqFlags(flags), port(_port),  
cacheLineSize(port.sys->cacheLineSize()),

+  buffer(size)
 {
 freeRequests.resize(max_pending);
 for (auto &e : freeRequests)
@@ -465,7 +465,7 @@
 const bool old_eob(atEndOfBlock());

 if (port.sys->bypassCaches())
-resumeFillFunctional();
+resumeFillBypass();
 else
 resumeFillTiming();

@@ -474,7 +474,7 @@
 }

 void
-DmaReadFifo::resumeFillFunctional()
+DmaReadFifo::resumeFillBypass()
 {
 const size_t fifo_space = buffer.capacity() - buffer.size();
 if (fifo_space >= cacheLineSize || buffer.capacity() < cacheLineSize) {
@@ -483,11 +483,13 @@
 std::vector tmp_buffer(xfer_size);

 assert(pendingRequests.empty());
-DPRINTF(DMA, "KVM Bypassing startAddr=%#x xfer_size=%#x " \
+DPRINTF(DMA, "Direct bypass startAddr=%#x xfer_size=%#x " \
 "fifo_space=%#x block_remaining=%#x\n",
 nextAddr, xfer_size, fifo_space, block_remaining);

-proxy.readBlob(nextAddr, tmp_buffer.data(), xfer_size);
+port.dmaAction(MemCmd::ReadReq, nextAddr, xfer_size, nullptr,
+tmp_buffer.data(), 0, reqFlags);
+
 buffer.write(tmp_buffer.begin(), xfer_size);
 nextAddr += xfer_size;
 }
diff --git a/src/dev/dma_device.hh b/src/dev/dma_device.hh
index 3904a08..330be1a 100644
--- a/src/dev/dma_device.hh
+++ b/src/dev/dma_device.hh
@@ -49,7 +49,6 @@
 #include "base/circlebuf.hh"
 #include "dev/io_device.hh"
 #include "mem/backdoor.hh"
-#include "mem/port_proxy.hh"
 #include "params/DmaDevice.hh"
 #include "sim/drain.hh"
 #include "sim/system.hh"
@@ -508,7 +507,6 @@
 const Request::Flags reqFlags;

 DmaPort &port;
-PortProxy proxy;

 const int cacheLineSize;

@@ -554,8 +552,8 @@
 /** Try to issue new DMA requests during normal execution*/
 void resumeFillTiming();

-/** Try to bypass DMA requests in KVM execution mode */
-void resumeFillFunctional();
+/** Try to bypass DMA requests in non-caching mode */
+void resumeFillBypass();

   private: // Internal state
 Fifo buffer;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38720
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: I5e959288eb533d09cffa7b79938aa2f61e4aff7d
Gerrit-Change-Number: 38720
Gerrit-PatchSet: 13
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Nikos Nikoleris 
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]: dev: Teach the DmaPort to use atomic memory backdoors.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: dev: Teach the DmaPort to use atomic memory backdoors.
..

dev: Teach the DmaPort to use atomic memory backdoors.

This is implemented similary to the NonCachingSimpleCPU, except that
both the normal atomic and noncaching atomic behaviors are implemented
by the same class. The sendDma function now dispatches to a method which
implements one or the other behavior since that function was getting too
big and complex.

Change-Id: I7972971ef41d1373424e587cf67c8444d50de748
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38719
Reviewed-by: Andreas Sandberg 
Maintainer: Andreas Sandberg 
Tested-by: kokoro 
---
M src/dev/dma_device.cc
M src/dev/dma_device.hh
2 files changed, 137 insertions(+), 35 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/dev/dma_device.cc b/src/dev/dma_device.cc
index f60cba8..bbdae7d 100644
--- a/src/dev/dma_device.cc
+++ b/src/dev/dma_device.cc
@@ -40,8 +40,13 @@

 #include "dev/dma_device.hh"

+#include 
+#include 
+#include 
 #include 

+#include "base/logging.hh"
+#include "base/trace.hh"
 #include "debug/DMA.hh"
 #include "debug/Drain.hh"
 #include "sim/clocked_object.hh"
@@ -56,7 +61,7 @@
 { }

 void
-DmaPort::handleResp(PacketPtr pkt, Tick delay)
+DmaPort::handleRespPacket(PacketPtr pkt, Tick delay)
 {
 // Should always see a response with a sender state.
 assert(pkt->isResponse());
@@ -65,16 +70,24 @@
 auto *state = dynamic_cast(pkt->senderState);
 assert(state);

+handleResp(state, pkt->getAddr(), pkt->req->getSize(), delay);
+
+delete pkt;
+}
+
+void
+DmaPort::handleResp(DmaReqState *state, Addr addr, Addr size, Tick delay)
+{
 DPRINTF(DMA, "Received response %s for addr: %#x size: %d nb: %d,"  \
 " tot: %d sched %d\n",
-pkt->cmdString(), pkt->getAddr(), pkt->req->getSize(),
+MemCmd(state->cmd).toString(), addr, size,
 state->numBytes, state->totBytes,
 state->completionEvent ?
 state->completionEvent->scheduled() : 0);

 // Update the number of bytes received based on the request rather
 // than the packet as the latter could be rounded up to line sizes.
-state->numBytes += pkt->req->getSize();
+state->numBytes += size;
 assert(state->totBytes >= state->numBytes);

 // If we have reached the total number of bytes for this DMA request,
@@ -89,8 +102,6 @@
 delete state;
 }

-delete pkt;
-
 // We might be drained at this point, if so signal the drain event.
 if (pendingCount == 0)
 signalDrainDone();
@@ -121,7 +132,7 @@
 assert(pkt->req->isUncacheable() ||
!(pkt->cacheResponding() && !pkt->hasSharers()));

-handleResp(pkt);
+handleRespPacket(pkt);

 return true;
 }
@@ -226,6 +237,92 @@
 transmitList.size(), inRetry ? 1 : 0);
 }

+bool
+DmaPort::sendAtomicReq(DmaReqState *state)
+{
+PacketPtr pkt = state->createPacket();
+DPRINTF(DMA, "Sending  DMA for addr: %#x size: %d\n",
+state->gen.addr(), state->gen.size());
+Tick lat = sendAtomic(pkt);
+
+// Check if we're done, since handleResp may delete state.
+bool done = !state->gen.next();
+handleRespPacket(pkt, lat);
+return done;
+}
+
+bool
+DmaPort::sendAtomicBdReq(DmaReqState *state)
+{
+bool done = false;
+
+auto bd_it = memBackdoors.contains(state->gen.addr());
+if (bd_it == memBackdoors.end()) {
+// We don't have a backdoor for this address, so use a packet.
+
+PacketPtr pkt = state->createPacket();
+DPRINTF(DMA, "Sending DMA for addr: %#x size: %d\n",
+state->gen.addr(), state->gen.size());
+
+MemBackdoorPtr bd = nullptr;
+Tick lat = sendAtomicBackdoor(pkt, bd);
+
+// If we got a backdoor, record it.
+if (bd && memBackdoors.insert(bd->range(), bd) !=  
memBackdoors.end()) {
+// Invalidation callback which finds this backdoor and removes  
it.

+auto callback = [this](const MemBackdoor &backdoor) {
+for (auto it = memBackdoors.begin();
+it != memBackdoors.end(); it++) {
+if (it->second == &backdoor) {
+memBackdoors.erase(it);
+return;
+}
+}
+panic("Got invalidation for unknown memory backdoor.");
+};
+bd->addInvalidationCallback(callback);
+}
+
+// Check if we're done now, since handleResp may delete state.
+done = !state->gen.next();
+handleRespPacket(pkt, lat);
+} else {
+// We have a backdoor that can at least partially satisfy this  
request.
+DPRINTF(DMA, "H

[gem5-dev] Change in gem5/gem5[develop]: dev: Generate DMA packets as needed.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: dev: Generate DMA packets as needed.
..

dev: Generate DMA packets as needed.

Instead of generating all of the DMA packets when a request is
initiated, keep track of the necessary properties and generate them as
needed.

The primary benefit of this approach is that if we don't actually need
packets, for instance if we can satisfy the request using a memory
backdoor, we can just skip them. Otherwise we'll have wasted time
creating them, and then have to clean them up.

Change-Id: I04d399fb7bce1ff9a44979c311be356baf2db555
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38717
Reviewed-by: Andreas Sandberg 
Maintainer: Andreas Sandberg 
Tested-by: kokoro 
---
M src/dev/dma_device.cc
M src/dev/dma_device.hh
2 files changed, 105 insertions(+), 82 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/dev/dma_device.cc b/src/dev/dma_device.cc
index da35833..f60cba8 100644
--- a/src/dev/dma_device.cc
+++ b/src/dev/dma_device.cc
@@ -42,7 +42,6 @@

 #include 

-#include "base/chunk_generator.hh"
 #include "debug/DMA.hh"
 #include "debug/Drain.hh"
 #include "sim/clocked_object.hh"
@@ -59,10 +58,10 @@
 void
 DmaPort::handleResp(PacketPtr pkt, Tick delay)
 {
-// should always see a response with a sender state
+// Should always see a response with a sender state.
 assert(pkt->isResponse());

-// get the DMA sender state
+// Get the DMA sender state.
 auto *state = dynamic_cast(pkt->senderState);
 assert(state);

@@ -73,17 +72,16 @@
 state->completionEvent ?
 state->completionEvent->scheduled() : 0);

-assert(pendingCount != 0);
-pendingCount--;
-
-// update the number of bytes received based on the request rather
-// than the packet as the latter could be rounded up to line sizes
+// Update the number of bytes received based on the request rather
+// than the packet as the latter could be rounded up to line sizes.
 state->numBytes += pkt->req->getSize();
 assert(state->totBytes >= state->numBytes);

-// if we have reached the total number of bytes for this DMA
-// request, then signal the completion and delete the sate
+// If we have reached the total number of bytes for this DMA request,
+// then signal the completion and delete the sate.
 if (state->totBytes == state->numBytes) {
+assert(pendingCount != 0);
+pendingCount--;
 if (state->completionEvent) {
 delay += state->delay;
 device->schedule(state->completionEvent, curTick() + delay);
@@ -91,18 +89,35 @@
 delete state;
 }

-// delete the packet
 delete pkt;

-// we might be drained at this point, if so signal the drain event
+// We might be drained at this point, if so signal the drain event.
 if (pendingCount == 0)
 signalDrainDone();
 }

+PacketPtr
+DmaPort::DmaReqState::createPacket()
+{
+RequestPtr req = std::make_shared(
+gen.addr(), gen.size(), flags, id);
+req->setStreamId(sid);
+req->setSubstreamId(ssid);
+req->taskId(ContextSwitchTaskId::DMA);
+
+PacketPtr pkt = new Packet(req, cmd);
+
+if (data)
+pkt->dataStatic(data + gen.complete());
+
+pkt->senderState = this;
+return pkt;
+}
+
 bool
 DmaPort::recvTimingResp(PacketPtr pkt)
 {
-// We shouldn't ever get a cacheable block in Modified state
+// We shouldn't ever get a cacheable block in Modified state.
 assert(pkt->req->isUncacheable() ||
!(pkt->cacheResponding() && !pkt->hasSharers()));

@@ -146,41 +161,20 @@
uint8_t *data, uint32_t sid, uint32_t ssid, Tick delay,
Request::Flags flag)
 {
-// one DMA request sender state for every action, that is then
-// split into many requests and packets based on the block size,
-// i.e. cache line size
-DmaReqState *reqState = new DmaReqState(event, size, delay);
-
-RequestPtr req = nullptr;
-
 DPRINTF(DMA, "Starting DMA for addr: %#x size: %d sched: %d\n", addr,  
size,

 event ? event->scheduled() : -1);
-for (ChunkGenerator gen(addr, size, cacheLineSize);
- !gen.done(); gen.next()) {

-req = std::make_shared(
-gen.addr(), gen.size(), flag, requestorId);
+// One DMA request sender state for every action, that is then
+// split into many requests and packets based on the block size,
+// i.e. cache line size.
+transmitList.push_back(
+new DmaReqState(cmd, addr, cacheLineSize, size,
+data, flag, requestorId, sid, ssid, event, delay));
+pendingCount++;

-req->setStreamId(sid);
-req->setSubstreamId(ssid);
-
-req->taskId(ContextSwitchTa

[gem5-dev] Change in gem5/gem5[develop]: base: Re-implement CircleBuf without using CircularQueue.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: base: Re-implement CircleBuf without using CircularQueue.
..

base: Re-implement CircleBuf without using CircularQueue.

CircularQueue provides iterators which make it easier for users to
interact with it and helps abstract its internal state, but at the same
time it prevents standard algorithms like std::copy from recognizing
opportunities to use bulk copies to speed up execution. It also hides
the seams when wrapping around the buffer happens which std::copy
wouldn't know how to handle.

CircleBuf seems to be intended as a simpler type which doesn't hold
complex entries like the CircularQueue does, and instead just acts as a
wrap around buffer, like the name suggests. This change reimplements it
to not use CircularQueue, and to instead use an underlying vector.

The way internal indexing is handled CircularQueue was simplified
recently, and using the same scheme here means that this code is
actually not much more verbose than it was before. It also intrinsically
handles indexing and bulk accesses, and uses std::copy_n in a way that
lets it recognize and take advantage of contiguous storage and bulk
copies.

Change-Id: I78e7cfe174c52f60c95c81e5cd3d71c6052d4d41
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38896
Reviewed-by: Daniel Carvalho 
Maintainer: Gabe Black 
Maintainer: Andreas Sandberg 
Tested-by: kokoro 
---
M src/base/circlebuf.hh
1 file changed, 97 insertions(+), 26 deletions(-)

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



diff --git a/src/base/circlebuf.hh b/src/base/circlebuf.hh
index 8d7297a..0f1cea6 100644
--- a/src/base/circlebuf.hh
+++ b/src/base/circlebuf.hh
@@ -40,32 +40,55 @@

 #include 
 #include 
+#include 
 #include 

-#include "base/circular_queue.hh"
 #include "base/logging.hh"
 #include "sim/serialize.hh"

 /**
- * Circular buffer backed by a vector though a CircularQueue.
+ * Circular buffer backed by a vector.
  *
- * The data in the cricular buffer is stored in a standard
- * vector.
- *
+ * The data in the cricular buffer is stored in a standard vector.
  */
 template
-class CircleBuf : public CircularQueue
+class CircleBuf
 {
+  private:
+std::vector buffer;
+size_t start = 0;
+size_t used = 0;
+size_t maxSize;
+
   public:
-explicit CircleBuf(size_t size)
-: CircularQueue(size) {}
-using CircularQueue::empty;
-using CircularQueue::size;
-using CircularQueue::capacity;
-using CircularQueue::begin;
-using CircularQueue::end;
-using CircularQueue::pop_front;
-using CircularQueue::advance_tail;
+using value_type = T;
+using iterator = typename std::vector::iterator;
+using const_iterator = typename std::vector::const_iterator;
+
+explicit CircleBuf(size_t size) : buffer(size), maxSize(size) {}
+
+bool empty() const { return used == 0; }
+size_t size() const { return used; }
+size_t capacity() const { return maxSize; }
+
+iterator begin() { return buffer.begin() + start % maxSize; }
+const_iterator begin() const { return buffer.begin() + start %  
maxSize; }

+iterator end() { return buffer.begin() + (start + used) % maxSize; }
+const_iterator
+end() const
+{
+return buffer.begin() + (start + used) % maxSize;
+}
+
+/**
+ * Throw away any data in the buffer.
+ */
+void
+flush()
+{
+start = 0;
+used = 0;
+}

 /**
  * Copy buffer contents without advancing the read pointer
@@ -91,10 +114,27 @@
 void
 peek(OutputIterator out, off_t offset, size_t len) const
 {
-panic_if(offset + len > size(),
+panic_if(offset + len > used,
  "Trying to read past end of circular buffer.");

-std::copy(begin() + offset, begin() + offset + len, out);
+if (!len)
+return;
+
+// The iterator for the next byte to copy out.
+auto next_it = buffer.begin() + (start + offset) % maxSize;
+// How much there is to copy from until the end of the buffer.
+const size_t to_end = buffer.end() - next_it;
+
+// If the data to be copied wraps, take care of the first part.
+if (to_end < len) {
+// Copy it.
+out = std::copy_n(next_it, to_end, out);
+// Start copying again at the start of buffer.
+next_it = buffer.begin();
+len -= to_end;
+}
+// Copy the remaining (or only) chunk of data.
+std::copy_n(next_it, len, out);
 }

 /**
@@ -108,11 +148,15 @@
 read(OutputIterator out, size_t len)
 {
 peek(out, len);
-pop_front(len);
+used -= len;
+start += len;
 }

 /**
-

[gem5-dev] Re: Testing fail on GCN3_X86 on Arm machine

[Poremba, Matthew via gem5-dev]

[AMD Public Use]

The defines causing an error seem to be set in the HSA header file itself but 
only for x86 type hosts. I'm hesitant to change this since it is pulled 
directly, unmodified, from the AMD HSA runtime repo including license, etc.

However, we might be able to do some scons magic to define one of these by 
asking the host OS what the endianness is. For example use 'sys.byteorder' in 
python to determine endianness and then define one or the other.


-Matt

From: Bobby Bruce 
Sent: Tuesday, January 12, 2021 1:10 PM
To: gem5 Developer List 
Cc: Dutu, Alexandru ; Poremba, Matthew 
; Kyle Roarty ; Nathanael Premillieu 

Subject: Re: [gem5-dev] Re: Testing fail on GCN3_X86 on Arm machine

[CAUTION: External Email]
I'm guessing you're correct and this is because you're testing on an ARM 
machine. GCN3_X86 should compile fine, though we don't test on ARM hosts (we'd 
like to change this soon).

I've added a Jira ticket covering this bug: 
https://gem5.atlassian.net/browse/GEM5-881.

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

web: 
https://www.bobbybruce.net

Please consider making a submission to GI@ICSE '21: 
http://geneticimprovementofsoftware.com/gi2021icse.html


On Tue, Jan 12, 2021 at 12:44 AM Nathanael Premillieu via gem5-dev 
mailto:gem5-dev@gem5.org>> wrote:
Hi,

Thanks Jason and Matt for your answers.
I'm not really interested in GCN3, it's just that it is tested by default when 
running all the tests.
And as I'm working on a change that is not specific to a particular 
architecture, I wanted to test it on everything.
For the moment I have relaunched the tests without the "GCN3_X86" isa.
But I'll also test on a x86 machine with everything.

Thanks,
Nathanael

From: Jason Lowe-Power via gem5-dev 
[mailto:gem5-dev@gem5.org]
Sent: Tuesday, January 12, 2021 12:17 AM
To: gem5 Developer List mailto:gem5-dev@gem5.org>>
Cc: Dutu, Alexandru mailto:alexandru.d...@amd.com>>; 
Poremba, Matthew mailto:matthew.pore...@amd.com>>; 
Kyle Roarty mailto:kroa...@wisc.edu>>; Jason Lowe-Power 
mailto:ja...@lowepower.com>>
Subject: [gem5-dev] Re: Testing fail on GCN3_X86 on Arm machine

Hi all,

It doesn't surprise me at all that GCN3 doesn't build on Arm. Nathanael, you 
should be able to ignore those errors. We don't test that combination of 
host-target and I have not heard of anyone trying to use that combination of 
host-target.

If you're trying to use the GPU model, I'd suggest moving to an x86 host. 
Unfortunately, right now the GPU model is intricately tied to the host OS. 
Hence why the docker is all but required to get the GPU model working.

Cheers,
Jason

On Mon, Jan 11, 2021 at 10:59 AM Matt Sinclair via gem5-dev 
mailto:gem5-dev@gem5.org>> wrote:
Hi Nathanael,

I have personally never tested GCN3 on an ARM machine, so I can't say 
definitively if it will work there or not.  But if you are not, I strongly 
recommend testing anything you need to test with GCN3 inside the docker we 
created:

- 
https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/util/dockerfiles/gcn-gpu/README.md
- 
http://www.gem5.org/2020/05/27/modern-gpu-applications.html

[gem5-dev] Change in gem5/gem5[develop]: misc: Fix coding style for class-opening braces

[Daniel Carvalho (Gerrit) via gem5-dev]

Daniel Carvalho has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/39015 )



Change subject: misc: Fix coding style for class-opening braces
..

misc: Fix coding style for class-opening braces

The systemc dir was not included in this fix.

First it was identified that there were only occurrences
at 0, 1, and 2 levels of indentation, using:

grep -nrE --exclude-dir=systemc \
"^ *class [A-Za-z].* {$" src/

Then the following commands were run to replace:

class X ... {

by:

class X ...
{

Level 0:
grep -nrl --exclude-dir=systemc
"^class [A-Za-z].* {$" src/ | \
xargs sed -Ei \
's/^class ([A-Za-z].*) \{$/class \1\n\{/g'

Level 1:
grep -nrl --exclude-dir=systemc \
"^class [A-Za-z].* {$" src/ | \
xargs sed -Ei \
's/^class ([A-Za-z].*) \{$/class \1\n\{/g'

and so on.

Change-Id: I17615ce16a333d69867b27c7bae0f4fdafd8b2eb
Signed-off-by: Daniel R. Carvalho 
---
M src/arch/arm/insts/sve.hh
M src/arch/arm/isa.hh
M src/arch/arm/pmu.hh
M src/arch/arm/table_walker.hh
M src/base/bitunion.test.cc
M src/cpu/exec_context.hh
M src/cpu/inst_res.hh
M src/cpu/o3/fu_pool.hh
M src/cpu/o3/inst_queue.hh
M src/cpu/o3/mem_dep_unit.hh
M src/cpu/o3/probe/simple_trace.hh
M src/cpu/pc_event.hh
M src/cpu/pred/multiperspective_perceptron.hh
M src/cpu/pred/multiperspective_perceptron_64KB.hh
M src/cpu/pred/multiperspective_perceptron_8KB.hh
M src/cpu/pred/multiperspective_perceptron_tage.hh
M src/cpu/pred/multiperspective_perceptron_tage_64KB.hh
M src/cpu/pred/tage_sc_l.hh
M src/cpu/pred/tage_sc_l_64KB.hh
M src/cpu/reg_class.hh
M src/cpu/simple/exec_context.hh
M src/cpu/testers/gpu_ruby_test/episode.hh
M src/cpu/trace/trace_cpu.hh
M src/dev/hsa/hsa_packet_processor.hh
M src/dev/storage/ide_disk.hh
M src/kern/operatingsystem.hh
M src/mem/abstract_mem.hh
M src/mem/cache/base.hh
M src/mem/cache/cache_blk.hh
M src/mem/cache/mshr.hh
M src/mem/cache/prefetch/associative_set.hh
M src/mem/cache/prefetch/base.hh
M src/mem/cache/queue_entry.hh
M src/mem/cache/write_queue_entry.hh
M src/mem/packet_queue.hh
M src/mem/ruby/system/CacheRecorder.hh
M src/mem/snoop_filter.hh
M src/sim/futex_map.hh
M src/sim/linear_solver.hh
M src/sim/mathexpr.hh
M src/sim/se_signal.hh
M src/sim/serialize.hh
M src/sim/syscall_emul_buf.hh
M src/unittest/stattest.cc
44 files changed, 236 insertions(+), 118 deletions(-)



diff --git a/src/arch/arm/insts/sve.hh b/src/arch/arm/insts/sve.hh
index c0fa29c..ce9ff98 100644
--- a/src/arch/arm/insts/sve.hh
+++ b/src/arch/arm/insts/sve.hh
@@ -53,7 +53,8 @@
 const char* svePredTypeToStr(SvePredType pt);

 /// Index generation instruction, immediate operands
-class SveIndexIIOp : public ArmStaticInst {
+class SveIndexIIOp : public ArmStaticInst
+{
   protected:
 IntRegIndex dest;
 int8_t imm1;
@@ -69,7 +70,8 @@
 Addr pc, const Loader::SymbolTable *symtab) const override;
 };

-class SveIndexIROp : public ArmStaticInst {
+class SveIndexIROp : public ArmStaticInst
+{
   protected:
 IntRegIndex dest;
 int8_t imm1;
@@ -85,7 +87,8 @@
 Addr pc, const Loader::SymbolTable *symtab) const override;
 };

-class SveIndexRIOp : public ArmStaticInst {
+class SveIndexRIOp : public ArmStaticInst
+{
   protected:
 IntRegIndex dest;
 IntRegIndex op1;
@@ -101,7 +104,8 @@
 Addr pc, const Loader::SymbolTable *symtab) const override;
 };

-class SveIndexRROp : public ArmStaticInst {
+class SveIndexRROp : public ArmStaticInst
+{
   protected:
 IntRegIndex dest;
 IntRegIndex op1;
@@ -118,7 +122,8 @@
 };

 // Predicate count SVE instruction.
-class SvePredCountOp : public ArmStaticInst {
+class SvePredCountOp : public ArmStaticInst
+{
   protected:
 IntRegIndex dest;
 IntRegIndex gp;
@@ -137,7 +142,8 @@
 };

 // Predicate count SVE instruction (predicated).
-class SvePredCountPredOp : public ArmStaticInst {
+class SvePredCountPredOp : public ArmStaticInst
+{
   protected:
 IntRegIndex dest;
 IntRegIndex op1;
@@ -154,7 +160,8 @@
 };

 /// While predicate generation SVE instruction.
-class SveWhileOp : public ArmStaticInst {
+class SveWhileOp : public ArmStaticInst
+{
   protected:
 IntRegIndex dest, op1, op2;
 bool srcIs32b;
@@ -170,7 +177,8 @@
 };

 /// Compare and terminate loop SVE instruction.
-class SveCompTermOp : public ArmStaticInst {
+class SveCompTermOp : public ArmStaticInst
+{
   protected:
 IntRegIndex op1, op2;

@@ -184,7 +192,8 @@
 };

 /// Unary, constructive, predicated (merging) SVE instruction.
-class SveUnaryPredOp : public ArmStaticInst {
+class SveUnaryPredOp : public ArmStaticInst
+{
   protected:
 IntRegIndex dest, op1, gp;

@@ -199,7 +208,8 @@
 };

 /// Unary, constructive, unpredicated SVE instruction.
-class SveUnaryUnpredOp : public ArmStaticInst {
+class SveUnaryUnpredOp : public ArmStaticInst
+{
   protected:
 IntRegIndex dest, op1;

@@ 

[gem5-dev] Change in gem5/gem5[develop]: util: Add verifier for opening braces of classes

[Daniel Carvalho (Gerrit) via gem5-dev]

Daniel Carvalho has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/39016 )



Change subject: util: Add verifier for opening braces of classes
..

util: Add verifier for opening braces of classes

Make sure that opening braces of classes are not declared
in the same line of the class name.

This does not work for multi-line classes.

Change-Id: I232df1a9ebd974b9f4f66e1d96d03b12513bd49f
Signed-off-by: Daniel R. Carvalho 
---
M util/style/verifiers.py
1 file changed, 27 insertions(+), 0 deletions(-)



diff --git a/util/style/verifiers.py b/util/style/verifiers.py
index 7d27fda..ddfc0fb 100644
--- a/util/style/verifiers.py
+++ b/util/style/verifiers.py
@@ -460,6 +460,33 @@
   "comparisons with false/False.\n")
 return line

+class ClassBraces(LineVerifier):
+""" Check if the opening braces of classes are not on the same line of
+the class name.
+
+@todo Make this work for multi-line class declarations. e.g.,
+
+class MultiLineClass
+  : public BaseClass {
+"""
+
+languages = set(('C', 'C++'))
+test_name = 'class opening brace position'
+opt_name = 'classbrace'
+
+regex = re.compile(r'\A(\s*)(class\s+[A-Z].*\S)\s*\{')
+
+def check_line(self, line, **kwargs):
+return self.regex.search(line) == None
+
+def fix_line(self, line, **kwargs):
+match = self.regex.search(line)
+if match:
+# Group 1 is indentation, group 2 is class declaration
+line = match.group(1) + match.group(2) + "\n" + \
+match.group(1) + "{"
+return line
+
 def is_verifier(cls):
 """Determine if a class is a Verifier that can be instantiated"""


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39016
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: I232df1a9ebd974b9f4f66e1d96d03b12513bd49f
Gerrit-Change-Number: 39016
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Carvalho 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Testing fail on GCN3_X86 on Arm machine

[Bobby Bruce via gem5-dev]

I'm guessing you're correct and this is because you're testing on an ARM
machine. GCN3_X86 should compile fine, though we don't test on ARM hosts
(we'd like to change this soon).

I've added a Jira ticket covering this bug:
https://gem5.atlassian.net/browse/GEM5-881.

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

web: https://www.bobbybruce.net

Please consider making a submission to GI@ICSE '21:
http://geneticimprovementofsoftware.com/gi2021icse.html


On Tue, Jan 12, 2021 at 12:44 AM Nathanael Premillieu via gem5-dev <
gem5-dev@gem5.org> wrote:

> Hi,
>
>
>
> Thanks Jason and Matt for your answers.
>
> I’m not really interested in GCN3, it’s just that it is tested by default
> when running all the tests.
>
> And as I’m working on a change that is not specific to a particular
> architecture, I wanted to test it on everything.
>
> For the moment I have relaunched the tests without the “GCN3_X86” isa.
>
> But I’ll also test on a x86 machine with everything.
>
>
>
> Thanks,
>
> Nathanael
>
>
>
> *From:* Jason Lowe-Power via gem5-dev [mailto:gem5-dev@gem5.org]
> *Sent:* Tuesday, January 12, 2021 12:17 AM
> *To:* gem5 Developer List 
> *Cc:* Dutu, Alexandru ; Poremba, Matthew <
> matthew.pore...@amd.com>; Kyle Roarty ; Jason
> Lowe-Power 
> *Subject:* [gem5-dev] Re: Testing fail on GCN3_X86 on Arm machine
>
>
>
> Hi all,
>
>
>
> It doesn't surprise me at all that GCN3 doesn't build on Arm. Nathanael,
> you should be able to ignore those errors. We don't test that combination
> of host-target and I have not heard of anyone trying to use that
> combination of host-target.
>
>
>
> If you're trying to use the GPU model, I'd suggest moving to an x86 host.
> Unfortunately, right now the GPU model is intricately tied to the host OS.
> Hence why the docker is all but required to get the GPU model working.
>
>
>
> Cheers,
>
> Jason
>
>
>
> On Mon, Jan 11, 2021 at 10:59 AM Matt Sinclair via gem5-dev <
> gem5-dev@gem5.org> wrote:
>
> Hi Nathanael,
>
>
>
> I have personally never tested GCN3 on an ARM machine, so I can't say
> definitively if it will work there or not.  But if you are not, I strongly
> recommend testing anything you need to test with GCN3 inside the docker we
> created:
>
>
>
> -
> https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/util/dockerfiles/gcn-gpu/README.md
>
> - http://www.gem5.org/2020/05/27/modern-gpu-applications.html
>
>
>
> Thanks,
>
> Matt
>
>
>
> On Mon, Jan 11, 2021 at 8:42 AM Nathanael Premillieu via gem5-dev <
> gem5-dev@gem5.org> wrote:
>
> Hi,
>
>
>
> I’m currently working on some changes in gem5 and before submitting them,
> I’m using the testing framework as required.
>
> However, even without my changes, I get failures when building GCN3_X86
> (See the trace at the end).
>
> I guess it is linked to the fact that I’m working on an Arm machine. Do I
> need to set up something to make it work?
>
>
>
> Thanks,
>
> Nathanael Premillieu
>
>
>
> The trace:
>
> /scratch/npremill/gem5_public.git/build/GCN3_X86/gem5.opt
>
> You may want to run with only a single ISA(--isa=), use --skip-build, or
> use 'rerun'.
>
> MOESI_AMD_Base-dir.sm:220: Warning: Non-void return ignored, return type
> is 'bool'
>
> MOESI_AMD_Base-dir.sm:1052: Warning: Non-void return ignored, return type
> is 'Tick'
>
> MOESI_AMD_Base-dir.sm:1056: Warning: Non-void return ignored, return type
> is 'Tick'
>
> MOESI_AMD_Base-dir.sm:1060: Warning: Non-void return ignored, return type
> is 'Tick'
>
> MOESI_AMD_Base-dir.sm:1064: Warning: Non-void return ignored, return type
> is 'Tick'
>
> MOESI_AMD_Base-dir.sm:1068: Warning: Non-void return ignored, return type
> is 'Tick'
>
> MOESI_AMD_Base-dir.sm:1072: Warning: Non-void return ignored, return type
> is 'Tick'
>
> MOESI_AMD_Base-dir.sm:1076: Warning: Non-void return ignored, return type
> is 'Tick'
>
> MOESI_AMD_Base-dir.sm:586: Warning: Unused action: l_queueMemWBReq, Write
> WB data to memory
>
> MOESI_AMD_Base-dir.sm:953: Warning: Unused action:
> mwc_markSinkWriteCancel, Mark to sink impending VicDirty
>
> MOESI_AMD_Base-dir.sm:1051: Warning: Unused action: dl_deallocateL3,
> deallocate the L3 block
>
> MOESI_AMD_Base-dir.sm:1087: Warning: Unused action:
> yy_recycleResponseQueue, recycle response queue
>
> MOESI_AMD_Base-dma.sm:187: Warning: Non-void return ignored, return type
> is 'Tick'
>
> MOESI_AMD_Base-dma.sm:191: Warning: Non-void return ignored, return type
> is 'Tick'
>
> MOESI_AMD_Base-CorePair.sm:325: Warning: Non-void return ignored, return
> type is 'bool'
>
> MOESI_AMD_Base-CorePair.sm:802: Warning: Non-void return ignored, return
> type is 'Tick'
>
> MOESI_AMD_Base-CorePair.sm:806: Warning: Non-void return ignored, return
> type is 'Tick'
>
> MOESI_AMD_Base-CorePair.sm:810: Warning: Non-void return ignored, return
> type is 'Tick'
>
> MOESI_AMD_Base-CorePair.sm:814: Warning: Non-void return ignored, return
> type is 'Tick'
>
> GPU_VIPER-TCP.sm:166: Warning: Non-void return ignored, return type is
> 'bool'
>
> GPU_VI

[gem5-dev] Change in gem5/gem5[develop]: base: Remove begin() and end() from CircleBuf.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: base: Remove begin() and end() from CircleBuf.
..

base: Remove begin() and end() from CircleBuf.

These functions return iterators which are inconsistent with the usage
model for this type. It should be accessed using the peek, push, and pop
methods and not iterators. If you need a class with iterators which is
oriented around accessing individual elements at a time, the
CircularQueue type is likely a better choice.

Change-Id: I9f37eab12e490b63d870d378a91f601dad353f25
---
M src/base/circlebuf.hh
1 file changed, 0 insertions(+), 11 deletions(-)



diff --git a/src/base/circlebuf.hh b/src/base/circlebuf.hh
index 0f1cea6..bcfa91a 100644
--- a/src/base/circlebuf.hh
+++ b/src/base/circlebuf.hh
@@ -62,8 +62,6 @@

   public:
 using value_type = T;
-using iterator = typename std::vector::iterator;
-using const_iterator = typename std::vector::const_iterator;

 explicit CircleBuf(size_t size) : buffer(size), maxSize(size) {}

@@ -71,15 +69,6 @@
 size_t size() const { return used; }
 size_t capacity() const { return maxSize; }

-iterator begin() { return buffer.begin() + start % maxSize; }
-const_iterator begin() const { return buffer.begin() + start %  
maxSize; }

-iterator end() { return buffer.begin() + (start + used) % maxSize; }
-const_iterator
-end() const
-{
-return buffer.begin() + (start + used) % maxSize;
-}
-
 /**
  * Throw away any data in the buffer.
  */

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38998
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: I9f37eab12e490b63d870d378a91f601dad353f25
Gerrit-Change-Number: 38998
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] Re: Testing fail on GCN3_X86 on Arm machine

[Nathanael Premillieu via gem5-dev]

Hi,

Thanks Jason and Matt for your answers.
I’m not really interested in GCN3, it’s just that it is tested by default when 
running all the tests.
And as I’m working on a change that is not specific to a particular 
architecture, I wanted to test it on everything.
For the moment I have relaunched the tests without the “GCN3_X86” isa.
But I’ll also test on a x86 machine with everything.

Thanks,
Nathanael

From: Jason Lowe-Power via gem5-dev [mailto:gem5-dev@gem5.org]
Sent: Tuesday, January 12, 2021 12:17 AM
To: gem5 Developer List 
Cc: Dutu, Alexandru ; Poremba, Matthew 
; Kyle Roarty ; Jason Lowe-Power 

Subject: [gem5-dev] Re: Testing fail on GCN3_X86 on Arm machine

Hi all,

It doesn't surprise me at all that GCN3 doesn't build on Arm. Nathanael, you 
should be able to ignore those errors. We don't test that combination of 
host-target and I have not heard of anyone trying to use that combination of 
host-target.

If you're trying to use the GPU model, I'd suggest moving to an x86 host. 
Unfortunately, right now the GPU model is intricately tied to the host OS. 
Hence why the docker is all but required to get the GPU model working.

Cheers,
Jason

On Mon, Jan 11, 2021 at 10:59 AM Matt Sinclair via gem5-dev 
mailto:gem5-dev@gem5.org>> wrote:
Hi Nathanael,

I have personally never tested GCN3 on an ARM machine, so I can't say 
definitively if it will work there or not.  But if you are not, I strongly 
recommend testing anything you need to test with GCN3 inside the docker we 
created:

- 
https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/util/dockerfiles/gcn-gpu/README.md
- http://www.gem5.org/2020/05/27/modern-gpu-applications.html

Thanks,
Matt

On Mon, Jan 11, 2021 at 8:42 AM Nathanael Premillieu via gem5-dev 
mailto:gem5-dev@gem5.org>> wrote:
Hi,

I’m currently working on some changes in gem5 and before submitting them, I’m 
using the testing framework as required.
However, even without my changes, I get failures when building GCN3_X86 (See 
the trace at the end).
I guess it is linked to the fact that I’m working on an Arm machine. Do I need 
to set up something to make it work?

Thanks,
Nathanael Premillieu

The trace:
/scratch/npremill/gem5_public.git/build/GCN3_X86/gem5.opt
You may want to run with only a single ISA(--isa=), use --skip-build, or use 
'rerun'.
MOESI_AMD_Base-dir.sm:220: Warning: Non-void return ignored, return type is 
'bool'
MOESI_AMD_Base-dir.sm:1052: Warning: Non-void return ignored, return type is 
'Tick'
MOESI_AMD_Base-dir.sm:1056: Warning: Non-void return ignored, return type is 
'Tick'
MOESI_AMD_Base-dir.sm:1060: Warning: Non-void return ignored, return type is 
'Tick'
MOESI_AMD_Base-dir.sm:1064: Warning: Non-void return ignored, return type is 
'Tick'
MOESI_AMD_Base-dir.sm:1068: Warning: Non-void return ignored, return type is 
'Tick'
MOESI_AMD_Base-dir.sm:1072: Warning: Non-void return ignored, return type is 
'Tick'
MOESI_AMD_Base-dir.sm:1076: Warning: Non-void return ignored, return type is 
'Tick'
MOESI_AMD_Base-dir.sm:586: Warning: Unused action: l_queueMemWBReq, Write WB 
data to memory
MOESI_AMD_Base-dir.sm:953: Warning: Unused action: mwc_markSinkWriteCancel, 
Mark to sink impending VicDirty
MOESI_AMD_Base-dir.sm:1051: Warning: Unused action: dl_deallocateL3, deallocate 
the L3 block
MOESI_AMD_Base-dir.sm:1087: Warning: Unused action: yy_recycleResponseQueue, 
recycle response queue
MOESI_AMD_Base-dma.sm:187: Warning: Non-void return ignored, return type is 
'Tick'
MOESI_AMD_Base-dma.sm:191: Warning: Non-void return ignored, return type is 
'Tick'
MOESI_AMD_Base-CorePair.sm:325: Warning: Non-void return ignored, return type 
is 'bool'
MOESI_AMD_Base-CorePair.sm:802: Warning: Non-void return ignored, return type 
is 'Tick'
MOESI_AMD_Base-CorePair.sm:806: Warning: Non-void return ignored, return type 
is 'Tick'
MOESI_AMD_Base-CorePair.sm:810: Warning: Non-void return ignored, return type 
is 'Tick'
MOESI_AMD_Base-CorePair.sm:814: Warning: Non-void return ignored, return type 
is 'Tick'
GPU_VIPER-TCP.sm:166: Warning: Non-void return ignored, return type is 'bool'
GPU_VIPER-TCP.sm:451: Warning: Non-void return ignored, return type is 'Tick'
GPU_VIPER-TCP.sm:455: Warning: Non-void return ignored, return type is 'Tick'
GPU_VIPER-TCP.sm:385: Warning: Unused action: norl_issueRdBlkOrloadDone, local 
load done
GPU_VIPER-SQC.sm:143: Warning: Non-void return ignored, return type is 'bool'
GPU_VIPER-SQC.sm:275: Warning: Non-void return ignored, return type is 'Tick'
GPU_VIPER-SQC.sm:279: Warning: Non-void return ignored, return type is 'Tick'
GPU_VIPER-TCC.sm:168: Warning: Non-void return ignored, return type is 'bool'
GPU_VIPER-TCC.sm:551: Warning: Non-void return ignored, return type is 'Tick'
GPU_VIPER-TCC.sm:555: Warning: Non-void return ignored, return type is 'Tick'
GPU_VIPER-TCC.sm:559: Warning: Non-void return ignored, return type is 'Tick'
GPU_VIPER-TCC.sm:583: Warning: Non-void return ignored, return type is 'Tick'
MOESI_AMD_Base-L3cache.sm:196: Warning: 

[gem5-dev] Change in gem5/gem5[develop]: sim: Break the eventq.hh dependency in core.hh.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: sim: Break the eventq.hh dependency in core.hh.
..

sim: Break the eventq.hh dependency in core.hh.

The original implementation of curTick used a thread local variable,
_curEventQueue, and its getCurTick() method, to retrieve the curTick for
the currently active event queue. That meant that core.hh needed to
include eventq.hh so that the EventQueue type was available, which also
indirectly brought in a lot of other dependencies.

Unfortunately this couldn't easily be fixed by making curTick()
non-inline since this added a significant amount of overhead when
tested.

Instead, this change makes the code in core.hh/core.cc keep a pointer
directly to a Tick. The code which sets _curEventQueue now also sets
that pointer when _curEventQueue changes.

The way curTick() now reaches into the guts of the current EventQueue
directly is not great from a modularity perspective, but if curTick is
considered an extension of the EventQueue, then it's just odd that this
part is broken out into a different file.

Change-Id: I8341b40fe75e90672eb1d70e1a368975fcbfe926
---
M src/sim/core.cc
M src/sim/core.hh
M src/sim/eventq.hh
3 files changed, 27 insertions(+), 3 deletions(-)



diff --git a/src/sim/core.cc b/src/sim/core.cc
index 8b36245..ace699a 100644
--- a/src/sim/core.cc
+++ b/src/sim/core.cc
@@ -41,6 +41,13 @@

 using namespace std;

+namespace Gem5Internal
+{
+
+__thread Tick *_curTickPtr;
+
+} // namespace Gem5Internal
+
 namespace SimClock {
 /// The simulated frequency of curTick(). (In ticks per second)
 Tick Frequency;
diff --git a/src/sim/core.hh b/src/sim/core.hh
index 2e443e7..c592049 100644
--- a/src/sim/core.hh
+++ b/src/sim/core.hh
@@ -39,10 +39,17 @@
 #include 

 #include "base/types.hh"
-#include "sim/eventq.hh"
+
+namespace Gem5Internal
+{
+
+// This pointer is maintained by curEventQueue in src/sim/eventq.hh.
+extern __thread Tick *_curTickPtr;
+
+} // namespace Gem5Internal

 /// The universal simulation clock.
-inline Tick curTick() { return _curEventQueue->getCurTick(); }
+inline Tick curTick() { return *Gem5Internal::_curTickPtr; }

 /// These are variables that are set based on the simulator frequency
 ///@{
diff --git a/src/sim/eventq.hh b/src/sim/eventq.hh
index 45a5ab8..3ef3f56 100644
--- a/src/sim/eventq.hh
+++ b/src/sim/eventq.hh
@@ -48,6 +48,7 @@
 #include "base/types.hh"
 #include "base/uncontended_mutex.hh"
 #include "debug/Event.hh"
+#include "sim/core.hh"
 #include "sim/serialize.hh"

 class EventQueue;   // forward declaration
@@ -81,7 +82,7 @@
 EventQueue *getEventQueue(uint32_t index);

 inline EventQueue *curEventQueue() { return _curEventQueue; }
-inline void curEventQueue(EventQueue *q) { _curEventQueue = q; }
+inline void curEventQueue(EventQueue *q);

 /**
  * Common base class for Event and GlobalEvent, so they can share flag
@@ -617,6 +618,8 @@
 class EventQueue
 {
   private:
+friend void curEventQueue(EventQueue *);
+
 std::string objName;
 Event *head;
 Tick _curTick;
@@ -968,6 +971,13 @@
 }
 };

+inline void
+curEventQueue(EventQueue *q)
+{
+_curEventQueue = q;
+Gem5Internal::_curTickPtr = q ? &q->_curTick : nullptr;
+}
+
 void dumpMainQueue();

 class EventManager

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38996
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: I8341b40fe75e90672eb1d70e1a368975fcbfe926
Gerrit-Change-Number: 38996
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: Remove the curTick prototype from base/statistics.hh.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: base: Remove the curTick prototype from base/statistics.hh.
..

base: Remove the curTick prototype from base/statistics.hh.

This prototype might convince the compiler that it should refer to
curTick indirectly through the linker, but curTick is inline (and making
it not has very high overhead), so there's a decent chance no non-inline
version will be emitted.

Change-Id: Iab5aacb145d4a974bc1bc0abdf7275c40fbb9c38
---
M src/base/statistics.hh
1 file changed, 2 insertions(+), 3 deletions(-)



diff --git a/src/base/statistics.hh b/src/base/statistics.hh
index 1ad64e9..7115b88 100644
--- a/src/base/statistics.hh
+++ b/src/base/statistics.hh
@@ -81,9 +81,8 @@
 #include "base/intmath.hh"
 #include "base/str.hh"
 #include "base/types.hh"
-
-/** The current simulated tick. */
-extern Tick curTick();
+// For curTick().
+#include "sim/core.hh"

 /* A namespace for all of the Statistics */
 namespace Stats {

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38997
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: Iab5aacb145d4a974bc1bc0abdf7275c40fbb9c38
Gerrit-Change-Number: 38997
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