Ah, that's what I said days ago, but nobody seemed to respond to it. Nate
On Fri, Dec 16, 2011 at 9:08 AM, Andreas Hansson <[email protected]>wrote: > ...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]> 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
