Hello Gabe,

1. Const.

2. References.
3. I agree; however, in a higher level, what is the purpose of params()? Most 
of the classes I have seen simply duplicate the parameters as member variables, 
and in a way it serves to "expose" them so that the user becomes aware of their 
existence. Is keeping them hidden behind the params() call a decision we want 
to concede?


Regards,
Daniel
    Em terça-feira, 7 de janeiro de 2020 17:10:42 GMT+1, Jason Lowe-Power 
<ja...@lowepower.com> escreveu:  
 
 Hey Gabe,

Real quick reply as I'm not sure how much time I'll have to dig into this
in the next couple of days.

1. I agree it should be const everywhere.
2. I agree it should be references everywhere.
3. I'll have to think about this more :).

If you're planning on working on 1 & 2 or you would like some help with
these, please create a Jira issue so we can track it.

Thanks,
Jason

On Mon, Jan 6, 2020 at 6:04 PM Gabe Black <gabebl...@google.com> wrote:

> Hi folks. Right now we have a mishmash of ways to store the params
> structure passed to a SimObject class, if that class actually does store
> the params in the first place.
>
> The base SimObject class itself stores a const pointer to the very base
> SimObjectParams type. It defines a params() method which returns it as a
> const pointer.
>
> Some other classes (I only see the base System class atm, although there
> may be others) declare their own pointer locally. In the case of System,
> this pointer is not const, but the function that returns it makes it const
> (and casts it to const before returning it for some reason?).
>
> Other classes still declare a shadow _params member which is a const &, and
> return that from their params() method.
>
> Then beyond that, some types have constructors which expect a pointer to
> the params struct, and others expect a reference.
>
> This can result in a lot of confusion and annoying compile errors where you
> have to go in and add &s and *s in random places to get to the type you
> want. We should pick something and standardize on it.
>
>
> 1. Const vs. non-const. My vote is const. The params structs are set up in
> python, and the are often not looked at again once SimObjects are
> instantiated, although they may be. Allowing them to be fiddled with
> introduces the possibility of their value being stale and not reflected in
> the actual behavior of the simulation. These structures should be a one way
> uninterrupted conduit for config values from python to C++, passed through
> exactly once.
>
> If the params are treated as const (and once that's consistently applied),
> then the create() method can be marked const which will enforce that going
> forward.
>
>
> 2. Pointer vs. reference. Pointers were the historic implementation, and
> mixing in references inconsistently was a bad idea IMHO, but since the
> params structure should always be there and should always be one particular
> thing, I think references are the superior option. I think that means we
> should go through and convert all the existing SimObjects to accept
> references in their constructors, SimObject to store a const reference, and
> params() to return a const reference.
>
>
> 3. One off params() definition vs. centralized definition.
>
> As of today, many (but not all) SimObject classes define a params() method
> which casts the _param member to the param type used by that class to
> return. That means all those classes have individual typedefs which set up
> a Params type for that class, and a params() function which is often (but
> not always) just a copy of the n other params() functions floating around.
>
> It would be nice to centralize this somehow. I don't have a great drop in
> replacement for this since that just doesn't seem to be compatible with the
> way C++ works as of today, but I think it could be simplified a bit with a
> couple definitions the SimObject python<->C++ bridge could generate. These
> are rough ideas, so suggestions for improvements are welcome.
>
> template <typename SimObjectType>
> struct ParamsForSimObjectType;
>
> template <typename SimObjectType>
> using Params = ParamsForSimObjectType<SimObjectType>::Type;
>
> (generated by scons)
> template <>
> struct ParamsForSimObjectType<YourFavoriteSimObject>
> {
>    using Type = const YourFavoriteSimObjectParams &;
> };
>
> Then if you need a Params type for a SimObject type, you can use
> Params<System> for instance.
>
> template <typename SimObjectType>
> Params<SimObjectType>
> params(SimObjectType *so)
> {
>    try {
>        return dynamic_cast<Params<SimObjectType>>(so->base_params());
>    } catch (std::bad_cast &exc) {
>        panic("Failed to cast to derived param type.");
>    }
> }
>
> There are two related downsides to this version of params. First, this
> shouldn't live out in the global namespace since it's so generic. We'd want
> to put it in some sort of namespace, or make it a static member of
> something like the SimObject class. If it's part of a class, it might be
> confusing to people who are used to params() being a method and not a
> static function since it would now need a pointer to the simobject.
>
> This would mean that, for instance, a constructor of a class Foo could just
> use params(this) to get a properly cast FooParams without having to have a
> bunch of boilerplate and plumbing for every class.
>
> If it's used a lot, you could set up a temporary like this to make things
> shorter:
> auto p = params(this);
>
> It's possible that the template-ey-ness of this could get annoying with
> base classes of SimObjects which aren't set up in python, potentially
> requiring casting down to the class pybind set up to get the right answer.
> I think in that case you could define params() through pybind as well, and
> rely on function overload resolution and not templating to get the right
> implementation.
>
>
> I'll likely put this together into a design doc at least to have a record,
> but since it isn't totally baked and is relatively simple I thought I'd
> throw it out there as an email first.
>
> Gabe
> _______________________________________________
> gem5-dev mailing list
> gem5-dev@gem5.org
> http://m5sim.org/mailman/listinfo/gem5-dev
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev  
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to