[1/1] C-SKY:Fix dynamic linker's name when mfloat-abi=softfp.

Message ID 20201016025620.86747-1-cooper.qu@linux.alibaba.com
State Committed
Commit 7449320983b664aba506d7674ea0ce142dd3d4ed
Headers
Series [1/1] C-SKY:Fix dynamic linker's name when mfloat-abi=softfp. |

Commit Message

瞿仙淼 Oct. 16, 2020, 2:56 a.m. UTC
  The dynamic linker should be chosen according to float abi, the
predefined macro __CSKY_HARD_FLOAT__ stand for architecure not
abi.

	* sysdeps/csky/preconfigure: Use __CSKY_HARD_FLOAT_ABI__ instead
	of __CSKY_HARD_FLOAT__.
---
 sysdeps/csky/preconfigure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Adhemerval Zanella Oct. 16, 2020, 3:28 p.m. UTC | #1
On 15/10/2020 23:56, Cooper Qu via Libc-alpha wrote:
> The dynamic linker should be chosen according to float abi, the
> predefined macro __CSKY_HARD_FLOAT__ stand for architecure not
> abi.
> 
> 	* sysdeps/csky/preconfigure: Use __CSKY_HARD_FLOAT_ABI__ instead
> 	of __CSKY_HARD_FLOAT__.

Is this really correct? On a gcc built for csky-linux-gnuabiv2 using
build-many-glibcs.py:

$ csky-glibc-linux-gnuabiv2-gcc --version
csky-glibc-linux-gnuabiv2-gcc (GCC) 9.3.1 20200415 [releases/gcc-9 revision 54ab0a7d757:262f0d53795:1eccf9955614a6f0597bf624bbc88788b8b0fdc5]
[...]

$ csky-glibc-linux-gnuabiv2-gcc test.c -E -dM < /dev/null  | grep __CSKY_HARD_FLOAT_ABI__ ; echo $?

$ csky-glibc-linux-gnuabiv2-gcc test.c -E -dM < /dev/null  | grep __CSKY_HARD_FLOAT__ ; echo $?
#define __CSKY_HARD_FLOAT__ 1
0

It seems this flags was added along with -mfloat-abi=soft option on gcc11
(commit 01d56aeaffa1) which means that building a patched glibc with 
older gcc version will always set the 'float_abi' to always use soft-fp.

I think to get full backwards compatible you will need to handle
and __CSKY_HARD_FLOAT_FPU_SF__ to check whether - mfloat-abi is used, something 
like the below (untested):

---

  hard_float=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null |
       sed -n 's/^#define __CSKY_HARD_FLOAT__ \(.*\)/\1/p'`
  hard_float_fpu_sf=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null |
       sed -n 's/^#define __CSKY_HARD_FLOAT_FPU_SF__ \(.*\)/\1/p'`

  # __CSKY_HARD_FLOAT_ABI__ was added on gcc 11 to specify whether 
  # -mfloat-abi=hard is set.  On older gcc, the float ABI is defined solely
  # with __CSKY_HARD_FLOAT__.  If __CSKY_HARD_FLOAT__ is set it can be 
  # either a hard-float ABI (gcc older than 11, or gcc11 -mfloat-abi=hard 
  # (__CSKY_HARD_FLOAT_ABI__ is set) or -mfloat-abi=softfp 
  # (__CSKY_HARD_FLOAT_ABI__ is set).
  if test -n $hard_float; then
    if test -z $hard_float_fpu_sf; then
      float_abi=1
    else
      float_abi=0
    fi
  else
    float_abi=0
  fi

---
  
> ---
>  sysdeps/csky/preconfigure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sysdeps/csky/preconfigure b/sysdeps/csky/preconfigure
> index 16f3b60..11b887f 100644
> --- a/sysdeps/csky/preconfigure
> +++ b/sysdeps/csky/preconfigure
> @@ -3,7 +3,7 @@ csky*)
>      abi=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null |
>        sed -n 's/^#define __CSKYABI__ \(.*\)/\1/p'`
>      float_abi=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null |
> -      sed -n 's/^#define __CSKY_HARD_FLOAT__ \(.*\)/\1/p'`
> +      sed -n 's/^#define __CSKY_HARD_FLOAT_ABI__ \(.*\)/\1/p'`
>  
>      case "$abi" in
>      1)
>
  
瞿仙淼 Oct. 19, 2020, 6:24 a.m. UTC | #2
Hi Adhemerval,

>     *发件人:*Adhemerval Zanella <adhemerval.zanella@linaro.org>
>     *发送时间:*10/16/20 23:36:15
>     *收件人:*Cooper Qu <cooper.qu@linux.alibaba.com>, Alistair Francis
>     via Libc-alpha <libc-alpha@sourceware.org>, Mao Han
>     <han_mao@c-sky.com>
>     *主题:*Re: [PATCH 1/1] C-SKY:Fix dynamic linker's name when
>     mfloat-abi=softfp.
>
>
>
>         On 15/10/2020 23:56, Cooper Qu via Libc-alpha wrote:
>         > The dynamic linker should be chosen according to float abi, the
>         > predefined macro __CSKY_HARD_FLOAT__ stand for architecure not
>         > abi.
>         >
>         > * sysdeps/csky/preconfigure: Use __CSKY_HARD_FLOAT_ABI__ instead
>         > of __CSKY_HARD_FLOAT__.
>
>         Is this really correct? On a gcc built for csky-linux-gnuabiv2 using
>         build-many-glibcs.py:
>
>         $ csky-glibc-linux-gnuabiv2-gcc --version
>         csky-glibc-linux-gnuabiv2-gcc (GCC) 9.3.1 20200415 [releases/gcc-9 revision 54ab0a7d757:262f0d53795:1eccf9955614a6f0597bf624bbc88788b8b0fdc5]
>         [...]
>
>         $ csky-glibc-linux-gnuabiv2-gcc test.c -E -dM < /dev/null  | grep __CSKY_HARD_FLOAT_ABI__ ; echo $?
>
>         $ csky-glibc-linux-gnuabiv2-gcc test.c -E -dM < /dev/null  | grep __CSKY_HARD_FLOAT__ ; echo $?
>         #define __CSKY_HARD_FLOAT__ 1
>         0
>
>         It seems this flags was added along with -mfloat-abi=soft option on gcc11
>         (commit 01d56aeaffa1) which means that building a patched glibc with
>
>         older gcc version will always set the 'float_abi' to always use soft-fp.
>
Yes, it cannot be compatible with the old version GCC.
>
>
>         I think to get full backwards compatible you will need to handle
>         and __CSKY_HARD_FLOAT_FPU_SF__ to check whether - mfloat-abi is used, something
>
>         like the below (untested):
>
>         ---
>
>           hard_float=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null |
>                sed -n 's/^#define __CSKY_HARD_FLOAT__ \(.*\)/\1/p'`
>           hard_float_fpu_sf=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null |
>                sed -n 's/^#define __CSKY_HARD_FLOAT_FPU_SF__ \(.*\)/\1/p'`
>
>           # __CSKY_HARD_FLOAT_ABI__ was added on gcc 11 to specify whether
>
>           # -mfloat-abi=hard is set.  On older gcc, the float ABI is defined solely
>           # with __CSKY_HARD_FLOAT__.  If __CSKY_HARD_FLOAT__ is set it can be
>
>           # either a hard-float ABI (gcc older than 11, or gcc11 -mfloat-abi=hard
>
>           # (__CSKY_HARD_FLOAT_ABI__ is set) or -mfloat-abi=softfp
>           # (__CSKY_HARD_FLOAT_ABI__ is set).
>           if test -n $hard_float; then
>             if test -z $hard_float_fpu_sf; then
>               float_abi=1
>             else
>               float_abi=0
>             fi
>           else
>             float_abi=0
>           fi
>
This is a  feasible method, I will verify and fix it.Thanks very much!


Cooper
  
Adhemerval Zanella Oct. 19, 2020, 12:22 p.m. UTC | #3
On 19/10/2020 03:24, Cooper Qu wrote:
> Hi Adhemerval,
> 
>>     *发件人:*Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>     *发送时间:*10/16/20 23:36:15
>>     *收件人:*Cooper Qu <cooper.qu@linux.alibaba.com>, Alistair Francis via Libc-alpha <libc-alpha@sourceware.org>, Mao Han <han_mao@c-sky.com>
>>     *主题:*Re: [PATCH 1/1] C-SKY:Fix dynamic linker's name when mfloat-abi=softfp.
>>
>>
>>
>>         On 15/10/2020 23:56, Cooper Qu via Libc-alpha wrote:
>>         > The dynamic linker should be chosen according to float abi, the
>>         > predefined macro __CSKY_HARD_FLOAT__ stand for architecure not
>>         > abi.
>>         > 
>>         > * sysdeps/csky/preconfigure: Use __CSKY_HARD_FLOAT_ABI__ instead
>>         > of __CSKY_HARD_FLOAT__.
>>
>>         Is this really correct? On a gcc built for csky-linux-gnuabiv2 using
>>         build-many-glibcs.py:
>>
>>         $ csky-glibc-linux-gnuabiv2-gcc --version
>>         csky-glibc-linux-gnuabiv2-gcc (GCC) 9.3.1 20200415 [releases/gcc-9 revision 54ab0a7d757:262f0d53795:1eccf9955614a6f0597bf624bbc88788b8b0fdc5]
>>         [...]
>>
>>         $ csky-glibc-linux-gnuabiv2-gcc test.c -E -dM < /dev/null  | grep __CSKY_HARD_FLOAT_ABI__ ; echo $?
>>
>>         $ csky-glibc-linux-gnuabiv2-gcc test.c -E -dM < /dev/null  | grep __CSKY_HARD_FLOAT__ ; echo $?
>>         #define __CSKY_HARD_FLOAT__ 1
>>         0
>>
>>         It seems this flags was added along with -mfloat-abi=soft option on gcc11
>>         (commit 01d56aeaffa1) which means that building a patched glibc with 
>>         older gcc version will always set the 'float_abi' to always use soft-fp.
>>
> Yes, it cannot be compatible with the old version GCC.
>>
>>
>>         I think to get full backwards compatible you will need to handle
>>         and __CSKY_HARD_FLOAT_FPU_SF__ to check whether - mfloat-abi is used, something 
>>         like the below (untested):
>>
>>         ---
>>
>>           hard_float=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null |
>>                sed -n 's/^#define __CSKY_HARD_FLOAT__ \(.*\)/\1/p'`
>>           hard_float_fpu_sf=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null |
>>                sed -n 's/^#define __CSKY_HARD_FLOAT_FPU_SF__ \(.*\)/\1/p'`
>>
>>           # __CSKY_HARD_FLOAT_ABI__ was added on gcc 11 to specify whether 
>>           # -mfloat-abi=hard is set.  On older gcc, the float ABI is defined solely
>>           # with __CSKY_HARD_FLOAT__.  If __CSKY_HARD_FLOAT__ is set it can be 
>>           # either a hard-float ABI (gcc older than 11, or gcc11 -mfloat-abi=hard 
>>           # (__CSKY_HARD_FLOAT_ABI__ is set) or -mfloat-abi=softfp 
>>           # (__CSKY_HARD_FLOAT_ABI__ is set).
>>           if test -n $hard_float; then
>>             if test -z $hard_float_fpu_sf; then
>>               float_abi=1
>>             else
>>               float_abi=0
>>             fi
>>           else
>>             float_abi=0
>>           fi
>>
> This is a  feasible method, I will verify and fix it.Thanks very much!

Hi,

It seems that you still committed the older versions which is
incompatible with older GCC. Could you revert it and push
a version that work with any gcc version?
  
毛晗 Oct. 20, 2020, 2:22 a.m. UTC | #4
>>Hi,
>>
>> It seems that you still committed the older versions which is
>> incompatible with older GCC. Could you revert it and push
>> a version that work with any gcc version?
Sorry for missing your reply and the incomplete thought on the
incompatibility. I found you reply in the afternoon and disscussed
the solution with Cooper, but didn't revet the commit.
We've revet the commit now and will update that patch later.

Thanks,
Mao Han
  

Patch

diff --git a/sysdeps/csky/preconfigure b/sysdeps/csky/preconfigure
index 16f3b60..11b887f 100644
--- a/sysdeps/csky/preconfigure
+++ b/sysdeps/csky/preconfigure
@@ -3,7 +3,7 @@  csky*)
     abi=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null |
       sed -n 's/^#define __CSKYABI__ \(.*\)/\1/p'`
     float_abi=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null |
-      sed -n 's/^#define __CSKY_HARD_FLOAT__ \(.*\)/\1/p'`
+      sed -n 's/^#define __CSKY_HARD_FLOAT_ABI__ \(.*\)/\1/p'`
 
     case "$abi" in
     1)