@Gabe Black <[email protected]>
Here is an example of what this looks like. It requires slightly modifying
the boilerplate code generated for params structs, and then moving the
include from the top of the source file to inside the class. This brings
the struct into the class namespace.
This should work fine as long as pybind can work with a FQN of an object.
MySimObject.hh
#ifndef __MYSIMOBJECT_HH__
#define __MYSIMOBJECT_HH__
class MySimObject
{
public:
#include "MySimObjectParams.hh"
public:
MySimObject(Params * p) : myint(p->myint) {}
int & myInt() { return myint; }
private:
int myint;
};
#endif // __MYSIMOBJECT_HH__
MySimObjectParams.hh
// Generated automatically by SCons //
struct Params {
int myint;
};
main.cc
#include <iostream>
#include <cstdlib>
#include "MySimObject.hh"
int
main( int argc, char *argv[]){
MySimObject::Params p;
p.myint = 5;
MySimObject my_so_instance(&p);
std::cout << my_so_instance.myInt() << std::endl;
return EXIT_SUCCESS;
}
Ryan Gambord
<[email protected]>
On Thu, Oct 22, 2020 at 5:20 PM Gabe Black via gem5-dev <[email protected]>
wrote:
> [This email originated from outside of OSU. Use caution with links and
> attachments.]
> I definitely agree that we need to use more namespaces and be better
> citizens in the global namespace.
>
> See https://gem5.atlassian.net/browse/GEM5-730
>
> Mechanically, I don't think we can define something like
> Mem::Ruby::Network::BasicLink::Params since that would require adding a
> Params type to the not yet defined BasicLink class. You can do that with
> namespaces, but not with classes. We also couldn't add a params function to
> the SimObject for the same reason. Python doesn't define the SimObject
> class at all, even though it might look like it does because the python
> objects and C++ objects often (but not always) share the same name. The
> Python objects actually are the Params structs in C++, and then python uses
> the create method to make the actual SimObjects. The real C++ SimObject
> classes have no python analogue, although python has pointers to them and
> can indirectly call methods on them once they've been instantiated. That's
> how callbacks like init(), startup(), etc are handled.
>
> Gabe
>
> On Thu, Oct 22, 2020 at 10:25 AM Gambord, Ryan via gem5-dev <
> [email protected]> wrote:
>
>> While we're thinking about the params implementations, I'd also like to
>> propose that we stop cluttering the global namespace with collision-prone
>> names:
>>
>> src/.../.../MySimObject.hh
>> namespace NoClutter {
>> class MySimObject : ::SimObject
>> {
>> #include params/MySimObject.hh
>> };
>> };
>>
>> params/MySimObject.hh
>> // ... //
>> struct Params {
>> // ... //
>> };
>>
>> For example, ruby claimed the name "BasicLink", which is pretty generic.
>> Would be much better if we started moving towards
>> Mem::Ruby::Network::BasicLink::Params, which would be more idiomatic C++.
>> The params/xxx.hh sources could even include the boilerplate params()
>> method into the simobject class if you want. There is *the* canonical
>> use-case for mid-file includes -- when we have compile-time generated
>> boilerplate in separate sources.
>>
>> Right now, gem5 gives me "C with classes" vibes.
>>
>> Ryan Gambord
>> <[email protected]>
>>
>>
>>
>> On Thu, Oct 22, 2020 at 9:18 AM Jason Lowe-Power via gem5-dev <
>> [email protected]> wrote:
>>
>>> [This email originated from outside of OSU. Use caution with links and
>>> attachments.]
>>> Another simple proposal:
>>> - Remove params() from the API (with deprecation)
>>> - Update objects that can easily be updated and remove the params()
>>> calls/casts. I think this is most cases of the params use outside of the
>>> constructor.
>>> - Don't worry about the others that use the params() function in many
>>> places that require deeper updates
>>> - Update the documentation to say that the "best practice" is to not use
>>> the param function
>>> - In the future, make sure that new objects don't store the params
>>> object.
>>>
>>> Cheers,
>>> Jason
>>>
>>> On Thu, Oct 22, 2020 at 8:51 AM Giacomo Travaglini <
>>> [email protected]> wrote:
>>>
>>>> Let me add to the plate a simple proposal though still a bit verbose
>>>> and probably not that different from a manual cast
>>>>
>>>>
>>>>
>>>> Defining a template method in SimObject:
>>>>
>>>>
>>>>
>>>> *template <class Obj>*
>>>>
>>>> *Obj params()*
>>>>
>>>>
>>>> *{ return static_cast<Obj>(_params);*
>>>>
>>>> * Or more secure:*
>>>>
>>>> * param = dynamic_cast<Obj>(_params);*
>>>>
>>>> * assert(param);*
>>>>
>>>> * return param;*
>>>>
>>>> *}*
>>>>
>>>>
>>>>
>>>> And delegate the type specification on the client side:
>>>>
>>>>
>>>>
>>>> For example in my shiny new
>>>>
>>>>
>>>>
>>>> Class MyObject : public SimObject {}
>>>>
>>>>
>>>>
>>>> I would use
>>>>
>>>>
>>>>
>>>> auto p = params<MyObjectParams*>();
>>>>
>>>>
>>>>
>>>> You still have to type your param type so that doesn’t make it the best
>>>> but I believe it’s the simplest one
>>>>
>>>>
>>>>
>>>> Giacomo
>>>>
>>>>
>>>>
>>>> *From:* Jason Lowe-Power via gem5-dev <[email protected]>
>>>> *Sent:* 22 October 2020 16:26
>>>> *To:* gem5 Developer List <[email protected]>
>>>> *Cc:* Gabe Black <[email protected]>; Jason Lowe-Power <
>>>> [email protected]>
>>>> *Subject:* [gem5-dev] Re: SimObject params() method
>>>>
>>>>
>>>>
>>>> Hey Gabe,
>>>>
>>>>
>>>>
>>>> Thanks for bringing this up. I have also been bothered by the lack of
>>>> consistency with how params are used. I can't think of an example of when
>>>> you need to store the params object. I would be all for getting rid of the
>>>> params() function and updating the documentation to say that it's best
>>>> practice to *not* save the params struct after the constructor. If some
>>>> object had a good reason to go against this best practice, that would be
>>>> OK, and we wouldn't need to enforce any specific design or pattern on these
>>>> exceptions. I would prefer to remove the params() function than add more
>>>> template magic.
>>>>
>>>>
>>>>
>>>> On Thu, Oct 22, 2020 at 1:18 AM Gabe Black via gem5-dev <
>>>> [email protected]> wrote:
>>>>
>>>> Hi folks. I'm looking at the SimObject header, and I see a few things
>>>> in there which are marked as part of the API and maybe shouldn't be.
>>>> Specifically I'm talking about the Params typedef, and the params() method.
>>>> There is also the _params member variable which I can see being a part of
>>>> the API since it can be used by other classes to make their own params()
>>>> function (more on that below), but the Params typedef is more or less an
>>>> implementation detail, and the params() method is essentially worthless
>>>> because it returns a SimObjectParams which doesn't have anything except the
>>>> object's name in it, and you can already get that with the name() method.
>>>>
>>>>
>>>>
>>>> I agree. I think the typedef is useless and shouldn't be in the API.
>>>> It's unfortunate that the API proposals didn't get more reviews. I think
>>>> it's probably OK to just drop that from the API, but the params() function
>>>> itself is going to need to be deprecated.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> *Params pattern*
>>>>
>>>>
>>>>
>>>> This gets to the whole Params params() pattern which is sporadically
>>>> implemented in some SimObjects in gem5. This pattern is that a given
>>>> SimObject subclass will define a Params typedef which corresponds to its
>>>> Params struct type, and then also define a params() method which returns
>>>> the _params from SimObject cast up to that type.
>>>>
>>>>
>>>>
>>>> The Params typedef itself primarily makes the definition of the
>>>> constructor a little more legible, since the FullFooTypeForTheArmISAParams
>>>> can be really verbose.
>>>>
>>>>
>>>>
>>>> I think verbose is fine. I would vote to abolish all params typedefs.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> Storing the params struct itself could theoretically serve the purpose
>>>> of having a bunch of parameters and not having to create a member variable
>>>> for each one, spend a bunch of text copying values over in the constructor,
>>>> etc. I think most of the time this is unnecessary, but if an object has
>>>> tons of values in it for some reason this could make sense.
>>>>
>>>>
>>>>
>>>> I don't think there are any examples of this in the codebase. I think
>>>> in all cases the params data is copied into member variables. If there are
>>>> cases where data isn't copied, I doubt it was with a strong reason in mind.
>>>> The one exception to this might be Ruby, but it's an exception for all the
>>>> wrong reasons ;).
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> The params() method then makes the _params member available with the
>>>> appropriate type, so that all the FooParams members are accessible. It also
>>>> makes the params struct accessible outside the object which is used in a
>>>> place or two to read parameters without there needing to be a member in the
>>>> object, and probably some sort of accessor to read it.
>>>>
>>>>
>>>>
>>>> There are two main problems with this system. First, when used, it adds
>>>> a small but not trivial amount of boilerplate to any given class. Second,
>>>> it's very sporadically implemented. I think in a lot of places it's there
>>>> just because people aren't sure what it's for or if they need it, so they
>>>> just put one there regardless. I think I've done that in the past.
>>>>
>>>>
>>>>
>>>> *Alternative*
>>>>
>>>>
>>>>
>>>> The existence of the Params type and the params() method could be
>>>> partially eliminated by defining a templated params() method which took a
>>>> SimObject reference and/or pointer as its first parameter. It could then
>>>> figure out what Params struct went with that SimObject type using
>>>> typedef/template magic set up by the Params building code, and then perform
>>>> the appropriate cast.
>>>>
>>>>
>>>>
>>>> This has three downsides, two minor and one more serious. The minor one
>>>> is that when a class uses this method internally, it would have to do
>>>> something like params(this) instead of just params(). That's a fairly minor
>>>> difference and not that big a deal. For external consumers that would be
>>>> less of a problem since it would change from foo->params() to params(foo).
>>>>
>>>>
>>>>
>>>> The second minor issue is that the name params() is very short, and
>>>> likely to collide with other names. We could define that with SimObject as
>>>> a static method, but then that would make foo->params() turn into the more
>>>> verbose SimObject::params(foo), or (and I haven't checked if this is legal
>>>> syntax) the more odd looking foo->params(foo). The params() class can't be
>>>> a non-static method, because then the type of "this" would be fixed by
>>>> where it was defined, meaning it would not cast _params to the right type.
>>>> I was not able to find any mechanism in c++ that would let you treat "this"
>>>> as an argument for template type resolution.
>>>>
>>>>
>>>>
>>>> The third more serious problem is that this implicitly breaks the
>>>> ability to use two different SimObject types in python to represent the
>>>> same SimObject type in C++. I don't know if this happens in practice, and
>>>> it's also broken by the original Params pattern, since there can be only
>>>> one typedef in a given class. Since Params is applied adhoc manually,
>>>> something that is generally not good, it actually avoids this problem by
>>>> just not existing anywhere that would break that assumption.
>>>>
>>>>
>>>>
>>>> *Other option*
>>>>
>>>>
>>>>
>>>> Another option would be to have a templated class which would define a
>>>> Params type and a params() method, and inherit that into each SimObject
>>>> which wants to have those members. It would itself inherit from its
>>>> parameter which would keep the inheritance hierarchy intact, and make it
>>>> possible to override Params and params from super classes:
>>>>
>>>>
>>>>
>>>> FooObject : public ParamsMembers<SimObject>
>>>>
>>>>
>>>>
>>>> This has a similar problem to the above if exactly what Params type to
>>>> use is automatic, although here it could be an additional template
>>>> argument. This also trades some boilerplate for less boilerplate, has to be
>>>> applied manually to any classes that want to take advantage of it, and
>>>> obfuscates the definition of those classes.
>>>>
>>>>
>>>>
>>>> Gabe
>>>>
>>>> _______________________________________________
>>>> gem5-dev mailing list -- [email protected]
>>>> To unsubscribe send an email to [email protected]
>>>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>>>>
>>>> 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]
>>> To unsubscribe send an email to [email protected]
>>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>>
>> _______________________________________________
>> gem5-dev mailing list -- [email protected]
>> To unsubscribe send an email to [email protected]
>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>
> _______________________________________________
> gem5-dev mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
