[PR 70965] Schedule extra pass_rebuild_cgraph_edges

[PR 70965] Schedule extra pass_rebuild_cgraph_edges

[Martin Jambor]

Hi,

after discussing this with Honza, we have decided that scheduling an
extra pass_rebuild_cgraph_edges after pass_fixup_cfg is the correct
way to keep the cgraph consistent with gimple IL when early IPA passes
need it, such as is the case with the testcase in PR 70965.

While needing an extra pass is never nice, this is a consequence of
splitting pass_build_ssa_passes out of early optimization passes so
that pass_chkp can be in between.

The patch below fices the ICE in PR 70965 and has passed bootstrap and
testing on x86_64-linux.  OK for trunk?

Thanks,

Martin


2016-11-24  Martin Jambor  <mjambor@suse.cz>

gcc/
	* passes.def (pass_build_ssa_passes): Add pass_rebuild_cgraph_edges.

gcc/testsuite/
	* g++.dg/pr70965.C: New test.
---
 gcc/passes.def                 |  1 +
 gcc/testsuite/g++.dg/pr70965.C | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/pr70965.C

diff --git a/gcc/passes.def b/gcc/passes.def
index 85a5af0..5faf17f 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -56,6 +56,7 @@ along with GCC; see the file COPYING3.  If not see
   NEXT_PASS (pass_build_ssa_passes);
   PUSH_INSERT_PASSES_WITHIN (pass_build_ssa_passes)
       NEXT_PASS (pass_fixup_cfg);
+      NEXT_PASS (pass_rebuild_cgraph_edges);
       NEXT_PASS (pass_build_ssa);
       NEXT_PASS (pass_warn_nonnull_compare);
       NEXT_PASS (pass_ubsan);
diff --git a/gcc/testsuite/g++.dg/pr70965.C b/gcc/testsuite/g++.dg/pr70965.C
new file mode 100644
index 0000000..d8a2c35
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr70965.C
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -std=c++11" } */
+
+struct A {};
+struct B {};
+struct C { using p = int *; template <typename> using ra = A; };
+struct J : C { template <typename> struct K { typedef C::ra<int> o; }; };
+template <typename> struct D
+{
+  struct H : J::K<int>::o { H (J::p, A) : J::K<int>::o () {} };
+  H d;
+  D (const char *, const A &x = A ()) : d (0, x) {}
+};
+extern template class D<char>;
+enum L { M };
+struct F { virtual char *foo (); };
+template <class> struct I : B { static int foo (int) {} };
+struct G { typedef I<int> t; };
+void foo (int) { G::t::foo (0); }
+void bar (const D<char> &, const D<int> &, int, L);
+void baz () try { foo (0); } catch (F &e) { bar (e.foo (), "", 0, M); }
-- 
2.10.2

Re: [PR 70965] Schedule extra pass_rebuild_cgraph_edges

[Richard Biener]

On Thu, Nov 24, 2016 at 5:44 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> after discussing this with Honza, we have decided that scheduling an
> extra pass_rebuild_cgraph_edges after pass_fixup_cfg is the correct
> way to keep the cgraph consistent with gimple IL when early IPA passes
> need it, such as is the case with the testcase in PR 70965.
>
> While needing an extra pass is never nice, this is a consequence of
> splitting pass_build_ssa_passes out of early optimization passes so
> that pass_chkp can be in between.
>
> The patch below fices the ICE in PR 70965 and has passed bootstrap and
> testing on x86_64-linux.  OK for trunk?

Ok.

Richard.

> Thanks,
>
> Martin
>
>
> 2016-11-24  Martin Jambor  <mjambor@suse.cz>
>
> gcc/
>         * passes.def (pass_build_ssa_passes): Add pass_rebuild_cgraph_edges.
>
> gcc/testsuite/
>         * g++.dg/pr70965.C: New test.
> ---
>  gcc/passes.def                 |  1 +
>  gcc/testsuite/g++.dg/pr70965.C | 21 +++++++++++++++++++++
>  2 files changed, 22 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/pr70965.C
>
> diff --git a/gcc/passes.def b/gcc/passes.def
> index 85a5af0..5faf17f 100644
> --- a/gcc/passes.def
> +++ b/gcc/passes.def
> @@ -56,6 +56,7 @@ along with GCC; see the file COPYING3.  If not see
>    NEXT_PASS (pass_build_ssa_passes);
>    PUSH_INSERT_PASSES_WITHIN (pass_build_ssa_passes)
>        NEXT_PASS (pass_fixup_cfg);
> +      NEXT_PASS (pass_rebuild_cgraph_edges);
>        NEXT_PASS (pass_build_ssa);
>        NEXT_PASS (pass_warn_nonnull_compare);
>        NEXT_PASS (pass_ubsan);
> diff --git a/gcc/testsuite/g++.dg/pr70965.C b/gcc/testsuite/g++.dg/pr70965.C
> new file mode 100644
> index 0000000..d8a2c35
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr70965.C
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -std=c++11" } */
> +
> +struct A {};
> +struct B {};
> +struct C { using p = int *; template <typename> using ra = A; };
> +struct J : C { template <typename> struct K { typedef C::ra<int> o; }; };
> +template <typename> struct D
> +{
> +  struct H : J::K<int>::o { H (J::p, A) : J::K<int>::o () {} };
> +  H d;
> +  D (const char *, const A &x = A ()) : d (0, x) {}
> +};
> +extern template class D<char>;
> +enum L { M };
> +struct F { virtual char *foo (); };
> +template <class> struct I : B { static int foo (int) {} };
> +struct G { typedef I<int> t; };
> +void foo (int) { G::t::foo (0); }
> +void bar (const D<char> &, const D<int> &, int, L);
> +void baz () try { foo (0); } catch (F &e) { bar (e.foo (), "", 0, M); }
> --
> 2.10.2
>

Re: [PR 70965] Schedule extra pass_rebuild_cgraph_edges

[Jan Hubicka]

> On Thu, Nov 24, 2016 at 5:44 PM, Martin Jambor <mjambor@suse.cz> wrote:
> > Hi,
> >
> > after discussing this with Honza, we have decided that scheduling an
> > extra pass_rebuild_cgraph_edges after pass_fixup_cfg is the correct
> > way to keep the cgraph consistent with gimple IL when early IPA passes
> > need it, such as is the case with the testcase in PR 70965.
> >
> > While needing an extra pass is never nice, this is a consequence of
> > splitting pass_build_ssa_passes out of early optimization passes so
> > that pass_chkp can be in between.
> >
> > The patch below fices the ICE in PR 70965 and has passed bootstrap and
> > testing on x86_64-linux.  OK for trunk?
> 
> Ok.
> 
> Richard.
> 
> > Thanks,
> >
> > Martin
> >
> >
> > 2016-11-24  Martin Jambor  <mjambor@suse.cz>
> >
> > gcc/
> >         * passes.def (pass_build_ssa_passes): Add pass_rebuild_cgraph_edges.
> >
> > gcc/testsuite/
> >         * g++.dg/pr70965.C: New test.
> > ---
> >  gcc/passes.def                 |  1 +
> >  gcc/testsuite/g++.dg/pr70965.C | 21 +++++++++++++++++++++
> >  2 files changed, 22 insertions(+)
> >  create mode 100644 gcc/testsuite/g++.dg/pr70965.C
> >
> > diff --git a/gcc/passes.def b/gcc/passes.def
> > index 85a5af0..5faf17f 100644
> > --- a/gcc/passes.def
> > +++ b/gcc/passes.def
> > @@ -56,6 +56,7 @@ along with GCC; see the file COPYING3.  If not see
> >    NEXT_PASS (pass_build_ssa_passes);
> >    PUSH_INSERT_PASSES_WITHIN (pass_build_ssa_passes)
> >        NEXT_PASS (pass_fixup_cfg);
> > +      NEXT_PASS (pass_rebuild_cgraph_edges);
> >        NEXT_PASS (pass_build_ssa);
> >        NEXT_PASS (pass_warn_nonnull_compare);
> >        NEXT_PASS (pass_ubsan);

Actually you want to rebuild at the end of pass_build_ssa_passes passes queue.
This may still trip an ICE if one of passes bellow modify CFG (which pass_nothorw
does)

Path is OK with that change.
Honza
> > diff --git a/gcc/testsuite/g++.dg/pr70965.C b/gcc/testsuite/g++.dg/pr70965.C
> > new file mode 100644
> > index 0000000..d8a2c35
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/pr70965.C
> > @@ -0,0 +1,21 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -std=c++11" } */
> > +
> > +struct A {};
> > +struct B {};
> > +struct C { using p = int *; template <typename> using ra = A; };
> > +struct J : C { template <typename> struct K { typedef C::ra<int> o; }; };
> > +template <typename> struct D
> > +{
> > +  struct H : J::K<int>::o { H (J::p, A) : J::K<int>::o () {} };
> > +  H d;
> > +  D (const char *, const A &x = A ()) : d (0, x) {}
> > +};
> > +extern template class D<char>;
> > +enum L { M };
> > +struct F { virtual char *foo (); };
> > +template <class> struct I : B { static int foo (int) {} };
> > +struct G { typedef I<int> t; };
> > +void foo (int) { G::t::foo (0); }
> > +void bar (const D<char> &, const D<int> &, int, L);
> > +void baz () try { foo (0); } catch (F &e) { bar (e.foo (), "", 0, M); }
> > --
> > 2.10.2
> >

Re: [PR 70965] Schedule extra pass_rebuild_cgraph_edges

[Martin Jambor]

Hi,

On Fri, Nov 25, 2016 at 02:10:51PM +0100, Jan Hubicka wrote:
> > On Thu, Nov 24, 2016 at 5:44 PM, Martin Jambor <mjambor@suse.cz> wrote:
> > >
> > > ...
> > >
> > > 2016-11-24  Martin Jambor  <mjambor@suse.cz>
> > >
> > > gcc/
> > >         * passes.def (pass_build_ssa_passes): Add pass_rebuild_cgraph_edges.
> > >
> > > gcc/testsuite/
> > >         * g++.dg/pr70965.C: New test.
> > > ---
> > >  gcc/passes.def                 |  1 +
> > >  gcc/testsuite/g++.dg/pr70965.C | 21 +++++++++++++++++++++
> > >  2 files changed, 22 insertions(+)
> > >  create mode 100644 gcc/testsuite/g++.dg/pr70965.C
> > >
> > > diff --git a/gcc/passes.def b/gcc/passes.def
> > > index 85a5af0..5faf17f 100644
> > > --- a/gcc/passes.def
> > > +++ b/gcc/passes.def
> > > @@ -56,6 +56,7 @@ along with GCC; see the file COPYING3.  If not see
> > >    NEXT_PASS (pass_build_ssa_passes);
> > >    PUSH_INSERT_PASSES_WITHIN (pass_build_ssa_passes)
> > >        NEXT_PASS (pass_fixup_cfg);
> > > +      NEXT_PASS (pass_rebuild_cgraph_edges);
> > >        NEXT_PASS (pass_build_ssa);
> > >        NEXT_PASS (pass_warn_nonnull_compare);
> > >        NEXT_PASS (pass_ubsan);
> 
> Actually you want to rebuild at the end of pass_build_ssa_passes passes queue.
> This may still trip an ICE if one of passes bellow modify CFG (which pass_nothorw
> does)
> 
> Path is OK with that change.

Well, I have already committed the patch as it was.  But given the
above, I will commit the following to trunk after bootstrapping and
testing.

Thanks,

Martin


2016-12-02  Martin Jambor  <mjambor@suse.cz>

	* passes.def: Move pass_rebuild_cgraph_edges to the end of
	pass_build_ssa_passes.
---
 gcc/passes.def | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/passes.def b/gcc/passes.def
index 1117b8b..7b12a41 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -56,12 +56,12 @@ along with GCC; see the file COPYING3.  If not see
   NEXT_PASS (pass_build_ssa_passes);
   PUSH_INSERT_PASSES_WITHIN (pass_build_ssa_passes)
       NEXT_PASS (pass_fixup_cfg);
-      NEXT_PASS (pass_rebuild_cgraph_edges);
       NEXT_PASS (pass_build_ssa);
       NEXT_PASS (pass_warn_nonnull_compare);
       NEXT_PASS (pass_ubsan);
       NEXT_PASS (pass_early_warn_uninitialized);
       NEXT_PASS (pass_nothrow);
+      NEXT_PASS (pass_rebuild_cgraph_edges);
   POP_INSERT_PASSES ()
 
   NEXT_PASS (pass_chkp_instrumentation_passes);
-- 
2.10.2

Re: [PR 70965] Schedule extra pass_rebuild_cgraph_edges

[Jan Hubicka]

> Hi,
> 
> On Fri, Nov 25, 2016 at 02:10:51PM +0100, Jan Hubicka wrote:
> > > On Thu, Nov 24, 2016 at 5:44 PM, Martin Jambor <mjambor@suse.cz> wrote:
> > > >
> > > > ...
> > > >
> > > > 2016-11-24  Martin Jambor  <mjambor@suse.cz>
> > > >
> > > > gcc/
> > > >         * passes.def (pass_build_ssa_passes): Add pass_rebuild_cgraph_edges.
> > > >
> > > > gcc/testsuite/
> > > >         * g++.dg/pr70965.C: New test.
> > > > ---
> > > >  gcc/passes.def                 |  1 +
> > > >  gcc/testsuite/g++.dg/pr70965.C | 21 +++++++++++++++++++++
> > > >  2 files changed, 22 insertions(+)
> > > >  create mode 100644 gcc/testsuite/g++.dg/pr70965.C
> > > >
> > > > diff --git a/gcc/passes.def b/gcc/passes.def
> > > > index 85a5af0..5faf17f 100644
> > > > --- a/gcc/passes.def
> > > > +++ b/gcc/passes.def
> > > > @@ -56,6 +56,7 @@ along with GCC; see the file COPYING3.  If not see
> > > >    NEXT_PASS (pass_build_ssa_passes);
> > > >    PUSH_INSERT_PASSES_WITHIN (pass_build_ssa_passes)
> > > >        NEXT_PASS (pass_fixup_cfg);
> > > > +      NEXT_PASS (pass_rebuild_cgraph_edges);
> > > >        NEXT_PASS (pass_build_ssa);
> > > >        NEXT_PASS (pass_warn_nonnull_compare);
> > > >        NEXT_PASS (pass_ubsan);
> > 
> > Actually you want to rebuild at the end of pass_build_ssa_passes passes queue.
> > This may still trip an ICE if one of passes bellow modify CFG (which pass_nothorw
> > does)
> > 
> > Path is OK with that change.
> 
> Well, I have already committed the patch as it was.  But given the
> above, I will commit the following to trunk after bootstrapping and
> testing.
> 
> Thanks,
> 
> Martin
> 
> 
> 2016-12-02  Martin Jambor  <mjambor@suse.cz>
> 
> 	* passes.def: Move pass_rebuild_cgraph_edges to the end of
> 	pass_build_ssa_passes.

Thanks,
Honza