| Message ID | 20130121105515.GH7269@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
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
--- 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)); +}