Definition of EAF_NOESCAPE and fnspec strings

Definition of EAF_NOESCAPE and fnspec strings

[Jan Hubicka]

Hi,
I implemented simple propagation of EAF arguments for ipa-modref (that is not
hard to do).  My main aim was to detect cases where THIS parameter does not
escape but is used to read/write pointed to memory.  This is meant to
avoid poor code produced when we i.e. offline destructors on cold path.

Unfortunately detecting EAF_NOESCAPE for such parameters breaks.  This is already
visible on memcpy if we let it to be be handled via its fnspec
attribute. If I disable special handling in structalias:

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index a4832b75436..2b614913b57 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -4389,7 +4389,6 @@ find_func_aliases_for_builtin_call (struct function *fn, gcall *t)
       case BUILT_IN_STRCPY:
       case BUILT_IN_STRNCPY:
       case BUILT_IN_BCOPY:
-      case BUILT_IN_MEMCPY:
       case BUILT_IN_MEMMOVE:
       case BUILT_IN_MEMPCPY:
       case BUILT_IN_STPCPY:

In the following testcase we miss the fact that memcpy may merge i4p PTA
set to i3p.

extern void exit (int);
extern void abort (void);
int size = 8;

int main (void)
{
  int i3 = -1, i4 = 55;
  int *i3p = &i3;
  int *i4p = &i4;

  memcpy(&i3p, &i4p, size);
  if (i3p != i4p)
    abort ();
  exit (0);
}

This seems to be documented for some degree:

	  /* Add *tem = nonlocal, do not add *tem = callused as
	     EAF_NOESCAPE parameters do not escape to other parameters
	     and all other uses appear in NONLOCAL as well.  */

But it also means that some of our FNSPECs are wrong now.  I wonder if we can
split this porperty to two different flags like EAF_NOEASCAPE and
EAF_INDIRECT_NOESCAPE?

Auto-detecting EAF_INDIRECT_NOESCAPE seems bit harder at least assuming
that any read can actually load few bits of pointer possibly written
there beause if simple member functions accesses values pointer by THIS
and return them we lose a track if the returned value is an escape point
I would say.

The difference in constraints are:

--- good/a.c.035t.ealias	2020-11-08 13:34:04.397499515 +0100
+++ a.c.035t.ealias	2020-11-08 13:39:15.483438469 +0100
@@ -106,7 +106,15 @@
 size = NONLOCAL
 size.0_1 = size
 _2 = size.0_1
-i3p = i4p
+callarg(18) = &i3p
+callarg(18) = callarg(18) + UNKNOWN
+CALLUSED(16) = callarg(18)
+CALLCLOBBERED(17) = callarg(18)
+*callarg(18) = NONLOCAL
+callarg(19) = &i4p
+callarg(19) = callarg(19) + UNKNOWN
+CALLUSED(16) = callarg(19)
+ESCAPED = _2
 i3p.1_3 = i3p
 i4p.2_4 = i4p
 ESCAPED = &NULL

...

 Call clobber information
 
-ESCAPED, points-to NULL, points-to vars: { }
+ESCAPED, points-to non-local, points-to NULL, points-to vars: { }
 
 Flow-insensitive points-to information
 
-i3p.1_3, points-to NULL, points-to vars: { D.1947 D.1948 }
+i3p.1_3, points-to non-local, points-to escaped, points-to NULL, points-to vars: { D.1947 }
 i4p.2_4, points-to NULL, points-to vars: { D.1948 }
 
 main ()

Honza

Re: Definition of EAF_NOESCAPE and fnspec strings

[Richard Biener]

On Sun, 8 Nov 2020, Jan Hubicka wrote:

> Hi,
> I implemented simple propagation of EAF arguments for ipa-modref (that is not
> hard to do).  My main aim was to detect cases where THIS parameter does not
> escape but is used to read/write pointed to memory.  This is meant to
> avoid poor code produced when we i.e. offline destructors on cold path.
> 
> Unfortunately detecting EAF_NOESCAPE for such parameters breaks.  This is already
> visible on memcpy if we let it to be be handled via its fnspec
> attribute. If I disable special handling in structalias:
>
> diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
> index a4832b75436..2b614913b57 100644
> --- a/gcc/tree-ssa-structalias.c
> +++ b/gcc/tree-ssa-structalias.c
> @@ -4389,7 +4389,6 @@ find_func_aliases_for_builtin_call (struct function *fn, gcall *t)
>        case BUILT_IN_STRCPY:
>        case BUILT_IN_STRNCPY:
>        case BUILT_IN_BCOPY:
> -      case BUILT_IN_MEMCPY:
>        case BUILT_IN_MEMMOVE:
>        case BUILT_IN_MEMPCPY:
>        case BUILT_IN_STPCPY:
> 
> In the following testcase we miss the fact that memcpy may merge i4p PTA
> set to i3p.
> 
> extern void exit (int);
> extern void abort (void);
> int size = 8;
> 
> int main (void)
> {
>   int i3 = -1, i4 = 55;
>   int *i3p = &i3;
>   int *i4p = &i4;
> 
>   memcpy(&i3p, &i4p, size);
>   if (i3p != i4p)
>     abort ();
>   exit (0);
> }
> 
> This seems to be documented for some degree:
> 
> 	  /* Add *tem = nonlocal, do not add *tem = callused as
> 	     EAF_NOESCAPE parameters do not escape to other parameters
> 	     and all other uses appear in NONLOCAL as well.  */
> 
> But it also means that some of our FNSPECs are wrong now.  I wonder if we can
> split this porperty to two different flags like EAF_NOEASCAPE and
> EAF_INDIRECT_NOESCAPE?

Note that EAF_NOESCAPE allows "escaping" to the return value (see
handle_rhs_call).  I guess for simplicity we could allow escaping
of not escaped but USED params to other written to params.  I think
we also don't handle "escaping" of a parameter indirectly to itself, thus

int i;
int *p = &i;
int **q = &p;
foo (&q);
if (*q != i)
  abort ();

and

foo (int ***p)
{
  *p = **p;
} 

or so with foos param EAF_NOESCAPE (but not EAF_DIRECT).

Splitting up EAF_NOESCAPE makes it quite difficult to understand.
Arguably explicit handling of memcpy and friends _does_ pay off
for points-to analysis since I'd say modelling all possible and useful
things in fnspec would make it a monster ... you'd basically want
to have a way to specify additional constraints in the fnspec itself,
like *1 = *2, but then also distinguish points-to effects from
may-alias ones.

I wonder if we should teach the GIMPLE FE to parse 'fn spec'
so we can write unit tests for the attribute ... or maybe simply
add this to the __GIMPLE spec string.

> Auto-detecting EAF_INDIRECT_NOESCAPE seems bit harder at least assuming
> that any read can actually load few bits of pointer possibly written
> there beause if simple member functions accesses values pointer by THIS
> and return them we lose a track if the returned value is an escape point
> I would say.
> 
> The difference in constraints are:
> 
> --- good/a.c.035t.ealias	2020-11-08 13:34:04.397499515 +0100
> +++ a.c.035t.ealias	2020-11-08 13:39:15.483438469 +0100
> @@ -106,7 +106,15 @@
>  size = NONLOCAL
>  size.0_1 = size
>  _2 = size.0_1
> -i3p = i4p
> +callarg(18) = &i3p
> +callarg(18) = callarg(18) + UNKNOWN
> +CALLUSED(16) = callarg(18)
> +CALLCLOBBERED(17) = callarg(18)
> +*callarg(18) = NONLOCAL
> +callarg(19) = &i4p
> +callarg(19) = callarg(19) + UNKNOWN
> +CALLUSED(16) = callarg(19)
> +ESCAPED = _2
>  i3p.1_3 = i3p
>  i4p.2_4 = i4p
>  ESCAPED = &NULL
> 
> ...
> 
>  Call clobber information
>  
> -ESCAPED, points-to NULL, points-to vars: { }
> +ESCAPED, points-to non-local, points-to NULL, points-to vars: { }
>  
>  Flow-insensitive points-to information
>  
> -i3p.1_3, points-to NULL, points-to vars: { D.1947 D.1948 }
> +i3p.1_3, points-to non-local, points-to escaped, points-to NULL, points-to vars: { D.1947 }
>  i4p.2_4, points-to NULL, points-to vars: { D.1948 }
>  
>  main ()
> 
> Honza
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend

Re: Definition of EAF_NOESCAPE and fnspec strings

[Jan Hubicka]

> > But it also means that some of our FNSPECs are wrong now.  I wonder if we can
> > split this porperty to two different flags like EAF_NOEASCAPE and
> > EAF_INDIRECT_NOESCAPE?
> 
> Note that EAF_NOESCAPE allows "escaping" to the return value (see
> handle_rhs_call).  I guess for simplicity we could allow escaping

I see, missed that! 

> of not escaped but USED params to other written to params.  I think
> we also don't handle "escaping" of a parameter indirectly to itself, thus
> 
> int i;
> int *p = &i;
> int **q = &p;
> foo (&q);
> if (*q != i)
>   abort ();
> 
> and
> 
> foo (int ***p)
> {
>   *p = **p;
> } 
> 
> or so with foos param EAF_NOESCAPE (but not EAF_DIRECT).
> 
> Splitting up EAF_NOESCAPE makes it quite difficult to understand.
> Arguably explicit handling of memcpy and friends _does_ pay off
> for points-to analysis since I'd say modelling all possible and useful
> things in fnspec would make it a monster ... you'd basically want
> to have a way to specify additional constraints in the fnspec itself,
> like *1 = *2, but then also distinguish points-to effects from
> may-alias ones.

Yep, i am not arguing for eliminating special case of memcpy (because we
have the additional info that it only copies pointers from *src to
*dest).

However I find current definition of EAF_NOESCAPE bit hard to handle in
modref, since naturally it is quite reliable to track all uses of ssa
name that correspond to parameters, but it is harder to track where
values read from pointed-to memory can eventually go.

What I do is to walk all uses of SSA_NAME that correspond to parameter
and try to unerstand it.  If it is one of
 1) memory load via derefence of name
 2) memory store where name is base of LHS
 3) memory store where name is rhs
 4) name is passed as a value to function 
 5) dereferenced value from name is passed to funtion
 6) used as value normal gimple expression
 7) used in return (as RHS or base of memory dereference address)
 8) it is used only in reference but not as base (as array index or so)

Then I merge in (and) flags I determine as follow:

For 1) clear EAF_USED and recurse to LHS name. Based on its flags I
decide on:
  - EAF_DIRECT (if LHS has EAF_UNUSED),
  - EAF_NOCLOBBER (if LHS has EAF_NOCLOBBER)
  - EAF_NOESCAPE (if LHS has EAF_NOESCAPE). 

For 2) I clear EAF_NOCLOBBER and EAF_UNUSED flag

For 3) I give up (clear all flags) since I would need to track where the
memory is going.

For 4) I determine flag of function parameter

For 5) I need to do same handling as 1) where flag of "loaded value" is
flag of the function

For 6) I determine flags of LHS and merge them in

For 7) I clear NOESCAPE if rhs is name itself
and UNUSED + NOESCAPE if rhs is derefernece from name.

For 8) I do nothing.  Here the names are non-pointers that I track
because of earlier dereference.



So I think 7) can be relaxed.  Main problem is hoever that we often see 1)
and then 3) or 7) on LHS that makes us punt very often.

The fact that pointer directly does not escape but pointed to memory can
seems still very useful since one does not need to add *ptr to points-to
sets. But I will try relaxing 7).

If we allow values escaping to other parameters and itself, I think I
can relax 3) if base of the store is default def of PARM_DECL.
> 
> I wonder if we should teach the GIMPLE FE to parse 'fn spec'
> so we can write unit tests for the attribute ... or maybe simply
> add this to the __GIMPLE spec string.

May be nice and also describe carefully that NOESCAPE and NOCLOBBER also
reffers to indirect references.  Current description
"Nonzero if the argument does not escape."
reads to me that it is about ptr itself, not about *ptr and also it does
not speak of the escaping to return value etc.

Honza

Re: Definition of EAF_NOESCAPE and fnspec strings

[Richard Biener]

On Mon, 9 Nov 2020, Jan Hubicka wrote:

> > > But it also means that some of our FNSPECs are wrong now.  I wonder if we can
> > > split this porperty to two different flags like EAF_NOEASCAPE and
> > > EAF_INDIRECT_NOESCAPE?
> > 
> > Note that EAF_NOESCAPE allows "escaping" to the return value (see
> > handle_rhs_call).  I guess for simplicity we could allow escaping
> 
> I see, missed that! 
> 
> > of not escaped but USED params to other written to params.  I think
> > we also don't handle "escaping" of a parameter indirectly to itself, thus
> > 
> > int i;
> > int *p = &i;
> > int **q = &p;
> > foo (&q);
> > if (*q != i)
> >   abort ();
> > 
> > and
> > 
> > foo (int ***p)
> > {
> >   *p = **p;
> > } 
> > 
> > or so with foos param EAF_NOESCAPE (but not EAF_DIRECT).
> > 
> > Splitting up EAF_NOESCAPE makes it quite difficult to understand.
> > Arguably explicit handling of memcpy and friends _does_ pay off
> > for points-to analysis since I'd say modelling all possible and useful
> > things in fnspec would make it a monster ... you'd basically want
> > to have a way to specify additional constraints in the fnspec itself,
> > like *1 = *2, but then also distinguish points-to effects from
> > may-alias ones.
> 
> Yep, i am not arguing for eliminating special case of memcpy (because we
> have the additional info that it only copies pointers from *src to
> *dest).
> 
> However I find current definition of EAF_NOESCAPE bit hard to handle in
> modref, since naturally it is quite reliable to track all uses of ssa
> name that correspond to parameters, but it is harder to track where
> values read from pointed-to memory can eventually go.

Yeah - also the fnspec of memcpy _is_ wrong at the moment ...

> What I do is to walk all uses of SSA_NAME that correspond to parameter
> and try to unerstand it.  If it is one of
>  1) memory load via derefence of name
>  2) memory store where name is base of LHS
>  3) memory store where name is rhs
>  4) name is passed as a value to function 
>  5) dereferenced value from name is passed to funtion
>  6) used as value normal gimple expression
>  7) used in return (as RHS or base of memory dereference address)
>  8) it is used only in reference but not as base (as array index or so)
> 
> Then I merge in (and) flags I determine as follow:
> 
> For 1) clear EAF_USED and recurse to LHS name. Based on its flags I
> decide on:
>   - EAF_DIRECT (if LHS has EAF_UNUSED),
>   - EAF_NOCLOBBER (if LHS has EAF_NOCLOBBER)
>   - EAF_NOESCAPE (if LHS has EAF_NOESCAPE). 
> 
> For 2) I clear EAF_NOCLOBBER and EAF_UNUSED flag
> 
> For 3) I give up (clear all flags) since I would need to track where the
> memory is going.
> 
> For 4) I determine flag of function parameter
> 
> For 5) I need to do same handling as 1) where flag of "loaded value" is
> flag of the function
> 
> For 6) I determine flags of LHS and merge them in

guess you could track a SSA lattice "based on parameter N" which
would need to be a mask (thus only track up to 32 parameters?)

> For 7) I clear NOESCAPE if rhs is name itself
> and UNUSED + NOESCAPE if rhs is derefernece from name.
> 
> For 8) I do nothing.  Here the names are non-pointers that I track
> because of earlier dereference.
> 
> 
> 
> So I think 7) can be relaxed.  Main problem is hoever that we often see 1)
> and then 3) or 7) on LHS that makes us punt very often.
> 
> The fact that pointer directly does not escape but pointed to memory can
> seems still very useful since one does not need to add *ptr to points-to
> sets. But I will try relaxing 7).
> 
> If we allow values escaping to other parameters and itself, I think I
> can relax 3) if base of the store is default def of PARM_DECL.

I think the important part is to get things correct.  Maybe it's worth
to add write/read flags where the argument _does_ escape in case the
function itself is otherwise pure/const.  For PTA that doesn't make
a difference (and fnspec was all about PTA ...) but for alias-analysis
it does.

> > 
> > I wonder if we should teach the GIMPLE FE to parse 'fn spec'
> > so we can write unit tests for the attribute ... or maybe simply
> > add this to the __GIMPLE spec string.
> 
> May be nice and also describe carefully that NOESCAPE and NOCLOBBER also
> reffers to indirect references.  Current description
> "Nonzero if the argument does not escape."
> reads to me that it is about ptr itself, not about *ptr and also it does
> not speak of the escaping to return value etc.

Well, if 'ptr' escapes then obvoiously all memory reachable from 'ptr'
escapes - escaping is always transitive.

And as escaping is in the context of the caller sth escaping to the
caller itself (via return) can hardly be considered escaping (again
this was designed for PTA ...).

I guess it makes sense to be able to separate escaping from the rest.

Richard.

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend

Detect EAF flags in ipa-modref

[Jan Hubicka]

> > 
> > Yep, i am not arguing for eliminating special case of memcpy (because we
> > have the additional info that it only copies pointers from *src to
> > *dest).
> > 
> > However I find current definition of EAF_NOESCAPE bit hard to handle in
> > modref, since naturally it is quite reliable to track all uses of ssa
> > name that correspond to parameters, but it is harder to track where
> > values read from pointed-to memory can eventually go.
> 
> Yeah - also the fnspec of memcpy _is_ wrong at the moment ...

Yep.
> > 
> > For 6) I determine flags of LHS and merge them in
> 
> guess you could track a SSA lattice "based on parameter N" which
> would need to be a mask (thus only track up to 32 parameters?)

Yes, I can do that if we relax NOESCAPE this way.
> 
> > For 7) I clear NOESCAPE if rhs is name itself
> > and UNUSED + NOESCAPE if rhs is derefernece from name.
> > 
> > For 8) I do nothing.  Here the names are non-pointers that I track
> > because of earlier dereference.
> > 
> > 
> > 
> > So I think 7) can be relaxed.  Main problem is hoever that we often see 1)
> > and then 3) or 7) on LHS that makes us punt very often.
> > 
> > The fact that pointer directly does not escape but pointed to memory can
> > seems still very useful since one does not need to add *ptr to points-to
> > sets. But I will try relaxing 7).
> > 
> > If we allow values escaping to other parameters and itself, I think I
> > can relax 3) if base of the store is default def of PARM_DECL.
> 
> I think the important part is to get things correct.  Maybe it's worth

Indeed :)
> to add write/read flags where the argument _does_ escape in case the
> function itself is otherwise pure/const.  For PTA that doesn't make
> a difference (and fnspec was all about PTA ...) but for alias-analysis
> it does.

I detect them independently (UNUSED/NOCLOBBER flags which is not perfect
since we do not have OUTPUT flag like in fnspecs), but currently they
are unused since we do not track "pure/const except for known
exceptions".  This is not hard to add.
> 
> > > 
> > > I wonder if we should teach the GIMPLE FE to parse 'fn spec'
> > > so we can write unit tests for the attribute ... or maybe simply
> > > add this to the __GIMPLE spec string.
> > 
> > May be nice and also describe carefully that NOESCAPE and NOCLOBBER also
> > reffers to indirect references.  Current description
> > "Nonzero if the argument does not escape."
> > reads to me that it is about ptr itself, not about *ptr and also it does
> > not speak of the escaping to return value etc.
> 
> Well, if 'ptr' escapes then obvoiously all memory reachable from 'ptr'
> escapes - escaping is always transitive.

Yes, but if values pointed to by ptr escapes, ptr itself does not need
to escape.  This is easy to detect (and is common case of THIS pointer)
but we have no way to express it via EAF flags.
> 
> And as escaping is in the context of the caller sth escaping to the
> caller itself (via return) can hardly be considered escaping (again
> this was designed for PTA ...).
> 
> I guess it makes sense to be able to separate escaping from the rest.
I think current definition (escaping via return is not an escape) is
also OK for modref propagation.  We may have EAF_NORETURN that says that
value never escapes to return value that would be also easy to detect.

This is kind of minimal patch for the EAF flags discovery.  It works
only in local ipa-modref and gives up on cyclic SSA graphs.  Adding
propagation is easy and proper IPA mode needs collecting call sites+arg
pairs that affect the answer.

It passes testuite except for sso/t2.c testcase where it affects bit of first
dumped structure R1.
We correctly determine that it is noescape/noclobber from the dump function and
it seems that this triggers kind of strange SRA.  I will look into it.

I am running full bootstrap/regtest.

On tramp3d the effect is not great, but it does something.

PTA query stats:
  pt_solution_includes: 397269 disambiguations, 606922 queries
  pt_solutions_intersect: 138302 disambiguations, 416884 queries

to

PTA query stats:
  pt_solution_includes: 401540 disambiguations, 609093 queries
  pt_solutions_intersect: 138616 disambiguations, 417174 queries

2020-11-09  Jan Hubicka  <hubicka@ucw.cz>

	* builtins.c (builtin_fnspec): Fix fnspecs of memcpy and friends.
	* gimple.c: Include ipa-modref-tree.h and ipa-mdoref.h.
	(gimple_call_arg_flags): Use modref to determine flags.
	* ipa-modref.c: Include gimple-ssa.h, tree-phinodes.h,
	tree-ssa-operands.h, stringpool.h and tree-ssanames.h.
	(modref_summary::useful_p): Summary is also useful if EAF flags are
	known.
	(dump_eaf_flags): New.
	(modref_summary::dump): Dump EAF flags.
	(get_modref_function_summary): Be ready for
	current_function_decl == NULL.
	(memory_access_to): New function.
	(deref_flags): New function.
	(analyze_ssa_name_flags): New function.
	(analyze_parms): New function.
	(analyze_function): New function.
	* ipa-modref.h (struct modref_summary): Add arg_flags.

gcc/testsuite/ChangeLog:

2020-11-09  Jan Hubicka  <hubicka@ucw.cz>

	* gcc.dg/torture/pta-ptrarith-1.c: Make parameter escape.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index da25343beb1..c55f54e2aef 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -12939,16 +12939,16 @@ builtin_fnspec (tree callee)
 	 argument.  */
       case BUILT_IN_STRCAT:
       case BUILT_IN_STRCAT_CHK:
-	return "1cW R ";
+	return "1cW r ";
       case BUILT_IN_STRNCAT:
       case BUILT_IN_STRNCAT_CHK:
-	return "1cW R3";
+	return "1cW r3";
       case BUILT_IN_STRCPY:
       case BUILT_IN_STRCPY_CHK:
-	return "1cO R ";
+	return "1cO r ";
       case BUILT_IN_STPCPY:
       case BUILT_IN_STPCPY_CHK:
-	return ".cO R ";
+	return ".cO r ";
       case BUILT_IN_STRNCPY:
       case BUILT_IN_MEMCPY:
       case BUILT_IN_MEMMOVE:
@@ -12957,15 +12957,15 @@ builtin_fnspec (tree callee)
       case BUILT_IN_STRNCPY_CHK:
       case BUILT_IN_MEMCPY_CHK:
       case BUILT_IN_MEMMOVE_CHK:
-	return "1cO3R3";
+	return "1cO3r3";
       case BUILT_IN_MEMPCPY:
       case BUILT_IN_MEMPCPY_CHK:
-	return ".cO3R3";
+	return ".cO3r3";
       case BUILT_IN_STPNCPY:
       case BUILT_IN_STPNCPY_CHK:
-	return ".cO3R3";
+	return ".cO3r3";
       case BUILT_IN_BCOPY:
-	return ".cR3O3";
+	return ".cr3O3";
       case BUILT_IN_BZERO:
 	return ".cO2";
       case BUILT_IN_MEMCMP:
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 1afed88e1f1..72a0d05580a 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -46,6 +46,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "asan.h"
 #include "langhooks.h"
 #include "attr-fnspec.h"
+#include "ipa-modref-tree.h"
+#include "ipa-modref.h"
 
 
 /* All the tuples have their operand vector (if present) at the very bottom
@@ -1532,24 +1534,45 @@ int
 gimple_call_arg_flags (const gcall *stmt, unsigned arg)
 {
   attr_fnspec fnspec = gimple_call_fnspec (stmt);
-
-  if (!fnspec.known_p ())
-    return 0;
-
   int flags = 0;
 
-  if (!fnspec.arg_specified_p (arg))
-    ;
-  else if (!fnspec.arg_used_p (arg))
-    flags = EAF_UNUSED;
-  else
+  if (fnspec.known_p ())
     {
-      if (fnspec.arg_direct_p (arg))
-	flags |= EAF_DIRECT;
-      if (fnspec.arg_noescape_p (arg))
-	flags |= EAF_NOESCAPE;
-      if (fnspec.arg_readonly_p (arg))
-	flags |= EAF_NOCLOBBER;
+      if (!fnspec.arg_specified_p (arg))
+	;
+      else if (!fnspec.arg_used_p (arg))
+	flags = EAF_UNUSED;
+      else
+	{
+	  if (fnspec.arg_direct_p (arg))
+	    flags |= EAF_DIRECT;
+	  if (fnspec.arg_noescape_p (arg))
+	    flags |= EAF_NOESCAPE;
+	  if (fnspec.arg_readonly_p (arg))
+	    flags |= EAF_NOCLOBBER;
+	}
+    }
+  tree callee = gimple_call_fndecl (stmt);
+  if (callee)
+    {
+      cgraph_node *node = cgraph_node::get (callee);
+      modref_summary *summary = node ? get_modref_function_summary (node)
+				: NULL;
+
+      if (summary && summary->arg_flags.length () > arg)
+	{
+	  int modref_flags = summary->arg_flags [arg];
+
+	  /* We have possibly optimized out load.  Be conservative here.  */
+	  if (!node->binds_to_current_def_p ())
+	    {
+	      if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
+		modref_flags &= ~EAF_UNUSED;
+	      if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
+		modref_flags &= ~EAF_DIRECT;
+	    }
+	  flags |= modref_flags;
+	}
     }
   return flags;
 }
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 3f46bebed3c..6df4c8fbf68 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -61,6 +61,12 @@ along with GCC; see the file COPYING3.  If not see
 #include "ipa-fnsummary.h"
 #include "attr-fnspec.h"
 #include "symtab-clones.h"
+#include "gimple-ssa.h"
+#include "tree-phinodes.h"
+#include "tree-ssa-operands.h"
+#include "ssa-iterators.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
 
 /* We record fnspec specifiers for call edges since they depends on actual
    gimple statements.  */
@@ -186,6 +192,8 @@ modref_summary::useful_p (int ecf_flags)
 {
   if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
     return false;
+  if (arg_flags.length ())
+    return true;
   if (loads && !loads->every_base)
     return true;
   if (ecf_flags & ECF_PURE)
@@ -355,6 +363,22 @@ dump_lto_records (modref_records_lto *tt, FILE *out)
     }
 }
 
+/* Dump EAF flags.  */
+
+static void
+dump_eaf_flags (FILE *out, int flags)
+{
+  if (flags & EAF_DIRECT)
+    fprintf (out, " direct");
+  if (flags & EAF_NOCLOBBER)
+    fprintf (out, " noclobber");
+  if (flags & EAF_NOESCAPE)
+    fprintf (out, " noescape");
+  if (flags & EAF_UNUSED)
+    fprintf (out, " unused");
+  fprintf (out, "\n");
+}
+
 /* Dump summary.  */
 
 void
@@ -372,6 +396,15 @@ modref_summary::dump (FILE *out)
     }
   if (writes_errno)
     fprintf (out, "  Writes errno\n");
+  if (arg_flags.length ())
+    {
+      for (unsigned int i = 0; i < arg_flags.length (); i++)
+	if (arg_flags[i])
+	  {
+	    fprintf (out, "  parm %i flags:", i);
+	    dump_eaf_flags (out, arg_flags[i]);
+	  }
+    }
 }
 
 /* Dump summary.  */
@@ -402,7 +435,8 @@ get_modref_function_summary (cgraph_node *func)
      function.  */
   enum availability avail;
   func = func->function_or_virtual_thunk_symbol
-	     (&avail, cgraph_node::get (current_function_decl));
+		 (&avail, current_function_decl ?
+			  cgraph_node::get (current_function_decl) : NULL);
   if (avail <= AVAIL_INTERPOSABLE)
     return NULL;
 
@@ -1067,6 +1101,266 @@ remove_summary (bool lto, bool nolto, bool ipa)
 	     " - modref done with result: not tracked.\n");
 }
 
+/* Return true if OP accesses memory pointed to by SSA_NAME.  */
+
+bool
+memory_access_to (tree op, tree ssa_name)
+{
+  tree base = get_base_address (op);
+  if (!base)
+    return false;
+  if (TREE_CODE (base) != MEM_REF && TREE_CODE (base) != TARGET_MEM_REF)
+    return false;
+  return TREE_OPERAND (base, 0) == ssa_name;
+}
+
+/* We have val = *arg. 
+   return EAF flags of ARG that can be determined from EAF flags of VAL
+   (which are known to be FLAGS).  If IGNORE_STORES is true we can ignore
+   all stores to VAL, i.e. when handling noreturn function.  */
+
+static int
+deref_flags (int flags, bool ignore_stores)
+{
+  int ret = 0;
+  if (flags & EAF_UNUSED)
+    ret |= EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
+  else
+    {
+      if ((flags & EAF_NOCLOBBER) || ignore_stores)
+	ret |= EAF_NOCLOBBER;
+      if ((flags & EAF_NOESCAPE) || ignore_stores)
+	ret |= EAF_NOESCAPE;
+    }
+  return ret;
+}
+
+/* Analyze EAF flags for SSA name NAME.
+   KNOWN_FLAGS is a cache for flags we already determined.  */
+
+static int
+analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth)
+{
+  imm_use_iterator ui;
+  gimple *use_stmt;
+  int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
+
+  /* See if value is already computed.  */
+  if ((*known_flags)[SSA_NAME_VERSION (name)])
+    {
+      /* Punt on cycles for now, so we do not need dataflow.  */
+      if ((*known_flags)[SSA_NAME_VERSION (name)] == 1)
+	{
+	  if (dump_file)
+	    fprintf (dump_file,
+		     "%*sGiving up on a cycle in SSA graph\n", depth * 4, "");
+	  return 0;
+	}
+      return (*known_flags)[SSA_NAME_VERSION (name)] - 2;
+    }
+  /* Recursion guard.  */
+  (*known_flags)[SSA_NAME_VERSION (name)] = 1;
+
+  if (dump_file)
+    {
+      fprintf (dump_file,
+	       "%*sAnalyzing flags of ssa name: ", depth * 4, "");
+      print_generic_expr (dump_file, name);
+      fprintf (dump_file, "\n");
+    }
+
+  FOR_EACH_IMM_USE_STMT (use_stmt, ui, name)
+    {
+      if (flags == 0)
+	{
+	  BREAK_FROM_IMM_USE_STMT (ui);
+	}
+      if (dump_file)
+	{
+	  fprintf (dump_file, "%*s  Analyzing stmt:", depth * 4, "");
+	  print_gimple_stmt (dump_file, use_stmt, 0);
+	}
+
+      /* Gimple return may load the return value.  */
+      if (gimple_code (use_stmt) == GIMPLE_RETURN)
+	{
+	  if (memory_access_to (gimple_return_retval
+				   (as_a <greturn *> (use_stmt)), name))
+	    flags &= ~EAF_UNUSED;
+	}
+      /* Account for LHS store, arg loads and flags from callee function.  */
+      else if (is_gimple_call (use_stmt))
+	{
+	  tree callee = gimple_call_fndecl (use_stmt);
+
+	  /* Recursion would require bit of propagation; give up for now.  */
+	  if (callee && recursive_call_p (current_function_decl, callee))
+	    flags = 0;
+	  else
+	    {
+	      int ecf_flags = gimple_call_flags (use_stmt);
+	      bool ignore_stores = ignore_stores_p (current_function_decl,
+						    ecf_flags);
+
+	      /* Handle *name = func (...).  */
+	      if (gimple_call_lhs (use_stmt)
+		  && memory_access_to (gimple_call_lhs (use_stmt), name))
+		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+
+	      /* Handle all function parameters.  */
+	      for (unsigned i = 0; i < gimple_call_num_args (use_stmt); i++)
+		/* Name is directly passed to the callee.  */
+		if (gimple_call_arg (use_stmt, i) == name
+		    && !(ecf_flags & (ECF_CONST | ECF_NOVOPS)))
+		  {
+		    int call_flags = gimple_call_arg_flags (as_a <gcall *>
+								 (use_stmt), i);
+		    if (ignore_stores)
+		      call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE;
+
+		    flags &= call_flags;
+		  }
+		/* Name is dereferenced and passed to a callee.  */
+		else if (memory_access_to (gimple_call_arg (use_stmt, i), name))
+		  {
+		    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
+		      flags &= ~EAF_UNUSED;
+		    else
+		      flags &= deref_flags (gimple_call_arg_flags
+						(as_a <gcall *> (use_stmt), i),
+					    ignore_stores);
+		  }
+	    }
+	}
+      else if (gimple_assign_load_p (use_stmt))
+	{
+	  /* Memory to memory copy.  */
+	  if (gimple_store_p (use_stmt))
+	    {
+	      /* Handle *name = *exp.  */
+	      if (memory_access_to (gimple_assign_lhs (use_stmt), name))
+		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+
+	      /* Handle *lhs = *name.
+
+		 We do not track memory locations, so assume that value
+		 is used arbitrarily.  */
+	      if (memory_access_to (gimple_assign_rhs1 (use_stmt), name))
+		flags = 0;
+	    }
+	  /* Handle lhs = *name.  */
+	  else if (memory_access_to (gimple_assign_rhs1 (use_stmt), name))
+	    flags &= deref_flags (analyze_ssa_name_flags
+				      (gimple_assign_lhs (use_stmt),
+				       known_flags, depth + 1), false);
+	}
+      else if (gimple_store_p (use_stmt))
+	{
+	  /* Handle *lhs = name.  */
+	  if (is_gimple_assign (use_stmt)
+	      && gimple_assign_rhs1 (use_stmt) == name)
+	    {
+	      if (dump_file)
+		fprintf (dump_file, "%*s  ssa name saved to memory\n",
+			 depth * 4, "");
+	      flags = 0;
+	    }
+	  /* Handle *name = exp.  */
+	  else if (is_gimple_assign (use_stmt)
+		   && memory_access_to (gimple_assign_lhs (use_stmt), name))
+	    flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+	  /* ASM statements etc.  */
+	  else if (!is_gimple_assign (use_stmt))
+	    {
+	      if (dump_file)
+		fprintf (dump_file, "%*s  Unhandled store\n",
+			 depth * 4, "");
+	      flags = 0;
+	    }
+	}
+      else if (is_gimple_assign (use_stmt))
+	{
+	  enum tree_code code = gimple_assign_rhs_code (use_stmt);
+
+	  /* See if operation is a merge as considered by
+	     tree-ssa-structalias.c:find_func_aliases.  */
+	  if (!truth_value_p (code)
+	      && code != POINTER_DIFF_EXPR
+	      && (code != POINTER_PLUS_EXPR
+		  || gimple_assign_rhs1 (use_stmt) == name))
+	    flags &= analyze_ssa_name_flags
+			       (gimple_assign_lhs (use_stmt), known_flags,
+				depth + 1);
+	}
+      else if (gimple_code (use_stmt) == GIMPLE_PHI)
+	{
+	  flags &= analyze_ssa_name_flags
+			     (gimple_phi_result (use_stmt), known_flags,
+			      depth + 1);
+	}
+      /* Conditions are not considered escape points
+	 by tree-ssa-structalias.  */
+      else if (gimple_code (use_stmt) == GIMPLE_COND)
+	;
+      else
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "%*s  Unhandled stmt\n", depth * 4, "");
+	  flags = 0;
+	}
+
+      if (dump_file)
+	{
+	  fprintf (dump_file, "%*s  current flags of ", depth * 4, "");
+	  print_generic_expr (dump_file, name);
+	  dump_eaf_flags (dump_file, flags);
+	}
+    }
+  if (dump_file)
+    {
+      fprintf (dump_file, "%*sflags of ssa name ", depth * 4, "");
+      print_generic_expr (dump_file, name);
+      dump_eaf_flags (dump_file, flags);
+    }
+  (*known_flags)[SSA_NAME_VERSION (name)] = flags + 2;
+  return flags;
+}
+
+/* Determine EAF flags for function parameters.  */
+
+static void
+analyze_parms (modref_summary *summary)
+{
+  unsigned int parm_index = 0;
+  unsigned int count = 0;
+
+  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
+       parm = TREE_CHAIN (parm))
+    count++;
+
+  if (!count)
+    return;
+
+  auto_vec<int> known_flags;
+  known_flags.safe_grow_cleared (num_ssa_names);
+
+  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++,
+       parm = TREE_CHAIN (parm))
+    {
+      tree name = ssa_default_def (cfun, parm);
+      if (!name)
+	continue;
+      int flags = analyze_ssa_name_flags (name, &known_flags, 0);
+
+      if (flags)
+	{
+	  if (parm_index >= summary->arg_flags.length ())
+	    summary->arg_flags.safe_grow_cleared (count);
+	  summary->arg_flags [parm_index] = flags;
+	}
+    }
+}
+
 /* Analyze function F.  IPA indicates whether we're running in local mode
    (false) or the IPA mode (true).  */
 
@@ -1174,6 +1468,10 @@ analyze_function (function *f, bool ipa)
 				  param_modref_max_accesses);
       summary_lto->writes_errno = false;
     }
+
+  if (!ipa)
+    analyze_parms (summary);
+
   int ecf_flags = flags_from_decl_or_type (current_function_decl);
   auto_vec <gimple *, 32> recursive_calls;
 
@@ -1191,8 +1489,9 @@ analyze_function (function *f, bool ipa)
 	      || ((!summary || !summary->useful_p (ecf_flags))
 		  && (!summary_lto || !summary_lto->useful_p (ecf_flags))))
 	    {
-	      remove_summary (lto, nolto, ipa);
-	      return;
+	      collapse_loads (summary, summary_lto);
+	      collapse_stores (summary, summary_lto);
+	      break;
 	    }
 	}
     }
diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
index 31ceffa8d34..8fa05aaf7fb 100644
--- a/gcc/ipa-modref.h
+++ b/gcc/ipa-modref.h
@@ -29,6 +29,7 @@ struct GTY(()) modref_summary
   /* Load and stores in function (transitively closed to all callees)  */
   modref_records *loads;
   modref_records *stores;
+  auto_vec<int> GTY((skip)) arg_flags;
 
   modref_summary ();
   ~modref_summary ();
diff --git a/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c b/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c
index 99a548840df..85b68068b12 100644
--- a/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c
+++ b/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c
@@ -6,11 +6,14 @@ struct Foo {
   int *p;
 };
 
+struct Foo *ff;
+
 void __attribute__((noinline))
 foo (void *p)
 {
   struct Foo *f = (struct Foo *)p - 1;
   *f->p = 0;
+  ff = f;
 }
 
 int bar (void)

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

Hi,
this is updated patch for autodetection of EAF flags.  Still the goal is
to avoid fancy stuff and get besic logic in place (so no dataflow, no IPA
propagation, no attempts to handle trickier cases).  There is one new failure

./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c   -Wno-scalar-storage-order -O1 -fno-inline  output pattern test
./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c   -Wno-scalar-storage-order -O2  output pattern test
./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c   -Wno-scalar-storage-order -Os  output pattern test

Which I blieve is bug exposed by detecting dump function to be EAF_DIRECT and
NOESCAPE (which it is) and packing/updacking the bitfields leads in one bit
difference. Still no idea why.

Patch seems to be quite effective on cc1plus turning:

Alias oracle query stats:
  refs_may_alias_p: 65808750 disambiguations, 75664890 queries
  ref_maybe_used_by_call_p: 153485 disambiguations, 66711204 queries
  call_may_clobber_ref_p: 22816 disambiguations, 28889 queries
  nonoverlapping_component_refs_p: 0 disambiguations, 36846 queries
  nonoverlapping_refs_since_match_p: 27271 disambiguations, 58917 must overlaps, 86958 queries
  aliasing_component_refs_p: 65808 disambiguations, 2067256 queries
  TBAA oracle: 25929211 disambiguations 60395141 queries
               12391384 are in alias set 0
               10783783 queries asked about the same object
               126 queries asked about the same alias set
               0 access volatile
               9598698 are dependent in the DAG
               1691939 are aritificially in conflict with void *

Modref stats:
  modref use: 14284 disambiguations, 53336 queries
  modref clobber: 1660281 disambiguations, 2130440 queries
  4311165 tbaa queries (2.023603 per modref query)
  685304 base compares (0.321673 per modref query)

PTA query stats:
  pt_solution_includes: 959190 disambiguations, 13169678 queries
  pt_solutions_intersect: 1050969 disambiguations, 13246686 queries

Alias oracle query stats:
  refs_may_alias_p: 66914578 disambiguations, 76692648 queries
  ref_maybe_used_by_call_p: 244077 disambiguations, 67732086 queries
  call_may_clobber_ref_p: 111475 disambiguations, 116613 queries
  nonoverlapping_component_refs_p: 0 disambiguations, 37091 queries
  nonoverlapping_refs_since_match_p: 27267 disambiguations, 59019 must overlaps, 87056 queries
  aliasing_component_refs_p: 65870 disambiguations, 2063394 queries
  TBAA oracle: 26024415 disambiguations 60579490 queries
               12450910 are in alias set 0
               10806673 queries asked about the same object
               125 queries asked about the same alias set
               0 access volatile
               9605837 are dependent in the DAG
               1691530 are aritificially in conflict with void *

Modref stats:
  modref use: 14272 disambiguations, 277680 queries
  modref clobber: 1669753 disambiguations, 7849135 queries
  4330162 tbaa queries (0.551674 per modref query)
  699241 base compares (0.089085 per modref query)

PTA query stats:
  pt_solution_includes: 1833920 disambiguations, 13846032 queries
  pt_solutions_intersect: 1093785 disambiguations, 13309954 queries

So almost twice as many pt_solution_includes disambiguations.
I will re-run the stats overnight to be sure that it is not independent
change (but both build was from almost same checkout).

Bootstrapped/regtested x86_64-linux, OK?
(I will analyze more the t2.c failure)

Honza

gcc/ChangeLog:

2020-11-10  Jan Hubicka  <hubicka@ucw.cz>

	* gimple.c: Include ipa-modref-tree.h and ipa-modref.h.
	(gimple_call_arg_flags): Use modref to determine flags.
	* ipa-modref.c: Include gimple-ssa.h, tree-phinodes.h,
	tree-ssa-operands.h, stringpool.h and tree-ssanames.h.
	(analyze_ssa_name_flags): Declare.
	(modref_summary::useful_p): Summary is also useful if arg flags are
	known.
	(dump_eaf_flags): New function.
	(modref_summary::dump): Use it.
	(get_modref_function_summary): Be read for current_function_decl
	being NULL.
	(memory_access_to): New function.
	(deref_flags): New function.
	(call_lhs_flags): New function.
	(analyze_parms): New function.
	(analyze_function): Use it.
	* ipa-modref.h (struct modref_summary): Add arg_flags.

gcc/testsuite/ChangeLog:

2020-11-10  Jan Hubicka  <hubicka@ucw.cz>

	* gcc.dg/torture/pta-ptrarith-1.c: Escape parametrs.

diff --git a/gcc/gimple.c b/gcc/gimple.c
index 1afed88e1f1..da90716aa23 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -46,6 +46,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "asan.h"
 #include "langhooks.h"
 #include "attr-fnspec.h"
+#include "ipa-modref-tree.h"
+#include "ipa-modref.h"
 
 
 /* All the tuples have their operand vector (if present) at the very bottom
@@ -1532,24 +1534,45 @@ int
 gimple_call_arg_flags (const gcall *stmt, unsigned arg)
 {
   attr_fnspec fnspec = gimple_call_fnspec (stmt);
-
-  if (!fnspec.known_p ())
-    return 0;
-
   int flags = 0;
 
-  if (!fnspec.arg_specified_p (arg))
-    ;
-  else if (!fnspec.arg_used_p (arg))
-    flags = EAF_UNUSED;
-  else
+  if (fnspec.known_p ())
     {
-      if (fnspec.arg_direct_p (arg))
-	flags |= EAF_DIRECT;
-      if (fnspec.arg_noescape_p (arg))
-	flags |= EAF_NOESCAPE;
-      if (fnspec.arg_readonly_p (arg))
-	flags |= EAF_NOCLOBBER;
+      if (!fnspec.arg_specified_p (arg))
+	;
+      else if (!fnspec.arg_used_p (arg))
+	flags = EAF_UNUSED;
+      else
+	{
+	  if (fnspec.arg_direct_p (arg))
+	    flags |= EAF_DIRECT;
+	  if (fnspec.arg_noescape_p (arg))
+	    flags |= EAF_NOESCAPE;
+	  if (fnspec.arg_readonly_p (arg))
+	    flags |= EAF_NOCLOBBER;
+	}
+    }
+  tree callee = gimple_call_fndecl (stmt);
+  if (callee)
+    {
+      cgraph_node *node = cgraph_node::get (callee);
+      modref_summary *summary = node ? get_modref_function_summary (node)
+				: NULL;
+
+      if (summary && summary->arg_flags.length () > arg)
+	{
+	  int modref_flags = summary->arg_flags[arg];
+
+	  /* We have possibly optimized out load.  Be conservative here.  */
+	  if (!node->binds_to_current_def_p ())
+	    {
+	      if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
+		modref_flags &= ~EAF_UNUSED;
+	      if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
+		modref_flags &= ~EAF_DIRECT;
+	    }
+	  flags |= modref_flags;
+	}
     }
   return flags;
 }
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 3f46bebed3c..bde626115ff 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -61,6 +61,14 @@ along with GCC; see the file COPYING3.  If not see
 #include "ipa-fnsummary.h"
 #include "attr-fnspec.h"
 #include "symtab-clones.h"
+#include "gimple-ssa.h"
+#include "tree-phinodes.h"
+#include "tree-ssa-operands.h"
+#include "ssa-iterators.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
+
+static int analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth);
 
 /* We record fnspec specifiers for call edges since they depends on actual
    gimple statements.  */
@@ -186,6 +194,8 @@ modref_summary::useful_p (int ecf_flags)
 {
   if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
     return false;
+  if (arg_flags.length ())
+    return true;
   if (loads && !loads->every_base)
     return true;
   if (ecf_flags & ECF_PURE)
@@ -355,6 +365,22 @@ dump_lto_records (modref_records_lto *tt, FILE *out)
     }
 }
 
+/* Dump EAF flags.  */
+
+static void
+dump_eaf_flags (FILE *out, int flags)
+{
+  if (flags & EAF_DIRECT)
+    fprintf (out, " direct");
+  if (flags & EAF_NOCLOBBER)
+    fprintf (out, " noclobber");
+  if (flags & EAF_NOESCAPE)
+    fprintf (out, " noescape");
+  if (flags & EAF_UNUSED)
+    fprintf (out, " unused");
+  fprintf (out, "\n");
+}
+
 /* Dump summary.  */
 
 void
@@ -372,6 +398,15 @@ modref_summary::dump (FILE *out)
     }
   if (writes_errno)
     fprintf (out, "  Writes errno\n");
+  if (arg_flags.length ())
+    {
+      for (unsigned int i = 0; i < arg_flags.length (); i++)
+	if (arg_flags[i])
+	  {
+	    fprintf (out, "  parm %i flags:", i);
+	    dump_eaf_flags (out, arg_flags[i]);
+	  }
+    }
 }
 
 /* Dump summary.  */
@@ -402,7 +437,8 @@ get_modref_function_summary (cgraph_node *func)
      function.  */
   enum availability avail;
   func = func->function_or_virtual_thunk_symbol
-	     (&avail, cgraph_node::get (current_function_decl));
+		 (&avail, current_function_decl ?
+			  cgraph_node::get (current_function_decl) : NULL);
   if (avail <= AVAIL_INTERPOSABLE)
     return NULL;
 
@@ -1067,6 +1103,315 @@ remove_summary (bool lto, bool nolto, bool ipa)
 	     " - modref done with result: not tracked.\n");
 }
 
+/* Return true if OP accesses memory pointed to by SSA_NAME.  */
+
+bool
+memory_access_to (tree op, tree ssa_name)
+{
+  tree base = get_base_address (op);
+  if (!base)
+    return false;
+  if (TREE_CODE (base) != MEM_REF && TREE_CODE (base) != TARGET_MEM_REF)
+    return false;
+  return TREE_OPERAND (base, 0) == ssa_name;
+}
+
+/* Consider statement val = *arg.
+   return EAF flags of ARG that can be determined from EAF flags of VAL
+   (which are known to be FLAGS).  If IGNORE_STORES is true we can ignore
+   all stores to VAL, i.e. when handling noreturn function.  */
+
+static int
+deref_flags (int flags, bool ignore_stores)
+{
+  int ret = 0;
+  if (flags & EAF_UNUSED)
+    ret |= EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
+  else
+    {
+      if ((flags & EAF_NOCLOBBER) || ignore_stores)
+	ret |= EAF_NOCLOBBER;
+      if ((flags & EAF_NOESCAPE) || ignore_stores)
+	ret |= EAF_NOESCAPE;
+    }
+  return ret;
+}
+
+/* Call statements may return their parameters.  Consider argument number
+   ARG of USE_STMT and determine flags that can needs to be cleared
+   in case pointer possibly indirectly references from ARG I is returned.
+   KNOWN_FLAGS and DEPTH are same as in analyze_ssa_name_flags.  */
+
+static int
+call_lhs_flags (gimple *use_stmt, int arg, vec<int> *known_flags, int depth)
+{
+  /* If there is no return value, no flags are affected.  */
+  if (!gimple_call_lhs (use_stmt))
+    return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
+
+  /* If we know that function returns given argument and it is not ARG
+     we can still be happy.  */
+  int flags = gimple_call_return_flags (as_a <gcall *> (use_stmt));
+  if ((flags & ERF_RETURNS_ARG)
+      && (flags & ERF_RETURN_ARG_MASK) != arg)
+    return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
+
+  /* If return value is SSA name determine its flags.  */
+  if (TREE_CODE (gimple_call_lhs (use_stmt)) == SSA_NAME)
+    return analyze_ssa_name_flags
+		       (gimple_call_lhs (use_stmt), known_flags,
+			depth + 1);
+  /* In the case of memory store we can do nothing.  */
+  else
+    return 0;
+}
+
+/* Analyze EAF flags for SSA name NAME.
+   KNOWN_FLAGS is a cache for flags we already determined.
+   DEPTH is a recursion depth used to make debug output prettier.  */
+
+static int
+analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth)
+{
+  imm_use_iterator ui;
+  gimple *use_stmt;
+  int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
+
+  /* See if value is already computed.  */
+  if ((*known_flags)[SSA_NAME_VERSION (name)])
+    {
+      /* Punt on cycles for now, so we do not need dataflow.  */
+      if ((*known_flags)[SSA_NAME_VERSION (name)] == 1)
+	{
+	  if (dump_file)
+	    fprintf (dump_file,
+		     "%*sGiving up on a cycle in SSA graph\n", depth * 4, "");
+	  return 0;
+	}
+      return (*known_flags)[SSA_NAME_VERSION (name)] - 2;
+    }
+  /* Recursion guard.  */
+  (*known_flags)[SSA_NAME_VERSION (name)] = 1;
+
+  if (dump_file)
+    {
+      fprintf (dump_file,
+	       "%*sAnalyzing flags of ssa name: ", depth * 4, "");
+      print_generic_expr (dump_file, name);
+      fprintf (dump_file, "\n");
+    }
+
+  FOR_EACH_IMM_USE_STMT (use_stmt, ui, name)
+    {
+      if (flags == 0)
+	{
+	  BREAK_FROM_IMM_USE_STMT (ui);
+	}
+      if (is_gimple_debug (use_stmt))
+	continue;
+      if (dump_file)
+	{
+	  fprintf (dump_file, "%*s  Analyzing stmt:", depth * 4, "");
+	  print_gimple_stmt (dump_file, use_stmt, 0);
+	}
+
+      /* Gimple return may load the return value.  */
+      if (gimple_code (use_stmt) == GIMPLE_RETURN)
+	{
+	  if (memory_access_to (gimple_return_retval
+				   (as_a <greturn *> (use_stmt)), name))
+	    flags &= ~EAF_UNUSED;
+	}
+      /* Account for LHS store, arg loads and flags from callee function.  */
+      else if (is_gimple_call (use_stmt))
+	{
+	  tree callee = gimple_call_fndecl (use_stmt);
+
+	  /* Recursion would require bit of propagation; give up for now.  */
+	  if (callee && recursive_call_p (current_function_decl, callee))
+	    flags = 0;
+	  else
+	    {
+	      int ecf_flags = gimple_call_flags (use_stmt);
+	      bool ignore_stores = ignore_stores_p (current_function_decl,
+						    ecf_flags);
+
+	      /* Handle *name = func (...).  */
+	      if (gimple_call_lhs (use_stmt)
+		  && memory_access_to (gimple_call_lhs (use_stmt), name))
+		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+
+	      /* Handle all function parameters.  */
+	      for (unsigned i = 0; i < gimple_call_num_args (use_stmt); i++)
+		/* Name is directly passed to the callee.  */
+		if (gimple_call_arg (use_stmt, i) == name)
+		  {
+		    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
+		      flags &= ignore_stores
+			       ? 0
+			       : call_lhs_flags (use_stmt, i,
+						 known_flags, depth);
+		    else
+		      {
+			int call_flags = gimple_call_arg_flags (as_a <gcall *>
+								 (use_stmt), i);
+			if (ignore_stores)
+			  call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE;
+			else
+			  call_flags &= call_lhs_flags (use_stmt, i,
+							known_flags, depth);
+
+			flags &= call_flags;
+		      }
+		  }
+		/* Name is dereferenced and passed to a callee.  */
+		else if (memory_access_to (gimple_call_arg (use_stmt, i), name))
+		  {
+		    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
+		      flags &= ~EAF_UNUSED;
+		    else
+		      flags &= deref_flags (gimple_call_arg_flags
+						(as_a <gcall *> (use_stmt), i),
+					    ignore_stores);
+		    if (!ignore_stores)
+		      flags &= call_lhs_flags (use_stmt, i, known_flags,
+					       depth);
+		  }
+	    }
+	  /* Only NOCLOBBER or DIRECT flags alone are not useful (see comments
+	     in tree-ssa-alias.c).  Give up earlier.  */
+	  if ((flags & ~(EAF_DIRECT | EAF_NOCLOBBER)) == 0)
+	    flags = 0;
+	}
+      else if (gimple_assign_load_p (use_stmt))
+	{
+	  /* Memory to memory copy.  */
+	  if (gimple_store_p (use_stmt))
+	    {
+	      /* Handle *name = *exp.  */
+	      if (memory_access_to (gimple_assign_lhs (use_stmt), name))
+		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+
+	      /* Handle *lhs = *name.
+
+		 We do not track memory locations, so assume that value
+		 is used arbitrarily.  */
+	      if (memory_access_to (gimple_assign_rhs1 (use_stmt), name))
+		flags = 0;
+	    }
+	  /* Handle lhs = *name.  */
+	  else if (memory_access_to (gimple_assign_rhs1 (use_stmt), name))
+	    flags &= deref_flags (analyze_ssa_name_flags
+				      (gimple_assign_lhs (use_stmt),
+				       known_flags, depth + 1), false);
+	}
+      else if (gimple_store_p (use_stmt))
+	{
+	  /* Handle *lhs = name.  */
+	  if (is_gimple_assign (use_stmt)
+	      && gimple_assign_rhs1 (use_stmt) == name)
+	    {
+	      if (dump_file)
+		fprintf (dump_file, "%*s  ssa name saved to memory\n",
+			 depth * 4, "");
+	      flags = 0;
+	    }
+	  /* Handle *name = exp.  */
+	  else if (is_gimple_assign (use_stmt)
+		   && memory_access_to (gimple_assign_lhs (use_stmt), name))
+	    flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+	  /* ASM statements etc.  */
+	  else if (!is_gimple_assign (use_stmt))
+	    {
+	      if (dump_file)
+		fprintf (dump_file, "%*s  Unhandled store\n",
+			 depth * 4, "");
+	      flags = 0;
+	    }
+	}
+      else if (is_gimple_assign (use_stmt))
+	{
+	  enum tree_code code = gimple_assign_rhs_code (use_stmt);
+
+	  /* See if operation is a merge as considered by
+	     tree-ssa-structalias.c:find_func_aliases.  */
+	  if (!truth_value_p (code)
+	      && code != POINTER_DIFF_EXPR
+	      && (code != POINTER_PLUS_EXPR
+		  || gimple_assign_rhs1 (use_stmt) == name))
+	    flags &= analyze_ssa_name_flags
+			       (gimple_assign_lhs (use_stmt), known_flags,
+				depth + 1);
+	}
+      else if (gimple_code (use_stmt) == GIMPLE_PHI)
+	{
+	  flags &= analyze_ssa_name_flags
+			     (gimple_phi_result (use_stmt), known_flags,
+			      depth + 1);
+	}
+      /* Conditions are not considered escape points
+	 by tree-ssa-structalias.  */
+      else if (gimple_code (use_stmt) == GIMPLE_COND)
+	;
+      else
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "%*s  Unhandled stmt\n", depth * 4, "");
+	  flags = 0;
+	}
+
+      if (dump_file)
+	{
+	  fprintf (dump_file, "%*s  current flags of ", depth * 4, "");
+	  print_generic_expr (dump_file, name);
+	  dump_eaf_flags (dump_file, flags);
+	}
+    }
+  if (dump_file)
+    {
+      fprintf (dump_file, "%*sflags of ssa name ", depth * 4, "");
+      print_generic_expr (dump_file, name);
+      dump_eaf_flags (dump_file, flags);
+    }
+  (*known_flags)[SSA_NAME_VERSION (name)] = flags + 2;
+  return flags;
+}
+
+/* Determine EAF flags for function parameters.  */
+
+static void
+analyze_parms (modref_summary *summary)
+{
+  unsigned int parm_index = 0;
+  unsigned int count = 0;
+
+  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
+       parm = TREE_CHAIN (parm))
+    count++;
+
+  if (!count)
+    return;
+
+  auto_vec<int> known_flags;
+  known_flags.safe_grow_cleared (num_ssa_names);
+
+  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++,
+       parm = TREE_CHAIN (parm))
+    {
+      tree name = ssa_default_def (cfun, parm);
+      if (!name)
+	continue;
+      int flags = analyze_ssa_name_flags (name, &known_flags, 0);
+
+      if (flags)
+	{
+	  if (parm_index >= summary->arg_flags.length ())
+	    summary->arg_flags.safe_grow_cleared (count);
+	  summary->arg_flags[parm_index] = flags;
+	}
+    }
+}
+
 /* Analyze function F.  IPA indicates whether we're running in local mode
    (false) or the IPA mode (true).  */
 
@@ -1174,6 +1519,10 @@ analyze_function (function *f, bool ipa)
 				  param_modref_max_accesses);
       summary_lto->writes_errno = false;
     }
+
+  if (!ipa)
+    analyze_parms (summary);
+
   int ecf_flags = flags_from_decl_or_type (current_function_decl);
   auto_vec <gimple *, 32> recursive_calls;
 
@@ -1191,8 +1540,9 @@ analyze_function (function *f, bool ipa)
 	      || ((!summary || !summary->useful_p (ecf_flags))
 		  && (!summary_lto || !summary_lto->useful_p (ecf_flags))))
 	    {
-	      remove_summary (lto, nolto, ipa);
-	      return;
+	      collapse_loads (summary, summary_lto);
+	      collapse_stores (summary, summary_lto);
+	      break;
 	    }
 	}
     }
diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
index 31ceffa8d34..8fa05aaf7fb 100644
--- a/gcc/ipa-modref.h
+++ b/gcc/ipa-modref.h
@@ -29,6 +29,7 @@ struct GTY(()) modref_summary
   /* Load and stores in function (transitively closed to all callees)  */
   modref_records *loads;
   modref_records *stores;
+  auto_vec<int> GTY((skip)) arg_flags;
 
   modref_summary ();
   ~modref_summary ();
diff --git a/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c b/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c
index 99a548840df..85b68068b12 100644
--- a/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c
+++ b/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c
@@ -6,11 +6,14 @@ struct Foo {
   int *p;
 };
 
+struct Foo *ff;
+
 void __attribute__((noinline))
 foo (void *p)
 {
   struct Foo *f = (struct Foo *)p - 1;
   *f->p = 0;
+  ff = f;
 }
 
 int bar (void)

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

> 
> Alias oracle query stats:
>   refs_may_alias_p: 65808750 disambiguations, 75664890 queries
>   ref_maybe_used_by_call_p: 153485 disambiguations, 66711204 queries
>   call_may_clobber_ref_p: 22816 disambiguations, 28889 queries
>   nonoverlapping_component_refs_p: 0 disambiguations, 36846 queries
>   nonoverlapping_refs_since_match_p: 27271 disambiguations, 58917 must overlaps, 86958 queries
>   aliasing_component_refs_p: 65808 disambiguations, 2067256 queries
>   TBAA oracle: 25929211 disambiguations 60395141 queries
>                12391384 are in alias set 0
>                10783783 queries asked about the same object
>                126 queries asked about the same alias set
>                0 access volatile
>                9598698 are dependent in the DAG
>                1691939 are aritificially in conflict with void *
> 
> Modref stats:
>   modref use: 14284 disambiguations, 53336 queries
>   modref clobber: 1660281 disambiguations, 2130440 queries
>   4311165 tbaa queries (2.023603 per modref query)
>   685304 base compares (0.321673 per modref query)
> 
> PTA query stats:
>   pt_solution_includes: 959190 disambiguations, 13169678 queries
>   pt_solutions_intersect: 1050969 disambiguations, 13246686 queries

I re-run the mainline withtout EAF flag propagation and the results
are correct, so it is not due to independent change pushed between my
tests.

Honza

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

> Bootstrapped/regtested x86_64-linux, OK?
> (I will analyze more the t2.c failure)

I have found independent reproducer that is now in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97775

Honza

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

> > Bootstrapped/regtested x86_64-linux, OK?
> > (I will analyze more the t2.c failure)
> 
> I have found independent reproducer that is now in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97775
... and with Jakub's fix the testcase works now.
Honza
> 
> Honza

Re: Detect EAF flags in ipa-modref

[Richard Biener]

On Tue, 10 Nov 2020, Jan Hubicka wrote:

> Hi,
> this is updated patch for autodetection of EAF flags.  Still the goal is
> to avoid fancy stuff and get besic logic in place (so no dataflow, no IPA
> propagation, no attempts to handle trickier cases).  There is one new failure
> 
> ./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c   -Wno-scalar-storage-order -O1 -fno-inline  output pattern test
> ./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c   -Wno-scalar-storage-order -O2  output pattern test
> ./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c   -Wno-scalar-storage-order -Os  output pattern test
> 
> Which I blieve is bug exposed by detecting dump function to be EAF_DIRECT and
> NOESCAPE (which it is) and packing/updacking the bitfields leads in one bit
> difference. Still no idea why.
> 
> Patch seems to be quite effective on cc1plus turning:
> 
> Alias oracle query stats:
>   refs_may_alias_p: 65808750 disambiguations, 75664890 queries
>   ref_maybe_used_by_call_p: 153485 disambiguations, 66711204 queries
>   call_may_clobber_ref_p: 22816 disambiguations, 28889 queries
>   nonoverlapping_component_refs_p: 0 disambiguations, 36846 queries
>   nonoverlapping_refs_since_match_p: 27271 disambiguations, 58917 must overlaps, 86958 queries
>   aliasing_component_refs_p: 65808 disambiguations, 2067256 queries
>   TBAA oracle: 25929211 disambiguations 60395141 queries
>                12391384 are in alias set 0
>                10783783 queries asked about the same object
>                126 queries asked about the same alias set
>                0 access volatile
>                9598698 are dependent in the DAG
>                1691939 are aritificially in conflict with void *
> 
> Modref stats:
>   modref use: 14284 disambiguations, 53336 queries
>   modref clobber: 1660281 disambiguations, 2130440 queries
>   4311165 tbaa queries (2.023603 per modref query)
>   685304 base compares (0.321673 per modref query)
> 
> PTA query stats:
>   pt_solution_includes: 959190 disambiguations, 13169678 queries
>   pt_solutions_intersect: 1050969 disambiguations, 13246686 queries
> 
> Alias oracle query stats:
>   refs_may_alias_p: 66914578 disambiguations, 76692648 queries
>   ref_maybe_used_by_call_p: 244077 disambiguations, 67732086 queries
>   call_may_clobber_ref_p: 111475 disambiguations, 116613 queries
>   nonoverlapping_component_refs_p: 0 disambiguations, 37091 queries
>   nonoverlapping_refs_since_match_p: 27267 disambiguations, 59019 must overlaps, 87056 queries
>   aliasing_component_refs_p: 65870 disambiguations, 2063394 queries
>   TBAA oracle: 26024415 disambiguations 60579490 queries
>                12450910 are in alias set 0
>                10806673 queries asked about the same object
>                125 queries asked about the same alias set
>                0 access volatile
>                9605837 are dependent in the DAG
>                1691530 are aritificially in conflict with void *
> 
> Modref stats:
>   modref use: 14272 disambiguations, 277680 queries
>   modref clobber: 1669753 disambiguations, 7849135 queries
>   4330162 tbaa queries (0.551674 per modref query)
>   699241 base compares (0.089085 per modref query)
> 
> PTA query stats:
>   pt_solution_includes: 1833920 disambiguations, 13846032 queries
>   pt_solutions_intersect: 1093785 disambiguations, 13309954 queries
> 
> So almost twice as many pt_solution_includes disambiguations.
> I will re-run the stats overnight to be sure that it is not independent
> change (but both build was from almost same checkout).
> 
> Bootstrapped/regtested x86_64-linux, OK?
> (I will analyze more the t2.c failure)
> 
> Honza
> 
> gcc/ChangeLog:
> 
> 2020-11-10  Jan Hubicka  <hubicka@ucw.cz>
> 
> 	* gimple.c: Include ipa-modref-tree.h and ipa-modref.h.
> 	(gimple_call_arg_flags): Use modref to determine flags.
> 	* ipa-modref.c: Include gimple-ssa.h, tree-phinodes.h,
> 	tree-ssa-operands.h, stringpool.h and tree-ssanames.h.
> 	(analyze_ssa_name_flags): Declare.
> 	(modref_summary::useful_p): Summary is also useful if arg flags are
> 	known.
> 	(dump_eaf_flags): New function.
> 	(modref_summary::dump): Use it.
> 	(get_modref_function_summary): Be read for current_function_decl
> 	being NULL.
> 	(memory_access_to): New function.
> 	(deref_flags): New function.
> 	(call_lhs_flags): New function.
> 	(analyze_parms): New function.
> 	(analyze_function): Use it.
> 	* ipa-modref.h (struct modref_summary): Add arg_flags.
> 
> gcc/testsuite/ChangeLog:
> 
> 2020-11-10  Jan Hubicka  <hubicka@ucw.cz>
> 
> 	* gcc.dg/torture/pta-ptrarith-1.c: Escape parametrs.
> 
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index 1afed88e1f1..da90716aa23 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -46,6 +46,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "asan.h"
>  #include "langhooks.h"
>  #include "attr-fnspec.h"
> +#include "ipa-modref-tree.h"
> +#include "ipa-modref.h"
>  
>  
>  /* All the tuples have their operand vector (if present) at the very bottom
> @@ -1532,24 +1534,45 @@ int
>  gimple_call_arg_flags (const gcall *stmt, unsigned arg)
>  {
>    attr_fnspec fnspec = gimple_call_fnspec (stmt);
> -
> -  if (!fnspec.known_p ())
> -    return 0;
> -
>    int flags = 0;
>  
> -  if (!fnspec.arg_specified_p (arg))
> -    ;
> -  else if (!fnspec.arg_used_p (arg))
> -    flags = EAF_UNUSED;
> -  else
> +  if (fnspec.known_p ())
>      {
> -      if (fnspec.arg_direct_p (arg))
> -	flags |= EAF_DIRECT;
> -      if (fnspec.arg_noescape_p (arg))
> -	flags |= EAF_NOESCAPE;
> -      if (fnspec.arg_readonly_p (arg))
> -	flags |= EAF_NOCLOBBER;
> +      if (!fnspec.arg_specified_p (arg))
> +	;
> +      else if (!fnspec.arg_used_p (arg))
> +	flags = EAF_UNUSED;
> +      else
> +	{
> +	  if (fnspec.arg_direct_p (arg))
> +	    flags |= EAF_DIRECT;
> +	  if (fnspec.arg_noescape_p (arg))
> +	    flags |= EAF_NOESCAPE;
> +	  if (fnspec.arg_readonly_p (arg))
> +	    flags |= EAF_NOCLOBBER;
> +	}
> +    }
> +  tree callee = gimple_call_fndecl (stmt);
> +  if (callee)
> +    {
> +      cgraph_node *node = cgraph_node::get (callee);
> +      modref_summary *summary = node ? get_modref_function_summary (node)
> +				: NULL;
> +
> +      if (summary && summary->arg_flags.length () > arg)

So could we make modref "transform" push this as fnspec attribute or
would that not really be an optimization?

> +	{
> +	  int modref_flags = summary->arg_flags[arg];
> +
> +	  /* We have possibly optimized out load.  Be conservative here.  */
> +	  if (!node->binds_to_current_def_p ())
> +	    {
> +	      if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
> +		modref_flags &= ~EAF_UNUSED;
> +	      if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
> +		modref_flags &= ~EAF_DIRECT;
> +	    }
> +	  flags |= modref_flags;
> +	}
>      }
>    return flags;
>  }
> diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
> index 3f46bebed3c..bde626115ff 100644
> --- a/gcc/ipa-modref.c
> +++ b/gcc/ipa-modref.c
> @@ -61,6 +61,14 @@ along with GCC; see the file COPYING3.  If not see
>  #include "ipa-fnsummary.h"
>  #include "attr-fnspec.h"
>  #include "symtab-clones.h"
> +#include "gimple-ssa.h"
> +#include "tree-phinodes.h"
> +#include "tree-ssa-operands.h"
> +#include "ssa-iterators.h"
> +#include "stringpool.h"
> +#include "tree-ssanames.h"
> +
> +static int analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth);
>  
>  /* We record fnspec specifiers for call edges since they depends on actual
>     gimple statements.  */
> @@ -186,6 +194,8 @@ modref_summary::useful_p (int ecf_flags)
>  {
>    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
>      return false;
> +  if (arg_flags.length ())
> +    return true;
>    if (loads && !loads->every_base)
>      return true;
>    if (ecf_flags & ECF_PURE)
> @@ -355,6 +365,22 @@ dump_lto_records (modref_records_lto *tt, FILE *out)
>      }
>  }
>  
> +/* Dump EAF flags.  */
> +
> +static void
> +dump_eaf_flags (FILE *out, int flags)
> +{
> +  if (flags & EAF_DIRECT)
> +    fprintf (out, " direct");
> +  if (flags & EAF_NOCLOBBER)
> +    fprintf (out, " noclobber");
> +  if (flags & EAF_NOESCAPE)
> +    fprintf (out, " noescape");
> +  if (flags & EAF_UNUSED)
> +    fprintf (out, " unused");
> +  fprintf (out, "\n");
> +}
> +
>  /* Dump summary.  */
>  
>  void
> @@ -372,6 +398,15 @@ modref_summary::dump (FILE *out)
>      }
>    if (writes_errno)
>      fprintf (out, "  Writes errno\n");
> +  if (arg_flags.length ())
> +    {
> +      for (unsigned int i = 0; i < arg_flags.length (); i++)
> +	if (arg_flags[i])
> +	  {
> +	    fprintf (out, "  parm %i flags:", i);
> +	    dump_eaf_flags (out, arg_flags[i]);
> +	  }
> +    }
>  }
>  
>  /* Dump summary.  */
> @@ -402,7 +437,8 @@ get_modref_function_summary (cgraph_node *func)
>       function.  */
>    enum availability avail;
>    func = func->function_or_virtual_thunk_symbol
> -	     (&avail, cgraph_node::get (current_function_decl));
> +		 (&avail, current_function_decl ?
> +			  cgraph_node::get (current_function_decl) : NULL);
>    if (avail <= AVAIL_INTERPOSABLE)
>      return NULL;
>  
> @@ -1067,6 +1103,315 @@ remove_summary (bool lto, bool nolto, bool ipa)
>  	     " - modref done with result: not tracked.\n");
>  }
>  
> +/* Return true if OP accesses memory pointed to by SSA_NAME.  */
> +
> +bool
> +memory_access_to (tree op, tree ssa_name)
> +{
> +  tree base = get_base_address (op);
> +  if (!base)
> +    return false;
> +  if (TREE_CODE (base) != MEM_REF && TREE_CODE (base) != TARGET_MEM_REF)
> +    return false;
> +  return TREE_OPERAND (base, 0) == ssa_name;
> +}
> +
> +/* Consider statement val = *arg.
> +   return EAF flags of ARG that can be determined from EAF flags of VAL
> +   (which are known to be FLAGS).  If IGNORE_STORES is true we can ignore
> +   all stores to VAL, i.e. when handling noreturn function.  */
> +
> +static int
> +deref_flags (int flags, bool ignore_stores)
> +{
> +  int ret = 0;
> +  if (flags & EAF_UNUSED)
> +    ret |= EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> +  else
> +    {
> +      if ((flags & EAF_NOCLOBBER) || ignore_stores)
> +	ret |= EAF_NOCLOBBER;
> +      if ((flags & EAF_NOESCAPE) || ignore_stores)
> +	ret |= EAF_NOESCAPE;
> +    }
> +  return ret;
> +}
> +
> +/* Call statements may return their parameters.  Consider argument number
> +   ARG of USE_STMT and determine flags that can needs to be cleared
> +   in case pointer possibly indirectly references from ARG I is returned.
> +   KNOWN_FLAGS and DEPTH are same as in analyze_ssa_name_flags.  */
> +
> +static int
> +call_lhs_flags (gimple *use_stmt, int arg, vec<int> *known_flags, int depth)
> +{
> +  /* If there is no return value, no flags are affected.  */
> +  if (!gimple_call_lhs (use_stmt))
> +    return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
> +
> +  /* If we know that function returns given argument and it is not ARG
> +     we can still be happy.  */
> +  int flags = gimple_call_return_flags (as_a <gcall *> (use_stmt));
> +  if ((flags & ERF_RETURNS_ARG)
> +      && (flags & ERF_RETURN_ARG_MASK) != arg)
> +    return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
> +
> +  /* If return value is SSA name determine its flags.  */
> +  if (TREE_CODE (gimple_call_lhs (use_stmt)) == SSA_NAME)
> +    return analyze_ssa_name_flags
> +		       (gimple_call_lhs (use_stmt), known_flags,
> +			depth + 1);
> +  /* In the case of memory store we can do nothing.  */
> +  else
> +    return 0;
> +}
> +
> +/* Analyze EAF flags for SSA name NAME.
> +   KNOWN_FLAGS is a cache for flags we already determined.
> +   DEPTH is a recursion depth used to make debug output prettier.  */
> +
> +static int
> +analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth)

C++ has references which makes the access to known_flags nicer ;)

> +{
> +  imm_use_iterator ui;
> +  gimple *use_stmt;
> +  int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
> +
> +  /* See if value is already computed.  */
> +  if ((*known_flags)[SSA_NAME_VERSION (name)])
> +    {
> +      /* Punt on cycles for now, so we do not need dataflow.  */
> +      if ((*known_flags)[SSA_NAME_VERSION (name)] == 1)
> +	{
> +	  if (dump_file)
> +	    fprintf (dump_file,
> +		     "%*sGiving up on a cycle in SSA graph\n", depth * 4, "");
> +	  return 0;
> +	}
> +      return (*known_flags)[SSA_NAME_VERSION (name)] - 2;
> +    }
> +  /* Recursion guard.  */
> +  (*known_flags)[SSA_NAME_VERSION (name)] = 1;

This also guards against multiple evaluations of the same stmts
but only in some cases?  Consider

  _1 = ..;
  _2 = _1 + _3;
  _4 = _1 + _5;
  _6 = _2 + _4;

where we visit _2 = and _4 = from _1 but from both are going
to visit _6.

Maybe I'm blind but you're not limiting depth?  Guess that asks
for problems, esp. as you are recursing rather than using a
worklist or so?

I see you try to "optimize" the walk by only visiting def->use
links from parameters but then a RPO walk over all stmts would
be simpler iteration-wise ...

> +  if (dump_file)
> +    {
> +      fprintf (dump_file,
> +	       "%*sAnalyzing flags of ssa name: ", depth * 4, "");
> +      print_generic_expr (dump_file, name);
> +      fprintf (dump_file, "\n");
> +    }
> +
> +  FOR_EACH_IMM_USE_STMT (use_stmt, ui, name)
> +    {
> +      if (flags == 0)
> +	{
> +	  BREAK_FROM_IMM_USE_STMT (ui);
> +	}
> +      if (is_gimple_debug (use_stmt))
> +	continue;
> +      if (dump_file)
> +	{
> +	  fprintf (dump_file, "%*s  Analyzing stmt:", depth * 4, "");
> +	  print_gimple_stmt (dump_file, use_stmt, 0);
> +	}
> +
> +      /* Gimple return may load the return value.  */
> +      if (gimple_code (use_stmt) == GIMPLE_RETURN)

 if (greturn *ret = dyn_cast <greturn *> (use_stmt))

makes the as_a below not needed, similar for the other cases (it
also makes accesses cheaper, avoiding some checking code).

> +	{
> +	  if (memory_access_to (gimple_return_retval
> +				   (as_a <greturn *> (use_stmt)), name))
> +	    flags &= ~EAF_UNUSED;
> +	}
> +      /* Account for LHS store, arg loads and flags from callee function.  */
> +      else if (is_gimple_call (use_stmt))
> +	{
> +	  tree callee = gimple_call_fndecl (use_stmt);
> +
> +	  /* Recursion would require bit of propagation; give up for now.  */
> +	  if (callee && recursive_call_p (current_function_decl, callee))
> +	    flags = 0;
> +	  else
> +	    {
> +	      int ecf_flags = gimple_call_flags (use_stmt);
> +	      bool ignore_stores = ignore_stores_p (current_function_decl,
> +						    ecf_flags);
> +
> +	      /* Handle *name = func (...).  */
> +	      if (gimple_call_lhs (use_stmt)
> +		  && memory_access_to (gimple_call_lhs (use_stmt), name))
> +		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> +
> +	      /* Handle all function parameters.  */
> +	      for (unsigned i = 0; i < gimple_call_num_args (use_stmt); i++)
> +		/* Name is directly passed to the callee.  */
> +		if (gimple_call_arg (use_stmt, i) == name)
> +		  {
> +		    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
> +		      flags &= ignore_stores
> +			       ? 0
> +			       : call_lhs_flags (use_stmt, i,
> +						 known_flags, depth);
> +		    else
> +		      {
> +			int call_flags = gimple_call_arg_flags (as_a <gcall *>
> +								 (use_stmt), i);
> +			if (ignore_stores)
> +			  call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE;
> +			else
> +			  call_flags &= call_lhs_flags (use_stmt, i,
> +							known_flags, depth);
> +
> +			flags &= call_flags;
> +		      }
> +		  }
> +		/* Name is dereferenced and passed to a callee.  */
> +		else if (memory_access_to (gimple_call_arg (use_stmt, i), name))
> +		  {
> +		    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
> +		      flags &= ~EAF_UNUSED;
> +		    else
> +		      flags &= deref_flags (gimple_call_arg_flags
> +						(as_a <gcall *> (use_stmt), i),
> +					    ignore_stores);
> +		    if (!ignore_stores)
> +		      flags &= call_lhs_flags (use_stmt, i, known_flags,
> +					       depth);
> +		  }

Hmm, you forget gimple_call_chain at least which is at least
argument passing?  Possibly the chain argument is never a function
parameter but how do you know... (partial inlining?)

Then there's gimple_call_fn for indirect function calls to a
function parameter?

I guess it would be nice to amend the gimple_walk_ stuff to have
a SSA name callback where the tree and the SSA use is passed.
Well, for another time.

> +	    }
> +	  /* Only NOCLOBBER or DIRECT flags alone are not useful (see comments
> +	     in tree-ssa-alias.c).  Give up earlier.  */
> +	  if ((flags & ~(EAF_DIRECT | EAF_NOCLOBBER)) == 0)
> +	    flags = 0;
> +	}
> +      else if (gimple_assign_load_p (use_stmt))
> +	{
> +	  /* Memory to memory copy.  */
> +	  if (gimple_store_p (use_stmt))
> +	    {
> +	      /* Handle *name = *exp.  */
> +	      if (memory_access_to (gimple_assign_lhs (use_stmt), name))
> +		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> +
> +	      /* Handle *lhs = *name.
> +
> +		 We do not track memory locations, so assume that value
> +		 is used arbitrarily.  */
> +	      if (memory_access_to (gimple_assign_rhs1 (use_stmt), name))
> +		flags = 0;
> +	    }
> +	  /* Handle lhs = *name.  */
> +	  else if (memory_access_to (gimple_assign_rhs1 (use_stmt), name))
> +	    flags &= deref_flags (analyze_ssa_name_flags
> +				      (gimple_assign_lhs (use_stmt),
> +				       known_flags, depth + 1), false);
> +	}
> +      else if (gimple_store_p (use_stmt))
> +	{
> +	  /* Handle *lhs = name.  */
> +	  if (is_gimple_assign (use_stmt)
> +	      && gimple_assign_rhs1 (use_stmt) == name)
> +	    {
> +	      if (dump_file)
> +		fprintf (dump_file, "%*s  ssa name saved to memory\n",
> +			 depth * 4, "");
> +	      flags = 0;
> +	    }
> +	  /* Handle *name = exp.  */
> +	  else if (is_gimple_assign (use_stmt)
> +		   && memory_access_to (gimple_assign_lhs (use_stmt), name))
> +	    flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> +	  /* ASM statements etc.  */
> +	  else if (!is_gimple_assign (use_stmt))
> +	    {
> +	      if (dump_file)
> +		fprintf (dump_file, "%*s  Unhandled store\n",
> +			 depth * 4, "");
> +	      flags = 0;
> +	    }
> +	}
> +      else if (is_gimple_assign (use_stmt))
> +	{
> +	  enum tree_code code = gimple_assign_rhs_code (use_stmt);
> +
> +	  /* See if operation is a merge as considered by
> +	     tree-ssa-structalias.c:find_func_aliases.  */
> +	  if (!truth_value_p (code)
> +	      && code != POINTER_DIFF_EXPR
> +	      && (code != POINTER_PLUS_EXPR
> +		  || gimple_assign_rhs1 (use_stmt) == name))
> +	    flags &= analyze_ssa_name_flags
> +			       (gimple_assign_lhs (use_stmt), known_flags,
> +				depth + 1);
> +	}
> +      else if (gimple_code (use_stmt) == GIMPLE_PHI)
> +	{
> +	  flags &= analyze_ssa_name_flags
> +			     (gimple_phi_result (use_stmt), known_flags,
> +			      depth + 1);
> +	}
> +      /* Conditions are not considered escape points
> +	 by tree-ssa-structalias.  */
> +      else if (gimple_code (use_stmt) == GIMPLE_COND)
> +	;
> +      else
> +	{
> +	  if (dump_file)
> +	    fprintf (dump_file, "%*s  Unhandled stmt\n", depth * 4, "");
> +	  flags = 0;
> +	}
> +
> +      if (dump_file)
> +	{
> +	  fprintf (dump_file, "%*s  current flags of ", depth * 4, "");
> +	  print_generic_expr (dump_file, name);
> +	  dump_eaf_flags (dump_file, flags);
> +	}
> +    }
> +  if (dump_file)
> +    {
> +      fprintf (dump_file, "%*sflags of ssa name ", depth * 4, "");
> +      print_generic_expr (dump_file, name);
> +      dump_eaf_flags (dump_file, flags);
> +    }
> +  (*known_flags)[SSA_NAME_VERSION (name)] = flags + 2;
> +  return flags;
> +}
> +
> +/* Determine EAF flags for function parameters.  */
> +
> +static void
> +analyze_parms (modref_summary *summary)
> +{
> +  unsigned int parm_index = 0;
> +  unsigned int count = 0;
> +
> +  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
> +       parm = TREE_CHAIN (parm))
> +    count++;
> +
> +  if (!count)
> +    return;
> +
> +  auto_vec<int> known_flags;
> +  known_flags.safe_grow_cleared (num_ssa_names);

, true for exact reserve.  Could be space optimized by not using
auto_vec<int> but auto_vec<usigned char>?

> +
> +  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++,
> +       parm = TREE_CHAIN (parm))
> +    {
> +      tree name = ssa_default_def (cfun, parm);
> +      if (!name)
> +	continue;

looks like the vec might be quite sparse ...

> +      int flags = analyze_ssa_name_flags (name, &known_flags, 0);
> +
> +      if (flags)
> +	{
> +	  if (parm_index >= summary->arg_flags.length ())
> +	    summary->arg_flags.safe_grow_cleared (count);

you want , true for exact reserve.

> +	  summary->arg_flags[parm_index] = flags;

maybe this as well - does it make sense to skip !is_gimple_reg_type
params in the counting?  Guess it makes lookup too complicated.
But we do have testcases with >30000 parameters ...

> +	}
> +    }
> +}
> +
>  /* Analyze function F.  IPA indicates whether we're running in local mode
>     (false) or the IPA mode (true).  */
>  
> @@ -1174,6 +1519,10 @@ analyze_function (function *f, bool ipa)
>  				  param_modref_max_accesses);
>        summary_lto->writes_errno = false;
>      }
> +
> +  if (!ipa)
> +    analyze_parms (summary);
> +
>    int ecf_flags = flags_from_decl_or_type (current_function_decl);
>    auto_vec <gimple *, 32> recursive_calls;
>  
> @@ -1191,8 +1540,9 @@ analyze_function (function *f, bool ipa)
>  	      || ((!summary || !summary->useful_p (ecf_flags))
>  		  && (!summary_lto || !summary_lto->useful_p (ecf_flags))))
>  	    {
> -	      remove_summary (lto, nolto, ipa);
> -	      return;
> +	      collapse_loads (summary, summary_lto);
> +	      collapse_stores (summary, summary_lto);
> +	      break;
>  	    }
>  	}
>      }
> diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
> index 31ceffa8d34..8fa05aaf7fb 100644
> --- a/gcc/ipa-modref.h
> +++ b/gcc/ipa-modref.h
> @@ -29,6 +29,7 @@ struct GTY(()) modref_summary
>    /* Load and stores in function (transitively closed to all callees)  */
>    modref_records *loads;
>    modref_records *stores;
> +  auto_vec<int> GTY((skip)) arg_flags;
>  
>    modref_summary ();
>    ~modref_summary ();
> diff --git a/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c b/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c
> index 99a548840df..85b68068b12 100644
> --- a/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c
> +++ b/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c
> @@ -6,11 +6,14 @@ struct Foo {
>    int *p;
>  };
>  
> +struct Foo *ff;
> +
>  void __attribute__((noinline))
>  foo (void *p)
>  {
>    struct Foo *f = (struct Foo *)p - 1;
>    *f->p = 0;
> +  ff = f;
>  }
>  
>  int bar (void)
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

> > +  tree callee = gimple_call_fndecl (stmt);
> > +  if (callee)
> > +    {
> > +      cgraph_node *node = cgraph_node::get (callee);
> > +      modref_summary *summary = node ? get_modref_function_summary (node)
> > +				: NULL;
> > +
> > +      if (summary && summary->arg_flags.length () > arg)
> 
> So could we make modref "transform" push this as fnspec attribute or
> would that not really be an optimization?

It was my original plan to synthetize fnspecs, but I think it is not
very good idea: we have the summary readily available and we can
represent information that fnspecs can't
(do not have artificial limits on number of parameters or counts)

I would preffer fnspecs to be used only for in-compiler declarations.
> > +
> > +/* Analyze EAF flags for SSA name NAME.
> > +   KNOWN_FLAGS is a cache for flags we already determined.
> > +   DEPTH is a recursion depth used to make debug output prettier.  */
> > +
> > +static int
> > +analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth)
> 
> C++ has references which makes the access to known_flags nicer ;)

Yay, will chang that :)
> 
> > +{
> > +  imm_use_iterator ui;
> > +  gimple *use_stmt;
> > +  int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
> > +
> > +  /* See if value is already computed.  */
> > +  if ((*known_flags)[SSA_NAME_VERSION (name)])
> > +    {
> > +      /* Punt on cycles for now, so we do not need dataflow.  */
> > +      if ((*known_flags)[SSA_NAME_VERSION (name)] == 1)
> > +	{
> > +	  if (dump_file)
> > +	    fprintf (dump_file,
> > +		     "%*sGiving up on a cycle in SSA graph\n", depth * 4, "");
> > +	  return 0;
> > +	}
> > +      return (*known_flags)[SSA_NAME_VERSION (name)] - 2;
> > +    }
> > +  /* Recursion guard.  */
> > +  (*known_flags)[SSA_NAME_VERSION (name)] = 1;
> 
> This also guards against multiple evaluations of the same stmts
> but only in some cases?  Consider
> 
>   _1 = ..;
>   _2 = _1 + _3;
>   _4 = _1 + _5;
>   _6 = _2 + _4;
> 
> where we visit _2 = and _4 = from _1 but from both are going
> to visit _6.

Here we first push _6, then we go for _2 then for _1 evaluate _1,
evalueate _2, go for _4 and evaluate _4, and evaluate _6.
It is DFS and you need backward edge in DFS (comming from a PHI).

Cycles seems to somewhat matter for GCC: we do have a lot of functions
that walk linked lists that we could track otherwise.
> 
> Maybe I'm blind but you're not limiting depth?  Guess that asks
> for problems, esp. as you are recursing rather than using a
> worklist or so?
> 
> I see you try to "optimize" the walk by only visiting def->use
> links from parameters but then a RPO walk over all stmts would
> be simpler iteration-wise ...
We usually evaluate just small part of bigger functions (since we lose
track quite easily after hitting first memory store).  My plan was to
change this to actual dataflow once we have it well defined 
(this means after discussing EAF flags with you and adding the logic to
track callsites for true IPA pass that midly complicated things - for
every ssa name I track callsite/arg pair where it is passed to
either directly or indirectly.  Then this is translaed into call summary
and used by IPA pass to compute final flags)

I guess I can add --param ipa-modref-walk-depth for now and handle
dataflow incremntally?
In particular I am not sure if I should just write iterated RPO myself
or use tree-ssa-propagate.h (the second may be overkill).
> 
> > +  if (dump_file)
> > +    {
> > +      fprintf (dump_file,
> > +	       "%*sAnalyzing flags of ssa name: ", depth * 4, "");
> > +      print_generic_expr (dump_file, name);
> > +      fprintf (dump_file, "\n");
> > +    }
> > +
> > +  FOR_EACH_IMM_USE_STMT (use_stmt, ui, name)
> > +    {
> > +      if (flags == 0)
> > +	{
> > +	  BREAK_FROM_IMM_USE_STMT (ui);
> > +	}
> > +      if (is_gimple_debug (use_stmt))
> > +	continue;
> > +      if (dump_file)
> > +	{
> > +	  fprintf (dump_file, "%*s  Analyzing stmt:", depth * 4, "");
> > +	  print_gimple_stmt (dump_file, use_stmt, 0);
> > +	}
> > +
> > +      /* Gimple return may load the return value.  */
> > +      if (gimple_code (use_stmt) == GIMPLE_RETURN)
> 
>  if (greturn *ret = dyn_cast <greturn *> (use_stmt))
> 
> makes the as_a below not needed, similar for the other cases (it
> also makes accesses cheaper, avoiding some checking code).

Looks cleaner indeed.
> 
> > +	{
> > +	  if (memory_access_to (gimple_return_retval
> > +				   (as_a <greturn *> (use_stmt)), name))
> > +	    flags &= ~EAF_UNUSED;
> > +	}
> > +      /* Account for LHS store, arg loads and flags from callee function.  */
> > +      else if (is_gimple_call (use_stmt))
> > +	{
> > +	  tree callee = gimple_call_fndecl (use_stmt);
> > +
> > +	  /* Recursion would require bit of propagation; give up for now.  */
> > +	  if (callee && recursive_call_p (current_function_decl, callee))
> > +	    flags = 0;
> > +	  else
> > +	    {
> > +	      int ecf_flags = gimple_call_flags (use_stmt);
> > +	      bool ignore_stores = ignore_stores_p (current_function_decl,
> > +						    ecf_flags);
> > +
> > +	      /* Handle *name = func (...).  */
> > +	      if (gimple_call_lhs (use_stmt)
> > +		  && memory_access_to (gimple_call_lhs (use_stmt), name))
> > +		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> > +
> > +	      /* Handle all function parameters.  */
> > +	      for (unsigned i = 0; i < gimple_call_num_args (use_stmt); i++)
> > +		/* Name is directly passed to the callee.  */
> > +		if (gimple_call_arg (use_stmt, i) == name)
> > +		  {
> > +		    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
> > +		      flags &= ignore_stores
> > +			       ? 0
> > +			       : call_lhs_flags (use_stmt, i,
> > +						 known_flags, depth);
> > +		    else
> > +		      {
> > +			int call_flags = gimple_call_arg_flags (as_a <gcall *>
> > +								 (use_stmt), i);
> > +			if (ignore_stores)
> > +			  call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE;
> > +			else
> > +			  call_flags &= call_lhs_flags (use_stmt, i,
> > +							known_flags, depth);
> > +
> > +			flags &= call_flags;
> > +		      }
> > +		  }
> > +		/* Name is dereferenced and passed to a callee.  */
> > +		else if (memory_access_to (gimple_call_arg (use_stmt, i), name))
> > +		  {
> > +		    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
> > +		      flags &= ~EAF_UNUSED;
> > +		    else
> > +		      flags &= deref_flags (gimple_call_arg_flags
> > +						(as_a <gcall *> (use_stmt), i),
> > +					    ignore_stores);
> > +		    if (!ignore_stores)
> > +		      flags &= call_lhs_flags (use_stmt, i, known_flags,
> > +					       depth);
> > +		  }
> 
> Hmm, you forget gimple_call_chain at least which is at least
> argument passing?  Possibly the chain argument is never a function
> parameter but how do you know... (partial inlining?)

Ah, right. I forgot about chain.
I wil ladd it.
> 
> Then there's gimple_call_fn for indirect function calls to a
> function parameter?

Well, it is never load or store, so I do not really care about it.
for

void test (int (*foo)())
{
  foo();
}

I should be able to derive EAF_UNUSED.

void test (int (**foo)())
{
  (*foo)();
}

I will see sparate load.
> 
> I guess it would be nice to amend the gimple_walk_ stuff to have
> a SSA name callback where the tree and the SSA use is passed.
> Well, for another time.
> 
> > +	    }
> > +	  /* Only NOCLOBBER or DIRECT flags alone are not useful (see comments
> > +	     in tree-ssa-alias.c).  Give up earlier.  */
> > +	  if ((flags & ~(EAF_DIRECT | EAF_NOCLOBBER)) == 0)
> > +	    flags = 0;
> > +	}
> > +      else if (gimple_assign_load_p (use_stmt))
> > +	{
> > +	  /* Memory to memory copy.  */
> > +	  if (gimple_store_p (use_stmt))
> > +	    {
> > +	      /* Handle *name = *exp.  */
> > +	      if (memory_access_to (gimple_assign_lhs (use_stmt), name))
> > +		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> > +
> > +	      /* Handle *lhs = *name.
> > +
> > +		 We do not track memory locations, so assume that value
> > +		 is used arbitrarily.  */
> > +	      if (memory_access_to (gimple_assign_rhs1 (use_stmt), name))
> > +		flags = 0;
> > +	    }
> > +	  /* Handle lhs = *name.  */
> > +	  else if (memory_access_to (gimple_assign_rhs1 (use_stmt), name))
> > +	    flags &= deref_flags (analyze_ssa_name_flags
> > +				      (gimple_assign_lhs (use_stmt),
> > +				       known_flags, depth + 1), false);
> > +	}
> > +      else if (gimple_store_p (use_stmt))
> > +	{
> > +	  /* Handle *lhs = name.  */
> > +	  if (is_gimple_assign (use_stmt)
> > +	      && gimple_assign_rhs1 (use_stmt) == name)
> > +	    {
> > +	      if (dump_file)
> > +		fprintf (dump_file, "%*s  ssa name saved to memory\n",
> > +			 depth * 4, "");
> > +	      flags = 0;
> > +	    }
> > +	  /* Handle *name = exp.  */
> > +	  else if (is_gimple_assign (use_stmt)
> > +		   && memory_access_to (gimple_assign_lhs (use_stmt), name))
> > +	    flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> > +	  /* ASM statements etc.  */
> > +	  else if (!is_gimple_assign (use_stmt))
> > +	    {
> > +	      if (dump_file)
> > +		fprintf (dump_file, "%*s  Unhandled store\n",
> > +			 depth * 4, "");
> > +	      flags = 0;
> > +	    }
> > +	}
> > +      else if (is_gimple_assign (use_stmt))
> > +	{
> > +	  enum tree_code code = gimple_assign_rhs_code (use_stmt);
> > +
> > +	  /* See if operation is a merge as considered by
> > +	     tree-ssa-structalias.c:find_func_aliases.  */
> > +	  if (!truth_value_p (code)
> > +	      && code != POINTER_DIFF_EXPR
> > +	      && (code != POINTER_PLUS_EXPR
> > +		  || gimple_assign_rhs1 (use_stmt) == name))
> > +	    flags &= analyze_ssa_name_flags
> > +			       (gimple_assign_lhs (use_stmt), known_flags,
> > +				depth + 1);
> > +	}
> > +      else if (gimple_code (use_stmt) == GIMPLE_PHI)
> > +	{
> > +	  flags &= analyze_ssa_name_flags
> > +			     (gimple_phi_result (use_stmt), known_flags,
> > +			      depth + 1);
> > +	}
> > +      /* Conditions are not considered escape points
> > +	 by tree-ssa-structalias.  */
> > +      else if (gimple_code (use_stmt) == GIMPLE_COND)
> > +	;
> > +      else
> > +	{
> > +	  if (dump_file)
> > +	    fprintf (dump_file, "%*s  Unhandled stmt\n", depth * 4, "");
> > +	  flags = 0;
> > +	}
> > +
> > +      if (dump_file)
> > +	{
> > +	  fprintf (dump_file, "%*s  current flags of ", depth * 4, "");
> > +	  print_generic_expr (dump_file, name);
> > +	  dump_eaf_flags (dump_file, flags);
> > +	}
> > +    }
> > +  if (dump_file)
> > +    {
> > +      fprintf (dump_file, "%*sflags of ssa name ", depth * 4, "");
> > +      print_generic_expr (dump_file, name);
> > +      dump_eaf_flags (dump_file, flags);
> > +    }
> > +  (*known_flags)[SSA_NAME_VERSION (name)] = flags + 2;
> > +  return flags;
> > +}
> > +
> > +/* Determine EAF flags for function parameters.  */
> > +
> > +static void
> > +analyze_parms (modref_summary *summary)
> > +{
> > +  unsigned int parm_index = 0;
> > +  unsigned int count = 0;
> > +
> > +  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
> > +       parm = TREE_CHAIN (parm))
> > +    count++;
> > +
> > +  if (!count)
> > +    return;
> > +
> > +  auto_vec<int> known_flags;
> > +  known_flags.safe_grow_cleared (num_ssa_names);
> 
> , true for exact reserve.  Could be space optimized by not using
> auto_vec<int> but auto_vec<usigned char>?

I think there is not that much memory wasted, but indeed chars will be
more effective.
> 
> > +
> > +  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++,
> > +       parm = TREE_CHAIN (parm))
> > +    {
> > +      tree name = ssa_default_def (cfun, parm);
> > +      if (!name)
> > +	continue;
> 
> looks like the vec might be quite sparse ...
> 
> > +      int flags = analyze_ssa_name_flags (name, &known_flags, 0);
> > +
> > +      if (flags)
> > +	{
> > +	  if (parm_index >= summary->arg_flags.length ())
> > +	    summary->arg_flags.safe_grow_cleared (count);
> 
> you want , true for exact reserve.
> 
> > +	  summary->arg_flags[parm_index] = flags;
> 
> maybe this as well - does it make sense to skip !is_gimple_reg_type
> params in the counting?  Guess it makes lookup too complicated.
> But we do have testcases with >30000 parameters ...

Well, the index needs to be actual index all call argument.
As said, i did not see any noticeable modref memory use increase at WPA
(I watched) since it is at most 1 byte for parameter in case something
got tracked.

Thanks for review!

Honza

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

Hi,
here is updaed patch.

Honza

Bootstrapped/regtested x86_64-linux, OK (after the fnspec fixes)?

2020-11-10  Jan Hubicka  <hubicka@ucw.cz>

	* gimple.c: Include ipa-modref-tree.h and ipa-modref.h.
	(gimple_call_arg_flags): Use modref to determine flags.
	* ipa-modref.c: Include gimple-ssa.h, tree-phinodes.h,
	tree-ssa-operands.h, stringpool.h and tree-ssanames.h.
	(analyze_ssa_name_flags): Declare.
	(modref_summary::useful_p): Summary is also useful if arg flags are
	known.
	(dump_eaf_flags): New function.
	(modref_summary::dump): Use it.
	(get_modref_function_summary): Be read for current_function_decl
	being NULL.
	(memory_access_to): New function.
	(deref_flags): New function.
	(call_lhs_flags): New function.
	(analyze_parms): New function.
	(analyze_function): Use it.
	* ipa-modref.h (struct modref_summary): Add arg_flags.
	* doc/invoke.texi (ipa-modref-max-depth): Document.
	* params.opt (ipa-modref-max-depth): New param.

gcc/testsuite/ChangeLog:

2020-11-10  Jan Hubicka  <hubicka@ucw.cz>

	* gcc.dg/torture/pta-ptrarith-1.c: Escape parametrs.

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d2a188d7c75..0bd76d2841e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12953,6 +12953,10 @@ memory locations using the mod/ref information.  This parameter ought to be
 bigger than @option{--param ipa-modref-max-bases} and @option{--param
 ipa-modref-max-refs}.
 
+@item ipa-modref-max-depth
+Specified the maximum depth of DFS walk used by modref escape analysis.
+Setting to 0 disables the analysis completely.
+
 @item profile-func-internal-id
 A parameter to control whether to use function internal id in profile
 database lookup. If the value is 0, the compiler uses an id that
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 1afed88e1f1..da90716aa23 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -46,6 +46,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "asan.h"
 #include "langhooks.h"
 #include "attr-fnspec.h"
+#include "ipa-modref-tree.h"
+#include "ipa-modref.h"
 
 
 /* All the tuples have their operand vector (if present) at the very bottom
@@ -1532,24 +1534,45 @@ int
 gimple_call_arg_flags (const gcall *stmt, unsigned arg)
 {
   attr_fnspec fnspec = gimple_call_fnspec (stmt);
-
-  if (!fnspec.known_p ())
-    return 0;
-
   int flags = 0;
 
-  if (!fnspec.arg_specified_p (arg))
-    ;
-  else if (!fnspec.arg_used_p (arg))
-    flags = EAF_UNUSED;
-  else
+  if (fnspec.known_p ())
     {
-      if (fnspec.arg_direct_p (arg))
-	flags |= EAF_DIRECT;
-      if (fnspec.arg_noescape_p (arg))
-	flags |= EAF_NOESCAPE;
-      if (fnspec.arg_readonly_p (arg))
-	flags |= EAF_NOCLOBBER;
+      if (!fnspec.arg_specified_p (arg))
+	;
+      else if (!fnspec.arg_used_p (arg))
+	flags = EAF_UNUSED;
+      else
+	{
+	  if (fnspec.arg_direct_p (arg))
+	    flags |= EAF_DIRECT;
+	  if (fnspec.arg_noescape_p (arg))
+	    flags |= EAF_NOESCAPE;
+	  if (fnspec.arg_readonly_p (arg))
+	    flags |= EAF_NOCLOBBER;
+	}
+    }
+  tree callee = gimple_call_fndecl (stmt);
+  if (callee)
+    {
+      cgraph_node *node = cgraph_node::get (callee);
+      modref_summary *summary = node ? get_modref_function_summary (node)
+				: NULL;
+
+      if (summary && summary->arg_flags.length () > arg)
+	{
+	  int modref_flags = summary->arg_flags[arg];
+
+	  /* We have possibly optimized out load.  Be conservative here.  */
+	  if (!node->binds_to_current_def_p ())
+	    {
+	      if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
+		modref_flags &= ~EAF_UNUSED;
+	      if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
+		modref_flags &= ~EAF_DIRECT;
+	    }
+	  flags |= modref_flags;
+	}
     }
   return flags;
 }
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 3f46bebed3c..30e76580fb0 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -61,6 +61,15 @@ along with GCC; see the file COPYING3.  If not see
 #include "ipa-fnsummary.h"
 #include "attr-fnspec.h"
 #include "symtab-clones.h"
+#include "gimple-ssa.h"
+#include "tree-phinodes.h"
+#include "tree-ssa-operands.h"
+#include "ssa-iterators.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
+
+static int analyze_ssa_name_flags (tree name,
+				   vec<unsigned char> &known_flags, int depth);
 
 /* We record fnspec specifiers for call edges since they depends on actual
    gimple statements.  */
@@ -186,6 +195,8 @@ modref_summary::useful_p (int ecf_flags)
 {
   if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
     return false;
+  if (arg_flags.length ())
+    return true;
   if (loads && !loads->every_base)
     return true;
   if (ecf_flags & ECF_PURE)
@@ -355,6 +366,22 @@ dump_lto_records (modref_records_lto *tt, FILE *out)
     }
 }
 
+/* Dump EAF flags.  */
+
+static void
+dump_eaf_flags (FILE *out, int flags)
+{
+  if (flags & EAF_DIRECT)
+    fprintf (out, " direct");
+  if (flags & EAF_NOCLOBBER)
+    fprintf (out, " noclobber");
+  if (flags & EAF_NOESCAPE)
+    fprintf (out, " noescape");
+  if (flags & EAF_UNUSED)
+    fprintf (out, " unused");
+  fprintf (out, "\n");
+}
+
 /* Dump summary.  */
 
 void
@@ -372,6 +399,15 @@ modref_summary::dump (FILE *out)
     }
   if (writes_errno)
     fprintf (out, "  Writes errno\n");
+  if (arg_flags.length ())
+    {
+      for (unsigned int i = 0; i < arg_flags.length (); i++)
+	if (arg_flags[i])
+	  {
+	    fprintf (out, "  parm %i flags:", i);
+	    dump_eaf_flags (out, arg_flags[i]);
+	  }
+    }
 }
 
 /* Dump summary.  */
@@ -402,7 +438,8 @@ get_modref_function_summary (cgraph_node *func)
      function.  */
   enum availability avail;
   func = func->function_or_virtual_thunk_symbol
-	     (&avail, cgraph_node::get (current_function_decl));
+		 (&avail, current_function_decl ?
+			  cgraph_node::get (current_function_decl) : NULL);
   if (avail <= AVAIL_INTERPOSABLE)
     return NULL;
 
@@ -634,7 +671,7 @@ merge_call_side_effects (modref_summary *cur_summary,
       cur_summary->loads->collapse ();
     }
 
-  parm_map.safe_grow_cleared (gimple_call_num_args (stmt));
+  parm_map.safe_grow_cleared (gimple_call_num_args (stmt), true);
   for (unsigned i = 0; i < gimple_call_num_args (stmt); i++)
     {
       parm_map[i] = parm_map_for_arg (stmt, i);
@@ -1067,6 +1104,326 @@ remove_summary (bool lto, bool nolto, bool ipa)
 	     " - modref done with result: not tracked.\n");
 }
 
+/* Return true if OP accesses memory pointed to by SSA_NAME.  */
+
+bool
+memory_access_to (tree op, tree ssa_name)
+{
+  tree base = get_base_address (op);
+  if (!base)
+    return false;
+  if (TREE_CODE (base) != MEM_REF && TREE_CODE (base) != TARGET_MEM_REF)
+    return false;
+  return TREE_OPERAND (base, 0) == ssa_name;
+}
+
+/* Consider statement val = *arg.
+   return EAF flags of ARG that can be determined from EAF flags of VAL
+   (which are known to be FLAGS).  If IGNORE_STORES is true we can ignore
+   all stores to VAL, i.e. when handling noreturn function.  */
+
+static int
+deref_flags (int flags, bool ignore_stores)
+{
+  int ret = 0;
+  if (flags & EAF_UNUSED)
+    ret |= EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
+  else
+    {
+      if ((flags & EAF_NOCLOBBER) || ignore_stores)
+	ret |= EAF_NOCLOBBER;
+      if ((flags & EAF_NOESCAPE) || ignore_stores)
+	ret |= EAF_NOESCAPE;
+    }
+  return ret;
+}
+
+/* Call statements may return their parameters.  Consider argument number
+   ARG of USE_STMT and determine flags that can needs to be cleared
+   in case pointer possibly indirectly references from ARG I is returned.
+   KNOWN_FLAGS and DEPTH are same as in analyze_ssa_name_flags.  */
+
+static int
+call_lhs_flags (gcall *call, int arg,
+		vec<unsigned char> &known_flags, int depth)
+{
+  /* If there is no return value, no flags are affected.  */
+  if (!gimple_call_lhs (call))
+    return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
+
+  /* If we know that function returns given argument and it is not ARG
+     we can still be happy.  */
+  int flags = gimple_call_return_flags (call);
+  if ((flags & ERF_RETURNS_ARG)
+      && (flags & ERF_RETURN_ARG_MASK) != arg)
+    return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
+
+  /* If return value is SSA name determine its flags.  */
+  if (TREE_CODE (gimple_call_lhs (call)) == SSA_NAME)
+    return analyze_ssa_name_flags
+		       (gimple_call_lhs (call), known_flags,
+			depth + 1);
+  /* In the case of memory store we can do nothing.  */
+  else
+    return 0;
+}
+
+/* Analyze EAF flags for SSA name NAME.
+   KNOWN_FLAGS is a cache for flags we already determined.
+   DEPTH is a recursion depth used to make debug output prettier.  */
+
+static int
+analyze_ssa_name_flags (tree name, vec<unsigned char> &known_flags, int depth)
+{
+  imm_use_iterator ui;
+  gimple *use_stmt;
+  int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
+
+  /* See if value is already computed.  */
+  if (known_flags[SSA_NAME_VERSION (name)])
+    {
+      /* Punt on cycles for now, so we do not need dataflow.  */
+      if (known_flags[SSA_NAME_VERSION (name)] == 1)
+	{
+	  if (dump_file)
+	    fprintf (dump_file,
+		     "%*sGiving up on a cycle in SSA graph\n", depth * 4, "");
+	  return 0;
+	}
+      return known_flags[SSA_NAME_VERSION (name)] - 2;
+    }
+  if (depth == param_modref_max_depth)
+    {
+      if (dump_file)
+	fprintf (dump_file,
+		 "%*sGiving up on max depth\n", depth * 4, "");
+      return 0;
+    }
+  /* Recursion guard.  */
+  known_flags[SSA_NAME_VERSION (name)] = 1;
+
+  if (dump_file)
+    {
+      fprintf (dump_file,
+	       "%*sAnalyzing flags of ssa name: ", depth * 4, "");
+      print_generic_expr (dump_file, name);
+      fprintf (dump_file, "\n");
+    }
+
+  FOR_EACH_IMM_USE_STMT (use_stmt, ui, name)
+    {
+      if (flags == 0)
+	{
+	  BREAK_FROM_IMM_USE_STMT (ui);
+	}
+      if (is_gimple_debug (use_stmt))
+	continue;
+      if (dump_file)
+	{
+	  fprintf (dump_file, "%*s  Analyzing stmt:", depth * 4, "");
+	  print_gimple_stmt (dump_file, use_stmt, 0);
+	}
+
+      /* Gimple return may load the return value.  */
+      if (greturn *ret = dyn_cast <greturn *> (use_stmt))
+	{
+	  if (memory_access_to (gimple_return_retval (ret), name))
+	    flags &= ~EAF_UNUSED;
+	}
+      /* Account for LHS store, arg loads and flags from callee function.  */
+      else if (gcall *call = dyn_cast <gcall *> (use_stmt))
+	{
+	  tree callee = gimple_call_fndecl (call);
+
+	  /* Recursion would require bit of propagation; give up for now.  */
+	  if (callee && recursive_call_p (current_function_decl, callee))
+	    flags = 0;
+	  else
+	    {
+	      int ecf_flags = gimple_call_flags (call);
+	      bool ignore_stores = ignore_stores_p (current_function_decl,
+						    ecf_flags);
+
+	      /* Handle *name = func (...).  */
+	      if (gimple_call_lhs (call)
+		  && memory_access_to (gimple_call_lhs (call), name))
+		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+
+	      /* We do not track accesses to the static chain (we could)
+		 so give up.  */
+	      if (gimple_call_chain (call)
+		  && (gimple_call_chain (call) == name))
+		flags = 0;
+
+	      /* Handle all function parameters.  */
+	      for (unsigned i = 0; i < gimple_call_num_args (call); i++)
+		/* Name is directly passed to the callee.  */
+		if (gimple_call_arg (call, i) == name)
+		  {
+		    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
+		      flags &= ignore_stores
+			       ? 0
+			       : call_lhs_flags (call, i, known_flags, depth);
+		    else
+		      {
+			int call_flags = gimple_call_arg_flags (call, i);
+			if (ignore_stores)
+			  call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE;
+			else
+			  call_flags &= call_lhs_flags (call, i,
+							known_flags, depth);
+
+			flags &= call_flags;
+		      }
+		  }
+		/* Name is dereferenced and passed to a callee.  */
+		else if (memory_access_to (gimple_call_arg (call, i), name))
+		  {
+		    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
+		      flags &= ~EAF_UNUSED;
+		    else
+		      flags &= deref_flags (gimple_call_arg_flags (call, i),
+					    ignore_stores);
+		    if (!ignore_stores)
+		      flags &= call_lhs_flags (call, i, known_flags, depth);
+		  }
+	    }
+	  /* Only NOCLOBBER or DIRECT flags alone are not useful (see comments
+	     in tree-ssa-alias.c).  Give up earlier.  */
+	  if ((flags & ~(EAF_DIRECT | EAF_NOCLOBBER)) == 0)
+	    flags = 0;
+	}
+      else if (gimple_assign_load_p (use_stmt))
+	{
+	  gassign *assign = as_a <gassign *> (use_stmt);
+	  /* Memory to memory copy.  */
+	  if (gimple_store_p (assign))
+	    {
+	      /* Handle *name = *exp.  */
+	      if (memory_access_to (gimple_assign_lhs (assign), name))
+		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+
+	      /* Handle *lhs = *name.
+
+		 We do not track memory locations, so assume that value
+		 is used arbitrarily.  */
+	      if (memory_access_to (gimple_assign_rhs1 (assign), name))
+		flags = 0;
+	    }
+	  /* Handle lhs = *name.  */
+	  else if (memory_access_to (gimple_assign_rhs1 (assign), name))
+	    flags &= deref_flags (analyze_ssa_name_flags
+				      (gimple_assign_lhs (assign),
+				       known_flags, depth + 1), false);
+	}
+      else if (gimple_store_p (use_stmt))
+	{
+	  gassign *assign = dyn_cast <gassign *> (use_stmt);
+
+	  /* Handle *lhs = name.  */
+	  if (assign && gimple_assign_rhs1 (assign) == name)
+	    {
+	      if (dump_file)
+		fprintf (dump_file, "%*s  ssa name saved to memory\n",
+			 depth * 4, "");
+	      flags = 0;
+	    }
+	  /* Handle *name = exp.  */
+	  else if (assign
+		   && memory_access_to (gimple_assign_lhs (assign), name))
+	    flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+	  /* ASM statements etc.  */
+	  else if (!assign)
+	    {
+	      if (dump_file)
+		fprintf (dump_file, "%*s  Unhandled store\n",
+			 depth * 4, "");
+	      flags = 0;
+	    }
+	}
+      else if (gassign *assign = dyn_cast <gassign *> (use_stmt))
+	{
+	  enum tree_code code = gimple_assign_rhs_code (assign);
+
+	  /* See if operation is a merge as considered by
+	     tree-ssa-structalias.c:find_func_aliases.  */
+	  if (!truth_value_p (code)
+	      && code != POINTER_DIFF_EXPR
+	      && (code != POINTER_PLUS_EXPR
+		  || gimple_assign_rhs1 (assign) == name))
+	    flags &= analyze_ssa_name_flags
+			       (gimple_assign_lhs (assign), known_flags,
+				depth + 1);
+	}
+      else if (gphi *phi = dyn_cast <gphi *> (use_stmt))
+	{
+	  flags &= analyze_ssa_name_flags
+			     (gimple_phi_result (phi), known_flags,
+			      depth + 1);
+	}
+      /* Conditions are not considered escape points
+	 by tree-ssa-structalias.  */
+      else if (gimple_code (use_stmt) == GIMPLE_COND)
+	;
+      else
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "%*s  Unhandled stmt\n", depth * 4, "");
+	  flags = 0;
+	}
+
+      if (dump_file)
+	{
+	  fprintf (dump_file, "%*s  current flags of ", depth * 4, "");
+	  print_generic_expr (dump_file, name);
+	  dump_eaf_flags (dump_file, flags);
+	}
+    }
+  if (dump_file)
+    {
+      fprintf (dump_file, "%*sflags of ssa name ", depth * 4, "");
+      print_generic_expr (dump_file, name);
+      dump_eaf_flags (dump_file, flags);
+    }
+  known_flags[SSA_NAME_VERSION (name)] = flags + 2;
+  return flags;
+}
+
+/* Determine EAF flags for function parameters.  */
+
+static void
+analyze_parms (modref_summary *summary)
+{
+  unsigned int parm_index = 0;
+  unsigned int count = 0;
+
+  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
+       parm = TREE_CHAIN (parm))
+    count++;
+
+  if (!count)
+    return;
+
+  auto_vec<unsigned char> known_flags;
+  known_flags.safe_grow_cleared (num_ssa_names, true);
+
+  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++,
+       parm = TREE_CHAIN (parm))
+    {
+      tree name = ssa_default_def (cfun, parm);
+      if (!name)
+	continue;
+      int flags = analyze_ssa_name_flags (name, known_flags, 0);
+
+      if (flags)
+	{
+	  if (parm_index >= summary->arg_flags.length ())
+	    summary->arg_flags.safe_grow_cleared (count, true);
+	  summary->arg_flags[parm_index] = flags;
+	}
+    }
+}
+
 /* Analyze function F.  IPA indicates whether we're running in local mode
    (false) or the IPA mode (true).  */
 
@@ -1174,6 +1531,10 @@ analyze_function (function *f, bool ipa)
 				  param_modref_max_accesses);
       summary_lto->writes_errno = false;
     }
+
+  if (!ipa)
+    analyze_parms (summary);
+
   int ecf_flags = flags_from_decl_or_type (current_function_decl);
   auto_vec <gimple *, 32> recursive_calls;
 
@@ -1191,8 +1552,9 @@ analyze_function (function *f, bool ipa)
 	      || ((!summary || !summary->useful_p (ecf_flags))
 		  && (!summary_lto || !summary_lto->useful_p (ecf_flags))))
 	    {
-	      remove_summary (lto, nolto, ipa);
-	      return;
+	      collapse_loads (summary, summary_lto);
+	      collapse_stores (summary, summary_lto);
+	      break;
 	    }
 	}
     }
@@ -1957,7 +2319,7 @@ compute_parm_map (cgraph_edge *callee_edge, vec<modref_parm_map> *parm_map)
 					: callee_edge->caller);
       callee_pi = IPA_NODE_REF (callee);
 
-      (*parm_map).safe_grow_cleared (count);
+      (*parm_map).safe_grow_cleared (count, true);
 
       for (i = 0; i < count; i++)
 	{
diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
index 31ceffa8d34..59872301cd6 100644
--- a/gcc/ipa-modref.h
+++ b/gcc/ipa-modref.h
@@ -29,6 +29,7 @@ struct GTY(()) modref_summary
   /* Load and stores in function (transitively closed to all callees)  */
   modref_records *loads;
   modref_records *stores;
+  auto_vec<unsigned char> GTY((skip)) arg_flags;
 
   modref_summary ();
   ~modref_summary ();
diff --git a/gcc/params.opt b/gcc/params.opt
index a33a371a395..70152bf59bb 100644
--- a/gcc/params.opt
+++ b/gcc/params.opt
@@ -931,6 +931,10 @@ Maximum number of accesse stored in each modref reference.
 Common Joined UInteger Var(param_modref_max_tests) Init(64)
 Maximum number of tests performed by modref query.
 
+-param=modref-max-depth=
+Common Joined UInteger Var(param_modref_max_depth) Init(256)
+Maximum depth of DFS walk used by modref escape analysis
+
 -param=tm-max-aggregate-size=
 Common Joined UInteger Var(param_tm_max_aggregate_size) Init(9) Param Optimization
 Size in bytes after which thread-local aggregates should be instrumented with the logging functions instead of save/restore pairs.

Re: Detect EAF flags in ipa-modref

[Richard Biener]

On Tue, 10 Nov 2020, Jan Hubicka wrote:

> Hi,
> here is updaed patch.
> 
> Honza
> 
> Bootstrapped/regtested x86_64-linux, OK (after the fnspec fixes)?

OK.

Thanks,
Richard.

> 
> 2020-11-10  Jan Hubicka  <hubicka@ucw.cz>
> 
> 	* gimple.c: Include ipa-modref-tree.h and ipa-modref.h.
> 	(gimple_call_arg_flags): Use modref to determine flags.
> 	* ipa-modref.c: Include gimple-ssa.h, tree-phinodes.h,
> 	tree-ssa-operands.h, stringpool.h and tree-ssanames.h.
> 	(analyze_ssa_name_flags): Declare.
> 	(modref_summary::useful_p): Summary is also useful if arg flags are
> 	known.
> 	(dump_eaf_flags): New function.
> 	(modref_summary::dump): Use it.
> 	(get_modref_function_summary): Be read for current_function_decl
> 	being NULL.
> 	(memory_access_to): New function.
> 	(deref_flags): New function.
> 	(call_lhs_flags): New function.
> 	(analyze_parms): New function.
> 	(analyze_function): Use it.
> 	* ipa-modref.h (struct modref_summary): Add arg_flags.
> 	* doc/invoke.texi (ipa-modref-max-depth): Document.
> 	* params.opt (ipa-modref-max-depth): New param.
> 
> gcc/testsuite/ChangeLog:
> 
> 2020-11-10  Jan Hubicka  <hubicka@ucw.cz>
> 
> 	* gcc.dg/torture/pta-ptrarith-1.c: Escape parametrs.
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index d2a188d7c75..0bd76d2841e 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -12953,6 +12953,10 @@ memory locations using the mod/ref information.  This parameter ought to be
>  bigger than @option{--param ipa-modref-max-bases} and @option{--param
>  ipa-modref-max-refs}.
>  
> +@item ipa-modref-max-depth
> +Specified the maximum depth of DFS walk used by modref escape analysis.
> +Setting to 0 disables the analysis completely.
> +
>  @item profile-func-internal-id
>  A parameter to control whether to use function internal id in profile
>  database lookup. If the value is 0, the compiler uses an id that
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index 1afed88e1f1..da90716aa23 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -46,6 +46,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "asan.h"
>  #include "langhooks.h"
>  #include "attr-fnspec.h"
> +#include "ipa-modref-tree.h"
> +#include "ipa-modref.h"
>  
>  
>  /* All the tuples have their operand vector (if present) at the very bottom
> @@ -1532,24 +1534,45 @@ int
>  gimple_call_arg_flags (const gcall *stmt, unsigned arg)
>  {
>    attr_fnspec fnspec = gimple_call_fnspec (stmt);
> -
> -  if (!fnspec.known_p ())
> -    return 0;
> -
>    int flags = 0;
>  
> -  if (!fnspec.arg_specified_p (arg))
> -    ;
> -  else if (!fnspec.arg_used_p (arg))
> -    flags = EAF_UNUSED;
> -  else
> +  if (fnspec.known_p ())
>      {
> -      if (fnspec.arg_direct_p (arg))
> -	flags |= EAF_DIRECT;
> -      if (fnspec.arg_noescape_p (arg))
> -	flags |= EAF_NOESCAPE;
> -      if (fnspec.arg_readonly_p (arg))
> -	flags |= EAF_NOCLOBBER;
> +      if (!fnspec.arg_specified_p (arg))
> +	;
> +      else if (!fnspec.arg_used_p (arg))
> +	flags = EAF_UNUSED;
> +      else
> +	{
> +	  if (fnspec.arg_direct_p (arg))
> +	    flags |= EAF_DIRECT;
> +	  if (fnspec.arg_noescape_p (arg))
> +	    flags |= EAF_NOESCAPE;
> +	  if (fnspec.arg_readonly_p (arg))
> +	    flags |= EAF_NOCLOBBER;
> +	}
> +    }
> +  tree callee = gimple_call_fndecl (stmt);
> +  if (callee)
> +    {
> +      cgraph_node *node = cgraph_node::get (callee);
> +      modref_summary *summary = node ? get_modref_function_summary (node)
> +				: NULL;
> +
> +      if (summary && summary->arg_flags.length () > arg)
> +	{
> +	  int modref_flags = summary->arg_flags[arg];
> +
> +	  /* We have possibly optimized out load.  Be conservative here.  */
> +	  if (!node->binds_to_current_def_p ())
> +	    {
> +	      if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
> +		modref_flags &= ~EAF_UNUSED;
> +	      if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
> +		modref_flags &= ~EAF_DIRECT;
> +	    }
> +	  flags |= modref_flags;
> +	}
>      }
>    return flags;
>  }
> diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
> index 3f46bebed3c..30e76580fb0 100644
> --- a/gcc/ipa-modref.c
> +++ b/gcc/ipa-modref.c
> @@ -61,6 +61,15 @@ along with GCC; see the file COPYING3.  If not see
>  #include "ipa-fnsummary.h"
>  #include "attr-fnspec.h"
>  #include "symtab-clones.h"
> +#include "gimple-ssa.h"
> +#include "tree-phinodes.h"
> +#include "tree-ssa-operands.h"
> +#include "ssa-iterators.h"
> +#include "stringpool.h"
> +#include "tree-ssanames.h"
> +
> +static int analyze_ssa_name_flags (tree name,
> +				   vec<unsigned char> &known_flags, int depth);
>  
>  /* We record fnspec specifiers for call edges since they depends on actual
>     gimple statements.  */
> @@ -186,6 +195,8 @@ modref_summary::useful_p (int ecf_flags)
>  {
>    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
>      return false;
> +  if (arg_flags.length ())
> +    return true;
>    if (loads && !loads->every_base)
>      return true;
>    if (ecf_flags & ECF_PURE)
> @@ -355,6 +366,22 @@ dump_lto_records (modref_records_lto *tt, FILE *out)
>      }
>  }
>  
> +/* Dump EAF flags.  */
> +
> +static void
> +dump_eaf_flags (FILE *out, int flags)
> +{
> +  if (flags & EAF_DIRECT)
> +    fprintf (out, " direct");
> +  if (flags & EAF_NOCLOBBER)
> +    fprintf (out, " noclobber");
> +  if (flags & EAF_NOESCAPE)
> +    fprintf (out, " noescape");
> +  if (flags & EAF_UNUSED)
> +    fprintf (out, " unused");
> +  fprintf (out, "\n");
> +}
> +
>  /* Dump summary.  */
>  
>  void
> @@ -372,6 +399,15 @@ modref_summary::dump (FILE *out)
>      }
>    if (writes_errno)
>      fprintf (out, "  Writes errno\n");
> +  if (arg_flags.length ())
> +    {
> +      for (unsigned int i = 0; i < arg_flags.length (); i++)
> +	if (arg_flags[i])
> +	  {
> +	    fprintf (out, "  parm %i flags:", i);
> +	    dump_eaf_flags (out, arg_flags[i]);
> +	  }
> +    }
>  }
>  
>  /* Dump summary.  */
> @@ -402,7 +438,8 @@ get_modref_function_summary (cgraph_node *func)
>       function.  */
>    enum availability avail;
>    func = func->function_or_virtual_thunk_symbol
> -	     (&avail, cgraph_node::get (current_function_decl));
> +		 (&avail, current_function_decl ?
> +			  cgraph_node::get (current_function_decl) : NULL);
>    if (avail <= AVAIL_INTERPOSABLE)
>      return NULL;
>  
> @@ -634,7 +671,7 @@ merge_call_side_effects (modref_summary *cur_summary,
>        cur_summary->loads->collapse ();
>      }
>  
> -  parm_map.safe_grow_cleared (gimple_call_num_args (stmt));
> +  parm_map.safe_grow_cleared (gimple_call_num_args (stmt), true);
>    for (unsigned i = 0; i < gimple_call_num_args (stmt); i++)
>      {
>        parm_map[i] = parm_map_for_arg (stmt, i);
> @@ -1067,6 +1104,326 @@ remove_summary (bool lto, bool nolto, bool ipa)
>  	     " - modref done with result: not tracked.\n");
>  }
>  
> +/* Return true if OP accesses memory pointed to by SSA_NAME.  */
> +
> +bool
> +memory_access_to (tree op, tree ssa_name)
> +{
> +  tree base = get_base_address (op);
> +  if (!base)
> +    return false;
> +  if (TREE_CODE (base) != MEM_REF && TREE_CODE (base) != TARGET_MEM_REF)
> +    return false;
> +  return TREE_OPERAND (base, 0) == ssa_name;
> +}
> +
> +/* Consider statement val = *arg.
> +   return EAF flags of ARG that can be determined from EAF flags of VAL
> +   (which are known to be FLAGS).  If IGNORE_STORES is true we can ignore
> +   all stores to VAL, i.e. when handling noreturn function.  */
> +
> +static int
> +deref_flags (int flags, bool ignore_stores)
> +{
> +  int ret = 0;
> +  if (flags & EAF_UNUSED)
> +    ret |= EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> +  else
> +    {
> +      if ((flags & EAF_NOCLOBBER) || ignore_stores)
> +	ret |= EAF_NOCLOBBER;
> +      if ((flags & EAF_NOESCAPE) || ignore_stores)
> +	ret |= EAF_NOESCAPE;
> +    }
> +  return ret;
> +}
> +
> +/* Call statements may return their parameters.  Consider argument number
> +   ARG of USE_STMT and determine flags that can needs to be cleared
> +   in case pointer possibly indirectly references from ARG I is returned.
> +   KNOWN_FLAGS and DEPTH are same as in analyze_ssa_name_flags.  */
> +
> +static int
> +call_lhs_flags (gcall *call, int arg,
> +		vec<unsigned char> &known_flags, int depth)
> +{
> +  /* If there is no return value, no flags are affected.  */
> +  if (!gimple_call_lhs (call))
> +    return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
> +
> +  /* If we know that function returns given argument and it is not ARG
> +     we can still be happy.  */
> +  int flags = gimple_call_return_flags (call);
> +  if ((flags & ERF_RETURNS_ARG)
> +      && (flags & ERF_RETURN_ARG_MASK) != arg)
> +    return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
> +
> +  /* If return value is SSA name determine its flags.  */
> +  if (TREE_CODE (gimple_call_lhs (call)) == SSA_NAME)
> +    return analyze_ssa_name_flags
> +		       (gimple_call_lhs (call), known_flags,
> +			depth + 1);
> +  /* In the case of memory store we can do nothing.  */
> +  else
> +    return 0;
> +}
> +
> +/* Analyze EAF flags for SSA name NAME.
> +   KNOWN_FLAGS is a cache for flags we already determined.
> +   DEPTH is a recursion depth used to make debug output prettier.  */
> +
> +static int
> +analyze_ssa_name_flags (tree name, vec<unsigned char> &known_flags, int depth)
> +{
> +  imm_use_iterator ui;
> +  gimple *use_stmt;
> +  int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
> +
> +  /* See if value is already computed.  */
> +  if (known_flags[SSA_NAME_VERSION (name)])
> +    {
> +      /* Punt on cycles for now, so we do not need dataflow.  */
> +      if (known_flags[SSA_NAME_VERSION (name)] == 1)
> +	{
> +	  if (dump_file)
> +	    fprintf (dump_file,
> +		     "%*sGiving up on a cycle in SSA graph\n", depth * 4, "");
> +	  return 0;
> +	}
> +      return known_flags[SSA_NAME_VERSION (name)] - 2;
> +    }
> +  if (depth == param_modref_max_depth)
> +    {
> +      if (dump_file)
> +	fprintf (dump_file,
> +		 "%*sGiving up on max depth\n", depth * 4, "");
> +      return 0;
> +    }
> +  /* Recursion guard.  */
> +  known_flags[SSA_NAME_VERSION (name)] = 1;
> +
> +  if (dump_file)
> +    {
> +      fprintf (dump_file,
> +	       "%*sAnalyzing flags of ssa name: ", depth * 4, "");
> +      print_generic_expr (dump_file, name);
> +      fprintf (dump_file, "\n");
> +    }
> +
> +  FOR_EACH_IMM_USE_STMT (use_stmt, ui, name)
> +    {
> +      if (flags == 0)
> +	{
> +	  BREAK_FROM_IMM_USE_STMT (ui);
> +	}
> +      if (is_gimple_debug (use_stmt))
> +	continue;
> +      if (dump_file)
> +	{
> +	  fprintf (dump_file, "%*s  Analyzing stmt:", depth * 4, "");
> +	  print_gimple_stmt (dump_file, use_stmt, 0);
> +	}
> +
> +      /* Gimple return may load the return value.  */
> +      if (greturn *ret = dyn_cast <greturn *> (use_stmt))
> +	{
> +	  if (memory_access_to (gimple_return_retval (ret), name))
> +	    flags &= ~EAF_UNUSED;
> +	}
> +      /* Account for LHS store, arg loads and flags from callee function.  */
> +      else if (gcall *call = dyn_cast <gcall *> (use_stmt))
> +	{
> +	  tree callee = gimple_call_fndecl (call);
> +
> +	  /* Recursion would require bit of propagation; give up for now.  */
> +	  if (callee && recursive_call_p (current_function_decl, callee))
> +	    flags = 0;
> +	  else
> +	    {
> +	      int ecf_flags = gimple_call_flags (call);
> +	      bool ignore_stores = ignore_stores_p (current_function_decl,
> +						    ecf_flags);
> +
> +	      /* Handle *name = func (...).  */
> +	      if (gimple_call_lhs (call)
> +		  && memory_access_to (gimple_call_lhs (call), name))
> +		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> +
> +	      /* We do not track accesses to the static chain (we could)
> +		 so give up.  */
> +	      if (gimple_call_chain (call)
> +		  && (gimple_call_chain (call) == name))
> +		flags = 0;
> +
> +	      /* Handle all function parameters.  */
> +	      for (unsigned i = 0; i < gimple_call_num_args (call); i++)
> +		/* Name is directly passed to the callee.  */
> +		if (gimple_call_arg (call, i) == name)
> +		  {
> +		    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
> +		      flags &= ignore_stores
> +			       ? 0
> +			       : call_lhs_flags (call, i, known_flags, depth);
> +		    else
> +		      {
> +			int call_flags = gimple_call_arg_flags (call, i);
> +			if (ignore_stores)
> +			  call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE;
> +			else
> +			  call_flags &= call_lhs_flags (call, i,
> +							known_flags, depth);
> +
> +			flags &= call_flags;
> +		      }
> +		  }
> +		/* Name is dereferenced and passed to a callee.  */
> +		else if (memory_access_to (gimple_call_arg (call, i), name))
> +		  {
> +		    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
> +		      flags &= ~EAF_UNUSED;
> +		    else
> +		      flags &= deref_flags (gimple_call_arg_flags (call, i),
> +					    ignore_stores);
> +		    if (!ignore_stores)
> +		      flags &= call_lhs_flags (call, i, known_flags, depth);
> +		  }
> +	    }
> +	  /* Only NOCLOBBER or DIRECT flags alone are not useful (see comments
> +	     in tree-ssa-alias.c).  Give up earlier.  */
> +	  if ((flags & ~(EAF_DIRECT | EAF_NOCLOBBER)) == 0)
> +	    flags = 0;
> +	}
> +      else if (gimple_assign_load_p (use_stmt))
> +	{
> +	  gassign *assign = as_a <gassign *> (use_stmt);
> +	  /* Memory to memory copy.  */
> +	  if (gimple_store_p (assign))
> +	    {
> +	      /* Handle *name = *exp.  */
> +	      if (memory_access_to (gimple_assign_lhs (assign), name))
> +		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> +
> +	      /* Handle *lhs = *name.
> +
> +		 We do not track memory locations, so assume that value
> +		 is used arbitrarily.  */
> +	      if (memory_access_to (gimple_assign_rhs1 (assign), name))
> +		flags = 0;
> +	    }
> +	  /* Handle lhs = *name.  */
> +	  else if (memory_access_to (gimple_assign_rhs1 (assign), name))
> +	    flags &= deref_flags (analyze_ssa_name_flags
> +				      (gimple_assign_lhs (assign),
> +				       known_flags, depth + 1), false);
> +	}
> +      else if (gimple_store_p (use_stmt))
> +	{
> +	  gassign *assign = dyn_cast <gassign *> (use_stmt);
> +
> +	  /* Handle *lhs = name.  */
> +	  if (assign && gimple_assign_rhs1 (assign) == name)
> +	    {
> +	      if (dump_file)
> +		fprintf (dump_file, "%*s  ssa name saved to memory\n",
> +			 depth * 4, "");
> +	      flags = 0;
> +	    }
> +	  /* Handle *name = exp.  */
> +	  else if (assign
> +		   && memory_access_to (gimple_assign_lhs (assign), name))
> +	    flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> +	  /* ASM statements etc.  */
> +	  else if (!assign)
> +	    {
> +	      if (dump_file)
> +		fprintf (dump_file, "%*s  Unhandled store\n",
> +			 depth * 4, "");
> +	      flags = 0;
> +	    }
> +	}
> +      else if (gassign *assign = dyn_cast <gassign *> (use_stmt))
> +	{
> +	  enum tree_code code = gimple_assign_rhs_code (assign);
> +
> +	  /* See if operation is a merge as considered by
> +	     tree-ssa-structalias.c:find_func_aliases.  */
> +	  if (!truth_value_p (code)
> +	      && code != POINTER_DIFF_EXPR
> +	      && (code != POINTER_PLUS_EXPR
> +		  || gimple_assign_rhs1 (assign) == name))
> +	    flags &= analyze_ssa_name_flags
> +			       (gimple_assign_lhs (assign), known_flags,
> +				depth + 1);
> +	}
> +      else if (gphi *phi = dyn_cast <gphi *> (use_stmt))
> +	{
> +	  flags &= analyze_ssa_name_flags
> +			     (gimple_phi_result (phi), known_flags,
> +			      depth + 1);
> +	}
> +      /* Conditions are not considered escape points
> +	 by tree-ssa-structalias.  */
> +      else if (gimple_code (use_stmt) == GIMPLE_COND)
> +	;
> +      else
> +	{
> +	  if (dump_file)
> +	    fprintf (dump_file, "%*s  Unhandled stmt\n", depth * 4, "");
> +	  flags = 0;
> +	}
> +
> +      if (dump_file)
> +	{
> +	  fprintf (dump_file, "%*s  current flags of ", depth * 4, "");
> +	  print_generic_expr (dump_file, name);
> +	  dump_eaf_flags (dump_file, flags);
> +	}
> +    }
> +  if (dump_file)
> +    {
> +      fprintf (dump_file, "%*sflags of ssa name ", depth * 4, "");
> +      print_generic_expr (dump_file, name);
> +      dump_eaf_flags (dump_file, flags);
> +    }
> +  known_flags[SSA_NAME_VERSION (name)] = flags + 2;
> +  return flags;
> +}
> +
> +/* Determine EAF flags for function parameters.  */
> +
> +static void
> +analyze_parms (modref_summary *summary)
> +{
> +  unsigned int parm_index = 0;
> +  unsigned int count = 0;
> +
> +  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
> +       parm = TREE_CHAIN (parm))
> +    count++;
> +
> +  if (!count)
> +    return;
> +
> +  auto_vec<unsigned char> known_flags;
> +  known_flags.safe_grow_cleared (num_ssa_names, true);
> +
> +  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++,
> +       parm = TREE_CHAIN (parm))
> +    {
> +      tree name = ssa_default_def (cfun, parm);
> +      if (!name)
> +	continue;
> +      int flags = analyze_ssa_name_flags (name, known_flags, 0);
> +
> +      if (flags)
> +	{
> +	  if (parm_index >= summary->arg_flags.length ())
> +	    summary->arg_flags.safe_grow_cleared (count, true);
> +	  summary->arg_flags[parm_index] = flags;
> +	}
> +    }
> +}
> +
>  /* Analyze function F.  IPA indicates whether we're running in local mode
>     (false) or the IPA mode (true).  */
>  
> @@ -1174,6 +1531,10 @@ analyze_function (function *f, bool ipa)
>  				  param_modref_max_accesses);
>        summary_lto->writes_errno = false;
>      }
> +
> +  if (!ipa)
> +    analyze_parms (summary);
> +
>    int ecf_flags = flags_from_decl_or_type (current_function_decl);
>    auto_vec <gimple *, 32> recursive_calls;
>  
> @@ -1191,8 +1552,9 @@ analyze_function (function *f, bool ipa)
>  	      || ((!summary || !summary->useful_p (ecf_flags))
>  		  && (!summary_lto || !summary_lto->useful_p (ecf_flags))))
>  	    {
> -	      remove_summary (lto, nolto, ipa);
> -	      return;
> +	      collapse_loads (summary, summary_lto);
> +	      collapse_stores (summary, summary_lto);
> +	      break;
>  	    }
>  	}
>      }
> @@ -1957,7 +2319,7 @@ compute_parm_map (cgraph_edge *callee_edge, vec<modref_parm_map> *parm_map)
>  					: callee_edge->caller);
>        callee_pi = IPA_NODE_REF (callee);
>  
> -      (*parm_map).safe_grow_cleared (count);
> +      (*parm_map).safe_grow_cleared (count, true);
>  
>        for (i = 0; i < count; i++)
>  	{
> diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
> index 31ceffa8d34..59872301cd6 100644
> --- a/gcc/ipa-modref.h
> +++ b/gcc/ipa-modref.h
> @@ -29,6 +29,7 @@ struct GTY(()) modref_summary
>    /* Load and stores in function (transitively closed to all callees)  */
>    modref_records *loads;
>    modref_records *stores;
> +  auto_vec<unsigned char> GTY((skip)) arg_flags;
>  
>    modref_summary ();
>    ~modref_summary ();
> diff --git a/gcc/params.opt b/gcc/params.opt
> index a33a371a395..70152bf59bb 100644
> --- a/gcc/params.opt
> +++ b/gcc/params.opt
> @@ -931,6 +931,10 @@ Maximum number of accesse stored in each modref reference.
>  Common Joined UInteger Var(param_modref_max_tests) Init(64)
>  Maximum number of tests performed by modref query.
>  
> +-param=modref-max-depth=
> +Common Joined UInteger Var(param_modref_max_depth) Init(256)
> +Maximum depth of DFS walk used by modref escape analysis
> +
>  -param=tm-max-aggregate-size=
>  Common Joined UInteger Var(param_tm_max_aggregate_size) Init(9) Param Optimization
>  Size in bytes after which thread-local aggregates should be instrumented with the logging functions instead of save/restore pairs.
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend

Re: Detect EAF flags in ipa-modref

[H.J. Lu]

On Fri, Nov 13, 2020 at 12:07 AM Richard Biener <rguenther@suse.de> wrote:
>
> On Tue, 10 Nov 2020, Jan Hubicka wrote:
>
> > Hi,
> > here is updaed patch.
> >
> > Honza
> >
> > Bootstrapped/regtested x86_64-linux, OK (after the fnspec fixes)?
>
> OK.
>
> Thanks,
> Richard.
>

This caused:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97836

H.J.

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

> On Fri, Nov 13, 2020 at 12:07 AM Richard Biener <rguenther@suse.de> wrote:
> >
> > On Tue, 10 Nov 2020, Jan Hubicka wrote:
> >
> > > Hi,
> > > here is updaed patch.
> > >
> > > Honza
> > >
> > > Bootstrapped/regtested x86_64-linux, OK (after the fnspec fixes)?
> >
> > OK.
> >
> > Thanks,
> > Richard.
> >
> 
> This caused:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97836

I have pushed the following fix as obvious.  The problem is caused by
fact that I made ipa-modref to set EAF_UNUSED flag for parameters that
are used as scalars but not used to read memory since that suits the
use of the flag in tree-ssa-alias and mostly tree-ssa-structalias with
exception of the rule of escaping to return value.

At monday I will disuss with Richard if we want to adjust EAF_UNUSED or
invernt EAF_NOREAD, but this should prevent wrong code issues.

Honza

2020-11-15  Jan Hubicka  <hubicka@ucw.cz>

	PR ipa/97836
	* ipa-modref.c (analyze_ssa_name_flags): Make return to clear
	EAF_UNUSED flag.

gcc/testsuite/ChangeLog:

2020-11-15  Jan Hubicka  <hubicka@ucw.cz>

	* gcc.c-torture/execute/pr97836.c: New test.

diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 5273c200f00..4a43c50aa66 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -1224,10 +1224,12 @@ analyze_ssa_name_flags (tree name, vec<unsigned char> &known_flags, int depth)
 	  print_gimple_stmt (dump_file, use_stmt, 0);
 	}
 
-      /* Gimple return may load the return value.  */
+      /* Gimple return may load the return value.
+	 Returning name counts as an use by tree-ssa-structalias.c  */
       if (greturn *ret = dyn_cast <greturn *> (use_stmt))
 	{
-	  if (memory_access_to (gimple_return_retval (ret), name))
+	  if (memory_access_to (gimple_return_retval (ret), name)
+	      || name == gimple_return_retval (ret))
 	    flags &= ~EAF_UNUSED;
 	}
       /* Account for LHS store, arg loads and flags from callee function.  */
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr97836.c b/gcc/testsuite/gcc.c-torture/execute/pr97836.c
new file mode 100644
index 00000000000..4585e1fc69d
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr97836.c
@@ -0,0 +1,17 @@
+int a;
+
+int b(int c) { return 0; }
+
+static int *d(int *e) {
+  if (a) {
+    a = a && b(*e);
+  }
+  return e;
+}
+
+int main() {
+  int f;
+  if (d(&f) != &f)
+    __builtin_abort();
+  return 0;
+}

Re: Detect EAF flags in ipa-modref

[Andreas Schwab]

This breaks bootstrap with go.

../../gcc/go/gofrontend/go-diagnostics.cc: In function 'std::string expand_message(const char*, va_list)':
../../gcc/go/gofrontend/go-diagnostics.cc:110:61: error: '<anonymous>' may be used uninitialized [-Werror=maybe-uninitialized]
  110 |                      "memory allocation failed in vasprintf");
      |                                                             ^

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

> This breaks bootstrap with go.
> 
> ../../gcc/go/gofrontend/go-diagnostics.cc: In function 'std::string expand_message(const char*, va_list)':
> ../../gcc/go/gofrontend/go-diagnostics.cc:110:61: error: '<anonymous>' may be used uninitialized [-Werror=maybe-uninitialized]
>   110 |                      "memory allocation failed in vasprintf");
>       |                                                             ^

Well, this may be simply a false positive (except that the warning is
clearly mislocalted and cryptic) I was bootstrapping with go just few
minutes ago, so I wonder what configure flags you use?

Honza
> 
> Andreas.
> 
> -- 
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> "And now for something completely different."

Re: Detect EAF flags in ipa-modref

[Rainer Orth]

Hi Jan,

>> This breaks bootstrap with go.
>> 
>> ../../gcc/go/gofrontend/go-diagnostics.cc: In function 'std::string expand_message(const char*, va_list)':
>> ../../gcc/go/gofrontend/go-diagnostics.cc:110:61: error: '<anonymous>' may be used uninitialized [-Werror=maybe-uninitialized]
>>   110 |                      "memory allocation failed in vasprintf");
>>       |                                                             ^
>
> Well, this may be simply a false positive (except that the warning is
> clearly mislocalted and cryptic) I was bootstrapping with go just few
> minutes ago, so I wonder what configure flags you use?

I'm seeing the same on both i386-pc-solaris2.11 and
sparc-sun-solaris2.11, so I don't think there are special configure
flags involved.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

> Hi Jan,
> 
> >> This breaks bootstrap with go.
> >> 
> >> ../../gcc/go/gofrontend/go-diagnostics.cc: In function 'std::string expand_message(const char*, va_list)':
> >> ../../gcc/go/gofrontend/go-diagnostics.cc:110:61: error: '<anonymous>' may be used uninitialized [-Werror=maybe-uninitialized]
> >>   110 |                      "memory allocation failed in vasprintf");
> >>       |                                                             ^
> >
> > Well, this may be simply a false positive (except that the warning is
> > clearly mislocalted and cryptic) I was bootstrapping with go just few
> > minutes ago, so I wonder what configure flags you use?
> 
> I'm seeing the same on both i386-pc-solaris2.11 and
> sparc-sun-solaris2.11, so I don't think there are special configure
> flags involved.

When I configure with ../configure --enable-languages=go my build
eventually finishes fine on curren trunk:

/aux/hubicka/trunk-git/build-go/./gcc/gccgo -B/aux/hubicka/trunk-git/build-go/./gcc/ -B/usr/local/x86_64-pc-linux-gnu/bin/ -B/usr/local/x86_64-pc-linux-gnu/lib/ -isystem /usr/local/x86_64-pc-linux-gnu/include -isystem /usr/local/x86_64-pc-linux-gnu/sys-include   -g -O2 -I ../x86_64-pc-linux-gnu/libgo -static-libstdc++ -static-libgcc  -L ../x86_64-pc-linux-gnu/libgo -L ../x86_64-pc-linux-gnu/libgo/.libs -o cgo ../../gotools/../libgo/go/cmd/cgo/ast.go ../../gotools/../libgo/go/cmd/cgo/doc.go ../../gotools/../libgo/go/cmd/cgo/gcc.go ../../gotools/../libgo/go/cmd/cgo/godefs.go ../../gotools/../libgo/go/cmd/cgo/main.go ../../gotools/../libgo/go/cmd/cgo/out.go ../../gotools/../libgo/go/cmd/cgo/util.go zdefaultcc.go ../x86_64-pc-linux-gnu/libgo/libgotool.a
make[2]: Leaving directory '/aux/hubicka/trunk-git/build-go/gotools'
make[1]: Leaving directory '/aux/hubicka/trunk-git/build-go'

On Debian SLES machines.  Having preprocessed go-diagnostics.cc and
.uninit1 dump would probably help.

These are often false positives, but I saw similar warnings caused by
wrong EAF_UNUSED flag discovery which led to init code being optimized
out before inlining, so I would like to look into the dumps and figure
out if that is uninit acting funny or there is some real problem.

Honza
> 
> 	Rainer
> 
> -- 
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Detect EAF flags in ipa-modref

[Rainer Orth]

Hi Jan,

>> I'm seeing the same on both i386-pc-solaris2.11 and
>> sparc-sun-solaris2.11, so I don't think there are special configure
>> flags involved.
>
> When I configure with ../configure --enable-languages=go my build
> eventually finishes fine on curren trunk:
[...]
> On Debian SLES machines.  Having preprocessed go-diagnostics.cc and
> .uninit1 dump would probably help.

apart from go/diagnostics.cc, I see the warning several times in
libstdc++, but only for the 32-bit multilib.

Try building either for i686-pc-linux-gnu or a bi-arch
x86_64-pc-linux-gnu build.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

> Hi Jan,
> 
> >> I'm seeing the same on both i386-pc-solaris2.11 and
> >> sparc-sun-solaris2.11, so I don't think there are special configure
> >> flags involved.
> >
> > When I configure with ../configure --enable-languages=go my build
> > eventually finishes fine on curren trunk:
> [...]
> > On Debian SLES machines.  Having preprocessed go-diagnostics.cc and
> > .uninit1 dump would probably help.
> 
> apart from go/diagnostics.cc, I see the warning several times in
> libstdc++, but only for the 32-bit multilib.
> 
> Try building either for i686-pc-linux-gnu or a bi-arch
> x86_64-pc-linux-gnu build.

My build is bi-arch.  I will check libstdc++ warnings, but we had some
such warnings previously there too (plus we have tons of uninitialized
warnings with LTO bootstrap).

This really may depend on glibc headers and how string functions get
expanded.  This is why I think having preprocessed diagnotics.cc
should help.

Honza
> 
> 	Rainer
> 
> -- 
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Detect EAF flags in ipa-modref

[Rainer Orth]

Hi Jan,

>> > When I configure with ../configure --enable-languages=go my build
>> > eventually finishes fine on curren trunk:
>> [...]
>> > On Debian SLES machines.  Having preprocessed go-diagnostics.cc and
>> > .uninit1 dump would probably help.
>> 
>> apart from go/diagnostics.cc, I see the warning several times in
>> libstdc++, but only for the 32-bit multilib.
>> 
>> Try building either for i686-pc-linux-gnu or a bi-arch
>> x86_64-pc-linux-gnu build.
>
> My build is bi-arch.  I will check libstdc++ warnings, but we had some
> such warnings previously there too (plus we have tons of uninitialized
> warnings with LTO bootstrap).

seems to be an issue on 32-bit hosts: I get the same failure on
i686-pc-linux-gnu.

> This really may depend on glibc headers and how string functions get
> expanded.  This is why I think having preprocessed diagnotics.cc
> should help.

This is Solaris, so no glibc in sight ;-)

I'll send the Linux/i686 preprocessed source and uninit1 dump in private
mail.  Reproduce with

ccplus -fpreprocessed go-diagnostics.ii -quiet -mtune=generic -march=pentiumpro -O2 -Wextra -Wall -Werror -o go-diagnostics.s

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Detect EAF flags in ipa-modref

[Andreas Schwab]

See PR97840.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

> See PR97840.
Thanks,
this is a false positive where we fail to discover that pointed-to type
is empty on non-x86_64 targets.  This is triggered by better alias
analysis caused by non-escape discovery.

While this is not a full fix (I hope someone with more experience on
C++ types and warnings will set up) I think it may be useful to avoid
warning on unused parameter.

Bootstrapped/regtested x86_64-linux, OK?

	PR middle-end/97840
	* tree-ssa-uninit.c (maybe_warn_pass_by_reference): Update prototype;
	silence warning on EAF_UNUSED parameters.
	(warn_uninitialized_vars): Update.
diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
index f23514395e0..1e074793b02 100644
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -443,7 +443,7 @@ maybe_warn_operand (ao_ref &ref, gimple *stmt, tree lhs, tree rhs,
    access implying read access to those objects.  */
 
 static void
-maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims)
+maybe_warn_pass_by_reference (gcall *stmt, wlimits &wlims)
 {
   if (!wlims.wmaybe_uninit)
     return;
@@ -501,6 +501,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims)
 	      && !TYPE_READONLY (TREE_TYPE (argtype)))
 	    continue;
 
+	  /* Ignore args we are not going to read from.  */
+	  if (gimple_call_arg_flags (stmt, argno - 1) & EAF_UNUSED)
+	    continue;
+
 	  if (save_always_executed && access->mode == access_read_only)
 	    /* Attribute read_only arguments imply read access.  */
 	    wlims.always_executed = true;
@@ -639,8 +643,8 @@ warn_uninitialized_vars (bool wmaybe_uninit)
 	  if (gimple_vdef (stmt))
 	    wlims.vdef_cnt++;
 
-	  if (is_gimple_call (stmt))
-	    maybe_warn_pass_by_reference (stmt, wlims);
+	  if (gcall *call = dyn_cast <gcall *> (stmt))
+	    maybe_warn_pass_by_reference (call, wlims);
 	  else if (gimple_assign_load_p (stmt)
 		   && gimple_has_location (stmt))
 	    {

Re: Detect EAF flags in ipa-modref

[Richard Biener]

On Sun, 15 Nov 2020, Jan Hubicka wrote:

> > See PR97840.
> Thanks,
> this is a false positive where we fail to discover that pointed-to type
> is empty on non-x86_64 targets.  This is triggered by better alias
> analysis caused by non-escape discovery.
> 
> While this is not a full fix (I hope someone with more experience on
> C++ types and warnings will set up) I think it may be useful to avoid
> warning on unused parameter.
> 
> Bootstrapped/regtested x86_64-linux, OK?

OK.

> 	PR middle-end/97840
> 	* tree-ssa-uninit.c (maybe_warn_pass_by_reference): Update prototype;
> 	silence warning on EAF_UNUSED parameters.
> 	(warn_uninitialized_vars): Update.
> diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
> index f23514395e0..1e074793b02 100644
> --- a/gcc/tree-ssa-uninit.c
> +++ b/gcc/tree-ssa-uninit.c
> @@ -443,7 +443,7 @@ maybe_warn_operand (ao_ref &ref, gimple *stmt, tree lhs, tree rhs,
>     access implying read access to those objects.  */
>  
>  static void
> -maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims)
> +maybe_warn_pass_by_reference (gcall *stmt, wlimits &wlims)
>  {
>    if (!wlims.wmaybe_uninit)
>      return;
> @@ -501,6 +501,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims)
>  	      && !TYPE_READONLY (TREE_TYPE (argtype)))
>  	    continue;
>  
> +	  /* Ignore args we are not going to read from.  */
> +	  if (gimple_call_arg_flags (stmt, argno - 1) & EAF_UNUSED)
> +	    continue;
> +
>  	  if (save_always_executed && access->mode == access_read_only)
>  	    /* Attribute read_only arguments imply read access.  */
>  	    wlims.always_executed = true;
> @@ -639,8 +643,8 @@ warn_uninitialized_vars (bool wmaybe_uninit)
>  	  if (gimple_vdef (stmt))
>  	    wlims.vdef_cnt++;
>  
> -	  if (is_gimple_call (stmt))
> -	    maybe_warn_pass_by_reference (stmt, wlims);
> +	  if (gcall *call = dyn_cast <gcall *> (stmt))
> +	    maybe_warn_pass_by_reference (call, wlims);
>  	  else if (gimple_assign_load_p (stmt)
>  		   && gimple_has_location (stmt))
>  	    {
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend

Re: Detect EAF flags in ipa-modref

[Andreas Schwab]

On Nov 15 2020, Jan Hubicka wrote:

>> See PR97840.
> Thanks,
> this is a false positive where we fail to discover that pointed-to type
> is empty on non-x86_64 targets.  This is triggered by better alias
> analysis caused by non-escape discovery.
>
> While this is not a full fix (I hope someone with more experience on
> C++ types and warnings will set up) I think it may be useful to avoid
> warning on unused parameter.
>
> Bootstrapped/regtested x86_64-linux, OK?

That doesn't fix anything.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

> On Nov 15 2020, Jan Hubicka wrote:
> 
> >> See PR97840.
> > Thanks,
> > this is a false positive where we fail to discover that pointed-to type
> > is empty on non-x86_64 targets.  This is triggered by better alias
> > analysis caused by non-escape discovery.
> >
> > While this is not a full fix (I hope someone with more experience on
> > C++ types and warnings will set up) I think it may be useful to avoid
> > warning on unused parameter.
> >
> > Bootstrapped/regtested x86_64-linux, OK?
> 
> That doesn't fix anything.

It does silence the warning if you remove inline from function
declaration (as creduce while minimizing the testcase - the minimized
testcase was undefined that is why I did not include it at the end).
I now implemented one by hand.

The reason is that gimple_call_arg_flags clears EAF_UNUSED
on symbols that !binds_to_current_def_p because we are worried that
symbol will be interposed by different version of the body with
same semantics that will actually read the arg.
This is bit paranoid check since we optimize things like *a == *a early
but with clang *a will trap if a==NULL.

Richi, I think we can add "safe" parameter to gimple_call_arg_flags and
bypass this logic when we use it for warnings only (having body that
does not use the value is quite strong hint that it is unused by the
function).

I played with bit more testcases and found that we also want to disable
warning for const functions and sometimes EAF_UNUSED flag is dropped
becaue of clobber that is not necessary to do.  If function only clobber
the target it can be considered unused past inlining.

I am testing this improved patch and plan to commit if there are no
complains, but still we need to handle binds_to_current_def.

On the other direction, Martin, I think we may also warn for args
that are !EAF_UNUSED and !EAF_NOCLOBBER.  This will catch some cases
where user did not add "const" specifier to the declaration but
parameter is detected to be readonly.

I also noticed that we do not detect EAF_UNUSED for fully unused
parameters (with no SSA names associated with them).  I will fix that
incrementally.

Honza

	PR middle-end/97840
	* ipa-modref.c (analyze_ssa_name_flags): Skip clobbers if inlining
	is done.
	* tree-ssa-uninit.c (maybe_warn_pass_by_reference): Make stmt gcall;
	skip const calls and unused arguments.
	(warn_uninitialized_vars): Update prototype.

gcc/testsuite/ChangeLog:

2020-11-16  Jan Hubicka  <hubicka@ucw.cz>

	* g++.dg/warn/unit-2.C: New test.

diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 4a43c50aa66..08fcc36a321 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -1333,7 +1331,14 @@ analyze_ssa_name_flags (tree name, vec<unsigned char> &known_flags, int depth)
 	  /* Handle *name = exp.  */
 	  else if (assign
 		   && memory_access_to (gimple_assign_lhs (assign), name))
-	    flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+	    {
+	      /* In general we can not ignore clobbers because they are
+		 barriers for code motion, however after inlining it is safe to
+		 do because local optimization passes do not consider clobbers
+		 from other functions.  Similar logic is in ipa-pure-const.c.  */
+	      if (!cfun->after_inlining && !gimple_clobber_p (assign))
+		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+	    }
 	  /* ASM statements etc.  */
 	  else if (!assign)
 	    {
diff --git a/gcc/testsuite/g++.dg/warn/unit-2.C b/gcc/testsuite/g++.dg/warn/unit-2.C
new file mode 100644
index 00000000000..30f3ceae191
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/unit-2.C
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wmaybe-uninitialized" } */
+struct a {int a;};
+__attribute__ ((noinline))
+void
+nowarn (const struct a *ptr)
+{
+  if (ptr)
+    asm volatile ("");
+}
+void
+test()
+{
+  struct a ptr;
+  nowarn (&ptr);
+}
+__attribute__ ((noinline))
+int
+nowarn2 (const struct a *ptr, const struct a ptr2)
+{
+  return ptr != 0 || ptr2.a;
+}
+int mem;
+int
+test2()
+{
+  struct a ptr,ptr2={0};
+  return nowarn2 (&ptr, ptr2);
+}
diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
index f23514395e0..c94831bfb75 100644
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -443,7 +443,7 @@ maybe_warn_operand (ao_ref &ref, gimple *stmt, tree lhs, tree rhs,
    access implying read access to those objects.  */
 
 static void
-maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims)
+maybe_warn_pass_by_reference (gcall *stmt, wlimits &wlims)
 {
   if (!wlims.wmaybe_uninit)
     return;
@@ -457,6 +457,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims)
   if (!fntype)
     return;
 
+  /* Const function do not read their arguments.  */
+  if (gimple_call_flags (stmt) & ECF_CONST)
+    return;
+
   const built_in_function fncode
     = (fndecl && gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)
        ? DECL_FUNCTION_CODE (fndecl) : (built_in_function)BUILT_IN_LAST);
@@ -523,6 +527,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims)
 	   (but not definitive) read access.  */
 	wlims.always_executed = false;
 
+      /* Ignore args we are not going to read from.  */
+      if (gimple_call_arg_flags (stmt, argno - 1) & EAF_UNUSED)
+	continue;
+
       tree arg = gimple_call_arg (stmt, argno - 1);
 
       ao_ref ref;
@@ -639,8 +647,8 @@ warn_uninitialized_vars (bool wmaybe_uninit)
 	  if (gimple_vdef (stmt))
 	    wlims.vdef_cnt++;
 
-	  if (is_gimple_call (stmt))
-	    maybe_warn_pass_by_reference (stmt, wlims);
+	  if (gcall *call = dyn_cast <gcall *> (stmt))
+	    maybe_warn_pass_by_reference (call, wlims);
 	  else if (gimple_assign_load_p (stmt)
 		   && gimple_has_location (stmt))
 	    {

Re: Detect EAF flags in ipa-modref

[Richard Biener]

On Mon, 16 Nov 2020, Jan Hubicka wrote:

> > On Nov 15 2020, Jan Hubicka wrote:
> > 
> > >> See PR97840.
> > > Thanks,
> > > this is a false positive where we fail to discover that pointed-to type
> > > is empty on non-x86_64 targets.  This is triggered by better alias
> > > analysis caused by non-escape discovery.
> > >
> > > While this is not a full fix (I hope someone with more experience on
> > > C++ types and warnings will set up) I think it may be useful to avoid
> > > warning on unused parameter.
> > >
> > > Bootstrapped/regtested x86_64-linux, OK?
> > 
> > That doesn't fix anything.
> 
> It does silence the warning if you remove inline from function
> declaration (as creduce while minimizing the testcase - the minimized
> testcase was undefined that is why I did not include it at the end).
> I now implemented one by hand.
> 
> The reason is that gimple_call_arg_flags clears EAF_UNUSED
> on symbols that !binds_to_current_def_p because we are worried that
> symbol will be interposed by different version of the body with
> same semantics that will actually read the arg.
> This is bit paranoid check since we optimize things like *a == *a early
> but with clang *a will trap if a==NULL.
> 
> Richi, I think we can add "safe" parameter to gimple_call_arg_flags and
> bypass this logic when we use it for warnings only (having body that
> does not use the value is quite strong hint that it is unused by the
> function).

Eh, please not.

> 
> I played with bit more testcases and found that we also want to disable
> warning for const functions and sometimes EAF_UNUSED flag is dropped
> becaue of clobber that is not necessary to do.  If function only clobber
> the target it can be considered unused past inlining.
> 
> I am testing this improved patch and plan to commit if there are no
> complains, but still we need to handle binds_to_current_def.
> 
> On the other direction, Martin, I think we may also warn for args
> that are !EAF_UNUSED and !EAF_NOCLOBBER.  This will catch some cases
> where user did not add "const" specifier to the declaration but
> parameter is detected to be readonly.
> 
> I also noticed that we do not detect EAF_UNUSED for fully unused
> parameters (with no SSA names associated with them).  I will fix that
> incrementally.

Make sure to not apply it based on that reason to aggregates ;)

> Honza
> 
> 	PR middle-end/97840
> 	* ipa-modref.c (analyze_ssa_name_flags): Skip clobbers if inlining
> 	is done.
> 	* tree-ssa-uninit.c (maybe_warn_pass_by_reference): Make stmt gcall;
> 	skip const calls and unused arguments.
> 	(warn_uninitialized_vars): Update prototype.
> 
> gcc/testsuite/ChangeLog:
> 
> 2020-11-16  Jan Hubicka  <hubicka@ucw.cz>
> 
> 	* g++.dg/warn/unit-2.C: New test.
> 
> diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
> index 4a43c50aa66..08fcc36a321 100644
> --- a/gcc/ipa-modref.c
> +++ b/gcc/ipa-modref.c
> @@ -1333,7 +1331,14 @@ analyze_ssa_name_flags (tree name, vec<unsigned char> &known_flags, int depth)
>  	  /* Handle *name = exp.  */
>  	  else if (assign
>  		   && memory_access_to (gimple_assign_lhs (assign), name))
> -	    flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> +	    {
> +	      /* In general we can not ignore clobbers because they are
> +		 barriers for code motion, however after inlining it is safe to
> +		 do because local optimization passes do not consider clobbers
> +		 from other functions.  Similar logic is in ipa-pure-const.c.  */
> +	      if (!cfun->after_inlining && !gimple_clobber_p (assign))
> +		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> +	    }
>  	  /* ASM statements etc.  */
>  	  else if (!assign)
>  	    {
> diff --git a/gcc/testsuite/g++.dg/warn/unit-2.C b/gcc/testsuite/g++.dg/warn/unit-2.C
> new file mode 100644
> index 00000000000..30f3ceae191
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/unit-2.C
> @@ -0,0 +1,29 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wmaybe-uninitialized" } */
> +struct a {int a;};
> +__attribute__ ((noinline))
> +void
> +nowarn (const struct a *ptr)
> +{
> +  if (ptr)
> +    asm volatile ("");
> +}
> +void
> +test()
> +{
> +  struct a ptr;
> +  nowarn (&ptr);
> +}
> +__attribute__ ((noinline))
> +int
> +nowarn2 (const struct a *ptr, const struct a ptr2)
> +{
> +  return ptr != 0 || ptr2.a;
> +}
> +int mem;
> +int
> +test2()
> +{
> +  struct a ptr,ptr2={0};
> +  return nowarn2 (&ptr, ptr2);
> +}
> diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
> index f23514395e0..c94831bfb75 100644
> --- a/gcc/tree-ssa-uninit.c
> +++ b/gcc/tree-ssa-uninit.c
> @@ -443,7 +443,7 @@ maybe_warn_operand (ao_ref &ref, gimple *stmt, tree lhs, tree rhs,
>     access implying read access to those objects.  */
>  
>  static void
> -maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims)
> +maybe_warn_pass_by_reference (gcall *stmt, wlimits &wlims)
>  {
>    if (!wlims.wmaybe_uninit)
>      return;
> @@ -457,6 +457,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims)
>    if (!fntype)
>      return;
>  
> +  /* Const function do not read their arguments.  */
> +  if (gimple_call_flags (stmt) & ECF_CONST)
> +    return;
> +
>    const built_in_function fncode
>      = (fndecl && gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)
>         ? DECL_FUNCTION_CODE (fndecl) : (built_in_function)BUILT_IN_LAST);
> @@ -523,6 +527,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims)
>  	   (but not definitive) read access.  */
>  	wlims.always_executed = false;
>  
> +      /* Ignore args we are not going to read from.  */
> +      if (gimple_call_arg_flags (stmt, argno - 1) & EAF_UNUSED)
> +	continue;
> +
>        tree arg = gimple_call_arg (stmt, argno - 1);
>  
>        ao_ref ref;
> @@ -639,8 +647,8 @@ warn_uninitialized_vars (bool wmaybe_uninit)
>  	  if (gimple_vdef (stmt))
>  	    wlims.vdef_cnt++;
>  
> -	  if (is_gimple_call (stmt))
> -	    maybe_warn_pass_by_reference (stmt, wlims);
> +	  if (gcall *call = dyn_cast <gcall *> (stmt))
> +	    maybe_warn_pass_by_reference (call, wlims);
>  	  else if (gimple_assign_load_p (stmt)
>  		   && gimple_has_location (stmt))
>  	    {
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

> > 
> > Richi, I think we can add "safe" parameter to gimple_call_arg_flags and
> > bypass this logic when we use it for warnings only (having body that
> > does not use the value is quite strong hint that it is unused by the
> > function).
> 
> Eh, please not.

OK, I do not care that much as long as we do not have false positives
everywhere :)

Hadling EAF_UNUSED and CONST functions is necessary so we do not get
false positive caused by us optimizing code out.  In this case of not
trusing EAF_UNUSED flag we will not optimize, so I do not really care.

Martin, we collected very many warnings when building with
configure --with-build-config=bootstrap-lto.mk
This patch fixes some of them, but there are many others, can you take a
look?

For the testcase in PR I think it is enough to use default_is_empty_type
to disable the warning, but it is not clear to me why the code uses
default_is_empty_record at first place.
> 
> > 
> > I played with bit more testcases and found that we also want to disable
> > warning for const functions and sometimes EAF_UNUSED flag is dropped
> > becaue of clobber that is not necessary to do.  If function only clobber
> > the target it can be considered unused past inlining.
> > 
> > I am testing this improved patch and plan to commit if there are no
> > complains, but still we need to handle binds_to_current_def.
> > 
> > On the other direction, Martin, I think we may also warn for args
> > that are !EAF_UNUSED and !EAF_NOCLOBBER.  This will catch some cases
> > where user did not add "const" specifier to the declaration but
> > parameter is detected to be readonly.
> > 
> > I also noticed that we do not detect EAF_UNUSED for fully unused
> > parameters (with no SSA names associated with them).  I will fix that
> > incrementally.
> 
> Make sure to not apply it based on that reason to aggregates ;)

Sure, we already have detection of unused params in ipa-prop, so I guess
we want is_giple_ref (parm) && !default_def to imply EAF_UNUSED.

Honza

Re: Detect EAF flags in ipa-modref

[Martin Liška]

On 11/16/20 1:44 PM, Jan Hubicka wrote:
> Martin, we collected very many warnings when building with
> configure --with-build-config=bootstrap-lto.mk
> This patch fixes some of them, but there are many others, can you take a
> look?

Hello.

I guess you mean Martin Jambor, or me? Please CC :)

Martin

Re: Detect EAF flags in ipa-modref

[Jan Hubicka]

> On 11/16/20 1:44 PM, Jan Hubicka wrote:
> > Martin, we collected very many warnings when building with
> > configure --with-build-config=bootstrap-lto.mk
> > This patch fixes some of them, but there are many others, can you take a
> > look?
> 
> Hello.
> 
> I guess you mean Martin Jambor, or me? Please CC :)
Martin Sebor, added to CC (since he did most of work on tree-ssa-uninit
recently)

I filled some PRs on the isues now.
Honza
> 
> Martin

Re: Detect EAF flags in ipa-modref

[Richard Biener]

On Tue, 10 Nov 2020, Jan Hubicka wrote:

> > > +  tree callee = gimple_call_fndecl (stmt);
> > > +  if (callee)
> > > +    {
> > > +      cgraph_node *node = cgraph_node::get (callee);
> > > +      modref_summary *summary = node ? get_modref_function_summary (node)
> > > +				: NULL;
> > > +
> > > +      if (summary && summary->arg_flags.length () > arg)
> > 
> > So could we make modref "transform" push this as fnspec attribute or
> > would that not really be an optimization?
> 
> It was my original plan to synthetize fnspecs, but I think it is not
> very good idea: we have the summary readily available and we can
> represent information that fnspecs can't
> (do not have artificial limits on number of parameters or counts)
> 
> I would preffer fnspecs to be used only for in-compiler declarations.

Fine, I was just curious...

> > > +
> > > +/* Analyze EAF flags for SSA name NAME.
> > > +   KNOWN_FLAGS is a cache for flags we already determined.
> > > +   DEPTH is a recursion depth used to make debug output prettier.  */
> > > +
> > > +static int
> > > +analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth)
> > 
> > C++ has references which makes the access to known_flags nicer ;)
> 
> Yay, will chang that :)
> > 
> > > +{
> > > +  imm_use_iterator ui;
> > > +  gimple *use_stmt;
> > > +  int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
> > > +
> > > +  /* See if value is already computed.  */
> > > +  if ((*known_flags)[SSA_NAME_VERSION (name)])
> > > +    {
> > > +      /* Punt on cycles for now, so we do not need dataflow.  */
> > > +      if ((*known_flags)[SSA_NAME_VERSION (name)] == 1)
> > > +	{
> > > +	  if (dump_file)
> > > +	    fprintf (dump_file,
> > > +		     "%*sGiving up on a cycle in SSA graph\n", depth * 4, "");
> > > +	  return 0;
> > > +	}
> > > +      return (*known_flags)[SSA_NAME_VERSION (name)] - 2;
> > > +    }
> > > +  /* Recursion guard.  */
> > > +  (*known_flags)[SSA_NAME_VERSION (name)] = 1;
> > 
> > This also guards against multiple evaluations of the same stmts
> > but only in some cases?  Consider
> > 
> >   _1 = ..;
> >   _2 = _1 + _3;
> >   _4 = _1 + _5;
> >   _6 = _2 + _4;
> > 
> > where we visit _2 = and _4 = from _1 but from both are going
> > to visit _6.
> 
> Here we first push _6, then we go for _2 then for _1 evaluate _1,
> evalueate _2, go for _4 and evaluate _4, and evaluate _6.
> It is DFS and you need backward edge in DFS (comming from a PHI).

Hmm, but then we eventually evaluate _6 twice?

> 
> Cycles seems to somewhat matter for GCC: we do have a lot of functions
> that walk linked lists that we could track otherwise.
> > 
> > Maybe I'm blind but you're not limiting depth?  Guess that asks
> > for problems, esp. as you are recursing rather than using a
> > worklist or so?
> > 
> > I see you try to "optimize" the walk by only visiting def->use
> > links from parameters but then a RPO walk over all stmts would
> > be simpler iteration-wise ...
> We usually evaluate just small part of bigger functions (since we lose
> track quite easily after hitting first memory store).  My plan was to
> change this to actual dataflow once we have it well defined 
> (this means after discussing EAF flags with you and adding the logic to
> track callsites for true IPA pass that midly complicated things - for
> every ssa name I track callsite/arg pair where it is passed to
> either directly or indirectly.  Then this is translaed into call summary
> and used by IPA pass to compute final flags)
> 
> I guess I can add --param ipa-modref-walk-depth for now and handle
> dataflow incremntally?

Works for me.

> In particular I am not sure if I should just write iterated RPO myself
> or use tree-ssa-propagate.h (the second may be overkill).

tree-ssa-propagate.h is not to be used, it should DIE ;)

I guess you do want to iterate SSA cycles rather than BB cycles
so I suggest to re-surrect the SSA SCC discovery from the SCC
value-numbering (see tree-ssa-sccvn.c:DFS () on the gcc-8-branch)
which is non-recursive and micro-optimized.  Could put it
somewhere useful (tree-ssa.c?).

> > 
> > > +  if (dump_file)
> > > +    {
> > > +      fprintf (dump_file,
> > > +	       "%*sAnalyzing flags of ssa name: ", depth * 4, "");
> > > +      print_generic_expr (dump_file, name);
> > > +      fprintf (dump_file, "\n");
> > > +    }
> > > +
> > > +  FOR_EACH_IMM_USE_STMT (use_stmt, ui, name)
> > > +    {
> > > +      if (flags == 0)
> > > +	{
> > > +	  BREAK_FROM_IMM_USE_STMT (ui);
> > > +	}
> > > +      if (is_gimple_debug (use_stmt))
> > > +	continue;
> > > +      if (dump_file)
> > > +	{
> > > +	  fprintf (dump_file, "%*s  Analyzing stmt:", depth * 4, "");
> > > +	  print_gimple_stmt (dump_file, use_stmt, 0);
> > > +	}
> > > +
> > > +      /* Gimple return may load the return value.  */
> > > +      if (gimple_code (use_stmt) == GIMPLE_RETURN)
> > 
> >  if (greturn *ret = dyn_cast <greturn *> (use_stmt))
> > 
> > makes the as_a below not needed, similar for the other cases (it
> > also makes accesses cheaper, avoiding some checking code).
> 
> Looks cleaner indeed.
> > 
> > > +	{
> > > +	  if (memory_access_to (gimple_return_retval
> > > +				   (as_a <greturn *> (use_stmt)), name))
> > > +	    flags &= ~EAF_UNUSED;
> > > +	}
> > > +      /* Account for LHS store, arg loads and flags from callee function.  */
> > > +      else if (is_gimple_call (use_stmt))
> > > +	{
> > > +	  tree callee = gimple_call_fndecl (use_stmt);
> > > +
> > > +	  /* Recursion would require bit of propagation; give up for now.  */
> > > +	  if (callee && recursive_call_p (current_function_decl, callee))
> > > +	    flags = 0;
> > > +	  else
> > > +	    {
> > > +	      int ecf_flags = gimple_call_flags (use_stmt);
> > > +	      bool ignore_stores = ignore_stores_p (current_function_decl,
> > > +						    ecf_flags);
> > > +
> > > +	      /* Handle *name = func (...).  */
> > > +	      if (gimple_call_lhs (use_stmt)
> > > +		  && memory_access_to (gimple_call_lhs (use_stmt), name))
> > > +		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> > > +
> > > +	      /* Handle all function parameters.  */
> > > +	      for (unsigned i = 0; i < gimple_call_num_args (use_stmt); i++)
> > > +		/* Name is directly passed to the callee.  */
> > > +		if (gimple_call_arg (use_stmt, i) == name)
> > > +		  {
> > > +		    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
> > > +		      flags &= ignore_stores
> > > +			       ? 0
> > > +			       : call_lhs_flags (use_stmt, i,
> > > +						 known_flags, depth);
> > > +		    else
> > > +		      {
> > > +			int call_flags = gimple_call_arg_flags (as_a <gcall *>
> > > +								 (use_stmt), i);
> > > +			if (ignore_stores)
> > > +			  call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE;
> > > +			else
> > > +			  call_flags &= call_lhs_flags (use_stmt, i,
> > > +							known_flags, depth);
> > > +
> > > +			flags &= call_flags;
> > > +		      }
> > > +		  }
> > > +		/* Name is dereferenced and passed to a callee.  */
> > > +		else if (memory_access_to (gimple_call_arg (use_stmt, i), name))
> > > +		  {
> > > +		    if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
> > > +		      flags &= ~EAF_UNUSED;
> > > +		    else
> > > +		      flags &= deref_flags (gimple_call_arg_flags
> > > +						(as_a <gcall *> (use_stmt), i),
> > > +					    ignore_stores);
> > > +		    if (!ignore_stores)
> > > +		      flags &= call_lhs_flags (use_stmt, i, known_flags,
> > > +					       depth);
> > > +		  }
> > 
> > Hmm, you forget gimple_call_chain at least which is at least
> > argument passing?  Possibly the chain argument is never a function
> > parameter but how do you know... (partial inlining?)
> 
> Ah, right. I forgot about chain.
> I wil ladd it.
> > 
> > Then there's gimple_call_fn for indirect function calls to a
> > function parameter?
> 
> Well, it is never load or store, so I do not really care about it.
> for
> 
> void test (int (*foo)())
> {
>   foo();
> }
> 
> I should be able to derive EAF_UNUSED.
> 
> void test (int (**foo)())
> {
>   (*foo)();
> }
> 
> I will see sparate load.
> > 
> > I guess it would be nice to amend the gimple_walk_ stuff to have
> > a SSA name callback where the tree and the SSA use is passed.
> > Well, for another time.
> > 
> > > +	    }
> > > +	  /* Only NOCLOBBER or DIRECT flags alone are not useful (see comments
> > > +	     in tree-ssa-alias.c).  Give up earlier.  */
> > > +	  if ((flags & ~(EAF_DIRECT | EAF_NOCLOBBER)) == 0)
> > > +	    flags = 0;
> > > +	}
> > > +      else if (gimple_assign_load_p (use_stmt))
> > > +	{
> > > +	  /* Memory to memory copy.  */
> > > +	  if (gimple_store_p (use_stmt))
> > > +	    {
> > > +	      /* Handle *name = *exp.  */
> > > +	      if (memory_access_to (gimple_assign_lhs (use_stmt), name))
> > > +		flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> > > +
> > > +	      /* Handle *lhs = *name.
> > > +
> > > +		 We do not track memory locations, so assume that value
> > > +		 is used arbitrarily.  */
> > > +	      if (memory_access_to (gimple_assign_rhs1 (use_stmt), name))
> > > +		flags = 0;
> > > +	    }
> > > +	  /* Handle lhs = *name.  */
> > > +	  else if (memory_access_to (gimple_assign_rhs1 (use_stmt), name))
> > > +	    flags &= deref_flags (analyze_ssa_name_flags
> > > +				      (gimple_assign_lhs (use_stmt),
> > > +				       known_flags, depth + 1), false);
> > > +	}
> > > +      else if (gimple_store_p (use_stmt))
> > > +	{
> > > +	  /* Handle *lhs = name.  */
> > > +	  if (is_gimple_assign (use_stmt)
> > > +	      && gimple_assign_rhs1 (use_stmt) == name)
> > > +	    {
> > > +	      if (dump_file)
> > > +		fprintf (dump_file, "%*s  ssa name saved to memory\n",
> > > +			 depth * 4, "");
> > > +	      flags = 0;
> > > +	    }
> > > +	  /* Handle *name = exp.  */
> > > +	  else if (is_gimple_assign (use_stmt)
> > > +		   && memory_access_to (gimple_assign_lhs (use_stmt), name))
> > > +	    flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> > > +	  /* ASM statements etc.  */
> > > +	  else if (!is_gimple_assign (use_stmt))
> > > +	    {
> > > +	      if (dump_file)
> > > +		fprintf (dump_file, "%*s  Unhandled store\n",
> > > +			 depth * 4, "");
> > > +	      flags = 0;
> > > +	    }
> > > +	}
> > > +      else if (is_gimple_assign (use_stmt))
> > > +	{
> > > +	  enum tree_code code = gimple_assign_rhs_code (use_stmt);
> > > +
> > > +	  /* See if operation is a merge as considered by
> > > +	     tree-ssa-structalias.c:find_func_aliases.  */
> > > +	  if (!truth_value_p (code)
> > > +	      && code != POINTER_DIFF_EXPR
> > > +	      && (code != POINTER_PLUS_EXPR
> > > +		  || gimple_assign_rhs1 (use_stmt) == name))
> > > +	    flags &= analyze_ssa_name_flags
> > > +			       (gimple_assign_lhs (use_stmt), known_flags,
> > > +				depth + 1);
> > > +	}
> > > +      else if (gimple_code (use_stmt) == GIMPLE_PHI)
> > > +	{
> > > +	  flags &= analyze_ssa_name_flags
> > > +			     (gimple_phi_result (use_stmt), known_flags,
> > > +			      depth + 1);
> > > +	}
> > > +      /* Conditions are not considered escape points
> > > +	 by tree-ssa-structalias.  */
> > > +      else if (gimple_code (use_stmt) == GIMPLE_COND)
> > > +	;
> > > +      else
> > > +	{
> > > +	  if (dump_file)
> > > +	    fprintf (dump_file, "%*s  Unhandled stmt\n", depth * 4, "");
> > > +	  flags = 0;
> > > +	}
> > > +
> > > +      if (dump_file)
> > > +	{
> > > +	  fprintf (dump_file, "%*s  current flags of ", depth * 4, "");
> > > +	  print_generic_expr (dump_file, name);
> > > +	  dump_eaf_flags (dump_file, flags);
> > > +	}
> > > +    }
> > > +  if (dump_file)
> > > +    {
> > > +      fprintf (dump_file, "%*sflags of ssa name ", depth * 4, "");
> > > +      print_generic_expr (dump_file, name);
> > > +      dump_eaf_flags (dump_file, flags);
> > > +    }
> > > +  (*known_flags)[SSA_NAME_VERSION (name)] = flags + 2;
> > > +  return flags;
> > > +}
> > > +
> > > +/* Determine EAF flags for function parameters.  */
> > > +
> > > +static void
> > > +analyze_parms (modref_summary *summary)
> > > +{
> > > +  unsigned int parm_index = 0;
> > > +  unsigned int count = 0;
> > > +
> > > +  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
> > > +       parm = TREE_CHAIN (parm))
> > > +    count++;
> > > +
> > > +  if (!count)
> > > +    return;
> > > +
> > > +  auto_vec<int> known_flags;
> > > +  known_flags.safe_grow_cleared (num_ssa_names);
> > 
> > , true for exact reserve.  Could be space optimized by not using
> > auto_vec<int> but auto_vec<usigned char>?
> 
> I think there is not that much memory wasted, but indeed chars will be
> more effective.
> > 
> > > +
> > > +  for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++,
> > > +       parm = TREE_CHAIN (parm))
> > > +    {
> > > +      tree name = ssa_default_def (cfun, parm);
> > > +      if (!name)
> > > +	continue;
> > 
> > looks like the vec might be quite sparse ...
> > 
> > > +      int flags = analyze_ssa_name_flags (name, &known_flags, 0);
> > > +
> > > +      if (flags)
> > > +	{
> > > +	  if (parm_index >= summary->arg_flags.length ())
> > > +	    summary->arg_flags.safe_grow_cleared (count);
> > 
> > you want , true for exact reserve.
> > 
> > > +	  summary->arg_flags[parm_index] = flags;
> > 
> > maybe this as well - does it make sense to skip !is_gimple_reg_type
> > params in the counting?  Guess it makes lookup too complicated.
> > But we do have testcases with >30000 parameters ...
> 
> Well, the index needs to be actual index all call argument.
> As said, i did not see any noticeable modref memory use increase at WPA
> (I watched) since it is at most 1 byte for parameter in case something
> got tracked.
> 
> Thanks for review!
> 
> Honza
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend