Fix -fdump-ipa-all ICE

Fix -fdump-ipa-all ICE

[Jan Hubicka]

Hi,
this patch fixes ICE in dumping that triggers somewhat overactive sanity check.
I think it would be nice to get it into release branches so we could debug things
more easily.

I am going to commit to trunk, OK for release branches?
Honza
	PR middle-end/79931
	* ipa-devirt.c (dump_possible_polymorphic_call_targets): Fix ICE.
Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c	(revision 246970)
+++ ipa-devirt.c	(working copy)
@@ -3367,7 +3367,13 @@ dump_possible_polymorphic_call_targets (
       fprintf (f, "  Speculative targets:");
       dump_targets (f, targets);
     }
-  gcc_assert (targets.length () <= len);
+  /* Ugly: during callgraph construction the target cache may get populated
+     before all targets are found.  While this is harmless (because all local
+     types are discovered and only in those case we devirtualize fully and we
+     don't do speculative devirtualization before IPA stage) it triggers
+     assert here when dumping at that stage also populates the case with
+     speculative targets.  Quietly ignore this.  */
+  gcc_assert (symtab->state < IPA_SSA || targets.length () <= len);
   fprintf (f, "\n");
 }
 

Re: Fix -fdump-ipa-all ICE

[Jakub Jelinek]

On Mon, Apr 24, 2017 at 12:48:19PM +0200, Jan Hubicka wrote:
> Hi,
> this patch fixes ICE in dumping that triggers somewhat overactive sanity check.
> I think it would be nice to get it into release branches so we could debug things
> more easily.
> 
> I am going to commit to trunk, OK for release branches?
> Honza
> 	PR middle-end/79931
> 	* ipa-devirt.c (dump_possible_polymorphic_call_targets): Fix ICE.

No testcase in the patch?

> Index: ipa-devirt.c
> ===================================================================
> --- ipa-devirt.c	(revision 246970)
> +++ ipa-devirt.c	(working copy)
> @@ -3367,7 +3367,13 @@ dump_possible_polymorphic_call_targets (
>        fprintf (f, "  Speculative targets:");
>        dump_targets (f, targets);
>      }
> -  gcc_assert (targets.length () <= len);
> +  /* Ugly: during callgraph construction the target cache may get populated
> +     before all targets are found.  While this is harmless (because all local
> +     types are discovered and only in those case we devirtualize fully and we
> +     don't do speculative devirtualization before IPA stage) it triggers
> +     assert here when dumping at that stage also populates the case with
> +     speculative targets.  Quietly ignore this.  */
> +  gcc_assert (symtab->state < IPA_SSA || targets.length () <= len);
>    fprintf (f, "\n");
>  }
>  

	Jakub

Re: Fix -fdump-ipa-all ICE

[Martin Liška]

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

On 04/24/2017 12:51 PM, Jakub Jelinek wrote:
> No testcase in the patch?

As Honza is busy right not, I'm sending one.

Martin

[-- Attachment #2: 0001-Add-new-test-case.patch --]
[-- Type: text/x-patch, Size: 1351 bytes --]

From 87cef5e3123723f81c44dfafe86fa10b7925cea8 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Mon, 24 Apr 2017 14:02:54 +0200
Subject: [PATCH] Add new test-case.

gcc/testsuite/ChangeLog:

2017-04-24  Martin Liska  <mliska@suse.cz>

	* g++.dg/ipa/pr79931.C: New test.
---
 gcc/testsuite/g++.dg/ipa/pr79931.C | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/ipa/pr79931.C

diff --git a/gcc/testsuite/g++.dg/ipa/pr79931.C b/gcc/testsuite/g++.dg/ipa/pr79931.C
new file mode 100644
index 00000000000..78f6e03c458
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr79931.C
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-all" } */
+
+class DocumentImpl;
+struct NodeImpl
+{
+  virtual DocumentImpl * getOwnerDocument();
+  virtual NodeImpl * getParentNode();
+  virtual NodeImpl * removeChild(NodeImpl *oldChild);
+};
+struct AttrImpl : NodeImpl
+{
+  NodeImpl *insertBefore(NodeImpl *newChild, NodeImpl *refChild);
+};
+struct DocumentImpl : NodeImpl
+{
+  virtual NodeImpl *removeChild(NodeImpl *oldChild);
+  virtual int* getRanges();
+};
+NodeImpl *AttrImpl::insertBefore(NodeImpl *newChild, NodeImpl *refChild) {
+  NodeImpl *oldparent = newChild->getParentNode();
+  oldparent->removeChild(newChild);
+  this->getOwnerDocument()->getRanges();
+}
-- 
2.12.2

Re: Fix -fdump-ipa-all ICE

[Jan Hubicka]

> On 04/24/2017 12:51 PM, Jakub Jelinek wrote:
> > No testcase in the patch?
> 
> As Honza is busy right not, I'm sending one.
Thanks (in fact I just forgot to include it and was about to send it now) but
help is welcome!

Honza
> 
> Martin

> >From 87cef5e3123723f81c44dfafe86fa10b7925cea8 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Mon, 24 Apr 2017 14:02:54 +0200
> Subject: [PATCH] Add new test-case.
> 
> gcc/testsuite/ChangeLog:
> 
> 2017-04-24  Martin Liska  <mliska@suse.cz>
> 
> 	* g++.dg/ipa/pr79931.C: New test.
> ---
>  gcc/testsuite/g++.dg/ipa/pr79931.C | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/ipa/pr79931.C
> 
> diff --git a/gcc/testsuite/g++.dg/ipa/pr79931.C b/gcc/testsuite/g++.dg/ipa/pr79931.C
> new file mode 100644
> index 00000000000..78f6e03c458
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ipa/pr79931.C
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-ipa-all" } */
> +
> +class DocumentImpl;
> +struct NodeImpl
> +{
> +  virtual DocumentImpl * getOwnerDocument();
> +  virtual NodeImpl * getParentNode();
> +  virtual NodeImpl * removeChild(NodeImpl *oldChild);
> +};
> +struct AttrImpl : NodeImpl
> +{
> +  NodeImpl *insertBefore(NodeImpl *newChild, NodeImpl *refChild);
> +};
> +struct DocumentImpl : NodeImpl
> +{
> +  virtual NodeImpl *removeChild(NodeImpl *oldChild);
> +  virtual int* getRanges();
> +};
> +NodeImpl *AttrImpl::insertBefore(NodeImpl *newChild, NodeImpl *refChild) {
> +  NodeImpl *oldparent = newChild->getParentNode();
> +  oldparent->removeChild(newChild);
> +  this->getOwnerDocument()->getRanges();
> +}
> -- 
> 2.12.2
> 

Re: Fix -fdump-ipa-all ICE

[Jakub Jelinek]

On Mon, Apr 24, 2017 at 02:12:31PM +0200, Jan Hubicka wrote:
> > On 04/24/2017 12:51 PM, Jakub Jelinek wrote:
> > > No testcase in the patch?
> > 
> > As Honza is busy right not, I'm sending one.
> Thanks (in fact I just forgot to include it and was about to send it now) but
> help is welcome!

Ok for release branches.

	Jakub