public inbox for gcc-prs@sourceware.org
help / color / mirror / Atom feed
* Re: optimization/7147: ifcvt.c problem (regression)
@ 2002-09-29 9:42 sayle
0 siblings, 0 replies; 6+ messages in thread
From: sayle @ 2002-09-29 9:42 UTC (permalink / raw)
To: Franz.Sirl-kernel, gcc-bugs, gcc-prs, kevin.hendricks, nobody
Synopsis: ifcvt.c problem (regression)
State-Changed-From-To: open->closed
State-Changed-By: sayle
State-Changed-When: Sun Sep 29 09:42:07 2002
State-Changed-Why:
This problem has been fixed both on the gcc 3.2 branch and
on mainline by Richard Henderson's patch:
2002-07-18 Richard Henderson <rth@redhat.com>
PR optimization/7147
* ifcvt.c (noce_get_condition): Make certain that the condition
is valid at JUMP.
http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=7147
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: optimization/7147: ifcvt.c problem (regression)
@ 2002-07-18 6:26 Mark Mitchell
0 siblings, 0 replies; 6+ messages in thread
From: Mark Mitchell @ 2002-07-18 6:26 UTC (permalink / raw)
To: nobody; +Cc: gcc-prs
The following reply was made to PR optimization/7147; it has been noted by GNATS.
From: Mark Mitchell <mark@codesourcery.com>
To: Franz Sirl <Franz.Sirl-kernel@lauterbach.com>,
Richard Henderson <rth@redhat.com>
Cc: "gcc-gnats@gcc.gnu.org" <gcc-gnats@gcc.gnu.org>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: Re: optimization/7147: ifcvt.c problem (regression)
Date: Thu, 18 Jul 2002 06:24:18 -0700
> That fixes the small executable testcase I posted and inspection of the
> assembly shows that the original mozilla source code is fine too now.
> Bootstrap and regression testing on powerpc-linux-gnu without problems.
Great; let's do it.
Thanks,
--
Mark Mitchell mark@codesourcery.com
CodeSourcery, LLC http://www.codesourcery.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: optimization/7147: ifcvt.c problem (regression)
@ 2002-07-18 6:06 Franz Sirl
0 siblings, 0 replies; 6+ messages in thread
From: Franz Sirl @ 2002-07-18 6:06 UTC (permalink / raw)
To: nobody; +Cc: gcc-prs
The following reply was made to PR optimization/7147; it has been noted by GNATS.
From: Franz Sirl <Franz.Sirl-kernel@lauterbach.com>
To: Richard Henderson <rth@redhat.com>
Cc: gcc-gnats@gcc.gnu.org,gcc-patches@gcc.gnu.org,
Mark Mitchell <mark@codesourcery.com>
Subject: Re: optimization/7147: ifcvt.c problem (regression)
Date: Thu, 18 Jul 2002 15:05:16 +0200
At 23:27 16.07.2002, Richard Henderson wrote:
>This looks like fallout from
>
>2002-05-03 Richard Henderson <rth@redhat.com>
>
> PR opt/6534
> * ifcvt.c (noce_try_store_flag, noce_try_store_flag_constants,
> noce_try_store_flag_inc, noce_try_store_flag_mask, noce_try_cmove,
> noce_try_cmove_arith, noce_try_minmax, noce_try_abs): Insert new
> code before JUMP, not EARLIEST.
>
>I'm testing the following on x86. Give it a whirl on ppc as well?
>
>
>r~
>
>
> * ifcvt.c (noce_get_condition): Make certain that the condition
> is valid at JUMP.
That fixes the small executable testcase I posted and inspection of the
assembly shows that the original mozilla source code is fine too now.
Bootstrap and regression testing on powerpc-linux-gnu without problems.
Franz.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: optimization/7147: ifcvt.c problem (regression)
@ 2002-07-16 14:36 Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2002-07-16 14:36 UTC (permalink / raw)
To: nobody; +Cc: gcc-prs
The following reply was made to PR optimization/7147; it has been noted by GNATS.
From: Richard Henderson <rth@redhat.com>
To: Franz Sirl <Franz.Sirl-kernel@lauterbach.com>
Cc: gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org,
Mark Mitchell <mark@codesourcery.com>
Subject: Re: optimization/7147: ifcvt.c problem (regression)
Date: Tue, 16 Jul 2002 14:27:52 -0700
This looks like fallout from
2002-05-03 Richard Henderson <rth@redhat.com>
PR opt/6534
* ifcvt.c (noce_try_store_flag, noce_try_store_flag_constants,
noce_try_store_flag_inc, noce_try_store_flag_mask, noce_try_cmove,
noce_try_cmove_arith, noce_try_minmax, noce_try_abs): Insert new
code before JUMP, not EARLIEST.
I'm testing the following on x86. Give it a whirl on ppc as well?
r~
* ifcvt.c (noce_get_condition): Make certain that the condition
is valid at JUMP.
Index: ifcvt.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ifcvt.c,v
retrieving revision 1.78.2.5
diff -c -p -d -r1.78.2.5 ifcvt.c
*** ifcvt.c 3 May 2002 20:24:01 -0000 1.78.2.5
--- ifcvt.c 16 Jul 2002 21:21:30 -0000
*************** noce_try_abs (if_info)
*** 1504,1548 ****
return TRUE;
}
! /* Look for the condition for the jump first. We'd prefer to avoid
! get_condition if we can -- it tries to look back for the contents
! of an original compare. On targets that use normal integers for
! comparisons, e.g. alpha, this is wasteful. */
static rtx
noce_get_condition (jump, earliest)
rtx jump;
rtx *earliest;
{
! rtx cond;
! rtx set;
!
! /* If the condition variable is a register and is MODE_INT, accept it.
! Otherwise, fall back on get_condition. */
if (! any_condjump_p (jump))
return NULL_RTX;
set = pc_set (jump);
cond = XEXP (SET_SRC (set), 0);
! if (GET_CODE (XEXP (cond, 0)) == REG
! && GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) == MODE_INT)
{
*earliest = jump;
! /* If this branches to JUMP_LABEL when the condition is false,
! reverse the condition. */
! if (GET_CODE (XEXP (SET_SRC (set), 2)) == LABEL_REF
! && XEXP (XEXP (SET_SRC (set), 2), 0) == JUMP_LABEL (jump))
cond = gen_rtx_fmt_ee (reverse_condition (GET_CODE (cond)),
! GET_MODE (cond), XEXP (cond, 0),
! XEXP (cond, 1));
}
- else
- cond = get_condition (jump, earliest);
! return cond;
}
/* Return true if OP is ok for if-then-else processing. */
--- 1504,1576 ----
return TRUE;
}
! /* Similar to get_condition, only the resulting condition must be
! valid at JUMP, instead of at EARLIEST. */
static rtx
noce_get_condition (jump, earliest)
rtx jump;
rtx *earliest;
{
! rtx cond, set, tmp, insn;
! bool reverse;
if (! any_condjump_p (jump))
return NULL_RTX;
set = pc_set (jump);
+ /* If this branches to JUMP_LABEL when the condition is false,
+ reverse the condition. */
+ reverse = (GET_CODE (XEXP (SET_SRC (set), 2)) == LABEL_REF
+ && XEXP (XEXP (SET_SRC (set), 2), 0) == JUMP_LABEL (jump));
+
+ /* If the condition variable is a register and is MODE_INT, accept it. */
+
cond = XEXP (SET_SRC (set), 0);
! tmp = XEXP (cond, 0);
! if (REG_P (tmp) && GET_MODE_CLASS (GET_MODE (tmp)) == MODE_INT)
{
*earliest = jump;
! if (reverse)
cond = gen_rtx_fmt_ee (reverse_condition (GET_CODE (cond)),
! GET_MODE (cond), tmp, XEXP (cond, 1));
! return cond;
}
! /* Otherwise, fall back on canonicalize_condition to do the dirty
! work of manipulating MODE_CC values and COMPARE rtx codes. */
!
! tmp = canonicalize_condition (jump, cond, reverse, earliest, NULL_RTX);
! if (!tmp)
! return NULL_RTX;
!
! /* We are going to insert code before JUMP, not before EARLIEST.
! We must therefore be certain that the given condition is valid
! at JUMP by virtue of not having been modified since. */
! for (insn = *earliest; insn != jump; insn = NEXT_INSN (insn))
! if (INSN_P (insn) && modified_in_p (tmp, insn))
! break;
! if (insn == jump)
! return tmp;
!
! /* The condition was modified. See if we can get a partial result
! that doesn't follow all the reversals. Perhaps combine can fold
! them together later. */
! tmp = XEXP (tmp, 0);
! if (!REG_P (tmp) || GET_MODE_CLASS (GET_MODE (tmp)) != MODE_INT)
! return NULL_RTX;
! tmp = canonicalize_condition (jump, cond, reverse, earliest, tmp);
! if (!tmp)
! return NULL_RTX;
!
! /* For sanity's sake, re-validate the new result. */
! for (insn = *earliest; insn != jump; insn = NEXT_INSN (insn))
! if (INSN_P (insn) && modified_in_p (tmp, insn))
! return NULL_RTX;
!
! return tmp;
}
/* Return true if OP is ok for if-then-else processing. */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: optimization/7147: ifcvt.c problem (regression)
@ 2002-06-30 19:06 Franz Sirl
0 siblings, 0 replies; 6+ messages in thread
From: Franz Sirl @ 2002-06-30 19:06 UTC (permalink / raw)
To: nobody; +Cc: gcc-prs
The following reply was made to PR optimization/7147; it has been noted by GNATS.
From: Franz Sirl <Franz.Sirl-kernel@lauterbach.com>
To: gcc-gnats@gcc.gnu.org
Cc: gcc-patches@gcc.gnu.org,
Richard Henderson <rth@redhat.com>,
Mark Mitchell <mark@codesourcery.com>
Subject: Re: optimization/7147: ifcvt.c problem (regression)
Date: Sun, 30 Jun 2002 22:40:20 +0200
--------------Boundary-00=_8FDJ7SMSRRY283R9ID8T
Content-Type: text/plain;
charset="iso-8859-1"
Content-Transfer-Encoding: 8bit
On Freitag, 28. Juni 2002 00:55, Franz.Sirl-kernel@lauterbach.com wrote:
> >Number: 7147
> >Category: optimization
> >Synopsis: ifcvt.c problem (regression)
> >Confidential: no
> >Severity: serious
> >Priority: medium
> >Responsible: unassigned
> >State: open
> >Class: sw-bug
> >Submitter-Id: net
> >Arrival-Date: Thu Jun 27 15:56:02 PDT 2002
> >Closed-Date:
> >Last-Modified:
> >Originator: Franz.Sirl-kernel@lauterbach.com
> >Release: current gcc-3_1-branch
> >Organization:
> >Environment:
>
> powerpc-linux-gnu
>
> >Description:
>
> This testcase is derived from a reported c++ mozilla miscompilation. It
> fails at -O and works fine with -O -fexpensive-optimizations.
>
> My guess so far is that ifcvt.c is confused by the multiple SETs of pseudo
> 117 (from .rtl):
>
> (insn 27 25 28 (set (reg/v:SI 117)
> (reg:SI 3 r3)) -1 (nil)
> (nil))
>
> (insn 28 27 29 (set (reg:CC 120)
> (compare:CC (reg/v:SI 117)
> (const_int 0 [0x0]))) -1 (nil)
> (nil))
>
> (insn 29 28 31 (set (reg/v:SI 117)
> (eq:SI (reg:CC 120)
> (const_int 0 [0x0]))) -1 (nil)
> (nil))
>
> (jump_insn 31 29 32 (set (pc)
> (label_ref 34)) -1 (nil)
> (nil))
>
> (barrier 32 31 33)
>
> (note 33 32 34 0x30084680 NOTE_INSN_BLOCK_END)
>
> (code_label 34 33 35 3 ("lab1") "" [0 uses])
>
> (note 35 34 37 0x300846c0 NOTE_INSN_BLOCK_END)
>
> (insn 37 35 38 (set (reg:CC 121)
> (compare:CC (reg/v:SI 117)
> (const_int 0 [0x0]))) -1 (nil)
> (nil))
>
> (jump_insn 38 37 40 (set (pc)
> (if_then_else (eq (reg:CC 121)
> (const_int 0 [0x0]))
> (label_ref 48)
> (pc))) -1 (nil)
> (nil))
>
>
> Somehow the inversion via insn 28 and 29 isn't accounted for.
Actually the inversion is accounted for, but the multiple sets of pseudo reg
117 seem to confuse the compiler somehow.
Richard, the comment in front of noce_get_condition seems to suggest that you
weren't satisfied with the use of get_condition here anyway, so maybe
WANT_REG for canonicalized_condition should always be set to limit the search
range?
Anyway, the appended patch fixes the testcase for me, running a full bootstrap
on powerpc-linux-gnu and x86-linux-gnu now.
Franz.
PR optimization/7147
* ifcvt.c (noce_get_condition): Use canonicalized_condition directly.
Limit scan range if !flag_expensive_optimizations.
--------------Boundary-00=_8FDJ7SMSRRY283R9ID8T
Content-Type: text/plain;
charset="iso-8859-1";
name="gcc-pr7147.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="gcc-pr7147.patch"
Index: gcc/ifcvt.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/ifcvt.c,v
retrieving revision 1.78.2.5
diff -u -p -r1.78.2.5 ifcvt.c
--- gcc/ifcvt.c 3 May 2002 20:24:01 -0000 1.78.2.5
+++ gcc/ifcvt.c 30 Jun 2002 20:24:35 -0000
@@ -1505,9 +1505,9 @@ noce_try_abs (if_info)
}
/* Look for the condition for the jump first. We'd prefer to avoid
- get_condition if we can -- it tries to look back for the contents
- of an original compare. On targets that use normal integers for
- comparisons, e.g. alpha, this is wasteful. */
+ canonicalize_condition if we can -- it tries to look back for the
+ contents of an original compare. On targets that use normal integers
+ for comparisons, e.g. alpha, this is wasteful. */
static rtx
noce_get_condition (jump, earliest)
@@ -1516,6 +1516,7 @@ noce_get_condition (jump, earliest)
{
rtx cond;
rtx set;
+ int reverse;
/* If the condition variable is a register and is MODE_INT, accept it.
Otherwise, fall back on get_condition. */
@@ -1525,22 +1526,38 @@ noce_get_condition (jump, earliest)
set = pc_set (jump);
+ /* If this branches to JUMP_LABEL when the condition is false, reverse
+ the condition. */
+ reverse
+ = GET_CODE (XEXP (SET_SRC (set), 2)) == LABEL_REF
+ && XEXP (XEXP (SET_SRC (set), 2), 0) == JUMP_LABEL (jump);
+
cond = XEXP (SET_SRC (set), 0);
if (GET_CODE (XEXP (cond, 0)) == REG
&& GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) == MODE_INT)
{
*earliest = jump;
- /* If this branches to JUMP_LABEL when the condition is false,
- reverse the condition. */
- if (GET_CODE (XEXP (SET_SRC (set), 2)) == LABEL_REF
- && XEXP (XEXP (SET_SRC (set), 2), 0) == JUMP_LABEL (jump))
+ if (reverse)
cond = gen_rtx_fmt_ee (reverse_condition (GET_CODE (cond)),
GET_MODE (cond), XEXP (cond, 0),
XEXP (cond, 1));
}
else
- cond = get_condition (jump, earliest);
+ {
+ rtx want_reg;
+
+ /* If this is not a standard conditional jump, we can't parse it. */
+ if (GET_CODE (jump) != JUMP_INSN
+ || ! any_condjump_p (jump))
+ return 0;
+
+ /* With flag_expensive_optimizations unset, code maybe confused by
+ shared pseudos, so prevent going back too far in determining COND. */
+ want_reg = flag_expensive_optimizations ? NULL_RTX : XEXP (cond, 0);
+
+ cond = canonicalize_condition (jump, cond, reverse, earliest, want_reg);
+ }
return cond;
}
--------------Boundary-00=_8FDJ7SMSRRY283R9ID8T--
^ permalink raw reply [flat|nested] 6+ messages in thread
* optimization/7147: ifcvt.c problem (regression)
@ 2002-06-27 16:06 Franz.Sirl-kernel
0 siblings, 0 replies; 6+ messages in thread
From: Franz.Sirl-kernel @ 2002-06-27 16:06 UTC (permalink / raw)
To: gcc-gnats; +Cc: kevin.hendricks
>Number: 7147
>Category: optimization
>Synopsis: ifcvt.c problem (regression)
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: unassigned
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Thu Jun 27 15:56:02 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator: Franz.Sirl-kernel@lauterbach.com
>Release: current gcc-3_1-branch
>Organization:
>Environment:
powerpc-linux-gnu
>Description:
This testcase is derived from a reported c++ mozilla miscompilation. It fails at -O and works fine with -O -fexpensive-optimizations.
My guess so far is that ifcvt.c is confused by the multiple SETs of pseudo 117 (from .rtl):
(insn 27 25 28 (set (reg/v:SI 117)
(reg:SI 3 r3)) -1 (nil)
(nil))
(insn 28 27 29 (set (reg:CC 120)
(compare:CC (reg/v:SI 117)
(const_int 0 [0x0]))) -1 (nil)
(nil))
(insn 29 28 31 (set (reg/v:SI 117)
(eq:SI (reg:CC 120)
(const_int 0 [0x0]))) -1 (nil)
(nil))
(jump_insn 31 29 32 (set (pc)
(label_ref 34)) -1 (nil)
(nil))
(barrier 32 31 33)
(note 33 32 34 0x30084680 NOTE_INSN_BLOCK_END)
(code_label 34 33 35 3 ("lab1") "" [0 uses])
(note 35 34 37 0x300846c0 NOTE_INSN_BLOCK_END)
(insn 37 35 38 (set (reg:CC 121)
(compare:CC (reg/v:SI 117)
(const_int 0 [0x0]))) -1 (nil)
(nil))
(jump_insn 38 37 40 (set (pc)
(if_then_else (eq (reg:CC 121)
(const_int 0 [0x0]))
(label_ref 48)
(pc))) -1 (nil)
(nil))
Somehow the inversion via insn 28 and 29 isn't accounted for.
This is a regression from gcc-2.95.4.
>How-To-Repeat:
Compile with -O on powerpc-linux-gnu:
extern void abort (void);
extern void exit (int);
int sub1 (int val)
{
return val;
}
int testcond (int val)
{
int flag1;
{
int t1 = val;
{
int t2 = t1;
{
flag1 = sub1 (t2) ==0;
goto lab1;
};
}
lab1: ;
}
if (flag1 != 0)
return 0x4d0000;
else
return 0;
}
int main (void)
{
if (testcond (1))
abort ();
exit (0);
}
>Fix:
hacking stmt.c like that fixes the bug:
int
preserve_subexpressions_p ()
{
rtx insn;
if (flag_expensive_optimizations)
return 1;
if (optimize == 0 || cfun == 0 || cfun->stmt == 0 || loop_stack == 0)
return 0;
+ // make ifcvt happy
+ return 1;
+
insn = get_last_insn_anywhere ();
return (insn
&& (INSN_UID (insn) - INSN_UID (loop_stack->data.loop.start_label)
< n_non_fixed_regs * 3));
}
>Release-Note:
>Audit-Trail:
>Unformatted:
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2002-09-29 16:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-09-29 9:42 optimization/7147: ifcvt.c problem (regression) sayle
-- strict thread matches above, loose matches on Subject: below --
2002-07-18 6:26 Mark Mitchell
2002-07-18 6:06 Franz Sirl
2002-07-16 14:36 Richard Henderson
2002-06-30 19:06 Franz Sirl
2002-06-27 16:06 Franz.Sirl-kernel
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).