Re: [Buildroot] [PATCH 2/9] arch/powerpc: Clarify generic CPUs

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

On Fri, 29 Jul 2022 09:38:57 +0930
Joel Stanley <joel@jms.id.au> wrote:

> In the case where a specific CPU is not selected, set an appropriate
> gcc target for the architecture.
> 
> For clarity this introduces a BR2_generic_powerpc64 as a generic 64 bit
> CPU as this makes the it clearer when browsing menuconfig.
> 
> We can't have a generic ppc64le GCC, as attempting to configure glibc
> for powerpc64le fails:
> 
>  configure: error: ***  POWER8 or newer is required on powerpc64le.
>  __builtin_signbit is broken.  GCC 7.4 or newer is required to resolve
>  (PR83862). The compiler must support -mabi=ieeelongdouble and
>  -mlong-double-128 simultaneously.
> 
> Hence set Power8 as the default CPU for powerpc64le.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  arch/Config.in.powerpc | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/Config.in.powerpc b/arch/Config.in.powerpc
> index ef0e85fec680..8d392bfde814 100644
> --- a/arch/Config.in.powerpc
> +++ b/arch/Config.in.powerpc
> @@ -9,12 +9,17 @@ config BR2_POWERPC_CPU_HAS_SPE
>  
>  choice
>  	prompt "Target Architecture Variant"
> -	default BR2_generic_powerpc
> +	default BR2_generic_powerpc if !BR2_ARCH_IS_64
> +	default BR2_generic_powerpc64 if BR2_ARCH_IS_64
> +	default BR2_powerpc_power8 if BR2_powerpc64le

This default will never be used, because BR2_ARCH_IS_64 is true when
BR2_powerpc64le is true. So on powerpc64le, "default
BR2_generic_powerpc64 if BR2_ARCH_IS_64" will match.

So a better solution is:

+	default BR2_generic_powerpc if BR2_powerpc
+	default BR2_generic_powerpc64 if BR2_powerpc64
+	default BR2_powerpc_power8 if BR2_powerpc64le

>  	help
>  	  Specific CPU variant to use
>  config BR2_generic_powerpc
> -	bool "generic"
> -	# No C library supports this variant on ppc64le
> +	bool "generic (32 bit)"
> +	depends on !BR2_ARCH_IS_64

I think the (32 bit) is useless, because this one will only show up on
32-bit powerpc.

> +config BR2_generic_powerpc64
> +	bool "generic (64 bit)"
> +	depends on BR2_ARCH_IS_64
>  	depends on !BR2_powerpc64le

Ditto here.

However, a help text for both of these options would be useful, as
these "generic" options are always a bit magic as to what CPUs they
support exactly. The help text should also say that they map to
-mcpu=powerpc and -mcpu=powerpc64 respectively.

As Yann requested, the commit log needs to be changed, as "In the case
where a specific CPU is not selected, set an appropriate gcc target for
the architecture." is incorrect, as a specific CPU is always selected.

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot