Patchwork [PR,debug/46240] don't introduce debug bind stmts at merge edges

login
register
mail settings
Submitter Alexandre Oliva
Date Jan. 17, 2011, 3:34 p.m.
Message ID <orfwsrh7d8.fsf@livre.localdomain>
Download mbox | patch
Permalink /patch/79193/
State New
Headers show

Comments

Alexandre Oliva - Jan. 17, 2011, 3:34 p.m.
A throwing call must be at the end of a BB, right?  Well, then...  what
if we need to insert a debug bind stmt right after the call, because the
result of the call is assigned to a user variable?

If the fallthrough edge has a single pred, as usual, no problem,
inserting on the edge amounts to inserting in the beginning of the
subsequent block.  However, if there's more than one predecessor, we
can't insert the debug bind stmt at all, because inserting at the edge
would amount to creating a new BB just for this one debug stmt, which
would change the CFG and trigger undesirable codegen differences.

We used to guard against fallthrough BBs without a single predecessor,
but now that I see it's possible, I've reworked the code to deal with
this situation.  We still don't emit the debug bind stmt, but that's
because we don't have to: if we're getting to a confluence point that
joins the path with the call, that set a user variable, and another in
which the assignment wasn't present, there is going to be a PHI node
there, and there will be a debug bind stmt corresponding to that PHI
node, so the debug bind stmt we'd emit would be redundant anyway.  So,
we can just refrain from emitting it, without loss of debug info.  Yay!

Here's the patch.  Regstrapped on x86_64&32-linux-gnu.  Ok to install?
Richard Guenther - Jan. 17, 2011, 3:55 p.m.
On Mon, Jan 17, 2011 at 4:34 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> A throwing call must be at the end of a BB, right?  Well, then...  what
> if we need to insert a debug bind stmt right after the call, because the
> result of the call is assigned to a user variable?
>
> If the fallthrough edge has a single pred, as usual, no problem,
> inserting on the edge amounts to inserting in the beginning of the
> subsequent block.  However, if there's more than one predecessor, we
> can't insert the debug bind stmt at all, because inserting at the edge
> would amount to creating a new BB just for this one debug stmt, which
> would change the CFG and trigger undesirable codegen differences.
>
> We used to guard against fallthrough BBs without a single predecessor,
> but now that I see it's possible, I've reworked the code to deal with
> this situation.  We still don't emit the debug bind stmt, but that's
> because we don't have to: if we're getting to a confluence point that
> joins the path with the call, that set a user variable, and another in
> which the assignment wasn't present, there is going to be a PHI node
> there, and there will be a debug bind stmt corresponding to that PHI
> node, so the debug bind stmt we'd emit would be redundant anyway.  So,
> we can just refrain from emitting it, without loss of debug info.  Yay!
>
> Here's the patch.  Regstrapped on x86_64&32-linux-gnu.  Ok to install?

Ok.  I guess we are sure we never have an unused LHS and thus
no PHI node for the result?  Thus, I'd be more happy when that
asserts for phi_nodes would not be there (I think we can also have
non-cleaned up degenerate PHIs).

Thanks,
Richard.

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

Patch

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

	PR debug/46240
	* tree-into-ssa.c (maybe_register_def): Do not attempt to add
	debug bind stmt on merge edges.

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

	PR debug/46240
	* g++.dg/debug/pr46240.cc: New.

Index: gcc/tree-into-ssa.c
===================================================================
--- gcc/tree-into-ssa.c.orig	2011-01-13 04:04:25.411690621 -0200
+++ gcc/tree-into-ssa.c	2011-01-13 05:34:29.762940340 -0200
@@ -1869,11 +1869,32 @@  maybe_register_def (def_operand_p def_p,
 			gcc_assert (!ef);
 			ef = e;
 		      }
-		  gcc_assert (ef
-			      && single_pred_p (ef->dest)
-			      && !phi_nodes (ef->dest)
-			      && ef->dest != EXIT_BLOCK_PTR);
-		  gsi_insert_on_edge_immediate (ef, note);
+		  /* If there are other predecessors to ef->dest, then
+		     there must be PHI nodes for the modified
+		     variable, and therefore there will be debug bind
+		     stmts after the PHI nodes.  The debug bind notes
+		     we'd insert would force the creation of a new
+		     block (diverging codegen) and be redundant with
+		     the post-PHI bind stmts, so don't add them.
+
+		     As for the exit edge, there wouldn't be redundant
+		     bind stmts, but there wouldn't be a PC to bind
+		     them to either, so avoid diverging the CFG.  */
+		  if (ef && single_pred_p (ef->dest)
+		      && ef->dest != EXIT_BLOCK_PTR)
+		    {
+		      /* If there were PHI nodes in the node, we'd
+			 have to make sure the value we're binding
+			 doesn't need rewriting.  But there shouldn't
+			 be PHI nodes in a single-predecessor block,
+			 so just check our assumption is correct.  */
+		      gcc_checking_assert (!phi_nodes (ef->dest));
+		      gsi_insert_on_edge_immediate (ef, note);
+		    }
+		  else
+		    gcc_checking_assert (ef
+					 && (ef->dest == EXIT_BLOCK_PTR
+					     || phi_nodes (ef->dest)));
 		}
 	      else
 		gsi_insert_after (&gsi, note, GSI_SAME_STMT);
Index: gcc/testsuite/g++.dg/debug/pr46240.cc
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/g++.dg/debug/pr46240.cc	2011-01-13 04:42:40.460692068 -0200
@@ -0,0 +1,172 @@ 
+// { dg-do compile }
+// { dg-options "-O3 -g" }
+
+template <typename T>
+T &max (T &a, T &b)
+{
+  if (a < b) return b; else return a;
+}
+int foo (double);
+struct S
+{
+  struct T
+  {
+    int dims, count;
+    T (int, int) : dims (), count () {}
+  };
+  T *rep;
+  S () {}
+  S (int r, int c) : rep (new T (r, c)) {}
+  ~S () { delete rep; }
+};
+template <typename T>
+struct U
+{
+  static T epsilon () throw ();
+};
+template <class T>
+struct V
+{
+  struct W
+  {
+    T * data;
+    int count;
+    W (int n) : data (new T[n]), count () {}
+  };
+  V::W *rep;
+  S dimensions;
+  int slice_len;
+  V (S s) : rep (new V <T>::W (get_size (s))) {}
+  int capacity () { return slice_len; }
+  int get_size (S);
+};
+template <class T>
+struct Z : public V <T>
+{
+  Z () : V <T> (S (0, 0)) {}
+  Z (int r, int c) : V <T> (S (r, c)) {}
+};
+template <class T>
+struct A : public Z <T>
+{
+  A () : Z <T> () {}
+  A (int n, int m) : Z <T> (n, m) {}
+};
+template <class T>
+struct B : public V <T>
+{
+};
+struct C : public A <double>
+{
+  C () : A <double> () {}
+  C (int r, int c) : A <double> (r, c) {}
+};
+struct D : public B <double>
+{
+};
+template <class T>
+struct E
+{
+};
+template <class T>
+struct G : public E <T>
+{
+};
+struct H : public G <double>
+{
+};
+template <class R>
+struct I
+{
+  R scl, sum;
+  void accum (R val)
+  {
+    R t = __builtin_fabs (val);
+    if (scl == t)
+      sum += 1;
+  }
+  operator R () { __builtin_sqrt (sum); return R (); }
+};
+template <class R>
+struct J
+{
+  template < class U > void accum (U val) {}
+  operator R () { return R (); }
+};
+template <class R>
+struct K
+{
+  R max;
+  template <class U> void accum (U val)
+  {
+    double z = __builtin_fabs (val);
+    max = ::max (max, z);
+  }
+  operator R () { return max; }
+};
+template <class R>
+struct L
+{
+  unsigned num;
+  template <class U> void accum (U) {}
+  operator R () { return num; }
+};
+template <class T, class R, class S>
+void bar (V <T> &v, R &res, S acc)
+{
+  for (int i = 0; i < v.capacity (); i++)
+    acc.accum ((i));
+  res = acc;
+}
+template <class T, class R>
+void bar (B <T> &v, R)
+{
+  R res;
+  bar (v, res, I <R> ());
+}
+template <class T, class R>
+R bar (A <T> &v, R p)
+{
+  R res;
+  if (p == 2)
+    bar (v, res, I <R> ());
+  else if (p == 1)
+    bar (v, res, J <R> ());
+  else if (p == sizeof (float) ? (p) : foo (p))
+    {
+      if (p > 0)
+	bar (v, res, K <R> ());
+    }
+  else if (p == 0)
+    bar (v, res, L <R> ());
+  return res;
+}
+template <class CT, class VectorT, class R>
+void
+baz (CT m, R p, R tol, int maxiter, VectorT)
+{
+  VectorT y (0, 0), z (0, 1);
+  R q = 0;
+  R gamma = 0, gamma1 = 0;
+  gamma = bar (y, p);
+  (void) (bar (z, q) <= (gamma1 <= gamma));
+}
+int a = 100;
+template <class CT, class VectorT, class R>
+void
+test (CT m, R p, VectorT)
+{
+  VectorT x;
+  R sqrteps (U <R>::epsilon ());
+  baz (m, p, sqrteps, a, x);
+}
+void
+fn (D x, double p)
+{
+  bar (x, p);
+}
+void
+fn (H x, double p)
+{
+  test (x, p, C ());
+}