On Sun, Jan 11, 2015 at 11:06:52PM +0300, Ilya Verbin wrote:
> On 09 Jan 16:02, Jakub Jelinek wrote:
> > On Fri, Jan 09, 2015 at 05:57:02PM +0300, Ilya Verbin wrote:
> > > If one (by mistake) calls a non-target function from the target region,
> > > the
> > > offload compiler crashes in input_overwrite_node. This is because
> > > compute_ltrans_boundary during streaming-out includes to SET the
> > > non-offloadable nodes, called from offloadable nodes.
> > > Probably it's possible to ignore such incorrect nodes (and edges) in
> > > streaming-out, but such a situation can not appear in a correct OpenMP 4.0
> > > program, therefore I've added a check to scan_omp_1_stmt.
> >
> > Unlike variables, the spec last time I've checked isn't all that clear about
> > that.
>
> I think that GCC shouldn't allow such calls, at least for non-external
> functions. Otherwise why the 'declare target' directive is needed at all?
> As for external functions, it's an open question, e.g. for MIC it's OK to have
> in target region a call to a function from a native library, like printf.
>
> > > --- a/gcc/omp-low.c
> > > +++ b/gcc/omp-low.c
> > > @@ -2818,6 +2818,19 @@ scan_omp_1_stmt (gimple_stmt_iterator *gsi, bool
> > > *handled_ops_p,
> > > default:
> > > break;
> > > }
> > > + else if (!DECL_EXTERNAL (fndecl)
> > > + && !cgraph_node::get_create (fndecl)->offloadable)
> >
> > What about if fndecl is defined in the current TU, but as global symbol and
> > can be
> > interposed (e.g. is in a shared library and not hidden in there), the local
> > function
> > definition is without target attribute but the definition used at runtime
> > is not?
>
> I believe that if it is defined in the current TU and is used in a target
> region, then it should be declared as target in current TU too.
>
> > > + {
> > > + omp_context *octx;
> > > + if (cgraph_node::get (current_function_decl)->offloadable)
> > > + remove = true;
> > > + for (octx = ctx; octx && !remove; octx = octx->outer)
> > > + if (is_targetreg_ctx (octx))
> > > + remove = true;
> > > + if (remove)
> > > + error_at (gimple_location (stmt), "function called from "
> > > + "target region, but not marked as 'declare target'");
> >
> > %<declare target%> ?
>
> Fixed.
I guess I'm ok with warning in that case, but not erroring out, at least not
when -Werror.
If you get ICE if somebody does this, you should fix the problem during the
offloading LTO or where it is. Generally, the solution if something goes
wrong during the offloading compilation should be just to give up on the
offloading to the particular offloading target (i.e. fill in the sections
libgomp reads in a way that will result in host fallback).
Jakub
