With the #include method you could have multiple params for the same
simobject. I know mid-file includes are rare, but, like goto, there are
use-cases that make sense. In this case, imo, it improves clarity.
You can just inherit from MySimObject::Params, and you aren't restricted to
doing it in any specific source file.
$cat main.cc
#include <iostream>
#include <cstdlib>
#include "MySimObject.hh"
struct FooParams : MySimObject::Params {
char foo;
};
int
main( int argc, char *argv[]){
FooParams p;
p.myint = 5;
p.foo = 'f';
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 7:58 PM Gabe Black via gem5-dev <[email protected]>
wrote:
> [This email originated from outside of OSU. Use caution with links and
> attachments.]
> Also that (like the other mechanisms) means that there can be at most one
> C++ class for a given Params class. It would be impossible to have
> FooParams -> SimObj and BarParams -> SimObj since there is only room for
> one Params in a class.
>
> That said, we could turn the FooParams and BarParams into Params::Foo and
> Params::Bar. That would be fairly trivial, and would mean we'd only have
> one name to dodge, Params, although that's an easier name to collide with
> accidentally.
>
> I think a step in the right direction would be to start putting all our
> gem5 stuff into a gem5 namespace. Namespaces are easy to fold away in
> context (using, opening the namespace and working within it), and then we
> would only have to worry about internal collisions which I think are a lot
> easier to manage.
>
> We could even start doing that now without breaking compatibility by
> putting things in a gem5 namespace and then putting a "using namespace
> ::gem5;" at the end to maintain compatibility until we decide to switch.
>
> Gabe
>
> On Thu, Oct 22, 2020 at 7:50 PM Gabe Black <[email protected]> wrote:
>
>> That would work, but we don't have includes in the middle of a class like
>> that anywhere else in gem5 (that I can remember at least), which I think is
>> for the best :-).
>>
>> Gabe
>>
>> On Thu, Oct 22, 2020 at 6:28 PM Gambord, Ryan via gem5-dev <
>> [email protected]> wrote:
>>
>>> @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
>>
>> _______________________________________________
> 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
