----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2493/#review5569 -----------------------------------------------------------
Ship it! Looks fine other than typo "paket" in commit message. Suggestion below is optional. src/mem/packet.hh <http://reviews.gem5.org/r/2493/#comment4992> Is this ever called from anywhere other than the Packet constructor now? If not, I suggest getting rid of it as a separate function, to make it clear that it's not really a separable action anymore. - Steve Reinhardt On Nov. 16, 2014, 10:14 p.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2493/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2014, 10:14 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10543:6666406b414c > --------------------------- > mem: Remove redundant Packet::allocate calls > > This patch cleans up the packet memory allocation confusion. The data > is always allocated at the requesting side, when a packet is created > (or copied), and there is never a need for any device to allocate any > space if it is merely responding to a paket. This behaviour is in line > with how SystemC and TLM works as well, thus increasing > interoperability, and matching established conventions. > > The redundant calls to Packet::allocate are removed, and the checks in > the function are tightened up to make sure data is only ever allocated > once. There are still some oddities in the packet copy constructor > where we copy the data pointer if it is static (without ownership), > and allocate new space if the data is dynamic (with ownership). The > latter is being worked on further in a follow-on patch. > > > Diffs > ----- > > src/dev/alpha/backdoor.cc 1a9e235cab09 > src/dev/alpha/tsunami_cchip.cc 1a9e235cab09 > src/dev/alpha/tsunami_io.cc 1a9e235cab09 > src/dev/alpha/tsunami_pchip.cc 1a9e235cab09 > src/dev/arm/a9scu.cc 1a9e235cab09 > src/dev/arm/amba_device.cc 1a9e235cab09 > src/dev/arm/amba_fake.cc 1a9e235cab09 > src/dev/arm/energy_ctrl.cc 1a9e235cab09 > src/dev/arm/gic_pl390.cc 1a9e235cab09 > src/dev/arm/hdlcd.cc 1a9e235cab09 > src/dev/arm/kmi.cc 1a9e235cab09 > src/dev/arm/pl011.cc 1a9e235cab09 > src/dev/arm/pl111.cc 1a9e235cab09 > src/dev/arm/rtc_pl031.cc 1a9e235cab09 > src/dev/arm/rv_ctrl.cc 1a9e235cab09 > src/dev/arm/timer_cpulocal.cc 1a9e235cab09 > src/dev/arm/timer_sp804.cc 1a9e235cab09 > src/dev/arm/vgic.cc 1a9e235cab09 > src/dev/copy_engine.cc 1a9e235cab09 > src/dev/i8254xGBe.cc 1a9e235cab09 > src/dev/ide_ctrl.cc 1a9e235cab09 > src/dev/isa_fake.cc 1a9e235cab09 > src/dev/mips/malta_cchip.cc 1a9e235cab09 > src/dev/mips/malta_pchip.cc 1a9e235cab09 > src/dev/ns_gige.cc 1a9e235cab09 > src/dev/pciconfigall.cc 1a9e235cab09 > src/dev/pcidev.cc 1a9e235cab09 > src/dev/sinic.cc 1a9e235cab09 > src/dev/sparc/dtod.cc 1a9e235cab09 > src/dev/uart8250.cc 1a9e235cab09 > src/dev/virtio/base.cc 1a9e235cab09 > src/dev/virtio/pci.cc 1a9e235cab09 > src/mem/cache/cache_impl.hh 1a9e235cab09 > src/mem/packet.hh 1a9e235cab09 > src/mem/packet.cc 1a9e235cab09 > > Diff: http://reviews.gem5.org/r/2493/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
