[PATCH] ARC:fpu: add extra capability check before use of sqrt and fma builtins

public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed

* [PATCH] ARC:fpu: add extra capability check before use of sqrt and fma builtins
@ 2022-12-21 16:28 Pavel.Kozlov
  2023-01-16 14:18 ` Adhemerval Zanella Netto
  2023-01-17 12:12 ` [PATCH v2] " Pavel.Kozlov
  0 siblings, 2 replies; 5+ messages in thread
From: Pavel.Kozlov @ 2022-12-21 16:28 UTC (permalink / raw)
  To: libc-alpha; +Cc: linux-snps-arc, Pavel Kozlov

From: Pavel Kozlov <pavel.kozlov@synopsys.com>

Add extra check for compiler definitions to ensure that compiler provides
sqrt and fma hw fpu instructions else use software implementation.

As divide/sqrt and FMA hw support from CPU side is optional,
the compiler can be configured by options to generate hw FPU instructions,
but without use of FDDIV, FDSQRT, FSDIV, FSSQRT, FDMADD and FSMADD
instructions. In this case __builtin_sqrt and __builtin_sqrtf provided by
compiler can't be used inside the glibc code, as these builtins are used
in implementations of sqrt() and sqrtf() functions but at the same time
these builtins unfold to sqrt() and sqrtf(). So it is possible to receive
code like that:

0001c4b4 <__ieee754_sqrtf>:
   1c4b4:    0001 0000      b     0         ;1c4b4 <__ieee754_sqrtf>

The same is also true for __builtin_fma and __builtin_fmaf.
---
 sysdeps/arc/fpu/math-use-builtins-fma.h  | 14 ++++++++++++--
 sysdeps/arc/fpu/math-use-builtins-sqrt.h | 14 ++++++++++++--
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/sysdeps/arc/fpu/math-use-builtins-fma.h b/sysdeps/arc/fpu/math-use-builtins-fma.h
index eede75aa41be..082badf48201 100644
--- a/sysdeps/arc/fpu/math-use-builtins-fma.h
+++ b/sysdeps/arc/fpu/math-use-builtins-fma.h
@@ -1,4 +1,14 @@
-#define USE_FMA_BUILTIN 1
-#define USE_FMAF_BUILTIN 1
+#if defined __ARC_FPU_DP_DIV__
+# define USE_SQRT_BUILTIN 1
+#else
+# define USE_SQRT_BUILTIN 0
+#endif
+
+#if defined __ARC_FPU_SP_DIV__
+# define USE_SQRTF_BUILTIN 1
+#else
+# define USE_SQRTF_BUILTIN 0
+#endif
+
 #define USE_FMAL_BUILTIN 0
 #define USE_FMAF128_BUILTIN 0
diff --git a/sysdeps/arc/fpu/math-use-builtins-sqrt.h b/sysdeps/arc/fpu/math-use-builtins-sqrt.h
index e94c915ba66a..a449bc609295 100644
--- a/sysdeps/arc/fpu/math-use-builtins-sqrt.h
+++ b/sysdeps/arc/fpu/math-use-builtins-sqrt.h
@@ -1,4 +1,14 @@
-#define USE_SQRT_BUILTIN 1
-#define USE_SQRTF_BUILTIN 1
+#if defined __ARC_FPU_DP_DIV__
+# define USE_SQRT_BUILTIN 1
+#else
+# define USE_SQRT_BUILTIN 0
+#endif
+
+#if defined __ARC_FPU_SP_DIV__
+# define USE_SQRTF_BUILTIN 1
+#else
+# define USE_SQRTF_BUILTIN 0
+#endif
+
 #define USE_SQRTL_BUILTIN 0
 #define USE_SQRTF128_BUILTIN 0
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ARC:fpu: add extra capability check before use of sqrt and fma builtins
  2022-12-21 16:28 [PATCH] ARC:fpu: add extra capability check before use of sqrt and fma builtins Pavel.Kozlov
@ 2023-01-16 14:18 ` Adhemerval Zanella Netto
  2023-01-17 12:31   ` Pavel Kozlov
  2023-01-17 12:12 ` [PATCH v2] " Pavel.Kozlov
  1 sibling, 1 reply; 5+ messages in thread
From: Adhemerval Zanella Netto @ 2023-01-16 14:18 UTC (permalink / raw)
  To: Pavel.Kozlov, libc-alpha; +Cc: linux-snps-arc



On 21/12/22 13:28, Pavel.Kozlov--- via Libc-alpha wrote:
> From: Pavel Kozlov <pavel.kozlov@synopsys.com>
> 
> Add extra check for compiler definitions to ensure that compiler provides
> sqrt and fma hw fpu instructions else use software implementation.
> 
> As divide/sqrt and FMA hw support from CPU side is optional,
> the compiler can be configured by options to generate hw FPU instructions,
> but without use of FDDIV, FDSQRT, FSDIV, FSSQRT, FDMADD and FSMADD
> instructions. In this case __builtin_sqrt and __builtin_sqrtf provided by
> compiler can't be used inside the glibc code, as these builtins are used
> in implementations of sqrt() and sqrtf() functions but at the same time
> these builtins unfold to sqrt() and sqrtf(). So it is possible to receive
> code like that:
> 
> 0001c4b4 <__ieee754_sqrtf>:
>    1c4b4:    0001 0000      b     0         ;1c4b4 <__ieee754_sqrtf>
> 
> The same is also true for __builtin_fma and __builtin_fmaf.
> ---
>  sysdeps/arc/fpu/math-use-builtins-fma.h  | 14 ++++++++++++--
>  sysdeps/arc/fpu/math-use-builtins-sqrt.h | 14 ++++++++++++--
>  2 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/sysdeps/arc/fpu/math-use-builtins-fma.h b/sysdeps/arc/fpu/math-use-builtins-fma.h
> index eede75aa41be..082badf48201 100644
> --- a/sysdeps/arc/fpu/math-use-builtins-fma.h
> +++ b/sysdeps/arc/fpu/math-use-builtins-fma.h
> @@ -1,4 +1,14 @@
> -#define USE_FMA_BUILTIN 1
> -#define USE_FMAF_BUILTIN 1
> +#if defined __ARC_FPU_DP_DIV__
> +# define USE_SQRT_BUILTIN 1
> +#else
> +# define USE_SQRT_BUILTIN 0
> +#endif
> +
> +#if defined __ARC_FPU_SP_DIV__
> +# define USE_SQRTF_BUILTIN 1
> +#else
> +# define USE_SQRTF_BUILTIN 0
> +#endif
> +
>  #define USE_FMAL_BUILTIN 0
>  #define USE_FMAF128_BUILTIN 0

This is wrong, sqrt use macro do not belong for the fma switch file.

> diff --git a/sysdeps/arc/fpu/math-use-builtins-sqrt.h b/sysdeps/arc/fpu/math-use-builtins-sqrt.h
> index e94c915ba66a..a449bc609295 100644
> --- a/sysdeps/arc/fpu/math-use-builtins-sqrt.h
> +++ b/sysdeps/arc/fpu/math-use-builtins-sqrt.h
> @@ -1,4 +1,14 @@
> -#define USE_SQRT_BUILTIN 1
> -#define USE_SQRTF_BUILTIN 1
> +#if defined __ARC_FPU_DP_DIV__
> +# define USE_SQRT_BUILTIN 1
> +#else
> +# define USE_SQRT_BUILTIN 0
> +#endif
> +
> +#if defined __ARC_FPU_SP_DIV__
> +# define USE_SQRTF_BUILTIN 1
> +#else
> +# define USE_SQRTF_BUILTIN 0
> +#endif
> +
>  #define USE_SQRTL_BUILTIN 0
>  #define USE_SQRTF128_BUILTIN 0

This is ok.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ARC:fpu: add extra capability check before use of sqrt and fma builtins
  2023-01-16 14:18 ` Adhemerval Zanella Netto
@ 2023-01-17 12:31   ` Pavel Kozlov
  0 siblings, 0 replies; 5+ messages in thread
From: Pavel Kozlov @ 2023-01-17 12:31 UTC (permalink / raw)
  To: Adhemerval Zanella Netto, libc-alpha; +Cc: linux-snps-arc

> This is wrong, sqrt use macro do not belong for the fma switch file.

Thank you for the review and your notice. I was inattentive when 
moving changes from the branch I had. This has been fixed in
v2 of the patch [1].
I've manually checked (by objdump -d output review) that now 
expected code for libm is generated in different cases (when compiler 
provides support for extra instructions and sets macroses 
__ARC_FPU_DP_DIV__, __ARC_FPU_SP_DIV__, __ARC_FPU_DP_FMA__, 
__ARC_FPU_SP_FMA__ and when not).

[1]
http://lists.infradead.org/pipermail/linux-snps-arc/2023-January/006771.html

--
Pavel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] ARC:fpu: add extra capability check before use of sqrt and fma builtins
  2022-12-21 16:28 [PATCH] ARC:fpu: add extra capability check before use of sqrt and fma builtins Pavel.Kozlov
  2023-01-16 14:18 ` Adhemerval Zanella Netto
@ 2023-01-17 12:12 ` Pavel.Kozlov
  2023-01-17 14:31   ` Adhemerval Zanella Netto
  1 sibling, 1 reply; 5+ messages in thread
From: Pavel.Kozlov @ 2023-01-17 12:12 UTC (permalink / raw)
  To: libc-alpha; +Cc: linux-snps-arc, Pavel Kozlov

From: Pavel Kozlov <pavel.kozlov@synopsys.com>

Add extra check for compiler definitions to ensure that compiler provides
sqrt and fma hw fpu instructions else use software implementation.

As divide/sqrt and FMA hw support from CPU side is optional,
the compiler can be configured by options to generate hw FPU instructions,
but without use of FDDIV, FDSQRT, FSDIV, FSSQRT, FDMADD and FSMADD
instructions. In this case __builtin_sqrt and __builtin_sqrtf provided by
compiler can't be used inside the glibc code, as these builtins are used
in implementations of sqrt() and sqrtf() functions but at the same time
these builtins unfold to sqrt() and sqrtf(). So it is possible to receive
code like that:

0001c4b4 <__ieee754_sqrtf>:
   1c4b4:    0001 0000      b     0         ;1c4b4 <__ieee754_sqrtf>

The same is also true for __builtin_fma and __builtin_fmaf.

---
Changes in v2:
 - Fixed macros definitions for FMA

 sysdeps/arc/fpu/math-use-builtins-fma.h  | 14 ++++++++++++--
 sysdeps/arc/fpu/math-use-builtins-sqrt.h | 14 ++++++++++++--
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/sysdeps/arc/fpu/math-use-builtins-fma.h b/sysdeps/arc/fpu/math-use-builtins-fma.h
index eede75aa41be..2acd8113ce2c 100644
--- a/sysdeps/arc/fpu/math-use-builtins-fma.h
+++ b/sysdeps/arc/fpu/math-use-builtins-fma.h
@@ -1,4 +1,14 @@
-#define USE_FMA_BUILTIN 1
-#define USE_FMAF_BUILTIN 1
+#if defined __ARC_FPU_DP_FMA__
+# define USE_FMA_BUILTIN 1
+#else
+# define USE_FMA_BUILTIN 0
+#endif
+
+#if defined __ARC_FPU_SP_FMA__
+# define USE_FMAF_BUILTIN 1
+#else
+# define USE_FMAF_BUILTIN 0
+#endif
+
 #define USE_FMAL_BUILTIN 0
 #define USE_FMAF128_BUILTIN 0
diff --git a/sysdeps/arc/fpu/math-use-builtins-sqrt.h b/sysdeps/arc/fpu/math-use-builtins-sqrt.h
index e94c915ba66a..a449bc609295 100644
--- a/sysdeps/arc/fpu/math-use-builtins-sqrt.h
+++ b/sysdeps/arc/fpu/math-use-builtins-sqrt.h
@@ -1,4 +1,14 @@
-#define USE_SQRT_BUILTIN 1
-#define USE_SQRTF_BUILTIN 1
+#if defined __ARC_FPU_DP_DIV__
+# define USE_SQRT_BUILTIN 1
+#else
+# define USE_SQRT_BUILTIN 0
+#endif
+
+#if defined __ARC_FPU_SP_DIV__
+# define USE_SQRTF_BUILTIN 1
+#else
+# define USE_SQRTF_BUILTIN 0
+#endif
+
 #define USE_SQRTL_BUILTIN 0
 #define USE_SQRTF128_BUILTIN 0
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] ARC:fpu: add extra capability check before use of sqrt and fma builtins
  2023-01-17 12:12 ` [PATCH v2] " Pavel.Kozlov
@ 2023-01-17 14:31   ` Adhemerval Zanella Netto
  0 siblings, 0 replies; 5+ messages in thread
From: Adhemerval Zanella Netto @ 2023-01-17 14:31 UTC (permalink / raw)
  To: Pavel.Kozlov, libc-alpha; +Cc: linux-snps-arc



On 17/01/23 09:12, Pavel.Kozlov--- via Libc-alpha wrote:
> From: Pavel Kozlov <pavel.kozlov@synopsys.com>
> 
> Add extra check for compiler definitions to ensure that compiler provides
> sqrt and fma hw fpu instructions else use software implementation.
> 
> As divide/sqrt and FMA hw support from CPU side is optional,
> the compiler can be configured by options to generate hw FPU instructions,
> but without use of FDDIV, FDSQRT, FSDIV, FSSQRT, FDMADD and FSMADD
> instructions. In this case __builtin_sqrt and __builtin_sqrtf provided by
> compiler can't be used inside the glibc code, as these builtins are used
> in implementations of sqrt() and sqrtf() functions but at the same time
> these builtins unfold to sqrt() and sqrtf(). So it is possible to receive
> code like that:
> 
> 0001c4b4 <__ieee754_sqrtf>:
>    1c4b4:    0001 0000      b     0         ;1c4b4 <__ieee754_sqrtf>
> 
> The same is also true for __builtin_fma and __builtin_fmaf.

LGTM, thanks.  You might need to check with Carlos O'Donnel to see it you
could install this for 2.36.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 
> ---
> Changes in v2:
>  - Fixed macros definitions for FMA
> 
>  sysdeps/arc/fpu/math-use-builtins-fma.h  | 14 ++++++++++++--
>  sysdeps/arc/fpu/math-use-builtins-sqrt.h | 14 ++++++++++++--
>  2 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/sysdeps/arc/fpu/math-use-builtins-fma.h b/sysdeps/arc/fpu/math-use-builtins-fma.h
> index eede75aa41be..2acd8113ce2c 100644
> --- a/sysdeps/arc/fpu/math-use-builtins-fma.h
> +++ b/sysdeps/arc/fpu/math-use-builtins-fma.h
> @@ -1,4 +1,14 @@
> -#define USE_FMA_BUILTIN 1
> -#define USE_FMAF_BUILTIN 1
> +#if defined __ARC_FPU_DP_FMA__
> +# define USE_FMA_BUILTIN 1
> +#else
> +# define USE_FMA_BUILTIN 0
> +#endif
> +
> +#if defined __ARC_FPU_SP_FMA__
> +# define USE_FMAF_BUILTIN 1
> +#else
> +# define USE_FMAF_BUILTIN 0
> +#endif
> +
>  #define USE_FMAL_BUILTIN 0
>  #define USE_FMAF128_BUILTIN 0
> diff --git a/sysdeps/arc/fpu/math-use-builtins-sqrt.h b/sysdeps/arc/fpu/math-use-builtins-sqrt.h
> index e94c915ba66a..a449bc609295 100644
> --- a/sysdeps/arc/fpu/math-use-builtins-sqrt.h
> +++ b/sysdeps/arc/fpu/math-use-builtins-sqrt.h
> @@ -1,4 +1,14 @@
> -#define USE_SQRT_BUILTIN 1
> -#define USE_SQRTF_BUILTIN 1
> +#if defined __ARC_FPU_DP_DIV__
> +# define USE_SQRT_BUILTIN 1
> +#else
> +# define USE_SQRT_BUILTIN 0
> +#endif
> +
> +#if defined __ARC_FPU_SP_DIV__
> +# define USE_SQRTF_BUILTIN 1
> +#else
> +# define USE_SQRTF_BUILTIN 0
> +#endif
> +
>  #define USE_SQRTL_BUILTIN 0
>  #define USE_SQRTF128_BUILTIN 0

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-01-17 14:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-21 16:28 [PATCH] ARC:fpu: add extra capability check before use of sqrt and fma builtins Pavel.Kozlov
2023-01-16 14:18 ` Adhemerval Zanella Netto
2023-01-17 12:31   ` Pavel Kozlov
2023-01-17 12:12 ` [PATCH v2] " Pavel.Kozlov
2023-01-17 14:31   ` Adhemerval Zanella Netto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).