[6/n] OpenMP 4.0 offloading infrastructure: option handling

On 13 Oct 12:19, Jakub Jelinek wrote:
> On Sat, Oct 11, 2014 at 06:49:00PM +0400, Ilya Verbin wrote:
> > It introduces 2 new options:
> > 1. -foffload=<targets>=<options>
> >    By default, GCC will build offload images for all offload targets specified
> > in configure, with non-target-specific options passed to host compiler.
> > This option is used to control offload targets and options for them.
> > 
> > It can be used in a few ways:
> > * -foffload=disable
> >   Tells GCC to disable offload support.
> >   OpenMP target regions will be run in host fallback mode.
> > * -foffload=<targets>
> >   Tells GCC to build offload images for <targets>.
> >   They will be built with non-target-specific options passed to host compiler.
> > * -foffload=<options>
> >   Tells GCC to build offload images for all targets specified in configure. 
> >   They will be built with non-target-specific options passed to host compiler
> >   plus <options>.
> > * -foffload=<targets>=<options>
> >   Tells GCC to build offload images for <targets>.
> >   They will be built with non-target-specific options passed to host compiler
> >   plus <options>.
> > 
> > Options specified by -foffload are appended to the end of option set, so in case
> > of option conflicts they have more priority.
> 
> This looks good to me.
> 
> > 2. -foffload-abi=[lp64|ilp32]
> >    This option is supposed to tell mkoffload (and offload compiler) which ABI is
> > used in streamed GIMPLE.  This option is desirable, because host and offload
> > compilers must have the same ABI.  The option is generated by the host compiler
> > automatically, it should not be specified by user.
> 
> But I'd like to understand why is this one needed.
> Why should the compilers care?  Aggregates layout and alignment of
> integral/floating types must match between host and offload compilers, sure,
> but isn't that something streamed already in the LTO bytecode?
> Or is LTO streamer not streaming some types like long_type_node?
> I'd expect if host and offload compiler disagree on long type size that
> you'd just use a different integral type with the same size as long on the
> host.
> Different sized pointers are of course a bigger problem, but can't you just
> error out on that during reading of the LTO, or even handle it (just use
> some integral type for when is the pointer stored in memory, and just
> convert to pointer after reads from memory, and convert back before storing
> to memory).  Erroring out during LTO streaming in sounds just fine to me
> though.

Here is an additional check.  Is the whole 'option handling' patch OK?

Thanks,
  -- Ilya

On Fri, Oct 24, 2014 at 10:29:50PM +0400, Ilya Verbin wrote:
> diff --git a/gcc/opts.c b/gcc/opts.c
> index 9b2e1af..d1a626c 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -1732,6 +1732,13 @@ common_handle_option (struct gcc_options *opts,
>        /* Deferred.  */
>        break;
>  
> +#ifndef ACCEL_COMPILER

ifndef ?  I would have expected ifdef.

> +    case OPT_foffload_abi_:
> +      error_at (loc, "-foffload-abi option can be specified only for "
> +		"offload compiler");
> +      break;
> +#endif
> +
>      case OPT_fpack_struct_:
>        if (value <= 0 || (value & (value - 1)) || value > 16)
>  	error_at (loc,

	Jakub

On 24 Oct 20:33, Jakub Jelinek wrote:
> On Fri, Oct 24, 2014 at 10:29:50PM +0400, Ilya Verbin wrote:
> > diff --git a/gcc/opts.c b/gcc/opts.c
> > index 9b2e1af..d1a626c 100644
> > --- a/gcc/opts.c
> > +++ b/gcc/opts.c
> > @@ -1732,6 +1732,13 @@ common_handle_option (struct gcc_options *opts,
> >        /* Deferred.  */
> >        break;
> >  
> > +#ifndef ACCEL_COMPILER
> 
> ifndef ?  I would have expected ifdef.
> 
> > +    case OPT_foffload_abi_:
> > +      error_at (loc, "-foffload-abi option can be specified only for "
> > +		"offload compiler");
> > +      break;
> > +#endif
> > +
> >      case OPT_fpack_struct_:
> >        if (value <= 0 || (value & (value - 1)) || value > 16)
> >  	error_at (loc,

-foffload-abi option is intended only for the offload compiler, but we can't put
the option under ifdef in common.opt, therefore I added error_at under ifndef.

  -- Ilya