Re: [dpdk-dev] [PATCH] build: enable to build on power10 or newer for ppc

[David Marchand <david.marchand@redhat.com>]

On Wed, Jul 21, 2021 at 11:14 PM Thinh Tran <thinhtr@linux.vnet.ibm.com> wrote:
>
> A older version of complier would fail to generate code for new Power

compiler

> CPUs when it uses "-mcpu=native" argument.
> This patch will test if the compiler supports the current Power CPU type
> then proceeds with "-mcpu=native" argument, else it tries with older type.
> Limit to two older CPU type levels.

Such a change seems a bit late for 21.08, and is broken (see below).
In any case, I would need a review from ppc maintainer.


>
> Signed-off-by: Thinh Tran <thinhtr@linux.vnet.ibm.com>
> ---
>  config/ppc/check_cpu_platform.sh |  2 ++
>  config/ppc/meson.build           | 40 +++++++++++++++++++++++++-------
>  2 files changed, 34 insertions(+), 8 deletions(-)
>  create mode 100644 config/ppc/check_cpu_platform.sh
>
> diff --git a/config/ppc/check_cpu_platform.sh b/config/ppc/check_cpu_platform.sh
> new file mode 100644
> index 0000000000..cdea24561b
> --- /dev/null
> +++ b/config/ppc/check_cpu_platform.sh
> @@ -0,0 +1,2 @@
> +#! /bin/sh
> +LD_SHOW_AUXV=1 /bin/true | awk '/AT_PLATFORM/ {print $2}'|sed  's/\power//'
> diff --git a/config/ppc/meson.build b/config/ppc/meson.build
> index adf49e1f42..05aa860cfd 100644
> --- a/config/ppc/meson.build
> +++ b/config/ppc/meson.build
> @@ -7,16 +7,40 @@ endif
>  dpdk_conf.set('RTE_ARCH', 'ppc_64')
>  dpdk_conf.set('RTE_ARCH_PPC_64', 1)
>
> -# RHEL 7.x uses gcc 4.8.X which doesn't generate code for Power 9 CPUs,
> -# though it will detect a Power 9 CPU when the "-mcpu=native" argument
> -# is used, resulting in a build failure.
> -power9_supported = cc.has_argument('-mcpu=power9')
> -if not power9_supported
> -    cpu_instruction_set = 'power8'
> -    machine_args = ['-mcpu=power8', '-mtune=power8']
> -    dpdk_conf.set('RTE_MACHINE','power8')
> +# Checking compiler for supporting Power CPU platform
> +# For newer Power(N) System that current gcc may not supoort it yet,
> +# it falls back and try  N-1 and N-2

double space unneeded.
Plus, wording reads odd to me.

> +check_cpu = find_program(join_paths(meson.current_source_dir(),
> +                 'check_cpu_platform.sh'))

Why do you need a separate script?
The value it returns is constant on a given system.


Looking at the script itself, this breaks cross compilation.

Compiler for C supports arguments -Wno-missing-field-initializers
-Wmissing-field-initializers: YES (cached)
Program /home/dmarchan/dpdk/config/ppc/check_cpu_platform.sh found: YES

../../dpdk/config/ppc/meson.build:18:0: ERROR: String 'x86_64' cannot
be converted to int

A full log can be found at
/home/dmarchan/builds/build-ppc64le-power8/meson-logs/meson-log.txt
FAILED: build.ninja
/usr/bin/meson --internal regenerate /home/dmarchan/dpdk
/home/dmarchan/builds/build-ppc64le-power8 --backend ninja
ninja: error: rebuilding 'build.ninja': subcommand failed



> +
> +target_cpu = run_command(check_cpu.path()).stdout().strip()
> +
> +cpu_int = target_cpu.to_int()
> +cpu_flag = '-mcpu=power@0@'
> +tune_flag = '-mtune=power@0@'
> +machine_type = 'power@0@'
> +debug = 'configure the compiler to build DPDK for POWER@0@ platform'
> +
> +if cc.has_argument(cpu_flag.format(cpu_int))
> +
> +  # target system cpu is supported by the compiler, use '-mcpu=native'
> +  message(debug.format(target_cpu+'_native'))
> +  machine_args = ['-mcpu=native']
> +  dpdk_conf.set('RTE_MACHINE',machine_type.format(cpu_int))
> +elif cc.has_argument(cpu_flag.format(cpu_int-1))
> +  message(debug.format(cpu_int-1))
> +  machine_args = [cpu_flag.format(cpu_int-1),tune_flag.format(cpu_int-1)]
> +  dpdk_conf.set('RTE_MACHINE',machine_type.format(cpu_int-1))
> +elif cc.has_argument(cpu_flag.format(cpu_int-2))
> +  message(debug.format(cpu_int-2))
> +  machine_args = [cpu_flag.format(cpu_int-2),tune_flag.format(cpu_int-2)]
> +  dpdk_conf.set('RTE_MACHINE',machine_type.format(cpu_int-2))
> +else
> +  error('The compiler does not support POWER@0@ platform' .format(cpu_int))
>  endif
>
> +
> +

One line is enough.



>  # Certain POWER9 systems can scale as high as 1536 LCORES, but setting such a
>  # high value can waste memory, cause timeouts in time limited autotests, and is
>  # unlikely to be used in many production situations.  Similarly, keeping the
> --
> 2.17.1
>


-- 
David Marchand