On 08 Oct 12:26, Jakub Jelinek wrote:
> On Thu, Oct 02, 2014 at 07:14:57PM +0400, Ilya Verbin wrote:
> > @@ -1296,6 +1297,9 @@ static const char *const standard_startfile_prefix_2
> > relative to the driver. */
> > static const char *const tooldir_base_prefix = TOOLDIR_BASE_PREFIX;
> >
> > +/* A prefix to be used when this is an accelerator compiler. */
> > +static const char *const accel_dir_suffix = ACCEL_DIR_SUFFIX;
>
> Is ACCEL_DIR_SUFFIX here "" or something starting with "/ ?
>
> The reason I'm asking is that otherwise it seems gcc.c heavily uses
> dir_separator_str for the separators. Don't have any experience with
> targets where DIR_SEPARATOR is not / though, perhaps it is a non-issue.
It is "" for the host compiler and starts with "/" for the accel compiler.
Looking at gcc/configure.ac, there are a lot of paths with slashes.
So, I think it's ok to define ACCEL_DIR_SUFFIX in such a way.
> > +#ifndef ACCEL_COMPILER
> > /* We need to check standard_exec_prefix/just_machine_suffix/specs
> > for any override of as, ld and libraries. */
> > specs_file = (char *) alloca (strlen (standard_exec_prefix)
> > + strlen (just_machine_suffix) + sizeof ("specs"));
> > -
> > strcpy (specs_file, standard_exec_prefix);
> > strcat (specs_file, just_machine_suffix);
> > strcat (specs_file, "specs");
> > if (access (specs_file, R_OK) == 0)
> > read_specs (specs_file, true, false);
> > +#endif
>
> Why do you want to disable specs reading for the accel compiler?
> Then users won't have the possibility to override defaults etc. easily...
Bernd,
Do you need this ifndef for PTX? Or I could remove it?
> > @@ -7097,6 +7103,16 @@ main (int argc, char **argv)
> > obstack_grow (&collect_obstack, argv[0], strlen (argv[0]) + 1);
> > xputenv (XOBFINISH (&collect_obstack, char *));
> >
> > + if (strlen (OFFLOAD_TARGETS) > 0)
> > + {
> > + obstack_init (&collect_obstack);
> > + obstack_grow (&collect_obstack, "OFFLOAD_TARGET_NAMES=",
> > + sizeof ("OFFLOAD_TARGET_NAMES=") - 1);
> > + obstack_grow (&collect_obstack, OFFLOAD_TARGETS,
> > + strlen (OFFLOAD_TARGETS) + 1);
> > + xputenv (XOBFINISH (&collect_obstack, char *));
> > + }
> > +
>
> I'm surprised to see the obstack_init call here, but looking at
> gcc.c, I'm surprised to see it in more places.
>
> I've always thought that obstack_init was something you invoke generally
> once on a given obstack object, then work with the obstack and then
> obstack_free (..., NULL) it at the end. Now, if it wants the obstack to be
> live until exit, it just would not obstack_free it. But calling
> obstack_init on the already initialized obstack results IMHO in memory
> leaks. It should be initialized just once somewhere.
Right, I removed obstack_init from this patch.
> > +#define OFFLOAD_TARGET_NAMES_ENV "OFFLOAD_TARGET_NAMES"
>
> Missing comment about the env var and what it is for.
Fixed.
> > + char *suffix
> > + = XALLOCAVEC (char, strlen ("/accel//mkoffload") + strlen (target) +
> > 1);
>
> Use sizeof ("/accel//mkoffload") + strlen (target) instead?
Done.
> > + out:
>
> The goto is probably unnecessary here, indenting all the lines by
> 4 more spaces and using if (compiler) { ... } instead doesn't need
> any further warpping.
Done.
> > + /* By default linker does not discard .gnu.offload_lto_* sections.
> > */
> > + const char *linker_script = make_temp_file ("_linker_script.x");
> > + FILE *stream = fopen (linker_script, "w");
> > + if (!stream)
> > + fatal_error ("fopen %s: %m", linker_script);
> > + fprintf (stream, "SECTIONS { /DISCARD/ : { *("
> > + OFFLOAD_SECTION_NAME_PREFIX "*) } }\n");
> > + fclose (stream);
> > + printf ("%s\n", linker_script);
> > +
> > + goto finish;
> > + }
>
> Does this work with gold? Are there any other linkers that support plugins,
> but don't support linker scripts this way?
Oops, gold does not support scripts, outputted from plugins :(
"error: SECTIONS seen after other input files; try -T/--script"
Probably, we should update default linker scripts in binutils?
But without latest ld/gold all binaries compiled without -flto and with
offloading will contain intermediate bytecode...
Thanks,
-- Ilya
