[PATCH] Update source location for PRE inserted stmt

[Dehao Chen]

Sorry, new patch attached...

On Tue, Oct 30, 2012 at 4:38 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Wed, Oct 31, 2012 at 12:00 AM, Dehao Chen wrote:
>> This patch aims to improve debugging of optimized code. It ensures
>> that PRE inserted statements have the same source location as the
>> statement at the insertion point, instead of UNKNOWN_LOCATION.
>
> Wrong patch attached.
>
> However, is it really better to have the location of the insertion
> point than to have UNKNOWN_LOCATION? It's not where the value is
> computed in the source program...

Setting it to UNKNOWN_LOCATION is expecting it to inherit source
location from its previous stmt. However, backend optimization could
optimize the previous stmt away, making the inserted stmt with random
location. This patch just enforce the location to be the same as
previous stmt while insertion, so that random stuff does not happen.

In general, we want to reduce the number of UNKNOWN_LOCATIONS emitted.
At least we do not want to see UNKNOWN_LOCATION at the beginning of
any BB.

Thanks,
Dehao
>
> Ciao!
> Steven
-------------- next part --------------
Index: gcc/tree-ssa-pre.c
===================================================================
--- gcc/tree-ssa-pre.c	(revision 192809)
+++ gcc/tree-ssa-pre.c	(working copy)
@@ -3039,6 +3039,20 @@ inhibit_phi_insertion (basic_block bb, pre_expr ex
   return false;
 }
 
+static void
+insert_into_pred_update_location (edge pred, gimple_seq stmts)
+{
+  gimple_stmt_iterator gsi;
+  gimple stmt = last_stmt (pred->src);
+  location_t location = stmt ? gimple_location (stmt)
+			     : UNKNOWN_LOCATION;
+  if (location != UNKNOWN_LOCATION)
+    for (gsi = gsi_start (stmts); !gsi_end_p (gsi); gsi_next (&gsi))
+      if (gimple_location (gsi_stmt (gsi)) == UNKNOWN_LOCATION)
+	gimple_set_location (gsi_stmt (gsi), location);
+  gsi_insert_seq_on_edge (pred, stmts);
+}
+
 /* Insert the to-be-made-available values of expression EXPRNUM for each
    predecessor, stored in AVAIL, into the predecessors of BLOCK, and
    merge the result with a phi node, given the same value number as
@@ -3094,7 +3108,7 @@ insert_into_preds_of_block (basic_block block, uns
 	  builtexpr = create_expression_by_pieces (bprime, eprime,
 						   &stmts, type);
 	  gcc_assert (!(pred->flags & EDGE_ABNORMAL));
-	  gsi_insert_seq_on_edge (pred, stmts);
+	  insert_into_pred_update_location (pred, stmts);
 	  VEC_replace (pre_expr, avail, pred->dest_idx,
 		       get_or_alloc_expr_for_name (builtexpr));
 	  insertions = true;
@@ -3132,7 +3146,7 @@ insert_into_preds_of_block (basic_block block, uns
 						SSA_NAME_VERSION (lhs));
 			      gimple_set_plf (stmt, NECESSARY, false);
 			    }
-			  gsi_insert_seq_on_edge (pred, stmts);
+			  insert_into_pred_update_location (pred, stmts);
 			}
 		      VEC_replace (pre_expr, avail, pred->dest_idx,
 				   get_or_alloc_expr_for_name (forcedexpr));
@@ -3177,7 +3191,7 @@ insert_into_preds_of_block (basic_block block, uns
 			bitmap_set_bit (inserted_exprs, SSA_NAME_VERSION (lhs));
 		      gimple_set_plf (stmt, NECESSARY, false);
 		    }
-		  gsi_insert_seq_on_edge (pred, stmts);
+		  insert_into_pred_update_location (pred, stmts);
 		}
 	      VEC_replace (pre_expr, avail, pred->dest_idx,
 			   get_or_alloc_expr_for_name (forcedexpr));
Index: gcc/testsuite/gcc.dg/debug/dwarf2/pre.c
===================================================================
--- gcc/testsuite/gcc.dg/debug/dwarf2/pre.c	(revision 0)
+++ gcc/testsuite/gcc.dg/debug/dwarf2/pre.c	(revision 0)
@@ -0,0 +1,18 @@
+// This test makes sure PRE will not optimize the debug info away.
+// { dg-do compile { target { i?86-*-* x86_64-*-* } } }
+// { dg-options "-O2 -g -dA" }
+extern int x;
+
+int abc (int *a)
+{
+  int ret = 0;
+
+  if (x > 0)
+    ret += *a;
+  else
+    a++;
+
+  ret += *a;
+  return ret;
+}
+// { dg-final { scan-assembler "pre.c:13" } }