Thanks for the response. I guess my main point is that your items can be broken into two groups, 1-3 dealing with Requests and 4-5 dealing with Packets, which are actually largely independent. So I was not trying to deal with everything monolithically, but pointing out that we could further subdivide it. In fact I fear we are confusing things by trying to do this all in one shot, without being clear on the distinctions between these two categories. In fact, your suggestion that acquire & release should not be request flags since we have ways to represent them in packets seems not to acknowledge that we do need to represent them at the CPU/ISA interface where all we have is request flags and do not have a packet in which to encode those attributes.
Now if we do a really deep redesign of these interfaces, then these issues may no longer be so independent, but that would be a big change, and would mean we'd have to deal with all these issues more monolithically :). Steve On Fri, Aug 7, 2015 at 2:03 AM Andreas Hansson <[email protected]> wrote: > Hi Steve, > > Thanks for the clarification. I’d like to try and get back to my numbered > items again as this discussion is too big to deal with monolithically. > > The issue I have here is (3). There is no reason to have a one-hot-encoded > bit mask to denote a set of ‘commands’ that are all mutually exclusive. I > really think we need to redesign this interface, especially if we’re > suddenly to extend the list significantly. I am not sure what the best > route is, but it sounds like there is room for a request command/action > enum? > > Andreas > > On 07/08/2015 06:40, "gem5-dev on behalf of Steve Reinhardt" > <[email protected] on behalf of [email protected]> wrote: > > >So the original intent (loosely captured at > >http://gem5.org/General_Memory_System#Packets) is that a Request is the > >interface between the ISA and a CPU, while the Packet is what is conveyed > >between two components in the memory system (the CPU and the L1 cache, or > >two adjacent caches, or a cache and the memory controller). > > > >Notice that in the ISA description language, memory accesses are conveyed > >to the execution context using readMem and writeMem, which basically just > >take an address and a set of Request flags---not even an actual Request > >object, but the raw materials to make one There is no "command" in a > >request; you get one out-of-band bit depending on whether you call readMem > >or writeMem, and every other attribute of an access must be passed as a > >flag. > > > >Note also that I'm not *defending* this interface, merely *describing* it > >:). > > > >Given this interface, though, I don't see how we can avoid adding acquire > >and release flags to the Request object, as there's no other way to signal > >from the instruction to the CPU that that's what's desired. Any other > >approach would really mean changing the ISA/execution context interface. > > > >For better or for worse, all this Request stuff is largely orthogonal to > >the Packet interface and its MemCmd component. I actually kind of like > >how > >that works with the enum, lookup table and attributes---the attributes > >give > >you the orthogonality of being able to specify something like > >NeedsExclusive that you can check as needed without comparing against a > >long and ever-changing list of commands, while the enum and lookup table > >mean you are only occupying a minimal amount of space in the Packet object > >itself. > > > >The downside of the orthogonality is that there's also a lot of > >redundancy. I think they're more distinct than they appear on the surface > >though. For example, the Request object doesn't have to encode anything > >that's directly related to the coherence protocol, as that's out of the > >scope of the ISA. (Not so for the consistency model, though.) Also, you > >do need a fair degree of decoupling; the ISA may have generated a Request > >for a write of 8 bytes, but out in the memory hierarchy that may well turn > >into a ReadExReq or UpgradeReq of the cache block size. > > > >I certainly agree that the whole system has accumulated some baggage and > >could stand a good spring cleaning, if not a little downright > >re-architecting. There is a method to the madness though. > > > >Steve > > > > > > > >On Thu, Aug 6, 2015 at 4:22 PM Marc Orr <[email protected]> wrote: > > > >> Thanks Andreas and Tony, > >> > >> I think I understand Andreas' original email AFTER reading Tony's > >>response > >> :-). > >> > >> In addition to the two options Tony suggested (enumerated below): > >> > >> - Use regular read/write commands but have special flags denoting > >>them > >> as ACQUIRE/RELEASE (2) > >> - Or we could have multiple commands for the each of the > >> sync/serializing ops with attributes denoting them as > >>IsSynchornizing or > >> IsSerializing etc (4,5) > >> > >> I would add: > >> > >> - Add a new "regular" command: FENCE. Have special flags to denote > >>it as > >> ACQUIRE/RELEASE (2) > >> - Add a new "regular" command: FENCE. Use attributes to capture the > >> properties of a particular FENCE command: IsAcquire, IsRelease, etc. > >> (4,5) > >> > >> Any others? > >> > >> Also, on point (3) in Andreas' original email: I agree that Architecture > >> specific flags are not handled very well right now. I noticed that the > >> first 8 bits in request.hh (ARCH_BITS) are supposed to be architecture > >> specific (according the comment at least). I wonder if those bits are > >> sufficient to solve the problem that Andreas raises. > >> > >> Thanks, > >> Marc > >> > >> > >> > >> On Thu, Aug 6, 2015 at 3:18 PM, Gutierrez, Anthony < > >> [email protected]> wrote: > >> > >> > Thanks for this clarification, Andreas. We were mostly confused by the > >> > flags, cmd, and attributes because - as you pointed out - there seems > >>to > >> be > >> > a lot of overlap with no clear division between the three objects. I > >> think > >> > this patch is going in a good direction. > >> > > >> > For our particular case it seems we have a few options. Use regular > >> > read/write commands but have special flags denoting them as > >> ACQUIRE/RELEASE > >> > (2). Or we could have multiple commands for the each of the > >> > sync/serializing ops with attributes denoting them as IsSynchornizing > >>or > >> > IsSerializing etc (4,5). > >> > > >> > Are those suitable options? Are there other options? > >> > > >> > -----Original Message----- > >> > From: gem5-dev [mailto:[email protected]] On Behalf Of > Andreas > >> > Hansson > >> > Sent: Thursday, August 06, 2015 4:43 PM > >> > To: gem5 Developer List > >> > Subject: Re: [gem5-dev] extending request with more attributes > >> > > >> > Hi Marc (et al), > >> > > >> > Have a look at http://reviews.gem5.org/r/3004/ for addressing (5) in > >>my > >> > previous mail. > >> > > >> > Andreas > >> > > >> > On 06/08/2015 10:41, "gem5-dev on behalf of Andreas Hansson" > >> > <[email protected] on behalf of [email protected]> > >>wrote: > >> > > >> > >Hi Marc, > >> > > > >> > >I would suggest the following (again, this is up for discussion): > >> > > > >> > >1. Request flags should the kind of properties that we would have > >> > >associated with page-table entries. I am thinking memory attributes > >> > >like uncacheable, strictly ordered, secure, etc. > >> > > > >> > >2. Request flags could also be used to distinguish packets of > >>specific > >> > >types, where ‘normal’ packet commands like read/write are used, but > >>we > >> > >want to be able to single out what the ‘reason’ is. Prefetch, PT > >>walks, > >> > >Fetch, etc. > >> > > > >> > >3. We should try and sort out the current use of CLEAR_LL, > >>LOCKED_RMW, > >> > >LLSC, MEM_SWAP, and MEM_SWAP_COND. In my view these should not really > >> > >be request flags, because we have specific packets to represent these > >> > >actions. Also, most of these are ArchSpecific and should go in the > >> > >ArchSpecific flags. Perhaps we just need to figure out a better way > >>to > >> > >do this. > >> > > > >> > >4. Packet attributes should be common attributes shared by multiple > >> > >families of packet commands. Good examples are ‘NeedsResponse’, > >> > >‘IsRead’, ‘NeedsExclusive’ etc. > >> > > > >> > >5. Packet attributes should not be introduced if they only apply to a > >> > >single request/response. Thus, I think we should remove IsSWPrefetch > >> > >and IsHWPrefetch as an attribute. > >> > > > >> > >Does this align with your view of the world? > >> > > > >> > >Andreas > >> > > > >> > > > >> > > > >> > >On 05/08/2015 22:30, "gem5-dev on behalf of Marc Orr" > >> > ><[email protected] on behalf of [email protected]> wrote: > >> > > > >> > >>In response to reviews http://reviews.gem5.org/r/2821/ and > >> > >>http://reviews.gem5.org/r/3003/. > >> > >> > >> > >>I was going to look at addressing this today for AMD. I'm not > >> > >>extremely familiar with the organization of the request flags, > >>packet > >> > >>commands, packet attributes, and packet flags. That said, I was > >>under > >> > >>the impression that the request is supposed to capture information > >> > >>that exist at the instruction level. To me, acquire and release are > >>at > >> > >>the instruction level (not a spurious command between two memory > >> > >>controllers like flush command). > >> > >> > >> > >>Also, the 32 bits that we have to work with in the request seems > >> > >>rather small. Especially, when you consider that it needs to capture > >> > >>all of the possible attributes for a memory request across all of > >>the > >> > >>different ISAs... > >> > >> > >> > >>How would armv8 ldra and strl instructions convey to the cache > >> > >>hierarchy acquire/release attributes from the instruction (if it > >>were > >> > necessary)? > >> > >> > >> > >>Thanks, > >> > >>Marc > >> > >>_______________________________________________ > >> > >>gem5-dev mailing list > >> > >>[email protected] > >> > >>http://m5sim.org/mailman/listinfo/gem5-dev > >> > > > >> > > > >> > >-- 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. > >> > > > >> > >ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > >> > >Registered in England & Wales, Company No: 2557590 ARM Holdings plc, > >> > >Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in > >> > >England & Wales, Company No: 2548782 > >> > >_______________________________________________ > >> > >gem5-dev mailing list > >> > >[email protected] > >> > >http://m5sim.org/mailman/listinfo/gem5-dev > >> > > >> > > >> > -- 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. > >> > > >> > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > >> > Registered in England & Wales, Company No: 2557590 ARM Holdings plc, > >> > Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in > >> > England & Wales, Company No: 2548782 > >> > _______________________________________________ > >> > gem5-dev mailing list > >> > [email protected] > >> > http://m5sim.org/mailman/listinfo/gem5-dev > >> > _______________________________________________ > >> > gem5-dev mailing list > >> > [email protected] > >> > http://m5sim.org/mailman/listinfo/gem5-dev > >> > > >> _______________________________________________ > >> gem5-dev mailing list > >> [email protected] > >> http://m5sim.org/mailman/listinfo/gem5-dev > >> > >_______________________________________________ > >gem5-dev mailing list > >[email protected] > >http://m5sim.org/mailman/listinfo/gem5-dev > > > -- 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. > > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > Registered in England & Wales, Company No: 2557590 > ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, > Registered in England & Wales, Company No: 2548782 > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
