public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [AArch64] Fix predicate and constraint mismatch in logical atomic operations
@ 2014-09-25  3:45 Michael Collison
  2014-09-25  4:01 ` Andrew Pinski
  2014-11-04 10:44 ` Marcus Shawcroft
  0 siblings, 2 replies; 15+ messages in thread
From: Michael Collison @ 2014-09-25  3:45 UTC (permalink / raw)
  To: gcc-patches

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

On certain patterns in atomics.md the constraint 'n' is used in 
combination with the predicate atomic_op_operand. The constraint is too 
general and allows constants that are disallowed by the predicate. This 
causes an ICE In final_scan_insn when the insn cannot be split because 
the constraint and predicate do not match.

Tested on aarch64-none-elf, aarch64-linux-gnu. Additionally the 
originally reporter of the bug, (doko@ubuntu.com), applied the patch and 
successfully bootstrapped and tested with no regressions.

2014-09-23  Michael Collison <michael.collison@linaro.org>

     * config/aarch64/iterators.md (lconst_atomic): New mode attribute to
     support constraints for CONST_INT in atomic operations.
     * config/aarch64/atomics.md
     (atomic_<atomic_optab><mode>): Use lconst_atomic constraint.
     (atomic_nand<mode>): Likewise.
     (atomic_fetch_<atomic_optab><mode>): Likewise.
     (atomic_fetch_nand<mode>): Likewise.
     (atomic_<atomic_optab>_fetch<mode>): Likewise.
     (atomic_nand_fetch<mode>): Likewise.

-- 
Michael Collison
Linaro Toolchain Working Group
michael.collison@linaro.org


[-- Attachment #2: atomics-constraint.patch --]
[-- Type: text/x-patch, Size: 2859 bytes --]

--- ../../../../linaro-gcc4_9_git/gcc/config/aarch64/iterators.md	2014-09-22 10:10:04.520258964 -0700
+++ iterators.md	2014-09-16 14:27:10.459050672 -0700
@@ -349,6 +349,9 @@
 ;; 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")])
 
--- ../../../../linaro-gcc4_9_git/gcc/config/aarch64/atomics.md	2014-07-03 21:55:36.083476668 -0700
+++ atomics.md	2014-09-16 14:27:10.459050672 -0700
@@ -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>" "rn"))
+	(match_operand:ALLI 1 "<atomic_op_operand>" "r<lconst_atomic>"))
        (match_operand:SI 2 "const_int_operand")]		;; model
       UNSPECV_ATOMIC_OP))
        (clobber (reg:CC CC_REGNUM))
@@ -141,7 +141,7 @@
     (unspec_volatile:ALLI
       [(not:ALLI
 	(and:ALLI (match_dup 0)
-	  (match_operand:ALLI 1 "aarch64_logical_operand" "rn")))
+	  (match_operand:ALLI 1 "aarch64_logical_operand" "r<lconst_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>" "rn"))
+	(match_operand:ALLI 2 "<atomic_op_operand>" "r<lconst_atomic>"))
        (match_operand:SI 3 "const_int_operand")]		;; model
       UNSPECV_ATOMIC_OP))
    (clobber (reg:CC CC_REGNUM))
@@ -188,7 +188,7 @@
     (unspec_volatile:ALLI
       [(not:ALLI
 	 (and:ALLI (match_dup 1)
-	   (match_operand:ALLI 2 "aarch64_logical_operand" "rn")))
+	   (match_operand:ALLI 2 "aarch64_logical_operand" "r<lconst_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>" "rn")))
+      (match_operand:ALLI 2 "<atomic_op_operand>" "r<lconst_atomic>")))
    (set (match_dup 1)
     (unspec_volatile:ALLI
       [(match_dup 1) (match_dup 2)
@@ -233,7 +233,7 @@
     (not:ALLI
       (and:ALLI
 	(match_operand:ALLI 1 "aarch64_sync_memory_operand" "+Q")
-	(match_operand:ALLI 2 "aarch64_logical_operand" "rn"))))
+	(match_operand:ALLI 2 "aarch64_logical_operand" "r<lconst_atomic>"))))
    (set (match_dup 1)
     (unspec_volatile:ALLI
       [(match_dup 1) (match_dup 2)

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2014-09-25  3:45 [AArch64] Fix predicate and constraint mismatch in logical atomic operations Michael Collison
@ 2014-09-25  4:01 ` Andrew Pinski
  2014-09-25  4:08   ` Michael Collison
  2014-11-04 10:44 ` Marcus Shawcroft
  1 sibling, 1 reply; 15+ messages in thread
From: Andrew Pinski @ 2014-09-25  4:01 UTC (permalink / raw)
  To: Michael Collison; +Cc: GCC Patches

On Wed, Sep 24, 2014 at 8:45 PM, Michael Collison
<michael.collison@linaro.org> wrote:
> On certain patterns in atomics.md the constraint 'n' is used in combination
> with the predicate atomic_op_operand. The constraint is too general and
> allows constants that are disallowed by the predicate. This causes an ICE In
> final_scan_insn when the insn cannot be split because the constraint and
> predicate do not match.
>
> Tested on aarch64-none-elf, aarch64-linux-gnu. Additionally the originally
> reporter of the bug, (doko@ubuntu.com), applied the patch and successfully
> bootstrapped and tested with no regressions.

Testcase?

>
> 2014-09-23  Michael Collison <michael.collison@linaro.org>
>
>     * config/aarch64/iterators.md (lconst_atomic): New mode attribute to
>     support constraints for CONST_INT in atomic operations.
>     * config/aarch64/atomics.md
>     (atomic_<atomic_optab><mode>): Use lconst_atomic constraint.
>     (atomic_nand<mode>): Likewise.
>     (atomic_fetch_<atomic_optab><mode>): Likewise.
>     (atomic_fetch_nand<mode>): Likewise.
>     (atomic_<atomic_optab>_fetch<mode>): Likewise.
>     (atomic_nand_fetch<mode>): Likewise.
>
> --
> Michael Collison
> Linaro Toolchain Working Group
> michael.collison@linaro.org
>

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2014-09-25  4:01 ` Andrew Pinski
@ 2014-09-25  4:08   ` Michael Collison
  2014-09-25  4:10     ` Andrew Pinski
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Collison @ 2014-09-25  4:08 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: GCC Patches

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


Testcase attaached. Reproduce with "g++ -c -O2 -fPIE". URL for more 
detailed description:

https://bugs.linaro.org/show_bug.cgi?id=331


On 09/24/2014 09:01 PM, Andrew Pinski wrote:
> On Wed, Sep 24, 2014 at 8:45 PM, Michael Collison
> <michael.collison@linaro.org> wrote:
>> On certain patterns in atomics.md the constraint 'n' is used in combination
>> with the predicate atomic_op_operand. The constraint is too general and
>> allows constants that are disallowed by the predicate. This causes an ICE In
>> final_scan_insn when the insn cannot be split because the constraint and
>> predicate do not match.
>>
>> Tested on aarch64-none-elf, aarch64-linux-gnu. Additionally the originally
>> reporter of the bug, (doko@ubuntu.com), applied the patch and successfully
>> bootstrapped and tested with no regressions.
> Testcase?
>
>> 2014-09-23  Michael Collison <michael.collison@linaro.org>
>>
>>      * config/aarch64/iterators.md (lconst_atomic): New mode attribute to
>>      support constraints for CONST_INT in atomic operations.
>>      * config/aarch64/atomics.md
>>      (atomic_<atomic_optab><mode>): Use lconst_atomic constraint.
>>      (atomic_nand<mode>): Likewise.
>>      (atomic_fetch_<atomic_optab><mode>): Likewise.
>>      (atomic_fetch_nand<mode>): Likewise.
>>      (atomic_<atomic_optab>_fetch<mode>): Likewise.
>>      (atomic_nand_fetch<mode>): Likewise.
>>
>> --
>> Michael Collison
>> Linaro Toolchain Working Group
>> michael.collison@linaro.org
>>

-- 
Michael Collison
Linaro Toolchain Working Group
michael.collison@linaro.org


[-- Attachment #2: pfs_host.ii.xz --]
[-- Type: application/x-xz, Size: 62148 bytes --]

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2014-09-25  4:08   ` Michael Collison
@ 2014-09-25  4:10     ` Andrew Pinski
  2014-09-25  4:13       ` Michael Collison
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Pinski @ 2014-09-25  4:10 UTC (permalink / raw)
  To: Michael Collison; +Cc: GCC Patches

On Wed, Sep 24, 2014 at 9:08 PM, Michael Collison
<michael.collison@linaro.org> wrote:
>
> Testcase attaached. Reproduce with "g++ -c -O2 -fPIE". URL for more detailed
> description:
>
> https://bugs.linaro.org/show_bug.cgi?id=331

It would be a good idea to get a reduced testcase that you could add
to the GCC testsuite so this won't show up again.

Thanks,
Andrew Pinski

>
>
>
> On 09/24/2014 09:01 PM, Andrew Pinski wrote:
>>
>> On Wed, Sep 24, 2014 at 8:45 PM, Michael Collison
>> <michael.collison@linaro.org> wrote:
>>>
>>> On certain patterns in atomics.md the constraint 'n' is used in
>>> combination
>>> with the predicate atomic_op_operand. The constraint is too general and
>>> allows constants that are disallowed by the predicate. This causes an ICE
>>> In
>>> final_scan_insn when the insn cannot be split because the constraint and
>>> predicate do not match.
>>>
>>> Tested on aarch64-none-elf, aarch64-linux-gnu. Additionally the
>>> originally
>>> reporter of the bug, (doko@ubuntu.com), applied the patch and
>>> successfully
>>> bootstrapped and tested with no regressions.
>>
>> Testcase?
>>
>>> 2014-09-23  Michael Collison <michael.collison@linaro.org>
>>>
>>>      * config/aarch64/iterators.md (lconst_atomic): New mode attribute to
>>>      support constraints for CONST_INT in atomic operations.
>>>      * config/aarch64/atomics.md
>>>      (atomic_<atomic_optab><mode>): Use lconst_atomic constraint.
>>>      (atomic_nand<mode>): Likewise.
>>>      (atomic_fetch_<atomic_optab><mode>): Likewise.
>>>      (atomic_fetch_nand<mode>): Likewise.
>>>      (atomic_<atomic_optab>_fetch<mode>): Likewise.
>>>      (atomic_nand_fetch<mode>): Likewise.
>>>
>>> --
>>> Michael Collison
>>> Linaro Toolchain Working Group
>>> michael.collison@linaro.org
>>>
>
> --
> Michael Collison
> Linaro Toolchain Working Group
> michael.collison@linaro.org
>

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2014-09-25  4:10     ` Andrew Pinski
@ 2014-09-25  4:13       ` Michael Collison
  2014-09-25  4:17         ` Andrew Pinski
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Collison @ 2014-09-25  4:13 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: GCC Patches


I have that attached to the bug report at the URL provided. I will work 
on a testcase if you think it is warranted.

On 09/24/2014 09:10 PM, Andrew Pinski wrote:
> On Wed, Sep 24, 2014 at 9:08 PM, Michael Collison
> <michael.collison@linaro.org> wrote:
>> Testcase attaached. Reproduce with "g++ -c -O2 -fPIE". URL for more detailed
>> description:
>>
>> https://bugs.linaro.org/show_bug.cgi?id=331
> It would be a good idea to get a reduced testcase that you could add
> to the GCC testsuite so this won't show up again.
>
> Thanks,
> Andrew Pinski
>
>>
>>
>> On 09/24/2014 09:01 PM, Andrew Pinski wrote:
>>> On Wed, Sep 24, 2014 at 8:45 PM, Michael Collison
>>> <michael.collison@linaro.org> wrote:
>>>> On certain patterns in atomics.md the constraint 'n' is used in
>>>> combination
>>>> with the predicate atomic_op_operand. The constraint is too general and
>>>> allows constants that are disallowed by the predicate. This causes an ICE
>>>> In
>>>> final_scan_insn when the insn cannot be split because the constraint and
>>>> predicate do not match.
>>>>
>>>> Tested on aarch64-none-elf, aarch64-linux-gnu. Additionally the
>>>> originally
>>>> reporter of the bug, (doko@ubuntu.com), applied the patch and
>>>> successfully
>>>> bootstrapped and tested with no regressions.
>>> Testcase?
>>>
>>>> 2014-09-23  Michael Collison <michael.collison@linaro.org>
>>>>
>>>>       * config/aarch64/iterators.md (lconst_atomic): New mode attribute to
>>>>       support constraints for CONST_INT in atomic operations.
>>>>       * config/aarch64/atomics.md
>>>>       (atomic_<atomic_optab><mode>): Use lconst_atomic constraint.
>>>>       (atomic_nand<mode>): Likewise.
>>>>       (atomic_fetch_<atomic_optab><mode>): Likewise.
>>>>       (atomic_fetch_nand<mode>): Likewise.
>>>>       (atomic_<atomic_optab>_fetch<mode>): Likewise.
>>>>       (atomic_nand_fetch<mode>): Likewise.
>>>>
>>>> --
>>>> Michael Collison
>>>> Linaro Toolchain Working Group
>>>> michael.collison@linaro.org
>>>>
>> --
>> Michael Collison
>> Linaro Toolchain Working Group
>> michael.collison@linaro.org
>>

-- 
Michael Collison
Linaro Toolchain Working Group
michael.collison@linaro.org

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2014-09-25  4:13       ` Michael Collison
@ 2014-09-25  4:17         ` Andrew Pinski
  2014-09-25 10:12           ` Segher Boessenkool
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Pinski @ 2014-09-25  4:17 UTC (permalink / raw)
  To: Michael Collison; +Cc: GCC Patches

On Wed, Sep 24, 2014 at 9:13 PM, Michael Collison
<michael.collison@linaro.org> wrote:
>
> I have that attached to the bug report at the URL provided. I will work on a
> testcase if you think it is warranted.

Yes it is almost always warranted.

https://gcc.gnu.org/contribute.html#patches

Testcases   If you cannot follow the recommendations of the GCC coding
conventions about testcases, you should include a justification for
why adequate testcases cannot be added.

See the last part of that sentence.  You don't have any justification
on why you are not including testcases.

-- Pinski

>
>
> On 09/24/2014 09:10 PM, Andrew Pinski wrote:
>>
>> On Wed, Sep 24, 2014 at 9:08 PM, Michael Collison
>> <michael.collison@linaro.org> wrote:
>>>
>>> Testcase attaached. Reproduce with "g++ -c -O2 -fPIE". URL for more
>>> detailed
>>> description:
>>>
>>> https://bugs.linaro.org/show_bug.cgi?id=331
>>
>> It would be a good idea to get a reduced testcase that you could add
>> to the GCC testsuite so this won't show up again.
>>
>> Thanks,
>> Andrew Pinski
>>
>>>
>>>
>>> On 09/24/2014 09:01 PM, Andrew Pinski wrote:
>>>>
>>>> On Wed, Sep 24, 2014 at 8:45 PM, Michael Collison
>>>> <michael.collison@linaro.org> wrote:
>>>>>
>>>>> On certain patterns in atomics.md the constraint 'n' is used in
>>>>> combination
>>>>> with the predicate atomic_op_operand. The constraint is too general and
>>>>> allows constants that are disallowed by the predicate. This causes an
>>>>> ICE
>>>>> In
>>>>> final_scan_insn when the insn cannot be split because the constraint
>>>>> and
>>>>> predicate do not match.
>>>>>
>>>>> Tested on aarch64-none-elf, aarch64-linux-gnu. Additionally the
>>>>> originally
>>>>> reporter of the bug, (doko@ubuntu.com), applied the patch and
>>>>> successfully
>>>>> bootstrapped and tested with no regressions.
>>>>
>>>> Testcase?
>>>>
>>>>> 2014-09-23  Michael Collison <michael.collison@linaro.org>
>>>>>
>>>>>       * config/aarch64/iterators.md (lconst_atomic): New mode attribute
>>>>> to
>>>>>       support constraints for CONST_INT in atomic operations.
>>>>>       * config/aarch64/atomics.md
>>>>>       (atomic_<atomic_optab><mode>): Use lconst_atomic constraint.
>>>>>       (atomic_nand<mode>): Likewise.
>>>>>       (atomic_fetch_<atomic_optab><mode>): Likewise.
>>>>>       (atomic_fetch_nand<mode>): Likewise.
>>>>>       (atomic_<atomic_optab>_fetch<mode>): Likewise.
>>>>>       (atomic_nand_fetch<mode>): Likewise.
>>>>>
>>>>> --
>>>>> Michael Collison
>>>>> Linaro Toolchain Working Group
>>>>> michael.collison@linaro.org
>>>>>
>>> --
>>> Michael Collison
>>> Linaro Toolchain Working Group
>>> michael.collison@linaro.org
>>>
>
> --
> Michael Collison
> Linaro Toolchain Working Group
> michael.collison@linaro.org
>

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2014-09-25  4:17         ` Andrew Pinski
@ 2014-09-25 10:12           ` Segher Boessenkool
  2014-09-25 17:33             ` Michael Collison
  0 siblings, 1 reply; 15+ messages in thread
From: Segher Boessenkool @ 2014-09-25 10:12 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: Michael Collison, GCC Patches

On Wed, Sep 24, 2014 at 09:17:23PM -0700, Andrew Pinski wrote:
> On Wed, Sep 24, 2014 at 9:13 PM, Michael Collison
> <michael.collison@linaro.org> wrote:
> >
> > I have that attached to the bug report at the URL provided. I will work on a
> > testcase if you think it is warranted.
> 
> Yes it is almost always warranted.
> 
> https://gcc.gnu.org/contribute.html#patches
> 
> Testcases   If you cannot follow the recommendations of the GCC coding
> conventions about testcases, you should include a justification for
> why adequate testcases cannot be added.
> 
> See the last part of that sentence.  You don't have any justification
> on why you are not including testcases.

It is very hard to make a reliable testcase for such problems, because
they only happen when register allocation is under pressure.

The problem is not that "n" allows more than your predicate does.  The
predicate allows registers too, so the compiler happily made a register
contain some big const.  Now RA comes along, is out of registers but hey,
there is this "n", let's just put the big constant there!  Carnage.

So this is hard to test for; you can add some (big) code that exposed the
problem, but in a few months time that won't trigger the problem anymore
because earlier stages in the compiler will have generated slightly
different code.

It also does nothing to catch similar problems in other patterns.


Segher

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2014-09-25 10:12           ` Segher Boessenkool
@ 2014-09-25 17:33             ` Michael Collison
  2014-09-25 19:30               ` Segher Boessenkool
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Collison @ 2014-09-25 17:33 UTC (permalink / raw)
  To: Segher Boessenkool, Andrew Pinski; +Cc: GCC Patches

Segher,

The problem is the "CONST_INT 0", not a large constant. This constant is 
not accepted by the predicate, but is accepted by the constraint.

On 09/25/2014 03:12 AM, Segher Boessenkool wrote:
> On Wed, Sep 24, 2014 at 09:17:23PM -0700, Andrew Pinski wrote:
>> On Wed, Sep 24, 2014 at 9:13 PM, Michael Collison
>> <michael.collison@linaro.org> wrote:
>>> I have that attached to the bug report at the URL provided. I will work on a
>>> testcase if you think it is warranted.
>> Yes it is almost always warranted.
>>
>> https://gcc.gnu.org/contribute.html#patches
>>
>> Testcases   If you cannot follow the recommendations of the GCC coding
>> conventions about testcases, you should include a justification for
>> why adequate testcases cannot be added.
>>
>> See the last part of that sentence.  You don't have any justification
>> on why you are not including testcases.
> It is very hard to make a reliable testcase for such problems, because
> they only happen when register allocation is under pressure.
>
> The problem is not that "n" allows more than your predicate does.  The
> predicate allows registers too, so the compiler happily made a register
> contain some big const.  Now RA comes along, is out of registers but hey,
> there is this "n", let's just put the big constant there!  Carnage.
>
> So this is hard to test for; you can add some (big) code that exposed the
> problem, but in a few months time that won't trigger the problem anymore
> because earlier stages in the compiler will have generated slightly
> different code.
>
> It also does nothing to catch similar problems in other patterns.
>
>
> Segher

-- 
Michael Collison
Linaro Toolchain Working Group
michael.collison@linaro.org

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2014-09-25 17:33             ` Michael Collison
@ 2014-09-25 19:30               ` Segher Boessenkool
  2014-10-09 12:28                 ` Christophe Lyon
  0 siblings, 1 reply; 15+ messages in thread
From: Segher Boessenkool @ 2014-09-25 19:30 UTC (permalink / raw)
  To: Michael Collison; +Cc: Andrew Pinski, GCC Patches

On Thu, Sep 25, 2014 at 10:33:17AM -0700, Michael Collison wrote:
> The problem is the "CONST_INT 0", not a large constant. This constant is 
> not accepted by the predicate, but is accepted by the constraint.

Yes, bad choice of words, sorry.  Just read "big" as "not matching the
predicate".  The point is that everything works fine until RA, and that
makes it hard to make a useful test.


Segher


p.s.  Please don't top-post.  Thanks.

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2014-09-25 19:30               ` Segher Boessenkool
@ 2014-10-09 12:28                 ` Christophe Lyon
  0 siblings, 0 replies; 15+ messages in thread
From: Christophe Lyon @ 2014-10-09 12:28 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: Michael Collison, Andrew Pinski, GCC Patches

On 25 September 2014 21:30, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> On Thu, Sep 25, 2014 at 10:33:17AM -0700, Michael Collison wrote:
>> The problem is the "CONST_INT 0", not a large constant. This constant is
>> not accepted by the predicate, but is accepted by the constraint.
>
> Yes, bad choice of words, sorry.  Just read "big" as "not matching the
> predicate".  The point is that everything works fine until RA, and that
> makes it hard to make a useful test.
>

Hi,

IIUC, a testcase is not required/practicable.
So, may I ping this patch?
https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02209.html

Thanks,

Christophe.

>
> Segher
>
>
> p.s.  Please don't top-post.  Thanks.

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2014-09-25  3:45 [AArch64] Fix predicate and constraint mismatch in logical atomic operations Michael Collison
  2014-09-25  4:01 ` Andrew Pinski
@ 2014-11-04 10:44 ` Marcus Shawcroft
  2015-05-08 10:42   ` Richard Biener
  1 sibling, 1 reply; 15+ messages in thread
From: Marcus Shawcroft @ 2014-11-04 10:44 UTC (permalink / raw)
  To: Michael Collison; +Cc: gcc-patches

On 25 September 2014 04:45, Michael Collison
<michael.collison@linaro.org> wrote:
> On certain patterns in atomics.md the constraint 'n' is used in combination
> with the predicate atomic_op_operand. The constraint is too general and
> allows constants that are disallowed by the predicate. This causes an ICE In
> final_scan_insn when the insn cannot be split because the constraint and
> predicate do not match.
>
> Tested on aarch64-none-elf, aarch64-linux-gnu. Additionally the originally
> reporter of the bug, (doko@ubuntu.com), applied the patch and successfully
> bootstrapped and tested with no regressions.
>
> 2014-09-23  Michael Collison <michael.collison@linaro.org>
>
>     * config/aarch64/iterators.md (lconst_atomic): New mode attribute to
>     support constraints for CONST_INT in atomic operations.
>     * config/aarch64/atomics.md
>     (atomic_<atomic_optab><mode>): Use lconst_atomic constraint.
>     (atomic_nand<mode>): Likewise.
>     (atomic_fetch_<atomic_optab><mode>): Likewise.
>     (atomic_fetch_nand<mode>): Likewise.
>     (atomic_<atomic_optab>_fetch<mode>): Likewise.
>     (atomic_nand_fetch<mode>): Likewise.

OK Thanks.  /Marcus

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2014-11-04 10:44 ` Marcus Shawcroft
@ 2015-05-08 10:42   ` Richard Biener
  2015-06-15 12:20     ` Christophe Lyon
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Biener @ 2015-05-08 10:42 UTC (permalink / raw)
  To: Marcus Shawcroft; +Cc: Michael Collison, gcc-patches

On Tue, Nov 4, 2014 at 11:44 AM, Marcus Shawcroft
<marcus.shawcroft@gmail.com> wrote:
> On 25 September 2014 04:45, Michael Collison
> <michael.collison@linaro.org> wrote:
>> On certain patterns in atomics.md the constraint 'n' is used in combination
>> with the predicate atomic_op_operand. The constraint is too general and
>> allows constants that are disallowed by the predicate. This causes an ICE In
>> final_scan_insn when the insn cannot be split because the constraint and
>> predicate do not match.
>>
>> Tested on aarch64-none-elf, aarch64-linux-gnu. Additionally the originally
>> reporter of the bug, (doko@ubuntu.com), applied the patch and successfully
>> bootstrapped and tested with no regressions.
>>
>> 2014-09-23  Michael Collison <michael.collison@linaro.org>
>>
>>     * config/aarch64/iterators.md (lconst_atomic): New mode attribute to
>>     support constraints for CONST_INT in atomic operations.
>>     * config/aarch64/atomics.md
>>     (atomic_<atomic_optab><mode>): Use lconst_atomic constraint.
>>     (atomic_nand<mode>): Likewise.
>>     (atomic_fetch_<atomic_optab><mode>): Likewise.
>>     (atomic_fetch_nand<mode>): Likewise.
>>     (atomic_<atomic_optab>_fetch<mode>): Likewise.
>>     (atomic_nand_fetch<mode>): Likewise.
>
> OK Thanks.  /Marcus

Can you please backport this to all release branches as well?

Thanks,
Richard.

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2015-05-08 10:42   ` Richard Biener
@ 2015-06-15 12:20     ` Christophe Lyon
  2015-06-16  8:36       ` Christophe Lyon
  0 siblings, 1 reply; 15+ messages in thread
From: Christophe Lyon @ 2015-06-15 12:20 UTC (permalink / raw)
  To: Richard Biener; +Cc: Marcus Shawcroft, Michael Collison, gcc-patches

On 8 May 2015 at 12:42, Richard Biener <richard.guenther@gmail.com> wrote:
> On Tue, Nov 4, 2014 at 11:44 AM, Marcus Shawcroft
> <marcus.shawcroft@gmail.com> wrote:
>> On 25 September 2014 04:45, Michael Collison
>> <michael.collison@linaro.org> wrote:
>>> On certain patterns in atomics.md the constraint 'n' is used in combination
>>> with the predicate atomic_op_operand. The constraint is too general and
>>> allows constants that are disallowed by the predicate. This causes an ICE In
>>> final_scan_insn when the insn cannot be split because the constraint and
>>> predicate do not match.
>>>
>>> Tested on aarch64-none-elf, aarch64-linux-gnu. Additionally the originally
>>> reporter of the bug, (doko@ubuntu.com), applied the patch and successfully
>>> bootstrapped and tested with no regressions.
>>>
>>> 2014-09-23  Michael Collison <michael.collison@linaro.org>
>>>
>>>     * config/aarch64/iterators.md (lconst_atomic): New mode attribute to
>>>     support constraints for CONST_INT in atomic operations.
>>>     * config/aarch64/atomics.md
>>>     (atomic_<atomic_optab><mode>): Use lconst_atomic constraint.
>>>     (atomic_nand<mode>): Likewise.
>>>     (atomic_fetch_<atomic_optab><mode>): Likewise.
>>>     (atomic_fetch_nand<mode>): Likewise.
>>>     (atomic_<atomic_optab>_fetch<mode>): Likewise.
>>>     (atomic_nand_fetch<mode>): Likewise.
>>
>> OK Thanks.  /Marcus
>
> Can you please backport this to all release branches as well?
>

Hi Richard,

I have tested this backport against 4.8 and 4.9 branches.
I applies cleanly in both cases, shows no regression and fixes the ICE.

I'm afraid it's too late for committing into the 4.8 branch?

Sorry for the delay in handling this.

Christophe.

> Thanks,
> Richard.

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2015-06-15 12:20     ` Christophe Lyon
@ 2015-06-16  8:36       ` Christophe Lyon
  2015-06-16 10:31         ` Christophe Lyon
  0 siblings, 1 reply; 15+ messages in thread
From: Christophe Lyon @ 2015-06-16  8:36 UTC (permalink / raw)
  To: gcc-patches

On 15 June 2015 at 14:16, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> On 8 May 2015 at 12:42, Richard Biener <richard.guenther@gmail.com> wrote:
>> On Tue, Nov 4, 2014 at 11:44 AM, Marcus Shawcroft
>> <marcus.shawcroft@gmail.com> wrote:
>>> On 25 September 2014 04:45, Michael Collison
>>> <michael.collison@linaro.org> wrote:
>>>> On certain patterns in atomics.md the constraint 'n' is used in combination
>>>> with the predicate atomic_op_operand. The constraint is too general and
>>>> allows constants that are disallowed by the predicate. This causes an ICE In
>>>> final_scan_insn when the insn cannot be split because the constraint and
>>>> predicate do not match.
>>>>
>>>> Tested on aarch64-none-elf, aarch64-linux-gnu. Additionally the originally
>>>> reporter of the bug, (doko@ubuntu.com), applied the patch and successfully
>>>> bootstrapped and tested with no regressions.
>>>>
>>>> 2014-09-23  Michael Collison <michael.collison@linaro.org>
>>>>
>>>>     * config/aarch64/iterators.md (lconst_atomic): New mode attribute to
>>>>     support constraints for CONST_INT in atomic operations.
>>>>     * config/aarch64/atomics.md
>>>>     (atomic_<atomic_optab><mode>): Use lconst_atomic constraint.
>>>>     (atomic_nand<mode>): Likewise.
>>>>     (atomic_fetch_<atomic_optab><mode>): Likewise.
>>>>     (atomic_fetch_nand<mode>): Likewise.
>>>>     (atomic_<atomic_optab>_fetch<mode>): Likewise.
>>>>     (atomic_nand_fetch<mode>): Likewise.
>>>
>>> OK Thanks.  /Marcus
>>
>> Can you please backport this to all release branches as well?
>>
>
> Hi Richard,
>
> I have tested this backport against 4.8 and 4.9 branches.
> I applies cleanly in both cases, shows no regression and fixes the ICE.
>
> I'm afraid it's too late for committing into the 4.8 branch?
>
> Sorry for the delay in handling this.
>
> Christophe.
>

For the record, I have committed this backport as r224503. in the 4.9-branch.

I will commit it in the 4.8-branch when Richard confirms it's OK.


>> Thanks,
>> Richard.

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

* Re: [AArch64] Fix predicate and constraint mismatch in logical atomic operations
  2015-06-16  8:36       ` Christophe Lyon
@ 2015-06-16 10:31         ` Christophe Lyon
  0 siblings, 0 replies; 15+ messages in thread
From: Christophe Lyon @ 2015-06-16 10:31 UTC (permalink / raw)
  To: gcc-patches

On 16 June 2015 at 10:20, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> On 15 June 2015 at 14:16, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>> On 8 May 2015 at 12:42, Richard Biener <richard.guenther@gmail.com> wrote:
>>> On Tue, Nov 4, 2014 at 11:44 AM, Marcus Shawcroft
>>> <marcus.shawcroft@gmail.com> wrote:
>>>> On 25 September 2014 04:45, Michael Collison
>>>> <michael.collison@linaro.org> wrote:
>>>>> On certain patterns in atomics.md the constraint 'n' is used in combination
>>>>> with the predicate atomic_op_operand. The constraint is too general and
>>>>> allows constants that are disallowed by the predicate. This causes an ICE In
>>>>> final_scan_insn when the insn cannot be split because the constraint and
>>>>> predicate do not match.
>>>>>
>>>>> Tested on aarch64-none-elf, aarch64-linux-gnu. Additionally the originally
>>>>> reporter of the bug, (doko@ubuntu.com), applied the patch and successfully
>>>>> bootstrapped and tested with no regressions.
>>>>>
>>>>> 2014-09-23  Michael Collison <michael.collison@linaro.org>
>>>>>
>>>>>     * config/aarch64/iterators.md (lconst_atomic): New mode attribute to
>>>>>     support constraints for CONST_INT in atomic operations.
>>>>>     * config/aarch64/atomics.md
>>>>>     (atomic_<atomic_optab><mode>): Use lconst_atomic constraint.
>>>>>     (atomic_nand<mode>): Likewise.
>>>>>     (atomic_fetch_<atomic_optab><mode>): Likewise.
>>>>>     (atomic_fetch_nand<mode>): Likewise.
>>>>>     (atomic_<atomic_optab>_fetch<mode>): Likewise.
>>>>>     (atomic_nand_fetch<mode>): Likewise.
>>>>
>>>> OK Thanks.  /Marcus
>>>
>>> Can you please backport this to all release branches as well?
>>>
>>
>> Hi Richard,
>>
>> I have tested this backport against 4.8 and 4.9 branches.
>> I applies cleanly in both cases, shows no regression and fixes the ICE.
>>
>> I'm afraid it's too late for committing into the 4.8 branch?
>>
>> Sorry for the delay in handling this.
>>
>> Christophe.
>>
>
> For the record, I have committed this backport as r224503. in the 4.9-branch.
>
> I will commit it in the 4.8-branch when Richard confirms it's OK.
>
>
Now committed in gcc-4.8-branch as r224510 after Richard confirmed on IRC.

Christophe.

>>> Thanks,
>>> Richard.

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

end of thread, other threads:[~2015-06-16 10:29 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-25  3:45 [AArch64] Fix predicate and constraint mismatch in logical atomic operations Michael Collison
2014-09-25  4:01 ` Andrew Pinski
2014-09-25  4:08   ` Michael Collison
2014-09-25  4:10     ` Andrew Pinski
2014-09-25  4:13       ` Michael Collison
2014-09-25  4:17         ` Andrew Pinski
2014-09-25 10:12           ` Segher Boessenkool
2014-09-25 17:33             ` Michael Collison
2014-09-25 19:30               ` Segher Boessenkool
2014-10-09 12:28                 ` Christophe Lyon
2014-11-04 10:44 ` Marcus Shawcroft
2015-05-08 10:42   ` Richard Biener
2015-06-15 12:20     ` Christophe Lyon
2015-06-16  8:36       ` Christophe Lyon
2015-06-16 10:31         ` Christophe Lyon

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