LoongArch: Use "$fcsr0" instead of "$r0" in _FPU_{GET,SET}CW

Message ID 20240429073111.14572-2-xry111@xry111.site (mailing list archive)
State Committed
Commit 0c1d2c277a59f08fd3232b33d18644ea890190ea
Headers
Series LoongArch: Use "$fcsr0" instead of "$r0" in _FPU_{GET,SET}CW |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed

Commit Message

Xi Ruoyao April 29, 2024, 7:31 a.m. UTC
  Clang inline-asm parser does not allow using "$r0" in
movfcsr2gr/movgr2fcsr, so everything using _FPU_{GET,SET}CW is now
failing to build with Clang on LoongArch.  As we now requires Binutils
>= 2.41 which supports using "$fcsr0" here, use it instead of "$r0" to
fix the issue.

Link: https://github.com/loongson-community/discussions/issues/53#issuecomment-2081507390
Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=4142b2368353
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---

People in the Cc list will receive this twice because I've mistyped the
address of libc-alpha first time.  Sorry.  Stupid I :(.

 sysdeps/loongarch/fpu_control.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Xi Ruoyao May 23, 2024, 3:20 a.m. UTC | #1
Ping.

On Mon, 2024-04-29 at 15:31 +0800, Xi Ruoyao wrote:
> Clang inline-asm parser does not allow using "$r0" in
> movfcsr2gr/movgr2fcsr, so everything using _FPU_{GET,SET}CW is now
> failing to build with Clang on LoongArch.  As we now requires Binutils
> > = 2.41 which supports using "$fcsr0" here, use it instead of "$r0" to
> fix the issue.
> 
> Link: https://github.com/loongson-community/discussions/issues/53#issuecomment-2081507390
> Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=4142b2368353
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
> 
> People in the Cc list will receive this twice because I've mistyped the
> address of libc-alpha first time.  Sorry.  Stupid I :(.
> 
>  sysdeps/loongarch/fpu_control.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/loongarch/fpu_control.h b/sysdeps/loongarch/fpu_control.h
> index 54add4e01c..3cdf2417d9 100644
> --- a/sysdeps/loongarch/fpu_control.h
> +++ b/sysdeps/loongarch/fpu_control.h
> @@ -91,8 +91,8 @@ typedef unsigned int fpu_control_t __attribute__ ((__mode__ (__SI__)));
>  /* Macros for accessing the hardware control word.  */
>  extern fpu_control_t __loongarch_fpu_getcw (void) __THROW;
>  extern void __loongarch_fpu_setcw (fpu_control_t) __THROW;
> -#define _FPU_GETCW(cw) __asm__ volatile ("movfcsr2gr %0,$r0" : "=r"(cw))
> -#define _FPU_SETCW(cw) __asm__ volatile ("movgr2fcsr $r0,%0" : : "r"(cw))
> +#define _FPU_GETCW(cw) __asm__ volatile ("movfcsr2gr %0,$fcsr0" : "=r"(cw))
> +#define _FPU_SETCW(cw) __asm__ volatile ("movgr2fcsr $fcsr0,%0" : : "r"(cw))
>  
>  /* Default control word set at startup.  */
>  extern fpu_control_t __fpu_control;
  
Adhemerval Zanella May 23, 2024, 1:09 p.m. UTC | #2
On 29/04/24 04:31, Xi Ruoyao wrote:
> Clang inline-asm parser does not allow using "$r0" in
> movfcsr2gr/movgr2fcsr, so everything using _FPU_{GET,SET}CW is now
> failing to build with Clang on LoongArch.  As we now requires Binutils
>> = 2.41 which supports using "$fcsr0" here, use it instead of "$r0" to
> fix the issue.
> 
> Link: https://github.com/loongson-community/discussions/issues/53#issuecomment-2081507390
> Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=4142b2368353
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>

LGTM, thanks.  It is safe to use with old gcc and binutils or the new 
constraint required a specific gcc version?

> ---
> 
> People in the Cc list will receive this twice because I've mistyped the
> address of libc-alpha first time.  Sorry.  Stupid I :(.
> 
>  sysdeps/loongarch/fpu_control.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/loongarch/fpu_control.h b/sysdeps/loongarch/fpu_control.h
> index 54add4e01c..3cdf2417d9 100644
> --- a/sysdeps/loongarch/fpu_control.h
> +++ b/sysdeps/loongarch/fpu_control.h
> @@ -91,8 +91,8 @@ typedef unsigned int fpu_control_t __attribute__ ((__mode__ (__SI__)));
>  /* Macros for accessing the hardware control word.  */
>  extern fpu_control_t __loongarch_fpu_getcw (void) __THROW;
>  extern void __loongarch_fpu_setcw (fpu_control_t) __THROW;
> -#define _FPU_GETCW(cw) __asm__ volatile ("movfcsr2gr %0,$r0" : "=r"(cw))
> -#define _FPU_SETCW(cw) __asm__ volatile ("movgr2fcsr $r0,%0" : : "r"(cw))
> +#define _FPU_GETCW(cw) __asm__ volatile ("movfcsr2gr %0,$fcsr0" : "=r"(cw))
> +#define _FPU_SETCW(cw) __asm__ volatile ("movgr2fcsr $fcsr0,%0" : : "r"(cw))
>  
>  /* Default control word set at startup.  */
>  extern fpu_control_t __fpu_control;
  
caiyinyu May 26, 2024, 6:15 a.m. UTC | #3
在 2024/5/23 下午9:09, Adhemerval Zanella Netto 写道:
>
> On 29/04/24 04:31, Xi Ruoyao wrote:
>> Clang inline-asm parser does not allow using "$r0" in
>> movfcsr2gr/movgr2fcsr, so everything using _FPU_{GET,SET}CW is now
>> failing to build with Clang on LoongArch.  As we now requires Binutils
>>> = 2.41 which supports using "$fcsr0" here, use it instead of "$r0" to
>> fix the issue.
>>
>> Link: https://github.com/loongson-community/discussions/issues/53#issuecomment-2081507390
>> Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=4142b2368353
>> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> LGTM, thanks.  It is safe to use with old gcc and binutils or the new
> constraint required a specific gcc version?
1.  Starting from LoongArch glibc-2.39, the minimum required version of 
binutils
is 2.41(because of the LASX/LSX vector instructions), and binutils 
support for
this modification also begins with version 2.41

2.  All LoongArch gcc support this change.

The header files modified this time are export header files, but I think 
this
change is ok.  In conclusion, I plan to merge this commit.
>> ---
>>
>> People in the Cc list will receive this twice because I've mistyped the
>> address of libc-alpha first time.  Sorry.  Stupid I :(.
>>
>>   sysdeps/loongarch/fpu_control.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/sysdeps/loongarch/fpu_control.h b/sysdeps/loongarch/fpu_control.h
>> index 54add4e01c..3cdf2417d9 100644
>> --- a/sysdeps/loongarch/fpu_control.h
>> +++ b/sysdeps/loongarch/fpu_control.h
>> @@ -91,8 +91,8 @@ typedef unsigned int fpu_control_t __attribute__ ((__mode__ (__SI__)));
>>   /* Macros for accessing the hardware control word.  */
>>   extern fpu_control_t __loongarch_fpu_getcw (void) __THROW;
>>   extern void __loongarch_fpu_setcw (fpu_control_t) __THROW;
>> -#define _FPU_GETCW(cw) __asm__ volatile ("movfcsr2gr %0,$r0" : "=r"(cw))
>> -#define _FPU_SETCW(cw) __asm__ volatile ("movgr2fcsr $r0,%0" : : "r"(cw))
>> +#define _FPU_GETCW(cw) __asm__ volatile ("movfcsr2gr %0,$fcsr0" : "=r"(cw))
>> +#define _FPU_SETCW(cw) __asm__ volatile ("movgr2fcsr $fcsr0,%0" : : "r"(cw))
>>   
>>   /* Default control word set at startup.  */
>>   extern fpu_control_t __fpu_control;
  

Patch

diff --git a/sysdeps/loongarch/fpu_control.h b/sysdeps/loongarch/fpu_control.h
index 54add4e01c..3cdf2417d9 100644
--- a/sysdeps/loongarch/fpu_control.h
+++ b/sysdeps/loongarch/fpu_control.h
@@ -91,8 +91,8 @@  typedef unsigned int fpu_control_t __attribute__ ((__mode__ (__SI__)));
 /* Macros for accessing the hardware control word.  */
 extern fpu_control_t __loongarch_fpu_getcw (void) __THROW;
 extern void __loongarch_fpu_setcw (fpu_control_t) __THROW;
-#define _FPU_GETCW(cw) __asm__ volatile ("movfcsr2gr %0,$r0" : "=r"(cw))
-#define _FPU_SETCW(cw) __asm__ volatile ("movgr2fcsr $r0,%0" : : "r"(cw))
+#define _FPU_GETCW(cw) __asm__ volatile ("movfcsr2gr %0,$fcsr0" : "=r"(cw))
+#define _FPU_SETCW(cw) __asm__ volatile ("movgr2fcsr $fcsr0,%0" : : "r"(cw))
 
 /* Default control word set at startup.  */
 extern fpu_control_t __fpu_control;