Fix debug info for expr and jump stmt

Submitted by Dehao Chen on Oct. 30, 2012, 4:27 p.m.

Details

Message ID CAO2gOZVO3PZJjPtFVYKDMd80x9Rm1R6=8BA326on4rt1pe3HBQ@mail.gmail.com
State New
Headers show

Commit Message

Dehao Chen Oct. 30, 2012, 4:27 p.m.
On Tue, Oct 30, 2012 at 8:29 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, Oct 30, 2012 at 4:21 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Tue, Oct 30, 2012 at 04:15:38PM +0100, Richard Biener wrote:
>>> So maybe TER (well, those looking up the stmt) should pick the location
>>> from the TERed statement properly then?
>>
>> Perhaps, but Micha's patch doesn't do that.
>> But in that case IMHO it still would help to set all expr locations to
>> UNKNOWN_LOCATION during gimple lowering, to make it clear we ignore
>> the locations.
>
> Yes indeed.

I agree, but this looks like too bold a move at this point. Shall we
do that in 4.8?

BTW, I updated the patch to ensure pr43479.c works fine. The testing
is still on-going.

Dehao

gcc/ChangeLog:
2012-10-25  Dehao Chen  <dehao@google.com>

        * tree-eh.c (do_return_redirection): Set location for jump statement.
        (do_goto_redirection): Likewise.
        (frob_into_branch_around): Likewise.
        (lower_try_finally_nofallthru): Likewise.
        (lower_try_finally_copy): Likewise.
        (lower_try_finally_switch): Likewise.
        * expr.c (store_expr): Use current insn location instead of expr
        location.
        (expand_expr_real): Likewise.
        (expand_expr_real_1): Likewise.

gcc/testsuite/ChangeLog:
2012-10-25  Dehao Chen  <dehao@google.com>

        * g++.dg/debug/dwarf2/block.C: New testcase.

Comments

Dehao Chen Oct. 30, 2012, 4:30 p.m.
BTW, one thing I found confusing is that in expr.c, some code is for
frontend, while some are for rtl. Shall we separate them into two
files? And we don't expect to see EXPR_LOCATION in the rtl side.

Thanks,
Dehao
Dehao Chen Oct. 30, 2012, 5:27 p.m.
> gcc/ChangeLog:
> 2012-10-25  Dehao Chen  <dehao@google.com>
>
>         * tree-eh.c (do_return_redirection): Set location for jump statement.
>         (do_goto_redirection): Likewise.
>         (frob_into_branch_around): Likewise.
>         (lower_try_finally_nofallthru): Likewise.
>         (lower_try_finally_copy): Likewise.
>         (lower_try_finally_switch): Likewise.
>         * expr.c (store_expr): Use current insn location instead of expr
>         location.
>         (expand_expr_real): Likewise.
>         (expand_expr_real_1): Likewise.
>
> gcc/testsuite/ChangeLog:
> 2012-10-25  Dehao Chen  <dehao@google.com>
>
>         * g++.dg/debug/dwarf2/block.C: New testcase.

This patch bootstrapped and passed gcc regression tests.

Okay for trunk?

Thanks,
Dehao
Richard Guenther Oct. 31, 2012, 10 a.m.
On Tue, Oct 30, 2012 at 6:27 PM, Dehao Chen <dehao@google.com> wrote:
>> gcc/ChangeLog:
>> 2012-10-25  Dehao Chen  <dehao@google.com>
>>
>>         * tree-eh.c (do_return_redirection): Set location for jump statement.
>>         (do_goto_redirection): Likewise.
>>         (frob_into_branch_around): Likewise.
>>         (lower_try_finally_nofallthru): Likewise.
>>         (lower_try_finally_copy): Likewise.
>>         (lower_try_finally_switch): Likewise.
>>         * expr.c (store_expr): Use current insn location instead of expr
>>         location.
>>         (expand_expr_real): Likewise.
>>         (expand_expr_real_1): Likewise.
>>
>> gcc/testsuite/ChangeLog:
>> 2012-10-25  Dehao Chen  <dehao@google.com>
>>
>>         * g++.dg/debug/dwarf2/block.C: New testcase.
>
> This patch bootstrapped and passed gcc regression tests.
>
> Okay for trunk?

It looks sensible to me.  If nobody objects within 24h the patch is ok.

Thanks,
Richard.

> Thanks,
> Dehao
Jakub Jelinek Oct. 31, 2012, 10:02 a.m.
On Wed, Oct 31, 2012 at 11:00:26AM +0100, Richard Biener wrote:
> On Tue, Oct 30, 2012 at 6:27 PM, Dehao Chen <dehao@google.com> wrote:
> >> gcc/ChangeLog:
> >> 2012-10-25  Dehao Chen  <dehao@google.com>
> >>
> >>         * tree-eh.c (do_return_redirection): Set location for jump statement.
> >>         (do_goto_redirection): Likewise.
> >>         (frob_into_branch_around): Likewise.
> >>         (lower_try_finally_nofallthru): Likewise.
> >>         (lower_try_finally_copy): Likewise.
> >>         (lower_try_finally_switch): Likewise.
> >>         * expr.c (store_expr): Use current insn location instead of expr
> >>         location.
> >>         (expand_expr_real): Likewise.
> >>         (expand_expr_real_1): Likewise.
> >>
> >> gcc/testsuite/ChangeLog:
> >> 2012-10-25  Dehao Chen  <dehao@google.com>
> >>
> >>         * g++.dg/debug/dwarf2/block.C: New testcase.
> >
> > This patch bootstrapped and passed gcc regression tests.
> >
> > Okay for trunk?
> 
> It looks sensible to me.  If nobody objects within 24h the patch is ok.

Yeah.  But please also check gdb testsuite for this kind of patches.

	Jakub
Dehao Chen Oct. 31, 2012, 2:42 p.m.
> Yeah.  But please also check gdb testsuite for this kind of patches.

This patch also passed gdb testsuite.

Thanks,
Dehao

>
>         Jakub

Patch hide | download patch | download mbox

Index: testsuite/g++.dg/debug/dwarf2/block.C
===================================================================
--- testsuite/g++.dg/debug/dwarf2/block.C	(revision 0)
+++ testsuite/g++.dg/debug/dwarf2/block.C	(revision 0)
@@ -0,0 +1,29 @@ 
+// Compiler should not generate too many lexical blocks for this function.
+// { dg-do compile { target { i?86-*-* x86_64-*-* } } }
+// { dg-options "-O0 -fno-exceptions -g -dA" }
+
+union UElement {
+    void* pointer;
+    int integer;
+};
+struct UColToken {
+  unsigned source;
+  unsigned char **rulesToParseHdl;
+};
+
+int uhash_hashTokens(const union UElement k)
+{
+  int hash = 0;
+  struct UColToken *key = (struct UColToken *)k.pointer;
+  if (key != 0) {
+    int len = (key->source & 0xFF000000)>>24;
+    int inc = ((len - 32) / 32) + 1;
+    const unsigned char *p = (key->source & 0x00FFFFFF)
+			     + *(key->rulesToParseHdl);
+    const unsigned char *limit = p + len;
+    hash = *p + *limit;
+  }
+  return hash;
+}
+
+// { dg-final { scan-assembler-not "LBB10" } }
Index: tree-eh.c
===================================================================
--- tree-eh.c	(revision 192809)
+++ tree-eh.c	(working copy)
@@ -739,6 +739,7 @@  do_return_redirection (struct goto_queue_node *q,
     gimple_seq_add_seq (&q->repl_stmt, mod);
 
   x = gimple_build_goto (finlab);
+  gimple_set_location (x, q->location);
   gimple_seq_add_stmt (&q->repl_stmt, x);
 }
 
@@ -758,6 +759,7 @@  do_goto_redirection (struct goto_queue_node *q, tr
     gimple_seq_add_seq (&q->repl_stmt, mod);
 
   x = gimple_build_goto (finlab);
+  gimple_set_location (x, q->location);
   gimple_seq_add_stmt (&q->repl_stmt, x);
 }
 
@@ -857,6 +859,7 @@  frob_into_branch_around (gimple tp, eh_region regi
       if (!over)
 	over = create_artificial_label (loc);
       x = gimple_build_goto (over);
+      gimple_set_location (x, loc);
       gimple_seq_add_stmt (&cleanup, x);
     }
   gimple_seq_add_seq (&eh_seq, cleanup);
@@ -1085,6 +1088,7 @@  lower_try_finally_nofallthru (struct leh_state *st
 	  emit_post_landing_pad (&eh_seq, tf->region);
 
 	  x = gimple_build_goto (lab);
+	  gimple_set_location (x, gimple_location (tf->try_finally_expr));
 	  gimple_seq_add_stmt (&eh_seq, x);
 	}
     }
@@ -1223,6 +1227,7 @@  lower_try_finally_copy (struct leh_state *state, s
 
       tmp = lower_try_finally_fallthru_label (tf);
       x = gimple_build_goto (tmp);
+      gimple_set_location (x, tf_loc);
       gimple_seq_add_stmt (&new_stmt, x);
     }
 
@@ -1395,6 +1400,7 @@  lower_try_finally_switch (struct leh_state *state,
 
       tmp = lower_try_finally_fallthru_label (tf);
       x = gimple_build_goto (tmp);
+      gimple_set_location (x, tf_loc);
       gimple_seq_add_stmt (&switch_body, x);
     }
 
@@ -1423,6 +1429,7 @@  lower_try_finally_switch (struct leh_state *state,
       gimple_seq_add_stmt (&eh_seq, x);
 
       x = gimple_build_goto (finally_label);
+      gimple_set_location (x, tf_loc);
       gimple_seq_add_stmt (&eh_seq, x);
 
       tmp = build_int_cst (integer_type_node, eh_index);
Index: expr.c
===================================================================
--- expr.c	(revision 192809)
+++ expr.c	(working copy)
@@ -5030,7 +5030,7 @@  store_expr (tree exp, rtx target, int call_param_p
 {
   rtx temp;
   rtx alt_rtl = NULL_RTX;
-  location_t loc = EXPR_LOCATION (exp);
+  location_t loc = curr_insn_location ();
 
   if (VOID_TYPE_P (TREE_TYPE (exp)))
     {
@@ -7869,31 +7869,7 @@  expand_expr_real (tree exp, rtx target, enum machi
       return ret ? ret : const0_rtx;
     }
 
-  /* If this is an expression of some kind and it has an associated line
-     number, then emit the line number before expanding the expression.
-
-     We need to save and restore the file and line information so that
-     errors discovered during expansion are emitted with the right
-     information.  It would be better of the diagnostic routines
-     used the file/line information embedded in the tree nodes rather
-     than globals.  */
-  if (cfun && EXPR_HAS_LOCATION (exp))
-    {
-      location_t saved_location = input_location;
-      location_t saved_curr_loc = curr_insn_location ();
-      input_location = EXPR_LOCATION (exp);
-      set_curr_insn_location (input_location);
-
-      ret = expand_expr_real_1 (exp, target, tmode, modifier, alt_rtl);
-
-      input_location = saved_location;
-      set_curr_insn_location (saved_curr_loc);
-    }
-  else
-    {
-      ret = expand_expr_real_1 (exp, target, tmode, modifier, alt_rtl);
-    }
-
+  ret = expand_expr_real_1 (exp, target, tmode, modifier, alt_rtl);
   return ret;
 }
 
@@ -9250,8 +9226,15 @@  expand_expr_real_1 (tree exp, rtx target, enum mac
 	g = SSA_NAME_DEF_STMT (exp);
       if (g)
 	{
-	  rtx r = expand_expr_real (gimple_assign_rhs_to_tree (g), target,
-				    tmode, modifier, NULL);
+	  rtx r;
+	  location_t saved_loc = input_location;
+
+	  input_location = gimple_location (g);
+	  set_curr_insn_location (input_location);
+	  r = expand_expr_real (gimple_assign_rhs_to_tree (g), target,
+				tmode, modifier, NULL);
+	  input_location = saved_loc;
+	  set_curr_insn_location (saved_loc);
 	  if (REG_P (r) && !REG_EXPR (r))
 	    set_reg_attrs_for_decl_rtl (SSA_NAME_VAR (exp), r);
 	  return r;
@@ -9481,7 +9464,7 @@  expand_expr_real_1 (tree exp, rtx target, enum mac
 	       with non-BLKmode values.  */
 	    gcc_assert (GET_MODE (ret) != BLKmode);
 
-	    val = build_decl (EXPR_LOCATION (exp),
+	    val = build_decl (curr_insn_location (),
 			      VAR_DECL, NULL, TREE_TYPE (exp));
 	    DECL_ARTIFICIAL (val) = 1;
 	    DECL_IGNORED_P (val) = 1;