* [tree-ssa] Mainline merge as of 2003-04-06
@ 2003-04-09 20:37 Diego Novillo
2003-04-09 23:59 ` law
0 siblings, 1 reply; 6+ messages in thread
From: Diego Novillo @ 2003-04-09 20:37 UTC (permalink / raw)
To: gcc
This merge exposed a few bugs in the flowgraph builder. Thanks
to Steven Bosscher for finding the hundreds of new C++
regressions. In cp/call.c:print_z_candidate, I had stupidly
changed the call to errfn to pass &TREE_LOCUS instead of
TREE_LOCUS.
There are a couple new regressions due to line number mismatch.
Also, we are failing g++.dg/eh/cond1.C. The gimplifier is trying
to put the call to __cxa_throw on the RHS of an assignment.
Since __cxa_throw is of type void, we go on to ICE in the RTL
expanders.
This patch to the gimplifier fixes the symptom, but I'm not sure
if it fixes the problem. It's odd that we have a void object in
an INIT_EXPR, but I'm not at all familiar with the C++ FE.
Jason, does this fix look right? (it's still not committed):
--- gimplify.c 20 Mar 2003 21:21:15 -0000 1.1.2.32
+++ gimplify.c 9 Apr 2003 19:17:29 -0000
@@ -2319,7 +2320,8 @@ simplify_target_expr (expr_p, pre_p, pos
gimple_add_tmp_var (temp);
/* Build up the initialization and add it to pre_p. */
- init = build (MODIFY_EXPR, void_type_node, temp, init);
+ if (!VOID_TYPE_P (TREE_TYPE (init)))
+ init = build (MODIFY_EXPR, void_type_node, temp, init);
simplify_expr (&init, pre_p, post_p, is_simple_stmt, fb_none);
add_tree (init, pre_p);
The fixes to the flowgraph code were needed to better handle the
presence of empty statements in the function. The inliner is now
inlining functions after they've been optimized by the tree
optimizers. This was confusing the flowgraph in various places.
Bootstrapped and tested on ppc, x86 and x86-64. treelang should
now bootstrap, but I still haven't looked at the problems with Ada.
Diego.
* gimplify.c (simplify_expr): Handle VECTOR_CST nodes.
* tree-cfg.c (make_blocks): Ignore empty statement containers.
Create a basic block before processing containers that only have
empty statements.
(make_loop_expr_blocks): Use the container instead of the statement
when setting NEXT_BLOCK_LINK.
(make_cond_expr_blocks): Likewise.
(make_switch_expr_blocks): Likewise.
(make_bind_expr_blocks): Likewise.
(successor_block): If the last statement of the block is the empty
statement, use its container to get NEXT_BLOCK_LINK.
(stmt_starts_bb_p): Return false if the statement is NULL.
* tree-pretty-print.c (dump_generic_node): Handle VECTOR_CST nodes.
* tree-simple.c (is_simple_const): Accept VECTOR_CST as constants.
* objc/objc-lang.c (LANG_HOOKS_TREE_INLINING_TREE_CHAIN_MATTERS_P):
Define.
--- gimplify.c 2003/04/06 23:45:14 1.1
+++ gimplify.c 2003/04/06 23:45:27
@@ -457,6 +457,7 @@ simplify_expr (expr_p, pre_p, post_p, si
case REAL_CST:
case STRING_CST:
case COMPLEX_CST:
+ case VECTOR_CST:
break;
/* FIXME make this a decl. */
--- tree-cfg.c 2003/04/06 23:45:14 1.1
+++ tree-cfg.c 2003/04/08 03:24:25
@@ -278,14 +278,28 @@ make_blocks (first_p, next_block_link, p
tree prev_stmt;
tree *stmt_p = tsi_container (i);
+ /* Ignore empty containers. */
+ if (*stmt_p == empty_stmt_node || *stmt_p == error_mark_node)
+ continue;
+
prev_stmt = stmt;
stmt = tsi_stmt (i);
+ /* If the statement starts a new basic block or if we have determined
+ in a previous pass that we need to create a new block for STMT, do
+ so now. */
+ if (start_new_block || stmt_starts_bb_p (stmt, prev_stmt))
+ {
+ bb = create_bb ();
+ start_new_block = false;
+ }
+
/* Set the block for the container of non-executable statements. */
if (stmt == NULL_TREE)
{
- if (bb && *stmt_p != empty_stmt_node)
+ if (bb)
append_stmt_to_bb (stmt_p, bb, parent_stmt);
+ last = *stmt_p;
continue;
}
@@ -293,15 +307,6 @@ make_blocks (first_p, next_block_link, p
NEXT_BLOCK_LINK (stmt) = NULL_TREE;
code = TREE_CODE (stmt);
- /* If the statement starts a new basic block or if we have determined
- in a previous pass that we need to create a new block for STMT, do
- so now. */
- if (start_new_block || stmt_starts_bb_p (stmt, prev_stmt))
- {
- bb = create_bb ();
- start_new_block = false;
- }
-
/* Now add STMT to BB and create the subgraphs for special statement
codes. */
append_stmt_to_bb (stmt_p, bb, parent_stmt);
@@ -380,9 +385,8 @@ make_blocks (first_p, next_block_link, p
last = stmt;
}
- /* If LAST_STMT is set, link it to NEXT_BLOCK_LINK. This allows making
- edges from the last block inside a lexical scope (see
- successor_block). */
+ /* If LAST is set, link it to NEXT_BLOCK_LINK. This allows making edges
+ from the last block inside a lexical scope (see successor_block). */
if (last)
{
NEXT_BLOCK_LINK (last) = next_block_link;
@@ -421,17 +425,17 @@ make_loop_expr_blocks (loop_p, next_bloc
LOOP_EXPR body, will naturally loop back. */
STRIP_CONTAINERS (loop);
si = tsi_start (&LOOP_EXPR_BODY (loop));
- next_block_link = tsi_stmt (si);
+ next_block_link = *(tsi_container (si));
/* If the loop body is empty, point NEXT_BLOCK_LINK to the statement
following the LOOP_EXPR node, as we do with the other control
structures. */
- if (next_block_link == NULL_TREE)
+ if (body_is_empty (LOOP_EXPR_BODY (loop)))
{
si = tsi_start (loop_p);
tsi_next (&si);
if (!tsi_end_p (si))
- next_block_link = tsi_stmt (si);
+ next_block_link = *(tsi_container (si));
}
make_blocks (&LOOP_EXPR_BODY (loop), next_block_link, loop, NULL);
@@ -461,7 +465,7 @@ make_cond_expr_blocks (cond_p, next_bloc
si = tsi_start (cond_p);
tsi_next (&si);
if (!tsi_end_p (si))
- next_block_link = tsi_stmt (si);
+ next_block_link = *(tsi_container (si));
STRIP_CONTAINERS (cond);
make_blocks (&COND_EXPR_THEN (cond), next_block_link, cond, NULL);
@@ -492,7 +496,7 @@ make_switch_expr_blocks (switch_e_p, nex
si = tsi_start (switch_e_p);
tsi_next (&si);
if (!tsi_end_p (si))
- next_block_link = tsi_stmt (si);
+ next_block_link = *(tsi_container (si));
STRIP_CONTAINERS (switch_e);
make_blocks (&SWITCH_BODY (switch_e), next_block_link, switch_e, NULL);
@@ -531,7 +535,7 @@ make_bind_expr_blocks (bind_p, next_bloc
si = tsi_start (bind_p);
tsi_next (&si);
if (!tsi_end_p (si))
- next_block_link = tsi_stmt (si);
+ next_block_link = *(tsi_container (si));
/* By passing the current block ENTRY to make_blocks, we will keep adding
statements to ENTRY until we find a block terminating statement inside
@@ -1103,17 +1107,9 @@ remove_bb (bb, remove_stmts)
remove_edge (bb->pred);
}
+ /* Remove edges to BB's successors. */
while (bb->succ != NULL)
- {
- tree phi;
-
- /* PHI nodes in successors of this block now have one less
- alternative. */
- for (phi = phi_nodes (bb->succ->dest); phi; phi = TREE_CHAIN (phi))
- remove_phi_arg (phi, bb);
-
- remove_edge (bb->succ);
- }
+ ssa_remove_edge (bb->succ);
bb->pred = NULL;
bb->succ = NULL;
@@ -1384,15 +1380,7 @@ cleanup_cond_expr_graph (bb)
{
next = e->succ_next;
if (e != taken_edge)
- {
- tree phi;
-
- /* Remove the appropriate PHI alternative in the
- target block for each non executable edge. */
- for (phi = phi_nodes (e->dest); phi; phi = TREE_CHAIN (phi))
- remove_phi_arg (phi, e->dest);
- remove_edge (e);
- }
+ ssa_remove_edge (e);
}
}
}
@@ -1431,15 +1419,7 @@ cleanup_switch_expr_graph (switch_bb)
basic_block chain_bb = successor_block (switch_bb);
edge e = find_edge (switch_bb, chain_bb);
if (e)
- {
- tree phi;
-
- /* Remove the appropriate PHI alternative in the
- target block for each unexecutable edge. */
- for (phi = phi_nodes (e->dest); phi; phi = TREE_CHAIN (phi))
- remove_phi_arg (phi, e->src);
- remove_edge (e);
- }
+ ssa_remove_edge (e);
break;
}
}
@@ -1474,15 +1454,7 @@ disconnect_unreachable_case_labels (bb)
{
next = e->succ_next;
if (e != taken_edge)
- {
- tree phi;
-
- /* Remove the appropriate PHI alternative in the
- target block for each non executable edge. */
- for (phi = phi_nodes (e->dest); phi; phi = TREE_CHAIN (phi))
- remove_phi_arg (phi, e->src);
- remove_edge (e);
- }
+ ssa_remove_edge (e);
}
}
}
@@ -1749,6 +1721,7 @@ dump_tree_bb (outf, prefix, bb, indent)
char *s_indent;
basic_block loop_bb;
block_stmt_iterator si;
+ tree phi;
s_indent = (char *) alloca ((size_t) indent + 1);
memset ((void *) s_indent, ' ', (size_t) indent);
@@ -1792,6 +1765,14 @@ dump_tree_bb (outf, prefix, bb, indent)
else
fprintf (outf, "nil\n");
+ if (bb->aux)
+ for (phi = phi_nodes (bb); phi; phi = TREE_CHAIN (phi))
+ {
+ fprintf (outf, "%s%s# ", s_indent, prefix);
+ print_generic_stmt (outf, phi, 0);
+ fprintf (outf, "\n");
+ }
+
for (si = bsi_start (bb); !bsi_end_p (si); bsi_next (&si))
{
fprintf (outf, "%s%s%d ", s_indent, prefix, get_lineno (bsi_stmt (si)));
@@ -2062,7 +2043,10 @@ successor_block (bb)
NEXT_BLOCK_LINK for BB's last statement. If NEXT_BLOCK_LINK is still
NULL, then BB is the last basic block in the function. In which case
we have reached the end of the flowgraph and return EXIT_BLOCK_PTR. */
- if (last_stmt && NEXT_BLOCK_LINK (last_stmt))
+ if (last_stmt == NULL_TREE)
+ last_stmt = *bb->end_tree_p;
+
+ if (NEXT_BLOCK_LINK (last_stmt))
return bb_for_stmt (NEXT_BLOCK_LINK (last_stmt));
else
return EXIT_BLOCK_PTR;
@@ -2181,12 +2165,16 @@ stmt_starts_bb_p (t, prev_t)
tree t;
tree prev_t;
{
- enum tree_code code = TREE_CODE (t);
-
+ enum tree_code code;
+
+ if (t == NULL_TREE)
+ return false;
+
/* LABEL_EXPRs and CASE_LABEL_EXPRs start a new basic block only if the
preceding statement wasn't a label of the same type. This prevents
the creation of consecutive blocks that have nothing but a single
label. */
+ code = TREE_CODE (t);
if (code == LABEL_EXPR || code == CASE_LABEL_EXPR)
{
if (prev_t && TREE_CODE (prev_t) == code)
@@ -3126,6 +3114,60 @@ bsi_insert_on_edge (e, stmt)
}
+/* These 2 routines are used to process BSI's in reverse within a block.
+ When there is a decent implementation of bsi_prev, we can get rid of
+ these forever! */
+
+/* Push another block_stmt_iterator onto the stack. */
+void
+push_bsi (list, bsi)
+ bsi_list_p *list;
+ block_stmt_iterator bsi;
+{
+ bsi_list_p tmp;
+ if (*list == NULL)
+ {
+ *list = new_bsi_list ();
+ (*list)->bsi[0] = bsi;
+ }
+ else
+ {
+ if ((*list)->curr_index == (BSI_NUM_ELEMENTS - 1))
+ {
+ tmp = new_bsi_list ();
+ tmp->bsi[0] = bsi;
+ tmp->next = *list;
+ *list = tmp;
+ }
+ else
+ {
+ (*list)->bsi[++((*list)->curr_index)] = bsi;
+ }
+ }
+}
+
+/* Pop a block_stmt_iterator off the stack. */
+block_stmt_iterator
+pop_bsi (list)
+ bsi_list_p *list;
+{
+ block_stmt_iterator bsi;
+ bsi_list_p tmp;
+ if (!list)
+ abort ();
+
+ tmp = *list;
+ bsi = tmp->bsi[(tmp->curr_index)--];
+ if (tmp->curr_index< 0)
+ {
+ tmp = *list;
+ *list = (*list)->next;
+ free (tmp);
+ }
+ return bsi;
+}
+
+
/* Replace the statement pointed by TP1 with the statement pointed by TP2.
Note that this function will not replace COMPOUND_EXPR nodes, only
individual statements.
@@ -3216,13 +3258,29 @@ merge_tree_blocks (basic_block bb1, basi
/* BB2's successors are now BB1's. */
while (bb1->succ)
- remove_edge (bb1->succ);
+ ssa_remove_edge (bb1->succ);
while (bb2->succ)
{
- edge e = bb2->succ;
- make_edge (bb1, e->dest, e->flags);
- remove_edge (e);
+ tree phi;
+ edge new_edge, old_edge;
+
+ old_edge = bb2->succ;
+ new_edge = make_edge (bb1, old_edge->dest, old_edge->flags);
+
+ /* Update PHI nodes at BB2's successor. The arguments that used to
+ come from BB2 now come from BB1. */
+ for (phi = phi_nodes (old_edge->dest); phi; phi = TREE_CHAIN (phi))
+ {
+ int i;
+ for (i = 0; i < PHI_NUM_ARGS (phi); i++)
+ if (PHI_ARG_EDGE (phi, i) == old_edge)
+ PHI_ARG_EDGE (phi, i) = new_edge;
+ }
+
+ /* Note that we shouldn't call ssa_remove_edge here because we've
+ already dealt with PHI nodes. */
+ remove_edge (old_edge);
}
/* BB2's dominator children are now BB1's. Also, remove BB2 as a
--- tree-pretty-print.c 2003/04/06 23:45:14 1.1
+++ tree-pretty-print.c 2003/04/07 01:37:51
@@ -37,7 +37,7 @@ static const char *op_symbol PARAMS ((t
static void pretty_print_string PARAMS ((output_buffer *, const char*));
static void print_call_name PARAMS ((output_buffer *, tree));
static void newline_and_indent PARAMS ((output_buffer *, int));
-static inline void maybe_init_pretty_print PARAMS ((void));
+static void maybe_init_pretty_print PARAMS ((void));
static void print_declaration PARAMS ((output_buffer *, tree, int,
int));
static void print_struct_decl PARAMS ((output_buffer *, tree, int));
@@ -477,6 +477,20 @@ dump_generic_node (buffer, node, spc, fl
output_add_string (buffer, "\"");
break;
+ case VECTOR_CST:
+ {
+ tree elt;
+ output_add_string (buffer, "{ ");
+ for (elt = TREE_VECTOR_CST_ELTS (node); elt; elt = TREE_CHAIN (elt))
+ {
+ dump_generic_node (buffer, TREE_VALUE (elt), spc, flags);
+ if (TREE_CHAIN (elt))
+ output_add_string (buffer, ", ");
+ }
+ output_add_string (buffer, " }");
+ }
+ break;
+
case FUNCTION_TYPE:
break;
@@ -1335,6 +1349,9 @@ dump_generic_node (buffer, node, spc, fl
for (i = 0; i < PHI_NUM_ARGS (node); i++)
{
dump_generic_node (buffer, PHI_ARG_DEF (node, i), spc, flags);
+ output_add_string (buffer, "(");
+ output_decimal (buffer, PHI_ARG_EDGE (node, i)->src->index);
+ output_add_string (buffer, ")");
if (i < PHI_NUM_ARGS (node) - 1)
output_add_string (buffer, ", ");
}
--- tree-simple.c 2003/04/06 23:45:14 1.1
+++ tree-simple.c 2003/04/06 23:45:27
@@ -649,7 +649,8 @@ is_simple_const (t)
|| TREE_CODE (t) == STRING_CST
|| TREE_CODE (t) == LABEL_DECL
|| TREE_CODE (t) == RESULT_DECL
- || TREE_CODE (t) == COMPLEX_CST);
+ || TREE_CODE (t) == COMPLEX_CST
+ || TREE_CODE (t) == VECTOR_CST);
}
int
--- objc/objc-lang.c 2 Mar 2003 19:59:14 -0000 1.24.2.5
+++ objc/objc-lang.c 8 Apr 2003 18:12:40 -0000
@@ -100,6 +100,9 @@ static void objc_init_options
#undef LANG_HOOKS_TREE_INLINING_CONVERT_PARM_FOR_INLINING
#define LANG_HOOKS_TREE_INLINING_CONVERT_PARM_FOR_INLINING \
c_convert_parm_for_inlining
+#undef LANG_HOOKS_TREE_INLINING_TREE_CHAIN_MATTERS_P
+#define LANG_HOOKS_TREE_INLINING_TREE_CHAIN_MATTERS_P \
+ c_tree_chain_matters_p
#undef LANG_HOOKS_CALLGRAPH_EXPAND_FUNCTION
#define LANG_HOOKS_CALLGRAPH_EXPAND_FUNCTION c_expand_body
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [tree-ssa] Mainline merge as of 2003-04-06
2003-04-09 20:37 [tree-ssa] Mainline merge as of 2003-04-06 Diego Novillo
@ 2003-04-09 23:59 ` law
2003-04-10 9:30 ` Diego Novillo
0 siblings, 1 reply; 6+ messages in thread
From: law @ 2003-04-09 23:59 UTC (permalink / raw)
To: Diego Novillo; +Cc: gcc
In message <20030409193634.GA24420@tornado.toronto.redhat.com>, Diego Novillo w
rites:
>The fixes to the flowgraph code were needed to better handle the
>presence of empty statements in the function. The inliner is now
>inlining functions after they've been optimized by the tree
>optimizers. This was confusing the flowgraph in various places.
Note it's still confusing things. Consider:
void __cxa_bad_cast() ()
{
void * <UVcf50>;
struct bad_cast * T.1;
void * _ZTISt8bad_cast.2;
# BLOCK 0 (k.C:13). PRED:. SUCC:.
{
<UVcf50> = __cxa_allocate_exception (1);
try
{
# BLOCK 1 (k.C:14). PRED:. SUCC:.
T.1 = (struct bad_cast *)<UVcf50>;
{
struct bad_cast * const this;
this = (struct bad_cast * const)T.1;
{
try
{
# BLOCK 2 (k.C:7). PRED:. SUCC:.
{
{
(void)0
}
}
}
catch
{
# BLOCK 3 (k.C:7). PRED:. SUCC:.
<<<eh_filter ()>>>
{
# BLOCK 4 (k.C:7). PRED:. SUCC:.
__cxa_call_unexpected (<<<exception object>>>)
}
}
}
};
(void)0
}
catch
{
# BLOCK 5 (k.C:14). PRED:. SUCC:.
<<<eh_filter ()>>>
{
# BLOCK 6 (k.C:14). PRED:. SUCC:.
terminate ()
}
};
# BLOCK 7 (k.C:14). PRED:. SUCC:.
_ZTISt8bad_cast.2 = &_ZTISt8bad_cast;
__cxa_throw (<UVcf50>, _ZTISt8bad_cast.2, 0B)
}
}
$17 = void
[ This is obviously before we add edges to the CFG. ]
Of particular interest is block #2. There are no executable statements in
this block (one could certainly argue that we shouldn't have created a
block in this case -- that's on my TODO list).
Anyway, we want to create an edge from block #2 to it's successor block.
However, it's not immediately clear what its successor block ought to
be. The code as it stands right now will try to make an edge to the
next statement in the parent's block -- but that happens to be an
empty_stmt_node which was ignored when building the CFG (ie, it has no
block). This causes us to segfault.
We really want to wire an edge from block #2 to block #7, but that
would require that we know how to get to the parent of the empty
statement node.
How is this supposed to work?
Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [tree-ssa] Mainline merge as of 2003-04-06
2003-04-09 23:59 ` law
@ 2003-04-10 9:30 ` Diego Novillo
2003-04-10 16:40 ` law
0 siblings, 1 reply; 6+ messages in thread
From: Diego Novillo @ 2003-04-10 9:30 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc
On Wed, 2003-04-09 at 18:54, law@redhat.com wrote:
> # BLOCK 1 (k.C:14). PRED:. SUCC:.
> T.1 = (struct bad_cast *)<UVcf50>;
> {
> struct bad_cast * const this;
>
> this = (struct bad_cast * const)T.1;
> {
> try
> {
>
> # BLOCK 2 (k.C:7). PRED:. SUCC:.
> {
> {
> (void)0
> }
> }
> }
> [ ... ]
>
> How is this supposed to work?
>
When you are about to make the recursive call to make_blocks to build
the blocks for the body of the TRY_EXPR, you should set the
NEXT_BLOCK_LINK argument to whichever statement should execute after
that body has executed. For instance, the handlers for COND_EXPR and
SWITCH_EXPR set it to the statement following the entry statement:
if ()
{
s1;
s2;
s3;
}
s4;
When make_cond_expr_blocks() calls make_blocks(), the argument
NEXT_BLOCK_LINK will be set to s4. This way, when making an edge out of
s3's block, make_edges will know that it should go to s4's block.
The main loop in make_blocks should be smart enough to use the BIND_EXPR
of that TRY_EXPR's body and store NEXT_BLOCK_LINK in the BIND_EXPR. To
visualize this, you can use this:
main()
{
int a;
a = foo();
if (a > 3)
{
{
}
}
a++;
}
If you want, I could take a look at the patch, but at first sight it
seems that you are only missing setting NEXT_BLOCK_LINK when building
the body of the TRY_EXPR.
Diego.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [tree-ssa] Mainline merge as of 2003-04-06
2003-04-10 9:30 ` Diego Novillo
@ 2003-04-10 16:40 ` law
2003-04-10 20:20 ` Diego Novillo
0 siblings, 1 reply; 6+ messages in thread
From: law @ 2003-04-10 16:40 UTC (permalink / raw)
To: Diego Novillo; +Cc: gcc
In message <1049944224.5236.145.camel@frodo.toronto.redhat.com>, Diego Novillo
writes:
>When you are about to make the recursive call to make_blocks to build
>the blocks for the body of the TRY_EXPR, you should set the
>NEXT_BLOCK_LINK argument to whichever statement should execute after
>that body has executed.
Remember, we're trying to get the correct NEXT_BLOCK_LINK for block #2 for
the following code:
# BLOCK 0 (k.C:13). PRED:. SUCC:.
{
<UVcf50> = __cxa_allocate_exception (1);
try
{
# BLOCK 1 (k.C:14). PRED:. SUCC:.
T.1 = (struct bad_cast *)<UVcf50>;
{
struct bad_cast * const this;
this = (struct bad_cast * const)T.1;
{
try
{
# BLOCK 2 (k.C:7). PRED:. SUCC:.
{
{
(void)0
}
}
}
catch
{
# BLOCK 3 (k.C:7). PRED:. SUCC:.
<<<eh_filter ()>>>
{
# BLOCK 4 (k.C:7). PRED:. SUCC:.
__cxa_call_unexpected (<<<exception object>>>)
}
}
}
};
1-> (void)0
}
catch
{
# BLOCK 5 (k.C:14). PRED:. SUCC:.
<<<eh_filter ()>>>
{
# BLOCK 6 (k.C:14). PRED:. SUCC:.
terminate ()
}
};
# BLOCK 7 (k.C:14). PRED:. SUCC:.
2-> _ZTISt8bad_cast.2 = &_ZTISt8bad_cast;
__cxa_throw (<UVcf50>, _ZTISt8bad_cast.2, 0B)
}
}
Should NEXT_BLOCK_LINK point to the statement at point #1 or point #2 in
the above code?
If it should point to #2, then all these little fragments in tree-cfg.c
must skip empty statements when trying to determine what NEXT_BLOCK_LINK
ought to be to catch cases were a tree ends with one or more empty
statement statements:
/* Determine NEXT_BLOCK_LINK for statements inside the COND_EXPR body. */
si = tsi_start (cond_p);
tsi_next (&si);
if (!tsi_end_p (si))
next_block_link = *(tsi_container (si));
Would skipping through empty statements in these loops potentially
cause problems elsewhere?
--
If NEXT_BLOCK_LINK should point to #1 above (which is what we do right
now as far as I can tell), then, well, I'm not sure how to proceed, but
I know it won't be particularly pretty
Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [tree-ssa] Mainline merge as of 2003-04-06
2003-04-10 16:40 ` law
@ 2003-04-10 20:20 ` Diego Novillo
2003-04-14 16:08 ` law
0 siblings, 1 reply; 6+ messages in thread
From: Diego Novillo @ 2003-04-10 20:20 UTC (permalink / raw)
To: law; +Cc: gcc
If it's any consolation, I think I just reproduced the problem
with a COND_EXPR:
-----------------------------------------------------------------------------
void foo(int x)
{
if (x > 3)
{;}
else
bar();
x = 9;
}
main()
{
int j;
foo(j);
return j;
}
-----------------------------------------------------------------------------
At -O3 we inline the optimized foo() into main(), producing:
-----------------------------------------------------------------------------
{
# BLOCK 0. PRED: -1. SUCC: 2 1.
{
int x;
x = j;
{
extern bar;
if (x > 3)
{
# BLOCK 1 (a.c:4). PRED: 0. SUCC:.
{
(void)0
}
}
else
{
# BLOCK 2 (a.c:6). PRED: 0. SUCC:.
bar ()
};
(void)0
}
};
# BLOCK 3 (a.c:15). PRED:. SUCC:.
return j;
}
-----------------------------------------------------------------------------
We die as we try to create an edge out of BLOCK 1.
> 2-> _ZTISt8bad_cast.2 = &_ZTISt8bad_cast;
> __cxa_throw (<UVcf50>, _ZTISt8bad_cast.2, 0B)
> }
> }
>
>
> Should NEXT_BLOCK_LINK point to the statement at point #1 or point #2 in
> the above code?
>
Well, there's no point for control to reach that empty statement
in #1. I think making it go to #2 is sensible.
> If it should point to #2, then all these little fragments in tree-cfg.c
> must skip empty statements when trying to determine what NEXT_BLOCK_LINK
> ought to be to catch cases were a tree ends with one or more empty
> statement statements:
>
> /* Determine NEXT_BLOCK_LINK for statements inside the COND_EXPR body. */
> si = tsi_start (cond_p);
> tsi_next (&si);
> if (!tsi_end_p (si))
> next_block_link = *(tsi_container (si));
>
> Would skipping through empty statements in these loops potentially
> cause problems elsewhere?
>
The interesting problem is what happens when we run out of
statements in the chain holding the COND_EXPR. This means that
we should have every statement in the body know about
NEXT_BLOCK_LINK.
Oh, hell. Now both our lives suck. You happy now? :)
Seriously, though. I think we should be compacting the
instruction stream as we go out of SSA. Having the flow graph
make all these contortions to accomodate empty_stmt_node is just
not worth it.
Thoughts? Diego.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [tree-ssa] Mainline merge as of 2003-04-06
2003-04-10 20:20 ` Diego Novillo
@ 2003-04-14 16:08 ` law
0 siblings, 0 replies; 6+ messages in thread
From: law @ 2003-04-14 16:08 UTC (permalink / raw)
To: Diego Novillo; +Cc: gcc
In message <20030410194619.GB2202@tornado.toronto.redhat.com>, Diego Novillo wr
ites:
>If it's any consolation, I think I just reproduced the problem
>with a COND_EXPR:
Not a great surprise, I was pretty sure it would be possible to create
a similar situation using just C code.
>> If it should point to #2, then all these little fragments in tree-cfg.c
>> must skip empty statements when trying to determine what NEXT_BLOCK_LINK
>> ought to be to catch cases were a tree ends with one or more empty
>> statement statements:
>>
>> /* Determine NEXT_BLOCK_LINK for statements inside the COND_EXPR body. *
>/
>> si = tsi_start (cond_p);
>> tsi_next (&si);
>> if (!tsi_end_p (si))
>> next_block_link = *(tsi_container (si));
>>
>> Would skipping through empty statements in these loops potentially
>> cause problems elsewhere?
>>
>The interesting problem is what happens when we run out of
>statements in the chain holding the COND_EXPR. This means that
>we should have every statement in the body know about
>NEXT_BLOCK_LINK.
Sorry, I don't follow why every statement in the body would need to
know about NEXT_BLOCK_LINK. It seems to me you replace the code above
with something like this:
/* Determine NEXT_BLOCK_LINK for statements inside the body. */
si = tsi_start (expr_p);
tsi_next (&si);
/* Ignore any empty statements at the tail of this tree. */
while (!tsi_end_p (si) && tsi_stmt (si) == NULL)
tsi_next (&si);
if (!tsi_end_p (si) && tsi_stmt (si) != NULL_TREE)
next_block_link = *(tsi_container (si));
The basic idea being to first skip any empty statements at the tail of a
tree. After skipping, if we are not at the end of the tree and we have
a real statement, then NEXT_BLOCK_LINK would be that real statement.
Otherwise NEXT_BLOCK_LINK comes from the parent tree via the
next_block_link argument that's passed into these functions.
>Seriously, though. I think we should be compacting the
>instruction stream as we go out of SSA. Having the flow graph
>make all these contortions to accomodate empty_stmt_node is just
>not worth it.
Compacting the tree is probably a good idea, but my gut tells me that
there are probably going to be cases where getting rid of the empty
statement nodes may require some unpleasant contortions.
Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2003-04-14 15:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-04-09 20:37 [tree-ssa] Mainline merge as of 2003-04-06 Diego Novillo
2003-04-09 23:59 ` law
2003-04-10 9:30 ` Diego Novillo
2003-04-10 16:40 ` law
2003-04-10 20:20 ` Diego Novillo
2003-04-14 16:08 ` law
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).