Hi Joel,

Look at where the directories are instantiated in configs/Ruby.py. they
should be children of ruby system and not system. I thought they already
were otherwise I would have fixed it as part of an earlier patch committed
around the same time .

Malek
On Jan 21, 2013 4:42 PM, "Joel Hestness" <[email protected]> wrote:

> Hey Nilay,
>   I've tried updating to the latest gem5, and I've found a bug, which looks
> to be related to this changeset.  Specifically, in the case that not all
> Ruby components are clocked at the same frequency, the assertion in
> MessageBuffer::enqueue (src/mem/ruby/buffers/MessageBuffer.cc:213) can fail
> if the m_LastEnqueueTime of a message was previously set by a component
> with a faster clock.  Attached is the config file and debug output with
> trace showing what happens in my simulation.  Note that the Ruby network
> components have clock=500 in the config.ini, while the directory has
> clock=1000.
>
>   Any ideas how to fix this?
>
>   Thanks,
>   Joel
>
>
>
> On Mon, Jan 14, 2013 at 10:18 AM, Nilay Vaish <[email protected]> wrote:
>
> > changeset 4ae4f3f4b870 in /z/repo/gem5
> > details: http://repo.gem5.org/gem5?cmd=changeset;node=4ae4f3f4b870
> > description:
> >         Ruby: use ClockedObject in Consumer class
> >         Many Ruby structures inherit from the Consumer, which is used for
> > scheduling
> >         events. The Consumer used to relay on an Event Manager for
> > scheduling events
> >         and on g_system_ptr for time. With this patch, the Consumer will
> > now use a
> >         ClockedObject to schedule events and to query for current time.
> > This resulted
> >         in several structures being converted from SimObjects to
> > ClockedObjects. Also,
> >         the MessageBuffer class now requires a pointer to a ClockedObject
> > so as to
> >         query for time.
> >
> > diffstat:
> >
> >  src/mem/protocol/RubySlicc_Profiler.sm                            |   2
> +-
> >  src/mem/protocol/RubySlicc_Types.sm                               |   3
> +-
> >  src/mem/ruby/buffers/MessageBuffer.cc                             |  85
> > ++++-----
> >  src/mem/ruby/buffers/MessageBuffer.hh                             |  26
> > ++-
> >  src/mem/ruby/common/Consumer.cc                                   |   8
> +-
> >  src/mem/ruby/common/Consumer.hh                                   |   6
> +-
> >  src/mem/ruby/network/BasicRouter.cc                               |   2
> +-
> >  src/mem/ruby/network/BasicRouter.hh                               |   4
> +-
> >  src/mem/ruby/network/BasicRouter.py                               |   4
> +-
> >  src/mem/ruby/network/Network.cc                                   |   2
> +-
> >  src/mem/ruby/network/Network.hh                                   |   4
> +-
> >  src/mem/ruby/network/Network.py                                   |   3
> +-
> >  src/mem/ruby/network/garnet/fixed-pipeline/GarnetLink_d.py        |   4
> +-
> >  src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc  |   1
> +
> >  src/mem/ruby/network/garnet/fixed-pipeline/NetworkLink_d.cc       |   2
> +-
> >  src/mem/ruby/network/garnet/fixed-pipeline/NetworkLink_d.hh       |   4
> +-
> >  src/mem/ruby/network/garnet/flexible-pipeline/FlexibleConsumer.hh |   2
> +-
> >  src/mem/ruby/network/garnet/flexible-pipeline/GarnetLink.py       |   4
> +-
> >  src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc |   1
> +
> >  src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.cc      |   2
> +-
> >  src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.hh      |   4
> +-
> >  src/mem/ruby/network/simple/PerfectSwitch.cc                      |   4
> +-
> >  src/mem/ruby/network/simple/PerfectSwitch.hh                      |   2
> +-
> >  src/mem/ruby/network/simple/Switch.cc                             |   4
> +-
> >  src/mem/ruby/network/simple/Throttle.cc                           |  13
> +-
> >  src/mem/ruby/network/simple/Throttle.hh                           |   9
> +-
> >  src/mem/ruby/profiler/Profiler.cc                                 |   4
> +-
> >  src/mem/ruby/profiler/Profiler.hh                                 |   2
> +-
> >  src/mem/ruby/slicc_interface/AbstractController.cc                |   2
> +-
> >  src/mem/ruby/slicc_interface/AbstractController.hh                |   4
> +-
> >  src/mem/ruby/slicc_interface/Controller.py                        |   4
> +-
> >  src/mem/ruby/slicc_interface/Message.hh                           |   9
> +-
> >  src/mem/ruby/slicc_interface/RubySlicc_Profiler_interface.cc      |   5
> +-
> >  src/mem/ruby/slicc_interface/RubySlicc_Profiler_interface.hh      |   2
> +-
> >  src/mem/ruby/system/MemoryControl.hh                              |   2
> +
> >  src/mem/ruby/system/Sequencer.cc                                  |  30
> > +--
> >  src/mem/ruby/system/TimerTable.cc                                 |   6
> +-
> >  src/mem/ruby/system/TimerTable.hh                                 |  13
> +-
> >  src/mem/slicc/ast/AST.py                                          |   2
> +-
> >  src/mem/slicc/symbols/StateMachine.py                             |  17
> +-
> >  40 files changed, 155 insertions(+), 152 deletions(-)
> >
> > diffs (truncated from 1088 to 300 lines):
> >
> > diff -r 794711cac18b -r 4ae4f3f4b870
> src/mem/protocol/RubySlicc_Profiler.sm
> > --- a/src/mem/protocol/RubySlicc_Profiler.sm    Mon Jan 14 10:23:56 2013
> > -0500
> > +++ b/src/mem/protocol/RubySlicc_Profiler.sm    Mon Jan 14 10:04:21 2013
> > -0600
> > @@ -47,4 +47,4 @@
> >  void profile_average_latency_estimate(int latency);
> >
> >  // profile the total message delay of a message across a virtual network
> > -void profileMsgDelay(int virtualNetwork, int delayCycles);
> > +void profileMsgDelay(int virtualNetwork, Time delayCycles);
> > diff -r 794711cac18b -r 4ae4f3f4b870 src/mem/protocol/RubySlicc_Types.sm
> > --- a/src/mem/protocol/RubySlicc_Types.sm       Mon Jan 14 10:23:56 2013
> > -0500
> > +++ b/src/mem/protocol/RubySlicc_Types.sm       Mon Jan 14 10:04:21 2013
> > -0600
> > @@ -36,13 +36,12 @@
> >  //
> >
> >  external_type(MessageBuffer, buffer="yes", inport="yes", outport="yes");
> > -
> >  external_type(OutPort, primitive="yes");
> >
> >  structure(InPort, external = "yes", primitive="yes") {
> >    bool isReady();
> >    void dequeue();
> > -  int dequeue_getDelayCycles();
> > +  Time dequeue_getDelayCycles();
> >    void recycle();
> >    bool isEmpty();
> >  }
> > diff -r 794711cac18b -r 4ae4f3f4b870
> src/mem/ruby/buffers/MessageBuffer.cc
> > --- a/src/mem/ruby/buffers/MessageBuffer.cc     Mon Jan 14 10:23:56 2013
> > -0500
> > +++ b/src/mem/ruby/buffers/MessageBuffer.cc     Mon Jan 14 10:04:21 2013
> > -0600
> > @@ -42,6 +42,8 @@
> >  {
> >      m_msg_counter = 0;
> >      m_consumer_ptr = NULL;
> > +    m_clockobj_ptr = NULL;
> > +
> >      m_ordering_set = false;
> >      m_strict_fifo = true;
> >      m_size = 0;
> > @@ -66,10 +68,10 @@
> >  int
> >  MessageBuffer::getSize()
> >  {
> > -    if (m_time_last_time_size_checked == g_system_ptr->getTime()) {
> > +    if (m_time_last_time_size_checked == m_clockobj_ptr->curCycle()) {
> >          return m_size_last_time_size_checked;
> >      } else {
> > -        m_time_last_time_size_checked = g_system_ptr->getTime();
> > +        m_time_last_time_size_checked = m_clockobj_ptr->curCycle();
> >          m_size_last_time_size_checked = m_size;
> >          return m_size;
> >      }
> > @@ -89,11 +91,11 @@
> >      // until next cycle, but enqueue operations effect the visible
> >      // size immediately
> >      int current_size = max(m_size_at_cycle_start, m_size);
> > -    if (m_time_last_time_pop < g_system_ptr->getTime()) {
> > +    if (m_time_last_time_pop < m_clockobj_ptr->curCycle()) {
> >          // no pops this cycle - m_size is correct
> >          current_size = m_size;
> >      } else {
> > -        if (m_time_last_time_enqueue < g_system_ptr->getTime()) {
> > +        if (m_time_last_time_enqueue < m_clockobj_ptr->curCycle()) {
> >              // no enqueues this cycle - m_size_at_cycle_start is correct
> >              current_size = m_size_at_cycle_start;
> >          } else {
> > @@ -155,9 +157,9 @@
> >      m_size++;
> >
> >      // record current time incase we have a pop that also adjusts my
> size
> > -    if (m_time_last_time_enqueue < g_system_ptr->getTime()) {
> > +    if (m_time_last_time_enqueue < m_clockobj_ptr->curCycle()) {
> >          m_msgs_this_cycle = 0;  // first msg this cycle
> > -        m_time_last_time_enqueue = g_system_ptr->getTime();
> > +        m_time_last_time_enqueue = m_clockobj_ptr->curCycle();
> >      }
> >      m_msgs_this_cycle++;
> >
> > @@ -168,7 +170,7 @@
> >      // Calculate the arrival time of the message, that is, the first
> >      // cycle the message can be dequeued.
> >      assert(delta>0);
> > -    Time current_time = g_system_ptr->getTime();
> > +    Time current_time = m_clockobj_ptr->curCycle();
> >      Time arrival_time = 0;
> >      if (!RubySystem::getRandomization() || (m_randomization == false)) {
> >          // No randomization
> > @@ -191,11 +193,10 @@
> >          if (arrival_time < m_last_arrival_time) {
> >              panic("FIFO ordering violated: %s name: %s current time: %d
> "
> >                    "delta: %d arrival_time: %d last arrival_time: %d\n",
> > -                  *this, m_name,
> > -                  current_time * g_system_ptr->clockPeriod(),
> > -                  delta * g_system_ptr->clockPeriod(),
> > -                  arrival_time * g_system_ptr->clockPeriod(),
> > -                  m_last_arrival_time * g_system_ptr->clockPeriod());
> > +                  *this, m_name, current_time *
> > m_clockobj_ptr->clockPeriod(),
> > +                  delta * m_clockobj_ptr->clockPeriod(),
> > +                  arrival_time * m_clockobj_ptr->clockPeriod(),
> > +                  m_last_arrival_time * m_clockobj_ptr->clockPeriod());
> >          }
> >      }
> >
> > @@ -208,10 +209,10 @@
> >      Message* msg_ptr = message.get();
> >      assert(msg_ptr != NULL);
> >
> > -    assert(g_system_ptr->getTime() >= msg_ptr->getLastEnqueueTime() &&
> > +    assert(m_clockobj_ptr->curCycle() >= msg_ptr->getLastEnqueueTime()
> &&
> >             "ensure we aren't dequeued early");
> >
> > -    msg_ptr->setDelayedCycles(g_system_ptr->getTime() -
> > +    msg_ptr->setDelayedCycles(m_clockobj_ptr->curCycle() -
> >                                msg_ptr->getLastEnqueueTime() +
> >                                msg_ptr->getDelayedCycles());
> >      msg_ptr->setLastEnqueueTime(arrival_time);
> > @@ -222,9 +223,8 @@
> >      push_heap(m_prio_heap.begin(), m_prio_heap.end(),
> >          greater<MessageBufferNode>());
> >
> > -    DPRINTF(RubyQueue, "Enqueue with arrival_time %lld.\n",
> > -            arrival_time * g_system_ptr->clockPeriod());
> > -    DPRINTF(RubyQueue, "Enqueue Message: %s.\n", (*(message.get())));
> > +    DPRINTF(RubyQueue, "Enqueue arrival_time: %lld, Message: %s\n",
> > +            arrival_time * m_clockobj_ptr->clockPeriod(),
> > *(message.get()));
> >
> >      // Schedule the wakeup
> >      if (m_consumer_ptr != NULL) {
> > @@ -235,18 +235,11 @@
> >      }
> >  }
> >
> > -int
> > +Time
> >  MessageBuffer::dequeue_getDelayCycles(MsgPtr& message)
> >  {
> > -    int delay_cycles = -1;  // null value
> > -
> >      dequeue(message);
> > -
> > -    // get the delay cycles
> > -    delay_cycles = setAndReturnDelayCycles(message);
> > -
> > -    assert(delay_cycles >= 0);
> > -    return delay_cycles;
> > +    return setAndReturnDelayCycles(message);
> >  }
> >
> >  void
> > @@ -259,21 +252,17 @@
> >      DPRINTF(RubyQueue, "Enqueue message is %s\n", (*(message.get())));
> >  }
> >
> > -int
> > +Time
> >  MessageBuffer::dequeue_getDelayCycles()
> >  {
> > -    int delay_cycles = -1;  // null value
> > -
> >      // get MsgPtr of the message about to be dequeued
> >      MsgPtr message = m_prio_heap.front().m_msgptr;
> >
> >      // get the delay cycles
> > -    delay_cycles = setAndReturnDelayCycles(message);
> > -
> > +    Time delayCycles = setAndReturnDelayCycles(message);
> >      dequeue();
> >
> > -    assert(delay_cycles >= 0);
> > -    return delay_cycles;
> > +    return delayCycles;
> >  }
> >
> >  void
> > @@ -287,9 +276,9 @@
> >
> >      // record previous size and time so the current buffer size isn't
> >      // adjusted until next cycle
> > -    if (m_time_last_time_pop < g_system_ptr->getTime()) {
> > +    if (m_time_last_time_pop < m_clockobj_ptr->curCycle()) {
> >          m_size_at_cycle_start = m_size;
> > -        m_time_last_time_pop = g_system_ptr->getTime();
> > +        m_time_last_time_pop = m_clockobj_ptr->curCycle();
> >      }
> >      m_size--;
> >  }
> > @@ -315,11 +304,11 @@
> >      MessageBufferNode node = m_prio_heap.front();
> >      pop_heap(m_prio_heap.begin(), m_prio_heap.end(),
> >          greater<MessageBufferNode>());
> > -    node.m_time = g_system_ptr->getTime() + m_recycle_latency;
> > +    node.m_time = m_clockobj_ptr->curCycle() + m_recycle_latency;
> >      m_prio_heap.back() = node;
> >      push_heap(m_prio_heap.begin(), m_prio_heap.end(),
> >          greater<MessageBufferNode>());
> > -    m_consumer_ptr->scheduleEventAbsolute(g_system_ptr->getTime() +
> > +    m_consumer_ptr->scheduleEventAbsolute(m_clockobj_ptr->curCycle() +
> >                                            m_recycle_latency);
> >  }
> >
> > @@ -335,7 +324,7 @@
> >      //
> >      while(!m_stall_msg_map[addr].empty()) {
> >          m_msg_counter++;
> > -        MessageBufferNode msgNode(g_system_ptr->getTime() + 1,
> > +        MessageBufferNode msgNode(m_clockobj_ptr->curCycle() + 1,
> >                                    m_msg_counter,
> >                                    m_stall_msg_map[addr].front());
> >
> > @@ -364,7 +353,7 @@
> >
> >          while(!(map_iter->second).empty()) {
> >              m_msg_counter++;
> > -            MessageBufferNode msgNode(g_system_ptr->getTime() + 1,
> > +            MessageBufferNode msgNode(m_clockobj_ptr->curCycle() + 1,
> >                                        m_msg_counter,
> >                                        (map_iter->second).front());
> >
> > @@ -397,23 +386,19 @@
> >      (m_stall_msg_map[addr]).push_back(message);
> >  }
> >
> > -int
> > +Time
> >  MessageBuffer::setAndReturnDelayCycles(MsgPtr msg_ptr)
> >  {
> > -    int delay_cycles = -1;  // null value
> > -
> >      // get the delay cycles of the message at the top of the queue
> >
> >      // this function should only be called on dequeue
> >      // ensure the msg hasn't been enqueued
> > -    assert(msg_ptr->getLastEnqueueTime() <= g_system_ptr->getTime());
> > -    msg_ptr->setDelayedCycles(g_system_ptr->getTime() -
> > -                              msg_ptr->getLastEnqueueTime() +
> > -                              msg_ptr->getDelayedCycles());
> > -    delay_cycles = msg_ptr->getDelayedCycles();
> > +    assert(msg_ptr->getLastEnqueueTime() <= m_clockobj_ptr->curCycle());
> > +    msg_ptr->setDelayedCycles(m_clockobj_ptr->curCycle() -
> > +                             msg_ptr->getLastEnqueueTime() +
> > +                             msg_ptr->getDelayedCycles());
> >
> > -    assert(delay_cycles >= 0);
> > -    return delay_cycles;
> > +    return msg_ptr->getDelayedCycles();
> >  }
> >
> >  void
> > @@ -440,7 +425,7 @@
> >  MessageBuffer::isReady() const
> >  {
> >      return ((m_prio_heap.size() > 0) &&
> > -            (m_prio_heap.front().m_time <= g_system_ptr->getTime()));
> > +            (m_prio_heap.front().m_time <= m_clockobj_ptr->curCycle()));
> >  }
> >
> >  bool
> > diff -r 794711cac18b -r 4ae4f3f4b870
> src/mem/ruby/buffers/MessageBuffer.hh
> > --- a/src/mem/ruby/buffers/MessageBuffer.hh     Mon Jan 14 10:23:56 2013
> > -0500
> > +++ b/src/mem/ruby/buffers/MessageBuffer.hh     Mon Jan 14 10:04:21 2013
> > -0600
> > @@ -86,6 +86,12 @@
> >          m_consumer_ptr = consumer_ptr;
> >      }
> >
> > +    void setClockObj(ClockedObject* obj)
> > +    {
> > +        assert(m_clockobj_ptr == NULL);
> > +        m_clockobj_ptr = obj;
> > +    }
> > +
> >      void setDescription(const std::string& name) { m_name = name; }
> >      std::string getDescription() { return m_name;}
> >
> > @@ -110,12 +116,13 @@
> >
> >      void enqueue(MsgPtr message) { enqueue(message, 1); }
> >      void enqueue(MsgPtr message, Time delta);
> > -    //  void enqueueAbsolute(const MsgPtr& message, Time absolute_time);
> > -    int dequeue_getDelayCycles(MsgPtr& message);  // returns delay
> > -                                                  // cycles of the
> > -                                                  // message
> > +
> > +    //!  returns delay ticks of the message.
> > +    Time dequeue_getDelayCycles(MsgPtr& message);
> >      void dequeue(MsgPtr& message);
> > -    int dequeue_getDelayCycles();  // returns delay cycles of the
> message
> > +
> > +    //! returns delay cycles of the message
> > +    Time dequeue_getDelayCycles();
> >      void dequeue() { pop(); }
> >      void pop();
> >      void recycle();
> > @@ -156,16 +163,19 @@
> >      int m_recycle_latency;
> >
> >      // Private Methods
> > -    int setAndReturnDelayCycles(MsgPtr message);
> > +    Time setAndReturnDelayCycles(MsgPtr message);
> >
> >      // Private copy constructor and assignment operator
> > _______________________________________________
> > gem5-dev mailing list
> > [email protected]
> > http://m5sim.org/mailman/listinfo/gem5-dev
> >
>
>
>
> --
>   Joel Hestness
>   PhD Student, Computer Architecture
>   Dept. of Computer Science, University of Wisconsin - Madison
>   http://www.cs.utexas.edu/~hestness
>
> _______________________________________________
> 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

Reply via email to