public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][committed] aarch64: PR target/109855 Add predicate and constraints to define_subst in aarch64-simd.md
@ 2023-05-23 10:09 Kyrylo Tkachov
  0 siblings, 0 replies; only message in thread
From: Kyrylo Tkachov @ 2023-05-23 10:09 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 3111 bytes --]

Hi all,

In this PR we ICE because the substituted pattern for mla "lost" its predicate and constraint for operand 0
because the define_subst template:
  [(set (match_operand:<VDBL> 0)
        (vec_concat:<VDBL>
         (match_dup 1)
         (match_operand:VDZ 2 "aarch64_simd_or_scalar_imm_zero")))])

Uses match_operand instead of match_dup for operand 0. We can't use match_dup 0 for it because we need to specify the widened mode.
The problem is fixed by adding a "register_operand" predicate and "=w" constraint to the match_operand.
This makes sense conceptually too as the transformation we're targeting only applies to instructions that write a "w" register.
With this change the mddump pattern that ICEs goes from:
(define_insn ("aarch64_mlav4hi_vec_concatz_le")
     [
        (set (match_operand:V8HI 0 ("") ("")) <<------ Missing constraint!
            (vec_concat:V8HI (plus:V4HI (mult:V4HI (match_operand:V4HI 2 ("register_operand") ("w"))
                        (match_operand:V4HI 3 ("register_operand") ("w")))
                    (match_operand:V4HI 1 ("register_operand") ("0")))
                (match_operand:V4HI 4 ("aarch64_simd_or_scalar_imm_zero") (""))))
    ] ("(!BYTES_BIG_ENDIAN) && (TARGET_SIMD)") ("mla\t%0.4h, %2.4h, %3.4h")

to the proper:
(define_insn ("aarch64_mlav4hi_vec_concatz_le")
     [
        (set (match_operand:V8HI 0 ("register_operand") ("=w")) <<-------- Constraint in the right place
            (vec_concat:V8HI (plus:V4HI (mult:V4HI (match_operand:V4HI 2 ("register_operand") ("w"))
                        (match_operand:V4HI 3 ("register_operand") ("w")))
                    (match_operand:V4HI 1 ("register_operand") ("0")))
                (match_operand:V4HI 4 ("aarch64_simd_or_scalar_imm_zero") (""))))
    ] ("(!BYTES_BIG_ENDIAN) && (TARGET_SIMD)") ("mla\t%0.4h, %2.4h, %3.4h")

This seems to do the right thing for multi-alternative patterns as well, the annotated pattern for aarch64_cmltv8qi is:
(define_insn ("aarch64_cmltv8qi")
     [
        (set (match_operand:V8QI 0 ("register_operand") ("=w,w"))
            (neg:V8QI (lt:V8QI (match_operand:V8QI 1 ("register_operand") ("w,w"))
                    (match_operand:V8QI 2 ("aarch64_simd_reg_or_zero") ("w,ZDz")))))
    ]

whereas the substituted version now looks like:
(define_insn ("aarch64_cmltv8qi_vec_concatz_le")
     [
        (set (match_operand:V16QI 0 ("register_operand") ("=w,w"))
            (vec_concat:V16QI (neg:V8QI (lt:V8QI (match_operand:V8QI 1 ("register_operand") ("w,w"))
                        (match_operand:V8QI 2 ("aarch64_simd_reg_or_zero") ("w,ZDz"))))
                (match_operand:V8QI 3 ("aarch64_simd_or_scalar_imm_zero") (""))))
    ]

Bootstrapped and tested on aarch64-none-linux-gnu.
Pushing to trunk.
Thanks,
Kyrill

gcc/ChangeLog:

	PR target/109855
	* config/aarch64/aarch64-simd.md (add_vec_concat_subst_le): Add predicate
	and constraint for operand 0.
	(add_vec_concat_subst_be): Likewise.

gcc/testsuite/ChangeLog:

	PR target/109855
	* gcc.target/aarch64/pr109855.c: New test.

[-- Attachment #2: subst-pred.patch --]
[-- Type: application/octet-stream, Size: 1516 bytes --]

diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index fbf38d7dfe58f2ae48312f75133412cb0c7ca0fa..ec53127410f24d7b32907969acc1d2ded8e97720 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -26,7 +26,7 @@ (define_subst "add_vec_concat_subst_le"
   [(set (match_operand:VDZ 0)
         (match_operand:VDZ 1))]
   "!BYTES_BIG_ENDIAN"
-  [(set (match_operand:<VDBL> 0)
+  [(set (match_operand:<VDBL> 0 "register_operand" "=w")
         (vec_concat:<VDBL>
          (match_dup 1)
          (match_operand:VDZ 2 "aarch64_simd_or_scalar_imm_zero")))])
@@ -35,7 +35,7 @@ (define_subst "add_vec_concat_subst_be"
   [(set (match_operand:VDZ 0)
         (match_operand:VDZ 1))]
   "BYTES_BIG_ENDIAN"
-  [(set (match_operand:<VDBL> 0)
+  [(set (match_operand:<VDBL> 0 "register_operand" "=w")
         (vec_concat:<VDBL>
          (match_operand:VDZ 2 "aarch64_simd_or_scalar_imm_zero")
          (match_dup 1)))])
diff --git a/gcc/testsuite/gcc.target/aarch64/pr109855.c b/gcc/testsuite/gcc.target/aarch64/pr109855.c
new file mode 100644
index 0000000000000000000000000000000000000000..90db90c6cf9a86555cf9734c219d8d5b8229fa82
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr109855.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+typedef char __attribute__((__vector_size__ (4))) U;
+typedef short __attribute__((__vector_size__ (8))) V;
+
+U
+foo (V v, V w)
+{
+  return __builtin_convertvector (w * w + w, U);
+}
+

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-05-23 10:10 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-23 10:09 [PATCH][committed] aarch64: PR target/109855 Add predicate and constraints to define_subst in aarch64-simd.md Kyrylo Tkachov

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).