I don't think changing BitUnions is either advisable or will solve the problem, but if you have any particular ideas please let me know since I might be overlooking something.
I see what you're saying as far as having lots of functions, but I don't actually think it's a problem. A simple wrapper like I proposed would be inlined and so not actually take up space in the binary, and at the callsight since the implementation of the wrapper is trivial it shouldn't take up any space their either. Then you're calling the actual function, but specialized on the base type, ie uint64_t, etc. Those will all be the same function for anything based on a particular underlying type, and so there won't be any extra copies of functions. You also don't have to store data or pass data to other functions as BitUnions to get the functional benefits. You can store them as the underlying types, and then just put them into BitUnions when you want to manipulate their bitfields. If there's a specific example you'd like to address that would be helpful. In the abstract I don't think there's anything that needs to be done, but there could be circumstances I'm not thinking of. Gabe On Thu, Sep 20, 2018 at 4:36 AM Giacomo Travaglini < [email protected]> wrote: > Hi Gabe, > > > Thanks for your reply. Before I clarify what I am trying to do in the > checkpointing subsystem, let me say > > that even if my concerns started after I had a look at the serializing > code, this is more like a general problem. > > *It potentially affects every function which is making use > of BitUnionType<T>.* > > > So to be honest I am more interested on this, on being able to provide > another interface for > > BitUnions where you make distinction among registers (in argument type > deduction) only > > based on the underlying storage type (uint8_t, uint16_t, uint32_t, > uint64_t). so that a template will generate > > *only one function* if it receives two different 32bit registers. > > > Now going to the serializing code > > Doing something like > > > *template <typename T>* > *void arrayParamOut(CheckpointOut &cp, const std::string &name,* > * const BitUnionType<T> *u, unsigned size)* > *{* > * arrayParamOut(cp, name, static_cast<BitUnionBaseType<T> *>(u), size);* > *}* > > as you correctly mentioned works for SERIALIZE_ARRAY, but has three > problems IMHO. > > > 1) you cannot do explicit instantiation (for the reasons mentioned in > previous email) hence this has to live in the serialize.hh > > 2) It will still create one different arrayParamOut function for every > register, which means big binary and longer compilation time > > 3) This approach will require us to create other templates, for > SERIALIZE_CONTAINER (STL version) > > > *template <class T>* > *void arrayParamOut(CheckpointOut &cp, const std::string &name,* > * const std::vector<BitUnionBaseType<T>> > ¶m);* > > *template <class T>* > *void arrayParamOut(CheckpointOut &cp, const std::string &name,* > * const std::list<BitUnionBaseType<T>> > ¶m);* > > *template <class T>* > *void arrayParamOut(CheckpointOut &cp, const std::string &name,* > * const std::set<BitUnionBaseType<T>> > ¶m);* > > > Now we don't actually need to do this. We should in theory serialize > containers of BitUnions > just by using the already existing (vector example) > > *template <class T>* > *void arrayParamOut(CheckpointOut &cp, const std::string &name,* > * const std::vector<T> ¶m);* > > This is because If we want to specialize for BitUnions, I think we should > do this in *showParam *and* parseParams *rather than paramIn/paramOut > (src/sim/serialize.cc). > In fact, that's actually where the real datatype matters: paramIn/Out are > just wrappers > to the show/parseParam and they depend on the container type more than > the datatype in the container. > As an example (simplest one): > > template <class T> > void > paramOut(CheckpointOut &os, const string &name, const T ¶m) > { > os << name << "="; -> DataType agnostic > showParam(os, param); -> DataType dependent, specialize > this for BitUnion > os << "\n"; -> DataType agnostic > } > > arrayParamOut(CheckpointOut &os, const string &name, const list<T> ¶m) > { > typename list<T>::const_iterator it = param.begin(); > > os << name << "="; > if (param.size() > 0) > showParam(os, *it); > it++; > while (it != param.end()) { > os << " "; > showParam(os, *it); > it++; > } > os << "\n"; > } > > Consider that *parseParam* and *showParam* are the building block of > every serialization (scalar, array and container). > *If we specialize them for BitUnion (only two functions),** we unlock STL > and ARRAY serialization of BitUnions for free* > *(without the need of specializing all the STL templates)* > > So if we do that we need to > > a) Move showParam/parseParam from serialize.cc to serialize.hh > b) Explicit instantiations we are doing for int,char and other integral > types. > > * But we cannot explicitly instantiate BitUnions!!! * > > And in both a) and b) you have the problem of the the templates bloating > your binary > > I hope this explains how I got to the point and why I feel the need of > changing the BitUnion template interface so that something like this is > possible > > BitUnion<T> where T is uint32_t, uint64_t etc so that I can do explicit > instantiation > > Let me know if you have any suggestions > > Thanks > > Giacomo > > > ------------------------------ > *From:* Gabe Black <[email protected]> > *Sent:* 20 September 2018 00:15:22 > *To:* gem5 Developer List; Giacomo Travaglini > *Cc:* Jason Lowe-Power; Potter, Brandon; Andreas Sandberg; Jui-min Lee; > En-Hao Chang > *Subject:* Re: Change in gem5/gem5[master]: base: Enable specializing > templates on BitUnion types. > > Hi Giacomo. Since this is more of a conversation than commenting on that > particular CL, I've moved this over to the dev list. > > What you're talking about seems very reasonable, and I think should be > supported by the templates you're referring to. I *think* something like > the following should work: > > template <typename T> > void arrayParamOut(CheckpointOut &cp, const std::string &name, > const BitUnionType<T> *u, unsigned size) > { > arrayParamOut(cp, name, static_cast<BitUnionBaseType<T> *>(u), size); > } > > The reason this should work is that a basic assumption about BitUnion > types is that they're just > a lot of syntactical wrapping around the underlying type, ie all that's > actually stored in memory for a BitUnion64 is a single uint64_t, and all > the other stuff just operates on that underlying storage. Without making > any additional assumptions, at least as far as I can see, you should be > able to treat an array of BitUnions in memory as if they were just an array > of the underlying type. > > > In the past, I've sent an email or two to the dev list about how I had > thought the checkpointing mechanism could be improved. I'd have to dig up > the old emails where I went through it all, but I think the gist of it was > to make the checkpoint hierarchically structured, so that it was easier to > see if some piece of it was, for instance, not consumed when resuming a > checkpoint, and to make it easier to structure data in the checkpoint in > each object's little area. I think I suggested json for that purpose. > Andreas also had some ideas about having blobs of data like a cache's > contents, etc., off in separate files in a standardized way. Unfortunately > I haven't had any time allocated to that and haven't been able to work on > it, and it may be outside of the scope of what you're doing, but if you're > doing more than just code cleanup it would be good to brainstorm about > those sorts of things and make sure those ideas get considered. > > Relatedly, we're looking at how checkpoints work internally, and someone > else (cc-ed) is actually looking at making a checkpointing 2.0 type > mechanism which we'd like to integrate into gem5 and which would, > presumably, have nice features like what we talked about before. They asked > me to tell them about how the existing mechanism works, and my intention > was to write up a doc describing the system as it exists now so that it's > documented upstream on the gem5 website, in addition to having something I > can point my colleague to. > > If you're already digging through that code though, you almost certainly > have a better idea how it all works right now than I do, and will > particularly know what parts your changing and what you're changing them > to. Would you mind doing some sort of write up about how checkpointing is > done today that I can point them to? > > Gabe > > On Wed, Sep 19, 2018 at 7:54 AM Giacomo Travaglini (Gerrit) < > [email protected]> wrote: > > View Change <https://gem5-review.googlesource.com/c/public/gem5/+/7202> > > 1 comment: > > - > > File src/base/bitunion.hh: > <https://gem5-review.googlesource.com/#/c/7202/3/src/base/bitunion.hh> > - > > Patch Set #3, Line 366: > > <https://gem5-review.googlesource.com/#/c/7202/3/src/base/bitunion.hh@366> > void > func(BitUnionType<T> u) { BitUnionBaseType<T> b = u; } > > Hi Gabe, > > I am currently trying to clean up the serializing code and to extend > the SERIALIZE_CONTAINER and SERIALIZE_ARRAY to the BitUnion type. > Coming across the BitUnionType, I'd have some questions and some > comments > about its usage. > > From what I see in the following lines of code > > void func(BitUnionType<T> u) { BitUnionBaseType<T> b = u; } > > [...] > > using BitUnionType = BitfieldBackend::BitUnionOperators<T>; > > [...] > > typedef BitfieldBackend::BitUnionOperators< \ > BitfieldUnderlyingClasses##name> name; > > the template parameter T is actually something like > BitfieldUnderlyingClassesCPSR > or BitfieldUnderlyingClassesHCR etc. (CPSR and HCR are arm > registers just in case) > This has the following effect: > > 1) It will be quite difficult and undersirable to do explicit > instantiation of > a function which is taking a BitUnionType as a parameter (see > "func" above) since I will > have to instantiate it for every register. > I could live without explicit instantiation, however the main > problem is the following one > > 2) We generate a different version of func for every single > register. > I am actually concerned about this more: the template spawning > gazillion functions for every > defined BitUnion, and bloating your binary. > We can say that in general a template function doesn't receive all > the registers as parameters, but this is not true for general functions > like the serializing ones (paramIn and paramOut in > src/sim/serialize.hh). > In theory I'd like to serialize most of the BitUnions. > > My question is: do you think there is a way for using the > underlying storage type rather than > BitUnionType<T>? To me it should all be reduced to having 4 > template instantiations: for 8/16/32/64 bit registers, so that > > BitUnion32(Reg1) > BitUnion32(Reg2) > > Use the same function. > > Please let me know your considerations > > Thanks > > To view, visit change 7202 > <https://gem5-review.googlesource.com/c/public/gem5/+/7202>. To > unsubscribe, or for help writing mail filters, visit settings > <https://gem5-review.googlesource.com/settings>. > Gerrit-Project: public/gem5 > Gerrit-Branch: master > Gerrit-Change-Id: Ia6c94de92986b85ec9e5fcb197459d450111fb36 > Gerrit-Change-Number: 7202 > Gerrit-PatchSet: 3 > Gerrit-Owner: Gabe Black <[email protected]> > Gerrit-Reviewer: Andreas Sandberg <[email protected]> > Gerrit-Reviewer: Brandon Potter <[email protected]> > Gerrit-Reviewer: Gabe Black <[email protected]> > Gerrit-Reviewer: Jason Lowe-Power <[email protected]> > Gerrit-CC: Giacomo Travaglini <[email protected]> > Gerrit-Comment-Date: Wed, 19 Sep 2018 14:54:06 +0000 > Gerrit-HasComments: Yes > Gerrit-Has-Labels: No > Gerrit-MessageType: comment > > 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
