Re: [PATCH] cgraphclones: Don't share DECL_ARGUMENTS between thunk and its artificial thunk [PR108854]
> Hi! > > The following testcase ICEs on x86_64-linux with -m32. The problem is > we create an artificial thunk and because of -fPIC, ia32 and thunk > destination which doesn't bind locally can't use a mi thunk. > The ICE is because during expansion to RTL we see SSA_NAME for a PARM_DECL, > but the PARM_DECL doesn't have DECL_CONTEXT of the current function. > This is because duplicate_thunk_for_node creates a new DECL_ARGUMENTS chain > only if some arguments need modification. > > The following patch fixes it by copying the DECL_ARGUMENTS list even if > the arguments can stay as is, to update DECL_CONTEXT on them. While for > mi thunks it doesn't really matter because we don't use those arguments > in any way, for other thunks it is important. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2023-02-23 Jakub Jelinek > > PR middle-end/108854 > * cgraphclones.cc (duplicate_thunk_for_node): If no parameter > changes are needed, copy at least DECL_ARGUMENTS PARM_DECL > nodes and adjust their DECL_CONTEXT. > > * g++.dg/opt/pr108854.C: New test. > > --- gcc/cgraphclones.cc.jj2023-02-22 20:50:27.417519830 +0100 > +++ gcc/cgraphclones.cc 2023-02-23 17:12:59.875133883 +0100 > @@ -218,7 +218,17 @@ duplicate_thunk_for_node (cgraph_node *t >body_adj.modify_formal_parameters (); > } >else > -new_decl = copy_node (thunk->decl); > +{ > + new_decl = copy_node (thunk->decl); > + for (tree *arg = _ARGUMENTS (new_decl); > +*arg; arg = _CHAIN (*arg)) > + { > + tree next = DECL_CHAIN (*arg); > + *arg = copy_node (*arg); > + DECL_CONTEXT (*arg) = new_decl; > + DECL_CHAIN (*arg) = next; This makes sense to me. I wonder if we don't want to update abstract origin too like we do in tree-inline? Maybe it is unecessary since we don't do debug info for thunks Jan
Re: [PATCH] cgraphclones: Don't share DECL_ARGUMENTS between thunk and its artificial thunk [PR108854]
On Fri, 24 Feb 2023, Jakub Jelinek wrote: > Hi! > > The following testcase ICEs on x86_64-linux with -m32. The problem is > we create an artificial thunk and because of -fPIC, ia32 and thunk > destination which doesn't bind locally can't use a mi thunk. > The ICE is because during expansion to RTL we see SSA_NAME for a PARM_DECL, > but the PARM_DECL doesn't have DECL_CONTEXT of the current function. > This is because duplicate_thunk_for_node creates a new DECL_ARGUMENTS chain > only if some arguments need modification. > > The following patch fixes it by copying the DECL_ARGUMENTS list even if > the arguments can stay as is, to update DECL_CONTEXT on them. While for > mi thunks it doesn't really matter because we don't use those arguments > in any way, for other thunks it is important. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. > 2023-02-23 Jakub Jelinek > > PR middle-end/108854 > * cgraphclones.cc (duplicate_thunk_for_node): If no parameter > changes are needed, copy at least DECL_ARGUMENTS PARM_DECL > nodes and adjust their DECL_CONTEXT. > > * g++.dg/opt/pr108854.C: New test. > > --- gcc/cgraphclones.cc.jj2023-02-22 20:50:27.417519830 +0100 > +++ gcc/cgraphclones.cc 2023-02-23 17:12:59.875133883 +0100 > @@ -218,7 +218,17 @@ duplicate_thunk_for_node (cgraph_node *t >body_adj.modify_formal_parameters (); > } >else > -new_decl = copy_node (thunk->decl); > +{ > + new_decl = copy_node (thunk->decl); > + for (tree *arg = _ARGUMENTS (new_decl); > +*arg; arg = _CHAIN (*arg)) > + { > + tree next = DECL_CHAIN (*arg); > + *arg = copy_node (*arg); > + DECL_CONTEXT (*arg) = new_decl; > + DECL_CHAIN (*arg) = next; > + } > +} > >gcc_checking_assert (!DECL_STRUCT_FUNCTION (new_decl)); >gcc_checking_assert (!DECL_INITIAL (new_decl)); > --- gcc/testsuite/g++.dg/opt/pr108854.C.jj2023-02-23 17:11:19.275583506 > +0100 > +++ gcc/testsuite/g++.dg/opt/pr108854.C 2023-02-23 17:11:02.723822009 > +0100 > @@ -0,0 +1,37 @@ > +// PR middle-end/108854 > +// { dg-do compile { target c++11 } } > +// { dg-options "-O3" } > +// { dg-additional-options "-fPIC" { target fpic } } > + > +struct A { A (int); ~A (); }; > +struct B { B (int, bool); ~B (); }; > +template > +struct C { void m1 (T); void m2 (T &&); }; > +class D; > +struct E { virtual void m3 (); }; > +template > +struct F { virtual bool m4 (D &); }; > +struct D { virtual D m5 () { return D (); } }; > +void foo (void *, void *); > +struct G { > + int a; > + C b; > + void m4 (D ) { B l (a, true); r.m5 (); b.m1 (); b.m2 (); } > +}; > +struct H : E, F { > + template > + H (int, T); > + bool m4 (D ) { A l (a); b.m4 (r); if (c) return true; } // { dg-warning > "control reaches end of non-void function" } > + int a; > + bool c; > + G b; > +}; > +inline void bar (F ) { D s, t; p.m4 (t); foo (, ); } > +enum I { I1, I2 }; > +template > +struct J; > +template > +void baz () { int g = 0, h = 0; T i (g, h); bar (i); } > +template > +void qux () { baz > (); } > +void corge () { qux (); qux (); } > > Jakub > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)
[PATCH] cgraphclones: Don't share DECL_ARGUMENTS between thunk and its artificial thunk [PR108854]
Hi! The following testcase ICEs on x86_64-linux with -m32. The problem is we create an artificial thunk and because of -fPIC, ia32 and thunk destination which doesn't bind locally can't use a mi thunk. The ICE is because during expansion to RTL we see SSA_NAME for a PARM_DECL, but the PARM_DECL doesn't have DECL_CONTEXT of the current function. This is because duplicate_thunk_for_node creates a new DECL_ARGUMENTS chain only if some arguments need modification. The following patch fixes it by copying the DECL_ARGUMENTS list even if the arguments can stay as is, to update DECL_CONTEXT on them. While for mi thunks it doesn't really matter because we don't use those arguments in any way, for other thunks it is important. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2023-02-23 Jakub Jelinek PR middle-end/108854 * cgraphclones.cc (duplicate_thunk_for_node): If no parameter changes are needed, copy at least DECL_ARGUMENTS PARM_DECL nodes and adjust their DECL_CONTEXT. * g++.dg/opt/pr108854.C: New test. --- gcc/cgraphclones.cc.jj 2023-02-22 20:50:27.417519830 +0100 +++ gcc/cgraphclones.cc 2023-02-23 17:12:59.875133883 +0100 @@ -218,7 +218,17 @@ duplicate_thunk_for_node (cgraph_node *t body_adj.modify_formal_parameters (); } else -new_decl = copy_node (thunk->decl); +{ + new_decl = copy_node (thunk->decl); + for (tree *arg = _ARGUMENTS (new_decl); + *arg; arg = _CHAIN (*arg)) + { + tree next = DECL_CHAIN (*arg); + *arg = copy_node (*arg); + DECL_CONTEXT (*arg) = new_decl; + DECL_CHAIN (*arg) = next; + } +} gcc_checking_assert (!DECL_STRUCT_FUNCTION (new_decl)); gcc_checking_assert (!DECL_INITIAL (new_decl)); --- gcc/testsuite/g++.dg/opt/pr108854.C.jj 2023-02-23 17:11:19.275583506 +0100 +++ gcc/testsuite/g++.dg/opt/pr108854.C 2023-02-23 17:11:02.723822009 +0100 @@ -0,0 +1,37 @@ +// PR middle-end/108854 +// { dg-do compile { target c++11 } } +// { dg-options "-O3" } +// { dg-additional-options "-fPIC" { target fpic } } + +struct A { A (int); ~A (); }; +struct B { B (int, bool); ~B (); }; +template +struct C { void m1 (T); void m2 (T &&); }; +class D; +struct E { virtual void m3 (); }; +template +struct F { virtual bool m4 (D &); }; +struct D { virtual D m5 () { return D (); } }; +void foo (void *, void *); +struct G { + int a; + C b; + void m4 (D ) { B l (a, true); r.m5 (); b.m1 (); b.m2 (); } +}; +struct H : E, F { + template + H (int, T); + bool m4 (D ) { A l (a); b.m4 (r); if (c) return true; } // { dg-warning "control reaches end of non-void function" } + int a; + bool c; + G b; +}; +inline void bar (F ) { D s, t; p.m4 (t); foo (, ); } +enum I { I1, I2 }; +template +struct J; +template +void baz () { int g = 0, h = 0; T i (g, h); bar (i); } +template +void qux () { baz > (); } +void corge () { qux (); qux (); } Jakub