[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu dot
                   |                            |org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
           Keywords|                            |wrong-code
   Last reconfirmed|0000-00-00 00:00:00         |2008-01-01 17:27:11
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[steven at gcc dot gnu dot org]

------- Comment #6 from steven at gcc dot gnu dot org  2008-02-03 11:39 -------
According to comment #5, the loop2 pass creates the RTL that exposes the
problem in combine.  I don't believe that this is really true.  There is no
loop in the test case, so the loop2 pass should be a NOP.  At least, on the
trunk it should be.

Unfortunately it is not clear what compiler Dave used when he identified loop2
as the problem-maker.  GCC 4.2 and GCC 4.3 are very different here.  What might
be happening, is that going into and out of cfglayout mode in loop2 in GCC 4.2
makes it look like loop2 messes up things.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[steven at gcc dot gnu dot org]

------- Comment #7 from steven at gcc dot gnu dot org  2008-02-03 12:05 -------
The trunk also inlines ret1() at -O2, and therefore does not fail.


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|http://bugs.gentoo.org/show_|
                   |bug.cgi?id=203543           |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[steven at gcc dot gnu dot org]

------- Comment #8 from steven at gcc dot gnu dot org  2008-02-03 12:26 -------
We call try_combine() with the following insns:

i1:
(insn 8 7 9 2 t.c:4 (set (reg/v:SI 94 [ i ])
        (reg:SI 28 %r28)) 37 {*pa.md:2542} (expr_list:REG_DEAD (reg:SI 28 %r28)
        (nil)))

i2:
(insn 31 10 32 2 t.c:5 (set (reg:SI 101)
        (neg:SI (reg/v:SI 94 [ i ]))) 136 {negsi2} (nil))

i3:
(insn 32 31 16 2 t.c:5 (set (reg/v:SI 94 [ i ])
        (if_then_else:SI (ge (reg/v:SI 94 [ i ])
                (reg:SI 98))
            (reg:SI 101)
            (reg/v:SI 94 [ i ]))) 24 {*pa.md:1388} (expr_list:REG_DEAD (reg:SI
101)
        (expr_list:REG_DEAD (reg:SI 98)
            (nil))))

At this point, r98 holds the value -10000.  In i1 the returned result of ret1()
is loaded into "i".
Somehow combine decides that the above sequence is equal to "i = abs(i)",
losing the comparison to -10000 as if it is a comparison against zero.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[steven at gcc dot gnu dot org]

------- Comment #9 from steven at gcc dot gnu dot org  2008-02-03 12:54 -------
Further investigation shows that simplify_if_then_else() called from
combine.c:5211 (combine.c r131744) turns this:

(if_then_else:SI (ge (reg/v:SI 94 [ i ])
        (reg:SI 98))
    (neg:SI (reg/v:SI 94 [ i ]))
    (reg/v:SI 94 [ i ]))

into this:

(neg:SI (abs:SI (reg/v:SI 94 [ i ])))

which is obviously wrong.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[steven at gcc dot gnu dot org]

------- Comment #10 from steven at gcc dot gnu dot org  2008-02-03 13:19 -------
The following code from simplify_if_then_else() triggers:

  /* Look for cases where we have (abs x) or (neg (abs X)).  */

  if (GET_MODE_CLASS (mode) == MODE_INT
      && GET_CODE (false_rtx) == NEG
      && rtx_equal_p (true_rtx, XEXP (false_rtx, 0))
      && comparison_p
      && rtx_equal_p (true_rtx, XEXP (cond, 0))
      && ! side_effects_p (true_rtx))
    switch (true_code)
      {
      case GT:
      case GE:
        return simplify_gen_unary (ABS, mode, true_rtx, mode);
      case LT:
      case LE:
        return
          simplify_gen_unary (NEG, mode,
                              simplify_gen_unary (ABS, mode, true_rtx, mode),
                              mode);
      default:
        break;
      }


with:

true_code = LT
cond == (lt (reg/v:SI 94 [ i ]) (reg:SI 98))
true_rtx == (reg/v:SI 94 [ i ])
false_rtx == (neg:SI (reg/v:SI 94 [ i ]))

or in human understandable language: "if (i < reg_98) i = i; else i = -1;"

We end up returning "(neg (abs:SI (reg/v:SI 94 [ i ])))".

The problem is that the if-condition in the quoted code does not look at what
is being compared against.  There should be a test to make sure the comparison
is against const0_rtx.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[steven at gcc dot gnu dot org]

------- Comment #11 from steven at gcc dot gnu dot org  2008-02-03 13:25 -------
My RTL fu is not sufficient to determine whether the following one-liner patch
is DTRT.  Eric, could you have a look at this, please?

Index: combine.c
===================================================================
--- combine.c   (revision 132078)
+++ combine.c   (working copy)
@@ -5393,9 +5393,10 @@
   /* Look for cases where we have (abs x) or (neg (abs X)).  */

   if (GET_MODE_CLASS (mode) == MODE_INT
+      && comparison_p
+      && XEXP (cond, 1) == const0_rtx
       && GET_CODE (false_rtx) == NEG
       && rtx_equal_p (true_rtx, XEXP (false_rtx, 0))
-      && comparison_p
       && rtx_equal_p (true_rtx, XEXP (cond, 0))
       && ! side_effects_p (true_rtx))
     switch (true_code)


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[rguenther at suse dot de]

------- Comment #12 from rguenther at suse dot de  2008-02-03 13:29 -------
Subject: Re:  Incorrect branching with -Ox on hppa

On Sun, 3 Feb 2008, steven at gcc dot gnu dot org wrote:

> ------- Comment #10 from steven at gcc dot gnu dot org  2008-02-03 13:19 -------
> The following code from simplify_if_then_else() triggers:
> 
>   /* Look for cases where we have (abs x) or (neg (abs X)).  */

In fact, this transformation is only valid for signed x because
iff x == 0x8000, (abs x) is undefined while (neg x) is not.

So depending on how abs is implemented in the md file the transformation
may be value changing;  thus I suggest to rip it out.

Richard.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[rguenth at gcc dot gnu dot org]

------- Comment #13 from rguenth at gcc dot gnu dot org  2008-02-03 14:00 -------
I was just worried if (neg:SI 0x800...) == (abs:SI 0x800...) is guaranteed,
which it is if (abs:SI x) is defined as if (x<0) x = -x. 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[steven at gcc dot gnu dot org]

------- Comment #14 from steven at gcc dot gnu dot org  2008-02-03 14:02 -------
Instead of const0_rtx, I probably should have used CONST0_RTX(mode) ...


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[dave at hiauly1 dot hia dot nrc dot ca]

------- Comment #15 from dave at hiauly1 dot hia dot nrc dot ca  2008-02-03 21:46 -------
Subject: Re:  Incorrect branching with -Ox on hppa

> Instead of const0_rtx, I probably should have used CONST0_RTX(mode) ...

I've tried Steven's change on hppa2.0w-hp-hpux11.11 and can confirm
that it resolves the testsuite problem with head.  I've got full
bootstraps running on 4.2 and head with the following change.

Index: combine.c
===================================================================
--- combine.c   (revision 132077)
+++ combine.c   (working copy)
@@ -5393,9 +5393,10 @@
   /* Look for cases where we have (abs x) or (neg (abs X)).  */

   if (GET_MODE_CLASS (mode) == MODE_INT
+      && comparison_p
+      && XEXP (cond, 1) == CONST0_RTX (mode)
       && GET_CODE (false_rtx) == NEG
       && rtx_equal_p (true_rtx, XEXP (false_rtx, 0))
-      && comparison_p
       && rtx_equal_p (true_rtx, XEXP (cond, 0))
       && ! side_effects_p (true_rtx))
     switch (true_code)

Dave


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[ebotcazou at gcc dot gnu dot org]

------- Comment #16 from ebotcazou at gcc dot gnu dot org  2008-02-04 08:12 -------
> Index: combine.c
> ===================================================================
> --- combine.c   (revision 132077)
> +++ combine.c   (working copy)
> @@ -5393,9 +5393,10 @@
>    /* Look for cases where we have (abs x) or (neg (abs X)).  */
> 
>    if (GET_MODE_CLASS (mode) == MODE_INT
> +      && comparison_p
> +      && XEXP (cond, 1) == CONST0_RTX (mode)
>        && GET_CODE (false_rtx) == NEG
>        && rtx_equal_p (true_rtx, XEXP (false_rtx, 0))
> -      && comparison_p
>        && rtx_equal_p (true_rtx, XEXP (cond, 0))
>        && ! side_effects_p (true_rtx))
>      switch (true_code)

This patch is OK for all branches in my opinion but run it by a RM because
the PR is not formally marked as a regression.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[ebotcazou at gcc dot gnu dot org]

------- Comment #17 from ebotcazou at gcc dot gnu dot org  2008-02-04 08:30 -------
> This patch is OK for all branches in my opinion but run it by a RM because
> the PR is not formally marked as a regression.

In fact Steven's original patch is slightly better, const0_rtx is valid for all
integral modes.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[rguenther at suse dot de]

------- Comment #18 from rguenther at suse dot de  2008-02-04 09:53 -------
Subject: Re:  Incorrect branching with -Ox on hppa

On Mon, 4 Feb 2008, ebotcazou at gcc dot gnu dot org wrote:

> 
> 
> ------- Comment #16 from ebotcazou at gcc dot gnu dot org  2008-02-04 08:12 -------
> > Index: combine.c
> > ===================================================================
> > --- combine.c   (revision 132077)
> > +++ combine.c   (working copy)
> > @@ -5393,9 +5393,10 @@
> >    /* Look for cases where we have (abs x) or (neg (abs X)).  */
> > 
> >    if (GET_MODE_CLASS (mode) == MODE_INT
> > +      && comparison_p
> > +      && XEXP (cond, 1) == CONST0_RTX (mode)
> >        && GET_CODE (false_rtx) == NEG
> >        && rtx_equal_p (true_rtx, XEXP (false_rtx, 0))
> > -      && comparison_p
> >        && rtx_equal_p (true_rtx, XEXP (cond, 0))
> >        && ! side_effects_p (true_rtx))
> >      switch (true_code)
> 
> This patch is OK for all branches in my opinion but run it by a RM because
> the PR is not formally marked as a regression.

The patch is fine for 4.3.

Richard.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[danglin at gcc dot gnu dot org]

------- Comment #19 from danglin at gcc dot gnu dot org  2008-02-09 00:31 -------
Subject: Bug 34627

Author: danglin
Date: Sat Feb  9 00:30:13 2008
New Revision: 132193

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=132193
Log:
        PR middle-end/34627
        combine.c (simplify_if_then_else): Make sure the comparison is
        against const0_rtx when simplifying to (abs x) or (neg (abs X)).


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/combine.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[danglin at gcc dot gnu dot org]

------- Comment #20 from danglin at gcc dot gnu dot org  2008-02-09 00:35 -------
Subject: Bug 34627

Author: danglin
Date: Sat Feb  9 00:34:19 2008
New Revision: 132194

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=132194
Log:
        PR middle-end/34627
        combine.c (simplify_if_then_else): Make sure the comparison is
        against const0_rtx when simplifying to (abs x) or (neg (abs X)).


Modified:
    branches/gcc-4_2-branch/gcc/ChangeLog
    branches/gcc-4_2-branch/gcc/combine.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[danglin at gcc dot gnu dot org]

------- Comment #21 from danglin at gcc dot gnu dot org  2008-02-09 00:37 -------
Subject: Bug 34627

Author: danglin
Date: Sat Feb  9 00:37:00 2008
New Revision: 132195

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=132195
Log:
        PR middle-end/34627
        combine.c (simplify_if_then_else): Make sure the comparison is
        against const0_rtx when simplifying to (abs x) or (neg (abs X)).


Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/combine.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[danglin at gcc dot gnu dot org]

------- Comment #22 from danglin at gcc dot gnu dot org  2008-02-09 00:39 -------
Fixed.


-- 

danglin at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[steven at gcc dot gnu dot org]

------- Comment #23 from steven at gcc dot gnu dot org  2008-02-09 09:20 -------
Please fix the ChangeLog entries.  You forgot the asterisk "*" before combine.c
in every commit you made.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627

[Bug middle-end/34627] Incorrect branching with -Ox on hppa

[dave at hiauly1 dot hia dot nrc dot ca]

------- Comment #24 from dave at hiauly1 dot hia dot nrc dot ca  2008-02-09 15:06 -------
Subject: Re:  Incorrect branching with -Ox on hppa

> Please fix the ChangeLog entries.  You forgot the asterisk "*" before combine.c
> in every commit you made.

Done, sorry.

Dave


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34627