public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Feng Xue OS <fxue@os.amperecomputing.com>
To: Tamar Christina <Tamar.Christina@arm.com>,
	Martin Jambor	<mjambor@suse.cz>, Jan Hubicka <hubicka@ucw.cz>,
	"gcc-patches@gcc.gnu.org"	<gcc-patches@gcc.gnu.org>
Cc: nd <nd@arm.com>
Subject: [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707)
Date: Thu, 13 Feb 2020 05:39:00 -0000	[thread overview]
Message-ID: <BYAPR01MB4869B0D993323A66E070C91AF71A0@BYAPR01MB4869.prod.exchangelabs.com> (raw)
In-Reply-To: <DB6PR0802MB2245C19201F178B3F925B7BDFF180@DB6PR0802MB2245.eurprd08.prod.outlook.com>

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

I've submitted a bug tracker, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93707.

The root cause is that for a self-recursive function, a for-all-contexts clone could generate
an edge whose callee is not the function. Therefore, to check whether an edge stands for a
recursive call during cloning, we should not use ordinary way of comparing caller and callee
of the edge.

Bootstrapped/regtested on x86_64-linux and aarch64-linux, and also tested Spec 2017 with
option "--param ipa-cp-eval-threshold=1". 

Feng
---
2020-02-13  Feng Xue  <fxue@os.amperecomputing.com>

        PR ipa/93707
        * ipa-cp.c (self_recursive_pass_through_p): Add a new parameter "node",
        and check recursiveness by comparing "node" and cs->caller.
        (self_recursive_agg_pass_through_p): Likewise.
        (find_more_scalar_values_for_callers_subset): Add "node" argument to
        self_recursive_pass_through_p call.
        (intersect_aggregates_with_edge): Add a new parameter "node", and add
        "node" argument to self_cursive_agg_pass_through_p call.
        (find_aggregate_values_for_callers_subset): Add "node" argument to
        self_recursive_pass_through_p and intersect_aggregates_with_edge calls.
        (cgraph_edge_brings_all_agg_vals_for_node): Add "node" argument to
        intersect_aggregates_with_edge call.

> ________________________________________
> From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org>
> on behalf of Tamar Christina <Tamar.Christina@arm.com>
> Sent: Tuesday, February 11, 2020 6:05 PM
> To: Feng Xue OS; Martin Jambor; Jan Hubicka; gcc-patches@gcc.gnu.org
> Cc: nd
> Subject: RE: [PATCH V2] Generalized value pass-through for self-recursive
> function (ipa/pr93203)
>
> Hi Feng,
>
> This patch (commit a0f6a8cb414b687f22c9011a894d5e8e398c4be0) is causing
> ICEs in the GCC and perlbench benchmark in Spec2017.
>
> during IPA pass: cp
> lto1: internal compiler error: in find_more_scalar_values_for_callers_subset,
> at ipa-cp.c:4709
> 0x1698187 find_more_scalar_values_for_callers_subset
>         ../.././gcc/ipa-cp.c:4709
> 0x169f7d3 decide_about_value<tree_node*>
>         ../.././gcc/ipa-cp.c:5490
> 0x169fdc3 decide_whether_version_node
>         ../.././gcc/ipa-cp.c:5537
> 0x169fdc3 ipcp_decision_stage
>         ../.././gcc/ipa-cp.c:5718
> 0x169fdc3 ipcp_driver
>         ../.././gcc/ipa-cp.c:5901
> Please submit a full bug report,
>
> Thanks,
> Tamar
>

[-- Attachment #2: 0001-Fix-bug-in-recursiveness-check-for-function-in-clone.patch --]
[-- Type: application/octet-stream, Size: 6301 bytes --]

From 40ff412bfef584461ddc16fbbaaad3275a294d64 Mon Sep 17 00:00:00 2001
From: Feng Xue <fxue@os.amperecomputing.com>
Date: Wed, 12 Feb 2020 13:04:33 -0500
Subject: [PATCH] Fix bug in recursiveness check for function in clone (PR
 ipa/93707)

---
 gcc/ipa-cp.c                       | 54 +++++++++++++++++-------------
 gcc/testsuite/gcc.dg/ipa/pr93707.c | 29 ++++++++++++++++
 2 files changed, 60 insertions(+), 23 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93707.c

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 4f5b72e6994..9fbeeae9e77 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -4617,17 +4617,21 @@ create_specialized_node (struct cgraph_node *node,
   return new_node;
 }
 
-/* Return true, if JFUNC, which describes a i-th parameter of call CS, is a
-   pass-through function to itself.  When SIMPLE is true, further check if
-   JFUNC is a simple no-operation pass-through.  */
+/* Given an edge CS, along which we will do cloning for NODE, return true,
+   if CS represents a self-recursive call to NODE, and if JFUNC, which
+   describes the i-th argument of CS, is a pass-through jump function to the
+   i-th parameter of NODE.  When SIMPLE is true, further check if JFUNC is a
+   simple no-operation pass-through.  */
 
 static bool
-self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i,
-			       bool simple = true)
-{
-  enum availability availability;
-  if (cs->caller == cs->callee->function_symbol (&availability)
-      && availability > AVAIL_INTERPOSABLE
+self_recursive_pass_through_p (cgraph_edge *cs, cgraph_node *node,
+			       ipa_jump_func *jfunc,
+			       int i, bool simple = true)
+{
+   /* CS could be created from a for-all-contexts clone, thus NODE might not
+      be callee of CS.  Here we could not resort to ordinary way of comparing
+      caller and callee of CS to check recursiveness.  */
+  if (cs->caller == node
       && jfunc->type == IPA_JF_PASS_THROUGH
       && (!simple || ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
       && ipa_get_jf_pass_through_formal_id (jfunc) == i)
@@ -4635,18 +4639,19 @@ self_recursive_pass_through_p (cgraph_edge *cs, ipa_jump_func *jfunc, int i,
   return false;
 }
 
-/* Return true, if JFUNC, which describes a part of an aggregate represented
-   or pointed to by the i-th parameter of call CS, is a pass-through function
-   to itself.  When SIMPLE is true, further check if JFUNC is a simple
-   no-operation pass-through.  */
+/* Given an edge CS, along which we will do cloning for NODE, return true,
+   if CS represents a self-recursive call to NODE, and if JFUNC, which
+   describes a part of an aggregate represented or pointed to by the i-th
+   argument of CS, is a pass-through jump function to same aggregate part of
+   the i-th parameter of NODE.  When SIMPLE is true, further check if JFUNC
+   is a simple no-operation pass-through.  */
 
 static bool
-self_recursive_agg_pass_through_p (cgraph_edge *cs, ipa_agg_jf_item *jfunc,
+self_recursive_agg_pass_through_p (cgraph_edge *cs, cgraph_node *node,
+				   ipa_agg_jf_item *jfunc,
 				   int i, bool simple = true)
 {
-  enum availability availability;
-  if (cs->caller == cs->callee->function_symbol (&availability)
-      && availability > AVAIL_INTERPOSABLE
+  if (cs->caller == node
       && jfunc->jftype == IPA_JF_LOAD_AGG
       && jfunc->offset == jfunc->value.load_agg.offset
       && (!simple || jfunc->value.pass_through.operation == NOP_EXPR)
@@ -4704,7 +4709,7 @@ find_more_scalar_values_for_callers_subset (struct cgraph_node *node,
 
 	     Given that i is 0, recursive propagation via (i & 1) also gets
 	     0.  */
-	  if (self_recursive_pass_through_p (cs, jump_func, i, false))
+	  if (self_recursive_pass_through_p (cs, node, jump_func, i, false))
 	    {
 	      gcc_assert (newval);
 	      t = ipa_get_jf_arith_result (
@@ -4952,7 +4957,9 @@ intersect_with_agg_replacements (struct cgraph_node *node, int index,
    whatsoever, return a released vector.  */
 
 static vec<ipa_agg_value>
-intersect_aggregates_with_edge (struct cgraph_edge *cs, int index,
+intersect_aggregates_with_edge (struct cgraph_edge *cs,
+				struct cgraph_node *node,
+				int index,
 				vec<ipa_agg_value> inter)
 {
   struct ipa_jump_func *jfunc;
@@ -5085,7 +5092,7 @@ intersect_aggregates_with_edge (struct cgraph_edge *cs, int index,
 
 		       Given that *i is 0, recursive propagation via (*i & 1)
 		       also gets 0.  */
-		    if (self_recursive_agg_pass_through_p (cs, ti, index,
+		    if (self_recursive_agg_pass_through_p (cs, node, ti, index,
 							   false))
 		      value = ipa_get_jf_arith_result (
 					ti->value.pass_through.operation,
@@ -5156,11 +5163,11 @@ find_aggregate_values_for_callers_subset (struct cgraph_node *node,
 	{
 	  struct ipa_jump_func *jfunc
 	    = ipa_get_ith_jump_func (IPA_EDGE_REF (cs), i);
-	  if (self_recursive_pass_through_p (cs, jfunc, i)
+	  if (self_recursive_pass_through_p (cs, node, jfunc, i)
 	      && (!plats->aggs_by_ref
 		  || ipa_get_jf_pass_through_agg_preserved (jfunc)))
 	    continue;
-	  inter = intersect_aggregates_with_edge (cs, i, inter);
+	  inter = intersect_aggregates_with_edge (cs, node, i, inter);
 
 	  if (!inter.exists ())
 	    goto next_param;
@@ -5265,7 +5272,8 @@ cgraph_edge_brings_all_agg_vals_for_node (struct cgraph_edge *cs,
       if (plats->aggs_bottom)
 	return false;
 
-      vec<ipa_agg_value> values = intersect_aggregates_with_edge (cs, i, vNULL);
+      vec<ipa_agg_value> values
+		= intersect_aggregates_with_edge (cs, node, i, vNULL);
       if (!values.exists ())
 	return false;
 
diff --git a/gcc/testsuite/gcc.dg/ipa/pr93707.c b/gcc/testsuite/gcc.dg/ipa/pr93707.c
new file mode 100644
index 00000000000..e200a3a432b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr93707.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 --param ipa-cp-eval-threshold=1" } */
+
+int foo();
+int data[100];
+
+__attribute__((noinline)) static int recur_fn (int i, int j, int depth)
+{
+   if (depth > 10)
+     return 1;
+
+   data[i + j]++;
+
+   if (depth & 3)
+     recur_fn (i, 1, depth + 1);
+   else
+     recur_fn (i, j & 1, depth + 1);
+
+   foo();
+
+   return i + j;
+}
+
+int caller (int v, int depth)
+{
+  recur_fn (1, v, depth);
+
+  return 0;
+}
-- 
2.17.1


  reply	other threads:[~2020-02-13  5:39 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-25 13:52 [PATCH] Generalized value pass-through for self-recursive function (ipa/pr93203) Feng Xue OS
2020-01-25 16:17 ` [PATCH V2] " Feng Xue OS
2020-02-03  1:57   ` Ping* " Feng Xue OS
2020-02-05 17:27   ` Martin Jambor
2020-02-10  3:28     ` Feng Xue OS
2020-02-11 10:05       ` Tamar Christina
2020-02-11 13:10         ` Feng Xue OS
2020-02-11 14:31           ` Tamar Christina
2020-02-13  5:39             ` Feng Xue OS [this message]
2020-02-17  8:44               ` [PATCH] Fix bug in recursiveness check for function to be cloned (ipa/pr93707) Tamar Christina
2020-02-18 15:16                 ` Ping: " Feng Xue OS
2020-02-19 16:28               ` Martin Jambor
2020-02-20  3:36                 ` Feng Xue OS
2020-02-21 18:16                   ` Martin Jambor
2020-02-22  3:32                     ` Feng Xue OS
2020-02-24 15:41                       ` Martin Jambor
2020-02-20 12:57                 ` Tamar Christina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BYAPR01MB4869B0D993323A66E070C91AF71A0@BYAPR01MB4869.prod.exchangelabs.com \
    --to=fxue@os.amperecomputing.com \
    --cc=Tamar.Christina@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=mjambor@suse.cz \
    --cc=nd@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).