Message ID | 566AC080.8080500@mentor.com |
---|---|
State | New |
Headers | show |
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.
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
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);