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

On 09/12/15 11:01, Tom de Vries wrote:
[ 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.

And for that same reason, I've removed the same assert from find_func_aliases_for_builtin_call.

Committed to trunk as obvious.

Thanks,
- Tom
Remove invalid assert in find_func_aliases_for_builtin_call

2015-12-10  Tom de Vries  <t...@codesourcery.com>

	* tree-ssa-structalias.c (find_func_aliases_for_builtin_call): Remove
	invalid assert.

---
 gcc/tree-ssa-structalias.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 15e351e..c350862 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -4533,7 +4533,6 @@ find_func_aliases_for_builtin_call (struct function *fn, gcall *t)
 	      gcc_assert (TREE_CODE (fnarg) == ADDR_EXPR);
 	      tree fndecl = TREE_OPERAND (fnarg, 0);
 	      tree arg = gimple_call_arg (t, argpos);
-	      gcc_assert (TREE_CODE (arg) == ADDR_EXPR);
 
 	      varinfo_t fi = get_vi_for_tree (fndecl);
 	      find_func_aliases_for_call_arg (fi, 0, arg);

Reply via email to