[gomp4] Offload option handling

[Andrey Turetskiy]

Hi,
This patch (attached) contains the prototype of mechanism for passing
options to offload target compiler.
If one need to pass additional options for target compiler, one may
add option ‘–ftarget-options=<target name>=<target options>’ to host
compiler (target name can be skipped, that will append specified
options for every target).

Options preparing takes place in lto-wrapper in several stages:
1. Read (host) options from .gnu.target_lto_.opts section for every
object file and merge them as during lto
2. Remove CL_TARGET, CL_DRIVER and CL_LTO options
3. For every offload target: search for option ‘–ftarget-options’ with
argument corresponding to target triple, parse it and merge with
remained host options

Here is an example of option merging:
./install/bin/gcc -fopenmp -c a.c -O0 -mno-sse -ftarget-options=="-O2 -msse"
./install/bin/gcc -fopenmp -c b.c -O0 -fpack-struct
-ftarget-options=="-O3 -mavx"
./install/bin/gcc -fopenmp a.o b.o
After merging at stage 3 we get: -O3 -fpack-struct
-ftarget-options==”-O2 –msse” -ftarget-options==”-O3 –mavx” –msse
–mavx
These options are passed to mkoffload and thus to target compiler.
-ftarget-options==”
” itself is ignored by target compiler.

However there is an issue: options specified by -ftarget-options are
not checked for correctness as usual, so incorrect options inside
-ftarget-options==”
” may lead to GCC crashes. Need to investigate
possibilities of reusing existing option checks to solve the problem.

How does this look? Do you agree with the approach?

Ping.
https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00616.html

There's a problem when offloading from a compiler for one target machine
to another: the machine specific options don't necessarily match. This
patch tries to address this.
The idea is that since we have two options sections anyway, with
different section name prefixes, we can arrange to pass only
target-independent options in the omp_ version of the options section.
However, some target-specific options (specifically the ones specifying
the ABI) need to be preserved somehow, so there's a new target hook for
translating them to common a -foffload-* syntax.
How does this look? Comments on the approach, and ok for the
gomp-4_0-branch?

How about passing target-specific options from the command-line? Will it be
possible with this approach?
We had a patch, that parses options from -foffload-target=<target>=<options> and
stores them to the environment variable. Now it is obsolete, but I like the
idea of having such a user-adjustable options for the accel compilers.
https://gcc.gnu.org/ml/gcc-patches/2013-12/msg01242.html
-- Ilya

[Bernd Schmidt]

Post by Andrey Turetskiy
How does this look? Do you agree with the approach?

I have no objections to supporting a -ftarget-options switch. I had
posted a patch a while ago that looked somewhat similar, but also
contained an automatic translation step from things like -march=x86_64
to a generic -foffload-abi=lp64. I still think such a mechanism is
desirable.


Bernd

[Jakub Jelinek]

Post by Andrey Turetskiy
Hi,
This patch (attached) contains the prototype of mechanism for passing
options to offload target compiler.
If one need to pass additional options for target compiler, one may
add option ‘–ftarget-options=<target name>=<target options>’ to host
compiler (target name can be skipped, that will append specified
options for every target).
1. Read (host) options from .gnu.target_lto_.opts section for every
object file and merge them as during lto
2. Remove CL_TARGET, CL_DRIVER and CL_LTO options
3. For every offload target: search for option ‘–ftarget-options’ with
argument corresponding to target triple, parse it and merge with
remained host options
./install/bin/gcc -fopenmp -c a.c -O0 -mno-sse -ftarget-options=="-O2 -msse"

I don't like the == in there. Doesn't <target name>, being a target triplet
or something like that, always have to start with alphanumeric character,
and options always have to start with -? Thus, can't you decide from the
first character after -ftarget-options= whether it is for all or specific
target? If the character after = is whitespace or -, it will be for
all targets and
-ftarget-options=<target options>
form, otherwise it must be
-ftarget-options=<target name>=<target options>
form.

Jakub

[Andrey Turetskiy]

Post by Jakub Jelinek
I don't like the == in there. Doesn't <target name>, being a target triplet
or something like that, always have to start with alphanumeric character,
and options always have to start with -? Thus, can't you decide from the
first character after -ftarget-options= whether it is for all or specific
target? If the character after = is whitespace or -, it will be for
all targets and
-ftarget-options=<target options>
form, otherwise it must be
-ftarget-options=<target name>=<target options>
form.

Yes, I think it would be better.
This patch is just a prototype to show the main idea, so I'd like to
hear if you agree with the approach.

Post by Jakub Jelinek
I have no objections to supporting a -ftarget-options switch. I had posted a
patch a while ago that looked somewhat similar, but also contained an
automatic translation step from things like -march=x86_64 to a generic
-foffload-abi=lp64. I still think such a mechanism is desirable.

I'm going to apply your patch on '-ftarget-options' stuff and check if
it works together.

[Andrey Turetskiy]

On Wed, Sep 17, 2014 at 5:11 PM, Andrey Turetskiy

Post by Andrey Turetskiy

Post by Jakub Jelinek
I have no objections to supporting a -ftarget-options switch. I had posted a
patch a while ago that looked somewhat similar, but also contained an
automatic translation step from things like -march=x86_64 to a generic
-foffload-abi=lp64. I still think such a mechanism is desirable.

I'm going to apply your patch on '-ftarget-options' stuff and check if
it works together.

I've applied your option patch on our offload branch (w/o
'-ftarget-options' switch yet) and it seems to be working fine.
However the patch looks a bit unfinished:

@@ -440,7 +554,11 @@ access_check (const char *name, int mode

static char*
prepare_target_image (const char *target, const char *compiler_path,
- unsigned in_argc, char *in_argv[])
+ unsigned in_argc, char *in_argv[],
+ struct cl_decoded_option *compiler_opts,
+ unsigned int compiler_opt_count,
+ struct cl_decoded_option * /*linker_opts */,
+ unsigned int /*linker_opt_count*/)
{
const char **argv;
struct obstack argv_obstack;
@@ -469,8 +587,6 @@ prepare_target_image (const char *target
/* Generate temp file name. */
filename = make_temp_file (".target.o");

- /* -------------------------------------- */
- /* Run gcc for target. */
obstack_init (&argv_obstack);
obstack_ptr_grow (&argv_obstack, compiler);
obstack_ptr_grow (&argv_obstack, "-o");
@@ -479,6 +595,8 @@ prepare_target_image (const char *target
for (i = 1; i < in_argc; ++i)
if (strncmp (in_argv[i], "-fresolution=", sizeof ("-fresolution=") - 1))
obstack_ptr_grow (&argv_obstack, in_argv[i]);
+
+ append_compiler_options (&argv_obstack, compiler_opts, compiler_opt_count);
obstack_ptr_grow (&argv_obstack, NULL);

argv = XOBFINISH (&argv_obstack, const char **);

argv = XOBFINISH (&argv_obstack, char **);

What do you suppose to do with 'linker_opts'?
Do you want to call 'append_linker_options (&argv_obstack,
linker_opts, linker_opt_count, false)'? That would explain why you
have 'include_target_options' argument in the function.

[Bernd Schmidt]

Post by Andrey Turetskiy
I've applied your option patch on our offload branch (w/o
'-ftarget-options' switch yet) and it seems to be working fine.
@@ -440,7 +554,11 @@ access_check (const char *name, int mode
static char*
prepare_target_image (const char *target, const char *compiler_path,
- unsigned in_argc, char *in_argv[])
+ unsigned in_argc, char *in_argv[],
+ struct cl_decoded_option *compiler_opts,
+ unsigned int compiler_opt_count,
+ struct cl_decoded_option * /*linker_opts */,
+ unsigned int /*linker_opt_count*/)
{
const char **argv;
struct obstack argv_obstack;
@@ -469,8 +587,6 @@ prepare_target_image (const char *target
/* Generate temp file name. */
filename = make_temp_file (".target.o");
- /* -------------------------------------- */
- /* Run gcc for target. */
obstack_init (&argv_obstack);
obstack_ptr_grow (&argv_obstack, compiler);
obstack_ptr_grow (&argv_obstack, "-o");
@@ -479,6 +595,8 @@ prepare_target_image (const char *target
for (i = 1; i < in_argc; ++i)
if (strncmp (in_argv[i], "-fresolution=", sizeof ("-fresolution=") - 1))
obstack_ptr_grow (&argv_obstack, in_argv[i]);
+
+ append_compiler_options (&argv_obstack, compiler_opts, compiler_opt_count);
obstack_ptr_grow (&argv_obstack, NULL);
argv = XOBFINISH (&argv_obstack, const char **);
argv = XOBFINISH (&argv_obstack, char **);
What do you suppose to do with 'linker_opts'?

Unclear at this point. For ptx, at the moment we don't even perform a
linking step. This is likely to change, but even then the host linker
options are probably not relevant. Most likely we could scan for things
like "-lgfortran" which we could assume must exist on the offload target
and pass these down.


Bernd