On Jun 14, 2012, "H.J. Lu" <hjl.to...@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?

for  gcc/ChangeLog
from  Alexandre Oliva  <aol...@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
for  gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aol...@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 ()

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

Reply via email to