----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3259/#review7741 -----------------------------------------------------------
I'm curious how much this matters... are there a significant number of cases right now where we're unnecessarily allocating a data buffer? My gut reaction is that if we're really looking to defer buffer allocation until we really know whether or not it's necessary, then we should just wait until setData() and just do the allocation then if needed. (You could also conceivably do the allocation in make*Response(), but I see how that would be complicated by us calling setData() in satisfyCpuSideRequest() before we call make*Response().) If there are really cases where we currently call setData() but which should be protected by a hasData()/hasRespData() check we could add that inside setData(). src/mem/cache/cache.cc (line 190) <http://reviews.gem5.org/r/3259/#comment6671> are there read requests which don't expect data in the response? unless that's the case, an assert might be more appropriate. src/mem/cache/cache.cc (line 2057) <http://reviews.gem5.org/r/3259/#comment6672> is there a case where we reach this code w/o pkt->hasData() being true? given that 'respond' is only set if we have a modified copy, it seems unlikely... again, if not, perhaps an assert is more apporpriate? src/mem/cache/mshr.cc (line 123) <http://reviews.gem5.org/r/3259/#comment6673> seems complicated to have to track these transitions to know when to call pkt->allocate() and when not to... - Steve Reinhardt On Dec. 15, 2015, 4:40 p.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3259/ > ----------------------------------------------------------- > > (Updated Dec. 15, 2015, 4:40 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11281:e01a026a48ad > --------------------------- > mem: Do not allocate space for packet data if not needed > > This patch looks at the request and response command to determine if > either actually has any data payload, and if not, we do not allocate > any space for packet data. > > The only tricky case is where the command type is changed as part of > the MSHR functionality. In these cases where the original packet had > no data, but the new packet does, we need to explicitly call > allocate(). > > > Diffs > ----- > > src/mem/cache/cache.cc a3b41de1c4f1 > src/mem/cache/mshr.cc a3b41de1c4f1 > src/mem/packet.hh a3b41de1c4f1 > > Diff: http://reviews.gem5.org/r/3259/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
