Re: [gem5-dev] Review Request 3753: ruby: Check MessageBuffer space in garnet NetworkInterface

[Tushar Krishna]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3753/#review9197
---



src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 139)


If a flit is waiting for the output buffer to become free, this will get 
counted in its network delay component in the current implementation.
It should be counted in its queueing delay component I think ...
Queueing delay is the delay at the message buffers and NICs, network delay 
is from src NI to dest NI and includes delay in routers and links.



src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 200)


See my comment below in the function definition regarding the name of this 
variable ...



src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 203)


We should consume the link irrespective of whether the state of the stall 
queue. If we do not consume, effectively we are modeling a quueue inside the 
network link.

I can see why you added this condition though - you don't want more than 
one enqueue into the message buffer in the same cycle.
If the stall queue already enqueued something, you are making the one from 
the link wait.

How about the following:

bool unstalledMessage = checkStallQueue();

if (inNetLink->isReady(curCycle()) {
flit *t_flit = inNetLink->consumeLink();

...
if (outNode_ptr[vnet]->areNSlotsAvailable(1, curTime) && !unstalledMessage) 
{
// enqueue into protocol buffer
}
else {
// enqueue into stall queue
}



src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 264)


The name of this variable is a little confusing.

Hmm I think I can see why you named it like that. If a message that was 
stalled gets unstalled, then this variable becomes true.

But if the stall queue is empty, unstalledMessage is returned false, which 
means stalledMessage is true .. which is not quite right.


- Tushar Krishna


On Dec. 9, 2016, 1:58 a.m., Matthew Poremba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3753/
> ---
> 
> (Updated Dec. 9, 2016, 1:58 a.m.)
> 
> 
> Review request for Default and Tushar Krishna.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11761:fd4126ef5e73
> ---
> ruby: Check MessageBuffer space in garnet NetworkInterface
> 
> Garnet's NetworkInterface does not consider the size of MessageBuffers when
> ejecting a Message from the network. Add a size check for the MessageBuffer
> and only enqueue if space is available. If space is not available, the
> message if placed in a queue and the credit is held. A callback from the
> MessageBuffer is implemented to wake the NetworkInterface. If there are
> messages in the stalled queue, they are processed first, in a FIFO manner
> and if succesfully ejected, the credit is finally sent back upstream. The
> maximum size of the stall queue is equal to the number of valid VNETs
> with MessageBuffers attached.
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/network/MessageBuffer.hh 
> 78ef8daecd81de0c392034809b3bc155396bf983 
>   src/mem/ruby/network/MessageBuffer.cc 
> 78ef8daecd81de0c392034809b3bc155396bf983 
>   src/mem/ruby/network/garnet2.0/NetworkInterface.hh 
> 78ef8daecd81de0c392034809b3bc155396bf983 
>   src/mem/ruby/network/garnet2.0/NetworkInterface.cc 
> 78ef8daecd81de0c392034809b3bc155396bf983 
> 
> Diff: http://reviews.gem5.org/r/3753/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthew Poremba
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3753: ruby: Check MessageBuffer space in garnet NetworkInterface

[Matthew Poremba]

> On Dec. 9, 2016, 12:26 a.m., Tushar Krishna wrote:
> > This is an excellent patch! Something that has been long due.
> > (BTW does the same problem occur in simple network as well?)

Thanks for the fast review! I've updated it based on your comments and i'm 
rerunning some longer tests to make sure everything is still OK.

Also- SimpleNetwork's Throttles take care of the machanism provided by this 
patch already, so it only needs to be implemented in Garnet2.0.


> On Dec. 9, 2016, 12:26 a.m., Tushar Krishna wrote:
> > src/mem/ruby/network/garnet2.0/NetworkInterface.cc, line 200
> > 
> >
> > Stylistic comment:
> > Can we move this piece of code to a new function which returns 
> > true/false and sets the value of unstalledMessage?
> > 
> > Otherwise the wakeup function is very long now and performs the actual 
> > action of reading from the link only at the very end.

Moved this to a new function.


> On Dec. 9, 2016, 12:26 a.m., Tushar Krishna wrote:
> > src/mem/ruby/network/garnet2.0/NetworkInterface.cc, line 213
> > 
> >
> > These three lines should become a new send_credit function.
> > 
> > Credit *credit_flit = new Credit(stallFlit->get_vc(), true, curCycle());
> > 
> > outCreditQueue->insert(credit_flit);
> > outCreditLink->scheduleEventAbsolute(clockEdge(Cycles(1)));
> > 
> > The function can be called here and later.
> > Having two pieces of code where credits are being created and sent can 
> > lead to a lot of bugs whenever anyone makes changes and forgets to make 
> > changes at both places (have seen that a lot from experience) :)

Good point. Moved this to a new function to reduce code reuse.


> On Dec. 9, 2016, 12:26 a.m., Tushar Krishna wrote:
> > src/mem/ruby/network/garnet2.0/NetworkInterface.cc, line 257
> > 
> >
> > Looks like the link not consumed if there are stalled messages.
> > But this would mean that the router will keep inserting flits into the 
> > link which may not get consumed, effectively modeling a large queue inside 
> > the link which is incorrect.
> > The NI always consumes the link. This is because the router checked for 
> > credits before sending to the NI.
> > 
> > 
> > 
> > Here is my suggestion for this piece of code:
> > if (inNetLink->isReady(curCycle()) {
> > flit *t_flit = inNetLink->consumeLink();
> > 
> > // Now check if message buffer has slots.
> > // If it does, then { (i) insert msg into msg buffer, (ii) call 
> > send_credit function. }
> > // else { insert msg into stall queue.}
> > 
> > // Implement a send_credit function that can be called from here or 
> > when the stall queue is read.
> > 
> > ...
> > 
> > The extLinkAvail piece of code is not needed. It gets subsumed here.

Good suggestion. I consolidated this and the previous two issues into one block.


- Matthew


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3753/#review9195
---


On Dec. 9, 2016, 1:58 a.m., Matthew Poremba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3753/
> ---
> 
> (Updated Dec. 9, 2016, 1:58 a.m.)
> 
> 
> Review request for Default and Tushar Krishna.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11761:fd4126ef5e73
> ---
> ruby: Check MessageBuffer space in garnet NetworkInterface
> 
> Garnet's NetworkInterface does not consider the size of MessageBuffers when
> ejecting a Message from the network. Add a size check for the MessageBuffer
> and only enqueue if space is available. If space is not available, the
> message if placed in a queue and the credit is held. A callback from the
> MessageBuffer is implemented to wake the NetworkInterface. If there are
> messages in the stalled queue, they are processed first, in a FIFO manner
> and if succesfully ejected, the credit is finally sent back upstream. The
> maximum size of the stall queue is equal to the number of valid VNETs
> with MessageBuffers attached.
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/network/MessageBuffer.hh 
> 78ef8daecd81de0c392034809b3bc155396bf983 
>   src/mem/ruby/network/MessageBuffer.cc 
> 78ef8daecd81de0c392034809b3bc155396bf983 
>   src/mem/ruby/network/garnet2.0/NetworkInterface.hh 
> 78ef8daecd81de0c392034809b3bc155396bf983 
>   src/mem/ruby/network/garnet2.0/NetworkInterface.cc 
> 78ef8daecd81de0c392034809b3bc155396bf983 
> 
> Diff: http://reviews.gem5.or

Re: [gem5-dev] Review Request 3753: ruby: Check MessageBuffer space in garnet NetworkInterface

[Matthew Poremba]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3753/
---

(Updated Dec. 9, 2016, 1:58 a.m.)


Review request for Default and Tushar Krishna.


Changes
---

Update patch to address issues


Repository: gem5


Description (updated)
---

Changeset 11761:fd4126ef5e73
---
ruby: Check MessageBuffer space in garnet NetworkInterface

Garnet's NetworkInterface does not consider the size of MessageBuffers when
ejecting a Message from the network. Add a size check for the MessageBuffer
and only enqueue if space is available. If space is not available, the
message if placed in a queue and the credit is held. A callback from the
MessageBuffer is implemented to wake the NetworkInterface. If there are
messages in the stalled queue, they are processed first, in a FIFO manner
and if succesfully ejected, the credit is finally sent back upstream. The
maximum size of the stall queue is equal to the number of valid VNETs
with MessageBuffers attached.


Diffs (updated)
-

  src/mem/ruby/network/MessageBuffer.hh 
78ef8daecd81de0c392034809b3bc155396bf983 
  src/mem/ruby/network/MessageBuffer.cc 
78ef8daecd81de0c392034809b3bc155396bf983 
  src/mem/ruby/network/garnet2.0/NetworkInterface.hh 
78ef8daecd81de0c392034809b3bc155396bf983 
  src/mem/ruby/network/garnet2.0/NetworkInterface.cc 
78ef8daecd81de0c392034809b3bc155396bf983 

Diff: http://reviews.gem5.org/r/3753/diff/


Testing
---


Thanks,

Matthew Poremba

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3753: ruby: Check MessageBuffer space in garnet NetworkInterface

[Tushar Krishna]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3753/#review9195
---


This is an excellent patch! Something that has been long due.
(BTW does the same problem occur in simple network as well?)


src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 139)


This is a great change! Anyone wanting to add more stats can do that here.



src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 200)


Stylistic comment:
Can we move this piece of code to a new function which returns true/false 
and sets the value of unstalledMessage?

Otherwise the wakeup function is very long now and performs the actual 
action of reading from the link only at the very end.



src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 213)


These three lines should become a new send_credit function.

Credit *credit_flit = new Credit(stallFlit->get_vc(), true, curCycle());

outCreditQueue->insert(credit_flit);
outCreditLink->scheduleEventAbsolute(clockEdge(Cycles(1)));

The function can be called here and later.
Having two pieces of code where credits are being created and sent can lead 
to a lot of bugs whenever anyone makes changes and forgets to make changes at 
both places (have seen that a lot from experience) :)



src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 238)


Looks like only HEAD_TAIL and TAIL_ flits set extLinkAvail to false and are 
consumed here, while other flits are consumed later in the code. It will be a 
little confusing for a newbie looking at the code.



src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 245)


Adding to the point above - 

My worry is that someone new looking at the code might get a little 
confused about the fact that the consumeLink happens twice. And that the HEAD 
and HEAD_TAIL are sometimes consumed here, and sometimes later in the code.

See my suggestion below on some code restructuring.



src/mem/ruby/network/garnet2.0/NetworkInterface.cc (line 257)


Looks like the link not consumed if there are stalled messages.
But this would mean that the router will keep inserting flits into the link 
which may not get consumed, effectively modeling a large queue inside the link 
which is incorrect.
The NI always consumes the link. This is because the router checked for 
credits before sending to the NI.

Here is my suggestion for this piece of code:
if (inNetLink->isReady(curCycle()) {
flit *t_flit = inNetLink->consumeLink();

// Now check if message buffer has slots.
// If it does, then { (i) insert msg into msg buffer, (ii) call send_credit 
function. }
// else { insert msg into stall queue.}

// Implement a send_credit function that can be called from here or when 
the stall queue is read.

...

The extLinkAvail piece of code is not needed. It gets subsumed here.


- Tushar Krishna


On Dec. 8, 2016, 11:34 p.m., Matthew Poremba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3753/
> ---
> 
> (Updated Dec. 8, 2016, 11:34 p.m.)
> 
> 
> Review request for Default and Tushar Krishna.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11761:e47a6baba2f2
> ---
> ruby: Check MessageBuffer space in garnet NetworkInterface
> 
> Garnet's NetworkInterface does not consider the size of MessageBuffers when
> ejecting a Message from the network. Add a size check for the MessageBuffer
> and only enqueue if space is available. If space is not available, the
> message if placed in a queue and the credit is held. A callback from the
> MessageBuffer is implemented to wake the NetworkInterface. If there are
> messages in the stalled queue, they are processed first, in a FIFO manner
> and if succesfully ejected, the credit is finally sent back upstream. The
> maximum size of the stall queue is equal to the number of valid VNETs
> with MessageBuffers attached.
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/network/MessageBuffer.hh 
> 78ef8daecd81de0c392034809b3bc155396bf983 
>   src/mem/ruby/network/MessageBuffer.cc 
> 78ef8daecd81de0c392034809b3bc155396bf983 
>   src/mem/ruby/network/garnet2.0/NetworkInterface.hh 
> 78ef8daecd81de0c392034809b3bc155396bf983 
>   src/mem/ruby/network/garnet2.0/NetworkInterface.cc 
> 78ef8daecd81de0c392034809b3bc155396bf983 
> 
> Diff: http://review

Re: [gem5-dev] Review Request 3751: ruby: Check all VNETs for injection in garnet NetworkInterface

[Tushar Krishna]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3751/#review9194
---

Ship it!


Ship It!

- Tushar Krishna


On Dec. 8, 2016, 11:32 p.m., Matthew Poremba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3751/
> ---
> 
> (Updated Dec. 8, 2016, 11:32 p.m.)
> 
> 
> Review request for Default and Tushar Krishna.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11758:a3f355233ef0
> ---
> ruby: Check all VNETs for injection in garnet NetworkInterface
> 
> The NetworkInterface wakeup currently iterates over all VNETs and breaks the
> loop if a VNET is unable to allocate a VC. This can cause a deadlock if a
> lower numbered VNET is unable to allocate a VC while a higher numbered VNET
> has idle VCs. This seems like a bug as Garnet 1.0 uses a while loop over an
> if-statement, suggesting the break was intended for this while loop. This
> patch removes the break statement, which allows up to one message to be
> dequeued from a VNET and injected into the network.
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/network/garnet2.0/NetworkInterface.cc 
> 78ef8daecd81de0c392034809b3bc155396bf983 
> 
> Diff: http://reviews.gem5.org/r/3751/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Matthew Poremba
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Review Request 3753: ruby: Check MessageBuffer space in garnet NetworkInterface

[Matthew Poremba]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3753/
---

Review request for Default and Tushar Krishna.


Repository: gem5


Description
---

Changeset 11761:e47a6baba2f2
---
ruby: Check MessageBuffer space in garnet NetworkInterface

Garnet's NetworkInterface does not consider the size of MessageBuffers when
ejecting a Message from the network. Add a size check for the MessageBuffer
and only enqueue if space is available. If space is not available, the
message if placed in a queue and the credit is held. A callback from the
MessageBuffer is implemented to wake the NetworkInterface. If there are
messages in the stalled queue, they are processed first, in a FIFO manner
and if succesfully ejected, the credit is finally sent back upstream. The
maximum size of the stall queue is equal to the number of valid VNETs
with MessageBuffers attached.


Diffs
-

  src/mem/ruby/network/MessageBuffer.hh 
78ef8daecd81de0c392034809b3bc155396bf983 
  src/mem/ruby/network/MessageBuffer.cc 
78ef8daecd81de0c392034809b3bc155396bf983 
  src/mem/ruby/network/garnet2.0/NetworkInterface.hh 
78ef8daecd81de0c392034809b3bc155396bf983 
  src/mem/ruby/network/garnet2.0/NetworkInterface.cc 
78ef8daecd81de0c392034809b3bc155396bf983 

Diff: http://reviews.gem5.org/r/3753/diff/


Testing
---


Thanks,

Matthew Poremba

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3297: ruby: Add occupancy stats to MessageBuffers

[Matthew Poremba]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3297/#review9193
---


I've posted an updated version of this patch which removes conflicts with a 
recent commit. We should coordinate which should be updated/reviewed/discarded.

- Matthew Poremba


On Jan. 23, 2016, 8:59 p.m., Joel Hestness wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3297/
> ---
> 
> (Updated Jan. 23, 2016, 8:59 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11299:483fab98c031
> ---
> ruby: Add occupancy stats to MessageBuffers
> 
> The most important statistic for measuring memory hierarchy performance is
> throughput, which is affected by independent variables, buffer sizing and
> communication latency. It is difficult/impossible to debug performance issues
> through series buffers without knowing which are the bottlenecks. For finite
> buffers, this patch adds statistics for the average number of messages in the
> buffer, the occupancy of the buffer slots, and number of message stalls.
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/network/MessageBuffer.hh d1f8610cdffd 
>   src/mem/ruby/network/MessageBuffer.cc d1f8610cdffd 
> 
> Diff: http://reviews.gem5.org/r/3297/diff/
> 
> 
> Testing
> ---
> 
> NOTE: The message count stat could be used as an alternative way to count
> total buffered messages RE: http://reviews.gem5.org/r/3283/. Input on how to
> tighten things up is welcome.
> 
> 
> Thanks,
> 
> Joel Hestness
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Review Request 3752: ruby: Add occupancy stats to MessageBuffers

[Matthew Poremba]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3752/
---

Review request for Default and Joel Hestness.


Repository: gem5


Description
---

Changeset 11760:676d3e045d22
---
ruby: Add occupancy stats to MessageBuffers

This patch is an updated version of /r/3297.

"The most important statistic for measuring memory hierarchy performance is
throughput, which is affected by independent variables, buffer sizing and
communication latency. It is difficult/impossible to debug performance issues
through series buffers without knowing which are the bottlenecks. For finite
buffers, this patch adds statistics for the average number of messages in the
buffer, the occupancy of the buffer slots, and number of message stalls."


Diffs
-

  src/mem/ruby/network/MessageBuffer.hh 
78ef8daecd81de0c392034809b3bc155396bf983 
  src/mem/ruby/network/MessageBuffer.cc 
78ef8daecd81de0c392034809b3bc155396bf983 

Diff: http://reviews.gem5.org/r/3752/diff/


Testing
---


Thanks,

Matthew Poremba

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3283: ruby: Make MessageBuffers actually finite sized

[Matthew Poremba]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3283/#review9192
---

Ship it!


Any chance we could ship this finally? I have been using this for months 
without any issues and have some new patches I would like to push out that 
depend on this one.

- Matthew Poremba


On Jan. 17, 2016, 11:47 p.m., Joel Hestness wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3283/
> ---
> 
> (Updated Jan. 17, 2016, 11:47 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11298:1afa8d03
> ---
> ruby: Make MessageBuffers actually finite sized
> 
> When Ruby controllers stall messages in MessageBuffers, the buffer moves those
> messages off the priority heap and into a per-address stall map. When buffers
> are finite-sized, the test areNSlotsAvailable() only checks the size of the
> priority heap, but ignores the stall map, so the map is allowed to grow
> unbounded if the controller stalls numerous messages. This patch fixes the
> problem by tracking the stall map size and testing the total number of 
> messages
> in the buffer appropriately.
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/network/MessageBuffer.hh d1f8610cdffd 
>   src/mem/ruby/network/MessageBuffer.cc d1f8610cdffd 
> 
> Diff: http://reviews.gem5.org/r/3283/diff/
> 
> 
> Testing
> ---
> 
> Multiple tests to exercise the capacity of buffers, including high bandwidth
> demand and MLP situations concurrently accessing numerous different line
> addresses. Regressions are unchanged, since they do not use finite-sized
> buffering.
> 
> NOTE: When using finite-sized buffers, this fix can cause decreased
> performance due to insufficient buffering, a condition that was masked with
> the unbounded stall map. Users may need to adjust buffer sizes to revalidate
> performance.
> 
> 
> Thanks,
> 
> Joel Hestness
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Review Request 3751: ruby: Check all VNETs for injection in garnet NetworkInterface

[Matthew Poremba]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3751/
---

Review request for Default and Tushar Krishna.


Repository: gem5


Description
---

Changeset 11758:a3f355233ef0
---
ruby: Check all VNETs for injection in garnet NetworkInterface

The NetworkInterface wakeup currently iterates over all VNETs and breaks the
loop if a VNET is unable to allocate a VC. This can cause a deadlock if a
lower numbered VNET is unable to allocate a VC while a higher numbered VNET
has idle VCs. This seems like a bug as Garnet 1.0 uses a while loop over an
if-statement, suggesting the break was intended for this while loop. This
patch removes the break statement, which allows up to one message to be
dequeued from a VNET and injected into the network.


Diffs
-

  src/mem/ruby/network/garnet2.0/NetworkInterface.cc 
78ef8daecd81de0c392034809b3bc155396bf983 

Diff: http://reviews.gem5.org/r/3751/diff/


Testing
---


Thanks,

Matthew Poremba

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3710: cpu: Resolve targets of predicted 'taken' conditional direct branches at decode (o3)

[John Kalamatianos]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3710/#review9191
---

Ship it!


Ship It!

- John Kalamatianos


On Nov. 18, 2016, 10:21 a.m., Arthur Perais wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3710/
> ---
> 
> (Updated Nov. 18, 2016, 10:21 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11707:1d085f66c4ca
> ---
> cpu: Resolve targets of predicted 'taken' conditional direct branches at 
> decode (o3)
> 
> The target of taken conditional direct branches does not 
> need to be resolved in IEW: the target can be computed at 
> decode, usually using the decoded instruction word and the PC.
> 
> The higher-than-necessary penalty is taken only on conditional
> branches that are predicted taken but miss in the BTB. Thus, 
> this is mostly inconsequential on IPC if the BTB is big/associative 
> enough (fewer capacity/conflict misses). Nonetheless, what gem5 
> simulates is not representative of how conditional branch targets 
> can be handled.
> 
> 
> Diffs
> -
> 
>   src/cpu/o3/decode_impl.hh c38fcdaa5fe5 
> 
> Diff: http://reviews.gem5.org/r/3710/diff/
> 
> 
> Testing
> ---
> 
> util/regress --modes=se
> 
> build/NULL/tests/opt/quick/se/51.memcheck/null/none/memcheck: FAILED! (Python 
> says "NameError: name 'TrafficGen' is not defined"). I'm guessing this is OK 
> as I get the same error without my patch.
> 
> 
> Thanks,
> 
> Arthur Perais
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3708: commit 752c67b134f4cb0b7ca68a907c39a5a482de30b3

[Pierre-Yves Péneau]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3708/#review9190
---

Ship it!


Ship It!

- Pierre-Yves Péneau


On Nov. 17, 2016, 9:02 p.m., Rahul Thakur wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3708/
> ---
> 
> (Updated Nov. 17, 2016, 9:02 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11705:f55d4a414fa1
> ---
> commit 752c67b134f4cb0b7ca68a907c39a5a482de30b3
> Author: Rahul Thakur 
> Date:   Thu Oct 27 17:44:40 2016 -0700
> 
> mem: Add memory footprint probe
> 
> Change-Id: I0fba8995edd63df4ef49969347be6d2aefceca9f
> 
> 
> Diffs
> -
> 
>   COPYING c38fcdaa5fe5 
>   configs/dram/lat_mem_rd.py c38fcdaa5fe5 
>   src/mem/probes/MemFootprintProbe.py PRE-CREATION 
>   src/mem/probes/SConscript c38fcdaa5fe5 
>   src/mem/probes/mem_footprint.hh PRE-CREATION 
>   src/mem/probes/mem_footprint.cc PRE-CREATION 
> 
> Diff: http://reviews.gem5.org/r/3708/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Thakur
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3708: commit 752c67b134f4cb0b7ca68a907c39a5a482de30b3

[Tony Gutierrez]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3708/#review9189
---

Ship it!


Ship It!

- Tony Gutierrez


On Nov. 17, 2016, 12:02 p.m., Rahul Thakur wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3708/
> ---
> 
> (Updated Nov. 17, 2016, 12:02 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11705:f55d4a414fa1
> ---
> commit 752c67b134f4cb0b7ca68a907c39a5a482de30b3
> Author: Rahul Thakur 
> Date:   Thu Oct 27 17:44:40 2016 -0700
> 
> mem: Add memory footprint probe
> 
> Change-Id: I0fba8995edd63df4ef49969347be6d2aefceca9f
> 
> 
> Diffs
> -
> 
>   COPYING c38fcdaa5fe5 
>   configs/dram/lat_mem_rd.py c38fcdaa5fe5 
>   src/mem/probes/MemFootprintProbe.py PRE-CREATION 
>   src/mem/probes/SConscript c38fcdaa5fe5 
>   src/mem/probes/mem_footprint.hh PRE-CREATION 
>   src/mem/probes/mem_footprint.cc PRE-CREATION 
> 
> Diff: http://reviews.gem5.org/r/3708/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Thakur
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3750: arm, config: Add dist-gem5 support to the big.LITTLE(tm) config

[Gabor Dozsa]

> On Dec. 7, 2016, 6:41 p.m., Jason Lowe-Power wrote:
> > configs/example/arm/dist_bigLITTLE.py, line 53
> > 
> >
> > incorrect indentation.

Fixed.


- Gabor


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3750/#review9187
---


On Dec. 8, 2016, 1:35 p.m., Gabor Dozsa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3750/
> ---
> 
> (Updated Dec. 8, 2016, 1:35 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11759:5cda58819109
> ---
> arm,config: Add dist-gem5 support to the big.LITTLE(tm) config
> 
> This patch extends the example big.LITTLE configuration to enable
> dist-gem5 simulations of big.LITTLE systems.
> 
> Change-Id: I49c095ab3c737b6a082f7c6f15f514c269217756
> Reviewed-by: Andreas Sandberg 
> 
> 
> Diffs
> -
> 
>   configs/example/arm/dist_bigLITTLE.py PRE-CREATION 
> 
> Diff: http://reviews.gem5.org/r/3750/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabor Dozsa
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3750: arm, config: Add dist-gem5 support to the big.LITTLE(tm) config

[Gabor Dozsa]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3750/
---

(Updated Dec. 8, 2016, 1:35 p.m.)


Review request for Default.


Summary (updated)
-

arm,config: Add dist-gem5 support to the big.LITTLE(tm) config


Repository: gem5


Description (updated)
---

Changeset 11759:5cda58819109
---
arm,config: Add dist-gem5 support to the big.LITTLE(tm) config

This patch extends the example big.LITTLE configuration to enable
dist-gem5 simulations of big.LITTLE systems.

Change-Id: I49c095ab3c737b6a082f7c6f15f514c269217756
Reviewed-by: Andreas Sandberg 


Diffs (updated)
-

  configs/example/arm/dist_bigLITTLE.py PRE-CREATION 

Diff: http://reviews.gem5.org/r/3750/diff/


Testing
---


Thanks,

Gabor Dozsa

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3748: arm, config: Refactor the example big.LITTLE(tm) configuration

[Gabor Dozsa]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3748/
---

(Updated Dec. 8, 2016, 1:24 p.m.)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11757:4e2ed6befc63
---
arm,config: Refactor the example big.LITTLE(tm) configuration

This patch prepares future extensions and customisation of the example
big.LITTLE configuration script. It breaks out the major phases into
functions so they can be called from other python scripts.

Change-Id: I2cb7c207c410fe14602cf17af7482719abba6c24
Reviewed-by: Andreas Sandberg 


Diffs (updated)
-

  configs/example/arm/fs_bigLITTLE.py 0d38e56356c7 

Diff: http://reviews.gem5.org/r/3748/diff/


Testing
---


Thanks,

Gabor Dozsa

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Cron /z/m5/regression/do-regression quick

[Cron Daemon]

* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64i/minor-timing: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64i/simple-timing: 
CHANGED!
* 
build/RISCV/tests/opt/quick/se/02.insttest/riscv/linux-rv64i/simple-timing-ruby:
 CHANGED!
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/minor-timing: passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/o3-timing: passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-atomic: passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-timing: passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby: 
passed.
* build/ALPHA/tests/opt/quick/se/01.hello-2T-smt/alpha/linux/o3-timing-mt: 
passed.
* build/ALPHA/tests/opt/quick/se/50.memtest/alpha/linux/memtest-ruby: 
passed.
* 
build/ALPHA/tests/opt/quick/se/03.learning-gem5/alpha/linux/learning-gem5-p1-simple:
 passed.
* 
build/ALPHA/tests/opt/quick/se/03.learning-gem5/alpha/linux/learning-gem5-p1-two-level:
 passed.
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-atomic: 
passed.
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-atomic-dual:
 passed.
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-timing: 
passed.
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-timing-dual:
 passed.
* 
build/ALPHA/tests/opt/quick/fs/80.netperf-stream/alpha/linux/twosys-tsunami-simple-atomic:
 passed.
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/o3-timing: passed.
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/simple-atomic: passed.
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/simple-timing: passed.
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/simple-timing-ruby: 
passed.
* 
build/MIPS/tests/opt/quick/se/03.learning-gem5/mips/linux/learning-gem5-p1-simple:
 passed.
* 
build/MIPS/tests/opt/quick/se/03.learning-gem5/mips/linux/learning-gem5-p1-two-level:
 passed.
* build/NULL/tests/opt/quick/se/70.tgen/null/none/tgen-dram-ctrl: passed.
* build/NULL/tests/opt/quick/se/50.memtest/null/none/memtest: passed.
* build/NULL/tests/opt/quick/se/51.memcheck/null/none/memcheck: 
passed.* build/NULL/tests/opt/quick/se/50.memtest/null/none/memtest-filter: 
passed.
* build/NULL/tests/opt/quick/se/70.tgen/null/none/tgen-simple-mem: passed.
* build/NULL/tests/opt/quick/se/60.rubytest/null/none/rubytest-ruby: passed.
* 
build/NULL_MOESI_hammer/tests/opt/quick/se/60.rubytest/null/none/rubytest-ruby-MOESI_hammer:
 passed.
* 
build/NULL_MESI_Two_Level/tests/opt/quick/se/60.rubytest/null/none/rubytest-ruby-MESI_Two_Level:
 passed.
* 
build/NULL_MOESI_CMP_directory/tests/opt/quick/se/60.rubytest/null/none/rubytest-ruby-MOESI_CMP_directory:
 passed.
* 
build/NULL_MOESI_CMP_token/tests/opt/quick/se/60.rubytest/null/none/rubytest-ruby-MOESI_CMP_token:
 passed.
* build/POWER/tests/opt/quick/se/00.hello/power/linux/o3-timing: passed.
* build/POWER/tests/opt/quick/se/00.hello/power/linux/simple-atomic: passed.
* build/SPARC/tests/opt/quick/se/00.hello/sparc/linux/simple-atomic: passed.
* build/SPARC/tests/opt/quick/se/00.hello/sparc/linux/simple-timing: passed.
* build/SPARC/tests/opt/quick/se/00.hello/sparc/linux/simple-timing-ruby: 
passed.
* build/SPARC/tests/opt/quick/se/02.insttest/sparc/linux/o3-timing: passed.
* build/SPARC/tests/opt/quick/se/02.insttest/sparc/linux/simple-atomic: 
passed.
* build/SPARC/tests/opt/quick/se/02.insttest/sparc/linux/simple-timing: 
passed.
* 
build/SPARC/tests/opt/quick/se/03.learning-gem5/sparc/linux/learning-gem5-p1-simple:
 passed.* 
build/SPARC/tests/opt/quick/se/03.learning-gem5/sparc/linux/learning-gem5-p1-two-level:
 passed.
* build/SPARC/tests/opt/quick/se/10.mcf/sparc/linux/simple-atomic: passed.
* 
build/SPARC/tests/opt/quick/se/40.m5threads-test-atomic/sparc/linux/simple-atomic-mp:
 passed.
* 
build/SPARC/tests/opt/quick/se/40.m5threads-test-atomic/sparc/linux/o3-timing-mp:
 passed.
* 
build/SPARC/tests/opt/quick/se/40.m5threads-test-atomic/sparc/linux/simple-timing-mp:
 passed.
* build/SPARC/tests/opt/quick/se/50.vortex/sparc/linux/simple-atomic: 
passed.
* build/SPARC/tests/opt/quick/se/50.vortex/sparc/linux/simple-timing: 
passed.
* build/SPARC/tests/opt/quick/se/70.twolf/sparc/linux/simple-atomic: passed.
* build/SPARC/tests/opt/quick/se/70.twolf/sparc/linux/simple-timing: passed.
* build/X86/tests/opt/quick/se/00.hello/x86/linux/o3-timing: passed.
* build/X86/tests/opt/quick/se/00.hello/x86/linux/simple-atomic: passed.
* build/X86/tests/opt/quick/se/00.hello/x86/linux/simple-timing: passed.
* build/X86/tests/opt/quick/se/00.hello/x86/linux/simple-timing-ruby: 
passed.
* 
build/X86/tests/opt/quick/se/03.learning-gem5/x86/linux/learning-gem5-p1-simple:
 passed.
*