Good points. I'm fine with deprecating the plain get()/set(); while their semantics is potentially useful, it shouldn't be the "default". getGE/setGE would be fine, though now that I think about it, we could also just switch to getLE/setLE, as the benefit of working with a big-endian architecture is purely theoretical for us.
Steve On Sun, Jul 5, 2015 at 11:03 AM Andreas Sandberg <[email protected]> wrote: > I my original proposal actually contained getGE()/setGE() that use the > guest endianness. I decided to scrap them since we don¹t really have any > devices using them. I can add them back into the patch I¹m intending to > post tomorrow. > > Another option for devices that want to use the same endianness as the > guest would be to query the systems endianness and store that in a > constant that¹s passed to get(ByteOrder). I would prefer this since it > doesn¹t clutter the API. > > As for deprecating get()/set(), I think we still want to do that in order > to catch endianness bugs. Their endianness was always confusing and we > should weed out any use of them in our internal code base. Code that isn¹t > in the main repo ought to be reviewed as well and if using guest > endianness is the desired mode of operation, that should be explicit. > > //Andreas > > On 04/07/2015 19:14, "Steve Reinhardt" <[email protected]> wrote: > > >I think there's a distinction here between device models that are trying > >to > >emulate real-world devices and experimental device models. In the latter > >case, it makes sense to use get() and set() as the are written, because > >you > >just want the simulated system to send you e.g. a 32-bit value that you > >can > >stick in a uint32_t and not have to worry about it. Chances are you're > >only > >using this with an ISA of one endianness, but if the device magically > >switches its apparent endianness when you go to use an ISA of the opposite > >endianness, then that's actually a good thing, because it keeps your > >software simpler. > > > >I can see where the last three categories would be valuable if you're > >modeling a specific real-world device that has a specific endianness, so > >I'm fine with adding them. I'm not sure what the use case is for getRaw() > >and setRaw() though. At least with the current get() and set() you're > >converting values into a known endianness, but these methods (as I > >understand them) would leave the device needing to know what the guest > >endianness is (which I argue it shouldn't), or else explicitly use > >gtoh/htog, in which case why not just keep what we already have? If you're > >just looking to do a byte-level transfer, you should use getPtr() and do a > >memcpy instead. > > > >So in summary my counter-proposal is: > >- Leave get()/set() alone, don't deprecate them > >- Add getLE()/setLE(), getBE()/setBE(), and get(ByteOrder)/set(ByteOrder) > >as proposed > >- Forget getRaw()/setRaw() unless there's a use case I'm not thinking of > > > >Steve > > > >On Fri, Jul 3, 2015 at 7:08 AM Andreas Sandberg <[email protected] > > > >wrote: > > > >> Fellow Developers, > >> > >> There have been more than a few cases when I have been working on memory > >> mapped devices that I have been a bit confused/annoyed by the semantics > >> of Packet::get() and Packet::set(). Currently, these two methods (which > >> are used pretty much everywhere we implement devices) assume that > >> devices are always of the same endianness as the host, i.e., they > >> perform a gtoh or htog endian conversion. This makes it very hard to > >> work with devices that are require a specific endianness. > >> > >> I suspect the only reason things work at the moment is that we don't > >> really support anything other than little endian on little endian in > >> full system mode at the moment. > >> > >> I'd like to change the get/set API to make endianness explicit. > >> Specifically, I'd like to add the following methods to Packet: > >> * getRaw() / setRaw() - Get data without conversion. > >> * getLE() / setLE() - Get data stored as little endian. > >> * getBE() / setBE() - Get data stored as big endian. > >> * get(ByteOrder) / set(value, ByteOrder) - For devices with > >> configurable byte order > >> > >> The plan is to keep the old get & set methods but to flag them as > >> deprecated once all devices have been transitioned to the new interface. > >> Flagging the old interface as deprecated means that we'll be able to > >> slowly transition old code without breaking anything. However, old code > >> would still trigger non-fatal compiler warnings. > >> > >> Any thoughts on this? > >> > >> Thanks, > >> Andreas > >> > >> > >> -- 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 > > > -- 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
