public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Assigning correct source location for deallocator
@ 2012-10-08  0:38 Dehao Chen
  2012-10-09 19:35 ` Jason Merrill
  0 siblings, 1 reply; 7+ messages in thread
From: Dehao Chen @ 2012-10-08  0:38 UTC (permalink / raw)
  To: GCC Patches; +Cc: Richard Guenther

[-- Attachment #1: Type: text/plain, Size: 666 bytes --]

Hi,

R191338 did not completely fix the location for deallocator. This
patch covers more cases for deallocator.

Bootstrapped and passed gcc regression test on x86.

Okay for trunk?

Thanks,
Dehao

gcc/ChangeLog:

2012-10-07  Dehao Chen  <dehao@google.com>

* tree-eh.c (lower_try_finally_onedest): Set correct location for
deallocator.
* gimplify.c (gimplify_expr): Set correct location for TRY stmt.

gcc/cp/ChangeLog:

2012-10-07  Dehao Chen  <dehao@google.com>

* cp-gimplify.c (cp_genericize_r): Set location for TRY expr.

gcc/testsuite/ChangeLog:

2012-10-07  Dehao Chen  <dehao@google.com>

* g++.dg/debug/dwarf2/deallocator.C: Cover more deallocator cases.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3340 bytes --]

Index: gcc/cp/cp-gimplify.c
===================================================================
--- gcc/cp/cp-gimplify.c	(revision 192168)
+++ gcc/cp/cp-gimplify.c	(working copy)
@@ -948,11 +948,12 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees,
      to lower this construct before scanning it, so we need to lower these
      before doing anything else.  */
   else if (TREE_CODE (stmt) == CLEANUP_STMT)
-    *stmt_p = build2 (CLEANUP_EH_ONLY (stmt) ? TRY_CATCH_EXPR
-					     : TRY_FINALLY_EXPR,
-		      void_type_node,
-		      CLEANUP_BODY (stmt),
-		      CLEANUP_EXPR (stmt));
+    *stmt_p = build2_loc (input_location,
+			  CLEANUP_EH_ONLY (stmt) ? TRY_CATCH_EXPR
+						 : TRY_FINALLY_EXPR,
+			  void_type_node,
+			  CLEANUP_BODY (stmt),
+			  CLEANUP_EXPR (stmt));
 
   else if (TREE_CODE (stmt) == IF_STMT)
     {
Index: gcc/gimplify.c
===================================================================
--- gcc/gimplify.c	(revision 192168)
+++ gcc/gimplify.c	(working copy)
@@ -7475,6 +7475,10 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gi
 				     TREE_CODE (*expr_p) == TRY_FINALLY_EXPR
 				     ? GIMPLE_TRY_FINALLY
 				     : GIMPLE_TRY_CATCH);
+	    if (LOCATION_LOCUS (saved_location) != UNKNOWN_LOCATION)
+	      gimple_set_location (try_, saved_location);
+	    else
+	      gimple_set_location (try_, EXPR_LOCATION (save_expr));
 	    if (TREE_CODE (*expr_p) == TRY_CATCH_EXPR)
 	      gimple_try_set_catch_is_cleanup (try_,
 					       TRY_CATCH_IS_CLEANUP (*expr_p));
Index: gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C
===================================================================
--- gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C	(revision 192168)
+++ gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C	(working copy)
@@ -18,6 +18,7 @@ int bar();
 
 void foo(int i)
 {
+  t test_outside;
   for (int j = 0; j < 10; j++)
     {
       t test;
@@ -28,6 +29,18 @@ void foo(int i)
 	  return;
 	}
     }
+  if (i)
+    {
+      t test;
+      if (i == 10)
+	{
+	  test.bar();
+	}
+    }
+  test_outside.foo();
   return;
 }
-// { dg-final { scan-assembler "deallocator.C:28" } }
+// { dg-final { scan-assembler "deallocator.C:29" } }
+// { dg-final { scan-assembler "deallocator.C:31" } }
+// { dg-final { scan-assembler "deallocator.C:38" } }
+// { dg-final { scan-assembler "deallocator.C:41" } }
Index: gcc/tree-eh.c
===================================================================
--- gcc/tree-eh.c	(revision 192168)
+++ gcc/tree-eh.c	(working copy)
@@ -1100,6 +1100,7 @@ lower_try_finally_onedest (struct leh_state *state
   struct goto_queue_node *q, *qe;
   gimple x;
   gimple_seq finally;
+  gimple_stmt_iterator gsi;
   tree finally_label;
   location_t loc = gimple_location (tf->try_finally_expr);
 
@@ -1120,6 +1121,17 @@ lower_try_finally_onedest (struct leh_state *state
 
   lower_eh_constructs_1 (state, &finally);
 
+  for (gsi = gsi_start (finally); !gsi_end_p (gsi); gsi_next (&gsi))
+    {
+      gimple stmt = gsi_stmt (gsi);
+      if (LOCATION_LOCUS (gimple_location (stmt)) == UNKNOWN_LOCATION)
+	{
+	  tree block = gimple_block (stmt);
+	  gimple_set_location (stmt, gimple_location (tf->try_finally_expr));
+	  gimple_set_block (stmt, block);
+	}
+    }
+
   if (tf->may_throw)
     {
       /* Only reachable via the exception edge.  Add the given label to

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Assigning correct source location for deallocator
  2012-10-08  0:38 [PATCH] Assigning correct source location for deallocator Dehao Chen
@ 2012-10-09 19:35 ` Jason Merrill
  2012-10-09 19:48   ` Jason Merrill
  2012-10-09 20:36   ` Dehao Chen
  0 siblings, 2 replies; 7+ messages in thread
From: Jason Merrill @ 2012-10-09 19:35 UTC (permalink / raw)
  To: Dehao Chen; +Cc: GCC Patches, Richard Guenther

On 10/07/2012 08:38 PM, Dehao Chen wrote:
> +    *stmt_p = build2_loc (input_location,

I think input_location in cp_genericize_r will always be the closing 
brace of the function, which might be right for a variable in the 
outermost block of the function, but not for variables in inner scopes.

Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Assigning correct source location for deallocator
  2012-10-09 19:35 ` Jason Merrill
@ 2012-10-09 19:48   ` Jason Merrill
  2012-10-09 20:36   ` Dehao Chen
  1 sibling, 0 replies; 7+ messages in thread
From: Jason Merrill @ 2012-10-09 19:48 UTC (permalink / raw)
  To: gcc-patches; +Cc: GCC Patches, Richard Guenther

On 10/07/2012 08:38 PM, Dehao Chen wrote:
> +    *stmt_p = build2_loc (input_location,

I think input_location in cp_genericize_r will always be the closing 
brace of the function, which might be right for a variable in the 
outermost block of the function, but not for variables in inner scopes.

Jason


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Assigning correct source location for deallocator
  2012-10-09 19:35 ` Jason Merrill
  2012-10-09 19:48   ` Jason Merrill
@ 2012-10-09 20:36   ` Dehao Chen
  2012-10-09 21:20     ` Dehao Chen
  2012-10-10  1:01     ` Jason Merrill
  1 sibling, 2 replies; 7+ messages in thread
From: Dehao Chen @ 2012-10-09 20:36 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches, Richard Guenther

[-- Attachment #1: Type: text/plain, Size: 532 bytes --]

Yes, you are right. I've changed to use EXPR_LOCATION (stmt) for the location.

New patch attached, testing is on-going.

Thanks,
Dehao

On Tue, Oct 9, 2012 at 12:35 PM, Jason Merrill <jason@redhat.com> wrote:
> On 10/07/2012 08:38 PM, Dehao Chen wrote:
>>
>> +    *stmt_p = build2_loc (input_location,
>
>
> I think input_location in cp_genericize_r will always be the closing brace
> of the function, which might be right for a variable in the outermost block
> of the function, but not for variables in inner scopes.
>
> Jason
>

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3346 bytes --]

Index: gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C
===================================================================
--- gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C	(revision 192168)
+++ gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C	(working copy)
@@ -18,6 +18,7 @@ int bar();
 
 void foo(int i)
 {
+  t test_outside;
   for (int j = 0; j < 10; j++)
     {
       t test;
@@ -28,6 +29,18 @@ void foo(int i)
 	  return;
 	}
     }
+  if (i)
+    {
+      t test;
+      if (i == 10)
+	{
+	  test.bar();
+	}
+    }
+  test_outside.foo();
   return;
 }
-// { dg-final { scan-assembler "deallocator.C:28" } }
+// { dg-final { scan-assembler "deallocator.C:29" } }
+// { dg-final { scan-assembler "deallocator.C:31" } }
+// { dg-final { scan-assembler "deallocator.C:38" } }
+// { dg-final { scan-assembler "deallocator.C:41" } }
Index: gcc/gimplify.c
===================================================================
--- gcc/gimplify.c	(revision 192168)
+++ gcc/gimplify.c	(working copy)
@@ -7475,6 +7475,10 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gi
 				     TREE_CODE (*expr_p) == TRY_FINALLY_EXPR
 				     ? GIMPLE_TRY_FINALLY
 				     : GIMPLE_TRY_CATCH);
+	    if (LOCATION_LOCUS (saved_location) != UNKNOWN_LOCATION)
+	      gimple_set_location (try_, saved_location);
+	    else
+	      gimple_set_location (try_, EXPR_LOCATION (save_expr));
 	    if (TREE_CODE (*expr_p) == TRY_CATCH_EXPR)
 	      gimple_try_set_catch_is_cleanup (try_,
 					       TRY_CATCH_IS_CLEANUP (*expr_p));
Index: gcc/tree-eh.c
===================================================================
--- gcc/tree-eh.c	(revision 192168)
+++ gcc/tree-eh.c	(working copy)
@@ -1100,6 +1100,7 @@ lower_try_finally_onedest (struct leh_state *state
   struct goto_queue_node *q, *qe;
   gimple x;
   gimple_seq finally;
+  gimple_stmt_iterator gsi;
   tree finally_label;
   location_t loc = gimple_location (tf->try_finally_expr);
 
@@ -1120,6 +1121,17 @@ lower_try_finally_onedest (struct leh_state *state
 
   lower_eh_constructs_1 (state, &finally);
 
+  for (gsi = gsi_start (finally); !gsi_end_p (gsi); gsi_next (&gsi))
+    {
+      gimple stmt = gsi_stmt (gsi);
+      if (LOCATION_LOCUS (gimple_location (stmt)) == UNKNOWN_LOCATION)
+	{
+	  tree block = gimple_block (stmt);
+	  gimple_set_location (stmt, gimple_location (tf->try_finally_expr));
+	  gimple_set_block (stmt, block);
+	}
+    }
+
   if (tf->may_throw)
     {
       /* Only reachable via the exception edge.  Add the given label to
Index: gcc/cp/cp-gimplify.c
===================================================================
--- gcc/cp/cp-gimplify.c	(revision 192168)
+++ gcc/cp/cp-gimplify.c	(working copy)
@@ -948,11 +948,12 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees,
      to lower this construct before scanning it, so we need to lower these
      before doing anything else.  */
   else if (TREE_CODE (stmt) == CLEANUP_STMT)
-    *stmt_p = build2 (CLEANUP_EH_ONLY (stmt) ? TRY_CATCH_EXPR
-					     : TRY_FINALLY_EXPR,
-		      void_type_node,
-		      CLEANUP_BODY (stmt),
-		      CLEANUP_EXPR (stmt));
+    *stmt_p = build2_loc (EXPR_LOCATION (stmt),
+			  CLEANUP_EH_ONLY (stmt) ? TRY_CATCH_EXPR
+						 : TRY_FINALLY_EXPR,
+			  void_type_node,
+			  CLEANUP_BODY (stmt),
+			  CLEANUP_EXPR (stmt));
 
   else if (TREE_CODE (stmt) == IF_STMT)
     {

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Assigning correct source location for deallocator
  2012-10-09 20:36   ` Dehao Chen
@ 2012-10-09 21:20     ` Dehao Chen
  2012-10-10  1:01     ` Jason Merrill
  1 sibling, 0 replies; 7+ messages in thread
From: Dehao Chen @ 2012-10-09 21:20 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches, Richard Guenther

The patch bootstrapped and passed gcc regression tests.

Thanks,
Dehao

On Tue, Oct 9, 2012 at 1:16 PM, Dehao Chen <dehao@google.com> wrote:
> Yes, you are right. I've changed to use EXPR_LOCATION (stmt) for the location.
>
> New patch attached, testing is on-going.
>
> Thanks,
> Dehao
>
> On Tue, Oct 9, 2012 at 12:35 PM, Jason Merrill <jason@redhat.com> wrote:
>> On 10/07/2012 08:38 PM, Dehao Chen wrote:
>>>
>>> +    *stmt_p = build2_loc (input_location,
>>
>>
>> I think input_location in cp_genericize_r will always be the closing brace
>> of the function, which might be right for a variable in the outermost block
>> of the function, but not for variables in inner scopes.
>>
>> Jason
>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Assigning correct source location for deallocator
  2012-10-09 20:36   ` Dehao Chen
  2012-10-09 21:20     ` Dehao Chen
@ 2012-10-10  1:01     ` Jason Merrill
  2012-10-10 21:44       ` Dehao Chen
  1 sibling, 1 reply; 7+ messages in thread
From: Jason Merrill @ 2012-10-10  1:01 UTC (permalink / raw)
  To: Dehao Chen; +Cc: GCC Patches, Richard Guenther

OK.

Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Assigning correct source location for deallocator
  2012-10-10  1:01     ` Jason Merrill
@ 2012-10-10 21:44       ` Dehao Chen
  0 siblings, 0 replies; 7+ messages in thread
From: Dehao Chen @ 2012-10-10 21:44 UTC (permalink / raw)
  To: GCC Patches; +Cc: c-compiler-team

This patch was committed and ported to google-4_7 branch.

Thanks,
Dehao

gcc/ChangeLog:

2012-10-07  Dehao Chen  <dehao@google.com>

* tree-eh.c (lower_try_finally_onedest): Set correct location for
deallocator.
* gimplify.c (gimplify_expr): Set correct location for TRY stmt.

gcc/cp/ChangeLog:

2012-10-07  Dehao Chen  <dehao@google.com>

* cp-gimplify.c (cp_genericize_r): Set location for TRY expr.

gcc/testsuite/ChangeLog:

2012-10-07  Dehao Chen  <dehao@google.com>

* g++.dg/debug/dwarf2/deallocator.C: Cover more deallocator cases.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-10-10 21:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-08  0:38 [PATCH] Assigning correct source location for deallocator Dehao Chen
2012-10-09 19:35 ` Jason Merrill
2012-10-09 19:48   ` Jason Merrill
2012-10-09 20:36   ` Dehao Chen
2012-10-09 21:20     ` Dehao Chen
2012-10-10  1:01     ` Jason Merrill
2012-10-10 21:44       ` Dehao Chen

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