This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] PR 9974: Insns on edges in bypass (take 2)
- From: Roger Sayle <roger at www dot eyesopen dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: Richard Henderson <rth at redhat dot com>
- Date: Tue, 1 Apr 2003 06:49:48 -0700 (MST)
- Subject: [PATCH] PR 9974: Insns on edges in bypass (take 2)
On Sun, 30 Mar 2003, Richard Henderson wrote:
> On Mon, Mar 24, 2003 at 09:18:52PM -0700, Roger Sayle wrote:
> > + valid prior to comit_edge_insertions. */
> typo.
Fixed.
> > + static bool
> > + reg_killed_on_edge (regno, e)
> > + unsigned int regno;
> > + edge e;
> > + {
> > + rtx insn, set;
> > +
> > + for (insn = e->insns; insn; insn = NEXT_INSN (insn))
> > + if (INSN_P (insn)
> > + && (set = single_set (insn)) != NULL_RTX
> > + && GET_CODE (SET_DEST (set)) == REG
> > + && REGNO (SET_DEST (set)) == regno)
> > + return true;
>
> I'd prefer that you used note_stores here.
Yep, that would be much better. I just duplicated the logic that was
used to check the insns that get placed on the edge. Unfortunately,
this ignores clobbers. note_stores, cool! However, writing the
note_stores callback I had the premonition someone must have done this
before. Sure enough reg_set_p calls note_stores with set_of_1 that
does exactly what I need.
> > if (new == pc_rtx)
> > ! {
> > ! edest = FALLTHRU_EDGE (bb);
> > ! dest = edest->insns ? NULL : edest->dest;
> > ! }
> > else if (GET_CODE (new) == LABEL_REF)
> > ! {
> > ! dest = BLOCK_FOR_INSN (XEXP (new, 0));
> > ! /* Don't bypass edges containing instructions. */
>
> I think the comment should get pushed up (so that it's clear
> that it applies to the fallthru edge as well). In addition,
> I think a bit more commentary as to *why* would be appropriate.
Done. I've added paragraphs both before the function and immediately
before the section quoted above warning of fact that we may have
insns inserted on edges.
The following patch has been tested on i686-pc-linux-gnu with a
full "make bootstrap", all languages except Ada and treelang, and
regression tested with a top-level "make -k check" with no new
regressions. I've also made built the C compiler on mips-sgi-irix6.5
(single stage not bootstrap) and confirmed that it still fixes the
attached test case.
Ok for mainline?
2003-04-01 Roger Sayle <roger at eyesopen dot com>
PR fortran/9974
* gcse.c (reg_killed_on_egde): New function to test whether the
given regno is overwritten by any instruction queued on an edge.
(bypass_block): Ignore substitutions killed on incoming edges.
Don't bypass outgoing edges that have queued instructions.
* gcc.c-torture/execute/20030324-1.c: New test case.
Index: gcse.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gcse.c,v
retrieving revision 1.241
diff -c -3 -p -r1.241 gcse.c
*** gcse.c 31 Mar 2003 06:28:55 -0000 1.241
--- gcse.c 1 Apr 2003 04:23:09 -0000
*************** static void find_implicit_sets PARAMS ((
*** 623,628 ****
--- 623,629 ----
static int one_cprop_pass PARAMS ((int, int, int));
static bool constprop_register PARAMS ((rtx, rtx, rtx, int));
static struct expr *find_bypass_set PARAMS ((int, int));
+ static bool reg_killed_on_edge PARAMS ((rtx, edge));
static int bypass_block PARAMS ((basic_block, rtx, rtx));
static int bypass_conditional_jumps PARAMS ((void));
static void alloc_pre_mem PARAMS ((int, int));
*************** find_bypass_set (regno, bb)
*** 4763,4773 ****
}
/* Subroutine of bypass_conditional_jumps that attempts to bypass the given
basic block BB which has more than one predecessor. If not NULL, SETCC
is the first instruction of BB, which is immediately followed by JUMP_INSN
JUMP. Otherwise, SETCC is NULL, and JUMP is the first insn of BB.
! Returns nonzero if a change was made. */
static int
bypass_block (bb, setcc, jump)
--- 4764,4798 ----
}
+ /* Subroutine of bypass_block that checks whether a pseudo is killed by
+ any of the instructions inserted on an edge. Jump bypassing places
+ condition code setters on CFG edges using insert_insn_on_edge. This
+ function is required to check that our data flow analysis is still
+ valid prior to commit_edge_insertions. */
+
+ static bool
+ reg_killed_on_edge (reg, e)
+ rtx reg;
+ edge e;
+ {
+ rtx insn;
+
+ for (insn = e->insns; insn; insn = NEXT_INSN (insn))
+ if (INSN_P (insn) && reg_set_p (reg, insn))
+ return true;
+
+ return false;
+ }
+
/* Subroutine of bypass_conditional_jumps that attempts to bypass the given
basic block BB which has more than one predecessor. If not NULL, SETCC
is the first instruction of BB, which is immediately followed by JUMP_INSN
JUMP. Otherwise, SETCC is NULL, and JUMP is the first insn of BB.
! Returns nonzero if a change was made.
!
! During the jump bypassing pass, we may place copies of SETCC instuctions
! on CFG edges. The following routine must be careful to pay attention to
! these inserted insns when performing its transformations. */
static int
bypass_block (bb, setcc, jump)
*************** bypass_block (bb, setcc, jump)
*** 4775,4781 ****
rtx setcc, jump;
{
rtx insn, note;
! edge e, enext;
int i, change;
int may_be_loop_header;
--- 4800,4806 ----
rtx setcc, jump;
{
rtx insn, note;
! edge e, enext, edest;
int i, change;
int may_be_loop_header;
*************** bypass_block (bb, setcc, jump)
*** 4830,4835 ****
--- 4855,4864 ----
if (! set)
continue;
+ /* Check the data flow is valid after edge insertions. */
+ if (e->insns && reg_killed_on_edge (reg_used->reg_rtx, e))
+ continue;
+
src = SET_SRC (pc_set (jump));
if (setcc != NULL)
*************** bypass_block (bb, setcc, jump)
*** 4840,4849 ****
new = simplify_replace_rtx (src, reg_used->reg_rtx,
SET_SRC (set->expr));
if (new == pc_rtx)
! dest = FALLTHRU_EDGE (bb)->dest;
else if (GET_CODE (new) == LABEL_REF)
! dest = BLOCK_FOR_INSN (XEXP (new, 0));
else
dest = NULL;
--- 4869,4895 ----
new = simplify_replace_rtx (src, reg_used->reg_rtx,
SET_SRC (set->expr));
+ /* Jump bypassing may have already placed instructions on
+ edges of the CFG. We can't bypass an outgoing edge that
+ has instructions associated with it, as these insns won't
+ get executed if the incoming edge is redirected. */
+
if (new == pc_rtx)
! {
! edest = FALLTHRU_EDGE (bb);
! dest = edest->insns ? NULL : edest->dest;
! }
else if (GET_CODE (new) == LABEL_REF)
! {
! dest = BLOCK_FOR_INSN (XEXP (new, 0));
! /* Don't bypass edges containing instructions. */
! for (edest = bb->succ; edest; edest = edest->succ_next)
! if (edest->dest == dest && edest->insns)
! {
! dest = NULL;
! break;
! }
! }
else
dest = NULL;
/* Testcase for PR fortran/9974. This was a miscompilation of the g77
front-end caused by the jump bypassing optimizations not handling
instructions inserted on CFG edges. */
extern void abort ();
int bar ()
{
return 1;
}
void foo (int x)
{
unsigned char error = 0;
if (! (error = ((x == 0) || bar ())))
bar ();
if (! error)
abort ();
}
int main()
{
foo (1);
return 0;
}
Roger
--
Roger Sayle, E-mail: roger at eyesopen dot com
OpenEye Scientific Software, WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road, Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507. Fax: (+1) 505-473-0833