> On 2011-12-03 12:33:49, Steve Reinhardt wrote:
> > 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.

The vector<bool> is supposedly a special case of the vector object and 
allocates a bit-vector.  So extra space should be particularly small.  I'll 
look into making be called less.


- Geoffrey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/915/#review1719
-----------------------------------------------------------


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

Reply via email to