public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Aarch64][target/PR 67143] Use correct constraints on operands for atomic operations.
@ 2015-08-11 13:05 Matthew Wahab
  2015-08-11 13:30 ` Matthew Wahab
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Matthew Wahab @ 2015-08-11 13:05 UTC (permalink / raw)
  To: gcc-patches

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

The Aarch64 backend implements the atomic operations on memory with the same
patterns iterated over logical and arithmetic operation. It also specifies
constraints on an operand for the operations but the constraints used are only
valid for the logical operations. This can lead to an ICE, with the compiler
unable to generate a valid instruction.

This patch reworks the atomic operation patterns to select the
appropriate constraint for the operation. The logical operations take
the constraints specified by the current lconst_atomic mode iterator
while the arithmetic operations (plus, sub) take constraint "I".

This patch also adds the test-case provided by Matthia Klose for the bugzilla
entry https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67143, slightly modified to
compile as plain C.

Tested for arm-none-eabi with cross-compiled check-gcc and for
arm-none-linux-gnueabihf with native bootstrap and make check.

Ok for trunk?
Matthew

gcc/
2015-08-10  Matthew Wahab  <matthew.wahab@arm.com>

	PR target/67143
	* config/aarch64/atomic.md (atomic_<optab><mode>): Replace
	'lconst_atomic' with 'const_atomic'.
	(atomic_fetch_<optab><mode>): Likewise.
	(atomic_<optab>_fetch<mode>): Likewise.
	* config/aarch64/iterators.md (lconst-atomic): Move below
	'const_atomic'.
	(const_atomic): New.

gcc/testsuite/
2015-08-10  Matthew Wahab  <matthew.wahab@arm.com>
	    Matthias Klose  <doko@debian.org>

	PR target/67143
	* gcc.target/aarch64/pr67143.c: New

[-- Attachment #2: atomic_consts.patch --]
[-- Type: text/x-patch, Size: 3471 bytes --]

diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index 1a38ac0..6e6be99 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -119,7 +119,7 @@
   [(set (match_operand:ALLI 0 "aarch64_sync_memory_operand" "+Q")
     (unspec_volatile:ALLI
       [(atomic_op:ALLI (match_dup 0)
-	(match_operand:ALLI 1 "<atomic_op_operand>" "r<lconst_atomic>"))
+	(match_operand:ALLI 1 "<atomic_op_operand>" "r<const_atomic>"))
        (match_operand:SI 2 "const_int_operand")]		;; model
       UNSPECV_ATOMIC_OP))
        (clobber (reg:CC CC_REGNUM))
@@ -164,7 +164,7 @@
    (set (match_dup 1)
     (unspec_volatile:ALLI
       [(atomic_op:ALLI (match_dup 1)
-	(match_operand:ALLI 2 "<atomic_op_operand>" "r<lconst_atomic>"))
+	(match_operand:ALLI 2 "<atomic_op_operand>" "r<const_atomic>"))
        (match_operand:SI 3 "const_int_operand")]		;; model
       UNSPECV_ATOMIC_OP))
    (clobber (reg:CC CC_REGNUM))
@@ -209,7 +209,7 @@
   [(set (match_operand:ALLI 0 "register_operand" "=&r")
     (atomic_op:ALLI
       (match_operand:ALLI 1 "aarch64_sync_memory_operand" "+Q")
-      (match_operand:ALLI 2 "<atomic_op_operand>" "r<lconst_atomic>")))
+      (match_operand:ALLI 2 "<atomic_op_operand>" "r<const_atomic>")))
    (set (match_dup 1)
     (unspec_volatile:ALLI
       [(match_dup 1) (match_dup 2)
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index 5d7966d..d3d6af7 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -345,9 +345,6 @@
 ;; Attribute to describe constants acceptable in logical operations
 (define_mode_attr lconst [(SI "K") (DI "L")])
 
-;; Attribute to describe constants acceptable in atomic logical operations
-(define_mode_attr lconst_atomic [(QI "K") (HI "K") (SI "K") (DI "L")])
-
 ;; Map a mode to a specific constraint character.
 (define_mode_attr cmode [(QI "q") (HI "h") (SI "s") (DI "d")])
 
@@ -843,6 +840,16 @@
    (plus "aarch64_plus_operand")
    (minus "aarch64_plus_operand")])
 
+;; Constants acceptable for atomic operations.
+;; This definition must appear in this file before the iterators it refers to.
+(define_code_attr const_atomic
+ [(plus "I") (minus "I")
+  (xor "<lconst_atomic>") (ior "<lconst_atomic>")
+  (and "<lconst_atomic>")])
+
+;; Attribute to describe constants acceptable in atomic logical operations
+(define_mode_attr lconst_atomic [(QI "K") (HI "K") (SI "K") (DI "L")])
+
 ;; -------------------------------------------------------------------
 ;; Int Iterators.
 ;; -------------------------------------------------------------------
diff --git a/gcc/testsuite/gcc.target/aarch64/pr67143.c b/gcc/testsuite/gcc.target/aarch64/pr67143.c
new file mode 100644
index 0000000..9335b33
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr67143.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+long a, c;
+int b;
+int d;
+void ut_dbg_assertion_failed() __attribute__((noreturn));
+long dict_index_is_spatial(int *);
+void btr_block_get_func(char *);
+long btr_page_get_level_low(unsigned char *);
+void btr_validate_level(long p1) {
+  unsigned char *e;
+  while (p1 != btr_page_get_level_low(e)) {
+    if (__builtin_expect(b, 0))
+      ut_dbg_assertion_failed();
+    if (dict_index_is_spatial(&d))
+      while (c != 5535) {
+        __sync_add_and_fetch(&a, 536870912);
+        btr_block_get_func("");
+      }
+  }
+  for (long i; i; ++i)
+    btr_validate_level(-i);
+}

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

* Re: [Aarch64][target/PR 67143] Use correct constraints on operands for atomic operations.
  2015-08-11 13:05 [Aarch64][target/PR 67143] Use correct constraints on operands for atomic operations Matthew Wahab
@ 2015-08-11 13:30 ` Matthew Wahab
  2015-08-11 14:07 ` James Greenhalgh
  2015-08-12 21:26 ` Joseph Myers
  2 siblings, 0 replies; 7+ messages in thread
From: Matthew Wahab @ 2015-08-11 13:30 UTC (permalink / raw)
  To: gcc-patches

On 11/08/15 14:05, Matthew Wahab wrote:

> Tested for arm-none-eabi with cross-compiled check-gcc and for
> arm-none-linux-gnueabihf with native bootstrap and make check.

This should have said: Tested for aarch64-none-eabi with cross-compiled check-gcc and 
for aarch64-none-linux-gnu with native bootstrap and make check.

Matthew

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

* Re: [Aarch64][target/PR 67143] Use correct constraints on operands for atomic operations.
  2015-08-11 13:05 [Aarch64][target/PR 67143] Use correct constraints on operands for atomic operations Matthew Wahab
  2015-08-11 13:30 ` Matthew Wahab
@ 2015-08-11 14:07 ` James Greenhalgh
  2015-08-12 13:41   ` Matthew Wahab
  2015-08-12 21:26 ` Joseph Myers
  2 siblings, 1 reply; 7+ messages in thread
From: James Greenhalgh @ 2015-08-11 14:07 UTC (permalink / raw)
  To: Matthew Wahab; +Cc: gcc-patches

On Tue, Aug 11, 2015 at 02:05:37PM +0100, Matthew Wahab wrote:
> The Aarch64 backend implements the atomic operations on memory with the same
> patterns iterated over logical and arithmetic operation. It also specifies
> constraints on an operand for the operations but the constraints used are only
> valid for the logical operations. This can lead to an ICE, with the compiler
> unable to generate a valid instruction.
> 
> This patch reworks the atomic operation patterns to select the
> appropriate constraint for the operation. The logical operations take
> the constraints specified by the current lconst_atomic mode iterator
> while the arithmetic operations (plus, sub) take constraint "I".
> 
> This patch also adds the test-case provided by Matthia Klose for the bugzilla
> entry https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67143, slightly modified to
> compile as plain C.
> 
> Tested for arm-none-eabi with cross-compiled check-gcc and for
> arm-none-linux-gnueabihf with native bootstrap and make check.
> 
> Ok for trunk?
>
> Matthew
> gcc/
> 2015-08-10  Matthew Wahab  <matthew.wahab@arm.com>
> 
> 	PR target/67143
> 	* config/aarch64/atomic.md (atomic_<optab><mode>): Replace
> 	'lconst_atomic' with 'const_atomic'.
> 	(atomic_fetch_<optab><mode>): Likewise.
> 	(atomic_<optab>_fetch<mode>): Likewise.
> 	* config/aarch64/iterators.md (lconst-atomic): Move below
> 	'const_atomic'.
> 	(const_atomic): New.
> 
> gcc/testsuite/
> 2015-08-10  Matthew Wahab  <matthew.wahab@arm.com>
> 	    Matthias Klose  <doko@debian.org>
> 
> 	PR target/67143
> 	* gcc.target/aarch64/pr67143.c: New

> diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
> index 5d7966d..d3d6af7 100644
> --- a/gcc/config/aarch64/iterators.md
> +++ b/gcc/config/aarch64/iterators.md
> @@ -345,9 +345,6 @@
>  ;; Attribute to describe constants acceptable in logical operations
>  (define_mode_attr lconst [(SI "K") (DI "L")])
>  
> -;; Attribute to describe constants acceptable in atomic logical operations
> -(define_mode_attr lconst_atomic [(QI "K") (HI "K") (SI "K") (DI "L")])
> -
>  ;; Map a mode to a specific constraint character.
>  (define_mode_attr cmode [(QI "q") (HI "h") (SI "s") (DI "d")])
>  
> @@ -843,6 +840,16 @@
>     (plus "aarch64_plus_operand")
>     (minus "aarch64_plus_operand")])
>  
> +;; Constants acceptable for atomic operations.
> +;; This definition must appear in this file before the iterators it refers to.
> +(define_code_attr const_atomic
> + [(plus "I") (minus "I")

I'm worried this still gives us a mismatch between constraints and
predicates. The relevant predicates here are:

  (define_predicate "aarch64_plus_operand"
    (ior (match_operand 0 "register_operand")
         (match_operand 0 "aarch64_plus_immediate")))

  (define_predicate "aarch64_plus_immediate"
    (and (match_code "const_int")
         (ior (match_test "aarch64_uimm12_shift (INTVAL (op))")
	      (match_test "aarch64_uimm12_shift (-INTVAL (op))"))))

But our constraint only permits:

  (define_constraint "I"
   "A constant that can be used with an ADD operation."
   (and (match_code "const_int")
        (match_test "aarch64_uimm12_shift (ival)")))

Does this mean we are now loading constants we don't need to in to
registers? I don't think we could cause this to ICE - but are we
generating less good code than we would like?

Thanks,
James

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

* Re: [Aarch64][target/PR 67143] Use correct constraints on operands for atomic operations.
  2015-08-11 14:07 ` James Greenhalgh
@ 2015-08-12 13:41   ` Matthew Wahab
  0 siblings, 0 replies; 7+ messages in thread
From: Matthew Wahab @ 2015-08-12 13:41 UTC (permalink / raw)
  To: James Greenhalgh; +Cc: gcc-patches

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

On 11/08/15 15:07, James Greenhalgh wrote:
> On Tue, Aug 11, 2015 at 02:05:37PM +0100, Matthew Wahab wrote:
>>
>> This patch reworks the atomic operation patterns to select the
>> appropriate constraint for the operation. The logical operations take
>> the constraints specified by the current lconst_atomic mode iterator
>> while the arithmetic operations (plus, sub) take constraint "I".
>
> I'm worried this still gives us a mismatch between constraints and
> predicates. The relevant predicates here are:
>
>    (define_predicate "aarch64_plus_operand"
>      (ior (match_operand 0 "register_operand")
>           (match_operand 0 "aarch64_plus_immediate")))
>
>    (define_predicate "aarch64_plus_immediate"
>      (and (match_code "const_int")
>           (ior (match_test "aarch64_uimm12_shift (INTVAL (op))")
> 	      (match_test "aarch64_uimm12_shift (-INTVAL (op))"))))
>
> But our constraint only permits:
>
>    (define_constraint "I"
>     "A constant that can be used with an ADD operation."
>     (and (match_code "const_int")
>          (match_test "aarch64_uimm12_shift (ival)")))
>
> Does this mean we are now loading constants we don't need to in to
> registers? I don't think we could cause this to ICE - but are we
> generating less good code than we would like?

Updated the patch to make the constraints for the arithmetic operations "IJ",
which preserves the existing behaviour. Also added two cases to the
gcc.target/aarch64/atomic-op-imm.c test to check the behaviour with large
nagative numbers.

Tested for aarch64-none-eabi with cross-compiled check-gcc and for
aarch64-none-linux-gnu with native bootstrap and make check.

Ok?
Matthew

gcc/
2015-08-12  Matthew Wahab  <matthew.wahab@arm.com>

	PR target/67143
	* config/aarch64/atomic.md (atomic_<optab><mode>): Replace
	'lconst_atomic' with 'const_atomic'.
	(atomic_fetch_<optab><mode>): Likewise.
	(atomic_<optab>_fetch<mode>): Likewise.
	* config/aarch64/iterators.md (lconst-atomic): Move below
	'const_atomic'.
	(const_atomic): New.

gcc/testsuite/
2015-08-12  Matthew Wahab  <matthew.wahab@arm.com>
	    Matthias Klose  <doko@debian.org>

	PR target/67143
	* gcc.target/aarch64/atomic-op-imm.c
	(atomic_fetch_add_negative_RELAXED): New.
	(atomic_fetch_sub_negative_ACQUIRE): New.
	* gcc.target/aarch64/pr67143.c: New


[-- Attachment #2: atomic_consts_v2.patch --]
[-- Type: text/x-patch, Size: 4366 bytes --]

diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index 1a38ac0..6e6be99 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -119,7 +119,7 @@
   [(set (match_operand:ALLI 0 "aarch64_sync_memory_operand" "+Q")
     (unspec_volatile:ALLI
       [(atomic_op:ALLI (match_dup 0)
-	(match_operand:ALLI 1 "<atomic_op_operand>" "r<lconst_atomic>"))
+	(match_operand:ALLI 1 "<atomic_op_operand>" "r<const_atomic>"))
        (match_operand:SI 2 "const_int_operand")]		;; model
       UNSPECV_ATOMIC_OP))
        (clobber (reg:CC CC_REGNUM))
@@ -164,7 +164,7 @@
    (set (match_dup 1)
     (unspec_volatile:ALLI
       [(atomic_op:ALLI (match_dup 1)
-	(match_operand:ALLI 2 "<atomic_op_operand>" "r<lconst_atomic>"))
+	(match_operand:ALLI 2 "<atomic_op_operand>" "r<const_atomic>"))
        (match_operand:SI 3 "const_int_operand")]		;; model
       UNSPECV_ATOMIC_OP))
    (clobber (reg:CC CC_REGNUM))
@@ -209,7 +209,7 @@
   [(set (match_operand:ALLI 0 "register_operand" "=&r")
     (atomic_op:ALLI
       (match_operand:ALLI 1 "aarch64_sync_memory_operand" "+Q")
-      (match_operand:ALLI 2 "<atomic_op_operand>" "r<lconst_atomic>")))
+      (match_operand:ALLI 2 "<atomic_op_operand>" "r<const_atomic>")))
    (set (match_dup 1)
     (unspec_volatile:ALLI
       [(match_dup 1) (match_dup 2)
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index 5d7966d..b8a45d1 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -345,9 +345,6 @@
 ;; Attribute to describe constants acceptable in logical operations
 (define_mode_attr lconst [(SI "K") (DI "L")])
 
-;; Attribute to describe constants acceptable in atomic logical operations
-(define_mode_attr lconst_atomic [(QI "K") (HI "K") (SI "K") (DI "L")])
-
 ;; Map a mode to a specific constraint character.
 (define_mode_attr cmode [(QI "q") (HI "h") (SI "s") (DI "d")])
 
@@ -843,6 +840,16 @@
    (plus "aarch64_plus_operand")
    (minus "aarch64_plus_operand")])
 
+;; Constants acceptable for atomic operations.
+;; This definition must appear in this file before the iterators it refers to.
+(define_code_attr const_atomic
+ [(plus "IJ") (minus "IJ")
+  (xor "<lconst_atomic>") (ior "<lconst_atomic>")
+  (and "<lconst_atomic>")])
+
+;; Attribute to describe constants acceptable in atomic logical operations
+(define_mode_attr lconst_atomic [(QI "K") (HI "K") (SI "K") (DI "L")])
+
 ;; -------------------------------------------------------------------
 ;; Int Iterators.
 ;; -------------------------------------------------------------------
diff --git a/gcc/testsuite/gcc.target/aarch64/atomic-op-imm.c b/gcc/testsuite/gcc.target/aarch64/atomic-op-imm.c
index 6c6f7e1..47d7a96 100644
--- a/gcc/testsuite/gcc.target/aarch64/atomic-op-imm.c
+++ b/gcc/testsuite/gcc.target/aarch64/atomic-op-imm.c
@@ -16,6 +16,18 @@ atomic_fetch_sub_ACQUIRE ()
 }
 
 int
+atomic_fetch_add_negative_RELAXED ()
+{
+  return __atomic_fetch_add (&v, -4096, __ATOMIC_RELAXED);
+}
+
+int
+atomic_fetch_sub_negative_ACQUIRE ()
+{
+  return __atomic_fetch_sub (&v, -4096, __ATOMIC_ACQUIRE);
+}
+
+int
 atomic_fetch_and_SEQ_CST ()
 {
   return __atomic_fetch_and (&v, 4096, __ATOMIC_SEQ_CST);
@@ -75,4 +87,4 @@ atomic_or_fetch_CONSUME ()
   return __atomic_or_fetch (&v, 4096, __ATOMIC_CONSUME);
 }
 
-/* { dg-final { scan-assembler-times "\tw\[0-9\]+, w\[0-9\]+, #*4096" 12 } } */
+/* { dg-final { scan-assembler-times "\tw\[0-9\]+, w\[0-9\]+, #*4096" 14 } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr67143.c b/gcc/testsuite/gcc.target/aarch64/pr67143.c
new file mode 100644
index 0000000..9335b33
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr67143.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+long a, c;
+int b;
+int d;
+void ut_dbg_assertion_failed() __attribute__((noreturn));
+long dict_index_is_spatial(int *);
+void btr_block_get_func(char *);
+long btr_page_get_level_low(unsigned char *);
+void btr_validate_level(long p1) {
+  unsigned char *e;
+  while (p1 != btr_page_get_level_low(e)) {
+    if (__builtin_expect(b, 0))
+      ut_dbg_assertion_failed();
+    if (dict_index_is_spatial(&d))
+      while (c != 5535) {
+        __sync_add_and_fetch(&a, 536870912);
+        btr_block_get_func("");
+      }
+  }
+  for (long i; i; ++i)
+    btr_validate_level(-i);
+}

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

* Re: [Aarch64][target/PR 67143] Use correct constraints on operands for atomic operations.
  2015-08-11 13:05 [Aarch64][target/PR 67143] Use correct constraints on operands for atomic operations Matthew Wahab
  2015-08-11 13:30 ` Matthew Wahab
  2015-08-11 14:07 ` James Greenhalgh
@ 2015-08-12 21:26 ` Joseph Myers
  2015-08-13 14:49   ` Matthew Wahab
  2 siblings, 1 reply; 7+ messages in thread
From: Joseph Myers @ 2015-08-12 21:26 UTC (permalink / raw)
  To: Matthew Wahab; +Cc: gcc-patches

On Tue, 11 Aug 2015, Matthew Wahab wrote:

> 	PR target/67143
> 	* gcc.target/aarch64/pr67143.c: New

What's architecture-specific about this test?  That is, why doesn't it 
just go in gcc.c-torture/compile (no dg- directives needed, automatically 
looped over optimization options)?

Architecture-specific test directories should only be for tests that are 
actually architecture-specific (e.g. using architecture-specific options, 
or asm, or built-in functions, or scan-assembler for particular output), 
not for tests that can be built generically for all architectures and 
still test for the original bug.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [Aarch64][target/PR 67143] Use correct constraints on operands for atomic operations.
  2015-08-12 21:26 ` Joseph Myers
@ 2015-08-13 14:49   ` Matthew Wahab
  2015-08-14 13:29     ` James Greenhalgh
  0 siblings, 1 reply; 7+ messages in thread
From: Matthew Wahab @ 2015-08-13 14:49 UTC (permalink / raw)
  To: gcc-patches; +Cc: Joseph Myers

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

On 12/08/15 22:26, Joseph Myers wrote:
> On Tue, 11 Aug 2015, Matthew Wahab wrote:
>
>> 	PR target/67143
>> 	* gcc.target/aarch64/pr67143.c: New
>
> What's architecture-specific about this test?  That is, why doesn't it
> just go in gcc.c-torture/compile (no dg- directives needed, automatically
> looped over optimization options)?
>

Attached is an updated patch which puts the pr67143.c test case in
gcc.c-torture/compile and drops the dg- directives.

Tested for aarch64-none-eabi with cross-compiled check-gcc. Also ran
check-gcc for x86_64-none-linux-gnu, to check the new test case.

Ok?
Matthew

gcc/
2015-08-13  Matthew Wahab  <matthew.wahab@arm.com>

	PR target/67143
	* config/aarch64/atomics.md (atomic_<optab><mode>): Replace
	'lconst_atomic' with 'const_atomic'.
	(atomic_fetch_<optab><mode>): Likewise.
	(atomic_<optab>_fetch<mode>): Likewise.
	* config/aarch64/iterators.md (lconst-atomic): Move below
	'const_atomic'.
	(const_atomic): New.

gcc/testsuite/
2015-08-13  Matthew Wahab  <matthew.wahab@arm.com>
	    Matthias Klose  <doko@debian.org>

	PR target/67143
	* gcc.c-torture/compile/pr67143.c: New
	* gcc.target/aarch64/atomic-op-imm.c
	(atomic_fetch_add_negative_RELAXED): New.
	(atomic_fetch_sub_negative_ACQUIRE): New.


[-- Attachment #2: atomic_consts_v3.patch --]
[-- Type: text/x-patch, Size: 4320 bytes --]

diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index 1a38ac0..6e6be99 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -119,7 +119,7 @@
   [(set (match_operand:ALLI 0 "aarch64_sync_memory_operand" "+Q")
     (unspec_volatile:ALLI
       [(atomic_op:ALLI (match_dup 0)
-	(match_operand:ALLI 1 "<atomic_op_operand>" "r<lconst_atomic>"))
+	(match_operand:ALLI 1 "<atomic_op_operand>" "r<const_atomic>"))
        (match_operand:SI 2 "const_int_operand")]		;; model
       UNSPECV_ATOMIC_OP))
        (clobber (reg:CC CC_REGNUM))
@@ -164,7 +164,7 @@
    (set (match_dup 1)
     (unspec_volatile:ALLI
       [(atomic_op:ALLI (match_dup 1)
-	(match_operand:ALLI 2 "<atomic_op_operand>" "r<lconst_atomic>"))
+	(match_operand:ALLI 2 "<atomic_op_operand>" "r<const_atomic>"))
        (match_operand:SI 3 "const_int_operand")]		;; model
       UNSPECV_ATOMIC_OP))
    (clobber (reg:CC CC_REGNUM))
@@ -209,7 +209,7 @@
   [(set (match_operand:ALLI 0 "register_operand" "=&r")
     (atomic_op:ALLI
       (match_operand:ALLI 1 "aarch64_sync_memory_operand" "+Q")
-      (match_operand:ALLI 2 "<atomic_op_operand>" "r<lconst_atomic>")))
+      (match_operand:ALLI 2 "<atomic_op_operand>" "r<const_atomic>")))
    (set (match_dup 1)
     (unspec_volatile:ALLI
       [(match_dup 1) (match_dup 2)
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index 5d7966d..b8a45d1 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -345,9 +345,6 @@
 ;; Attribute to describe constants acceptable in logical operations
 (define_mode_attr lconst [(SI "K") (DI "L")])
 
-;; Attribute to describe constants acceptable in atomic logical operations
-(define_mode_attr lconst_atomic [(QI "K") (HI "K") (SI "K") (DI "L")])
-
 ;; Map a mode to a specific constraint character.
 (define_mode_attr cmode [(QI "q") (HI "h") (SI "s") (DI "d")])
 
@@ -843,6 +840,16 @@
    (plus "aarch64_plus_operand")
    (minus "aarch64_plus_operand")])
 
+;; Constants acceptable for atomic operations.
+;; This definition must appear in this file before the iterators it refers to.
+(define_code_attr const_atomic
+ [(plus "IJ") (minus "IJ")
+  (xor "<lconst_atomic>") (ior "<lconst_atomic>")
+  (and "<lconst_atomic>")])
+
+;; Attribute to describe constants acceptable in atomic logical operations
+(define_mode_attr lconst_atomic [(QI "K") (HI "K") (SI "K") (DI "L")])
+
 ;; -------------------------------------------------------------------
 ;; Int Iterators.
 ;; -------------------------------------------------------------------
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr67143.c b/gcc/testsuite/gcc.c-torture/compile/pr67143.c
new file mode 100644
index 0000000..62c4186
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr67143.c
@@ -0,0 +1,21 @@
+long a, c;
+int b;
+int d;
+void ut_dbg_assertion_failed() __attribute__((noreturn));
+long dict_index_is_spatial(int *);
+void btr_block_get_func(char *);
+long btr_page_get_level_low(unsigned char *);
+void btr_validate_level(long p1) {
+  unsigned char *e;
+  while (p1 != btr_page_get_level_low(e)) {
+    if (__builtin_expect(b, 0))
+      ut_dbg_assertion_failed();
+    if (dict_index_is_spatial(&d))
+      while (c != 5535) {
+        __sync_add_and_fetch(&a, 536870912);
+        btr_block_get_func("");
+      }
+  }
+  for (long i; i; ++i)
+    btr_validate_level(-i);
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/atomic-op-imm.c b/gcc/testsuite/gcc.target/aarch64/atomic-op-imm.c
index 6c6f7e1..47d7a96 100644
--- a/gcc/testsuite/gcc.target/aarch64/atomic-op-imm.c
+++ b/gcc/testsuite/gcc.target/aarch64/atomic-op-imm.c
@@ -16,6 +16,18 @@ atomic_fetch_sub_ACQUIRE ()
 }
 
 int
+atomic_fetch_add_negative_RELAXED ()
+{
+  return __atomic_fetch_add (&v, -4096, __ATOMIC_RELAXED);
+}
+
+int
+atomic_fetch_sub_negative_ACQUIRE ()
+{
+  return __atomic_fetch_sub (&v, -4096, __ATOMIC_ACQUIRE);
+}
+
+int
 atomic_fetch_and_SEQ_CST ()
 {
   return __atomic_fetch_and (&v, 4096, __ATOMIC_SEQ_CST);
@@ -75,4 +87,4 @@ atomic_or_fetch_CONSUME ()
   return __atomic_or_fetch (&v, 4096, __ATOMIC_CONSUME);
 }
 
-/* { dg-final { scan-assembler-times "\tw\[0-9\]+, w\[0-9\]+, #*4096" 12 } } */
+/* { dg-final { scan-assembler-times "\tw\[0-9\]+, w\[0-9\]+, #*4096" 14 } } */

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

* Re: [Aarch64][target/PR 67143] Use correct constraints on operands for atomic operations.
  2015-08-13 14:49   ` Matthew Wahab
@ 2015-08-14 13:29     ` James Greenhalgh
  0 siblings, 0 replies; 7+ messages in thread
From: James Greenhalgh @ 2015-08-14 13:29 UTC (permalink / raw)
  To: Matthew Wahab; +Cc: gcc-patches, Joseph Myers

On Thu, Aug 13, 2015 at 03:48:22PM +0100, Matthew Wahab wrote:
> On 12/08/15 22:26, Joseph Myers wrote:
> > On Tue, 11 Aug 2015, Matthew Wahab wrote:
> >
> >> 	PR target/67143
> >> 	* gcc.target/aarch64/pr67143.c: New
> >
> > What's architecture-specific about this test?  That is, why doesn't it
> > just go in gcc.c-torture/compile (no dg- directives needed, automatically
> > looped over optimization options)?
> >
> 
> Attached is an updated patch which puts the pr67143.c test case in
> gcc.c-torture/compile and drops the dg- directives.
> 
> Tested for aarch64-none-eabi with cross-compiled check-gcc. Also ran
> check-gcc for x86_64-none-linux-gnu, to check the new test case.
> 
> Ok?
> Matthew

OK, and please backport it to the 5.x (and 4.9.x if needed) branches.

Thanks,
James

> gcc/
> 2015-08-13  Matthew Wahab  <matthew.wahab@arm.com>
> 
> 	PR target/67143
> 	* config/aarch64/atomics.md (atomic_<optab><mode>): Replace
> 	'lconst_atomic' with 'const_atomic'.
> 	(atomic_fetch_<optab><mode>): Likewise.
> 	(atomic_<optab>_fetch<mode>): Likewise.
> 	* config/aarch64/iterators.md (lconst-atomic): Move below
> 	'const_atomic'.
> 	(const_atomic): New.
> 
> gcc/testsuite/
> 2015-08-13  Matthew Wahab  <matthew.wahab@arm.com>
> 	    Matthias Klose  <doko@debian.org>
> 
> 	PR target/67143
> 	* gcc.c-torture/compile/pr67143.c: New
> 	* gcc.target/aarch64/atomic-op-imm.c
> 	(atomic_fetch_add_negative_RELAXED): New.
> 	(atomic_fetch_sub_negative_ACQUIRE): New.
> 

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

end of thread, other threads:[~2015-08-14 13:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-11 13:05 [Aarch64][target/PR 67143] Use correct constraints on operands for atomic operations Matthew Wahab
2015-08-11 13:30 ` Matthew Wahab
2015-08-11 14:07 ` James Greenhalgh
2015-08-12 13:41   ` Matthew Wahab
2015-08-12 21:26 ` Joseph Myers
2015-08-13 14:49   ` Matthew Wahab
2015-08-14 13:29     ` James Greenhalgh

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