[PATCH, PR68716] Fix GOMP/GOACC_parallel handling in find_func_clobbers

[Tom de Vries <Tom_deVries at mentor dot com>]

[ was: Re: [PATCH, PR46032] Handle BUILT_IN_GOMP_PARALLEL in ipa-pta ]

On 30/11/15 13:11, Tom de Vries wrote:
> On 30/11/15 10:16, Richard Biener wrote:
> > On Mon, 30 Nov 2015, Tom de Vries wrote:
> >
> > > Hi,
> > >
> > > this patch fixes PR46032.
> > >
> > > It handles a call:
> > > ...
> > >    __builtin_GOMP_parallel (fn, data, num_threads, flags)
> > > ...
> > > as:
> > > ...
> > >    fn (data)
> > > ...
> > > in ipa-pta.
> > >
> > > This improves ipa-pta alias analysis in the parallelized function fn,
> > > and
> > > allows vectorization in the testcase without a runtime alias test.
> > >
> > > Bootstrapped and reg-tested on x86_64.
> > >
> > > OK for stage3 trunk?
> >
> > +             /* Assign the passed argument to the appropriate incoming
> > +                parameter of the function.  */
> > +             struct constraint_expr lhs ;
> > +             lhs = get_function_part_constraint (fi, fi_parm_base + 0);
> > +             auto_vec<ce_s, 2> rhsc;
> > +             struct constraint_expr *rhsp;
> > +             get_constraint_for_rhs (arg, &rhsc);
> > +             while (rhsc.length () != 0)
> > +               {
> > +                 rhsp = &rhsc.last ();
> > +                 process_constraint (new_constraint (lhs, *rhsp));
> > +                 rhsc.pop ();
> > +               }
> >
> > please use style used elsewhere with
> >
> >               FOR_EACH_VEC_ELT (rhsc, j, rhsp)
> >                 process_constraint (new_constraint (lhs, *rhsp));
> >               rhsc.truncate (0);
>
> That code was copied from find_func_aliases_for_call.
> I've factored out the bit that I copied as
> find_func_aliases_for_call_arg, and fixed the style there (and dropped
> 'rhsc.truncate (0)' since AFAIU it's redundant at the end of a function).
>
> > +             /* Parameter passed by value is used.  */
> > +             lhs = get_function_part_constraint (fi, fi_uses);
> > +             struct constraint_expr *rhsp;
> > +             get_constraint_for_address_of (arg, &rhsc);
> >
> > This isn't correct - you want to use get_constraint_for (arg, &rhsc).
> > After all rhs is already an ADDR_EXPR.
>
> Can we add an assert somewhere to detect this incorrect usage?
>
> > +             FOR_EACH_VEC_ELT (rhsc, j, rhsp)
> > +               process_constraint (new_constraint (lhs, *rhsp));
> > +             rhsc.truncate (0);
> > +
> > +             /* The caller clobbers what the callee does.  */
> > +             lhs = get_function_part_constraint (fi, fi_clobbers);
> > +             rhs = get_function_part_constraint (cfi, fi_clobbers);
> > +             process_constraint (new_constraint (lhs, rhs));
> > +
> > +             /* The caller uses what the callee does.  */
> > +             lhs = get_function_part_constraint (fi, fi_uses);
> > +             rhs = get_function_part_constraint (cfi, fi_uses);
> > +             process_constraint (new_constraint (lhs, rhs));
> >
> > I don't see why you need those.  The solver should compute these
> > in even better precision (context sensitive on the call side).
> >
> > The same is true for the function parameter.  That is, the only
> > needed part of the patch should be that making sure we see
> > the "direct" call and assign parameters correctly.
>
> Dropped this bit.

Dropping the find_func_clobbers bit (in particular, the part copying the 
clobbers) caused PR68716.

Basically the find_func_clobbers bit tries to emulate the generic 
handling of calls in find_func_clobbers, but pretends the call is
  fn (data)
when handling
  __builtin_GOMP_parallel (fn, data, num_threads, flags)
.

I used the same comments as in the generic call handling to make it 
clear what part of the generic call handling is emulated.

Compared to the originally posted patch, the changes are:
- removed 'gcc_assert (TREE_CODE (arg) == ADDR_EXPR)'. Ran into a case
  where arg is NULL.
- use get_constraint_for instead of get_constraint_for_address_of as
  requested in a review comment above.
- handle GOACC_parallel as well

Bootstrapped and reg-tested on x86_64.

OK for stage3 trunk?

Thanks,
- Tom

Fix GOMP/GOACC_parallel handling in find_func_clobbers

2015-12-08  Tom de Vries  <tom@codesourcery.com>

	PR tree-optimization/68716
	* tree-ssa-structalias.c (find_func_clobbers): Fix handling of
	BUILT_IN_GOMP_PARALLEL and BUILT_IN_GOMP_PARALLEL.

	* testsuite/libgomp.c/omp-nested-2.c: New test.

---
 gcc/tree-ssa-structalias.c                 | 47 +++++++++++++++++++++++++++++-
 libgomp/testsuite/libgomp.c/omp-nested-2.c |  4 +++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 060ff3e..15e351e 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5082,7 +5082,52 @@ find_func_clobbers (struct function *fn, gimple *origt)
 	    return;
 	  case BUILT_IN_GOMP_PARALLEL:
 	  case BUILT_IN_GOACC_PARALLEL:
-	    return;
+	    {
+	      unsigned int fnpos, argpos;
+	      switch (DECL_FUNCTION_CODE (decl))
+		{
+		case BUILT_IN_GOMP_PARALLEL:
+		  /* __builtin_GOMP_parallel (fn, data, num_threads, flags).  */
+		  fnpos = 0;
+		  argpos = 1;
+		  break;
+		case BUILT_IN_GOACC_PARALLEL:
+		  /* __builtin_GOACC_parallel (device, fn, mapnum, hostaddrs,
+					       sizes, kinds, ...).  */
+		  fnpos = 1;
+		  argpos = 3;
+		  break;
+		default:
+		  gcc_unreachable ();
+		}
+
+	      tree fnarg = gimple_call_arg (t, fnpos);
+	      gcc_assert (TREE_CODE (fnarg) == ADDR_EXPR);
+	      tree fndecl = TREE_OPERAND (fnarg, 0);
+	      varinfo_t cfi = get_vi_for_tree (fndecl);
+
+	      tree arg = gimple_call_arg (t, argpos);
+
+	      /* Parameter passed by value is used.  */
+	      lhs = get_function_part_constraint (fi, fi_uses);
+	      struct constraint_expr *rhsp;
+	      get_constraint_for (arg, &rhsc);
+	      FOR_EACH_VEC_ELT (rhsc, j, rhsp)
+		process_constraint (new_constraint (lhs, *rhsp));
+	      rhsc.truncate (0);
+
+	      /* The caller clobbers what the callee does.  */
+	      lhs = get_function_part_constraint (fi, fi_clobbers);
+	      rhs = get_function_part_constraint (cfi, fi_clobbers);
+	      process_constraint (new_constraint (lhs, rhs));
+
+	      /* The caller uses what the callee does.  */
+	      lhs = get_function_part_constraint (fi, fi_uses);
+	      rhs = get_function_part_constraint (cfi, fi_uses);
+	      process_constraint (new_constraint (lhs, rhs));
+
+	      return;
+	    }
 	  /* printf-style functions may have hooks to set pointers to
 	     point to somewhere into the generated string.  Leave them
 	     for a later exercise...  */
diff --git a/libgomp/testsuite/libgomp.c/omp-nested-2.c b/libgomp/testsuite/libgomp.c/omp-nested-2.c
new file mode 100644
index 0000000..7495afb
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/omp-nested-2.c
@@ -0,0 +1,4 @@
+// { dg-do run }
+// { dg-additional-options "-fipa-pta" }
+
+#include "omp-nested-1.c"