> On 2011-12-12 12:45:52, Geoffrey Blake wrote: > > Made a derived class FunctionalPacket to enable partial functional reads to > > not induce more overhead in the Packet class as requested by Steve, Nate > > and Ali. Modified blobHelper() in src/mem/port.cc to use FunctionalPacket > > instead of Packet. Over areas could use functional packets > > (src/arch/x86/pagetable_walker.cc and src/cpu/testers/memtest/memtest.cc) > > but have left them alone because they do not really need to be fixed. > > Tested by compiling and running using the CheckerCPU that exercises this > > code path heavily. No bugs found. > > Ali Saidi wrote: > Any arguments? > > Andreas Hansson wrote: > I conceptually think the new sub-class is a non-desirable solution and > would like to keep the packets defined by the protocol (not the accessing > mode or semantics of the mode). If we have multiple protocols in gem5 (e.g. > memory-mapped and cache maintenance, and eventually Ruby protocol, AMBA++ > etc) all with a packet class, we would have to make FunctionalPackets of each > and every one of them. It makes the memory-system changes very challenging. > > Is there any other way we can solve this?
Then we're back to the previous options: 1. Add the FunctionalPacket code back to the normal Packet and #ifdef USE_CHECKER around it. 2. Add a pointer to the class to a std::vector which contains the bytes and only use it if we some how know this packet is actually function 3. Do the ugly but perhaps effective union of a 64-bit bitmask and a vector 4. Place these bytes in the request object, but again, how do you know you're a functional packet? 5. Go back to the original implementation and call resize left. I think the important point is we need to pick one that we're happy with. I don't want to see Geoff have to implement all 5 before this code goes in. Thanks, Ali - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/915/#review1749 ----------------------------------------------------------- On 2011-12-12 12:37:51, Geoffrey Blake wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/915/ > ----------------------------------------------------------- > > (Updated 2011-12-12 12:37:51) > > > 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/SConscript c1ab57ea8805 > src/mem/functional_packet.hh PRE-CREATION > src/mem/functional_packet.cc PRE-CREATION > src/mem/packet.hh c1ab57ea8805 > src/mem/packet.cc c1ab57ea8805 > src/mem/port.hh c1ab57ea8805 > src/mem/port.cc c1ab57ea8805 > > 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
