Re: [gentoo-dev] [arm17] [PATCH] toolchain-funcs.eclass: Update tc-is-softfloat for new ARM triplets

[Sergei Trofimovich <slyfox@gentoo.org>]

On Wed, 25 Jul 2018 00:09:28 +0100
James Le Cuirot <chewi@gentoo.org> wrote:

> The triplet will change from armv7a-hardfloat-linux-gnueabi to
> armv7a-unknown-linux-gnueabihf or similar. The function already
> treated the latter as hardfloat but ambiguous triplets such as
> arm-unknown-linux-gnueabi will change from hardfloat to softfloat in
> line with most everyone else. However, we will now check existing
> toolchains to avoid breaking existing systems, if possible.

[+arm@ CC]

1. This changelog is not clear if arm-unknown-linux-gnueabi will change
meaning in this commit.
2. Did Gentoo ever use arm-unknown-linux-gnueabi tuple? I don't see
it in recent profile history.
3. What are existing toolchain tuples? All the ones people use?

> ---
>  eclass/toolchain-funcs.eclass | 39 ++++++++++++++++++++++++++++-------
>  1 file changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/eclass/toolchain-funcs.eclass b/eclass/toolchain-funcs.eclass
> index cea8949b45d7..f484fffc2664 100644
> --- a/eclass/toolchain-funcs.eclass
> +++ b/eclass/toolchain-funcs.eclass
> @@ -204,13 +204,38 @@ tc-is-softfloat() {
>  		bfin*|h8300*)
>  			echo "only" ;;
>  		*)
> -			if [[ ${CTARGET//_/-} == *-softfloat-* ]] ; then
> -				echo "yes"
> -			elif [[ ${CTARGET//_/-} == *-softfp-* ]] ; then
> -				echo "softfp"
> -			else
> -				echo "no"
> -			fi
> +			case ${CTARGET//_/-} in
> +				*-softfloat-*)
> +					echo "yes" ;;
> +				*-softfp-*)
> +					echo "softfp" ;;
> +				arm*)
> +					# arm-unknown-linux-gnueabi is ambiguous. We used to
> +					# treat it as hardfloat but we now treat it as
> +					# softfloat like most everyone else. However, we
> +					# check existing toolchains to avoid breaking
> +					# existing systems, if possible.
> +					if type -P ${CTARGET}-cpp >/dev/null; then

I believe correct way to get cpp for target is
    "$(tc-getCPP ${CTARGET}) -E"

> +						if ${CTARGET}-cpp -E - <<< __ARM_PCS_VFP 2>/dev/null | grep -q __ARM_PCS_VFP; then

4. This magic is hard to follow and reason about.
I suggest moving out autodetection of current setup into another helper.
Bonus point for detection of mismatch of actual vs. intended state.

Then we could start warning users about the fact of inconsistency and point
to migration procedure. And we could have cleaner ${CTARGET} matches against
what Gentoo expects.

5. you don't use ${CFLAGS} here. I feel we should use them as people do occasionally
override defaults.

> +							# Confusingly __SOFTFP__ is defined only
> +							# when -mfloat-abi is soft, not softfp.
> +							if ${CTARGET}-cpp -E - <<< __SOFTFP__ 2>/dev/null | grep -q __SOFTFP__; then
> +								echo "softfp"
> +							else
> +								echo "yes"
> +							fi
> +						else
> +							echo "no"
> +						fi
> +					elif [[ ${CTARGET} == *-hardfloat-* || ${CTARGET} == *hf ]]; then

I suggest using *-gnueabihf. I don't think anything more generic is recognized by toolchains
as a hardfloat target.

Also please link to description of what you think canonical hardfloat tuples are supposed to
be. Upstreams do not agree on the definition.

> +						echo "no"
> +					else
> +						echo "yes"
> +					fi
> +					;;
> +				*)
> +					echo "no" ;;
> +			esac
>  			;;
>  	esac
>  }
> -- 
> 2.17.0
> 
> 


-- 

  Sergei