ipa-cp: Avoid ICEs when looking at expanded thunks and unoptimized functions (PR 92476)

Message ID ri6lfrzt2d3.fsf@suse.cz
State New
Headers show
Series
  • ipa-cp: Avoid ICEs when looking at expanded thunks and unoptimized functions (PR 92476)
Related show

Commit Message

Martin Jambor Nov. 29, 2019, 10:15 a.m.
Hi,

the patch below fixes the i686 failures reported in PR 92476.  Newly
expanded "artificial" thunks need to be analyzed when expanded so that
we create necessary function summaries and jump functions for them.
They still don't get IPA-CP lattices, so I looked at all accesses to
those and verified that only the functions saving IPA-VR and IPA-bits
analyses could try to access non-existing lattices.

After that, Martin's testcase in comment 4 of the bug also revealed two
places where we try to access summaries of unoptimized functions and
segfault, so I fixed those too.  Unfortunately it seems our testsuite
cannot optimize different LTO compilation units with different options
and so I could not add the testcase there.  But it no longer ICEs.

Bootstrapped and LTO-profile-bootstrapped and tested on x86_64-linux and
I also verified the -m32 testsuite failures are all gone.  OK for trunk?

Thanks,

Martin


2019-11-28  Martin Jambor  <mjambor@suse.cz>
            Jan Hubicka  <jh@suse.cz>

	PR ipa/92476
	* ipa-cp.c (set_single_call_flag): Set node_calling_single_call in
	the summary only if the summary exists.
	(find_more_scalar_values_for_callers_subset): Check node_dead in
	the summary only if the summary exists.
	(ipcp_store_bits_results): Ignore nodes without lattices.
	(ipcp_store_vr_results): Likewise.
	* cgraphclones.c: Include ipa-fnsummary.h and ipa-prop.h and the
	header files required by them.
	(cgraph_node::expand_all_artificial_thunks): Analyze expanded thunks.
---
 gcc/cgraphclones.c |  7 +++++++
 gcc/ipa-cp.c       | 10 ++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

-- 
2.24.0

Comments

Jan Hubicka Nov. 29, 2019, 10:31 a.m. | #1
> Hi,

> 

> the patch below fixes the i686 failures reported in PR 92476.  Newly

> expanded "artificial" thunks need to be analyzed when expanded so that

> we create necessary function summaries and jump functions for them.

> They still don't get IPA-CP lattices, so I looked at all accesses to

> those and verified that only the functions saving IPA-VR and IPA-bits

> analyses could try to access non-existing lattices.

> 

> After that, Martin's testcase in comment 4 of the bug also revealed two

> places where we try to access summaries of unoptimized functions and

> segfault, so I fixed those too.  Unfortunately it seems our testsuite

> cannot optimize different LTO compilation units with different options

> and so I could not add the testcase there.  But it no longer ICEs.

I think you can simply add different flag into different testcases:
20090210_1.c:/* { dg-options "-fPIC" { target { ! sparc*-*-* } } } */
20090218-1_1.c:/* { dg-options "-fgnu89-inline" } */
20090218-2_1.c:/* { dg-options { -fgnu89-inline } } */
20111207-1_1.c:/* { dg-options "-fno-lto" } */

> 

> Bootstrapped and LTO-profile-bootstrapped and tested on x86_64-linux and

> I also verified the -m32 testsuite failures are all gone.  OK for trunk?

> 

> Thanks,

> 

> Martin

> 

> 

> 2019-11-28  Martin Jambor  <mjambor@suse.cz>

>             Jan Hubicka  <jh@suse.cz>

> 

> 	PR ipa/92476

> 	* ipa-cp.c (set_single_call_flag): Set node_calling_single_call in

> 	the summary only if the summary exists.

> 	(find_more_scalar_values_for_callers_subset): Check node_dead in

> 	the summary only if the summary exists.

> 	(ipcp_store_bits_results): Ignore nodes without lattices.

> 	(ipcp_store_vr_results): Likewise.

> 	* cgraphclones.c: Include ipa-fnsummary.h and ipa-prop.h and the

> 	header files required by them.

> 	(cgraph_node::expand_all_artificial_thunks): Analyze expanded thunks.


OK, thanks
Honza
> ---

>  gcc/cgraphclones.c |  7 +++++++

>  gcc/ipa-cp.c       | 10 ++++++++--

>  2 files changed, 15 insertions(+), 2 deletions(-)

> 

> diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c

> index ac5c57a47aa..8e86e82a226 100644

> --- a/gcc/cgraphclones.c

> +++ b/gcc/cgraphclones.c

> @@ -80,6 +80,11 @@ along with GCC; see the file COPYING3.  If not see

>  #include "tree-inline.h"

>  #include "dumpfile.h"

>  #include "gimple-pretty-print.h"

> +#include "alloc-pool.h"

> +#include "symbol-summary.h"

> +#include "tree-vrp.h"

> +#include "ipa-prop.h"

> +#include "ipa-fnsummary.h"

>  

>  /* Create clone of edge in the node N represented by CALL_EXPR

>     the callgraph.  */

> @@ -267,6 +272,8 @@ cgraph_node::expand_all_artificial_thunks ()

>  	  {

>  	    thunk->thunk.thunk_p = false;

>  	    thunk->analyze ();

> +	    ipa_analyze_node (thunk);

> +	    inline_analyze_function (thunk);

>  	  }

>  	thunk->expand_all_artificial_thunks ();

>        }

> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c

> index 31a98a3d98a..7fb9f30f709 100644

> --- a/gcc/ipa-cp.c

> +++ b/gcc/ipa-cp.c

> @@ -1165,7 +1165,7 @@ set_single_call_flag (cgraph_node *node, void *)

>    /* Local thunks can be handled transparently, skip them.  */

>    while (cs && cs->caller->thunk.thunk_p && cs->caller->local)

>      cs = cs->next_caller;

> -  if (cs)

> +  if (cs && IPA_NODE_REF (cs->caller))

>      {

>        IPA_NODE_REF (cs->caller)->node_calling_single_call = true;

>        return true;

> @@ -4411,7 +4411,7 @@ find_more_scalar_values_for_callers_subset (struct cgraph_node *node,

>  	  struct ipa_jump_func *jump_func;

>  	  tree t;

>  

> -	  if (IPA_NODE_REF (cs->caller)->node_dead)

> +	  if (IPA_NODE_REF (cs->caller) && IPA_NODE_REF (cs->caller)->node_dead)

>  	    continue;

>  

>  	  if (!IPA_EDGE_REF (cs)

> @@ -5416,6 +5416,9 @@ ipcp_store_bits_results (void)

>  

>        if (info->ipcp_orig_node)

>  	info = IPA_NODE_REF (info->ipcp_orig_node);

> +      if (!info->lattices)

> +	/* Newly expanded artificial thunks do not have lattices.  */

> +	continue;

>  

>        unsigned count = ipa_get_param_count (info);

>        for (unsigned i = 0; i < count; i++)

> @@ -5489,6 +5492,9 @@ ipcp_store_vr_results (void)

>  

>        if (info->ipcp_orig_node)

>  	info = IPA_NODE_REF (info->ipcp_orig_node);

> +      if (!info->lattices)

> +	/* Newly expanded artificial thunks do not have lattices.  */

> +	continue;

>  

>        unsigned count = ipa_get_param_count (info);

>        for (unsigned i = 0; i < count; i++)

> -- 

> 2.24.0

>
Martin Jambor Nov. 29, 2019, 1:33 p.m. | #2
Hi,

On Fri, Nov 29 2019, Jan Hubicka wrote:
>> Hi,

>> 

>> the patch below fixes the i686 failures reported in PR 92476.  Newly

>> expanded "artificial" thunks need to be analyzed when expanded so that

>> we create necessary function summaries and jump functions for them.

>> They still don't get IPA-CP lattices, so I looked at all accesses to

>> those and verified that only the functions saving IPA-VR and IPA-bits

>> analyses could try to access non-existing lattices.

>> 

>> After that, Martin's testcase in comment 4 of the bug also revealed two

>> places where we try to access summaries of unoptimized functions and

>> segfault, so I fixed those too.  Unfortunately it seems our testsuite

>> cannot optimize different LTO compilation units with different options

>> and so I could not add the testcase there.  But it no longer ICEs.

> I think you can simply add different flag into different testcases:

> 20090210_1.c:/* { dg-options "-fPIC" { target { ! sparc*-*-* } } } */

> 20090218-1_1.c:/* { dg-options "-fgnu89-inline" } */

> 20090218-2_1.c:/* { dg-options { -fgnu89-inline } } */

> 20111207-1_1.c:/* { dg-options "-fno-lto" } */

>


Ah, servers me right for only grepping for dg-lto-options.

I have committed the fix and will commit the following testcase addition
as a follow-up.

Thanks,

Martin


2019-11-29  Martin Jambor  <mjambor@suse.cz>

	PR ipa/92476
        * g++.dg/lto/pr92476_[01].C: New test.
---
 gcc/testsuite/g++.dg/lto/pr92476_0.C | 20 ++++++++++++++++++++
 gcc/testsuite/g++.dg/lto/pr92476_1.C | 13 +++++++++++++
 2 files changed, 33 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/lto/pr92476_0.C
 create mode 100644 gcc/testsuite/g++.dg/lto/pr92476_1.C

diff --git a/gcc/testsuite/g++.dg/lto/pr92476_0.C b/gcc/testsuite/g++.dg/lto/pr92476_0.C
new file mode 100644
index 00000000000..5bbc9236f4d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr92476_0.C
@@ -0,0 +1,20 @@
+// { dg-lto-do link }
+// { dg-lto-options { { -O0 -flto -shared -fPIC -fvisibility=hidden } } }
+// { dg-require-effective-target fpic }
+// { dg-require-effective-target shared }
+// { dg-extra-ld-options "-shared" }
+
+namespace Passenger {
+namespace Json {
+class Value {};
+} // namespace Json
+namespace ConfigKit {
+class Translator {};
+} // namespace ConfigKit
+namespace LoggingKit {
+void initialize(const Json::Value & = Json::Value(),
+                const ConfigKit::Translator & = ConfigKit::Translator());
+void init_module() { initialize(); }
+} // namespace LoggingKit
+} // namespace Passenger
+
diff --git a/gcc/testsuite/g++.dg/lto/pr92476_1.C b/gcc/testsuite/g++.dg/lto/pr92476_1.C
new file mode 100644
index 00000000000..cd29613b808
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr92476_1.C
@@ -0,0 +1,13 @@
+// { dg-options { -O2 -flto -shared -fPIC -fvisibility=hidden } }
+
+namespace Passenger {
+namespace Json {
+class Value;
+}
+namespace ConfigKit {
+class Translator;
+}
+namespace LoggingKit {
+void initialize(const Json::Value &, const ConfigKit::Translator &) {}
+} // namespace LoggingKit
+} // namespace Passenger
-- 
2.24.0

Patch

diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index ac5c57a47aa..8e86e82a226 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -80,6 +80,11 @@  along with GCC; see the file COPYING3.  If not see
 #include "tree-inline.h"
 #include "dumpfile.h"
 #include "gimple-pretty-print.h"
+#include "alloc-pool.h"
+#include "symbol-summary.h"
+#include "tree-vrp.h"
+#include "ipa-prop.h"
+#include "ipa-fnsummary.h"
 
 /* Create clone of edge in the node N represented by CALL_EXPR
    the callgraph.  */
@@ -267,6 +272,8 @@  cgraph_node::expand_all_artificial_thunks ()
 	  {
 	    thunk->thunk.thunk_p = false;
 	    thunk->analyze ();
+	    ipa_analyze_node (thunk);
+	    inline_analyze_function (thunk);
 	  }
 	thunk->expand_all_artificial_thunks ();
       }
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 31a98a3d98a..7fb9f30f709 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1165,7 +1165,7 @@  set_single_call_flag (cgraph_node *node, void *)
   /* Local thunks can be handled transparently, skip them.  */
   while (cs && cs->caller->thunk.thunk_p && cs->caller->local)
     cs = cs->next_caller;
-  if (cs)
+  if (cs && IPA_NODE_REF (cs->caller))
     {
       IPA_NODE_REF (cs->caller)->node_calling_single_call = true;
       return true;
@@ -4411,7 +4411,7 @@  find_more_scalar_values_for_callers_subset (struct cgraph_node *node,
 	  struct ipa_jump_func *jump_func;
 	  tree t;
 
-	  if (IPA_NODE_REF (cs->caller)->node_dead)
+	  if (IPA_NODE_REF (cs->caller) && IPA_NODE_REF (cs->caller)->node_dead)
 	    continue;
 
 	  if (!IPA_EDGE_REF (cs)
@@ -5416,6 +5416,9 @@  ipcp_store_bits_results (void)
 
       if (info->ipcp_orig_node)
 	info = IPA_NODE_REF (info->ipcp_orig_node);
+      if (!info->lattices)
+	/* Newly expanded artificial thunks do not have lattices.  */
+	continue;
 
       unsigned count = ipa_get_param_count (info);
       for (unsigned i = 0; i < count; i++)
@@ -5489,6 +5492,9 @@  ipcp_store_vr_results (void)
 
       if (info->ipcp_orig_node)
 	info = IPA_NODE_REF (info->ipcp_orig_node);
+      if (!info->lattices)
+	/* Newly expanded artificial thunks do not have lattices.  */
+	continue;
 
       unsigned count = ipa_get_param_count (info);
       for (unsigned i = 0; i < count; i++)