diff mbox series

LoongArch: Add math-barriers.h

Message ID 20230214085713.44338-1-xry111@xry111.site
State Committed
Commit aa4b45a34687595d37539feb367b0c691e41362b
Headers
Series LoongArch: Add math-barriers.h |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Xi Ruoyao Feb. 14, 2023, 8:57 a.m. UTC 
  This patch implements the LoongArch specific math barriers in order to omit
the store and load from stack if possible.

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
 sysdeps/loongarch/fpu/math-barriers.h | 28 +++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 sysdeps/loongarch/fpu/math-barriers.h

Comments

Adhemerval Zanella Netto Feb. 15, 2023, 1:47 p.m. UTC | #1 
On 14/02/23 05:57, Xi Ruoyao via Libc-alpha wrote:
> This patch implements the LoongArch specific math barriers in order to omit
> the store and load from stack if possible.

LGTM, I have not see any regression with qemu.  As a side note, math/test-fenv
shows a failure so you might take a look, you might need to update ULPs as well.

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

> 
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>

We do not use signed-off-by, so no need to add it.

> ---
>  sysdeps/loongarch/fpu/math-barriers.h | 28 +++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 sysdeps/loongarch/fpu/math-barriers.h
> 
> diff --git a/sysdeps/loongarch/fpu/math-barriers.h b/sysdeps/loongarch/fpu/math-barriers.h
> new file mode 100644
> index 0000000000..3e977e43a6
> --- /dev/null
> +++ b/sysdeps/loongarch/fpu/math-barriers.h
> @@ -0,0 +1,28 @@
> +/* Control when floating-point expressions are evaluated.  LoongArch version.
> +   Copyright (C) 2023 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _LOONGARCH_MATH_BARRIERS_H
> +#define _LOONGARCH_MATH_BARRIERS_H 1
> +
> +/* Generic code forces values to memory; we don't need to do that.  */
> +#define math_opt_barrier(x)					\
> +  ({ __typeof (x) __x = (x); __asm ("" : "+frm" (__x)); __x; })
> +#define math_force_eval(x)						\
> +  ({ __typeof (x) __x = (x); __asm __volatile__ ("" : : "frm" (__x)); })
> +
> +#endif /* math-barriers.h */
Xi Ruoyao Feb. 15, 2023, 2:22 p.m. UTC | #2 
On Wed, 2023-02-15 at 10:47 -0300, Adhemerval Zanella Netto wrote:
> On 14/02/23 05:57, Xi Ruoyao via Libc-alpha wrote:
> > This patch implements the LoongArch specific math barriers in order to omit
> > the store and load from stack if possible.
> 
> LGTM, I have not see any regression with qemu.  As a side note, math/test-fenv
> shows a failure so you might take a look, you might need to update ULPs as well.

Hmm, on a real hardware I saw no failures even with GCC 13 trunk (some
aggressive FVRP optimization has been added into GCC 13). But maybe
things have been changed since my last test.  I'll test again.

> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
Adhemerval Zanella Netto Feb. 15, 2023, 2:25 p.m. UTC | #3 
On 15/02/23 11:22, Xi Ruoyao wrote:
> On Wed, 2023-02-15 at 10:47 -0300, Adhemerval Zanella Netto wrote:
>> On 14/02/23 05:57, Xi Ruoyao via Libc-alpha wrote:
>>> This patch implements the LoongArch specific math barriers in order to omit
>>> the store and load from stack if possible.
>>
>> LGTM, I have not see any regression with qemu.  As a side note, math/test-fenv
>> shows a failure so you might take a look, you might need to update ULPs as well.
> 
> Hmm, on a real hardware I saw no failures even with GCC 13 trunk (some
> aggressive FVRP optimization has been added into GCC 13). But maybe
> things have been changed since my last test.  I'll test again.

I meant that I saw math/test-fenv failures with and without the patch
and I the master qemu (f670b3eec7f5d1ed8c4573ef244e7b8c6b32001b).

> 
>> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>
Xi Ruoyao Feb. 16, 2023, 1:02 p.m. UTC | #4 
On Wed, 2023-02-15 at 11:25 -0300, Adhemerval Zanella Netto wrote:
> 
> I meant that I saw math/test-fenv failures with and without the patch
> and I the master qemu (f670b3eec7f5d1ed8c4573ef244e7b8c6b32001b).

Hi Adhemerval,

Which GCC version did you used?  I'll try with the same GCC version on
the hardware. If it still fails I'll consider it a QEMU issue and report
to QEMU maintainers.
Adhemerval Zanella Netto Feb. 16, 2023, 1:43 p.m. UTC | #5 
On 16/02/23 10:02, Xi Ruoyao wrote:
> On Wed, 2023-02-15 at 11:25 -0300, Adhemerval Zanella Netto wrote:
>>
>> I meant that I saw math/test-fenv failures with and without the patch
>> and I the master qemu (f670b3eec7f5d1ed8c4573ef244e7b8c6b32001b).
> 
> Hi Adhemerval,
> 
> Which GCC version did you used?  I'll try with the same GCC version on
> the hardware. If it still fails I'll consider it a QEMU issue and report
> to QEMU maintainers.
> 

This is the one I used:

gcc version 12.1.1 20220719 [releases/gcc-12 r12-8552-g65941a91091] (GCC)
Xi Ruoyao Feb. 27, 2023, 10:45 a.m. UTC | #6 
On Thu, 2023-02-16 at 10:43 -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 16/02/23 10:02, Xi Ruoyao wrote:
> > On Wed, 2023-02-15 at 11:25 -0300, Adhemerval Zanella Netto wrote:
> > > 
> > > I meant that I saw math/test-fenv failures with and without the
> > > patch
> > > and I the master qemu (f670b3eec7f5d1ed8c4573ef244e7b8c6b32001b).
> > 
> > Hi Adhemerval,
> > 
> > Which GCC version did you used?  I'll try with the same GCC version
> > on
> > the hardware. If it still fails I'll consider it a QEMU issue and
> > report
> > to QEMU maintainers.
> > 
> 
> This is the one I used:
> 
> gcc version 12.1.1 20220719 [releases/gcc-12 r12-8552-g65941a91091]
> (GCC)

On a hardware (3A5000-7A2000-EVB) the test passes with the same GCC.  My
first guess would be a QEMU issue.  There could be a difference in
Binutils version but I don't think a Binutils difference can cause fenv
failure...

BTW could you (or Yinyu) push the patch?  I don't have a write access of
glibc.git (yet).
Adhemerval Zanella Netto Feb. 27, 2023, 11:23 a.m. UTC | #7 
On 27/02/23 07:45, Xi Ruoyao wrote:
> On Thu, 2023-02-16 at 10:43 -0300, Adhemerval Zanella Netto wrote:
>>
>>
>> On 16/02/23 10:02, Xi Ruoyao wrote:
>>> On Wed, 2023-02-15 at 11:25 -0300, Adhemerval Zanella Netto wrote:
>>>>
>>>> I meant that I saw math/test-fenv failures with and without the
>>>> patch
>>>> and I the master qemu (f670b3eec7f5d1ed8c4573ef244e7b8c6b32001b).
>>>
>>> Hi Adhemerval,
>>>
>>> Which GCC version did you used?  I'll try with the same GCC version
>>> on
>>> the hardware. If it still fails I'll consider it a QEMU issue and
>>> report
>>> to QEMU maintainers.
>>>
>>
>> This is the one I used:
>>
>> gcc version 12.1.1 20220719 [releases/gcc-12 r12-8552-g65941a91091]
>> (GCC)
> 
> On a hardware (3A5000-7A2000-EVB) the test passes with the same GCC.  My
> first guess would be a QEMU issue.  There could be a difference in
> Binutils version but I don't think a Binutils difference can cause fenv
> failure...
> 

That is my guess as well it is qemu issue.

> BTW could you (or Yinyu) push the patch?  I don't have a write access of
> glibc.git (yet).
> 

Done.
WANG Xuerui Feb. 27, 2023, 11:25 a.m. UTC | #8 
On 2/27/23 19:23, Adhemerval Zanella Netto wrote:
>
> On 27/02/23 07:45, Xi Ruoyao wrote:
>> On Thu, 2023-02-16 at 10:43 -0300, Adhemerval Zanella Netto wrote:
>>>
>>> On 16/02/23 10:02, Xi Ruoyao wrote:
>>>> On Wed, 2023-02-15 at 11:25 -0300, Adhemerval Zanella Netto wrote:
>>>>> I meant that I saw math/test-fenv failures with and without the
>>>>> patch
>>>>> and I the master qemu (f670b3eec7f5d1ed8c4573ef244e7b8c6b32001b).
>>>> Hi Adhemerval,
>>>>
>>>> Which GCC version did you used?  I'll try with the same GCC version
>>>> on
>>>> the hardware. If it still fails I'll consider it a QEMU issue and
>>>> report
>>>> to QEMU maintainers.
>>>>
>>> This is the one I used:
>>>
>>> gcc version 12.1.1 20220719 [releases/gcc-12 r12-8552-g65941a91091]
>>> (GCC)
>> On a hardware (3A5000-7A2000-EVB) the test passes with the same GCC.  My
>> first guess would be a QEMU issue.  There could be a difference in
>> Binutils version but I don't think a Binutils difference can cause fenv
>> failure...
>>
> That is my guess as well it is qemu issue.
Interesting, I'll help check the qemu side this week.
diff mbox series

Patch

diff --git a/sysdeps/loongarch/fpu/math-barriers.h b/sysdeps/loongarch/fpu/math-barriers.h
new file mode 100644
index 0000000000..3e977e43a6
--- /dev/null
+++ b/sysdeps/loongarch/fpu/math-barriers.h
@@ -0,0 +1,28 @@ 
+/* Control when floating-point expressions are evaluated.  LoongArch version.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _LOONGARCH_MATH_BARRIERS_H
+#define _LOONGARCH_MATH_BARRIERS_H 1
+
+/* Generic code forces values to memory; we don't need to do that.  */
+#define math_opt_barrier(x)					\
+  ({ __typeof (x) __x = (x); __asm ("" : "+frm" (__x)); __x; })
+#define math_force_eval(x)						\
+  ({ __typeof (x) __x = (x); __asm __volatile__ ("" : : "frm" (__x)); })
+
+#endif /* math-barriers.h */