Patchwork Fix debug info for expr and jump stmt

login
register
mail settings
Submitter Dehao Chen
Date Oct. 30, 2012, 4:27 p.m.
Message ID <CAO2gOZVO3PZJjPtFVYKDMd80x9Rm1R6=8BA326on4rt1pe3HBQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/195537/
State New
Headers show

Comments

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.
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

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;