public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] aarch64: Fix up aarch64_compare_and_swaphi pattern [PR94368]
@ 2020-03-31  6:55 Jakub Jelinek
  2020-03-31  9:04 ` Richard Sandiford
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2020-03-31  6:55 UTC (permalink / raw)
  To: Richard Earnshaw, Richard Sandiford, Kyrylo Tkachov
  Cc: gcc-patches, Richard Henderson

Hi!

The following testcase ICEs in final_scan_insn_1.  The problem is in the
@aarch64_compare_and_swaphi define_insn_and_split, since 9 it uses
aarch64_plushi_operand predicate for the "expected value" operand, which
allows either 0..0xfff constants or 0x1000..0xf000 constants (i.e. HImode
values which when zero extended are either 0..0xfff or (0..0xfff) << 12).
The problem is that RA doesn't care about predicates, it honors just
constraints and the used constraint on the operand is n, which means any
HImode CONST_SCALAR_INT.  In the testcase LRA thus propagates the -1
value into the insn.
This is a define_insn_and_split which requires mandatory split.
But during split2 pass, we check the predicate (and don't check
constraints), which fails and thus we don't split it and during final ICE
because the mandatory splitting didn't happen.

The following patch fixes it by adding a matching constraint to the
predicate and using it.

Bootstrapped/regtested on aarch64-linux, ok for trunk?

2020-03-31  Jakub Jelinek  <jakub@redhat.com>

	PR target/94368
	* config/aarch64/constraints.md (Uph): New constraint.
	* config/aarch64/atomics.md (cas_short_expected_imm): New mode attr.
	(@aarch64_compare_and_swap<mode>): Use it instead of n in operand 2's
	constraint.

	* gcc.dg/pr94368.c: New test.

--- gcc/config/aarch64/constraints.md.jj	2020-01-17 19:35:54.101144322 +0100
+++ gcc/config/aarch64/constraints.md	2020-03-30 11:39:29.554441681 +0200
@@ -233,6 +233,13 @@ (define_constraint "Up3"
   (and (match_code "const_int")
        (match_test "(unsigned) exact_log2 (ival) <= 4")))
 
+(define_constraint "Uph"
+  "@internal
+  A constraint that matches HImode integers zero extendable to
+  SImode plus_operand."
+  (and (match_code "const_int")
+       (match_test "aarch64_plushi_immediate (op, VOIDmode)")))
+
 (define_memory_constraint "Q"
  "A memory address which uses a single base register with no offset."
  (and (match_code "mem")
--- gcc/config/aarch64/atomics.md.jj	2020-01-17 14:46:41.464792827 +0100
+++ gcc/config/aarch64/atomics.md	2020-03-30 11:42:02.256180919 +0200
@@ -38,6 +38,8 @@ (define_expand "@atomic_compare_and_swap
 
 (define_mode_attr cas_short_expected_pred
   [(QI "aarch64_reg_or_imm") (HI "aarch64_plushi_operand")])
+(define_mode_attr cas_short_expected_imm
+  [(QI "n") (HI "Uph")])
 
 (define_insn_and_split "@aarch64_compare_and_swap<mode>"
   [(set (reg:CC CC_REGNUM)					;; bool out
@@ -47,7 +49,8 @@ (define_insn_and_split "@aarch64_compare
       (match_operand:SHORT 1 "aarch64_sync_memory_operand" "+Q"))) ;; memory
    (set (match_dup 1)
     (unspec_volatile:SHORT
-      [(match_operand:SHORT 2 "<cas_short_expected_pred>" "rn")	;; expected
+      [(match_operand:SHORT 2 "<cas_short_expected_pred>"
+			      "r<cas_short_expected_imm>")	;; expected
        (match_operand:SHORT 3 "aarch64_reg_or_zero" "rZ")	;; desired
        (match_operand:SI 4 "const_int_operand")			;; is_weak
        (match_operand:SI 5 "const_int_operand")			;; mod_s
--- gcc/testsuite/gcc.dg/pr94368.c.jj	2020-03-30 11:47:18.573497812 +0200
+++ gcc/testsuite/gcc.dg/pr94368.c	2020-03-30 11:25:54.002642049 +0200
@@ -0,0 +1,25 @@
+/* PR target/94368 */
+/* { dg-do compile { target fpic } } */
+/* { dg-options "-fpic -O1 -fcommon" } */
+
+int b, c, d, e, f, h;
+short g;
+int foo (int) __attribute__ ((__const__));
+
+void
+bar (void)
+{
+  while (1)
+    {
+      while (1)
+	{
+	  __atomic_load_n (&e, 0);
+	  if (foo (2))
+	    __sync_val_compare_and_swap (&c, 0, f);
+	  b = 1;
+	  if (h == e)
+	    break;
+	}
+      __sync_val_compare_and_swap (&g, -1, f);
+    }
+}

	Jakub


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

* Re: [PATCH] aarch64: Fix up aarch64_compare_and_swaphi pattern [PR94368]
  2020-03-31  6:55 [PATCH] aarch64: Fix up aarch64_compare_and_swaphi pattern [PR94368] Jakub Jelinek
@ 2020-03-31  9:04 ` Richard Sandiford
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Sandiford @ 2020-03-31  9:04 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Richard Earnshaw, Kyrylo Tkachov, gcc-patches, Richard Henderson

Jakub Jelinek <jakub@redhat.com> writes:
> Hi!
>
> The following testcase ICEs in final_scan_insn_1.  The problem is in the
> @aarch64_compare_and_swaphi define_insn_and_split, since 9 it uses
> aarch64_plushi_operand predicate for the "expected value" operand, which
> allows either 0..0xfff constants or 0x1000..0xf000 constants (i.e. HImode
> values which when zero extended are either 0..0xfff or (0..0xfff) << 12).
> The problem is that RA doesn't care about predicates, it honors just
> constraints and the used constraint on the operand is n, which means any
> HImode CONST_SCALAR_INT.  In the testcase LRA thus propagates the -1
> value into the insn.
> This is a define_insn_and_split which requires mandatory split.
> But during split2 pass, we check the predicate (and don't check
> constraints), which fails and thus we don't split it and during final ICE
> because the mandatory splitting didn't happen.
>
> The following patch fixes it by adding a matching constraint to the
> predicate and using it.
>
> Bootstrapped/regtested on aarch64-linux, ok for trunk?

OK, thanks.

Richard

> 2020-03-31  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR target/94368
> 	* config/aarch64/constraints.md (Uph): New constraint.
> 	* config/aarch64/atomics.md (cas_short_expected_imm): New mode attr.
> 	(@aarch64_compare_and_swap<mode>): Use it instead of n in operand 2's
> 	constraint.
>
> 	* gcc.dg/pr94368.c: New test.
>
> --- gcc/config/aarch64/constraints.md.jj	2020-01-17 19:35:54.101144322 +0100
> +++ gcc/config/aarch64/constraints.md	2020-03-30 11:39:29.554441681 +0200
> @@ -233,6 +233,13 @@ (define_constraint "Up3"
>    (and (match_code "const_int")
>         (match_test "(unsigned) exact_log2 (ival) <= 4")))
>  
> +(define_constraint "Uph"
> +  "@internal
> +  A constraint that matches HImode integers zero extendable to
> +  SImode plus_operand."
> +  (and (match_code "const_int")
> +       (match_test "aarch64_plushi_immediate (op, VOIDmode)")))
> +
>  (define_memory_constraint "Q"
>   "A memory address which uses a single base register with no offset."
>   (and (match_code "mem")
> --- gcc/config/aarch64/atomics.md.jj	2020-01-17 14:46:41.464792827 +0100
> +++ gcc/config/aarch64/atomics.md	2020-03-30 11:42:02.256180919 +0200
> @@ -38,6 +38,8 @@ (define_expand "@atomic_compare_and_swap
>  
>  (define_mode_attr cas_short_expected_pred
>    [(QI "aarch64_reg_or_imm") (HI "aarch64_plushi_operand")])
> +(define_mode_attr cas_short_expected_imm
> +  [(QI "n") (HI "Uph")])
>  
>  (define_insn_and_split "@aarch64_compare_and_swap<mode>"
>    [(set (reg:CC CC_REGNUM)					;; bool out
> @@ -47,7 +49,8 @@ (define_insn_and_split "@aarch64_compare
>        (match_operand:SHORT 1 "aarch64_sync_memory_operand" "+Q"))) ;; memory
>     (set (match_dup 1)
>      (unspec_volatile:SHORT
> -      [(match_operand:SHORT 2 "<cas_short_expected_pred>" "rn")	;; expected
> +      [(match_operand:SHORT 2 "<cas_short_expected_pred>"
> +			      "r<cas_short_expected_imm>")	;; expected
>         (match_operand:SHORT 3 "aarch64_reg_or_zero" "rZ")	;; desired
>         (match_operand:SI 4 "const_int_operand")			;; is_weak
>         (match_operand:SI 5 "const_int_operand")			;; mod_s
> --- gcc/testsuite/gcc.dg/pr94368.c.jj	2020-03-30 11:47:18.573497812 +0200
> +++ gcc/testsuite/gcc.dg/pr94368.c	2020-03-30 11:25:54.002642049 +0200
> @@ -0,0 +1,25 @@
> +/* PR target/94368 */
> +/* { dg-do compile { target fpic } } */
> +/* { dg-options "-fpic -O1 -fcommon" } */
> +
> +int b, c, d, e, f, h;
> +short g;
> +int foo (int) __attribute__ ((__const__));
> +
> +void
> +bar (void)
> +{
> +  while (1)
> +    {
> +      while (1)
> +	{
> +	  __atomic_load_n (&e, 0);
> +	  if (foo (2))
> +	    __sync_val_compare_and_swap (&c, 0, f);
> +	  b = 1;
> +	  if (h == e)
> +	    break;
> +	}
> +      __sync_val_compare_and_swap (&g, -1, f);
> +    }
> +}
>
> 	Jakub

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

end of thread, other threads:[~2020-03-31  9:04 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-31  6:55 [PATCH] aarch64: Fix up aarch64_compare_and_swaphi pattern [PR94368] Jakub Jelinek
2020-03-31  9:04 ` Richard Sandiford

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