Re: [dpdk-dev] [PATCH v15 11/12] build: add Arm SoC meson option

[Thomas Monjalon <thomas@monjalon.net>]

21/01/2021 16:02, Juraj Linkeš:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 20/01/2021 09:41, Juraj Linkeš:
> > > From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > > > > 20/01/2021 02:04, Honnappa Nagarahalli:
> > > > > > > On Tue, Jan 19, 2021 at 04:52:19PM +0100, Thomas Monjalon wrote:
> > > > > > > > 19/01/2021 15:56, Juraj Linkeš:
> > > > > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > > > > 15/01/2021 14:26, Juraj Linkeš:
> > > > > > > > > > > --- a/meson_options.txt
> > > > > > > > > > > +++ b/meson_options.txt
> > > > > > > > > > > +option('arm_soc', type: 'string', value: '',
> > > > > > > > > > > +	description: 'Specify if you want to build for a
> > > > > > > > > > > +particular
> > > > > > > > > > > +aarch64 Arm SoC when building on an aarch64
> > > > > > > > > > > +machine.')
> > > > > > > > > >
> > > > > > > > > > Why the option is named "arm_soc" and not just "soc"?
> > > > > > > > > > The same option could be used by other archs, isn't it?
> > > > > > > > >
> > > > > > > > > Agree that a more generic name would be better.
> > > > > > > > > I'll change it to "soc" if there are no other suggestions.
> > > > > > > >
> > > > > > > > Another name could be "machine".
> > > > > > > > Should it be the same mechanism as compiling for a specific
> > > > > > > > x86 CPU from an x86 machine?
> > > > > > > >
> > > > > > > I'd rather not re-use the term "machine", for a new use,
> > > > > > > better to use a new term IMHO.
> > > > > > +1, agree. 'soc' sounds good to me.
> > > > >
> > > > > Another possible word is "platform", as in
> > > > > http://doc.dpdk.org/guides/platform/index.html
> > > > I am fine with 'platform' too.
> > > >
> > >
> > > 'platform' is likely the best and actually works nicely with
> > http://patches.dpdk.org/patch/85956/. Taken together, 'platform' could be
> > either 'native', 'generic' or an soc, which is, I believe, exactly what we want.
> > 
> > I am not sure what we want :)
> > We need to specify the instruction set, and the specific target.
> > We could deduce the instruction set from the target, but I think it is good to be
> > able to overwrite the instruction set in case there can be multiple instruction
> > sets for a target.
> > 
> 
> I think we had this (or similar) discussion here http://patches.dpdk.org/patch/85956/. I agree with your summary.
> 
> > I think "native" and "generic" should be specified as instruction set, in the
> > existing option "machine" or renamed as "instruction_set" or "isa".
> > 
> 
> Agree, but I would add that we also want "native" and "generic" as valid platforms. More below.
> 
> > Let's imagine the first option is "isa" and the new second option is "platform".
> > We can have a default "isa" per "platform".
> > The default "platform" would have a default "isa": native or generic?
> > 
> 
> In general, yes, but I let me expand the "platform" option a bit.
> 
> Let's dig into what "platform" means. I understand it to be a set of configuration options, e.g.:
> platform=native: use discovered values for all configuration options (where we can do that)
> platform=generic: use predetermined values to produce a "generic" build that would work on most machine of the corresponding type/arch

This is where I was disagreeing:
you propose to have 2 special values of platform (native and generic),
I propose to have only 1 default value for platform.

After more thoughts, I think it's fine.


> platform=other: use predetermined values to produce a build tailored to platform "other", such as some arm soc.
> 
> In all these cases, leave user the option to specify supported options (i.e. user can specifying "instruction_set" and that value would be used for machine args or "max_lcores" would set max lcores).
> 
> The default "instruction_set" would be different per platform:

What do you think about calling this option "isa"?

> platform=native => instruction_set=native
> platform=generic => the generic instruction_set for the architecture, as specified here: https://github.com/DPDK/dpdk/blob/main/config/meson.build#L79
> platform=other => the instruction set of the platform
> 
> When "instruction_set" is set to its default value (such as auto), the per-platform instruction set would be used. If the user specifies anything else, that value would be used.

Why auto? This is what we call native.

> I basically just reworded Bruce's proposal from the other thread, since I agree with it.
> 
> Using the "platform" option in this commit just extends the supported platforms (from "native" and "default") by adding the soc platforms. (or the other patch would extend the supported platforms, depending on in which order the patches would be applied)
> 
> > What else do we need?
> > 
> > 
> 
> I think the above proposal (actually implemented here: http://patches.dpdk.org/patch/85956/) gives us what we want, which I believe is this (which is basically your summary + maybe some extra thoughts):
> 1. Arm wants to have the ability to choose a configuration set (i.e. the "platform" option). We also refer to this as the "type" of the build.
> 2. We also want to keep the current behavior of fine tuning the isa, which the "instruction_set" option does.
> 3. We don't want to change the default or expected behavior much or not at all, which we can do by choosing the right default values for "platform" and "instruction_set".