diff mbox

[PR,61986] Produce aggregate replacement nodes in ascending order of offsets

Message ID 20140903084653.GC18073@virgil.suse
State New
Headers show

Commit Message

Martin Jambor Sept. 3, 2014, 8:46 a.m. UTC
Hi,

intersecting known aggregate values coming along a given set of call
graph edges requires that all lists are in ascending order of offsets
in order to perform it in only one sweep through each of them.
However, aggregate replacement nodes are produced in exactly the
opposite order.  This makes us miss an item in the intersection and
assert later.  The ordering is fixed by the following patch.

Bootstrapped and tested on x86_64-linux.  OK for the trunk and all
problematic branches (4.9 for sure, I am not sure about 4.8 at this
moment).

Thanks,

Martin


2014-09-02  Martin Jambor  <mjambor@suse.cz>

	PR ipa/61986
	* ipa-cp.c (find_aggregate_values_for_callers_subset): Chain
	created replacements in ascending order of offsets.
	(known_aggs_to_agg_replacement_list): Likewise.

testsuite/
	* gcc.dg/ipa/pr61986.c: New test.

Comments

Richard Biener Sept. 3, 2014, 10:47 a.m. UTC | #1
On Wed, Sep 3, 2014 at 10:46 AM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> intersecting known aggregate values coming along a given set of call
> graph edges requires that all lists are in ascending order of offsets
> in order to perform it in only one sweep through each of them.
> However, aggregate replacement nodes are produced in exactly the
> opposite order.  This makes us miss an item in the intersection and
> assert later.  The ordering is fixed by the following patch.
>
> Bootstrapped and tested on x86_64-linux.  OK for the trunk and all
> problematic branches (4.9 for sure, I am not sure about 4.8 at this
> moment).

Ok.

Thanks,
Richard.

> Thanks,
>
> Martin
>
>
> 2014-09-02  Martin Jambor  <mjambor@suse.cz>
>
>         PR ipa/61986
>         * ipa-cp.c (find_aggregate_values_for_callers_subset): Chain
>         created replacements in ascending order of offsets.
>         (known_aggs_to_agg_replacement_list): Likewise.
>
> testsuite/
>         * gcc.dg/ipa/pr61986.c: New test.
>
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 44d4c9a..58121d4 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -3146,7 +3146,8 @@ find_aggregate_values_for_callers_subset (struct cgraph_node *node,
>                                           vec<cgraph_edge *> callers)
>  {
>    struct ipa_node_params *dest_info = IPA_NODE_REF (node);
> -  struct ipa_agg_replacement_value *res = NULL;
> +  struct ipa_agg_replacement_value *res;
> +  struct ipa_agg_replacement_value **tail = &res;
>    struct cgraph_edge *cs;
>    int i, j, count = ipa_get_param_count (dest_info);
>
> @@ -3190,14 +3191,15 @@ find_aggregate_values_for_callers_subset (struct cgraph_node *node,
>           v->offset = item->offset;
>           v->value = item->value;
>           v->by_ref = plats->aggs_by_ref;
> -         v->next = res;
> -         res = v;
> +         *tail = v;
> +         tail = &v->next;
>         }
>
>      next_param:
>        if (inter.exists ())
>         inter.release ();
>      }
> +  *tail = NULL;
>    return res;
>  }
>
> @@ -3206,7 +3208,8 @@ find_aggregate_values_for_callers_subset (struct cgraph_node *node,
>  static struct ipa_agg_replacement_value *
>  known_aggs_to_agg_replacement_list (vec<ipa_agg_jump_function> known_aggs)
>  {
> -  struct ipa_agg_replacement_value *res = NULL;
> +  struct ipa_agg_replacement_value *res;
> +  struct ipa_agg_replacement_value **tail = &res;
>    struct ipa_agg_jump_function *aggjf;
>    struct ipa_agg_jf_item *item;
>    int i, j;
> @@ -3220,9 +3223,10 @@ known_aggs_to_agg_replacement_list (vec<ipa_agg_jump_function> known_aggs)
>         v->offset = item->offset;
>         v->value = item->value;
>         v->by_ref = aggjf->by_ref;
> -       v->next = res;
> -       res = v;
> +       *tail = v;
> +       tail = &v->next;
>        }
> +  *tail = NULL;
>    return res;
>  }
>
> diff --git a/gcc/testsuite/gcc.dg/ipa/pr61986.c b/gcc/testsuite/gcc.dg/ipa/pr61986.c
> new file mode 100644
> index 0000000..8d2f658
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/pr61986.c
> @@ -0,0 +1,48 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +int a, b, c;
> +
> +struct S
> +{
> +  int f0;
> +  int f1;
> +} d;
> +
> +static int fn2 (struct S);
> +void fn3 (struct S);
> +
> +void
> +fn1 (struct S p)
> +{
> +  struct S h = { 0, 0 };
> +  fn3 (p);
> +  fn2 (h);
> +}
> +
> +int
> +fn2 (struct S p)
> +{
> +  struct S j = { 0, 0 };
> +  fn3 (p);
> +  fn2 (j);
> +  return 0;
> +}
> +
> +void
> +fn3 (struct S p)
> +{
> +  for (; b; a++)
> +    c = p.f0;
> +  fn1 (d);
> +}
> +
> +void
> +fn4 ()
> +{
> +  for (;;)
> +    {
> +      struct S f = { 0, 0 };
> +      fn1 (f);
> +    }
> +}
diff mbox

Patch

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 44d4c9a..58121d4 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -3146,7 +3146,8 @@  find_aggregate_values_for_callers_subset (struct cgraph_node *node,
 					  vec<cgraph_edge *> callers)
 {
   struct ipa_node_params *dest_info = IPA_NODE_REF (node);
-  struct ipa_agg_replacement_value *res = NULL;
+  struct ipa_agg_replacement_value *res;
+  struct ipa_agg_replacement_value **tail = &res;
   struct cgraph_edge *cs;
   int i, j, count = ipa_get_param_count (dest_info);
 
@@ -3190,14 +3191,15 @@  find_aggregate_values_for_callers_subset (struct cgraph_node *node,
 	  v->offset = item->offset;
 	  v->value = item->value;
 	  v->by_ref = plats->aggs_by_ref;
-	  v->next = res;
-	  res = v;
+	  *tail = v;
+	  tail = &v->next;
 	}
 
     next_param:
       if (inter.exists ())
 	inter.release ();
     }
+  *tail = NULL;
   return res;
 }
 
@@ -3206,7 +3208,8 @@  find_aggregate_values_for_callers_subset (struct cgraph_node *node,
 static struct ipa_agg_replacement_value *
 known_aggs_to_agg_replacement_list (vec<ipa_agg_jump_function> known_aggs)
 {
-  struct ipa_agg_replacement_value *res = NULL;
+  struct ipa_agg_replacement_value *res;
+  struct ipa_agg_replacement_value **tail = &res;
   struct ipa_agg_jump_function *aggjf;
   struct ipa_agg_jf_item *item;
   int i, j;
@@ -3220,9 +3223,10 @@  known_aggs_to_agg_replacement_list (vec<ipa_agg_jump_function> known_aggs)
 	v->offset = item->offset;
 	v->value = item->value;
 	v->by_ref = aggjf->by_ref;
-	v->next = res;
-	res = v;
+	*tail = v;
+	tail = &v->next;
       }
+  *tail = NULL;
   return res;
 }
 
diff --git a/gcc/testsuite/gcc.dg/ipa/pr61986.c b/gcc/testsuite/gcc.dg/ipa/pr61986.c
new file mode 100644
index 0000000..8d2f658
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr61986.c
@@ -0,0 +1,48 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+int a, b, c;
+
+struct S
+{
+  int f0;
+  int f1;
+} d;
+
+static int fn2 (struct S);
+void fn3 (struct S);
+
+void
+fn1 (struct S p)
+{
+  struct S h = { 0, 0 };
+  fn3 (p);
+  fn2 (h);
+}
+
+int
+fn2 (struct S p)
+{
+  struct S j = { 0, 0 };
+  fn3 (p);
+  fn2 (j);
+  return 0;
+}
+
+void
+fn3 (struct S p)
+{
+  for (; b; a++)
+    c = p.f0;
+  fn1 (d);
+}
+
+void
+fn4 ()
+{
+  for (;;)
+    {
+      struct S f = { 0, 0 };
+      fn1 (f);
+    }
+}