...and as I just discussed with Ali: Option #7) If the valid bytes are consecutive (which I would assume they are), then why not just use two uints firstValidByte and nValidByte? For a normal packet firstValidByte is 0 and nValidByte is equal to the size. Only two words overhead to the packet and no dynamic datastructures.
Andreas From: [email protected] [mailto:[email protected]] On Behalf Of nathan binkert Sent: 16 December 2011 17:05 To: Ali Saidi Cc: Steve Reinhardt; Gabe Black; Geoffrey Blake; Andreas Hansson; Default Subject: Re: Review Request: Packet: Enable functional reads of partial data to packet class Option #6) If the functional Packet used the CRTP and was templated on the "regular" packet, I think that would solve this concern. So you'd create a FunctionalPacket<Packet> or a FunctionalPacket<FooProtocolPacket> Yes, somewhat nasty too. Nate On Fri, Dec 16, 2011 at 8:16 AM, Ali Saidi <[email protected]<mailto:[email protected]>> wrote: This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/915/ On December 12th, 2011, 12:45 p.m., 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. On December 15th, 2011, 2:50 p.m., Ali Saidi wrote: Any arguments? On December 16th, 2011, 1:47 a.m., 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 On December 12th, 2011, 12:37 p.m., Geoffrey Blake wrote: Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. By Geoffrey Blake. Updated 2011-12-12 12:37:51 Description 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. Testing Compiles. No functional changes made from CheckerCPU patch to this patch for packet class, and CheckerCPU fully exercised this code path during testing. 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) View Diff<http://reviews.m5sim.org/r/915/diff/> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
