diff mbox

[PR,70965] Schedule extra pass_rebuild_cgraph_edges

Message ID 20161124164456.m7aavu3k76zpc7z5@virgil.suse.cz
State New
Headers show

Commit Message

Martin Jambor Nov. 24, 2016, 4:44 p.m. UTC 
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

Comments

Richard Biener Nov. 25, 2016, 8:48 a.m. UTC | #1 
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
>
Jan Hubicka Nov. 25, 2016, 1:10 p.m. UTC | #2 
> 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
> >
diff mbox

Patch

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); }