[PATCH v2] powerpc: Fix __fesetround_inline_nocheck on POWER9+ (BZ 31682)

[PATCH v2] powerpc: Fix __fesetround_inline_nocheck on POWER9+ (BZ 31682)

[Adhemerval Zanella]

The e68b1151f7460d5fa88c3a567c13f66052da79a7 commit changed the
__fesetround_inline_nocheck implementation to use mffscrni
(through __fe_mffscrn) instead of mtfsfi.  For generic powerpc
ceil/floor/trunc, the function is supposed to disable the
floating-point inexact exception enablebbit, however mffscrni
does not change any exception enable bits.

This patch fixes by reverting the optimization for the
__fesetround_inline_nocheck.

Checked on powerpc-linux-gnu.
---
 sysdeps/powerpc/fpu/fenv_libc.h        | 16 +++++-----------
 sysdeps/powerpc/fpu/round_to_integer.h |  6 +++---
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h
index f9167056a8..84b53d5d3e 100644
--- a/sysdeps/powerpc/fpu/fenv_libc.h
+++ b/sysdeps/powerpc/fpu/fenv_libc.h
@@ -182,19 +182,13 @@ __fesetround_inline (int round)
   return 0;
 }
 
-/* Same as __fesetround_inline, however without runtime check to use DFP
-   mtfsfi syntax (as relax_fenv_state) or if round value is valid.  */
+/* Same as __fesetround_inline, but it also disable the floating-point
+   exceptions (bits 56:63 - VE, OE, UE, ZE, XE, NI, and RN).  It does
+   not check if ROUND is a valid value.  */
 static inline void
-__fesetround_inline_nocheck (const int round)
+__fesetround_inline_disable_except (const int round)
 {
-#ifdef _ARCH_PWR9
-  __fe_mffscrn (round);
-#else
-  if (__glibc_likely (GLRO(dl_hwcap2) & PPC_FEATURE2_ARCH_3_00))
-    __fe_mffscrn (round);
-  else
-    asm volatile ("mtfsfi 7,%0" : : "n" (round));
-#endif
+  asm volatile ("mtfsfi 7,%0" : : "n" (round));
 }
 
 #define FPSCR_MASK(bit) (1 << (31 - (bit)))
diff --git a/sysdeps/powerpc/fpu/round_to_integer.h b/sysdeps/powerpc/fpu/round_to_integer.h
index b68833640f..c053719530 100644
--- a/sysdeps/powerpc/fpu/round_to_integer.h
+++ b/sysdeps/powerpc/fpu/round_to_integer.h
@@ -42,14 +42,14 @@ set_fenv_mode (enum round_mode mode)
   switch (mode)
   {
   case CEIL:
-    __fesetround_inline_nocheck (FE_UPWARD);
+    __fesetround_inline_disable_except (FE_UPWARD);
     break;
   case FLOOR:
-    __fesetround_inline_nocheck (FE_DOWNWARD);
+    __fesetround_inline_disable_except (FE_DOWNWARD);
     break;
   case TRUNC:
   case ROUND:
-    __fesetround_inline_nocheck (FE_TOWARDZERO);
+    __fesetround_inline_disable_except (FE_TOWARDZERO);
     break;
   case NEARBYINT:
     /*  Disable FE_INEXACT exception  */
-- 
2.43.0

Re: [PATCH v2] powerpc: Fix __fesetround_inline_nocheck on POWER9+ (BZ 31682)

[Peter Bergner]

On 5/6/24 12:07 PM, Adhemerval Zanella wrote:
> floating-point inexact exception enablebbit, however mffscrni

Is that supposed to be "enable bit" or "enabled bit"?



> +/* Same as __fesetround_inline, but it also disable the floating-point
> +   exceptions (bits 56:63 - VE, OE, UE, ZE, XE, NI, and RN).  It does
> +   not check if ROUND is a valid value.  */

s/disable/disables/


The rest LGTM.

Reviewed-by: Peter Bergner <bergner@linux.ibm.com>


I'd like to hear Paul's comments though.

Peter

Re: [PATCH v2] powerpc: Fix __fesetround_inline_nocheck on POWER9+ (BZ 31682)

[Paul E Murphy]

On 5/6/24 12:07 PM, Adhemerval Zanella wrote:
> The e68b1151f7460d5fa88c3a567c13f66052da79a7 commit changed the
> __fesetround_inline_nocheck implementation to use mffscrni
> (through __fe_mffscrn) instead of mtfsfi.  For generic powerpc
> ceil/floor/trunc, the function is supposed to disable the
> floating-point inexact exception enablebbit, however mffscrni
> does not change any exception enable bits.
> 
> This patch fixes by reverting the optimization for the
> __fesetround_inline_nocheck.
> 
> Checked on powerpc-linux-gnu.
> ---
>   sysdeps/powerpc/fpu/fenv_libc.h        | 16 +++++-----------
>   sysdeps/powerpc/fpu/round_to_integer.h |  6 +++---
>   2 files changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h
> index f9167056a8..84b53d5d3e 100644
> --- a/sysdeps/powerpc/fpu/fenv_libc.h
> +++ b/sysdeps/powerpc/fpu/fenv_libc.h
> @@ -182,19 +182,13 @@ __fesetround_inline (int round)
>     return 0;
>   }
>   
> -/* Same as __fesetround_inline, however without runtime check to use DFP
> -   mtfsfi syntax (as relax_fenv_state) or if round value is valid.  */
> +/* Same as __fesetround_inline, but it also disable the floating-point
> +   exceptions (bits 56:63 - VE, OE, UE, ZE, XE, NI, and RN).  It does
> +   not check if ROUND is a valid value.  */

This function will only disable XE (assuming NI is always 0). mtfsfi 
operates on 4 bit fields.

>   static inline void
> -__fesetround_inline_nocheck (const int round)
> +__fesetround_inline_disable_except (const int round)
>   {
> -#ifdef _ARCH_PWR9
> -  __fe_mffscrn (round);
> -#else
> -  if (__glibc_likely (GLRO(dl_hwcap2) & PPC_FEATURE2_ARCH_3_00))
> -    __fe_mffscrn (round);
> -  else
> -    asm volatile ("mtfsfi 7,%0" : : "n" (round));
> -#endif
> +  asm volatile ("mtfsfi 7,%0" : : "n" (round));
>   }
>   
>   #define FPSCR_MASK(bit) (1 << (31 - (bit)))
> diff --git a/sysdeps/powerpc/fpu/round_to_integer.h b/sysdeps/powerpc/fpu/round_to_integer.h
> index b68833640f..c053719530 100644
> --- a/sysdeps/powerpc/fpu/round_to_integer.h
> +++ b/sysdeps/powerpc/fpu/round_to_integer.h
> @@ -42,14 +42,14 @@ set_fenv_mode (enum round_mode mode)
>     switch (mode)
>     {
>     case CEIL:
> -    __fesetround_inline_nocheck (FE_UPWARD);
> +    __fesetround_inline_disable_except (FE_UPWARD);
>       break;
>     case FLOOR:
> -    __fesetround_inline_nocheck (FE_DOWNWARD);
> +    __fesetround_inline_disable_except (FE_DOWNWARD);
>       break;
>     case TRUNC:
>     case ROUND:
> -    __fesetround_inline_nocheck (FE_TOWARDZERO);
> +    __fesetround_inline_disable_except (FE_TOWARDZERO);
>       break;
>     case NEARBYINT:
>       /*  Disable FE_INEXACT exception  */

Re: [PATCH v2] powerpc: Fix __fesetround_inline_nocheck on POWER9+ (BZ 31682)

[Adhemerval Zanella Netto]

On 06/05/24 17:07, Paul E Murphy wrote:
> 
> 
> On 5/6/24 12:07 PM, Adhemerval Zanella wrote:
>> The e68b1151f7460d5fa88c3a567c13f66052da79a7 commit changed the
>> __fesetround_inline_nocheck implementation to use mffscrni
>> (through __fe_mffscrn) instead of mtfsfi.  For generic powerpc
>> ceil/floor/trunc, the function is supposed to disable the
>> floating-point inexact exception enablebbit, however mffscrni
>> does not change any exception enable bits.
>>
>> This patch fixes by reverting the optimization for the
>> __fesetround_inline_nocheck.
>>
>> Checked on powerpc-linux-gnu.
>> ---
>>   sysdeps/powerpc/fpu/fenv_libc.h        | 16 +++++-----------
>>   sysdeps/powerpc/fpu/round_to_integer.h |  6 +++---
>>   2 files changed, 8 insertions(+), 14 deletions(-)
>>
>> diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h
>> index f9167056a8..84b53d5d3e 100644
>> --- a/sysdeps/powerpc/fpu/fenv_libc.h
>> +++ b/sysdeps/powerpc/fpu/fenv_libc.h
>> @@ -182,19 +182,13 @@ __fesetround_inline (int round)
>>     return 0;
>>   }
>>   -/* Same as __fesetround_inline, however without runtime check to use DFP
>> -   mtfsfi syntax (as relax_fenv_state) or if round value is valid.  */
>> +/* Same as __fesetround_inline, but it also disable the floating-point
>> +   exceptions (bits 56:63 - VE, OE, UE, ZE, XE, NI, and RN).  It does
>> +   not check if ROUND is a valid value.  */
> 
> This function will only disable XE (assuming NI is always 0). mtfsfi operates on 4 bit fields.

I will update the comment.

> 
>>   static inline void
>> -__fesetround_inline_nocheck (const int round)
>> +__fesetround_inline_disable_except (const int round)
>>   {
>> -#ifdef _ARCH_PWR9
>> -  __fe_mffscrn (round);
>> -#else
>> -  if (__glibc_likely (GLRO(dl_hwcap2) & PPC_FEATURE2_ARCH_3_00))
>> -    __fe_mffscrn (round);
>> -  else
>> -    asm volatile ("mtfsfi 7,%0" : : "n" (round));
>> -#endif
>> +  asm volatile ("mtfsfi 7,%0" : : "n" (round));
>>   }
>>     #define FPSCR_MASK(bit) (1 << (31 - (bit)))
>> diff --git a/sysdeps/powerpc/fpu/round_to_integer.h b/sysdeps/powerpc/fpu/round_to_integer.h
>> index b68833640f..c053719530 100644
>> --- a/sysdeps/powerpc/fpu/round_to_integer.h
>> +++ b/sysdeps/powerpc/fpu/round_to_integer.h
>> @@ -42,14 +42,14 @@ set_fenv_mode (enum round_mode mode)
>>     switch (mode)
>>     {
>>     case CEIL:
>> -    __fesetround_inline_nocheck (FE_UPWARD);
>> +    __fesetround_inline_disable_except (FE_UPWARD);
>>       break;
>>     case FLOOR:
>> -    __fesetround_inline_nocheck (FE_DOWNWARD);
>> +    __fesetround_inline_disable_except (FE_DOWNWARD);
>>       break;
>>     case TRUNC:
>>     case ROUND:
>> -    __fesetround_inline_nocheck (FE_TOWARDZERO);
>> +    __fesetround_inline_disable_except (FE_TOWARDZERO);
>>       break;
>>     case NEARBYINT:
>>       /*  Disable FE_INEXACT exception  */