> On Nov. 4, 2015, 5:37 a.m., Steve Reinhardt wrote:
> > src/dev/io_device.cc, line 60
> > <http://reviews.gem5.org/r/3172/diff/1/?file=50905#file50905line60>
> >
> >     out of scope for now, but just a thought: arguably this zeroing of the 
> > delay fields should probably take place in Packet::makeResponse()

No. It relly should never implicitly be zeroed.

If we re-use the packet without paying for the latency on the request path, we 
rely on it being paid for on the response path.


> On Nov. 4, 2015, 5:37 a.m., Steve Reinhardt wrote:
> > src/mem/bridge.cc, line 145
> > <http://reviews.gem5.org/r/3172/diff/1/?file=50907#file50907line145>
> >
> >     why ticksToCycles()?  aren't you adding a cycle value to a tick value?

My bad. Will fix.


> On Nov. 4, 2015, 5:37 a.m., Steve Reinhardt wrote:
> > src/mem/simple_mem.cc, line 164
> > <http://reviews.gem5.org/r/3172/diff/1/?file=50909#file50909line164>
> >
> >     this code looks awfully familiar :).  is there a way to use PacketQueue 
> > here to avoid the redundancy?

It was historically using the packet queue. I The main reason for moving away 
from it is to be able to bound the number of outstanding responses. Till now 
that hasn't happened, but I am inclined to not use the SimpleTimingPort here.


- Andreas


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


On Oct. 30, 2015, 10:35 a.m., Andreas Hansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3172/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2015, 10:35 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 11188:353cf88682b9
> ---------------------------
> mem: Use the packet delays and do not just zero them out
> 
> This patch updates the I/O devices, bridge and simple memory to take
> the packet header and payload delay into account in their latency
> calculations. In all cases we add the header delay, i.e. the
> accumulated pipeline delay of any crossbars, and the payload delay
> needed for deserialisation of any payload.
> 
> Due to the additional unknown latency contribution, the packet queue
> of the simple memory is changed to use insertion sorting based on the
> time stamp. Moreover, since the memory hands out exclusive (non
> shared) responses, we also need to ensure ordering for reads to the
> same address.
> 
> 
> Diffs
> -----
> 
>   src/dev/io_device.cc 4daf60db14d7 
>   src/dev/pcidev.cc 4daf60db14d7 
>   src/mem/bridge.cc 4daf60db14d7 
>   src/mem/simple_mem.hh 4daf60db14d7 
>   src/mem/simple_mem.cc 4daf60db14d7 
> 
> Diff: http://reviews.gem5.org/r/3172/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to