> On Nov. 3, 2015, 9:37 p.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() > > Andreas Hansson wrote: > 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.
ok, makes sense > On Nov. 3, 2015, 9:37 p.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? > > Andreas Hansson wrote: > 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. ok. would be nice to factor out the common code somehow, but it's a small enough thing that the redundancy is tolerable I suppose. > On Nov. 3, 2015, 9:37 p.m., Steve Reinhardt wrote: > > src/dev/io_device.cc, line 59 > > <http://reviews.gem5.org/r/3172/diff/1/?file=50905#file50905line59> > > > > minor, but 'additional_latency' is rather nondescript. maybe > > 'transmission_delay' or 'receive_delay' or 'receive_latency' or something > > like thet? > > Andreas Hansson wrote: > It is headerDelay (pipeline delay) and payloadDelay (de-serialisation > delay). I am happy to change, but I don't think there is a short and snappy > word describing it. I see you changed it anyway, thanks. 'receive_delay' may not be perfectly descriptive, but it's an improvement I think. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3172/#review7471 ----------------------------------------------------------- On Nov. 4, 2015, 1:34 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3172/ > ----------------------------------------------------------- > > (Updated Nov. 4, 2015, 1:34 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11191:d5063101794c > --------------------------- > 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 2d1d51615e0e > src/dev/pcidev.cc 2d1d51615e0e > src/mem/bridge.cc 2d1d51615e0e > src/mem/simple_mem.hh 2d1d51615e0e > src/mem/simple_mem.cc 2d1d51615e0e > > 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
