On Fri, Jul 17, 2020 at 1:30 PM Andrew Stubbs <a...@codesourcery.com> wrote:
>
> On 17/07/2020 07:20, Thomas Schwinge wrote:
> >> --- a/gcc/config/gcn/mkoffload.c
> >> +++ b/gcc/config/gcn/mkoffload.c
> >> @@ -33,31 +33,53 @@
> >>   #include <libgen.h>
> >>   #include "collect-utils.h"
> >>   #include "gomp-constants.h"
> >> +#include "simple-object.h"
> >> +#include "elf.h"
> >> +
> >> +/* These probably won't be in elf.h for a while.  */
> >> +#ifndef EM_AMDGPU
> >
> > Nope, it already is.  ;-P
> [...]
> > I've got the canary 'EM_AMDGPU', but not the other '#define's.
>
> Oops, I've committed this patch to fix it.
>
> The code now assumes only that the relocations are defined as a block.
> The rest are undefined and redefined individually.
>
> This matches the usage we've previously had in gcn-run.c and
> plugin-gcn.c (actually, those can be cleaned up now, but that's another
> patch).
>
> >> -/* Files to unlink.  */
> >> -static const char *gcn_s1_name;
> >> -static const char *gcn_s2_name;
> >> -static const char *gcn_o_name;
> >> -static const char *gcn_cfile_name;
> >>   static const char *gcn_dumpbase;
> >> +static struct obstack files_to_cleanup;
> >
> > (Good idea; should do similar in the other 'mkoffload's.)
>
> Actually, I think the original code was more readable, but now we have a
> (potentially) variable number of files to clean up I needed something
> more general.
>
> >> +uint32_t elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX803;  // Default GPU 
> >> architecture.
> >
> > For easier later maintenance, shouldn't this be a '#define' (or similar)
> > done next to where the GCC back end defines its default?
>
> I thought of this, but I don't think there's actually a problem. The
> default is defined via OPTION_DEFAULT_SPECS, so there ought to be an
> explicit option passed to mkoffload at all times. If that's not the case
> I think the problem lies elsewhere.
>
> In practice, a mismatch here will not fail silently, so we'll know to
> fix it.
>
> The way simple_object is supposed to work is to clone (or merge) the ELF
> headers from an existing binary. Unfortunately, the way mkoffload is
> currently coded we don't have any to clone from until too late. We could
> separate the assemble and link steps, but I chose not to go that way at
> this time.

You could defer the debug copying to after mkoffload assembled the
offloaded code and use those objects as template.  Maybe that's
what you refer to with separating assemble and link steps.

Richard.

> Andrew

Reply via email to