Hi Nilay,
As part of our check-ins tomorrow, I would like to check in a single line bug
fix to this patch. Specifically we plant to remove the assert in the overload
function of the MsgPtr operator>. It is perfectly valid to compare the same
MsgPtr to itself. Currently the assert prevents such a comparison.
I don't expect this to be much of an issue, but I wanted you to be aware.
Thanks,
Brad
Specific patch:
diff --git a/src/mem/ruby/slicc_interface/Message.hh
b/src/mem/ruby/slicc_interface/Message.hh
--- a/src/mem/ruby/slicc_interface/Message.hh
+++ b/src/mem/ruby/slicc_interface/Message.hh
@@ -121,7 +121,6 @@
const Message *r = rhs.get();
if (l->getLastEnqueueTime() == r->getLastEnqueueTime()) {
- assert(l->getMsgCounter() != r->getMsgCounter());
return l->getMsgCounter() > r->getMsgCounter();
}
return l->getLastEnqueueTime() > r->getLastEnqueueTime();
-----Original Message-----
From: gem5-dev [mailto:[email protected]] On Behalf Of Nilay Vaish
Sent: Saturday, July 04, 2015 8:44 AM
To: [email protected]
Subject: [gem5-dev] changeset in gem5: ruby: remove message buffer node
changeset f567e80c0714 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=f567e80c0714
description:
ruby: remove message buffer node
This structure's only purpose was to provide a comparison function for
ordering messages in the MessageBuffer. The comparison function is now
being moved to the Message class itself. So we no longer require this
structure.
diffstat:
src/mem/ruby/network/MessageBuffer.cc | 43 ++++++++---------
src/mem/ruby/network/MessageBuffer.hh | 11 ++--
src/mem/ruby/network/MessageBufferNode.cc | 39 ----------------
src/mem/ruby/network/MessageBufferNode.hh | 75 -------------------------------
src/mem/ruby/network/SConscript | 1 -
src/mem/ruby/slicc_interface/Message.hh | 26 ++++++++--
src/mem/ruby/structures/WireBuffer.cc | 33 ++++--------
src/mem/ruby/structures/WireBuffer.hh | 5 +-
8 files changed, 60 insertions(+), 173 deletions(-)
diffs (truncated from 477 to 300 lines):
diff -r bd37e25fb3b7 -r f567e80c0714 src/mem/ruby/network/MessageBuffer.cc
--- a/src/mem/ruby/network/MessageBuffer.cc Fri Jul 03 10:15:03 2015 -0400
+++ b/src/mem/ruby/network/MessageBuffer.cc Sat Jul 04 10:43:46 2015 -0500
@@ -122,7 +122,7 @@
DPRINTF(RubyQueue, "Peeking at head of queue.\n");
assert(isReady());
- const Message* msg_ptr = m_prio_heap.front().m_msgptr.get();
+ const Message* msg_ptr = m_prio_heap.front().get();
assert(msg_ptr);
DPRINTF(RubyQueue, "Message: %s\n", (*msg_ptr)); @@ -204,12 +204,11 @@
msg_ptr->updateDelayedTicks(m_sender->clockEdge());
msg_ptr->setLastEnqueueTime(arrival_time);
+ msg_ptr->setMsgCounter(m_msg_counter);
// Insert the message into the priority heap
- MessageBufferNode thisNode(arrival_time, m_msg_counter, message);
- m_prio_heap.push_back(thisNode);
- push_heap(m_prio_heap.begin(), m_prio_heap.end(),
- greater<MessageBufferNode>());
+ m_prio_heap.push_back(message);
+ push_heap(m_prio_heap.begin(), m_prio_heap.end(),
+ greater<MsgPtr>());
DPRINTF(RubyQueue, "Enqueue arrival_time: %lld, Message: %s\n",
arrival_time, *(message.get())); @@ -227,7 +226,7 @@
assert(isReady());
// get MsgPtr of the message about to be dequeued
- MsgPtr message = m_prio_heap.front().m_msgptr;
+ MsgPtr message = m_prio_heap.front();
// get the delay cycles
message->updateDelayedTicks(m_receiver->clockEdge());
@@ -242,7 +241,7 @@
}
pop_heap(m_prio_heap.begin(), m_prio_heap.end(),
- greater<MessageBufferNode>());
+ greater<MsgPtr>());
m_prio_heap.pop_back();
return delayCycles;
@@ -265,14 +264,12 @@
{
DPRINTF(RubyQueue, "Recycling.\n");
assert(isReady());
- MessageBufferNode node = m_prio_heap.front();
- pop_heap(m_prio_heap.begin(), m_prio_heap.end(),
- greater<MessageBufferNode>());
+ MsgPtr node = m_prio_heap.front();
+ pop_heap(m_prio_heap.begin(), m_prio_heap.end(),
+ greater<MsgPtr>());
- node.m_time = m_receiver->clockEdge(m_recycle_latency);
+ node->setLastEnqueueTime(m_receiver->clockEdge(m_recycle_latency));
m_prio_heap.back() = node;
- push_heap(m_prio_heap.begin(), m_prio_heap.end(),
- greater<MessageBufferNode>());
+ push_heap(m_prio_heap.begin(), m_prio_heap.end(),
+ greater<MsgPtr>());
m_consumer->
scheduleEventAbsolute(m_receiver->clockEdge(m_recycle_latency));
}
@@ -282,11 +279,13 @@
{
while(!lt.empty()) {
m_msg_counter++;
- MessageBufferNode msgNode(nextTick, m_msg_counter, lt.front());
+ MsgPtr m = lt.front();
+ m->setLastEnqueueTime(nextTick);
+ m->setMsgCounter(m_msg_counter);
- m_prio_heap.push_back(msgNode);
+ m_prio_heap.push_back(m);
push_heap(m_prio_heap.begin(), m_prio_heap.end(),
- greater<MessageBufferNode>());
+ greater<MsgPtr>());
m_consumer->scheduleEventAbsolute(nextTick);
lt.pop_front();
@@ -331,7 +330,7 @@
DPRINTF(RubyQueue, "Stalling due to %s\n", addr);
assert(isReady());
assert(addr.getOffset() == 0);
- MsgPtr message = m_prio_heap.front().m_msgptr;
+ MsgPtr message = m_prio_heap.front();
dequeue();
@@ -351,8 +350,8 @@
ccprintf(out, " consumer-yes ");
}
- vector<MessageBufferNode> copy(m_prio_heap);
- sort_heap(copy.begin(), copy.end(), greater<MessageBufferNode>());
+ vector<MsgPtr> copy(m_prio_heap);
+ sort_heap(copy.begin(), copy.end(), greater<MsgPtr>());
ccprintf(out, "%s] %s", copy, m_name); }
@@ -360,7 +359,7 @@
MessageBuffer::isReady() const
{
return ((m_prio_heap.size() > 0) &&
- (m_prio_heap.front().m_time <= m_receiver->clockEdge()));
+ (m_prio_heap.front()->getLastEnqueueTime() <=
+ m_receiver->clockEdge()));
}
bool
@@ -369,7 +368,7 @@
// Check the priority heap and read any messages that may
// correspond to the address in the packet.
for (unsigned int i = 0; i < m_prio_heap.size(); ++i) {
- Message *msg = m_prio_heap[i].m_msgptr.get();
+ Message *msg = m_prio_heap[i].get();
if (msg->functionalRead(pkt)) return true;
}
@@ -397,7 +396,7 @@
// Check the priority heap and write any messages that may
// correspond to the address in the packet.
for (unsigned int i = 0; i < m_prio_heap.size(); ++i) {
- Message *msg = m_prio_heap[i].m_msgptr.get();
+ Message *msg = m_prio_heap[i].get();
if (msg->functionalWrite(pkt)) {
num_functional_writes++;
}
diff -r bd37e25fb3b7 -r f567e80c0714 src/mem/ruby/network/MessageBuffer.hh
--- a/src/mem/ruby/network/MessageBuffer.hh Fri Jul 03 10:15:03 2015 -0400
+++ b/src/mem/ruby/network/MessageBuffer.hh Sat Jul 04 10:43:46 2015 -0500
@@ -43,7 +43,6 @@
#include "mem/ruby/common/Address.hh"
#include "mem/ruby/common/Consumer.hh"
-#include "mem/ruby/network/MessageBufferNode.hh"
#include "mem/ruby/slicc_interface/Message.hh"
#include "mem/packet.hh"
@@ -67,11 +66,11 @@
void
delayHead()
{
- MessageBufferNode node = m_prio_heap.front();
+ MsgPtr m = m_prio_heap.front();
std::pop_heap(m_prio_heap.begin(), m_prio_heap.end(),
- std::greater<MessageBufferNode>());
+ std::greater<MsgPtr>());
m_prio_heap.pop_back();
- enqueue(node.m_msgptr, Cycles(1));
+ enqueue(m, Cycles(1));
}
bool areNSlotsAvailable(unsigned int n); @@ -112,7 +111,7 @@
peekMsgPtr() const
{
assert(isReady());
- return m_prio_heap.front().m_msgptr;
+ return m_prio_heap.front();
}
void enqueue(MsgPtr message) { enqueue(message, Cycles(1)); } @@ -168,7
+167,7 @@
//! Consumer to signal a wakeup(), can be NULL
Consumer* m_consumer;
- std::vector<MessageBufferNode> m_prio_heap;
+ std::vector<MsgPtr> m_prio_heap;
// use a std::map for the stalled messages as this container is
// sorted and ensures a well-defined iteration order diff -r bd37e25fb3b7
-r f567e80c0714 src/mem/ruby/network/MessageBufferNode.cc
--- a/src/mem/ruby/network/MessageBufferNode.cc Fri Jul 03 10:15:03 2015 -0400
+++ /dev/null Thu Jan 01 00:00:00 1970 +0000
@@ -1,39 +0,0 @@
-/*
- * Copyright (c) 1999-2008 Mark D. Hill and David A. Wood
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met: redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer;
- * redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution;
- * neither the name of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include "mem/ruby/network/MessageBufferNode.hh"
-
-void
-MessageBufferNode::print(std::ostream& out) const -{
- out << "[";
- out << m_time << ", ";
- out << m_msg_counter << ", ";
- out << *m_msgptr << "; ";
- out << "]";
-}
diff -r bd37e25fb3b7 -r f567e80c0714 src/mem/ruby/network/MessageBufferNode.hh
--- a/src/mem/ruby/network/MessageBufferNode.hh Fri Jul 03 10:15:03 2015 -0400
+++ /dev/null Thu Jan 01 00:00:00 1970 +0000
@@ -1,75 +0,0 @@
-/*
- * Copyright (c) 1999-2008 Mark D. Hill and David A. Wood
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met: redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer;
- * redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution;
- * neither the name of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef __MEM_RUBY_BUFFERS_MESSAGEBUFFERNODE_HH__
-#define __MEM_RUBY_BUFFERS_MESSAGEBUFFERNODE_HH__
-
-#include <iostream>
-
-#include "mem/ruby/slicc_interface/Message.hh"
-
-class MessageBufferNode
-{
- public:
- MessageBufferNode()
- : m_time(0), m_msg_counter(0)
- {}
-
- MessageBufferNode(const Tick time, uint64_t counter,
- const MsgPtr& msgptr)
- : m_time(time), m_msg_counter(counter), m_msgptr(msgptr)
- {}
-
- void print(std::ostream& out) const;
-
- public:
- Tick m_time;
- uint64_t m_msg_counter; // FIXME, should this be a 64-bit value?
- MsgPtr m_msgptr;
-};
-
-inline bool
-operator>(const MessageBufferNode& n1, const MessageBufferNode& n2)
-{
- if (n1.m_time == n2.m_time) {
- assert(n1.m_msg_counter != n2.m_msg_counter);
- return n1.m_msg_counter > n2.m_msg_counter;
- } else {
- return n1.m_time > n2.m_time;
- }
-}
-
-inline std::ostream&
-operator<<(std::ostream& out, const MessageBufferNode& obj) -{
- obj.print(out);
- out << std::flush;
- return out;
-}
-
-#endif // __MEM_RUBY_BUFFERS_MESSAGEBUFFERNODE_HH__
diff -r bd37e25fb3b7 -r f567e80c0714 src/mem/ruby/network/SConscript
--- a/src/mem/ruby/network/SConscript Fri Jul 03 10:15:03 2015 -0400
+++ b/src/mem/ruby/network/SConscript Sat Jul 04 10:43:46 2015 -0500
@@ -40,6 +40,5 @@
Source('BasicLink.cc')
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
