This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

PATCH: Improve debuggability at -O0 and fix PR 29609


Hi,

this patch improve debuggability at -O0 and fixes PR 29609.

For break/continue/goto statements directly inside an if statement the
code generated is too optimized since tree-ssa.  On x86:

    if (!f3())
        goto failure; /* Line 23 */

is translated to:

        call    f3
        testl   %eax, %eax
        je      .L6

and therefore there is no possible breakpoint at line 23 which is a mess
at -O0.

This patch first add a new flag to GOTO_EXPR: GOTO_EXPLICIT_P
(field static_flag) so that gotos coming explicitely from user code can be
tracked. During gimple lower pass and only at -O0 the explicit gotos are
not merged with the conditionnal jump created by an if statement.


This is just enough to generate correct code but not yet enough to have to
correct debug line opcode generated. The jump insn may be removed by the
into_cfglayout pass. To avoid this issue a new flag is added to edge flags:
EDGE_EXPLICIT (only at -O0). When this flag is set cfgrtl doesn't remove
the jump and cfgcleanup does not try to remove this forwarding edge.


Now the code generated at -O0 -g is:

        .loc 1 22 0
        call    f3
        testl   %eax, %eax
        jne     .L6
        .loc 1 23 0
        jmp     .L7

Tristan.

Changelog:
2007-08-31  Tristan Gingold  <gingold@adacore.com>

	PR middle-end/29609
	* tree.h (GOTO_EXPLICIT_P): New macro.
	* gimple-low.c (lower_cond_expr): Create goto even for branch
	containing only a simple explicit goto.
	* c-typeck.c (c_finish_goto_label): Set GOTO_EXPLICIT_P.
	(c_finish_goto_ptr): Likewise.
	(c_finish_bc_stmt): Likewise.
	* cfgexpand.c (expand_gimple_basic_block): Set location before
	emitting jump (and not after).
	* cfgcleanup.c (try_forward_edges): Do not forward if an edge is
	explicit.
	(try_optimize_cfg): Ditto.
	* tree-flow.h: Prototype of simple_implicit_goto_p added.
	* basic-block.h (EDGE_EXPLICIT): New constant.
	(EDGE_ALL_FLAGS): Updated.
	* tree-cfg.c (make_goto_expr_edges): An explicit goto creates an
	explicit edge.
	(simple_implicit_goto_p): New function.
	* cfgrtl.c (try_redirect_by_replacing_jump): Do not replace explicit
	gotos.
	(rtl_verify_flow_info_1): Consider EDGE_EXPLICIT.

gcc/testsuite/Changelog:
2007-09-03  Tristan Gingold  <gingold@adacore.com>

	* gcc.df/cfgcleanup-1.c: New test.
	* gcc.df/cfgcleanup-2.c: New test.


Index: gcc/tree.h
===================================================================
--- gcc/tree.h (revision 126974)
+++ gcc/tree.h (working copy)
@@ -452,6 +452,8 @@ struct gimple_stmt GTY(())
CASE_LABEL_EXPR
CALL_CANNOT_INLINE_P in
CALL_EXPR
+ GOTO_EXPLICIT_P in
+ GOTO_EXPR
public_flag:
@@ -1149,6 +1151,11 @@ extern void omp_clause_range_check_faile
/* Used to mark a CALL_EXPR as not suitable for inlining. */
#define CALL_CANNOT_INLINE_P(NODE) ((NODE)->base.static_flag)
+/* Used to mark a GOTO_EXPR as coming directly from user code (either from
+ a goto statement or a break/continue statements. When optimization is
+ disabled user must be able to set a breakpoint on such statements. */
+#define GOTO_EXPLICIT_P(NODE) ((NODE)->base.static_flag)
+
/* In an expr node (usually a conversion) this means the node was made
implicitly and should not lead to any sort of warning. In a decl node,
warnings concerning the decl should be suppressed. This is used at
Index: gcc/gimple-low.c
===================================================================
--- gcc/gimple-low.c (revision 126974)
+++ gcc/gimple-low.c (working copy)
@@ -471,10 +471,10 @@ lower_cond_expr (tree_stmt_iterator *tsi
lower_stmt_body (else_branch, data);
then_goto = expr_only (then_branch);
- then_is_goto = then_goto && simple_goto_p (then_goto);
+ then_is_goto = then_goto && simple_implicit_goto_p (then_goto);
else_goto = expr_only (else_branch);
- else_is_goto = else_goto && simple_goto_p (else_goto);
+ else_is_goto = else_goto && simple_implicit_goto_p (else_goto);
if (!then_is_goto || !else_is_goto)
{
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c (revision 126974)
+++ gcc/c-typeck.c (working copy)
@@ -6872,6 +6872,7 @@ build_asm_expr (tree string, tree output
tree
c_finish_goto_label (tree label)
{
+ tree stmt;
tree decl = lookup_label (label);
if (!decl)
return NULL_TREE;
@@ -6912,7 +6913,9 @@ c_finish_goto_label (tree label)
}
TREE_USED (decl) = 1;
- return add_stmt (build1 (GOTO_EXPR, void_type_node, decl));
+ stmt = build1 (GOTO_EXPR, void_type_node, decl);
+ GOTO_EXPLICIT_P (stmt) = 1;
+ return add_stmt (stmt);
}
/* Generate a computed goto statement to EXPR. */
@@ -6920,10 +6923,14 @@ c_finish_goto_label (tree label)
tree
c_finish_goto_ptr (tree expr)
{
+ tree stmt;
+
if (pedantic)
pedwarn ("ISO C forbids %<goto *expr;%>");
expr = convert (ptr_type_node, expr);
- return add_stmt (build1 (GOTO_EXPR, void_type_node, expr));
+ stmt = build1 (GOTO_EXPR, void_type_node, expr);
+ GOTO_EXPLICIT_P (stmt) = 1;
+ return add_stmt (stmt);
}
/* Generate a C `return' statement. RETVAL is the expression for what
@@ -7320,6 +7327,7 @@ tree
c_finish_bc_stmt (tree *label_p, bool is_break)
{
bool skip;
+ tree stmt;
tree label = *label_p;
/* In switch statements break is sometimes stylistically used after
@@ -7359,7 +7367,9 @@ c_finish_bc_stmt (tree *label_p, bool is
if (skip)
return NULL_TREE;
- return add_stmt (build1 (GOTO_EXPR, void_type_node, label));
+ stmt = build1 (GOTO_EXPR, void_type_node, label);
+ GOTO_EXPLICIT_P (stmt) = 1;
+ return add_stmt (stmt);
}
/* A helper routine for c_process_expr_stmt and c_finish_stmt_expr. */
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c (revision 126974)
+++ gcc/cfgexpand.c (working copy)
@@ -1626,9 +1626,9 @@ expand_gimple_basic_block (basic_block b
if (e && e->dest != bb->next_bb)
{
- emit_jump (label_rtx_for_bb (e->dest));
if (e->goto_locus)
set_curr_insn_source_location (*e->goto_locus);
+ emit_jump (label_rtx_for_bb (e->dest));
e->flags &= ~EDGE_FALLTHRU;
}
Index: gcc/cfgcleanup.c
===================================================================
--- gcc/cfgcleanup.c (revision 126974)
+++ gcc/cfgcleanup.c (working copy)
@@ -430,11 +430,12 @@ try_forward_edges (int mode, basic_block
bool may_thread = first_pass | df_get_bb_dirty (b);
/* Skip complex edges because we don't know how to update them.
+ Skip explicit edge as we want to keep them.
Still handle fallthru edges, as we can succeed to forward fallthru
edge to the same place as the branch edge of conditional branch
and turn conditional branch to an unconditional branch. */
- if (e->flags & EDGE_COMPLEX)
+ if (e->flags & (EDGE_COMPLEX | EDGE_EXPLICIT))
{
ei_next (&ei);
continue;
@@ -464,7 +465,8 @@ try_forward_edges (int mode, basic_block
may_thread |= df_get_bb_dirty (target);
if (FORWARDER_BLOCK_P (target)
- && !(single_succ_edge (target)->flags & EDGE_CROSSING)
+ && !(single_succ_edge (target)->flags
+ & (EDGE_CROSSING | EDGE_EXPLICIT))
&& single_succ (target) != EXIT_BLOCK_PTR)
{
/* Bypass trivial infinite loops. */
@@ -2038,7 +2040,7 @@ try_optimize_cfg (int mode)
if (single_succ_p (b)
&& (s = single_succ_edge (b))
- && !(s->flags & EDGE_COMPLEX)
+ && !(s->flags & (EDGE_COMPLEX | EDGE_EXPLICIT))
&& (c = s->dest) != EXIT_BLOCK_PTR
&& single_pred_p (c)
&& b != c)
Index: gcc/tree-flow.h
===================================================================
--- gcc/tree-flow.h (revision 126974)
+++ gcc/tree-flow.h (working copy)
@@ -726,6 +726,7 @@ extern bool is_ctrl_stmt (tree);
extern bool is_ctrl_altering_stmt (tree);
extern bool computed_goto_p (tree);
extern bool simple_goto_p (tree);
+extern bool simple_implicit_goto_p (tree);
extern bool tree_can_make_abnormal_goto (tree);
extern basic_block single_noncomplex_succ (basic_block bb);
extern void tree_dump_bb (basic_block, FILE *, int);
Index: gcc/basic-block.h
===================================================================
--- gcc/basic-block.h (revision 126974)
+++ gcc/basic-block.h (working copy)
@@ -169,7 +169,8 @@ DEF_VEC_ALLOC_P(edge,heap);
#define EDGE_CROSSING 8192 /* Edge crosses between hot
and cold sections, when we
do partitioning. */
-#define EDGE_ALL_FLAGS 16383
+#define EDGE_EXPLICIT 16384 /* Edge is an explicit goto. */
+#define EDGE_ALL_FLAGS 32767
#define EDGE_COMPLEX (EDGE_ABNORMAL | EDGE_ABNORMAL_CALL | EDGE_EH)
Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c (revision 126974)
+++ gcc/tree-cfg.c (working copy)
@@ -807,7 +807,15 @@ make_goto_expr_edges (basic_block bb)
if (simple_goto_p (goto_t))
{
tree dest = GOTO_DESTINATION (goto_t);
- edge e = make_edge (bb, label_to_block (dest), EDGE_FALLTHRU);
+ int flags = EDGE_FALLTHRU;
+ edge e;
+
+ /* Marking an edge explicit prevent some optimizations (forwarding),
+ so do it only if not optimizing. */
+ if (!optimize && GOTO_EXPLICIT_P (goto_t))
+ flags |= EDGE_EXPLICIT;
+
+ e = make_edge (bb, label_to_block (dest), flags);
#ifdef USE_MAPPED_LOCATION
e->goto_locus = EXPR_LOCATION (goto_t);
#else
@@ -2481,6 +2489,12 @@ simple_goto_p (tree t)
&& TREE_CODE (GOTO_DESTINATION (t)) == LABEL_DECL);
}
+bool
+simple_implicit_goto_p (tree t)
+{
+ return simple_goto_p (t) && !GOTO_EXPLICIT_P (t);
+}
+
/* Return true if T can make an abnormal transfer of control flow.
Transfers of control flow associated with EH are excluded. */
Index: gcc/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c (revision 126974)
+++ gcc/cfgrtl.c (working copy)
@@ -700,6 +700,11 @@ try_redirect_by_replacing_jump (edge e,
|| BB_PARTITION (src) != BB_PARTITION (target))
return NULL;
+ /* We don't want to remove explicit jump when not optimizing otherwise
+ its location will be lost. */
+ if (!optimize && (e->flags & EDGE_EXPLICIT))
+ return NULL;
+
/* We can replace or remove a complex jump only when we have exactly
two edges. Also, if we have exactly one outgoing edge, we can
redirect that. */
@@ -1778,6 +1783,7 @@ rtl_verify_flow_info_1 (void)
| EDGE_CAN_FALLTHRU
| EDGE_IRREDUCIBLE_LOOP
| EDGE_LOOP_EXIT
+ | EDGE_EXPLICIT
| EDGE_CROSSING)) == 0)
n_branch++;


diff -rupN ../gcc/gcc/testsuite/gcc.dg/cfgcleanup-1.c gcc/testsuite/ gcc.dg/cfgcleanup-1.c
--- ../gcc/gcc/testsuite/gcc.dg/cfgcleanup-1.c 1970-01-01 01:00:00.000000000 +0100
+++ gcc/testsuite/gcc.dg/cfgcleanup-1.c 2007-08-03 09:17:01.000000000 +0200
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-g -O0" } */
+/* { dg-final { scan-assembler ".loc 1 15 0" { target i?86-*- linux* } } } */
+/* Test if the user can set a breakpoint on the break statement.
+ We simply check there is a line number debug info for this statement on
+ a very common platform. */
+void foc (void)
+{
+ int a, i;
+
+ for (i = 1; i <= 10; i++) {
+ if (i < 3)
+ a = 1;
+ else
+ break; /* line 15 */
+ a = 5;
+ }
+}
+
+int main(void)
+{
+ foc ();
+}
diff -rupN ../gcc/gcc/testsuite/gcc.dg/cfgcleanup-2.c gcc/testsuite/ gcc.dg/cfgcleanup-2.c
--- ../gcc/gcc/testsuite/gcc.dg/cfgcleanup-2.c 1970-01-01 01:00:00.000000000 +0100
+++ gcc/testsuite/gcc.dg/cfgcleanup-2.c 2007-08-03 09:16:25.000000000 +0200
@@ -0,0 +1,33 @@
+/* { dg-do compile } */
+/* { dg-options "-g -O0" } */
+/* { dg-final { scan-assembler ".loc 1 23 0" { target i?86-*- linux* } } } */
+/* Test if the user can set a breakpoint on the goto statement.
+ We simply check there is a line number debug info for this statement on
+ a very common platform. */
+int f3(void) /* this "fails", i.e. returns 0 */
+{
+ return 0;
+}
+
+int f4(void) /* this "succeeds", i.e. returns 1 */
+{
+ return 1;
+}
+
+
+int main(int argc, char *argv[])
+{
+ int c;
+
+ if (!f3())
+ goto failure; /* Line 23 */
+
+ if (!f4())
+ goto failure;
+
+ success:
+ return 0;
+
+ failure:
+ return 1;
+}



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]