| Message ID | 20141121201926.GC63468@msticlxl57.ims.intel.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Nov 21, 2014 at 11:19:26PM +0300, Ilya Verbin wrote: > Hi, > > '#pragma omp critical (name)' can be placed in the function, marked > with '#pragma omp declare target', in this case the corresponding node > should be marked as offloadable too. > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Please add a testcase for this. > * omp-low.c (lower_omp_critical): Mark critical sections > inside target functions as offloadable. > > > diff --git a/gcc/omp-low.c b/gcc/omp-low.c > index 3924282..6c5774c 100644 > --- a/gcc/omp-low.c > +++ b/gcc/omp-low.c > @@ -9366,16 +9366,6 @@ lower_omp_critical (gimple_stmt_iterator *gsi_p, omp_context *ctx) > DECL_ARTIFICIAL (decl) = 1; > DECL_IGNORED_P (decl) = 1; > > - /* If '#pragma omp critical' is inside target region, the symbol must > - be marked for offloading. */ > - omp_context *octx; > - for (octx = ctx->outer; octx; octx = octx->outer) > - if (is_targetreg_ctx (octx)) > - { > - varpool_node::get_create (decl)->offloadable = 1; > - break; > - } > - > varpool_node::finalize_decl (decl); > > critical_name_mutexes->put (name, decl); > @@ -9383,6 +9373,20 @@ lower_omp_critical (gimple_stmt_iterator *gsi_p, omp_context *ctx) > else > decl = *n; > > + /* If '#pragma omp critical' is inside target region or > + inside function marked as offloadable, the symbol must be > + marked as offloadable too. */ > + omp_context *octx; > + if (cgraph_node::get (current_function_decl)->offloadable) > + varpool_node::get_create (decl)->offloadable = 1; > + else > + for (octx = ctx->outer; octx; octx = octx->outer) > + if (is_targetreg_ctx (octx)) > + { > + varpool_node::get_create (decl)->offloadable = 1; > + break; > + } > + > lock = builtin_decl_explicit (BUILT_IN_GOMP_CRITICAL_NAME_START); > lock = build_call_expr_loc (loc, lock, 1, build_fold_addr_expr_loc (loc, decl)); > Jakub
> On 21 Nov 2014, at 23:36, Jakub Jelinek <jakub@redhat.com> wrote: > >> On Fri, Nov 21, 2014 at 11:19:26PM +0300, Ilya Verbin wrote: >> Hi, >> >> '#pragma omp critical (name)' can be placed in the function, marked >> with '#pragma omp declare target', in this case the corresponding node >> should be marked as offloadable too. >> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > Please add a testcase for this. By default with disabled offloading it will always PASS. Add anyway? -- Ilya
On Fri, Nov 21, 2014 at 1:08 PM, Ilya Verbin <iverbin@gmail.com> wrote: >> On 21 Nov 2014, at 23:36, Jakub Jelinek <jakub@redhat.com> wrote: >> >>> On Fri, Nov 21, 2014 at 11:19:26PM +0300, Ilya Verbin wrote: >>> Hi, >>> >>> '#pragma omp critical (name)' can be placed in the function, marked >>> with '#pragma omp declare target', in this case the corresponding node >>> should be marked as offloadable too. >>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? >> >> Please add a testcase for this. > > By default with disabled offloading it will always PASS. Add anyway? > Have you fixed the offloading issue with binutils 2.25?
On Sat, Nov 22, 2014 at 12:08:38AM +0300, Ilya Verbin wrote: > > On 21 Nov 2014, at 23:36, Jakub Jelinek <jakub@redhat.com> wrote: > > > >> On Fri, Nov 21, 2014 at 11:19:26PM +0300, Ilya Verbin wrote: > >> Hi, > >> > >> '#pragma omp critical (name)' can be placed in the function, marked > >> with '#pragma omp declare target', in this case the corresponding node > >> should be marked as offloadable too. > >> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > > > Please add a testcase for this. > > By default with disabled offloading it will always PASS. Add anyway? Doesn't matter. We want to test it both with host fallback and offloaded to various devices. The latter of course will be tested only if somebody configures the 2 compilers that way, less often, but still. Jakub
> On 22 Nov 2014, at 00:11, H.J. Lu <hjl.tools@gmail.com> wrote: > > Have you fixed the offloading issue with binutils 2.25? No, I'm still thinking how to make a patch better than the former... Probably will send it on Monday. (Regressions in make check disappeared after disabling offload IR in default configuration.) -- Ilya
diff --git a/gcc/omp-low.c b/gcc/omp-low.c index 3924282..6c5774c 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -9366,16 +9366,6 @@ lower_omp_critical (gimple_stmt_iterator *gsi_p, omp_context *ctx) DECL_ARTIFICIAL (decl) = 1; DECL_IGNORED_P (decl) = 1; - /* If '#pragma omp critical' is inside target region, the symbol must - be marked for offloading. */ - omp_context *octx; - for (octx = ctx->outer; octx; octx = octx->outer) - if (is_targetreg_ctx (octx)) - { - varpool_node::get_create (decl)->offloadable = 1; - break; - } - varpool_node::finalize_decl (decl); critical_name_mutexes->put (name, decl); @@ -9383,6 +9373,20 @@ lower_omp_critical (gimple_stmt_iterator *gsi_p, omp_context *ctx) else decl = *n; + /* If '#pragma omp critical' is inside target region or + inside function marked as offloadable, the symbol must be + marked as offloadable too. */ + omp_context *octx; + if (cgraph_node::get (current_function_decl)->offloadable) + varpool_node::get_create (decl)->offloadable = 1; + else + for (octx = ctx->outer; octx; octx = octx->outer) + if (is_targetreg_ctx (octx)) + { + varpool_node::get_create (decl)->offloadable = 1; + break; + } + lock = builtin_decl_explicit (BUILT_IN_GOMP_CRITICAL_NAME_START); lock = build_call_expr_loc (loc, lock, 1, build_fold_addr_expr_loc (loc, decl));