This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [gcn][patch] Add -mgpu option and plumb in assembler/linker


Hello Andrew,

I apologize for taking so long to reply, I was traveling for two past
weeks and just before that we suffered some local infrastructure
issues that prevented me from working on this too.

On Fri, Apr 28, 2017 at 06:06:39PM +0100, Andrew Stubbs wrote:
> This patch, for the "gcn" branch, does three things:
> 
> 1. Add specs to drive the LLVM assembler and linker. It requires them to be
> installed as "as" and "ld", under $target/bin, but then the compiler Just
> Works with these specs.

At the moment I prefer to use --with-as and --with-ld configure
options which are better suited for my setup.  The invocation of
assembler works well, the invocation of ld.lld works too, but with the
added caveat that collect2 afterwards attempts to do non-plufin LTO
and calls maybe_run_lto_and_relink, which wants to run nm, which is
not available and so it fails with a fatal_error.  It took me a while
to figure out what was going on and that the result was actually fine,
despite the error message.  I guess we are fine with passing -fno-lto
or rather disabling lto at configure time for the time being.

> 
> 2. Switch to HSACO format version 2, and have the assembler auto-set the
> architecture flags from -mcpu. This means the amdphdr utility is no longer
> required.

This is the one thing that was it difficult for me to get it working.
I had to upgrade my kernel and both run-time libraries to the newest
ROCm 1.5, re-compile llvm and lld from ROCm github branches, and
rewrite our testing kernel invoker to use non-deprecated HSA 1.1
functions (we had been using hsa_code_object_deserialize and friends
from HSA 1.0).  But finally, my kernels get loaded, started and work.

> 
> 3. Add -mgpu option and corresponding --with-gpu. I've deliberately used
> "gpu" instead of "cpu" because I want offloading compilers to be able to say
> "-mcpu=foo -foffload=-mgpu=bar", or even have the host compiler just
> understand -mgpu and DTRT.

As far as I am concerned, this seems like a good idea. 

Anyhow, thanks for submitting your patch, I apologize once again for
taking so long to test it.  Please commit the changes, I will wait
with (a bit overdue) merge from trunk until after you do.

Thanks,

Martin


> 
> The patch also removes the unused and unwritten "arch" and "tune" settings.
> They can be added back when useful, but the assembler requires a GPU name, I
> think, so we need that as input.
> 
> OK to commit to GCN branch?
> 
> Andrew
> 

> commit 5058457b0fa07865b366832828e74a53e5bd2964
> Author: Andrew Stubbs <ams@codesourcery.com>
> Date:   Fri Apr 28 14:37:25 2017 +0100
> 
>     Add -mgpu
>     
>     2017-04-28  Andrew Stubbs  <ams@codesourcery.com>
>     
>     	gcc/
>     	* config.gcc (amdgcn): Remove --with-arch and --with-tune.
>     	Add --with-gpu, and set default to "carrizo"
>     	(add_defaults): Add "gpu".
>     	* config/gcn/gcn-opts.h: New file.
>     	* config/gcn/gcn.c (output_file_start): Switch to HSACO version
>     	2 and auto-detection of GPU type (from -mcpu).
>     	(gcn_arch, gcn_tune): Remove.
>     	* config/gcn/gcn.h: Include gcn-opts.h.
>     	(enum processor_type): Move to gcn-opts.h.
>     	(LIBGCC_SPEC, ASM_SPEC, LINK_SPEC): Define.
>     	(gcn_arch, gcn_tune): Remove.
>     	(OPTION_DEFAULT_SPECS): Remove "arch" and "tune"; add "gpu".
>     	* config/gcn/gcn.opt: Include gcn-opts.h.
>     	(gpu_type): New Enum.
>     	(mgpu): New option.
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]