[PATCH] IPA: MODREF should skip EAF_* flags for indirect calls

[PATCH] IPA: MODREF should skip EAF_* flags for indirect calls

[Martin Liška]

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

Re: [PATCH] IPA: MODREF should skip EAF_* flags for indirect calls

[Martin Jambor]

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

Re: [PATCH] IPA: MODREF should skip EAF_* flags for indirect calls

[Jan Hubicka]

> 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

Re: [PATCH] IPA: MODREF should skip EAF_* flags for indirect calls

[Jan Hubicka]

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

Re: [PATCH] IPA: MODREF should skip EAF_* flags for indirect calls

[H.J. Lu]

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


-- 
H.J.

Re: [PATCH] IPA: MODREF should skip EAF_* flags for indirect calls

[Christophe Lyon]

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.
>

Re: [PATCH] IPA: MODREF should skip EAF_* flags for indirect calls

[Martin Liška]

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

Re: [PATCH] IPA: MODREF should skip EAF_* flags for indirect calls

[Jan Hubicka]

> 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&regtest not sure how I missed the
failure.

Honza

Re: [PATCH] IPA: MODREF should skip EAF_* flags for indirect calls

[Richard Biener]

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

Re: [PATCH] IPA: MODREF should skip EAF_* flags for indirect calls

[Jan Hubicka]

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

Re: [PATCH] IPA: MODREF should skip EAF_* flags for indirect calls

[Richard Biener]

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