public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Stam Markianos-Wright <stam.markianos-wright@arm.com>
To: <gcc-patches@gcc.gnu.org>
Cc: <kyrylo.tkachov@arm.com>, <Richard.Earnshaw@arm.com>,
	"Stam Markianos-Wright" <stam.markianos-wright@arm.com>
Subject: [committed gcc12 backport] arm: Stop vadcq, vsbcq intrinsics from overwriting the FPSCR NZ flags
Date: Thu, 18 May 2023 11:53:24 +0100	[thread overview]
Message-ID: <20230518105331.1301864-4-stam.markianos-wright@arm.com> (raw)
In-Reply-To: <20230518105331.1301864-1-stam.markianos-wright@arm.com>

Hi all,

We noticed that calls to the vadcq and vsbcq intrinsics, both of
which use __builtin_arm_set_fpscr_nzcvqc to set the Carry flag in
the FPSCR, would produce the following code:

```
< r2 is the *carry input >
vmrs	r3, FPSCR_nzcvqc
bic	r3, r3, #536870912
orr	r3, r3, r2, lsl #29
vmsr	FPSCR_nzcvqc, r3
```

when the MVE ACLE instead gives a different instruction sequence of:
```
< Rt is the *carry input >
VMRS Rs,FPSCR_nzcvqc
BFI Rs,Rt,#29,#1
VMSR FPSCR_nzcvqc,Rs
```

the bic + orr pair is slower and it's also wrong, because, if the
*carry input is greater than 1, then we risk overwriting the top two
bits of the FPSCR register (the N and Z flags).

This turned out to be a problem in the header file and the solution was
to simply add a `& 1x0u` to the `*carry` input: then the compiler knows
that we only care about the lowest bit and can optimise to a BFI.

Ok for trunk?

Thanks,
Stam Markianos-Wright

gcc/ChangeLog:

	* config/arm/arm_mve.h (__arm_vadcq_s32): Fix arithmetic.
	(__arm_vadcq_u32): Likewise.
	(__arm_vadcq_m_s32): Likewise.
	(__arm_vadcq_m_u32): Likewise.
	(__arm_vsbcq_s32): Likewise.
	(__arm_vsbcq_u32): Likewise.
	(__arm_vsbcq_m_s32): Likewise.
	(__arm_vsbcq_m_u32): Likewise.
	* config/arm/mve.md (get_fpscr_nzcvqc): Make unspec_volatile.

gcc/testsuite/ChangeLog:
	* gcc.target/arm/mve/mve_vadcq_vsbcq_fpscr_overwrite.c: New.

(cherry picked from commit f1417d051be094ffbce228e11951f3e12e8fca1c)
---
 gcc/config/arm/arm_mve.h                      | 16 ++---
 gcc/config/arm/mve.md                         |  2 +-
 .../arm/mve/mve_vadcq_vsbcq_fpscr_overwrite.c | 67 +++++++++++++++++++
 3 files changed, 76 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/arm/mve/mve_vadcq_vsbcq_fpscr_overwrite.c

diff --git a/gcc/config/arm/arm_mve.h b/gcc/config/arm/arm_mve.h
index 82ceec2bbfc..6bf1794d2ff 100644
--- a/gcc/config/arm/arm_mve.h
+++ b/gcc/config/arm/arm_mve.h
@@ -16055,7 +16055,7 @@ __extension__ extern __inline int32x4_t
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 __arm_vadcq_s32 (int32x4_t __a, int32x4_t __b, unsigned * __carry)
 {
-  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | (*__carry << 29));
+  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | ((*__carry & 0x1u) << 29));
   int32x4_t __res = __builtin_mve_vadcq_sv4si (__a, __b);
   *__carry = (__builtin_arm_get_fpscr_nzcvqc () >> 29) & 0x1u;
   return __res;
@@ -16065,7 +16065,7 @@ __extension__ extern __inline uint32x4_t
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 __arm_vadcq_u32 (uint32x4_t __a, uint32x4_t __b, unsigned * __carry)
 {
-  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | (*__carry << 29));
+  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | ((*__carry & 0x1u) << 29));
   uint32x4_t __res = __builtin_mve_vadcq_uv4si (__a, __b);
   *__carry = (__builtin_arm_get_fpscr_nzcvqc () >> 29) & 0x1u;
   return __res;
@@ -16075,7 +16075,7 @@ __extension__ extern __inline int32x4_t
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 __arm_vadcq_m_s32 (int32x4_t __inactive, int32x4_t __a, int32x4_t __b, unsigned * __carry, mve_pred16_t __p)
 {
-  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | (*__carry << 29));
+  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | ((*__carry & 0x1u) << 29));
   int32x4_t __res = __builtin_mve_vadcq_m_sv4si (__inactive, __a, __b, __p);
   *__carry = (__builtin_arm_get_fpscr_nzcvqc () >> 29) & 0x1u;
   return __res;
@@ -16085,7 +16085,7 @@ __extension__ extern __inline uint32x4_t
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 __arm_vadcq_m_u32 (uint32x4_t __inactive, uint32x4_t __a, uint32x4_t __b, unsigned * __carry, mve_pred16_t __p)
 {
-  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | (*__carry << 29));
+  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | ((*__carry & 0x1u) << 29));
   uint32x4_t __res =  __builtin_mve_vadcq_m_uv4si (__inactive, __a, __b, __p);
   *__carry = (__builtin_arm_get_fpscr_nzcvqc () >> 29) & 0x1u;
   return __res;
@@ -16131,7 +16131,7 @@ __extension__ extern __inline int32x4_t
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 __arm_vsbcq_s32 (int32x4_t __a, int32x4_t __b, unsigned * __carry)
 {
-  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | (*__carry << 29));
+  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | ((*__carry & 0x1u) << 29));
   int32x4_t __res = __builtin_mve_vsbcq_sv4si (__a, __b);
   *__carry = (__builtin_arm_get_fpscr_nzcvqc () >> 29) & 0x1u;
   return __res;
@@ -16141,7 +16141,7 @@ __extension__ extern __inline uint32x4_t
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 __arm_vsbcq_u32 (uint32x4_t __a, uint32x4_t __b, unsigned * __carry)
 {
-  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | (*__carry << 29));
+  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | ((*__carry & 0x1u) << 29));
   uint32x4_t __res =  __builtin_mve_vsbcq_uv4si (__a, __b);
   *__carry = (__builtin_arm_get_fpscr_nzcvqc () >> 29) & 0x1u;
   return __res;
@@ -16151,7 +16151,7 @@ __extension__ extern __inline int32x4_t
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 __arm_vsbcq_m_s32 (int32x4_t __inactive, int32x4_t __a, int32x4_t __b, unsigned * __carry, mve_pred16_t __p)
 {
-  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | (*__carry << 29));
+  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | ((*__carry & 0x1u) << 29));
   int32x4_t __res = __builtin_mve_vsbcq_m_sv4si (__inactive, __a, __b, __p);
   *__carry = (__builtin_arm_get_fpscr_nzcvqc () >> 29) & 0x1u;
   return __res;
@@ -16161,7 +16161,7 @@ __extension__ extern __inline uint32x4_t
 __attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
 __arm_vsbcq_m_u32 (uint32x4_t __inactive, uint32x4_t __a, uint32x4_t __b, unsigned * __carry, mve_pred16_t __p)
 {
-  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | (*__carry << 29));
+  __builtin_arm_set_fpscr_nzcvqc((__builtin_arm_get_fpscr_nzcvqc () & ~0x20000000u) | ((*__carry & 0x1u) << 29));
   uint32x4_t __res = __builtin_mve_vsbcq_m_uv4si (__inactive, __a, __b, __p);
   *__carry = (__builtin_arm_get_fpscr_nzcvqc () >> 29) & 0x1u;
   return __res;
diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md
index 1d5721df3e8..860d734b9a1 100644
--- a/gcc/config/arm/mve.md
+++ b/gcc/config/arm/mve.md
@@ -9775,7 +9775,7 @@ (define_expand "mve_vldrdq_gather_base_nowb_z_<supf>v2di"
 
 (define_insn "get_fpscr_nzcvqc"
  [(set (match_operand:SI 0 "register_operand" "=r")
-   (unspec:SI [(reg:SI VFPCC_REGNUM)] UNSPEC_GET_FPSCR_NZCVQC))]
+   (unspec_volatile:SI [(reg:SI VFPCC_REGNUM)] UNSPEC_GET_FPSCR_NZCVQC))]
  "TARGET_HAVE_MVE"
  "vmrs\\t%0, FPSCR_nzcvqc"
   [(set_attr "type" "mve_move")])
diff --git a/gcc/testsuite/gcc.target/arm/mve/mve_vadcq_vsbcq_fpscr_overwrite.c b/gcc/testsuite/gcc.target/arm/mve/mve_vadcq_vsbcq_fpscr_overwrite.c
new file mode 100644
index 00000000000..a8c6cce67c8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/mve/mve_vadcq_vsbcq_fpscr_overwrite.c
@@ -0,0 +1,67 @@
+/* { dg-do run } */
+/* { dg-require-effective-target arm_mve_hw } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_v8_1m_mve } */
+
+#include <arm_mve.h>
+
+volatile int32x4_t c1;
+volatile uint32x4_t c2;
+int *carry;
+
+int
+main ()
+{
+  int32x4_t a1 = vcreateq_s32 (0, 0);
+  int32x4_t b1 = vcreateq_s32 (0, 0);
+  int32x4_t inactive1 = vcreateq_s32 (0, 0);
+
+  uint32x4_t a2 = vcreateq_u32 (0, 0);
+  uint32x4_t b2 = vcreateq_u32 (0, 0);
+  uint32x4_t inactive2 = vcreateq_u32 (0, 0);
+
+  mve_pred16_t p = 0xFFFF;
+  (*carry) = 0xFFFFFFFF;
+
+  __builtin_arm_set_fpscr_nzcvqc (0);
+  c1 = vadcq (a1, b1, carry);
+  if (__builtin_arm_get_fpscr_nzcvqc () & !0x20000000)
+    __builtin_abort ();
+  (*carry) = 0xFFFFFFFF;
+  __builtin_arm_set_fpscr_nzcvqc (0);
+  c2 = vadcq (a2, b2, carry);
+  if (__builtin_arm_get_fpscr_nzcvqc () & !0x20000000)
+    __builtin_abort ();
+  (*carry) = 0xFFFFFFFF;
+  __builtin_arm_set_fpscr_nzcvqc (0);
+  c1 = vsbcq (a1, b1, carry);
+  if (__builtin_arm_get_fpscr_nzcvqc () & !0x20000000)
+    __builtin_abort ();
+  (*carry) = 0xFFFFFFFF;
+  __builtin_arm_set_fpscr_nzcvqc (0);
+  c2 = vsbcq (a2, b2, carry);
+  if (__builtin_arm_get_fpscr_nzcvqc () & !0x20000000)
+    __builtin_abort ();
+  (*carry) = 0xFFFFFFFF;
+  __builtin_arm_set_fpscr_nzcvqc (0);
+  c1 = vadcq_m (inactive1, a1, b1, carry, p);
+  if (__builtin_arm_get_fpscr_nzcvqc () & !0x20000000)
+    __builtin_abort ();
+  (*carry) = 0xFFFFFFFF;
+  __builtin_arm_set_fpscr_nzcvqc (0);
+  c2 = vadcq_m (inactive2, a2, b2, carry, p);
+  if (__builtin_arm_get_fpscr_nzcvqc () & !0x20000000)
+    __builtin_abort ();
+  (*carry) = 0xFFFFFFFF;
+  __builtin_arm_set_fpscr_nzcvqc (0);
+  c1 = vsbcq_m (inactive1, a1, b1, carry, p);
+  if (__builtin_arm_get_fpscr_nzcvqc () & !0x20000000)
+    __builtin_abort ();
+  (*carry) = 0xFFFFFFFF;
+  __builtin_arm_set_fpscr_nzcvqc (0);
+  c2 = vsbcq_m (inactive2, a2, b2, carry, p);
+  if (__builtin_arm_get_fpscr_nzcvqc () & !0x20000000)
+    __builtin_abort ();
+
+  return 0;
+}
-- 
2.25.1


  parent reply	other threads:[~2023-05-18 10:54 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-18 10:53 [committed gcc12 backport] arm: Mve testsuite improvements Stam Markianos-Wright
2023-05-18 10:53 ` [committed gcc12 backport] arm: Fix vstrwq* backend + testsuite Stam Markianos-Wright
2023-05-18 10:53 ` [committed gcc12 backport] arm: Mve backend + testsuite fixes 2 Stam Markianos-Wright
2023-05-18 10:53 ` Stam Markianos-Wright [this message]
2023-05-18 10:53 ` [committed gcc12 backport] arm: Add vorrq_n overloading into vorrq _Generic Stam Markianos-Wright
2023-05-18 10:53 ` [committed gcc12 backport] arm: Fix overloading of MVE scalar constant parameters on vbicq, vmvnq_m Stam Markianos-Wright
2023-05-18 10:53 ` [committed gcc12 backport] arm: Fix MVE header pointer overloads this time (and a bit more tidying) Stam Markianos-Wright
2023-05-18 10:53 ` [committed gcc12 backport] arm testsuite: Remove reduntant tests Stam Markianos-Wright
2023-05-18 10:53 ` [committed gcc12 backport] arm testsuite: XFAIL or relax registers in some tests [PR109697] Stam Markianos-Wright
2023-05-18 10:53 ` [committed gcc12 backport] arm testsuite: Shifts and get_FPSCR ACLE optimisation fixes Stam Markianos-Wright
2023-05-18 10:53 ` [committed gcc12 backport] [arm] complete vmsr/vmrs blank and case adjustments Stam Markianos-Wright

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230518105331.1301864-4-stam.markianos-wright@arm.com \
    --to=stam.markianos-wright@arm.com \
    --cc=Richard.Earnshaw@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=kyrylo.tkachov@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).