[PR49888, VTA] don't keep VALUEs bound to modified MEMs

[PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Alexandre Oliva]

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

Nothing in var-tracking was prepared to drop MEMs from bindings upon
writes that might modify the MEM contents.  That was correct, back when
it was variables (rather than values) that were bound to MEMs, and we
wanted the binding to remain even if the value stored in the variables
changed.

VTA changed that for gimple regs: we now bind variables to values, and
values to locations that hold that value, so we must unbind them when
the value stored in the MEM location changes, just like we already did
for REGs.

cselib might offer us some help to that end, but only within individual
basic blocks.  That's not quite enough: even a trivial testcase, as
provided in the bug report, was enough to defeat the first approach I
tried, of emitting MOps to unbind values from MEMs when cselib noticed
the MEM was modified.  Even in that trivial testcase, we wouldn't always
unbind the incoming argument from the MEM, because it would be recorded
using a different expression which we could only globally recognize as
equivalent.  And that didn't even take potential aliasing into account!

In order to fix this debug info correctness bug, I ended up going with
the following approach: after every MEM write, we go through all MEMs in
the dataflow set binding table and unbind VALUEs bound to
possibly-aliased MEMs.  I'm sure there are cases in which this may
remove perfectly valid location information, but getting the compiler to
figure out they are indeed valid is impossible in some cases (eg
incoming pointers known by the caller to not alias), and very, very
expensive in others (eg inferring the lack of overlap by following
equivalence chains).

In the end, this patch didn't cause much loss of debug information on
x86_64, but it does significantly reduce total coverage on i686 for
libstdc++, libgcc and cc1plus.  There was a also a significant drop in
call_site_value and entry_value expressions, on both x86_64 and i686,
slightly more pronounced on the latter.  Unfortunately, I couldn't
quantify how much of this reduction was because we of incorrect location
information we generated before, and how much is because of
overconservative decisions about potential aliasing.

This was all kind of expected.  What did surprise me was that this
didn't have any measurable impact on bootstrap time.  What surprised me
further was that testing this patch confirmed the boostrap speedup of
1.08 I had observed with the proposed patch for PR47624, that this patch
depends on.

This patch was regstrapped along with the patch for PR47624 (ping
<http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01320.html>) on
x86_64-linux-gnu and i686-linux-gnu.  Ok to install?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-clobber-detach-modified-mems-pr49888.patch --]
[-- Type: text/x-diff, Size: 6868 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/49888
	* var-tracking.c: Include alias.h.
	(overlapping_mems): New struct.
	(drop_overlapping_mem_locs): New.
	(clobber_overlapping_mems): New.
	(var_mem_delete_and_set, var_mem_delete): Call it.
	(val_bind): Likewise, but only if modified.
	(compute_bb_dataflow, emit_notes_in_bb): Call it on MEMs.
	* Makefile.in (var-tracking.o): Depend in $(ALIAS_H).
	
for  gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/49888
	* gcc.dg/guality/pr49888.c: New.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2012-05-16 14:58:11.000000000 -0300
+++ gcc/var-tracking.c	2012-05-21 02:24:57.000000000 -0300
@@ -116,6 +116,7 @@
 #include "pointer-set.h"
 #include "recog.h"
 #include "tm_p.h"
+#include "alias.h"
 
 /* var-tracking.c assumes that tree code with the same value as VALUE rtx code
    has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl.
@@ -1955,6 +1956,106 @@ var_regno_delete (dataflow_set *set, int
   *reg = NULL;
 }
 
+/* Hold parameters for the hashtab traversal function
+   drop_overlapping_mem_locs, see below.  */
+
+struct overlapping_mems
+{
+  dataflow_set *set;
+  rtx loc;
+};
+
+/* Remove all MEMs that overlap with COMS->LOC from the location list
+   of a hash table entry for a value.  */
+
+static int
+drop_overlapping_mem_locs (void **slot, void *data)
+{
+  struct overlapping_mems *coms = (struct overlapping_mems *)data;
+  dataflow_set *set = coms->set;
+  rtx mloc = coms->loc;
+  variable var = (variable) *slot;
+
+  if (var->onepart == ONEPART_VALUE)
+    {
+      location_chain loc, *locp;
+      bool changed = false;
+      rtx cur_loc;
+
+      gcc_assert (var->n_var_parts == 1);
+
+      if (shared_var_p (var, set->vars))
+	{
+	  for (loc = var->var_part[0].loc_chain; loc; loc = loc->next)
+	    if (GET_CODE (loc->loc) == MEM
+		&& !nonoverlapping_memrefs_p (loc->loc, mloc, false))
+	      break;
+
+	  if (!loc)
+	    return 1;
+
+	  slot = unshare_variable (set, slot, var, VAR_INIT_STATUS_UNKNOWN);
+	  var = (variable)*slot;
+	  gcc_assert (var->n_var_parts == 1);
+	}
+
+      if (VAR_LOC_1PAUX (var))
+	cur_loc = VAR_LOC_FROM (var);
+      else
+	cur_loc = var->var_part[0].cur_loc;
+
+      for (locp = &var->var_part[0].loc_chain, loc = *locp;
+	   loc; loc = *locp)
+	{
+	  if (GET_CODE (loc->loc) != MEM
+	      || nonoverlapping_memrefs_p (loc->loc, mloc, false))
+	    {
+	      locp = &loc->next;
+	      continue;
+	    }
+
+	  *locp = loc->next;
+	  /* If we have deleted the location which was last emitted
+	     we have to emit new location so add the variable to set
+	     of changed variables.  */
+	  if (cur_loc == loc->loc)
+	    {
+	      changed = true;
+	      var->var_part[0].cur_loc = NULL;
+	      if (VAR_LOC_1PAUX (var))
+		VAR_LOC_FROM (var) = NULL;
+	    }
+	  pool_free (loc_chain_pool, loc);
+	}
+
+      if (!var->var_part[0].loc_chain)
+	{
+	  var->n_var_parts--;
+	  changed = true;
+	}
+      if (changed)
+	variable_was_changed (var, set);
+    }
+
+  return 1;
+}
+
+/* Remove from SET all VALUE bindings to MEMs that overlap with LOC.  */
+
+static void
+clobber_overlapping_mems (dataflow_set *set, rtx loc)
+{
+  struct overlapping_mems coms;
+
+  coms.set = set;
+  coms.loc = loc;
+
+  set->traversed_vars = set->vars;
+  htab_traverse (shared_hash_htab (set->vars),
+		 drop_overlapping_mem_locs, &coms);
+  set->traversed_vars = NULL;
+}
+
 /* Set the location of DV, OFFSET as the MEM LOC.  */
 
 static void
@@ -1997,6 +2098,7 @@ var_mem_delete_and_set (dataflow_set *se
   tree decl = MEM_EXPR (loc);
   HOST_WIDE_INT offset = INT_MEM_OFFSET (loc);
 
+  clobber_overlapping_mems (set, loc);
   decl = var_debug_decl (decl);
 
   if (initialized == VAR_INIT_STATUS_UNKNOWN)
@@ -2017,6 +2119,7 @@ var_mem_delete (dataflow_set *set, rtx l
   tree decl = MEM_EXPR (loc);
   HOST_WIDE_INT offset = INT_MEM_OFFSET (loc);
 
+  clobber_overlapping_mems (set, loc);
   decl = var_debug_decl (decl);
   if (clobber)
     clobber_variable_part (set, NULL, dv_from_decl (decl), offset, NULL);
@@ -2060,6 +2163,9 @@ val_bind (dataflow_set *set, rtx val, rt
     {
       struct elt_loc_list *l = CSELIB_VAL_PTR (val)->locs;
 
+      if (modified)
+	clobber_overlapping_mems (set, loc);
+
       if (l && GET_CODE (l->loc) == VALUE)
 	l = canonical_cselib_val (CSELIB_VAL_PTR (l->loc))->locs;
 
@@ -6372,6 +6478,8 @@ compute_bb_dataflow (basic_block bb)
 		}
 	      else if (REG_P (uloc))
 		var_regno_delete (out, REGNO (uloc));
+	      else if (MEM_P (uloc))
+		clobber_overlapping_mems (out, uloc);
 
 	      val_store (out, val, dstv, insn, true);
 	    }
@@ -8871,6 +8979,8 @@ emit_notes_in_bb (basic_block bb, datafl
 		}
 	      else if (REG_P (uloc))
 		var_regno_delete (set, REGNO (uloc));
+	      else if (MEM_P (uloc))
+		clobber_overlapping_mems (set, uloc);
 
 	      val_store (set, val, dstv, insn, true);
 
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in.orig	2012-05-21 02:21:12.424703696 -0300
+++ gcc/Makefile.in	2012-05-21 02:22:26.000000000 -0300
@@ -3129,8 +3129,8 @@ var-tracking.o : var-tracking.c $(CONFIG
    $(RTL_H) $(TREE_H) hard-reg-set.h insn-config.h reload.h $(FLAGS_H) \
    $(BASIC_BLOCK_H) output.h sbitmap.h alloc-pool.h $(FIBHEAP_H) $(HASHTAB_H) \
    $(REGS_H) $(EXPR_H) $(TIMEVAR_H) $(TREE_PASS_H) $(TREE_FLOW_H) \
-   cselib.h $(TARGET_H) $(DIAGNOSTIC_CORE_H) $(PARAMS_H) $(DIAGNOSTIC_H) pointer-set.h \
-   $(RECOG_H) $(TM_P_H) tree-pretty-print.h
+   cselib.h $(TARGET_H) $(DIAGNOSTIC_CORE_H) $(PARAMS_H) $(DIAGNOSTIC_H) \
+   pointer-set.h $(RECOG_H) $(TM_P_H) tree-pretty-print.h $(ALIAS_H)
 profile.o : profile.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \
    $(TREE_H) $(FLAGS_H) output.h $(REGS_H) $(EXPR_H) $(FUNCTION_H) $(BASIC_BLOCK_H) \
    $(DIAGNOSTIC_CORE_H) $(COVERAGE_H) $(TREE_FLOW_H) value-prof.h cfghooks.h \
Index: gcc/testsuite/gcc.dg/guality/pr49888.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.dg/guality/pr49888.c	2012-05-16 14:58:12.000000000 -0300
@@ -0,0 +1,25 @@
+/* PR debug/49888 */
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+static int v;
+
+static void __attribute__((noinline, noclone))
+f (int *p)
+{
+  int c = *p;
+  v = c;
+  *p = 1; /* { dg-final { gdb-test 12 "c" "0" } } */
+  /* c may very well be optimized out at this point, so we test !c,
+     which will evaluate to the expected value.  We just want to make
+     sure it doesn't remain bound to *p as it did before, in which
+     case !c would evaluate to 0.  */
+  v = 0; /* { dg-final { gdb-test 17 "!c" "1" } } */
+}
+int
+main ()
+{
+  int a = 0;
+  f (&a);
+  return 0;
+}

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Jakub Jelinek]

On Wed, May 23, 2012 at 06:27:21AM -0300, Alexandre Oliva wrote:
> +static int
> +drop_overlapping_mem_locs (void **slot, void *data)
> +{
> +  struct overlapping_mems *coms = (struct overlapping_mems *)data;
> +  dataflow_set *set = coms->set;
> +  rtx mloc = coms->loc;
> +  variable var = (variable) *slot;
> +
> +  if (var->onepart == ONEPART_VALUE)
> +    {
> +      location_chain loc, *locp;
> +      bool changed = false;
> +      rtx cur_loc;
> +
> +      gcc_assert (var->n_var_parts == 1);
> +
> +      if (shared_var_p (var, set->vars))
> +	{
> +	  for (loc = var->var_part[0].loc_chain; loc; loc = loc->next)
> +	    if (GET_CODE (loc->loc) == MEM
> +		&& !nonoverlapping_memrefs_p (loc->loc, mloc, false))

Isn't nonoverlapping_memrefs_p predicate too conservative?
cselib.c uses canon_true_dependence to decide what should be invalidated.

	Jakub

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Richard Guenther]

On Wed, May 23, 2012 at 12:13 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, May 23, 2012 at 06:27:21AM -0300, Alexandre Oliva wrote:
>> +static int
>> +drop_overlapping_mem_locs (void **slot, void *data)
>> +{
>> +  struct overlapping_mems *coms = (struct overlapping_mems *)data;
>> +  dataflow_set *set = coms->set;
>> +  rtx mloc = coms->loc;
>> +  variable var = (variable) *slot;
>> +
>> +  if (var->onepart == ONEPART_VALUE)
>> +    {
>> +      location_chain loc, *locp;
>> +      bool changed = false;
>> +      rtx cur_loc;
>> +
>> +      gcc_assert (var->n_var_parts == 1);
>> +
>> +      if (shared_var_p (var, set->vars))
>> +     {
>> +       for (loc = var->var_part[0].loc_chain; loc; loc = loc->next)
>> +         if (GET_CODE (loc->loc) == MEM
>> +             && !nonoverlapping_memrefs_p (loc->loc, mloc, false))
>
> Isn't nonoverlapping_memrefs_p predicate too conservative?
> cselib.c uses canon_true_dependence to decide what should be invalidated.

Yeah, I think nonoverlapping_memrefs_p should become private to alias.c

Richard.

>        Jakub

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Alexandre Oliva]

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

On May 23, 2012, Jakub Jelinek <jakub@redhat.com> wrote:

> On Wed, May 23, 2012 at 06:27:21AM -0300, Alexandre Oliva wrote:
>> +	  for (loc = var->var_part[0].loc_chain; loc; loc = loc->next)
>> +	    if (GET_CODE (loc->loc) == MEM
>> +		&& !nonoverlapping_memrefs_p (loc->loc, mloc, false))

> Isn't nonoverlapping_memrefs_p predicate too conservative?
> cselib.c uses canon_true_dependence to decide what should be invalidated.

Yeah, I guess that should do.  I've finally managed to analyze the
effects of using canon_true_dependence on debug info, and it does seem
to produce reasonable results:

before after
= entry value count =
 33005  35700 i686/cc1plus
    99    116 i686/libgcc_s.so
   216    239 i686/libstdc++.so
 99902 104390 amd64/cc1plus
   308    319 amd64/libgcc_s.so
  8819   8864 amd64/libstdc++.so

= call site value count =
519877 514591 i686/cc1plus
   406    403 i686/libgcc_s.so
 10284  10121 i686/libstdc++.so
518401 518220 amd64/cc1plus
   341    341 amd64/libgcc_s.so
 11309  11261 amd64/libstdc++.so

loc info coverage             (before | after)

i686/cc1plus:
cov%    samples cumul                   cov%    samples cumul
0.0     155345/28%      155345/28%    | 0.0     155561/29%      155561/29%
0..5    6606/1% 161951/30%            | 0..5    7969/1% 163530/30%
6..10   5984/1% 167935/31%            | 6..10   6336/1% 169866/31%
11..15  4823/0% 172758/32%            | 11..15  5117/0% 174983/32%
16..20  6262/1% 179020/33%            | 16..20  6677/1% 181660/33%
21..25  5259/0% 184279/34%            | 21..25  5688/1% 187348/34%
26..30  5149/0% 189428/35%            | 26..30  5409/1% 192757/35%
31..35  4547/0% 193975/36%            | 31..35  4868/0% 197625/36%
36..40  7204/1% 201179/37%            | 36..40  7524/1% 205149/38%
41..45  8343/1% 209522/39%            | 41..45  8656/1% 213805/39%
46..50  8744/1% 218266/40%            | 46..50  9252/1% 223057/41%
51..55  5806/1% 224072/41%            | 51..55  6023/1% 229080/42%
56..60  6834/1% 230906/43%            | 56..60  7072/1% 236152/44%
61..65  5391/1% 236297/44%            | 61..65  5554/1% 241706/45%
66..70  9269/1% 245566/45%            | 66..70  9327/1% 251033/46%
71..75  6249/1% 251815/46%            | 71..75  6309/1% 257342/47%
76..80  8871/1% 260686/48%            | 76..80  8905/1% 266247/49%
81..85  8775/1% 269461/50%            | 81..85  8786/1% 275033/51%
86..90  13323/2%        282784/52%    | 86..90  13286/2%        288319/53%
91..95  21279/3%        304063/56%    | 91..95  20947/3%        309266/57%
96..99  21323/3%        325386/60%    | 96..99  20247/3%        329513/61%
100     211312/39%      536698/100%   | 100     206704/38%      536217/100%

i686/libgcc_s.so:
cov%    samples cumul                   cov%    samples cumul
0.0     511/21% 511/21%               | 0.0     512/21% 512/21%
0..5    44/1%   555/23%               | 0..5    67/2%   579/24%
6..10   45/1%   600/25%               | 6..10   41/1%   620/26%
11..15  35/1%   635/26%               | 11..15  37/1%   657/27%
16..20  28/1%   663/27%               | 16..20  29/1%   686/29%
21..25  32/1%   695/29%               | 21..25  32/1%   718/30%
26..30  40/1%   735/31%               | 26..30  41/1%   759/32%
31..35  32/1%   767/32%               | 31..35  34/1%   793/33%
36..40  29/1%   796/33%               | 36..40  33/1%   826/34%
41..45  42/1%   838/35%               | 41..45  44/1%   870/36%
46..50  47/1%   885/37%               | 46..50  56/2%   926/39%
51..55  33/1%   918/38%               | 51..55  35/1%   961/40%
56..60  45/1%   963/40%               | 56..60  48/2%   1009/42%
61..65  44/1%   1007/42%              | 61..65  43/1%   1052/44%
66..70  64/2%   1071/45%              | 66..70  68/2%   1120/47%
71..75  45/1%   1116/47%              | 71..75  45/1%   1165/49%
76..80  67/2%   1183/49%              | 76..80  69/2%   1234/52%
81..85  76/3%   1259/53%              | 81..85  76/3%   1310/55%
86..90  131/5%  1390/58%              | 86..90  119/5%  1429/60%
91..95  83/3%   1473/62%              | 91..95  81/3%   1510/63%
96..99  54/2%   1527/64%              | 96..99  49/2%   1559/66%
100     842/35% 2369/100%             | 100     802/33% 2361/100%

i686/libstdc++.so
cov%    samples cumul                   cov%    samples cumul
0.0     12708/37%       12708/37%     | 0.0     12737/37%       12737/37%
0..5    125/0%  12833/38%             | 0..5    263/0%  13000/38%
6..10   167/0%  13000/38%             | 6..10   201/0%  13201/39%
11..15  125/0%  13125/39%             | 11..15  157/0%  13358/39%
16..20  197/0%  13322/39%             | 16..20  216/0%  13574/40%
21..25  169/0%  13491/40%             | 21..25  194/0%  13768/40%
26..30  120/0%  13611/40%             | 26..30  155/0%  13923/41%
31..35  179/0%  13790/41%             | 31..35  188/0%  14111/41%
36..40  238/0%  14028/41%             | 36..40  257/0%  14368/42%
41..45  226/0%  14254/42%             | 41..45  266/0%  14634/43%
46..50  258/0%  14512/43%             | 46..50  270/0%  14904/44%
51..55  176/0%  14688/43%             | 51..55  199/0%  15103/44%
56..60  349/1%  15037/44%             | 56..60  362/1%  15465/46%
61..65  286/0%  15323/45%             | 61..65  288/0%  15753/46%
66..70  259/0%  15582/46%             | 66..70  260/0%  16013/47%
71..75  404/1%  15986/47%             | 71..75  416/1%  16429/48%
76..80  413/1%  16399/48%             | 76..80  403/1%  16832/50%
81..85  621/1%  17020/50%             | 81..85  617/1%  17449/51%
86..90  788/2%  17808/52%             | 86..90  733/2%  18182/54%
91..95  833/2%  18641/55%             | 91..95  774/2%  18956/56%
96..99  537/1%  19178/57%             | 96..99  495/1%  19451/57%
100     14426/42%       33604/100%    | 100     14148/42%       33599/100%


amd64/cc1plus:
cov%    samples cumul                   cov%    samples cumul
0.0     160910/31%      160910/31%    | 0.0     161076/31%      161076/31%
0..5    6266/1% 167176/32%            | 0..5    6543/1% 167619/32%
6..10   5484/1% 172660/33%            | 6..10   5615/1% 173234/33%
11..15  4874/0% 177534/34%            | 11..15  4989/0% 178223/34%
16..20  5001/0% 182535/35%            | 16..20  5108/0% 183331/35%
21..25  5383/1% 187918/36%            | 21..25  5458/1% 188789/36%
26..30  4182/0% 192100/37%            | 26..30  4286/0% 193075/37%
31..35  4731/0% 196831/38%            | 31..35  4814/0% 197889/38%
36..40  6995/1% 203826/39%            | 36..40  7068/1% 204957/40%
41..45  7749/1% 211575/41%            | 41..45  7814/1% 212771/41%
46..50  8686/1% 220261/42%            | 46..50  8788/1% 221559/43%
51..55  7650/1% 227911/44%            | 51..55  7728/1% 229287/44%
56..60  6394/1% 234305/45%            | 56..60  6457/1% 235744/46%
61..65  5629/1% 239934/46%            | 61..65  5685/1% 241429/47%
66..70  7371/1% 247305/48%            | 66..70  7448/1% 248877/48%
71..75  8137/1% 255442/49%            | 71..75  8162/1% 257039/50%
76..80  19364/3%        274806/53%    | 76..80  19363/3%        276402/54%
81..85  12056/2%        286862/55%    | 81..85  12024/2%        288426/56%
86..90  15509/3%        302371/58%    | 86..90  15431/3%        303857/59%
91..95  17138/3%        319509/62%    | 91..95  16897/3%        320754/62%
96..99  17654/3%        337163/65%    | 96..99  17034/3%        337788/66%
100     175723/34%      512886/100%   | 100     173852/33%      511640/100%

i686/libgcc_s.so:
cov%    samples cumul                   cov%    samples cumul
0.0     450/23% 450/23%                 0.0     450/23% 450/23%
0..5    46/2%   496/26%               | 0..5    47/2%   497/26%
6..10   29/1%   525/27%               | 6..10   30/1%   527/27%
11..15  33/1%   558/29%               | 11..15  34/1%   561/29%
16..20  32/1%   590/31%               | 16..20  33/1%   594/31%
21..25  31/1%   621/32%               | 21..25  31/1%   625/32%
26..30  20/1%   641/33%               | 26..30  20/1%   645/34%
31..35  24/1%   665/34%               | 31..35  24/1%   669/35%
36..40  39/2%   704/37%               | 36..40  41/2%   710/37%
41..45  53/2%   757/39%               | 41..45  52/2%   762/40%
46..50  25/1%   782/41%               | 46..50  25/1%   787/41%
51..55  32/1%   814/42%               | 51..55  32/1%   819/43%
56..60  40/2%   854/44%               | 56..60  40/2%   859/45%
61..65  42/2%   896/47%               | 61..65  43/2%   902/47%
66..70  55/2%   951/50%               | 66..70  54/2%   956/50%
71..75  69/3%   1020/53%              | 71..75  69/3%   1025/54%
76..80  41/2%   1061/55%              | 76..80  43/2%   1068/56%
81..85  33/1%   1094/57%              | 81..85  32/1%   1100/57%
86..90  74/3%   1168/61%              | 86..90  73/3%   1173/61%
91..95  57/2%   1225/64%              | 91..95  56/2%   1229/64%
96..99  45/2%   1270/66%              | 96..99  44/2%   1273/67%
100     632/33% 1902/100%             | 100     624/32% 1897/100%

amd64/libstdc++.so
cov%    samples cumul                   cov%    samples cumul
0.0     10565/36%       10565/36%     | 0.0     10594/36%       10594/36%
0..5    121/0%  10686/36%             | 0..5    141/0%  10735/36%
6..10   158/0%  10844/37%             | 6..10   187/0%  10922/37%
11..15  164/0%  11008/37%             | 11..15  186/0%  11108/37%
16..20  234/0%  11242/38%             | 16..20  237/0%  11345/38%
21..25  236/0%  11478/39%             | 21..25  239/0%  11584/39%
26..30  206/0%  11684/39%             | 26..30  212/0%  11796/40%
31..35  273/0%  11957/40%             | 31..35  286/0%  12082/41%
36..40  375/1%  12332/42%             | 36..40  376/1%  12458/42%
41..45  286/0%  12618/43%             | 41..45  285/0%  12743/43%
46..50  422/1%  13040/44%             | 46..50  419/1%  13162/44%
51..55  196/0%  13236/45%             | 51..55  193/0%  13355/45%
56..60  292/0%  13528/46%             | 56..60  303/1%  13658/46%
61..65  429/1%  13957/47%             | 61..65  431/1%  14089/48%
66..70  365/1%  14322/48%             | 66..70  373/1%  14462/49%
71..75  375/1%  14697/50%             | 71..75  378/1%  14840/50%
76..80  652/2%  15349/52%             | 76..80  650/2%  15490/52%
81..85  817/2%  16166/55%             | 81..85  798/2%  16288/55%
86..90  787/2%  16953/57%             | 86..90  780/2%  17068/58%
91..95  1090/3% 18043/61%             | 91..95  1076/3% 18144/61%
96..99  570/1%  18613/63%             | 96..99  562/1%  18706/63%
100     10683/36%       29296/100%    | 100     10565/36%       29271/100%

This patch was regstrapped along with the patch for PR47624 (ping
<http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01320.html>) on
x86_64-linux-gnu and i686-linux-gnu.  Ok to install?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-clobber-detach-modified-mems-pr49888.patch --]
[-- Type: text/x-diff, Size: 7115 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/49888
	* var-tracking.c: Include alias.h.
	(overlapping_mems): New struct.
	(drop_overlapping_mem_locs): New.
	(clobber_overlapping_mems): New.
	(var_mem_delete_and_set, var_mem_delete): Call it.
	(val_bind): Likewise, but only if modified.
	(compute_bb_dataflow, emit_notes_in_bb): Call it on MEMs.
	* Makefile.in (var-tracking.o): Depend in $(ALIAS_H).
	
for  gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/49888
	* gcc.dg/guality/pr49888.c: New.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2012-06-05 03:03:47.154714459 -0300
+++ gcc/var-tracking.c	2012-06-05 12:50:57.724756717 -0300
@@ -115,6 +115,7 @@
 #include "pointer-set.h"
 #include "recog.h"
 #include "tm_p.h"
+#include "alias.h"
 
 /* var-tracking.c assumes that tree code with the same value as VALUE rtx code
    has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl.
@@ -1954,6 +1955,111 @@ var_regno_delete (dataflow_set *set, int
   *reg = NULL;
 }
 
+/* Hold parameters for the hashtab traversal function
+   drop_overlapping_mem_locs, see below.  */
+
+struct overlapping_mems
+{
+  dataflow_set *set;
+  rtx loc, addr;
+};
+
+/* Remove all MEMs that overlap with COMS->LOC from the location list
+   of a hash table entry for a value.  COMS->ADDR must be a
+   canonicalized form of COMS->LOC's address, and COMS->LOC must be
+   canonicalized itself.  */
+
+static int
+drop_overlapping_mem_locs (void **slot, void *data)
+{
+  struct overlapping_mems *coms = (struct overlapping_mems *)data;
+  dataflow_set *set = coms->set;
+  rtx mloc = coms->loc, addr = coms->addr;
+  variable var = (variable) *slot;
+
+  if (var->onepart == ONEPART_VALUE)
+    {
+      location_chain loc, *locp;
+      bool changed = false;
+      rtx cur_loc;
+
+      gcc_assert (var->n_var_parts == 1);
+
+      if (shared_var_p (var, set->vars))
+	{
+	  for (loc = var->var_part[0].loc_chain; loc; loc = loc->next)
+	    if (GET_CODE (loc->loc) == MEM
+		&& canon_true_dependence (mloc, GET_MODE (mloc), addr,
+					  loc->loc, NULL))
+	      break;
+
+	  if (!loc)
+	    return 1;
+
+	  slot = unshare_variable (set, slot, var, VAR_INIT_STATUS_UNKNOWN);
+	  var = (variable)*slot;
+	  gcc_assert (var->n_var_parts == 1);
+	}
+
+      if (VAR_LOC_1PAUX (var))
+	cur_loc = VAR_LOC_FROM (var);
+      else
+	cur_loc = var->var_part[0].cur_loc;
+
+      for (locp = &var->var_part[0].loc_chain, loc = *locp;
+	   loc; loc = *locp)
+	{
+	  if (GET_CODE (loc->loc) != MEM
+	      || !canon_true_dependence (mloc, GET_MODE (mloc), addr,
+					 loc->loc, NULL))
+	    {
+	      locp = &loc->next;
+	      continue;
+	    }
+
+	  *locp = loc->next;
+	  /* If we have deleted the location which was last emitted
+	     we have to emit new location so add the variable to set
+	     of changed variables.  */
+	  if (cur_loc == loc->loc)
+	    {
+	      changed = true;
+	      var->var_part[0].cur_loc = NULL;
+	      if (VAR_LOC_1PAUX (var))
+		VAR_LOC_FROM (var) = NULL;
+	    }
+	  pool_free (loc_chain_pool, loc);
+	}
+
+      if (!var->var_part[0].loc_chain)
+	{
+	  var->n_var_parts--;
+	  changed = true;
+	}
+      if (changed)
+	variable_was_changed (var, set);
+    }
+
+  return 1;
+}
+
+/* Remove from SET all VALUE bindings to MEMs that overlap with LOC.  */
+
+static void
+clobber_overlapping_mems (dataflow_set *set, rtx loc)
+{
+  struct overlapping_mems coms;
+
+  coms.set = set;
+  coms.loc = canon_rtx (loc);
+  coms.addr = canon_rtx (get_addr (XEXP (loc, 0)));
+
+  set->traversed_vars = set->vars;
+  htab_traverse (shared_hash_htab (set->vars),
+		 drop_overlapping_mem_locs, &coms);
+  set->traversed_vars = NULL;
+}
+
 /* Set the location of DV, OFFSET as the MEM LOC.  */
 
 static void
@@ -1996,6 +2102,7 @@ var_mem_delete_and_set (dataflow_set *se
   tree decl = MEM_EXPR (loc);
   HOST_WIDE_INT offset = INT_MEM_OFFSET (loc);
 
+  clobber_overlapping_mems (set, loc);
   decl = var_debug_decl (decl);
 
   if (initialized == VAR_INIT_STATUS_UNKNOWN)
@@ -2016,6 +2123,7 @@ var_mem_delete (dataflow_set *set, rtx l
   tree decl = MEM_EXPR (loc);
   HOST_WIDE_INT offset = INT_MEM_OFFSET (loc);
 
+  clobber_overlapping_mems (set, loc);
   decl = var_debug_decl (decl);
   if (clobber)
     clobber_variable_part (set, NULL, dv_from_decl (decl), offset, NULL);
@@ -2059,6 +2167,9 @@ val_bind (dataflow_set *set, rtx val, rt
     {
       struct elt_loc_list *l = CSELIB_VAL_PTR (val)->locs;
 
+      if (modified)
+	clobber_overlapping_mems (set, loc);
+
       if (l && GET_CODE (l->loc) == VALUE)
 	l = canonical_cselib_val (CSELIB_VAL_PTR (l->loc))->locs;
 
@@ -6371,6 +6482,8 @@ compute_bb_dataflow (basic_block bb)
 		}
 	      else if (REG_P (uloc))
 		var_regno_delete (out, REGNO (uloc));
+	      else if (MEM_P (uloc))
+		clobber_overlapping_mems (out, uloc);
 
 	      val_store (out, val, dstv, insn, true);
 	    }
@@ -8870,6 +8983,8 @@ emit_notes_in_bb (basic_block bb, datafl
 		}
 	      else if (REG_P (uloc))
 		var_regno_delete (set, REGNO (uloc));
+	      else if (MEM_P (uloc))
+		clobber_overlapping_mems (set, uloc);
 
 	      val_store (set, val, dstv, insn, true);
 
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in.orig	2012-06-05 03:02:32.904609594 -0300
+++ gcc/Makefile.in	2012-06-05 12:54:21.000000000 -0300
@@ -3130,8 +3130,8 @@ var-tracking.o : var-tracking.c $(CONFIG
    $(RTL_H) $(TREE_H) hard-reg-set.h insn-config.h reload.h $(FLAGS_H) \
    $(BASIC_BLOCK_H) bitmap.h alloc-pool.h $(FIBHEAP_H) $(HASHTAB_H) \
    $(REGS_H) $(EXPR_H) $(TIMEVAR_H) $(TREE_PASS_H) $(TREE_FLOW_H) \
-   cselib.h $(TARGET_H) $(DIAGNOSTIC_CORE_H) $(PARAMS_H) $(DIAGNOSTIC_H) pointer-set.h \
-   $(RECOG_H) $(TM_P_H) $(TREE_PRETTY_PRINT_H)
+   cselib.h $(TARGET_H) $(DIAGNOSTIC_CORE_H) $(PARAMS_H) $(DIAGNOSTIC_H) \
+   pointer-set.h $(RECOG_H) $(TM_P_H) $(TREE_PRETTY_PRINT_H) $(ALIAS_H)
 profile.o : profile.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \
    $(TREE_H) $(FLAGS_H) $(REGS_H) $(EXPR_H) $(FUNCTION_H) $(BASIC_BLOCK_H) \
    $(DIAGNOSTIC_CORE_H) $(COVERAGE_H) $(TREE_FLOW_H) value-prof.h cfghooks.h \
Index: gcc/testsuite/gcc.dg/guality/pr49888.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.dg/guality/pr49888.c	2012-06-05 12:50:57.878744571 -0300
@@ -0,0 +1,25 @@
+/* PR debug/49888 */
+/* { dg-do run } */
+/* { dg-options "-g" } */
+
+static int v;
+
+static void __attribute__((noinline, noclone))
+f (int *p)
+{
+  int c = *p;
+  v = c;
+  *p = 1; /* { dg-final { gdb-test 12 "c" "0" } } */
+  /* c may very well be optimized out at this point, so we test !c,
+     which will evaluate to the expected value.  We just want to make
+     sure it doesn't remain bound to *p as it did before, in which
+     case !c would evaluate to 0.  */
+  v = 0; /* { dg-final { gdb-test 17 "!c" "1" } } */
+}
+int
+main ()
+{
+  int a = 0;
+  f (&a);
+  return 0;
+}

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Richard Henderson]

On 2012-06-05 12:33, Alexandre Oliva wrote:
> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/49888
> 	* var-tracking.c: Include alias.h.
> 	(overlapping_mems): New struct.
> 	(drop_overlapping_mem_locs): New.
> 	(clobber_overlapping_mems): New.
> 	(var_mem_delete_and_set, var_mem_delete): Call it.
> 	(val_bind): Likewise, but only if modified.
> 	(compute_bb_dataflow, emit_notes_in_bb): Call it on MEMs.
> 	* Makefile.in (var-tracking.o): Depend in $(ALIAS_H).
> 	
> for  gcc/testsuite/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/49888
> 	* gcc.dg/guality/pr49888.c: New.

Ok.


r~

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[H.J. Lu]

On Tue, Jun 12, 2012 at 1:42 PM, Richard Henderson <rth@redhat.com> wrote:
> On 2012-06-05 12:33, Alexandre Oliva wrote:
>> for  gcc/ChangeLog
>> from  Alexandre Oliva  <aoliva@redhat.com>
>>
>>       PR debug/49888
>>       * var-tracking.c: Include alias.h.
>>       (overlapping_mems): New struct.
>>       (drop_overlapping_mem_locs): New.
>>       (clobber_overlapping_mems): New.
>>       (var_mem_delete_and_set, var_mem_delete): Call it.
>>       (val_bind): Likewise, but only if modified.
>>       (compute_bb_dataflow, emit_notes_in_bb): Call it on MEMs.
>>       * Makefile.in (var-tracking.o): Depend in $(ALIAS_H).
>>
>> for  gcc/testsuite/ChangeLog
>> from  Alexandre Oliva  <aoliva@redhat.com>
>>
>>       PR debug/49888
>>       * gcc.dg/guality/pr49888.c: New.
>
> Ok.
>
>

It caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53671


-- 
H.J.

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Alexandre Oliva]

On Jun 14, 2012, "H.J. Lu" <hjl.tools@gmail.com> wrote:

> On Tue, Jun 12, 2012 at 1:42 PM, Richard Henderson <rth@redhat.com> wrote:
>> On 2012-06-05 12:33, Alexandre Oliva wrote:
>>> for  gcc/ChangeLog
>>> from  Alexandre Oliva  <aoliva@redhat.com>
>>> 
>>>       PR debug/49888
>>>       * var-tracking.c: Include alias.h.
>>>       (overlapping_mems): New struct.
>>>       (drop_overlapping_mem_locs): New.
>>>       (clobber_overlapping_mems): New.
>>>       (var_mem_delete_and_set, var_mem_delete): Call it.
>>>       (val_bind): Likewise, but only if modified.
>>>       (compute_bb_dataflow, emit_notes_in_bb): Call it on MEMs.
>>>       * Makefile.in (var-tracking.o): Depend in $(ALIAS_H).
>>> 
>>> for  gcc/testsuite/ChangeLog
>>> from  Alexandre Oliva  <aoliva@redhat.com>
>>> 
>>>       PR debug/49888
>>>       * gcc.dg/guality/pr49888.c: New.
>> 
>> Ok.

> It caused:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53671

I see a few of these failures myself.  They're in my test results.  I
guess I was so caught up in assessing the debug info quality changes
with this patch that I completely failed to look at the test results,
because they've been around for a while, and I have a few pristine-build
baselines in between.  Apologies for this mistake.

Anyway...  The problem is not too hard to understand, but it may be
somewhat hard to fix.  Basically, pushing registers to save them on the
stack implies writes that are currently thought to conflict with the
MEMs holding incoming arguments, and apparently there isn't enough
information in the cselib static table for us to realize the write
doesn't alias with any of the incoming arguments.

Using the dynamic tables during alias testing is one possibility I'm
looking into, but this won't be trivial and it could get expensive;
another, that has just occurred to me while composing this message, is
to use the cselib static table itself, for it *should* have enough info
for us to realize that argp and sp offset are related and, given proper
offsets, non-overlapping.

Now, neither approach is going to be an immediate fix.  Should I revert
the patch, or can we live with some debug info completeness regressions
for a bit?  I wouldn't mind reverting it, but I won't unless the broken
patch is actually causing trouble to any of us.

Again, sorry about the breakage.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[H.J. Lu]

On Fri, Jun 15, 2012 at 3:21 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Jun 14, 2012, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>
>> On Tue, Jun 12, 2012 at 1:42 PM, Richard Henderson <rth@redhat.com> wrote:
>>> On 2012-06-05 12:33, Alexandre Oliva wrote:
>>>> for  gcc/ChangeLog
>>>> from  Alexandre Oliva  <aoliva@redhat.com>
>>>>
>>>>       PR debug/49888
>>>>       * var-tracking.c: Include alias.h.
>>>>       (overlapping_mems): New struct.
>>>>       (drop_overlapping_mem_locs): New.
>>>>       (clobber_overlapping_mems): New.
>>>>       (var_mem_delete_and_set, var_mem_delete): Call it.
>>>>       (val_bind): Likewise, but only if modified.
>>>>       (compute_bb_dataflow, emit_notes_in_bb): Call it on MEMs.
>>>>       * Makefile.in (var-tracking.o): Depend in $(ALIAS_H).
>>>>
>>>> for  gcc/testsuite/ChangeLog
>>>> from  Alexandre Oliva  <aoliva@redhat.com>
>>>>
>>>>       PR debug/49888
>>>>       * gcc.dg/guality/pr49888.c: New.
>>>
>>> Ok.
>
>> It caused:
>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53671
>
> I see a few of these failures myself.  They're in my test results.  I
> guess I was so caught up in assessing the debug info quality changes
> with this patch that I completely failed to look at the test results,
> because they've been around for a while, and I have a few pristine-build
> baselines in between.  Apologies for this mistake.
>
> Anyway...  The problem is not too hard to understand, but it may be
> somewhat hard to fix.  Basically, pushing registers to save them on the
> stack implies writes that are currently thought to conflict with the
> MEMs holding incoming arguments, and apparently there isn't enough
> information in the cselib static table for us to realize the write
> doesn't alias with any of the incoming arguments.
>
> Using the dynamic tables during alias testing is one possibility I'm
> looking into, but this won't be trivial and it could get expensive;
> another, that has just occurred to me while composing this message, is
> to use the cselib static table itself, for it *should* have enough info
> for us to realize that argp and sp offset are related and, given proper
> offsets, non-overlapping.
>
> Now, neither approach is going to be an immediate fix.  Should I revert
> the patch, or can we live with some debug info completeness regressions
> for a bit?  I wouldn't mind reverting it, but I won't unless the broken
> patch is actually causing trouble to any of us.
>

If I understand it correctly, the new approach fails to handle push
properly.  If it is true, I think the patch should be reverted.

Thanks.


-- 
H.J.

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Alexandre Oliva]

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

On Jun 16, 2012, "H.J. Lu" <hjl.tools@gmail.com> wrote:

> If I understand it correctly, the new approach fails to handle push
> properly.

It's actually cselib that didn't deal with push properly, so it thinks
incoming stack arguments may be clobbered by them.  But that's not the
whole story, unfortunately.  I still don't have a complete fix for the
problem, but I have some patches that restore nearly all of the passes.

The first one extends RTX alias analysis so that cselib can recognize
that (mem:SI ARGP) and (mem:SI (plus (and (plus ARGP #-4) #-32) #-4))
don't alias.  Before the patch, we'd go for infinite sized objects upon
AND.

The second introduces an entry-point equivalence between ARGP and SP, so
that SP references in push and stack-align sequences can be
canonicalized to ARGP-based.

The third introduces address canonicalization that uses information in
the dataflow variable set in addition to the static cselib table.  This
is the one I'm still working on, because some expressions still fail to
canonicalize to ARGP although they could.

The fourth removes a now-redundant equivalence from the dynamic table;
the required information is always preserved in the static table.

I've regstrapped (and checked results! :-) all of these on
x86_64-linux-gnu and i686-linux-gnu.  It fixes all visible regressions
in x86_64-linux-gnu, and nearly all on i686-linux-gnu.

May I check these in and keep on working to complete the fix, or should
I revert the original patch and come back only with a patchset that
fixes all debug info regressions?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: alias-align-exprs-pr53671-pr49888.patch --]
[-- Type: text/x-diff, Size: 1737 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/53671
	PR debug/49888
	* alias.c (memrefs_conflict_p): Improve handling of AND for
	alignment.
	
Index: gcc/alias.c
===================================================================
--- gcc/alias.c.orig	2012-06-17 22:52:27.551102225 -0300
+++ gcc/alias.c	2012-06-17 22:59:00.674994588 -0300
@@ -2103,17 +2103,31 @@ memrefs_conflict_p (int xsize, rtx x, in
      at least as large as the alignment, assume no other overlap.  */
   if (GET_CODE (x) == AND && CONST_INT_P (XEXP (x, 1)))
     {
-      if (GET_CODE (y) == AND || ysize < -INTVAL (XEXP (x, 1)))
+      HOST_WIDE_INT sc = INTVAL (XEXP (x, 1));
+      unsigned HOST_WIDE_INT uc = sc;
+      if (xsize > 0 && sc < 0 && -uc == (uc & -uc))
+	{
+	  xsize -= sc + 1;
+	  c -= sc;
+	}
+      else if (GET_CODE (y) == AND || ysize < -INTVAL (XEXP (x, 1)))
 	xsize = -1;
       return memrefs_conflict_p (xsize, canon_rtx (XEXP (x, 0)), ysize, y, c);
     }
   if (GET_CODE (y) == AND && CONST_INT_P (XEXP (y, 1)))
     {
+      HOST_WIDE_INT sc = INTVAL (XEXP (y, 1));
+      unsigned HOST_WIDE_INT uc = sc;
+      if (ysize > 0 && sc < 0 && -uc == (uc & -uc))
+	{
+	  ysize -= sc + 1;
+	  c += sc;
+	}
       /* ??? If we are indexing far enough into the array/structure, we
 	 may yet be able to determine that we can not overlap.  But we
 	 also need to that we are far enough from the end not to overlap
 	 a following reference, so we do nothing with that for now.  */
-      if (GET_CODE (x) == AND || xsize < -INTVAL (XEXP (y, 1)))
+      else if (GET_CODE (x) == AND || xsize < -INTVAL (XEXP (y, 1)))
 	ysize = -1;
       return memrefs_conflict_p (xsize, x, ysize, canon_rtx (XEXP (y, 0)), c);
     }

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: vta-always-record-stack-base-pr53671-pr49888.patch --]
[-- Type: text/x-diff, Size: 1742 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/53671
	PR debug/49888
	* var-tracking.c (vt_initialize): Record initial offset between
	arg pointer and stack pointer.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2012-06-17 23:00:45.793675979 -0300
+++ gcc/var-tracking.c	2012-06-17 23:01:02.525351931 -0300
@@ -9507,6 +9507,41 @@ vt_initialize (void)
       valvar_pool = NULL;
     }
 
+  if (MAY_HAVE_DEBUG_INSNS)
+    {
+      rtx reg, expr;
+      int ofst;
+      cselib_val *val;
+
+#ifdef FRAME_POINTER_CFA_OFFSET
+      reg = frame_pointer_rtx;
+      ofst = FRAME_POINTER_CFA_OFFSET (current_function_decl);
+#else
+      reg = arg_pointer_rtx;
+      ofst = ARG_POINTER_CFA_OFFSET (current_function_decl);
+#endif
+
+      ofst -= INCOMING_FRAME_SP_OFFSET;
+
+      val = cselib_lookup_from_insn (reg, GET_MODE (reg), 1,
+				     VOIDmode, get_insns ());
+      preserve_value (val);
+      cselib_preserve_cfa_base_value (val, REGNO (reg));
+      expr = plus_constant (GET_MODE (stack_pointer_rtx),
+			    stack_pointer_rtx, -ofst);
+      cselib_add_permanent_equiv (val, expr, get_insns ());
+
+      if (ofst)
+	{
+	  val = cselib_lookup_from_insn (stack_pointer_rtx,
+					 GET_MODE (stack_pointer_rtx), 1,
+					 VOIDmode, get_insns ());
+	  preserve_value (val);
+	  expr = plus_constant (GET_MODE (reg), reg, ofst);
+	  cselib_add_permanent_equiv (val, expr, get_insns ());
+	}
+    }
+
   /* In order to factor out the adjustments made to the stack pointer or to
      the hard frame pointer and thus be able to use DW_OP_fbreg operations
      instead of individual location lists, we're going to rewrite MEMs based

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: vta-canonicalize-addr-with-dynamic-set-pr53671-pr49888.patch --]
[-- Type: text/x-diff, Size: 5328 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/53671
	PR debug/49888
	* var-tracking.c (vt_get_canonicalize_base): New.
	(vt_canonicalize_addr, vt_stack_offset_p): New.
	(vt_canon_true_dep): New.
	(drop_overlapping_mem_locs): Use vt_canon_true_dep.
	(clobber_overlaping_mems): Use vt_canonicalize_addr.
	
Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2012-06-17 23:01:02.525351931 -0300
+++ gcc/var-tracking.c	2012-06-17 23:01:06.839010574 -0300
@@ -1955,6 +1955,144 @@ var_regno_delete (dataflow_set *set, int
   *reg = NULL;
 }
 
+/* Strip constant offsets and alignments off of LOC.  Return the base
+   expression.  */
+
+static rtx
+vt_get_canonicalize_base (rtx loc)
+{
+  while ((GET_CODE (loc) == PLUS
+	  || GET_CODE (loc) == AND)
+	 && GET_CODE (XEXP (loc, 1)) == CONST_INT
+	 && (GET_CODE (loc) != AND
+	     || INTVAL (XEXP (loc, 1)) < 0))
+    loc = XEXP (loc, 0);
+
+  return loc;
+}
+
+/* Canonicalize LOC using equivalences from SET in addition to those
+   in the cselib static table.  */
+
+static rtx
+vt_canonicalize_addr (dataflow_set *set, rtx oloc)
+{
+  HOST_WIDE_INT ofst = 0;
+  enum machine_mode mode = GET_MODE (oloc);
+  rtx loc = canon_rtx (get_addr (oloc));
+
+  /* Try to substitute a base VALUE for equivalent expressions as much
+     as possible.  The goal here is to expand stack-related addresses
+     to one of the stack base registers, so that we can compare
+     addresses for overlaps.  */
+  while (GET_CODE (vt_get_canonicalize_base (loc)) == VALUE)
+    {
+      rtx x;
+      decl_or_value dv;
+      variable var;
+      location_chain l;
+
+      while (GET_CODE (loc) == PLUS)
+	{
+	  ofst += INTVAL (XEXP (loc, 1));
+	  loc = XEXP (loc, 0);
+	  continue;
+	}
+
+      /* Alignment operations can't normally be combined, so just
+	 canonicalize the base and we're done.  We'll normally have
+	 only one stack alignment anyway.  */
+      if (GET_CODE (loc) == AND)
+	{
+	  x = vt_canonicalize_addr (set, XEXP (loc, 0));
+	  if (x != XEXP (loc, 0))
+	    loc = gen_rtx_AND (mode, x, XEXP (loc, 1));
+	  loc = canon_rtx (get_addr (loc));
+	  break;
+	}
+
+      x = canon_rtx (get_addr (loc));
+
+      /* We've made progress!  Start over.  */
+      if (x != loc || GET_CODE (x) != VALUE)
+	{
+	  loc = x;
+	  continue;
+	}
+
+      dv = dv_from_rtx (x);
+      var = (variable) htab_find_with_hash (shared_hash_htab (set->vars),
+					    dv, dv_htab_hash (dv));
+      if (!var)
+	break;
+
+      /* Look for an improved equivalent expression.  */
+      for (l = var->var_part[0].loc_chain; l; l = l->next)
+	{
+	  rtx base = vt_get_canonicalize_base (l->loc);
+	  if (GET_CODE (base) == REG
+	      || (GET_CODE (base) == VALUE
+		  && canon_value_cmp (base, loc)))
+	    {
+	      loc = l->loc;
+	      break;
+	    }
+	}
+
+      /* No luck with the dataflow set, so we're done.  */
+      if (!l)
+	break;
+    }
+
+  /* Add OFST back in.  */
+  if (ofst)
+    {
+      /* Don't build new RTL if we can help it.  */
+      if (GET_CODE (oloc) == PLUS
+	  && XEXP (oloc, 0) == loc
+	  && INTVAL (XEXP (oloc, 1)) == ofst)
+	return oloc;
+
+      loc = plus_constant (mode, loc, ofst);
+    }
+
+  return loc;
+}
+
+/* Return true iff ADDR has a stack register as the base address.  */
+
+static inline bool
+vt_stack_offset_p (rtx addr)
+{
+  rtx base = vt_get_canonicalize_base (addr);
+
+  if (GET_CODE (base) != REG)
+    return false;
+
+  return REGNO_PTR_FRAME_P (REGNO (base));
+}
+
+/* Return true iff there's a true dependence between MLOC and LOC.
+   MADDR must be a canonicalized version of MLOC's address.  */
+
+static inline bool
+vt_canon_true_dep (dataflow_set *set, rtx mloc, rtx maddr, rtx loc)
+{
+  if (GET_CODE (loc) != MEM)
+    return false;
+
+  if (!canon_true_dependence (mloc, GET_MODE (mloc), maddr, loc, NULL))
+    return false;
+
+  if (!MEM_EXPR (loc) && vt_stack_offset_p (maddr))
+    {
+      rtx addr = vt_canonicalize_addr (set, XEXP (loc, 0));
+      return canon_true_dependence (mloc, GET_MODE (mloc), maddr, loc, addr);
+    }
+
+  return true;
+}
+
 /* Hold parameters for the hashtab traversal function
    drop_overlapping_mem_locs, see below.  */
 
@@ -1988,9 +2126,7 @@ drop_overlapping_mem_locs (void **slot, 
       if (shared_var_p (var, set->vars))
 	{
 	  for (loc = var->var_part[0].loc_chain; loc; loc = loc->next)
-	    if (GET_CODE (loc->loc) == MEM
-		&& canon_true_dependence (mloc, GET_MODE (mloc), addr,
-					  loc->loc, NULL))
+	    if (vt_canon_true_dep (set, mloc, addr, loc->loc))
 	      break;
 
 	  if (!loc)
@@ -2009,9 +2145,7 @@ drop_overlapping_mem_locs (void **slot, 
       for (locp = &var->var_part[0].loc_chain, loc = *locp;
 	   loc; loc = *locp)
 	{
-	  if (GET_CODE (loc->loc) != MEM
-	      || !canon_true_dependence (mloc, GET_MODE (mloc), addr,
-					 loc->loc, NULL))
+	  if (!vt_canon_true_dep (set, mloc, addr, loc->loc))
 	    {
 	      locp = &loc->next;
 	      continue;
@@ -2052,7 +2186,7 @@ clobber_overlapping_mems (dataflow_set *
 
   coms.set = set;
   coms.loc = canon_rtx (loc);
-  coms.addr = canon_rtx (get_addr (XEXP (loc, 0)));
+  coms.addr = vt_canonicalize_addr (set, XEXP (loc, 0));
 
   set->traversed_vars = set->vars;
   htab_traverse (shared_hash_htab (set->vars),

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: vta-drop-redundant-cfa-base-recording.patch --]
[-- Type: text/x-diff, Size: 789 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/53671
	PR debug/49888
	* var-tracking.c (vt_init_cfa_base): Drop redundant recording of
	CFA base.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2012-06-18 08:44:13.459569497 -0300
+++ gcc/var-tracking.c	2012-06-18 08:55:31.023984364 -0300
@@ -9582,9 +9582,6 @@ vt_init_cfa_base (void)
 				 VOIDmode, get_insns ());
   preserve_value (val);
   cselib_preserve_cfa_base_value (val, REGNO (cfa_base_rtx));
-  var_reg_decl_set (&VTI (ENTRY_BLOCK_PTR)->out, cfa_base_rtx,
-		    VAR_INIT_STATUS_INITIALIZED, dv_from_value (val->val_rtx),
-		    0, NULL_RTX, INSERT);
 }
 
 /* Allocate and initialize the data structures for variable tracking

[-- Attachment #6: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Richard Guenther]

On Wed, Jun 20, 2012 at 5:52 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Jun 16, 2012, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>
>> If I understand it correctly, the new approach fails to handle push
>> properly.
>
> It's actually cselib that didn't deal with push properly, so it thinks
> incoming stack arguments may be clobbered by them.  But that's not the
> whole story, unfortunately.  I still don't have a complete fix for the
> problem, but I have some patches that restore nearly all of the passes.
>
> The first one extends RTX alias analysis so that cselib can recognize
> that (mem:SI ARGP) and (mem:SI (plus (and (plus ARGP #-4) #-32) #-4))
> don't alias.  Before the patch, we'd go for infinite sized objects upon
> AND.

Just looking at this patch.  It looks reasonable, but I have a question on
the pre-existing condition

-      if (GET_CODE (y) == AND || ysize < -INTVAL (XEXP (x, 1)))
        xsize = -1;

so if this condition is not true then we simply strip off the AND of X and
do not adjust xsize at all?  Likewise we do not adjust c?  How can that
be conservatively correct?

Thus, I'd rather see

   if (GET_CODE (x) == AND && CONST_INT_P (XEXP (x, 1)))
     {
+      HOST_WIDE_INT sc = INTVAL (XEXP (x, 1));
+      unsigned HOST_WIDE_INT uc = sc;
+      if (xsize > 0 && sc < 0 && -uc == (uc & -uc))
+       {
+         xsize -= sc + 1;
+         c -= sc;
           return memrefs_conflict_p (xsize, canon_rtx (XEXP (x, 0)),
ysize, y, c);
         }
      }

as the sole supported case.  The patch is ok with the above change if it
passes bootstrap & regtest.  I suppose part of the comment that maybe
tries to explain the existing code (but does not match what it does IMHO),

    Assume that references
     besides AND are aligned, so if the size of the other reference is
     at least as large as the alignment, assume no other overlap.  */

should be removed then.

Thanks,
Richard.

>
> The second introduces an entry-point equivalence between ARGP and SP, so
> that SP references in push and stack-align sequences can be
> canonicalized to ARGP-based.
>
> The third introduces address canonicalization that uses information in
> the dataflow variable set in addition to the static cselib table.  This
> is the one I'm still working on, because some expressions still fail to
> canonicalize to ARGP although they could.
>
> The fourth removes a now-redundant equivalence from the dynamic table;
> the required information is always preserved in the static table.
>
> I've regstrapped (and checked results! :-) all of these on
> x86_64-linux-gnu and i686-linux-gnu.  It fixes all visible regressions
> in x86_64-linux-gnu, and nearly all on i686-linux-gnu.
>
> May I check these in and keep on working to complete the fix, or should
> I revert the original patch and come back only with a patchset that
> fixes all debug info regressions?
>
>
>
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer
>

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Jakub Jelinek]

On Wed, Jun 20, 2012 at 12:52:12AM -0300, Alexandre Oliva wrote:
> On Jun 16, 2012, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/53671
> 	PR debug/49888
> 	* alias.c (memrefs_conflict_p): Improve handling of AND for
> 	alignment.
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/53671
> 	PR debug/49888
> 	* var-tracking.c (vt_initialize): Record initial offset between
> 	arg pointer and stack pointer.
> 
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/53671
> 	PR debug/49888
> 	* var-tracking.c (vt_get_canonicalize_base): New.
> 	(vt_canonicalize_addr, vt_stack_offset_p): New.
> 	(vt_canon_true_dep): New.
> 	(drop_overlapping_mem_locs): Use vt_canon_true_dep.
> 	(clobber_overlaping_mems): Use vt_canonicalize_addr.
>
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	PR debug/53671
> 	PR debug/49888
> 	* var-tracking.c (vt_init_cfa_base): Drop redundant recording of
> 	CFA base.

Ok, thanks.

	Jakub

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Alexandre Oliva]

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

On Jun 20, 2012, Richard Guenther <richard.guenther@gmail.com> wrote:

> I have a question on the pre-existing condition

> -      if (GET_CODE (y) == AND || ysize < -INTVAL (XEXP (x, 1)))
>         xsize = -1;

> so if this condition is not true then we simply strip off the AND of X and
> do not adjust xsize at all?  Likewise we do not adjust c?  How can that
> be conservatively correct?

Yeah, xsize = -1 makes x “infinitely large”, so it will overlap if the
RTXs are in any way related, or something like that.

> Thus, I'd rather see

>    if (GET_CODE (x) == AND && CONST_INT_P (XEXP (x, 1)))
>      {
> +      HOST_WIDE_INT sc = INTVAL (XEXP (x, 1));
> +      unsigned HOST_WIDE_INT uc = sc;
> +      if (xsize > 0 && sc < 0 && -uc == (uc & -uc))
> +       {
> +         xsize -= sc + 1;
> +         c -= sc;
>            return memrefs_conflict_p (xsize, canon_rtx (XEXP (x, 0)),
> ysize, y, c);
>          }
>       }

> as the sole supported case.

Ack.  Regstrapped successfully, checking this in.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: alias-align-exprs-pr53671-pr49888.patch --]
[-- Type: text/x-diff, Size: 2199 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/53671
	PR debug/49888
	* alias.c (memrefs_conflict_p): Improve handling of AND for
	alignment.
	
Index: gcc/alias.c
===================================================================
--- gcc/alias.c.orig	2012-06-21 15:05:48.144424495 -0300
+++ gcc/alias.c	2012-06-21 15:21:56.000000000 -0300
@@ -2097,25 +2097,32 @@ memrefs_conflict_p (int xsize, rtx x, in
 	break;
       }
 
-  /* Treat an access through an AND (e.g. a subword access on an Alpha)
-     as an access with indeterminate size.  Assume that references
-     besides AND are aligned, so if the size of the other reference is
-     at least as large as the alignment, assume no other overlap.  */
+  /* Deal with alignment ANDs by adjusting offset and size so as to
+     cover the maximum range, without taking any previously known
+     alignment into account.  */
   if (GET_CODE (x) == AND && CONST_INT_P (XEXP (x, 1)))
     {
-      if (GET_CODE (y) == AND || ysize < -INTVAL (XEXP (x, 1)))
-	xsize = -1;
-      return memrefs_conflict_p (xsize, canon_rtx (XEXP (x, 0)), ysize, y, c);
+      HOST_WIDE_INT sc = INTVAL (XEXP (x, 1));
+      unsigned HOST_WIDE_INT uc = sc;
+      if (xsize > 0 && sc < 0 && -uc == (uc & -uc))
+	{
+	  xsize -= sc + 1;
+	  c -= sc;
+	  return memrefs_conflict_p (xsize, canon_rtx (XEXP (x, 0)),
+				     ysize, y, c);
+	}
     }
   if (GET_CODE (y) == AND && CONST_INT_P (XEXP (y, 1)))
     {
-      /* ??? If we are indexing far enough into the array/structure, we
-	 may yet be able to determine that we can not overlap.  But we
-	 also need to that we are far enough from the end not to overlap
-	 a following reference, so we do nothing with that for now.  */
-      if (GET_CODE (x) == AND || xsize < -INTVAL (XEXP (y, 1)))
-	ysize = -1;
-      return memrefs_conflict_p (xsize, x, ysize, canon_rtx (XEXP (y, 0)), c);
+      HOST_WIDE_INT sc = INTVAL (XEXP (y, 1));
+      unsigned HOST_WIDE_INT uc = sc;
+      if (ysize > 0 && sc < 0 && -uc == (uc & -uc))
+	{
+	  ysize -= sc + 1;
+	  c += sc;
+	  return memrefs_conflict_p (xsize, x,
+				     ysize, canon_rtx (XEXP (y, 0)), c);
+	}
     }
 
   if (CONSTANT_P (x))

[-- Attachment #3: Type: text/plain, Size: 258 bytes --]



-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Alexandre Oliva]

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

Here's one more patch that addresses a problem I found out while
investigating the PR53671 regressions: rather than recording incoming
stack args as MEMs with non-VALUE expressions, it's more consistent (and
less surprising) if we emit them as VALUE expressions, like other MEMs.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-record-incoming-parm-addr-by-value.patch --]
[-- Type: text/x-diff, Size: 855 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* var-tracking.c (vt_add_function_parameter): Use a preserved
	VALUE for the MEM address of an incoming parameter.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2012-06-21 22:26:01.341145372 -0300
+++ gcc/var-tracking.c	2012-06-21 22:26:09.644487877 -0300
@@ -9454,6 +9454,17 @@ vt_add_function_parameter (tree parm)
 			     VAR_INIT_STATUS_INITIALIZED, NULL, INSERT);
 	  dv = dv_from_value (val->val_rtx);
 	}
+
+      if (MEM_P (incoming))
+	{
+	  val = cselib_lookup_from_insn (XEXP (incoming, 0), mode, true,
+					 VOIDmode, get_insns ());
+	  if (val)
+	    {
+	      preserve_value (val);
+	      incoming = replace_equiv_address_nv (incoming, val->val_rtx);
+	    }
+	}
     }
 
   if (REG_P (incoming))

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Jakub Jelinek]

On Thu, Jun 21, 2012 at 10:58:05PM -0300, Alexandre Oliva wrote:
> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	* var-tracking.c (vt_add_function_parameter): Use a preserved
> 	VALUE for the MEM address of an incoming parameter.

Okay.

	Jakub

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Alexandre Oliva]

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

On Jun 14, 2012, "H.J. Lu" <hjl.tools@gmail.com> wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53671

These two patches cure the two remaining problems.

Basically, we have a problem of tracking equivalent FP-relative
addresses across basic blocks when SP varies.  Once we lost track of SP,
pushing an argument for a call (without any MEM_EXPRs) was regarded as
aliasing an incoming argument, so we removed the argument binding from
the var tracking table.

This patch is a bit kludgy in that, in a perfect world, we'd be able to
recognize a broader set of equivalent incoming expressions at dataflow
confluences, taking both static and dynamic sets into account.
Currently, we only take the dynamic set into account, and don't
recognize as equivalent arithmetically equivalent expressions that
aren't isomorphic.

Fixing the more general problem is more than I can tackle right now, in
part because I don't have an efficient algorithm in mind and I'm
concerned a brute-force approach may be far too expensive in terms of
CPU and memory.  In fact, this is *the* main open problem in VTA, that
I'd like to discuss at the Cauldron with interested parties.

So I put in a kludge that tries to canonicalize incoming SPs and, if
they match, record that the merged SP holds the same canonical
representation.  It solves one of the regressions, but it will *not*
solve the general problem: AFAICT, a function that calls alloca,
especially if it calls it in a loop, will not get a proper relationship
between SP and FP so as to enable alias analysis to tell that a stack
push for an outgoing argument doesn't alias an FP-relative incoming
argument.

Perhaps defining separate alias sets for the various portions of the
stack frame, and annotating pushes that save registers and pushes of
outgoing arguments with these alias sets might help, but I haven't tried
that.


The second patch simply adjusts a testcase to account for an
optimization possibility I hadn't taken into account that happened to be
exposed by the initial PR49888 patch, and the change happens to hide the
failure caused by a variant of the alias analysis problem described
above: we have an incoming pointer and a register-saving stack push that
must not alias because the incoming pointer couldn't possibly point to
the register-save area of a callee in a well-defined program, but alias
analysis can't figure that out.


Both patches were regstrapped on x86_64-linux-gnu and i686-linux-gnu.
Ok?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-canonicalize-stack-pointer-tracking-pr53671-pr49888.patch --]
[-- Type: text/x-diff, Size: 4741 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/53671
	PR debug/49888
	* var-tracking.c (attrs_list_by_loc_eq): New.
	(track_stack_pointer): New.
	(dataflow_set_merge): Use it.
	(vt_initialize): Record the initial stack pointer in the
	dataflow set.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2012-06-26 17:33:12.991323578 -0300
+++ gcc/var-tracking.c	2012-06-26 17:51:55.316453808 -0300
@@ -1462,6 +1462,17 @@ attrs_list_member (attrs list, decl_or_v
   return NULL;
 }
 
+/* Return the entry whose LOC field equals LOC.  */
+
+static attrs
+attrs_list_by_loc_eq (attrs list, rtx loc)
+{
+  for (; list; list = list->next)
+    if (list->loc == loc)
+      return list;
+  return NULL;
+}
+
 /* Insert the triplet DECL, OFFSET, LOC to the list *LISTP.  */
 
 static void
@@ -4028,6 +4039,86 @@ variable_merge_over_src (variable s2var,
   return 1;
 }
 
+/* Add to DST any needed binding for the canonicalization of the stack
+   pointer to yield the same expression as in SRC1 and SRC2, if they
+   both yield the same expression.
+
+   Return TRUE iff we found an equivalence.
+
+   ??? The return value, that was useful during testing, ended up
+   unused, but this single-use static function will be inlined, and
+   then the return value computation will be optimized out, so I'm
+   leaving it in.
+
+   ??? We use this kludge to avoid accidental aliasing between
+   incoming arguments and register-saving or outgoing-args pushes.  We
+   shouldn't have to add explicit stack pointer tracking for that:
+   intersect_loc_chains ought to be able to take information from the
+   static cselib table and recognize additional equivalences, but we
+   don't have a sufficiently efficient algorithm to do that yet.  */
+
+static bool
+track_stack_pointer (dataflow_set *dst, dataflow_set *src1, dataflow_set *src2)
+{
+  attrs dattr, s1attr, s2attr;
+  rtx daddr, s1addr, s2addr;
+  decl_or_value dv;
+
+  for (dattr = dst->regs[STACK_POINTER_REGNUM];
+       (dattr = attrs_list_by_loc_eq (dattr, stack_pointer_rtx))
+	 && (dattr->offset || !dv_is_value_p (dattr->dv));
+       dattr = dattr->next)
+    ;
+
+  for (s1attr = src1->regs[STACK_POINTER_REGNUM];
+       (s1attr = attrs_list_by_loc_eq (s1attr, stack_pointer_rtx))
+	 && (s1attr->offset || !dv_is_value_p (s1attr->dv));
+       s1attr = s1attr->next)
+    ;
+  if (!s1attr)
+    return false;
+
+  for (s2attr = src2->regs[STACK_POINTER_REGNUM];
+       (s2attr = attrs_list_by_loc_eq (s2attr, stack_pointer_rtx))
+	 && (s2attr->offset || !dv_is_value_p (s2attr->dv));
+       s2attr = s2attr->next)
+    ;
+  if (!s2attr)
+    return false;
+
+  if (dattr)
+    daddr = vt_canonicalize_addr (dst, dv_as_value (dattr->dv));
+  else
+    daddr = NULL;
+  s1addr = vt_canonicalize_addr (src1, dv_as_value (s1attr->dv));
+  s2addr = vt_canonicalize_addr (src2, dv_as_value (s2attr->dv));
+
+  if (!rtx_equal_p (s1addr, s2addr))
+    return false;
+
+  if (daddr && rtx_equal_p (daddr, s1addr))
+    return true;
+
+  dst_can_be_shared = false;
+  if (daddr)
+    dv = dattr->dv;
+  else if (vt_get_canonicalize_base (s1addr)
+	   != (cfa_base_rtx ? cfa_base_rtx : arg_pointer_rtx))
+    return false;
+  else
+    {
+      cselib_val *val = cselib_lookup (s1addr, GET_MODE (s1addr), 1, VOIDmode);
+      cselib_preserve_value (val);
+      dv = dv_from_value (val->val_rtx);
+    }
+
+  var_reg_decl_set (dst, stack_pointer_rtx,
+		    VAR_INIT_STATUS_INITIALIZED,
+		    dv, 0, NULL_RTX, INSERT);
+
+  return true;
+}
+
 /* Combine dataflow set information from SRC2 into DST, using PDST
    to carry over information across passes.  */
 
@@ -4066,6 +4157,8 @@ dataflow_set_merge (dataflow_set *dst, d
   FOR_EACH_HTAB_ELEMENT (shared_hash_htab (dsm.cur->vars), var, variable, hi)
     variable_merge_over_cur (var, &dsm);
 
+  track_stack_pointer (dst, src1, src2);
+
   if (dsm.src_onepart_cnt)
     dst_can_be_shared = false;
 
@@ -9682,6 +9775,18 @@ vt_initialize (void)
 	  expr = plus_constant (GET_MODE (reg), reg, ofst);
 	  cselib_add_permanent_equiv (val, expr, get_insns ());
 	}
+
+      /* VAL below will generally be the one set within the
+	 conditional block, but if OFST happens to be zero, we'll be
+	 happy to use the one corresponding to REG.
+
+	 ??? We shouldn't need this any more once dataflow merges
+	 start using equivalences from the cselib table in addition to
+	 those in dataflow sets.  */
+      var_reg_decl_set (&VTI (ENTRY_BLOCK_PTR)->out,
+			stack_pointer_rtx, VAR_INIT_STATUS_INITIALIZED,
+			dv_from_value (val->val_rtx), 0, NULL_RTX,
+			INSERT);
     }
 
   /* In order to factor out the adjustments made to the stack pointer or to

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: vta-adjust-testsuite-pr53671-pr49888.patch --]
[-- Type: text/x-diff, Size: 1152 bytes --]

for  gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/53671
	PR debug/49888
	* gcc.dg/guality/pr49888.c: Account for the possibility that
	the variable is optimized out at the first test.

Index: gcc/testsuite/gcc.dg/guality/pr49888.c
===================================================================
--- gcc/testsuite/gcc.dg/guality/pr49888.c.orig	2012-06-25 20:55:13.465033665 -0300
+++ gcc/testsuite/gcc.dg/guality/pr49888.c	2012-06-25 21:33:00.035392709 -0300
@@ -9,12 +9,13 @@ f (int *p)
 {
   int c = *p;
   v = c;
-  *p = 1; /* { dg-final { gdb-test 12 "c" "0" } } */
+  *p = 1; /* { dg-final { gdb-test 12 "!!c" "0" } } */
   /* c may very well be optimized out at this point, so we test !c,
      which will evaluate to the expected value.  We just want to make
      sure it doesn't remain bound to *p as it did before, in which
-     case !c would evaluate to 0.  */
-  v = 0; /* { dg-final { gdb-test 17 "!c" "1" } } */
+     case !c would evaluate to 0.  *p may also be regarded as aliasing
+     register saves, thus the !!c above.  */
+  v = 0; /* { dg-final { gdb-test 18 "!c" "1" } } */
 }
 int
 main ()

[-- Attachment #4: Type: text/plain, Size: 258 bytes --]



-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Richard Henderson]

On 06/26/2012 01:54 PM, Alexandre Oliva wrote:
> +  track_stack_pointer (dst, src1, src2);

Why does this function return a value then?


r~

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Alexandre Oliva]

On Jun 27, 2012, Richard Henderson <rth@redhat.com> wrote:

> On 06/26/2012 01:54 PM, Alexandre Oliva wrote:
>> +  track_stack_pointer (dst, src1, src2);

> Why does this function return a value then?

During testing, I used an assert on the return value to catch cases that
couldn't be handled.  The comments before that function say:

+   ??? The return value, that was useful during testing, ended up
+   unused, but this single-use static function will be inlined, and
+   then the return value computation will be optimized out, so I'm
+   leaving it in.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Alexandre Oliva]

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

On Jun 21, 2012, Alexandre Oliva <aoliva@redhat.com> wrote:

> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>

> 	PR debug/53671
> 	PR debug/49888
> 	* alias.c (memrefs_conflict_p): Improve handling of AND for
> 	alignment.
	
There was a thinko in this patch.  We can't move the offset by more than
we adjust the size, otherwise we fail to cover part of the required
range, as reported in PR rtl-optimization/53827.  This obvious patch
fixes it.  Regstrapped on x86_64- and i686-linux-gnu.  Uros confirmed it
fixed the bootstrap problem on alpha-linux-gnu.  I'm checking this in.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: alias-align-exprs-oops-pr53827-pr53671-pr49888.patch --]
[-- Type: text/x-diff, Size: 939 bytes --]

for  gcc/ChangeLog
from  Alexandre Oilva  <aoliva@redhat.com>

	PR rtl-optimization/53827
	PR debug/53671
	PR debug/49888
	* alias.c (memrefs_conflict_p): Adjust offset and size by the
	same amount for alignment ANDs.

Index: gcc/alias.c
===================================================================
--- gcc/alias.c.orig	2012-07-02 09:04:06.193137799 -0300
+++ gcc/alias.c	2012-07-02 09:04:08.000000000 -0300
@@ -2107,7 +2107,7 @@ memrefs_conflict_p (int xsize, rtx x, in
       if (xsize > 0 && sc < 0 && -uc == (uc & -uc))
 	{
 	  xsize -= sc + 1;
-	  c -= sc;
+	  c -= sc + 1;
 	  return memrefs_conflict_p (xsize, canon_rtx (XEXP (x, 0)),
 				     ysize, y, c);
 	}
@@ -2119,7 +2119,7 @@ memrefs_conflict_p (int xsize, rtx x, in
       if (ysize > 0 && sc < 0 && -uc == (uc & -uc))
 	{
 	  ysize -= sc + 1;
-	  c += sc;
+	  c += sc + 1;
 	  return memrefs_conflict_p (xsize, x,
 				     ysize, canon_rtx (XEXP (y, 0)), c);
 	}

[-- Attachment #3: Type: text/plain, Size: 258 bytes --]



-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [PR49888, VTA] don't keep VALUEs bound to modified MEMs

[Alexandre Oliva]

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

On Jun 21, 2012, Alexandre Oliva <aoliva@redhat.com> wrote:

> Here's one more patch that addresses a problem I found out while
> investigating the PR53671 regressions: rather than recording incoming
> stack args as MEMs with non-VALUE expressions, it's more consistent (and
> less surprising) if we emit them as VALUE expressions, like other MEMs.

> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok?


> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>

> 	* var-tracking.c (vt_add_function_parameter): Use a preserved
> 	VALUE for the MEM address of an incoming parameter.

This caused the bug described in PR debug/53820.

PA64 uses a pseudo for the internal arg pointer.  This pseudo isn't
recorded as a preserved VALUE, and it doesn't really make sense to
record a use of this pseudo at the entry point, before it is even set.
There's also code that prevents us from using pseudos in var-tracking.

This patch arranges for the internal arg pointer to be substituted for
the actual arg pointer.  John David Angling verified that the patch
fixes the problem on the affected platform.  I regstrapped it on x86_64-
and i686-linux-gnu.  I'm checking it in momentarily, as obvious.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-record-incoming-parm-addr-without-internal-arg-pointer-pr53820.patch --]
[-- Type: text/x-diff, Size: 1448 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/53820
	* var-tracking.c (vt_add_function_parameter): Convert
	internal_arg_pointer into arg_pointer-based address even
	without DRAP.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2012-07-02 10:51:19.036061050 -0300
+++ gcc/var-tracking.c	2012-07-02 12:14:04.000000000 -0300
@@ -9327,14 +9327,11 @@ vt_add_function_parameter (tree parm)
   if (GET_MODE (decl_rtl) == BLKmode || GET_MODE (incoming) == BLKmode)
     return;
 
-  /* If there is a DRAP register, rewrite the incoming location of parameters
-     passed on the stack into MEMs based on the argument pointer, as the DRAP
-     register can be reused for other purposes and we do not track locations
-     based on generic registers.  But the prerequisite is that this argument
-     pointer be also the virtual CFA pointer, see vt_initialize.  */
+  /* If there is a DRAP register or a pseudo in internal_arg_pointer,
+     rewrite the incoming location of parameters passed on the stack
+     into MEMs based on the argument pointer, so that incoming doesn't
+     depend on a pseudo.  */
   if (MEM_P (incoming)
-      && stack_realign_drap
-      && arg_pointer_rtx == cfa_base_rtx
       && (XEXP (incoming, 0) == crtl->args.internal_arg_pointer
 	  || (GET_CODE (XEXP (incoming, 0)) == PLUS
 	      && XEXP (XEXP (incoming, 0), 0)

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer