----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/915/#review1719 -----------------------------------------------------------
Functionally it looks fine to me. My only concern is that we're adding some (small) overhead to every single Packet just to deal with this one extremely rare corner case. Packets get dynamically allocated a *lot* in timing simulation, so it's painful to me to see us adding any overhead there. Is there a way we can at least avoid calling bytes_valid.resize() in every constructor? I'm not sure how much space a std::vector<bool> takes, or how much the default constructor costs; if either one is not negligible, it might even be worth just putting a pointer in the Packet and dynamically allocating the vector itself only when needed. I don't know if that is really worthwhile or not though. - Steve On 2011-11-29 08:18:07, Geoffrey Blake wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/915/ > ----------------------------------------------------------- > > (Updated 2011-11-29 08:18:07) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > Packet: Enable functional reads of partial data to packet class > > This patch fixes a long standing defficiency in the packet class where > it was unable to handle finding data that partially satisfied a request. > > This splits out changes made to the packet class in the checkercpu patch as > requested by Ali. > > > Diffs > ----- > > src/mem/packet.hh 62dee0c98d53 > src/mem/packet.cc 62dee0c98d53 > > Diff: http://reviews.m5sim.org/r/915/diff > > > Testing > ------- > > Compiles. No functional changes made from CheckerCPU patch to this patch for > packet class, and CheckerCPU fully exercised this code path during testing. > > > Thanks, > > Geoffrey > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
