Patchwork [PR,55253] Propagate aggs_contain_variable flag in aggregater IPA-CP

login
register
mail settings
Submitter Martin Jambor
Date Nov. 13, 2012, 5:03 p.m.
Message ID <20121113170333.GA27663@virgil.arch.suse.de>
Download mbox | patch
Permalink /patch/198743/
State New
Headers show

Comments

Martin Jambor - Nov. 13, 2012, 5:03 p.m.
Hi,

somehow the propagation of aggs_contain_variable in
merge_aggregate_lattices got lost when I was shuffling the code
around, leading to miscompilations.  This patch puts it back in.

Bootstrapped and tested on x86_64-linux, OK for trunk?

Thanks,

Martin


2012-11-13  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/55253
	* ipa-cp.c (merge_aggregate_lattices): Propagate aggs_contain_variable
	flag.

	* testsuite/gcc.dg/torture/pr55253.c: New test.
	* testsuite/gcc.dg/torture/pr55305.c: Likewise.
Jan Hubicka - Nov. 13, 2012, 5:06 p.m.
> Hi,
> 
> somehow the propagation of aggs_contain_variable in
> merge_aggregate_lattices got lost when I was shuffling the code
> around, leading to miscompilations.  This patch puts it back in.
> 
> Bootstrapped and tested on x86_64-linux, OK for trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 2012-11-13  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR tree-optimization/55253
> 	* ipa-cp.c (merge_aggregate_lattices): Propagate aggs_contain_variable
> 	flag.

OK, thanks!
Honza
Ian Taylor - Nov. 13, 2012, 9:08 p.m.
On Tue, Nov 13, 2012 at 9:03 AM, Martin Jambor <mjambor@suse.cz> wrote:

> Index: src/gcc/ipa-cp.c
> ===================================================================
> --- src.orig/gcc/ipa-cp.c
> +++ src/gcc/ipa-cp.c
> @@ -1276,6 +1276,8 @@ merge_aggregate_lattices (struct cgraph_
>      return true;
>    if (src_plats->aggs_bottom)
>      return set_agg_lats_contain_variable (dest_plats);
> +  if (src_plats->aggs_contain_variable)
> +    ret |= set_agg_lats_contain_variable (dest_plats);
>    dst_aglat = &dest_plats->aggs;

Out of curiousity, why are the lines just above the ones you added not

    if (src_plats->aggs_bottom)
      return set_agg_lats_to_bottom (dest_plats);

?

Ian
Martin Jambor - Nov. 14, 2012, 1:21 p.m.
Hi,

On Tue, Nov 13, 2012 at 01:08:56PM -0800, Ian Lance Taylor wrote:
> On Tue, Nov 13, 2012 at 9:03 AM, Martin Jambor <mjambor@suse.cz> wrote:
> 
> > Index: src/gcc/ipa-cp.c
> > ===================================================================
> > --- src.orig/gcc/ipa-cp.c
> > +++ src/gcc/ipa-cp.c
> > @@ -1276,6 +1276,8 @@ merge_aggregate_lattices (struct cgraph_
> >      return true;
> >    if (src_plats->aggs_bottom)
> >      return set_agg_lats_contain_variable (dest_plats);
> > +  if (src_plats->aggs_contain_variable)
> > +    ret |= set_agg_lats_contain_variable (dest_plats);
> >    dst_aglat = &dest_plats->aggs;
> 
> Out of curiousity, why are the lines just above the ones you added not
> 
>     if (src_plats->aggs_bottom)
>       return set_agg_lats_to_bottom (dest_plats);
> 

Since IPA-CP started gathering multiple known constant values per
lattice and can create specialized clones for just some contexts, our
bottom is not the classic bottom from simple constant propagation any
more (classic bottom would be contains_variable flag set and no known
values).  Bottom is a sort of unusable flag, but it does not make
other lattices unusable.  They should be quite rare, they occur when
their function is not local and also not versionable or not clonable,
when we accumulated too many known constants in a lattice (so that it
still adheres to the definition of lattice, here it really means a
lattice bottom) or, in case of propagation of values in aggregates,
when there is a (weird) mix of values passed by reference and values
passed by value.  At the moment, there are in fact no other reasons to
create a bottom lattice, though there might be more in the future.

If we know there might be more than the listed known values,
contains_variable and aggs_contain_variable flags are the way to go.

Martin

Patch

Index: src/gcc/ipa-cp.c
===================================================================
--- src.orig/gcc/ipa-cp.c
+++ src/gcc/ipa-cp.c
@@ -1276,6 +1276,8 @@  merge_aggregate_lattices (struct cgraph_
     return true;
   if (src_plats->aggs_bottom)
     return set_agg_lats_contain_variable (dest_plats);
+  if (src_plats->aggs_contain_variable)
+    ret |= set_agg_lats_contain_variable (dest_plats);
   dst_aglat = &dest_plats->aggs;
 
   for (struct ipcp_agg_lattice *src_aglat = src_plats->aggs;
Index: src/gcc/testsuite/gcc.dg/torture/pr55253.c
===================================================================
--- /dev/null
+++ src/gcc/testsuite/gcc.dg/torture/pr55253.c
@@ -0,0 +1,48 @@ 
+/* { dg-do run } */
+
+struct
+{
+    int mallocFailed;
+}
+*a;
+
+struct StrAccum
+{
+    int useMalloc;
+}
+b, c;
+
+static void
+fn1 (struct StrAccum *p1, int p2)
+{
+    if (p2 == 0)
+        return;
+    if (p1->useMalloc)
+        a->mallocFailed = 0;
+}
+
+void
+fn2 (struct StrAccum *p1)
+{
+    fn1 (p1, 1);
+}
+
+void
+fn3 (struct StrAccum *p1)
+{
+    fn1 (p1, 1);
+}
+
+void
+fn4 ()
+{
+    c.useMalloc = 1;
+    fn1 (&c, 0);
+}
+
+int
+main ()
+{
+    fn3 (&b);
+    return 0;
+}
Index: src/gcc/testsuite/gcc.dg/torture/pr55305.c
===================================================================
--- /dev/null
+++ src/gcc/testsuite/gcc.dg/torture/pr55305.c
@@ -0,0 +1,151 @@ 
+/* { dg-do run } */
+
+extern void exit (int) __attribute__ ((noreturn));
+extern void abort (void) __attribute__ ((noreturn));
+
+struct t
+{
+  char dummy;
+};
+
+struct m
+{
+  const struct t *t;
+  void (*m)(void);
+};
+
+struct s
+{
+  const struct m *m;
+  void *o;
+};
+
+struct e
+{
+  const struct t *t;
+  void *o;
+};
+
+struct ret
+{
+  struct s s;
+  _Bool b;
+};
+
+const struct t t1 = { 1 };
+const struct t t2 = { 2 };
+const struct t t3 = { 3 };
+const struct t t4 = { 4 };
+const struct t t5 = { 5 };
+
+void
+pass (void)
+{
+  exit (0);
+}
+
+void
+fail (void)
+{
+  abort ();
+}
+
+const struct m m1 = { &t4, fail };
+const struct m m2 = { &t5, pass };
+
+static struct e f2 (struct s s2, void *p);
+static struct e f3 (struct s, void *) __attribute__ ((noinline));
+static void f4 (struct s, void *) __attribute__ ((noinline));
+
+struct ret c (struct s, const struct t *) __attribute__ ((noinline));
+
+struct ret
+c (struct s s1, const struct t *t)
+{
+  struct ret r;
+
+  if (s1.m->t == t)
+    {
+      r.s.m = &m2;
+      r.s.o = s1.o;
+      r.b = 1;
+    }
+  else
+    {
+      r.s.m = 0;
+      r.s.o = 0;
+      r.b = 0;
+    }
+  return r;
+}
+
+void *m (void) __attribute__ ((noinline));
+
+void *
+m (void)
+{
+  return 0;
+}
+
+struct e
+f1 (struct s s1, void *p)
+{
+  struct ret r;
+  void *a;
+  struct s a2;
+
+  r = c (s1, &t5);
+  if (r.b)
+    return f2 (r.s, p);
+  a = m ();
+  a2.m = &m1;
+  a2.o = a;
+  return f2 (a2, p);
+}
+
+static struct e
+f2 (struct s s2, void *p)
+{
+  struct e e1;
+
+  e1 = f3 (s2, p);
+  if (e1.t == &t2 && e1.o == 0)
+    {
+      e1.t = 0;
+      e1.o = 0;
+    }
+  return e1;
+}
+
+static struct e
+f3 (struct s s1, void *p)
+{
+  struct e r;
+
+  f4 (s1, p);
+  r.t = &t3;
+  r.o = 0;
+  return r;
+}
+
+struct s g1;
+void *g2;
+
+static void
+f4 (struct s s1, void *p)
+{
+  g1 = s1;
+  g2 = p;
+  s1.m->m ();
+}
+
+int
+main ()
+{
+  struct s s1;
+
+  s1.m = &m2;
+  s1.o = 0;
+  f1 (s1, 0);
+  abort ();
+}