* [PATCH] Fix up ARM ICE (PR target/49069)
@ 2013-01-21 10:55 Jakub Jelinek
2013-01-22 10:38 ` Ramana Radhakrishnan
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2013-01-21 10:55 UTC (permalink / raw)
To: Ramana Radhakrishnan, Nick Clifton, Richard Earnshaw; +Cc: gcc-patches
Hi!
As can be seen on the testcase, this backend bug is still reproduceable even
on trunk, the backend just can't rely on cstoredi4 or cbranchdi4 expansion
not being performed with two constants, unless it has predicates that
disallow it (Steven's patch in the PR).
This patch just forces it into registers instead, it will be simplified by
RTL optimizers anyway later on (or another alternative is FAIL there in that
case, see my other patch in the PR).
I really don't care which way this is fixed, but having such ICE around for
so long when the fix is so easy is unnecessary.
Tested just on the testcase, given that previously we'd always ICE on it,
it can't make things worse.
Ok for trunk?
2013-01-21 Jakub Jelinek <jakub@redhat.com>
PR target/49069
* config/arm/arm.md (cbranchdi4, cstoredi4): Don't ICE if
both comparison operands are constants.
* gcc.dg/pr49069.c: New test.
--- gcc/config/arm/arm.md.jj 2013-01-11 09:03:13.000000000 +0100
+++ gcc/config/arm/arm.md 2013-01-17 16:57:58.246233079 +0100
@@ -7035,9 +7035,10 @@ (define_expand "cbranchdi4"
(pc)))]
"TARGET_32BIT"
"{
- /* We should not have two constants. */
- gcc_assert (GET_MODE (operands[1]) == DImode
- || GET_MODE (operands[2]) == DImode);
+ /* If we have two constants, force one into register. */
+ if (GET_MODE (operands[1]) != DImode
+ && GET_MODE (operands[2]) != DImode)
+ operands[1] = force_reg (DImode, operands[1]);
if (!arm_validize_comparison (&operands[0], &operands[1], &operands[2]))
FAIL;
@@ -7958,9 +7959,10 @@ (define_expand "cstoredi4"
(match_operand:DI 3 "cmpdi_operand" "")]))]
"TARGET_32BIT"
"{
- /* We should not have two constants. */
- gcc_assert (GET_MODE (operands[2]) == DImode
- || GET_MODE (operands[3]) == DImode);
+ /* If we have two constants, force one into register. */
+ if (GET_MODE (operands[2]) != DImode
+ && GET_MODE (operands[3]) != DImode)
+ operands[2] = force_reg (DImode, operands[2]);
if (!arm_validize_comparison (&operands[1],
&operands[2],
--- gcc/testsuite/gcc.dg/pr49069.c.jj 2012-11-17 15:43:17.572007394 +0100
+++ gcc/testsuite/gcc.dg/pr49069.c 2013-01-17 16:43:41.613146835 +0100
@@ -0,0 +1,15 @@
+/* PR target/49069 */
+/* { dg-do compile } */
+/* { dg-options "-Os -fno-tree-forwprop -Wno-div-by-zero" } */
+
+int a;
+const unsigned long long b[1] = { 1ULL };
+extern void bar (int);
+
+void
+foo (void)
+{
+ for (a = 0; a == 1; a = 2)
+ ;
+ bar (b[0] == (a == 0 ? a : a / 0));
+}
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix up ARM ICE (PR target/49069)
2013-01-21 10:55 [PATCH] Fix up ARM ICE (PR target/49069) Jakub Jelinek
@ 2013-01-22 10:38 ` Ramana Radhakrishnan
2013-01-23 8:42 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Ramana Radhakrishnan @ 2013-01-22 10:38 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: nickc, Richard Earnshaw, gcc-patches
On 01/21/13 10:55, Jakub Jelinek wrote:
> Hi!
>
> As can be seen on the testcase, this backend bug is still reproduceable even
> on trunk, the backend just can't rely on cstoredi4 or cbranchdi4 expansion
> not being performed with two constants, unless it has predicates that
> disallow it (Steven's patch in the PR).
> This patch just forces it into registers instead, it will be simplified by
> RTL optimizers anyway later on (or another alternative is FAIL there in that
> case, see my other patch in the PR).
> I really don't care which way this is fixed, but having such ICE around for
> so long when the fix is so easy is unnecessary.
> Tested just on the testcase, given that previously we'd always ICE on it,
> it can't make things worse.
>
> Ok for trunk?
>
> 2013-01-21 Jakub Jelinek <jakub@redhat.com>
>
> PR target/49069
> * config/arm/arm.md (cbranchdi4, cstoredi4): Don't ICE if
> both comparison operands are constants.
>
> * gcc.dg/pr49069.c: New test.
>
> --- gcc/config/arm/arm.md.jj 2013-01-11 09:03:13.000000000 +0100
> +++ gcc/config/arm/arm.md 2013-01-17 16:57:58.246233079 +0100
> @@ -7035,9 +7035,10 @@ (define_expand "cbranchdi4"
> (pc)))]
> "TARGET_32BIT"
> "{
> - /* We should not have two constants. */
> - gcc_assert (GET_MODE (operands[1]) == DImode
> - || GET_MODE (operands[2]) == DImode);
> + /* If we have two constants, force one into register. */
> + if (GET_MODE (operands[1]) != DImode
> + && GET_MODE (operands[2]) != DImode)
> + operands[1] = force_reg (DImode, operands[1]);
I don't know where we've got to with respect to providing CONST_INTs
modes these days but given that's on the cards I'd rather not have such
mechanisms for detecting both operands being const_ints here .
Instead I'd just use s_register_operand for operand1 and continue to use
cmpdi_operand for operand2 and fix it so.
>
> if (!arm_validize_comparison (&operands[0], &operands[1], &operands[2]))
> FAIL;
> @@ -7958,9 +7959,10 @@ (define_expand "cstoredi4"
> (match_operand:DI 3 "cmpdi_operand" "")]))]
> "TARGET_32BIT"
> "{
> - /* We should not have two constants. */
> - gcc_assert (GET_MODE (operands[2]) == DImode
> - || GET_MODE (operands[3]) == DImode);
> + /* If we have two constants, force one into register. */
> + if (GET_MODE (operands[2]) != DImode
> + && GET_MODE (operands[3]) != DImode)
> + operands[2] = force_reg (DImode, operands[2]);
And likewise .
Ok with those changes and if no regressions.
regards
Ramana
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix up ARM ICE (PR target/49069)
2013-01-22 10:38 ` Ramana Radhakrishnan
@ 2013-01-23 8:42 ` Jakub Jelinek
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2013-01-23 8:42 UTC (permalink / raw)
To: Ramana Radhakrishnan; +Cc: nickc, Richard Earnshaw, gcc-patches
On Tue, Jan 22, 2013 at 10:38:24AM +0000, Ramana Radhakrishnan wrote:
> Instead I'd just use s_register_operand for operand1 and continue to
> use cmpdi_operand for operand2 and fix it so.
>
> And likewise .
>
> Ok with those changes and if no regressions.
Managed to bootstrap/regtest this (via scratch rpm builds) on
armv7hl-linux-gnueabi, here is what I've committed:
2013-01-23 Jakub Jelinek <jakub@redhat.com>
PR target/49069
* config/arm/arm.md (cbranchdi4, cstoredi4): Use s_register_operand
instead of cmpdi_operand for first comparison operand.
Don't assert that comparison operands aren't both constants.
* gcc.dg/pr49069.c: New test.
--- gcc/config/arm/arm.md.jj 2013-01-18 17:59:43.765932181 +0100
+++ gcc/config/arm/arm.md 2013-01-22 12:07:13.572331618 +0100
@@ -7030,16 +7030,12 @@
(define_expand "cbranchdi4"
[(set (pc) (if_then_else
(match_operator 0 "expandable_comparison_operator"
- [(match_operand:DI 1 "cmpdi_operand" "")
+ [(match_operand:DI 1 "s_register_operand" "")
(match_operand:DI 2 "cmpdi_operand" "")])
(label_ref (match_operand 3 "" ""))
(pc)))]
"TARGET_32BIT"
"{
- /* We should not have two constants. */
- gcc_assert (GET_MODE (operands[1]) == DImode
- || GET_MODE (operands[2]) == DImode);
-
if (!arm_validize_comparison (&operands[0], &operands[1], &operands[2]))
FAIL;
emit_jump_insn (gen_cbranch_cc (operands[0], operands[1], operands[2],
@@ -7955,14 +7951,10 @@
(define_expand "cstoredi4"
[(set (match_operand:SI 0 "s_register_operand" "")
(match_operator:SI 1 "expandable_comparison_operator"
- [(match_operand:DI 2 "cmpdi_operand" "")
+ [(match_operand:DI 2 "s_register_operand" "")
(match_operand:DI 3 "cmpdi_operand" "")]))]
"TARGET_32BIT"
"{
- /* We should not have two constants. */
- gcc_assert (GET_MODE (operands[2]) == DImode
- || GET_MODE (operands[3]) == DImode);
-
if (!arm_validize_comparison (&operands[1],
&operands[2],
&operands[3]))
--- gcc/testsuite/gcc.dg/pr49069.c.jj 2012-11-17 15:43:17.572007394 +0100
+++ gcc/testsuite/gcc.dg/pr49069.c 2013-01-17 16:43:41.613146835 +0100
@@ -0,0 +1,15 @@
+/* PR target/49069 */
+/* { dg-do compile } */
+/* { dg-options "-Os -fno-tree-forwprop -Wno-div-by-zero" } */
+
+int a;
+const unsigned long long b[1] = { 1ULL };
+extern void bar (int);
+
+void
+foo (void)
+{
+ for (a = 0; a == 1; a = 2)
+ ;
+ bar (b[0] == (a == 0 ? a : a / 0));
+}
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-01-23 8:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-21 10:55 [PATCH] Fix up ARM ICE (PR target/49069) Jakub Jelinek
2013-01-22 10:38 ` Ramana Radhakrishnan
2013-01-23 8:42 ` Jakub Jelinek
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).