On Fri, Nov 20, 2020 at 11:56:44AM +0000, Juraj Linkeš wrote:
>
>
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richard...@intel.com>
> > Sent: Friday, November 20, 2020 11:20 AM
> > To: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>
> > Cc: Juraj Linkeš <juraj.lin...@pantheon.tech>; tho...@monjalon.net; Ruifeng
> > Wang <ruifeng.w...@arm.com>; Phil Yang <phil.y...@arm.com>;
> > vcchu...@amazon.com; Dharmik Thakkar <dharmik.thak...@arm.com>;
> > jerinjac...@gmail.com; hemant.agra...@nxp.com; Ajit Khaparde
> > (ajit.khapa...@broadcom.com) <ajit.khapa...@broadcom.com>;
> > ferruh.yi...@intel.com; dev@dpdk.org; nd <n...@arm.com>
> > Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu
> > counts
> > detection
> >
> > On Fri, Nov 20, 2020 at 10:15:42AM +0000, Bruce Richardson wrote:
> > > On Fri, Nov 20, 2020 at 04:33:12AM +0000, Honnappa Nagarahalli wrote:
> > > > <snip>
> > > >
> > > > > > > > > 18/11/2020 15:19, Juraj Linkeš:
> > > > > > > > > > From: Thomas Monjalon <tho...@monjalon.net>
> > > > > > > > > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > > > > > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas
> > > > > > > > > > > > Monjalon
> > > > > wrote:
> > > > > > > > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > > > > > > > > + description: 'maximum number of cores/threads
> > > > > > > > > > > > > > +supported by
> > > > > > > > > EAL.
> > > > > > > > > > > > > > +Set to positive integer to overwrite per-arch
> > > > > > > > > > > > > > +or cross-compilation
> > > > > > > > > > > defaults. Set to -1 to detect the number of cores on
> > > > > > > > > > > the build
> > > > > > > > > > > machine.') option('max_numa_nodes', type: 'integer',
> > > > > > > > > > > value:
> > > > > > > > > > > 0,
> > > > > > > > > > > > > > + description: 'maximum number of NUMA nodes
> > > > > supported
> > > > > > > > > > > > > > +by
> > > > > > > > > EAL.
> > > > > > > > > > > > > > +Set to positive integer to overwrite per-arch
> > > > > > > > > > > > > > +or cross-compilation defaults. Set to -1 to
> > > > > > > > > > > > > > +detect the number of numa nodes on the build
> > > > > > > > > > > > > > +machine.')
> > > > > > > > > > > > >
> > > > > > > > > > > > > First comment: I don't like having so long
> > > > > > > > > > > > > description.
> > > > > > > > > > > > > Second: I don't understand.
> > > > > > > > > > > > >
> > > > > > > > > > > > > It is said the default value is 0 so I expect it
> > > > > > > > > > > > > means automatic
> > > > > > > detection.
> > > > > > > > > > > > > But later it is said -1 is for detection. So ?
> > > > > > > > > > > > >
> > > > > > > > > > > > Zero is for the "per-arch or cross-compilation default".
> > > > > > > > > > > > This was discussed quite a bit in previous versions
> > > > > > > > > > > > and this was te best compromise we could come up
> > > > > > > > > > > > with. Having a default of auto-detect is definitely
> > > > > > > > > > > > not something I think we should go with - just
> > > > > > > > > > > > thinking of all the build CI jobs running on
> > > > > > > > > > > > 2 or 4 core VMs! However, Juraj really felt there
> > > > > > > > > > > > was value in having auto-detection, so it's set as a
> > > > > > > > > > > > -1 value, which I'm
> > > > > ok with.
> > > > > > > > > > >
> > > > > > > > > > > The problem is that I don't understand what 0 means.
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > There are three pieces of information which we need to
> > > > > > > > > > convey:
> > > > > > > > > > 1. The default value (0) indicates that per-arch or
> > > > > > > > > > cross-compilation defaults
> > > > > > > > > will be used.
> > > > > > > > > > 2. Positive integer values will be used instead of these
> > > > > > > > > > defaults.
> > > > > > > > >
> > > > > > > > > Where these positive values come from?
> > > > > > > > >
> > > > > > > >
> > > > > > > > From the user - they will have the option to set it to
> > > > > > > > whatever the like if they
> > > > > > > don't want to use defaults.
> > > > > > > >
> > > > > > > > > > 3. Detected values will be used for native build when the
> > > > > > > > > > value is -
> > 1.
> > > > > > > > >
> > > > > > > > > Why not detect for any native build set up with 0 (default)?
> > > > > > > > >
> > > > > > > >
> > > > > > > > I'll let Bruce explain this, but I'll just say that we
> > > > > > > > wanted to make the detection
> > > > > > > the default for native builds, so we're in agreement.
> > > > > > >
> > > > > > > I think most of us agree that the different understanding of
> > > > > > > the term "native build", is the cause of much of the
> > > > > > > disagreements and
> > > > Agree, that's the main reason.
> > > >
> > > > > > > points of dispute on this thread. From my view point, the term
> > > > > > > "native"
> > > > > can refer to:
> > > > > > >
> > > > > > > 1. what meson considers a native build, i.e. one not using a
> > > > > > > cross-file 2. a build for a different machine architecture to
> > > > > > > the one on the
> > > > > build
> > > > > > > machine (this largely overlaps with #1, except that e.g.
> > > > > > > 32-bit build on
> > > > > > > 64-bit may be considered a cross-build in this case).
> > > > Sorry, I did not understand #2 here. Are you saying, native "means" -
> > > > "a build
> > for a different machine architecture to the one on the build machine"
> > > >
> > > > > > > 3. a build tailored exactly for the build machine itself i.e.
> > > > > > > both ISA, and
> > > > > > > things like core counts.
> > > > > > > 4. a flag passed to the compiler to indicate the uarch level of
> > > > > > > the
> > > > > > > instruction set to be used, e.g. on x86, AVX2, AVX-512 etc.,
> > > > > > > based on
> > > > > > > that of the build machine.
> > > > > > >
> > > > > > > Historically, IIRC, in DPDK the "RTE_MACHINE" value was
> > > > > > > originally
> > > > > > > #4 since that was it's use on x86 in the first versions of DPDK.
> > > > > > > With the move from make to meson, that aspect was kept, but
> > > > > > > the meaning of #1 (I think we can ignore #2) also came into play.
> > > > > > > Finally, while for x86 architecture, the idea of #4 still
> > > > > > > held, for ARM use #3
> > > > > is of major concern.
> > > > Yes, #3 is the concern.
> > > >
> > > > At the same time, I am also interested in avoiding 'native' (or any
> > > > other
> > option) having different meaning for different architectures.
> > > > Now that we have introduced 'soc' option for Arm platforms, we are able
> > > > to
> > achieve the builds that would be produced by #3.
> > > > 'soc' combines both the 'platform' and 'instruction set' (as you have
> > > > defined
> > them below).
> > > >
> > >
> > > My thinking was that platform would be a synonym for "soc" for SOCs -
> > > it would just seem weird to refer to x86 or PPC server systems as
> > > soc's, so I thought "platform" a more neutral term.
> > > Also, as I defined it above, the idea of "platform" would always
> > > encompass the "instruction set" option too, unless the user explicitly
> > > overrode it - hence the "auto" default value.
> > >
> > Also, we could document the "instruction set" value as a 'hint', rather
> > than a
> > mandatory value, which gives the arm meson.build files the option to ignore
> > it if
> > it doesn't make sense to use for a given value of "platform/soc".
> >
> > /Bruce
>
> Let's (or let me) put together a new patch that would decouple ISA from
> platform. How should we do this?
>
> I want to send this commit separately - should I add the ISA/platform to that
> series or do a completely new series? I like the latter, since we're
> basically done with reviewing this commit.
>
Yes, let's start a new series for this. It makes the whole thing easier to
see and review.
/Bruce
