This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[tree-ssa] Fix tsi_insertRe: [tree-ssa] Bootstrap failure oni686-linux
- From: Diego Novillo <dnovillo at redhat dot com>
- To: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Cc: Jeff Law <law at redhat dot com>, Paul Brook <paul at nowt dot org>, Richard Henderson <rth at redhat dot com>, Zdenek Dvorak <rakdver at atrey dot karlin dot mff dot cuni dot cz>
- Date: Tue, 02 Sep 2003 17:45:17 -0400
- Subject: [tree-ssa] Fix tsi_insertRe: [tree-ssa] Bootstrap failure oni686-linux
- Organization: Red Hat Canada
- References: <200309021459.h82ExwIX020092@speedy.slc.redhat.com> <1062526033.6332.71.camel@frodo.toronto.redhat.com>
[ Redirected to gcc-patches ]
On Tue, 2003-09-02 at 14:07, Diego Novillo wrote:
> PRE seems to have exposed another bug in the edge insertion routines.
> From what I've seen so far, handle_switch_split is mishandling an
> insertion at [ XXX ].
>
> > switch (((re_opcode_t) * p++))
> > {
> > unconditional_jump:
> > ;
> > case jump:
> [ XXX ]
> > do
> > {
> > (mcnt) = *(p) & 0377;
> > }
> > while (0);
>
The root of the problem was in tree.c:tsi_link_after. We were not
updating basic block boundaries when shifting the CE nodes down. In
this particular case, PRE was trying to insert a statement on the edge 0
-> 2.
# BLOCK 0. PRED -1. SUCC: 5 2 -2
switch (T.2_9)
{
# BLOCK 1. PRED: 6. SUCC: 2.
unconditional_jump:;;
# BLOCK 2. PRED: 0 1. SUCC: 3.
case 0:;
# BLOCK 3. PRED: 2. SUCC: 4.
<ULb380>:;;
[ ... ]
In bsi_insert_on_edge we do all the transformations correctly, leaving
the code looking like this:
# BLOCK 0. PRED: -1. SUCC: 5 8 -2.
switch (T.2_9)
{
# BLOCK 1. PRED: 6. SUCC: 2.
unconditional_jump:;;
goto <UL770>;;
# BLOCK 8. PRED: 0. SUCC: 2.
case 0:;
pretmp.7_23 = p_10 + 2B;
# BLOCK 2. PRED: 8 1. SUCC: 3.
<UL770>:;;
# BLOCK 3. PRED: 2. SUCC: 4.
<ULb380>:;;
The edge insertion routines create the <UL770> label and the 'goto
<UL770>'. 'pretmp.7_23 = p_10 + 2B' is the new statement inserted by
PRE.
Everything seems fine, but when we call tsi_link_after from within
tree-cfg.c:handle_switch_split, we lose the pointer to block 3's
head_tree_p.
The reason we lose that is because of the way these labels happen to be
chained in this test case. The call to tsi_link_after is trying to
insert '<UL770>:' right after 'case 0:'. So, originally, the CE tree
looks like this
CE1 (BLOCK #8)
+--> case 0:
|
+--> CE2 (BLOCK #3)
+--> <ULb380>:
|
+--> CE3
Since <ULb380> is the first statement in basic block 3, head_tree_p
contains &CE2. But in tsi_link_after, we are substituing CE2 with
*another* COMPOUND_EXPR node. So now, head_tree_p for block 3 will be
pointing to a statement in another block:
CE1 (BLOCK #8)
+--> case 0:
|
+--> CE4
+--> <UL7770>:
|
+--> CE2 (BLOCK #3)
+--> <ULb380>:
|
+--> CE3
We were leaving head_tree_p pointing to CE4. That's why we were then
not able to walk down the code. Tree dumps weren't affected by this
because the CE chaining was correctly updated.
I'm still bootstrapping (without rth's patch). If it works, I will
commit it as this should be independent of Richard's changes. I'll also
add Jeff's test case to the testsuite.
Diego.
* Makefile.in (tree.o): Add dependency on $(BASIC_BLOCK_H) and
$(TREE_FLOW_H)
* tree.c: Include basic-block.h and tree-flow.h
(tsi_link_after): Adjust basic block tree pointers when inserting a
new COMPOUND_EXPR.
Index: Makefile.in
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Makefile.in,v
retrieving revision 1.903.2.110
diff -d -c -p -r1.903.2.110 Makefile.in
*** Makefile.in 20 Aug 2003 20:43:55 -0000 1.903.2.110
--- Makefile.in 2 Sep 2003 20:47:58 -0000
*************** langhooks.o : langhooks.c $(CONFIG_H) $(
*** 1511,1517 ****
$(LANGHOOKS_DEF_H) flags.h $(GGC_H) gt-langhooks.h
tree.o : tree.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) flags.h function.h \
toplev.h $(GGC_H) $(HASHTAB_H) $(TARGET_H) output.h $(TM_P_H) langhooks.h \
! real.h gt-tree.h tree-iterator.h
tree-dump.o: tree-dump.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \
$(C_TREE_H) flags.h langhooks.h toplev.h output.h c-pragma.h $(RTL_H) $(GGC_H) \
$(EXPR_H) $(SPLAY_TREE_H) $(TREE_DUMP_H)
--- 1511,1517 ----
$(LANGHOOKS_DEF_H) flags.h $(GGC_H) gt-langhooks.h
tree.o : tree.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) flags.h function.h \
toplev.h $(GGC_H) $(HASHTAB_H) $(TARGET_H) output.h $(TM_P_H) langhooks.h \
! real.h gt-tree.h tree-iterator.h $(BASIC_BLOCK_H) $(TREE_FLOW_H)
tree-dump.o: tree-dump.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \
$(C_TREE_H) flags.h langhooks.h toplev.h output.h c-pragma.h $(RTL_H) $(GGC_H) \
$(EXPR_H) $(SPLAY_TREE_H) $(TREE_DUMP_H)
Index: tree.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree.c,v
retrieving revision 1.263.2.49
diff -d -c -p -r1.263.2.49 tree.c
*** tree.c 27 Aug 2003 04:31:37 -0000 1.263.2.49
--- tree.c 2 Sep 2003 20:49:51 -0000
*************** Software Foundation, 59 Temple Place - S
*** 46,51 ****
--- 46,53 ----
#include "target.h"
#include "langhooks.h"
#include "tree-iterator.h"
+ #include "basic-block.h"
+ #include "tree-flow.h"
/* obstack.[ch] explicitly declined to prototype this. */
extern int _obstack_allocated_p (struct obstack *h, void *obj);
*************** tsi_link_after (tree_stmt_iterator *i, t
*** 5158,5163 ****
--- 5160,5180 ----
/* Create a new node with the same 'next' link as the current one. */
ce = build (COMPOUND_EXPR, void_type_node, t, next);
+ /* If the 'next' statement is a COMPOUND_EXPR and was the first
+ statement of a basic block, we need to adjust the 'head_tree_p'
+ field of that block because we are about to move the statement one
+ position down in the CE chain. */
+ {
+ basic_block bb = bb_for_stmt (next);
+ if (bb && bb->head_tree_p == &TREE_OPERAND (*(i->tp), 1))
+ {
+ /* We may also need to update end_tree_p. */
+ if (bb->head_tree_p == bb->end_tree_p)
+ bb->end_tree_p = &TREE_OPERAND (ce, 1);
+ bb->head_tree_p = &TREE_OPERAND (ce, 1);
+ }
+ }
+
/* Make the current one's 'next' link point to our new stmt. */
TREE_OPERAND (*(i->tp), 1) = ce;
*************** tsi_link_after (tree_stmt_iterator *i, t
*** 5165,5172 ****
if (mode == TSI_NEW_STMT)
i->tp = &(TREE_OPERAND (*(i->tp), 1));
}
-
-
}
/* Remove a stmt from the tree list. The iterator is updated to point to
--- 5182,5187 ----