diff mbox

Handle sizes and kinds params of GOACC_paralllel in find_func_clobbers

Message ID 566AC080.8080500@mentor.com
State New
Headers show

Commit Message

Tom de Vries Dec. 11, 2015, 12:24 p.m. UTC 
Hi,

while testing the oacc kernels patch series on top of trunk, using the 
optimal handling of BUILTIN_IN_GOACC_PARALLEL in fipa-pta  I ran into a 
failure where the stores to the omp_data_sizes array were removed by dse.

The call bb in the failing testcase normally looks like this:
...
   <bb 3>:
   .omp_data_arr.10.D.2550 = c.2_18;
   .omp_data_arr.10.c = &c;
   .omp_data_arr.10.D.2553 = b.1_15;
   .omp_data_arr.10.b = &b;
   .omp_data_arr.10.D.2556 = a.0_11;
   .omp_data_arr.10.a = &a;
    D.2572 = n_6(D);
   .omp_data_arr.10.n = &D.2572;
   .omp_data_sizes.11[0] = _8;
   .omp_data_sizes.11[1] = 0;
   .omp_data_sizes.11[2] = _8;
   .omp_data_sizes.11[3] = 0;
   .omp_data_sizes.11[4] = _8;
   .omp_data_sizes.11[5] = 0;
   .omp_data_sizes.11[6] = 4;
   __builtin_GOACC_parallel_keyed (-1, foo._omp_fn.0, 7,
                                   &.omp_data_arr.10,
                                   &.omp_data_sizes.11,
                                   &.omp_data_kinds.12, 0);
...

Dse removed the stores, because omp_data_sizes was not marked as a used 
by __builtin_GOACC_parallel_keyed.

We pretend in fipa-pta that __builtin_GOACC_parallel_keyed is never 
called, and instead handle the call foo._omp_fn.0 (&.omp_data_arr.10). 
That means the use of omp_data_sizes by __builtin_GOACC_parallel_keyed 
is ignored.

This patch fixes that (for both sizes and kinds arrays), as confirmed 
with a test run of target-libgomp c.exp on the accelerator.

OK for stage3 if bootstrap and reg-test succeeds?

Thanks,
- Tom

Comments

Richard Biener Dec. 11, 2015, 12:31 p.m. UTC | #1 
On Fri, 11 Dec 2015, Tom de Vries wrote:

> Hi,
> 
> while testing the oacc kernels patch series on top of trunk, using the optimal
> handling of BUILTIN_IN_GOACC_PARALLEL in fipa-pta  I ran into a failure where
> the stores to the omp_data_sizes array were removed by dse.
> 
> The call bb in the failing testcase normally looks like this:
> ...
>   <bb 3>:
>   .omp_data_arr.10.D.2550 = c.2_18;
>   .omp_data_arr.10.c = &c;
>   .omp_data_arr.10.D.2553 = b.1_15;
>   .omp_data_arr.10.b = &b;
>   .omp_data_arr.10.D.2556 = a.0_11;
>   .omp_data_arr.10.a = &a;
>    D.2572 = n_6(D);
>   .omp_data_arr.10.n = &D.2572;
>   .omp_data_sizes.11[0] = _8;
>   .omp_data_sizes.11[1] = 0;
>   .omp_data_sizes.11[2] = _8;
>   .omp_data_sizes.11[3] = 0;
>   .omp_data_sizes.11[4] = _8;
>   .omp_data_sizes.11[5] = 0;
>   .omp_data_sizes.11[6] = 4;
>   __builtin_GOACC_parallel_keyed (-1, foo._omp_fn.0, 7,
>                                   &.omp_data_arr.10,
>                                   &.omp_data_sizes.11,
>                                   &.omp_data_kinds.12, 0);
> ...
> 
> Dse removed the stores, because omp_data_sizes was not marked as a used by
> __builtin_GOACC_parallel_keyed.
> 
> We pretend in fipa-pta that __builtin_GOACC_parallel_keyed is never called,
> and instead handle the call foo._omp_fn.0 (&.omp_data_arr.10). That means the
> use of omp_data_sizes by __builtin_GOACC_parallel_keyed is ignored.
> 
> This patch fixes that (for both sizes and kinds arrays), as confirmed with a
> test run of target-libgomp c.exp on the accelerator.
> 
> OK for stage3 if bootstrap and reg-test succeeds?

Ok, though techincally they are used by the OMP runtime (but this we
could only represent by letting them escape).  I wonder what can of
worms we'd open if you LTO the OMP runtime in ... (and thus
builtins map to real functions!)

Thanks,
Richard.
Tom de Vries Dec. 14, 2015, 8:24 a.m. UTC | #2 
On 11/12/15 13:31, Richard Biener wrote:
> On Fri, 11 Dec 2015, Tom de Vries wrote:
>
>> Hi,
>>
>> while testing the oacc kernels patch series on top of trunk, using the optimal
>> handling of BUILTIN_IN_GOACC_PARALLEL in fipa-pta  I ran into a failure where
>> the stores to the omp_data_sizes array were removed by dse.
>>
>> The call bb in the failing testcase normally looks like this:
>> ...
>>    <bb 3>:
>>    .omp_data_arr.10.D.2550 = c.2_18;
>>    .omp_data_arr.10.c = &c;
>>    .omp_data_arr.10.D.2553 = b.1_15;
>>    .omp_data_arr.10.b = &b;
>>    .omp_data_arr.10.D.2556 = a.0_11;
>>    .omp_data_arr.10.a = &a;
>>     D.2572 = n_6(D);
>>    .omp_data_arr.10.n = &D.2572;
>>    .omp_data_sizes.11[0] = _8;
>>    .omp_data_sizes.11[1] = 0;
>>    .omp_data_sizes.11[2] = _8;
>>    .omp_data_sizes.11[3] = 0;
>>    .omp_data_sizes.11[4] = _8;
>>    .omp_data_sizes.11[5] = 0;
>>    .omp_data_sizes.11[6] = 4;
>>    __builtin_GOACC_parallel_keyed (-1, foo._omp_fn.0, 7,
>>                                    &.omp_data_arr.10,
>>                                    &.omp_data_sizes.11,
>>                                    &.omp_data_kinds.12, 0);
>> ...
>>
>> Dse removed the stores, because omp_data_sizes was not marked as a used by
>> __builtin_GOACC_parallel_keyed.
>>
>> We pretend in fipa-pta that __builtin_GOACC_parallel_keyed is never called,
>> and instead handle the call foo._omp_fn.0 (&.omp_data_arr.10). That means the
>> use of omp_data_sizes by __builtin_GOACC_parallel_keyed is ignored.
>>
>> This patch fixes that (for both sizes and kinds arrays), as confirmed with a
>> test run of target-libgomp c.exp on the accelerator.
>>
>> OK for stage3 if bootstrap and reg-test succeeds?
>
> Ok, though techincally they are used by the OMP runtime (but this we
> could only represent by letting them escape).  I wonder what can of
> worms we'd open if you LTO the OMP runtime in ... (and thus
> builtins map to real functions!)

I guess for fipa-pta, when encoutering a call to the built-in, we could 
add the equivalent of initial constraints to the runtime function.

I'd also imagine we don't want the built-in to be inlined,  since that 
would break the optimal treatment of the built-in.

Thanks,
- Tom
diff mbox

Patch

Handle sizes and kinds params of GOACC_paralllel in find_func_clobbers

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

	* tree-ssa-structalias.c (find_func_clobbers): Handle sizes and kinds
	parameters of GOACC_paralllel.

---
 gcc/tree-ssa-structalias.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index dfc0422..98d7d7b 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -5089,6 +5089,8 @@  find_func_clobbers (struct function *fn, gimple *origt)
 	  case BUILT_IN_GOACC_PARALLEL:
 	    {
 	      unsigned int fnpos, argpos;
+	      unsigned int implicit_use_args[2];
+	      unsigned int num_implicit_use_args = 0;
 	      switch (DECL_FUNCTION_CODE (decl))
 		{
 		case BUILT_IN_GOMP_PARALLEL:
@@ -5101,6 +5103,8 @@  find_func_clobbers (struct function *fn, gimple *origt)
 					       sizes, kinds, ...).  */
 		  fnpos = 1;
 		  argpos = 3;
+		  implicit_use_args[num_implicit_use_args++] = 4;
+		  implicit_use_args[num_implicit_use_args++] = 5;
 		  break;
 		default:
 		  gcc_unreachable ();
@@ -5121,6 +5125,18 @@  find_func_clobbers (struct function *fn, gimple *origt)
 		process_constraint (new_constraint (lhs, *rhsp));
 	      rhsc.truncate (0);
 
+	      /* Handle parameters used by the call, but not used in cfi, as
+		 implicitly used by cfi.  */
+	      lhs = get_function_part_constraint (cfi, fi_uses);
+	      for (unsigned i = 0; i < num_implicit_use_args; ++i)
+		{
+		  tree arg = gimple_call_arg (t, implicit_use_args[i]);
+		  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);