| Message ID | 7d70a502-f714-77d1-f790-995e245b9ab2@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | IPA: MODREF should skip EAF_* flags for indirect calls | expand |
Hi, On Fri, Aug 20 2021, Martin Liška wrote: > Hello. > > As showed in the PR, returning (EAF_NOCLOBBER | EAF_NOESCAPE) for an argument > that is a function pointer is problematic. Doing such a function call is a clobber. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin > > PR 101949 > > gcc/ChangeLog: > > * ipa-modref.c (analyze_ssa_name_flags): Do not propagate EAF > flags arguments for indirect functions. > > gcc/testsuite/ChangeLog: > > * gcc.dg/lto/pr101949_0.c: New test. > * gcc.dg/lto/pr101949_1.c: New test. > > Co-Authored-By: Richard Biener <rguenther@suse.de> > --- > gcc/ipa-modref.c | 3 +++ > gcc/testsuite/gcc.dg/lto/pr101949_0.c | 20 ++++++++++++++++++++ > gcc/testsuite/gcc.dg/lto/pr101949_1.c | 4 ++++ > 3 files changed, 27 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/lto/pr101949_0.c > create mode 100644 gcc/testsuite/gcc.dg/lto/pr101949_1.c > > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c > index fafd804d4ba..380ba6926b9 100644 > --- a/gcc/ipa-modref.c > +++ b/gcc/ipa-modref.c > @@ -1715,6 +1715,9 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth, > else if (callee && !ipa && recursive_call_p (current_function_decl, > callee)) > lattice[index].merge (0); > + /* Ignore indirect calls (PR101949). */ > + else if (callee == NULL_TREE) > + lattice[index].merge (0); I just had a quick glance, but wouldn't bailing out only when gimple_call_fn (call) == name suffice here? Otherwise it seems to trigger also when passing NAME to an indirectly called function about which we might learn something from gimple_call_fntype, as the code apparently tries later on. Martin
> Hello. > > As showed in the PR, returning (EAF_NOCLOBBER | EAF_NOESCAPE) for an argument > that is a function pointer is problematic. Doing such a function call is a clobber. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin > > PR 101949 > > gcc/ChangeLog: > > * ipa-modref.c (analyze_ssa_name_flags): Do not propagate EAF > flags arguments for indirect functions. > > gcc/testsuite/ChangeLog: > > * gcc.dg/lto/pr101949_0.c: New test. > * gcc.dg/lto/pr101949_1.c: New test. > > Co-Authored-By: Richard Biener <rguenther@suse.de> > --- > gcc/ipa-modref.c | 3 +++ > gcc/testsuite/gcc.dg/lto/pr101949_0.c | 20 ++++++++++++++++++++ > gcc/testsuite/gcc.dg/lto/pr101949_1.c | 4 ++++ > 3 files changed, 27 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/lto/pr101949_0.c > create mode 100644 gcc/testsuite/gcc.dg/lto/pr101949_1.c > > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c > index fafd804d4ba..380ba6926b9 100644 > --- a/gcc/ipa-modref.c > +++ b/gcc/ipa-modref.c > @@ -1715,6 +1715,9 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth, > else if (callee && !ipa && recursive_call_p (current_function_decl, > callee)) > lattice[index].merge (0); > + /* Ignore indirect calls (PR101949). */ > + else if (callee == NULL_TREE) > + lattice[index].merge (0); Thanks for looking into this bug - it is interesting that ipa-pta requires !EAF_NOCLOBBER when function is called... I have some work done on teaching ipa-modref (and other propagation passes) to use ipa-devirt info when the full set of callees is known. This goes oposite way. You can drop flags only when callee == NAME and you can just frop EAF_NOCLOBBER. For example in testcase struct a { void (*foo)(); void *bar; } void wrap (struct a *a) { a->foo (); } will prevent us from figuring out that bar can not be modified when you pass non-ecaping instance of struct a to wrap. Honza
> Thanks for looking into this bug - it is interesting that ipa-pta > requires !EAF_NOCLOBBER when function is called... > > I have some work done on teaching ipa-modref (and other propagation > passes) to use ipa-devirt info when the full set of callees is known. > This goes oposite way. > > You can drop flags only when callee == NAME and you can just frop > EAF_NOCLOBBER. For example in testcase > > struct a { > void (*foo)(); > void *bar; > } > > void wrap (struct a *a) > { > a->foo (); > } > > will prevent us from figuring out that bar can not be modified when you > pass non-ecaping instance of struct a to wrap. > I am testing this updated patch which implements that. I am not very happy about this (it punishes -fno-ipa-pta path for not very good reason), but we need bugfix for release branch. It is very easy now to add now EAF flags at modref side so we can track EAF_NOT_CALLED. tree-ssa-structalias side is always bit anoying wrt new EAF flags because it has three copies of the code building constraints for call (for normal, pure and const). Modref is already tracking if function can read/modify global memory. I plan to add flags for NRC and link chain and then we can represent effect of ECF_CONST and PURE by simply adding flags. I would thus would like to merge that code. We do various optimizations to reduce amount of constriants produced, but hopefully this is not very important (or can be implemented by special casing in unified code). Honza gcc/ChangeLog: 2021-08-22 Jan Hubicka <hubicka@ucw.cz> Martin Liska <mliska@suse.cz> * ipa-modref.c (analyze_ssa_name_flags): Indirect call implies ~EAF_NOCLOBBER. gcc/testsuite/ChangeLog: 2021-08-22 Jan Hubicka <hubicka@ucw.cz> Martin Liska <mliska@suse.cz> * gcc.dg/lto/pr101949_0.c: New test. * gcc.dg/lto/pr101949_1.c: New test. diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c index fafd804d4ba..549153865b8 100644 --- a/gcc/ipa-modref.c +++ b/gcc/ipa-modref.c @@ -1700,6 +1700,15 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth, else if (gcall *call = dyn_cast <gcall *> (use_stmt)) { tree callee = gimple_call_fndecl (call); + + /* IPA PTA internally it treats calling a function as "writing" to + the argument space of all functions the function pointer points to + (PR101949). We can not drop EAF_NOCLOBBER only when ipa-pta + is on since that would allow propagation of this from -fno-ipa-pta + to -fipa-pta functions. */ + if (gimple_call_fn (use_stmt) == name) + lattice[index].merge (~EAF_NOCLOBBER); + /* Return slot optimization would require bit of propagation; give up for now. */ if (gimple_call_return_slot_opt_p (call) diff --git a/gcc/testsuite/gcc.dg/lto/pr101949_0.c b/gcc/testsuite/gcc.dg/lto/pr101949_0.c new file mode 100644 index 00000000000..142dffe8780 --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/pr101949_0.c @@ -0,0 +1,20 @@ +/* { dg-lto-do run } */ +/* { dg-lto-options { "-O2 -fipa-pta -flto -flto-partition=1to1" } } */ + +extern int bar (int (*)(int *), int *); + +static int x; + +static int __attribute__ ((noinline)) foo (int *p) +{ + *p = 1; + x = 0; + return *p; +} + +int main () +{ + if (bar (foo, &x) != 0) + __builtin_abort (); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/lto/pr101949_1.c b/gcc/testsuite/gcc.dg/lto/pr101949_1.c new file mode 100644 index 00000000000..871d15c9bfb --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/pr101949_1.c @@ -0,0 +1,4 @@ +int __attribute__((noinline,noclone)) bar (int (*fn)(int *), int *p) +{ + return fn (p); +}
On Sun, Aug 22, 2021 at 10:32 AM Jan Hubicka <hubicka@ucw.cz> wrote: > > > Thanks for looking into this bug - it is interesting that ipa-pta > > requires !EAF_NOCLOBBER when function is called... > > > > I have some work done on teaching ipa-modref (and other propagation > > passes) to use ipa-devirt info when the full set of callees is known. > > This goes oposite way. > > > > You can drop flags only when callee == NAME and you can just frop > > EAF_NOCLOBBER. For example in testcase > > > > struct a { > > void (*foo)(); > > void *bar; > > } > > > > void wrap (struct a *a) > > { > > a->foo (); > > } > > > > will prevent us from figuring out that bar can not be modified when you > > pass non-ecaping instance of struct a to wrap. > > > > I am testing this updated patch which implements that. I am not very > happy about this (it punishes -fno-ipa-pta path for not very good > reason), but we need bugfix for release branch. > > It is very easy now to add now EAF flags at modref side > so we can track EAF_NOT_CALLED. > tree-ssa-structalias side is always bit anoying wrt new EAF flags > because it has three copies of the code building constraints for call > (for normal, pure and const). > > Modref is already tracking if function can read/modify global memory. I > plan to add flags for NRC and link chain and then we can represent > effect of ECF_CONST and PURE by simply adding flags. I would thus would > like to merge that code. We do various optimizations to reduce amount > of constriants produced, but hopefully this is not very important (or > can be implemented by special casing in unified code). > > Honza > > gcc/ChangeLog: > > 2021-08-22 Jan Hubicka <hubicka@ucw.cz> > Martin Liska <mliska@suse.cz> > > * ipa-modref.c (analyze_ssa_name_flags): Indirect call implies > ~EAF_NOCLOBBER. > > gcc/testsuite/ChangeLog: > > 2021-08-22 Jan Hubicka <hubicka@ucw.cz> > Martin Liska <mliska@suse.cz> > > * gcc.dg/lto/pr101949_0.c: New test. > * gcc.dg/lto/pr101949_1.c: New test. > > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c > index fafd804d4ba..549153865b8 100644 > --- a/gcc/ipa-modref.c > +++ b/gcc/ipa-modref.c > @@ -1700,6 +1700,15 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth, > else if (gcall *call = dyn_cast <gcall *> (use_stmt)) > { > tree callee = gimple_call_fndecl (call); > + > + /* IPA PTA internally it treats calling a function as "writing" to > + the argument space of all functions the function pointer points to > + (PR101949). We can not drop EAF_NOCLOBBER only when ipa-pta > + is on since that would allow propagation of this from -fno-ipa-pta > + to -fipa-pta functions. */ > + if (gimple_call_fn (use_stmt) == name) > + lattice[index].merge (~EAF_NOCLOBBER); > + > /* Return slot optimization would require bit of propagation; > give up for now. */ > if (gimple_call_return_slot_opt_p (call) > diff --git a/gcc/testsuite/gcc.dg/lto/pr101949_0.c b/gcc/testsuite/gcc.dg/lto/pr101949_0.c > new file mode 100644 > index 00000000000..142dffe8780 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/lto/pr101949_0.c > @@ -0,0 +1,20 @@ > +/* { dg-lto-do run } */ > +/* { dg-lto-options { "-O2 -fipa-pta -flto -flto-partition=1to1" } } */ > + > +extern int bar (int (*)(int *), int *); > + > +static int x; > + > +static int __attribute__ ((noinline)) foo (int *p) > +{ > + *p = 1; > + x = 0; > + return *p; > +} > + > +int main () > +{ > + if (bar (foo, &x) != 0) > + __builtin_abort (); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/lto/pr101949_1.c b/gcc/testsuite/gcc.dg/lto/pr101949_1.c > new file mode 100644 > index 00000000000..871d15c9bfb > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/lto/pr101949_1.c > @@ -0,0 +1,4 @@ > +int __attribute__((noinline,noclone)) bar (int (*fn)(int *), int *p) > +{ > + return fn (p); > +} On Linux/x86-64 with -m32, I got FAIL: gcc.dg/lto/pr101949 c_lto_pr101949_0.o-c_lto_pr101949_1.o execute -O2 -fipa-pta -flto -flto-partition=1to1
On 8/22/21 19:32, Jan Hubicka wrote: >> Thanks for looking into this bug - it is interesting that ipa-pta >> requires !EAF_NOCLOBBER when function is called... >> >> I have some work done on teaching ipa-modref (and other propagation >> passes) to use ipa-devirt info when the full set of callees is known. >> This goes oposite way. >> >> You can drop flags only when callee == NAME and you can just frop >> EAF_NOCLOBBER. For example in testcase >> >> struct a { >> void (*foo)(); >> void *bar; >> } >> >> void wrap (struct a *a) >> { >> a->foo (); >> } >> >> will prevent us from figuring out that bar can not be modified when you >> pass non-ecaping instance of struct a to wrap. >> > > I am testing this updated patch which implements that. I am not very > happy about this (it punishes -fno-ipa-pta path for not very good > reason), but we need bugfix for release branch. Hello. Thanks for working on that. But have really run the test-cases as the newly added test still aborts as it used to before you installed this patch? Martin > > It is very easy now to add now EAF flags at modref side > so we can track EAF_NOT_CALLED. > tree-ssa-structalias side is always bit anoying wrt new EAF flags > because it has three copies of the code building constraints for call > (for normal, pure and const). > > Modref is already tracking if function can read/modify global memory. I > plan to add flags for NRC and link chain and then we can represent > effect of ECF_CONST and PURE by simply adding flags. I would thus would > like to merge that code. We do various optimizations to reduce amount > of constriants produced, but hopefully this is not very important (or > can be implemented by special casing in unified code). > > Honza > > gcc/ChangeLog: > > 2021-08-22 Jan Hubicka <hubicka@ucw.cz> > Martin Liska <mliska@suse.cz> > > * ipa-modref.c (analyze_ssa_name_flags): Indirect call implies > ~EAF_NOCLOBBER. > > gcc/testsuite/ChangeLog: > > 2021-08-22 Jan Hubicka <hubicka@ucw.cz> > Martin Liska <mliska@suse.cz> > > * gcc.dg/lto/pr101949_0.c: New test. > * gcc.dg/lto/pr101949_1.c: New test. > > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c > index fafd804d4ba..549153865b8 100644 > --- a/gcc/ipa-modref.c > +++ b/gcc/ipa-modref.c > @@ -1700,6 +1700,15 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth, > else if (gcall *call = dyn_cast <gcall *> (use_stmt)) > { > tree callee = gimple_call_fndecl (call); > + > + /* IPA PTA internally it treats calling a function as "writing" to > + the argument space of all functions the function pointer points to > + (PR101949). We can not drop EAF_NOCLOBBER only when ipa-pta > + is on since that would allow propagation of this from -fno-ipa-pta > + to -fipa-pta functions. */ > + if (gimple_call_fn (use_stmt) == name) > + lattice[index].merge (~EAF_NOCLOBBER); > + > /* Return slot optimization would require bit of propagation; > give up for now. */ > if (gimple_call_return_slot_opt_p (call) > diff --git a/gcc/testsuite/gcc.dg/lto/pr101949_0.c b/gcc/testsuite/gcc.dg/lto/pr101949_0.c > new file mode 100644 > index 00000000000..142dffe8780 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/lto/pr101949_0.c > @@ -0,0 +1,20 @@ > +/* { dg-lto-do run } */ > +/* { dg-lto-options { "-O2 -fipa-pta -flto -flto-partition=1to1" } } */ > + > +extern int bar (int (*)(int *), int *); > + > +static int x; > + > +static int __attribute__ ((noinline)) foo (int *p) > +{ > + *p = 1; > + x = 0; > + return *p; > +} > + > +int main () > +{ > + if (bar (foo, &x) != 0) > + __builtin_abort (); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/lto/pr101949_1.c b/gcc/testsuite/gcc.dg/lto/pr101949_1.c > new file mode 100644 > index 00000000000..871d15c9bfb > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/lto/pr101949_1.c > @@ -0,0 +1,4 @@ > +int __attribute__((noinline,noclone)) bar (int (*fn)(int *), int *p) > +{ > + return fn (p); > +} >
On Sun, Aug 22, 2021 at 11:47 PM H.J. Lu via Gcc-patches < gcc-patches@gcc.gnu.org> wrote: > On Sun, Aug 22, 2021 at 10:32 AM Jan Hubicka <hubicka@ucw.cz> wrote: > > > > > Thanks for looking into this bug - it is interesting that ipa-pta > > > requires !EAF_NOCLOBBER when function is called... > > > > > > I have some work done on teaching ipa-modref (and other propagation > > > passes) to use ipa-devirt info when the full set of callees is known. > > > This goes oposite way. > > > > > > You can drop flags only when callee == NAME and you can just frop > > > EAF_NOCLOBBER. For example in testcase > > > > > > struct a { > > > void (*foo)(); > > > void *bar; > > > } > > > > > > void wrap (struct a *a) > > > { > > > a->foo (); > > > } > > > > > > will prevent us from figuring out that bar can not be modified when you > > > pass non-ecaping instance of struct a to wrap. > > > > > > > I am testing this updated patch which implements that. I am not very > > happy about this (it punishes -fno-ipa-pta path for not very good > > reason), but we need bugfix for release branch. > > > > It is very easy now to add now EAF flags at modref side > > so we can track EAF_NOT_CALLED. > > tree-ssa-structalias side is always bit anoying wrt new EAF flags > > because it has three copies of the code building constraints for call > > (for normal, pure and const). > > > > Modref is already tracking if function can read/modify global memory. I > > plan to add flags for NRC and link chain and then we can represent > > effect of ECF_CONST and PURE by simply adding flags. I would thus would > > like to merge that code. We do various optimizations to reduce amount > > of constriants produced, but hopefully this is not very important (or > > can be implemented by special casing in unified code). > > > > Honza > > > > gcc/ChangeLog: > > > > 2021-08-22 Jan Hubicka <hubicka@ucw.cz> > > Martin Liska <mliska@suse.cz> > > > > * ipa-modref.c (analyze_ssa_name_flags): Indirect call implies > > ~EAF_NOCLOBBER. > > > > gcc/testsuite/ChangeLog: > > > > 2021-08-22 Jan Hubicka <hubicka@ucw.cz> > > Martin Liska <mliska@suse.cz> > > > > * gcc.dg/lto/pr101949_0.c: New test. > > * gcc.dg/lto/pr101949_1.c: New test. > > > > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c > > index fafd804d4ba..549153865b8 100644 > > --- a/gcc/ipa-modref.c > > +++ b/gcc/ipa-modref.c > > @@ -1700,6 +1700,15 @@ analyze_ssa_name_flags (tree name, > vec<modref_lattice> &lattice, int depth, > > else if (gcall *call = dyn_cast <gcall *> (use_stmt)) > > { > > tree callee = gimple_call_fndecl (call); > > + > > + /* IPA PTA internally it treats calling a function as > "writing" to > > + the argument space of all functions the function pointer > points to > > + (PR101949). We can not drop EAF_NOCLOBBER only when ipa-pta > > + is on since that would allow propagation of this from > -fno-ipa-pta > > + to -fipa-pta functions. */ > > + if (gimple_call_fn (use_stmt) == name) > > + lattice[index].merge (~EAF_NOCLOBBER); > > + > > /* Return slot optimization would require bit of propagation; > > give up for now. */ > > if (gimple_call_return_slot_opt_p (call) > > diff --git a/gcc/testsuite/gcc.dg/lto/pr101949_0.c > b/gcc/testsuite/gcc.dg/lto/pr101949_0.c > > new file mode 100644 > > index 00000000000..142dffe8780 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/lto/pr101949_0.c > > @@ -0,0 +1,20 @@ > > +/* { dg-lto-do run } */ > > +/* { dg-lto-options { "-O2 -fipa-pta -flto -flto-partition=1to1" } } */ > > + > > +extern int bar (int (*)(int *), int *); > > + > > +static int x; > > + > > +static int __attribute__ ((noinline)) foo (int *p) > > +{ > > + *p = 1; > > + x = 0; > > + return *p; > > +} > > + > > +int main () > > +{ > > + if (bar (foo, &x) != 0) > > + __builtin_abort (); > > + return 0; > > +} > > diff --git a/gcc/testsuite/gcc.dg/lto/pr101949_1.c > b/gcc/testsuite/gcc.dg/lto/pr101949_1.c > > new file mode 100644 > > index 00000000000..871d15c9bfb > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/lto/pr101949_1.c > > @@ -0,0 +1,4 @@ > > +int __attribute__((noinline,noclone)) bar (int (*fn)(int *), int *p) > > +{ > > + return fn (p); > > +} > > On Linux/x86-64 with -m32, I got > > FAIL: gcc.dg/lto/pr101949 c_lto_pr101949_0.o-c_lto_pr101949_1.o > execute -O2 -fipa-pta -flto -flto-partition=1to1 > > It fails on aarch64 and arm too. > > -- > H.J. >
> Hello. > > Thanks for working on that. But have really run the test-cases as the newly > added test still aborts as it used to before you installed this patch? Eh, sorry, I had earlier version of patch that did if (gimple_call_fn (use_stmt) == name) lattice[index].merge (0); like yours and then I noticed that dropping things like EAF_NOT_RETURNED is not necessary. However instead if (gimple_call_fn (use_stmt) == name) lattice[index].merge (~EAF_NOCLOBBER); It should be if (gimple_call_fn (use_stmt) == name) lattice[index].merge (~(EAF_NOCLOBBER | EAF_UNUSED)); Since EAF_UNUSED implies all the other flags so the merge becomes noop with ~EAF_NOCLOBBER. I will test the fix. I remember re-running bootstrap®test not sure how I missed the failure. Honza
On Sun, Aug 22, 2021 at 7:32 PM Jan Hubicka <hubicka@ucw.cz> wrote: > > > Thanks for looking into this bug - it is interesting that ipa-pta > > requires !EAF_NOCLOBBER when function is called... > > > > I have some work done on teaching ipa-modref (and other propagation > > passes) to use ipa-devirt info when the full set of callees is known. > > This goes oposite way. > > > > You can drop flags only when callee == NAME and you can just frop > > EAF_NOCLOBBER. For example in testcase > > > > struct a { > > void (*foo)(); > > void *bar; > > } > > > > void wrap (struct a *a) > > { > > a->foo (); > > } > > > > will prevent us from figuring out that bar can not be modified when you > > pass non-ecaping instance of struct a to wrap. > > > > I am testing this updated patch which implements that. I am not very > happy about this (it punishes -fno-ipa-pta path for not very good > reason), but we need bugfix for release branch. Why does it "punish" -fno-ipa-pta? It merely "punishes" modref of no longer claiming that we do not alter the instruction stream pointed to by a->foo, sth that shouldn't be very common. Note that IPA PTA doesn't really know whether the passed argument is a function or not, also 'wrap' could just receive a void * pointer and still call the function. So we're very much relying on how this all plays out ... > It is very easy now to add now EAF flags at modref side > so we can track EAF_NOT_CALLED. > tree-ssa-structalias side is always bit anoying wrt new EAF flags > because it has three copies of the code building constraints for call > (for normal, pure and const). > > Modref is already tracking if function can read/modify global memory. I > plan to add flags for NRC and link chain and then we can represent > effect of ECF_CONST and PURE by simply adding flags. I would thus would > like to merge that code. We do various optimizations to reduce amount > of constriants produced, but hopefully this is not very important (or > can be implemented by special casing in unified code). > > Honza > > gcc/ChangeLog: > > 2021-08-22 Jan Hubicka <hubicka@ucw.cz> > Martin Liska <mliska@suse.cz> > > * ipa-modref.c (analyze_ssa_name_flags): Indirect call implies > ~EAF_NOCLOBBER. > > gcc/testsuite/ChangeLog: > > 2021-08-22 Jan Hubicka <hubicka@ucw.cz> > Martin Liska <mliska@suse.cz> > > * gcc.dg/lto/pr101949_0.c: New test. > * gcc.dg/lto/pr101949_1.c: New test. > > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c > index fafd804d4ba..549153865b8 100644 > --- a/gcc/ipa-modref.c > +++ b/gcc/ipa-modref.c > @@ -1700,6 +1700,15 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth, > else if (gcall *call = dyn_cast <gcall *> (use_stmt)) > { > tree callee = gimple_call_fndecl (call); > + > + /* IPA PTA internally it treats calling a function as "writing" to > + the argument space of all functions the function pointer points to > + (PR101949). We can not drop EAF_NOCLOBBER only when ipa-pta > + is on since that would allow propagation of this from -fno-ipa-pta > + to -fipa-pta functions. */ > + if (gimple_call_fn (use_stmt) == name) > + lattice[index].merge (~EAF_NOCLOBBER); > + > /* Return slot optimization would require bit of propagation; > give up for now. */ > if (gimple_call_return_slot_opt_p (call) > diff --git a/gcc/testsuite/gcc.dg/lto/pr101949_0.c b/gcc/testsuite/gcc.dg/lto/pr101949_0.c > new file mode 100644 > index 00000000000..142dffe8780 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/lto/pr101949_0.c > @@ -0,0 +1,20 @@ > +/* { dg-lto-do run } */ > +/* { dg-lto-options { "-O2 -fipa-pta -flto -flto-partition=1to1" } } */ > + > +extern int bar (int (*)(int *), int *); > + > +static int x; > + > +static int __attribute__ ((noinline)) foo (int *p) > +{ > + *p = 1; > + x = 0; > + return *p; > +} > + > +int main () > +{ > + if (bar (foo, &x) != 0) > + __builtin_abort (); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/lto/pr101949_1.c b/gcc/testsuite/gcc.dg/lto/pr101949_1.c > new file mode 100644 > index 00000000000..871d15c9bfb > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/lto/pr101949_1.c > @@ -0,0 +1,4 @@ > +int __attribute__((noinline,noclone)) bar (int (*fn)(int *), int *p) > +{ > + return fn (p); > +}
Hi, > > Why does it "punish" -fno-ipa-pta? It merely "punishes" modref of > no longer claiming that we do not alter the instruction stream pointed > to by a->foo, sth that shouldn't be very common. For example struct a { void (*foo)(); void *bar; } fn(struct a *a) { a->foo(); } With Maritn's patch we will drop EAF flags of A to NODIRECTESCAPE since we will think its derefernece is is used in all posible ways. With my patch we get NOT_RETURNED | NOESCAPE. Still we will make PTA to think that whatever is pointed to by bar may be clobbered and this seems unnecessary. I have to look into ipa-pta how it haldnes the "instruction stream clobbering". I was not aware it does something smart about indirect calls. Honza
On Mon, Aug 23, 2021 at 2:02 PM Jan Hubicka <hubicka@ucw.cz> wrote: > > Hi, > > > > Why does it "punish" -fno-ipa-pta? It merely "punishes" modref of > > no longer claiming that we do not alter the instruction stream pointed > > to by a->foo, sth that shouldn't be very common. > > For example > struct a { > void (*foo)(); > void *bar; > } > fn(struct a *a) > { > a->foo(); > } > > With Maritn's patch we will drop EAF flags of A to NODIRECTESCAPE since > we will think its derefernece is is used in all posible ways. > > With my patch we get NOT_RETURNED | NOESCAPE. > Still we will make PTA to think that whatever is pointed to by bar may > be clobbered and this seems unnecessary. > > I have to look into ipa-pta how it haldnes the "instruction stream > clobbering". I was not aware it does something smart about indirect > calls. It actually isn't "smart" but it simply makes indirect calls exactly the same as direct calls so if you have (*ESCAPED) (a, b, c); then it magically appears as a function call to all IPA PTA handled functions that escaped. So some handling is needed for correctness and yes, it's fancier than simply assuming all address-taken functions have unknown incoming parameters. Richard. > > Honza
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c index fafd804d4ba..380ba6926b9 100644 --- a/gcc/ipa-modref.c +++ b/gcc/ipa-modref.c @@ -1715,6 +1715,9 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth, else if (callee && !ipa && recursive_call_p (current_function_decl, callee)) lattice[index].merge (0); + /* Ignore indirect calls (PR101949). */ + else if (callee == NULL_TREE) + lattice[index].merge (0); else { int ecf_flags = gimple_call_flags (call); diff --git a/gcc/testsuite/gcc.dg/lto/pr101949_0.c b/gcc/testsuite/gcc.dg/lto/pr101949_0.c new file mode 100644 index 00000000000..142dffe8780 --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/pr101949_0.c @@ -0,0 +1,20 @@ +/* { dg-lto-do run } */ +/* { dg-lto-options { "-O2 -fipa-pta -flto -flto-partition=1to1" } } */ + +extern int bar (int (*)(int *), int *); + +static int x; + +static int __attribute__ ((noinline)) foo (int *p) +{ + *p = 1; + x = 0; + return *p; +} + +int main () +{ + if (bar (foo, &x) != 0) + __builtin_abort (); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/lto/pr101949_1.c b/gcc/testsuite/gcc.dg/lto/pr101949_1.c new file mode 100644 index 00000000000..871d15c9bfb --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/pr101949_1.c @@ -0,0 +1,4 @@ +int __attribute__((noinline,noclone)) bar (int (*fn)(int *), int *p) +{ + return fn (p); +}