public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [gcn][patch] Add -mgpu option and plumb in assembler/linker
@ 2017-04-28 17:23 Andrew Stubbs
  2017-04-28 17:27 ` Joseph Myers
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Andrew Stubbs @ 2017-04-28 17:23 UTC (permalink / raw)
  To: Martin Jambor, Jan Hubicka, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

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.

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.

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.

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


[-- Attachment #2: gcn-mgpu.patch --]
[-- Type: text/x-patch, Size: 6112 bytes --]

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.

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 4a77b66..b1df533 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -3901,20 +3901,20 @@ case "${target}" in
 		;;
 
 	amdgcn-*-*)
-		supported_defaults="arch tune"
+		supported_defaults="gpu"
 
-		for which in arch tune; do
-			eval "val=\$with_$which"
-			case ${val} in
-			"" | fiji)
-				# OK
-				;;
-			*)
-				echo "Unknown cpu used in --with-$which=$val." 1>&2
-				exit 1
-				;;
-			esac
-		done
+		case "$with_gpu" in
+		"")
+			with_gpu=carrizo
+			;;
+		carrizo | fiji)
+			# OK
+			;;
+		*)
+			echo "Unknown gpu used in --with-gpu=$val." 1>&2
+			exit 1
+			;;
+		esac
 		;;
 
 	hppa*-*-*)
@@ -4646,7 +4646,7 @@ case ${target} in
 esac
 
 t=
-all_defaults="abi cpu cpu_32 cpu_64 arch arch_32 arch_64 tune tune_32 tune_64 schedule float mode fpu nan fp_32 odd_spreg_32 divide llsc mips-plt synci tls lxc1-sxc1 madd4"
+all_defaults="abi cpu cpu_32 cpu_64 arch arch_32 arch_64 tune tune_32 tune_64 schedule float mode fpu nan fp_32 odd_spreg_32 divide llsc mips-plt synci tls lxc1-sxc1 madd4 gpu"
 for option in $all_defaults
 do
 	eval "val=\$with_"`echo $option | sed s/-/_/g`
diff --git a/gcc/config/gcn/gcn-opts.h b/gcc/config/gcn/gcn-opts.h
new file mode 100644
index 0000000..d0586d6
--- /dev/null
+++ b/gcc/config/gcn/gcn-opts.h
@@ -0,0 +1,27 @@
+/* Copyright (C) 2016-2017 Free Software Foundation, Inc.
+
+   This file is free software; you can redistribute it and/or modify it under
+   the terms of the GNU General Public License as published by the Free
+   Software Foundation; either version 3 of the License, or (at your option)
+   any later version.
+
+   This file is distributed in the hope that it will be useful, but WITHOUT
+   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+   FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+   for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef GCN_OPTS_H
+#define GCN_OPTS_H
+
+/* Which processor to generate code or schedule for.  */
+enum processor_type
+{
+  PROCESSOR_CARRIZO,
+  PROCESSOR_FIJI
+};
+
+#endif
diff --git a/gcc/config/gcn/gcn.c b/gcc/config/gcn/gcn.c
index eb6edd8..f378bf8 100644
--- a/gcc/config/gcn/gcn.c
+++ b/gcc/config/gcn/gcn.c
@@ -60,11 +60,6 @@
 /* This file should be included last.  */
 #include "target-def.h"
 
-/* Which instruction set architecture to use.  */
-int gcn_arch;
-/* Which cpu are we tuning for.  */
-int gcn_tune;
-
 static REAL_VALUE_TYPE dconst4, dconst1over2pi;
 static bool ext_gcn_constants_init = 0;
 
@@ -2006,8 +2001,8 @@ static void
 output_file_start (void)
 {
   fprintf (asm_out_file, "\t.hsatext\n");
-  fprintf (asm_out_file, "\t.hsa_code_object_version 1,0\n");
-  fprintf (asm_out_file, "\t.hsa_code_object_isa 8,0,1,\"AMD\",\"AMDGPU\"\n");
+  fprintf (asm_out_file, "\t.hsa_code_object_version 2,0\n");
+  fprintf (asm_out_file, "\t.hsa_code_object_isa\n");  // Autodetect
   fprintf (asm_out_file, "\t.section\t.AMDGPU.config\n");
   fprintf (asm_out_file, "\t.hsatext\n");
 }
diff --git a/gcc/config/gcn/gcn.h b/gcc/config/gcn/gcn.h
index 903022f..a3f9463 100644
--- a/gcc/config/gcn/gcn.h
+++ b/gcc/config/gcn/gcn.h
@@ -14,25 +14,28 @@
    along with GCC; see the file COPYING3.  If not see
    <http://www.gnu.org/licenses/>.  */
 
+#include "config/gcn/gcn-opts.h"
+
 \f
 /* FIXME */
 #define TARGET_CPU_CPP_BUILTINS()
 
-/* Which processor to generate code or schedule for.  */
-enum processor_type
-{
-  PROCESSOR_CARRIZO,
-};
+/* Temporarily disable libgcc until one actually exists.  */
+#undef  LIBGCC_SPEC
+#define LIBGCC_SPEC ""
+
+/* Use LLVM assembler options.  */
+#undef ASM_SPEC
+#define ASM_SPEC "-triple=amdgcn--amdhsa %{mgpu=*:-mcpu=%*} -filetype=obj"
 
-extern GTY(()) int gcn_arch;
-extern GTY(()) int gcn_tune;
+/* Default to relocatable executables as output.  */
+#undef LINK_SPEC
+#define LINK_SPEC "-shared"
 
 /* Support for a compile-time default architecture and tuning.  The rules are:
-   --with-arch is ignored if -march is specified.
-   --with-tune is ignored if -mtune is specified.  */
+   --with-gpu is ignored if -mgpu is specified.  */
 #define OPTION_DEFAULT_SPECS \
-  {"arch", "%{!march=*:-march=%(VALUE)}" }, \
-  {"tune", "%{!mtune=*:-mtune=%(VALUE)}" }
+  {"gpu", "%{!mgpu=*:-mgpu=%(VALUE)}"}
 
 /* Default target_flags if no switches specified.  */
 #ifndef TARGET_DEFAULT
diff --git a/gcc/config/gcn/gcn.opt b/gcc/config/gcn/gcn.opt
index 8fc02b7..77f0ef0 100644
--- a/gcc/config/gcn/gcn.opt
+++ b/gcc/config/gcn/gcn.opt
@@ -17,3 +17,20 @@
 ; You should have received a copy of the GNU General Public License
 ; along with GCC; see the file COPYING3.  If not see
 ; <http://www.gnu.org/licenses/>.
+
+HeaderInclude
+config/gcn/gcn-opts.h
+
+Enum
+Name(gpu_type) Type(enum processor_type)
+GCN GPU type to use:
+
+EnumValue
+Enum(gpu_type) String(carrizo) Value(PROCESSOR_CARRIZO)
+
+EnumValue
+Enum(gpu_type) String(fiji) Value(PROCESSOR_FIJI)
+
+mgpu=
+Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_gpu) Init(PROCESSOR_CARRIZO)
+Specify the name of the target GPU.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gcn][patch] Add -mgpu option and plumb in assembler/linker
  2017-04-28 17:23 [gcn][patch] Add -mgpu option and plumb in assembler/linker Andrew Stubbs
@ 2017-04-28 17:27 ` Joseph Myers
  2017-05-02 17:12 ` Martin Jambor
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Joseph Myers @ 2017-04-28 17:27 UTC (permalink / raw)
  To: Andrew Stubbs; +Cc: Martin Jambor, Jan Hubicka, gcc-patches

On Fri, 28 Apr 2017, Andrew Stubbs wrote:

> 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.

Command-line options should be documented in invoke.texi, configure 
options in install.texi.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gcn][patch] Add -mgpu option and plumb in assembler/linker
  2017-04-28 17:23 [gcn][patch] Add -mgpu option and plumb in assembler/linker Andrew Stubbs
  2017-04-28 17:27 ` Joseph Myers
@ 2017-05-02 17:12 ` Martin Jambor
  2017-05-03  9:30   ` Andrew Stubbs
  2017-05-16 10:37   ` Andrew Stubbs
  2017-05-29 17:48 ` Martin Jambor
  2017-06-01  8:52 ` [gcn][patch] Add -mgpu option and plumb in assembler/linker Thomas Schwinge
  3 siblings, 2 replies; 13+ messages in thread
From: Martin Jambor @ 2017-05-02 17:12 UTC (permalink / raw)
  To: Andrew Stubbs; +Cc: Jan Hubicka, gcc-patches

Hi Andrew,

sorry for replying only now but yesterday was public holiday here and
I am still only in the process of recovering from a long weekend.

While the only objection I have is the C++ style comment in
config/gcn/gcn.c, another problem, for me at least...

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.

...is that I do not have llvm linker at hand and without it I did not
manage to make the patch produce loadable code.  Because ROCm 1.5 has
been released today, I will update our environment, which is a bit
obsolete, get llvm ld and try again.  This might take me a few days,
so please bear with me for a little more, I would like to make sure it
works on carrizos.

Thanks,

Martin

> 
> 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.
> 
> 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.
> 
> 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?
>     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.
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gcn][patch] Add -mgpu option and plumb in assembler/linker
  2017-05-02 17:12 ` Martin Jambor
@ 2017-05-03  9:30   ` Andrew Stubbs
  2017-05-16 10:37   ` Andrew Stubbs
  1 sibling, 0 replies; 13+ messages in thread
From: Andrew Stubbs @ 2017-05-03  9:30 UTC (permalink / raw)
  To: Jan Hubicka, gcc-patches

On 02/05/17 18:08, Martin Jambor wrote:
> Hi Andrew,
> 
> sorry for replying only now but yesterday was public holiday here and
> I am still only in the process of recovering from a long weekend.

No problem, the UK had the same. :-)

> While the only objection I have is the C++ style comment in
> config/gcn/gcn.c, another problem, for me at least...
> 
> 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.
> 
> ...is that I do not have llvm linker at hand and without it I did not
> manage to make the patch produce loadable code.  Because ROCm 1.5 has
> been released today, I will update our environment, which is a bit
> obsolete, get llvm ld and try again.  This might take me a few days,
> so please bear with me for a little more, I would like to make sure it
> works on carrizos.

Understood. I do not have a carrizo to test on.

All my testing will be with FuryX Fiji discrete GPUs. The main 
difference, from the software point of view, is that the shared memory 
must be handled a little differently. (The existing HSA back-end appears 
incompatible, as are the samples from the HSA Foundation sources; the 
ROCm samples work fine.) However, the HSACO binary must encode the right 
magic numbers or the driver rejects it, hence the need for this patch.

Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gcn][patch] Add -mgpu option and plumb in assembler/linker
  2017-05-02 17:12 ` Martin Jambor
  2017-05-03  9:30   ` Andrew Stubbs
@ 2017-05-16 10:37   ` Andrew Stubbs
  1 sibling, 0 replies; 13+ messages in thread
From: Andrew Stubbs @ 2017-05-16 10:37 UTC (permalink / raw)
  To: Martin Jambor, Jan Hubicka, gcc-patches

On 02/05/17 18:08, Martin Jambor wrote:
>> 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.
> 
> ...is that I do not have llvm linker at hand and without it I did not
> manage to make the patch produce loadable code.  Because ROCm 1.5 has
> been released today, I will update our environment, which is a bit
> obsolete, get llvm ld and try again.  This might take me a few days,
> so please bear with me for a little more, I would like to make sure it
> works on carrizos.

Any news?

If you're happy with the patch I can fix the comment style, and add the 
documentation Joseph asked for, and get it committed.

Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gcn][patch] Add -mgpu option and plumb in assembler/linker
  2017-04-28 17:23 [gcn][patch] Add -mgpu option and plumb in assembler/linker Andrew Stubbs
  2017-04-28 17:27 ` Joseph Myers
  2017-05-02 17:12 ` Martin Jambor
@ 2017-05-29 17:48 ` Martin Jambor
  2017-06-06 19:53   ` Andrew Stubbs
  2017-06-01  8:52 ` [gcn][patch] Add -mgpu option and plumb in assembler/linker Thomas Schwinge
  3 siblings, 1 reply; 13+ messages in thread
From: Martin Jambor @ 2017-05-29 17:48 UTC (permalink / raw)
  To: Andrew Stubbs; +Cc: Jan Hubicka, gcc-patches

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.
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gcn][patch] Add -mgpu option and plumb in assembler/linker
  2017-04-28 17:23 [gcn][patch] Add -mgpu option and plumb in assembler/linker Andrew Stubbs
                   ` (2 preceding siblings ...)
  2017-05-29 17:48 ` Martin Jambor
@ 2017-06-01  8:52 ` Thomas Schwinge
  3 siblings, 0 replies; 13+ messages in thread
From: Thomas Schwinge @ 2017-06-01  8:52 UTC (permalink / raw)
  To: Andrew Stubbs; +Cc: Martin Jambor, Jan Hubicka, gcc-patches

Hi!

Sorry for the late reply.

On Fri, 28 Apr 2017 18:06:39 +0100, Andrew Stubbs <ams@codesourcery.com> wrote:
> 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.

I'm not sure I understand your last statement, or the intentions behind
it.

How would the host compiler (be able to) understand (or, disambiguate)
"-mgpu=[...]" in the (default) case of several offloading targets having
been configured?  I think it holds that "-m[...]" etc. must/can always
only apply to the current target (or "host", in "offloading speak").

And then, I don't have any strong opinion, but I don't see why a new
"-mgpu" option is preferable to using the existing "-march" etc. in
"-foffload=[...]".  For example, you can already now do things like
(exemplary):

    -march=x86_64 -foffload=-march=generic -foffload=nvptx-none=-march=cc_50 -foffload=gcn=-march=carrizo
    ^ target-specific ^ offload-target-specific, unless overridden... ^ ... here... ^ ..., and here

Likewise for the new "--with-gpu=[...]" vs. the existing
"--with-arch=[...]", where again I would, unless there is a specific
reason (that I didn't understand here), default to using the existing
option names instead of introducing new ones.


Grüße
 Thomas


> 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.
> 
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index 4a77b66..b1df533 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -3901,20 +3901,20 @@ case "${target}" in
>  		;;
>  
>  	amdgcn-*-*)
> -		supported_defaults="arch tune"
> +		supported_defaults="gpu"
>  
> -		for which in arch tune; do
> -			eval "val=\$with_$which"
> -			case ${val} in
> -			"" | fiji)
> -				# OK
> -				;;
> -			*)
> -				echo "Unknown cpu used in --with-$which=$val." 1>&2
> -				exit 1
> -				;;
> -			esac
> -		done
> +		case "$with_gpu" in
> +		"")
> +			with_gpu=carrizo
> +			;;
> +		carrizo | fiji)
> +			# OK
> +			;;
> +		*)
> +			echo "Unknown gpu used in --with-gpu=$val." 1>&2
> +			exit 1
> +			;;
> +		esac
>  		;;
>  
>  	hppa*-*-*)
> @@ -4646,7 +4646,7 @@ case ${target} in
>  esac
>  
>  t=
> -all_defaults="abi cpu cpu_32 cpu_64 arch arch_32 arch_64 tune tune_32 tune_64 schedule float mode fpu nan fp_32 odd_spreg_32 divide llsc mips-plt synci tls lxc1-sxc1 madd4"
> +all_defaults="abi cpu cpu_32 cpu_64 arch arch_32 arch_64 tune tune_32 tune_64 schedule float mode fpu nan fp_32 odd_spreg_32 divide llsc mips-plt synci tls lxc1-sxc1 madd4 gpu"
>  for option in $all_defaults
>  do
>  	eval "val=\$with_"`echo $option | sed s/-/_/g`
> diff --git a/gcc/config/gcn/gcn-opts.h b/gcc/config/gcn/gcn-opts.h
> new file mode 100644
> index 0000000..d0586d6
> --- /dev/null
> +++ b/gcc/config/gcn/gcn-opts.h
> @@ -0,0 +1,27 @@
> +/* Copyright (C) 2016-2017 Free Software Foundation, Inc.
> +
> +   This file is free software; you can redistribute it and/or modify it under
> +   the terms of the GNU General Public License as published by the Free
> +   Software Foundation; either version 3 of the License, or (at your option)
> +   any later version.
> +
> +   This file is distributed in the hope that it will be useful, but WITHOUT
> +   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> +   FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> +   for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with GCC; see the file COPYING3.  If not see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GCN_OPTS_H
> +#define GCN_OPTS_H
> +
> +/* Which processor to generate code or schedule for.  */
> +enum processor_type
> +{
> +  PROCESSOR_CARRIZO,
> +  PROCESSOR_FIJI
> +};
> +
> +#endif
> diff --git a/gcc/config/gcn/gcn.c b/gcc/config/gcn/gcn.c
> index eb6edd8..f378bf8 100644
> --- a/gcc/config/gcn/gcn.c
> +++ b/gcc/config/gcn/gcn.c
> @@ -60,11 +60,6 @@
>  /* This file should be included last.  */
>  #include "target-def.h"
>  
> -/* Which instruction set architecture to use.  */
> -int gcn_arch;
> -/* Which cpu are we tuning for.  */
> -int gcn_tune;
> -
>  static REAL_VALUE_TYPE dconst4, dconst1over2pi;
>  static bool ext_gcn_constants_init = 0;
>  
> @@ -2006,8 +2001,8 @@ static void
>  output_file_start (void)
>  {
>    fprintf (asm_out_file, "\t.hsatext\n");
> -  fprintf (asm_out_file, "\t.hsa_code_object_version 1,0\n");
> -  fprintf (asm_out_file, "\t.hsa_code_object_isa 8,0,1,\"AMD\",\"AMDGPU\"\n");
> +  fprintf (asm_out_file, "\t.hsa_code_object_version 2,0\n");
> +  fprintf (asm_out_file, "\t.hsa_code_object_isa\n");  // Autodetect
>    fprintf (asm_out_file, "\t.section\t.AMDGPU.config\n");
>    fprintf (asm_out_file, "\t.hsatext\n");
>  }
> diff --git a/gcc/config/gcn/gcn.h b/gcc/config/gcn/gcn.h
> index 903022f..a3f9463 100644
> --- a/gcc/config/gcn/gcn.h
> +++ b/gcc/config/gcn/gcn.h
> @@ -14,25 +14,28 @@
>     along with GCC; see the file COPYING3.  If not see
>     <http://www.gnu.org/licenses/>.  */
>  
> +#include "config/gcn/gcn-opts.h"
> +
>  \f
>  /* FIXME */
>  #define TARGET_CPU_CPP_BUILTINS()
>  
> -/* Which processor to generate code or schedule for.  */
> -enum processor_type
> -{
> -  PROCESSOR_CARRIZO,
> -};
> +/* Temporarily disable libgcc until one actually exists.  */
> +#undef  LIBGCC_SPEC
> +#define LIBGCC_SPEC ""
> +
> +/* Use LLVM assembler options.  */
> +#undef ASM_SPEC
> +#define ASM_SPEC "-triple=amdgcn--amdhsa %{mgpu=*:-mcpu=%*} -filetype=obj"
>  
> -extern GTY(()) int gcn_arch;
> -extern GTY(()) int gcn_tune;
> +/* Default to relocatable executables as output.  */
> +#undef LINK_SPEC
> +#define LINK_SPEC "-shared"
>  
>  /* Support for a compile-time default architecture and tuning.  The rules are:
> -   --with-arch is ignored if -march is specified.
> -   --with-tune is ignored if -mtune is specified.  */
> +   --with-gpu is ignored if -mgpu is specified.  */
>  #define OPTION_DEFAULT_SPECS \
> -  {"arch", "%{!march=*:-march=%(VALUE)}" }, \
> -  {"tune", "%{!mtune=*:-mtune=%(VALUE)}" }
> +  {"gpu", "%{!mgpu=*:-mgpu=%(VALUE)}"}
>  
>  /* Default target_flags if no switches specified.  */
>  #ifndef TARGET_DEFAULT
> diff --git a/gcc/config/gcn/gcn.opt b/gcc/config/gcn/gcn.opt
> index 8fc02b7..77f0ef0 100644
> --- a/gcc/config/gcn/gcn.opt
> +++ b/gcc/config/gcn/gcn.opt
> @@ -17,3 +17,20 @@
>  ; You should have received a copy of the GNU General Public License
>  ; along with GCC; see the file COPYING3.  If not see
>  ; <http://www.gnu.org/licenses/>.
> +
> +HeaderInclude
> +config/gcn/gcn-opts.h
> +
> +Enum
> +Name(gpu_type) Type(enum processor_type)
> +GCN GPU type to use:
> +
> +EnumValue
> +Enum(gpu_type) String(carrizo) Value(PROCESSOR_CARRIZO)
> +
> +EnumValue
> +Enum(gpu_type) String(fiji) Value(PROCESSOR_FIJI)
> +
> +mgpu=
> +Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_gpu) Init(PROCESSOR_CARRIZO)
> +Specify the name of the target GPU.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gcn][patch] Add -mgpu option and plumb in assembler/linker
  2017-05-29 17:48 ` Martin Jambor
@ 2017-06-06 19:53   ` Andrew Stubbs
  2017-06-21 10:06     ` Andrew Stubbs
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Stubbs @ 2017-06-06 19:53 UTC (permalink / raw)
  To: Andrew Stubbs, Jan Hubicka, gcc-patches

On 29/05/17 18:27, Martin Jambor wrote:
> 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.

And I've just been on vacation for a week. :-)

> On Fri, Apr 28, 2017 at 06:06:39PM +0100, Andrew Stubbs wrote:
> At the moment I prefer to use --with-as and --with-ld configure

Those only work with absolute paths, which is not suitable for a 
toolchain that may be installed in a user's home directory, which will 
be my use case.

> despite the error message.  I guess we are fine with passing -fno-lto
> or rather disabling lto at configure time for the time being.

I configure with --disable-lto.

> 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.

Hmm, I didn't realize it would be so hard. I guess I'm joining the party 
late.

>> 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.

Thomas objects to the new option, and after talking with him the 
reasoning seems sound. GCC has been moving away from -mcpu in any case, 
so I guess I'll put -march and -mtune back, and use those for the same 
purpose.

I'll commit the patch with those changes soonish.

Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [gcn][patch] Add -mgpu option and plumb in assembler/linker
  2017-06-06 19:53   ` Andrew Stubbs
@ 2017-06-21 10:06     ` Andrew Stubbs
  2023-11-24 14:55       ` GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669] (was: [gcn][patch] Add -mgpu option and plumb in assembler/linker) Thomas Schwinge
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Stubbs @ 2017-06-21 10:06 UTC (permalink / raw)
  To: Jan Hubicka, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 420 bytes --]

On 06/06/17 20:52, Andrew Stubbs wrote:
> Thomas objects to the new option, and after talking with him the 
> reasoning seems sound. GCC has been moving away from -mcpu in any case, 
> so I guess I'll put -march and -mtune back, and use those for the same 
> purpose.
> 
> I'll commit the patch with those changes soonish.

I've finally got around to pushing the patch.

It now uses -march to set the GPU name.

Andrew


[-- Attachment #2: gcn-march.patch --]
[-- Type: text/x-patch, Size: 4793 bytes --]

Switch to HSACO2

2017-06-21  Andrew Stubbs  <ams@codesourcery.com>

	gcc/
	* config.gcc (amdgcn): Set default to "carrizo"
	* 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.
	* config/gcn/gcn.opt: Include gcn-opts.h.
	(gpu_type): New Enum.
	(march, mtune): New options.

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 32f0f4b..99c9c4a 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -3930,7 +3930,7 @@ case "${target}" in
 		for which in arch tune; do
 			eval "val=\$with_$which"
 			case ${val} in
-			"" | fiji)
+			"" | carrizo | fiji)
 				# OK
 				;;
 			*)
@@ -3939,6 +3939,7 @@ case "${target}" in
 				;;
 			esac
 		done
+		[ "x$with_arch" = x ] && with_arch=carrizo
 		;;
 
 	hppa*-*-*)
diff --git a/gcc/config/gcn/gcn-opts.h b/gcc/config/gcn/gcn-opts.h
new file mode 100644
index 0000000..d0586d6
--- /dev/null
+++ b/gcc/config/gcn/gcn-opts.h
@@ -0,0 +1,27 @@
+/* Copyright (C) 2016-2017 Free Software Foundation, Inc.
+
+   This file is free software; you can redistribute it and/or modify it under
+   the terms of the GNU General Public License as published by the Free
+   Software Foundation; either version 3 of the License, or (at your option)
+   any later version.
+
+   This file is distributed in the hope that it will be useful, but WITHOUT
+   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+   FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+   for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef GCN_OPTS_H
+#define GCN_OPTS_H
+
+/* Which processor to generate code or schedule for.  */
+enum processor_type
+{
+  PROCESSOR_CARRIZO,
+  PROCESSOR_FIJI
+};
+
+#endif
diff --git a/gcc/config/gcn/gcn.c b/gcc/config/gcn/gcn.c
index eb6edd8..c80bdf5 100644
--- a/gcc/config/gcn/gcn.c
+++ b/gcc/config/gcn/gcn.c
@@ -60,11 +60,6 @@
 /* This file should be included last.  */
 #include "target-def.h"
 
-/* Which instruction set architecture to use.  */
-int gcn_arch;
-/* Which cpu are we tuning for.  */
-int gcn_tune;
-
 static REAL_VALUE_TYPE dconst4, dconst1over2pi;
 static bool ext_gcn_constants_init = 0;
 
@@ -2006,8 +2001,8 @@ static void
 output_file_start (void)
 {
   fprintf (asm_out_file, "\t.hsatext\n");
-  fprintf (asm_out_file, "\t.hsa_code_object_version 1,0\n");
-  fprintf (asm_out_file, "\t.hsa_code_object_isa 8,0,1,\"AMD\",\"AMDGPU\"\n");
+  fprintf (asm_out_file, "\t.hsa_code_object_version 2,0\n");
+  fprintf (asm_out_file, "\t.hsa_code_object_isa\n");  /* Autodetect.  */
   fprintf (asm_out_file, "\t.section\t.AMDGPU.config\n");
   fprintf (asm_out_file, "\t.hsatext\n");
 }
diff --git a/gcc/config/gcn/gcn.h b/gcc/config/gcn/gcn.h
index 903022f..3b41095 100644
--- a/gcc/config/gcn/gcn.h
+++ b/gcc/config/gcn/gcn.h
@@ -14,18 +14,22 @@
    along with GCC; see the file COPYING3.  If not see
    <http://www.gnu.org/licenses/>.  */
 
+#include "config/gcn/gcn-opts.h"
+
 \f
 /* FIXME */
 #define TARGET_CPU_CPP_BUILTINS()
 
-/* Which processor to generate code or schedule for.  */
-enum processor_type
-{
-  PROCESSOR_CARRIZO,
-};
+/* Temporarily disable libgcc until one actually exists.  */
+#undef  LIBGCC_SPEC
+#define LIBGCC_SPEC ""
+
+/* Use LLVM assembler options.  */
+#undef ASM_SPEC
+#define ASM_SPEC "-triple=amdgcn--amdhsa %{march=*:-mcpu=%*} -filetype=obj"
 
-extern GTY(()) int gcn_arch;
-extern GTY(()) int gcn_tune;
+#undef LINK_SPEC
+#define LINK_SPEC ""
 
 /* Support for a compile-time default architecture and tuning.  The rules are:
    --with-arch is ignored if -march is specified.
diff --git a/gcc/config/gcn/gcn.opt b/gcc/config/gcn/gcn.opt
index 8fc02b7..ffb5547 100644
--- a/gcc/config/gcn/gcn.opt
+++ b/gcc/config/gcn/gcn.opt
@@ -17,3 +17,24 @@
 ; You should have received a copy of the GNU General Public License
 ; along with GCC; see the file COPYING3.  If not see
 ; <http://www.gnu.org/licenses/>.
+
+HeaderInclude
+config/gcn/gcn-opts.h
+
+Enum
+Name(gpu_type) Type(enum processor_type)
+GCN GPU type to use:
+
+EnumValue
+Enum(gpu_type) String(carrizo) Value(PROCESSOR_CARRIZO)
+
+EnumValue
+Enum(gpu_type) String(fiji) Value(PROCESSOR_FIJI)
+
+march=
+Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_arch) Init(PROCESSOR_CARRIZO)
+Specify the name of the target GPU.
+
+mtune=
+Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_tune) Init(PROCESSOR_CARRIZO)
+Specify the name of the target GPU.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669] (was: [gcn][patch] Add -mgpu option and plumb in assembler/linker)
  2017-06-21 10:06     ` Andrew Stubbs
@ 2023-11-24 14:55       ` Thomas Schwinge
  2023-11-24 15:06         ` GCN: Remove 'last_arg' spec function (was: GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669]) Thomas Schwinge
  2023-11-24 15:55         ` GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669] Andrew Stubbs
  0 siblings, 2 replies; 13+ messages in thread
From: Thomas Schwinge @ 2023-11-24 14:55 UTC (permalink / raw)
  To: Andrew Stubbs, Julian Brown, gcc-patches, Joseph Myers

[-- Attachment #1: Type: text/plain, Size: 1602 bytes --]

Hi!

On 2017-06-21T11:06:24+0100, Andrew Stubbs <ams@codesourcery.com> wrote:
> --- a/gcc/config/gcn/gcn.opt
> +++ b/gcc/config/gcn/gcn.opt

> +march=
> +Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_arch) Init(PROCESSOR_CARRIZO)
> +Specify the name of the target GPU.
> +
> +mtune=
> +Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_tune) Init(PROCESSOR_CARRIZO)
> +Specify the name of the target GPU.

OK to push the attached
"GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669]"?

For reference, see also:

  - [...]
  - commit 718a3eab2906217c70f27077446fa0e8ddb1bf7b "[arm] Mark -marm and -mthumb as being inverse options"
  - commit aebe10d48ccc217273ee8a4e2c3805ed1e173a78 "driver: Also prune joined switches with negation" ['gcc/config/i386/i386.opt']
  - commit 6fdbe41963a7aecad80f27e9805c7e18cbd4ab48 "driver: Also prune joined switches with negation" ['gcc/config/aarch64/aarch64.opt', 'gcc/config/arm/arm.opt']
  - commit 17f2908fcf058e145cff275966e34f8c7f57c2c5 "RISC-V: For '-march' and '-mabi' options, add 'Negative' property mentions itself"
  - commit 69c426b89579312af91035c26fb1e270bfbcad00 "doc/options.texi: Fix the description of 'Negative'"


(..., and yes, GCC/nvptx does need similar treatment...)


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-GCN-Tag-march-.-mtune-.-as-Negative-of-themselves-PR.patch --]
[-- Type: text/x-diff, Size: 2235 bytes --]

From 0e03aa39bd5fbae8a5993bf9bf6dad34f9e85c2d Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas_schwinge@mentor.com>
Date: Wed, 22 Nov 2023 17:35:23 +0100
Subject: [PATCH] GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of
 themselves [PR112669]

Certain other command-line flags are mutually exclusive (random example: GCN
'-march=gfx906', '-march=gfx908').  If they're not appropriately marked up,
this does disturb the multilib selection machinery, for example:

    $ build-gcc-offload-amdgcn-amdhsa/gcc/xgcc -print-multi-directory -march=gfx906
    gfx906
    $ build-gcc-offload-amdgcn-amdhsa/gcc/xgcc -print-multi-directory -march=gfx908
    gfx908
    $ build-gcc-offload-amdgcn-amdhsa/gcc/xgcc -print-multi-directory -march=gfx906 -march=gfx908
    .

In the last invocation, '-march=gfx900 -march=gfx906', for example, in
'gcc/gcc.cc:set_multilib_dir' we see both flags -- which there doesn't exist a
matching multilib for, therefore we "fail" to the default ('.').  Tagges as
'Negative', only the last flag survives, and we, for example, get the expected:

    $ build-gcc-offload-amdgcn-amdhsa/gcc/xgcc -print-multi-directory -march=gfx906 -march=gfx908
    gfx908

I quickly found that the same also applies to GCN's '-mtune=[...]', but I've
not otherwise reviewed the GCN options.

	PR target/112669
	gcc/
	* config/gcn/gcn.opt (march=, mtune=): Tag as 'Negative' of
	themselves.
---
 gcc/config/gcn/gcn.opt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/config/gcn/gcn.opt b/gcc/config/gcn/gcn.opt
index 7a852c51c84..e5db6df92d7 100644
--- a/gcc/config/gcn/gcn.opt
+++ b/gcc/config/gcn/gcn.opt
@@ -44,11 +44,11 @@ EnumValue
 Enum(gpu_type) String(gfx1030) Value(PROCESSOR_GFX1030)
 
 march=
-Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_arch) Init(PROCESSOR_FIJI)
+Target RejectNegative Negative(march=) Joined ToLower Enum(gpu_type) Var(gcn_arch) Init(PROCESSOR_FIJI)
 Specify the name of the target GPU.
 
 mtune=
-Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_tune) Init(PROCESSOR_FIJI)
+Target RejectNegative Negative(mtune=) Joined ToLower Enum(gpu_type) Var(gcn_tune) Init(PROCESSOR_FIJI)
 Specify the name of the target GPU.
 
 m32
-- 
2.34.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* GCN: Remove 'last_arg' spec function (was: GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669])
  2023-11-24 14:55       ` GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669] (was: [gcn][patch] Add -mgpu option and plumb in assembler/linker) Thomas Schwinge
@ 2023-11-24 15:06         ` Thomas Schwinge
  2023-11-24 15:55           ` GCN: Remove 'last_arg' spec function Andrew Stubbs
  2023-11-24 15:55         ` GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669] Andrew Stubbs
  1 sibling, 1 reply; 13+ messages in thread
From: Thomas Schwinge @ 2023-11-24 15:06 UTC (permalink / raw)
  To: Andrew Stubbs, Julian Brown, gcc-patches; +Cc: Joseph Myers

[-- Attachment #1: Type: text/plain, Size: 556 bytes --]

Hi!

On 2023-11-24T15:55:52+0100, I wrote:
> OK to push the attached
> "GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669]"?

With that in place, we may then "GCN: Remove 'last_arg' spec function",
see attached; OK to push?


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-GCN-Remove-last_arg-spec-function.patch --]
[-- Type: text/x-diff, Size: 4091 bytes --]

From 0f171ad0253a2375131281e4dc629b8dce5f891d Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 22 Nov 2023 15:39:50 +0100
Subject: [PATCH] GCN: Remove 'last_arg' spec function

The LLVM 13.0.1 assembler ('llvm-mc') indeed still does complain in presence of
multiple '-mcpu=[...]' options:

    as: for the --mcpu option: may only occur zero or one times!

However, as of
"GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669]",
the GCC-side special handling is no longer necessary.

	gcc/
	* config.gcc <amdgcn-*-amdhsa> (extra_gcc_objs): Don't set.
	* config/gcn/driver-gcn.cc: Remove.
	* config/gcn/gcn-hsa.h (ASM_SPEC, EXTRA_SPEC_FUNCTIONS): Remove
	'last_arg' spec function.
	* config/gcn/t-gcn-hsa (driver-gcn.o): Remove.
---
 gcc/config.gcc               |  1 -
 gcc/config/gcn/driver-gcn.cc | 32 --------------------------------
 gcc/config/gcn/gcn-hsa.h     |  8 +-------
 gcc/config/gcn/t-gcn-hsa     |  4 ----
 4 files changed, 1 insertion(+), 44 deletions(-)
 delete mode 100644 gcc/config/gcn/driver-gcn.cc

diff --git a/gcc/config.gcc b/gcc/config.gcc
index f4fa45d709e..990fd3dea61 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1770,7 +1770,6 @@ amdgcn-*-amdhsa)
 	native_system_header_dir=/include
 	extra_modes=gcn/gcn-modes.def
 	extra_objs="${extra_objs} gcn-tree.o"
-	extra_gcc_objs="driver-gcn.o"
 	case "$host" in
 	x86_64*-*-linux-gnu )
 		if test "$ac_cv_search_dlopen" != no; then
diff --git a/gcc/config/gcn/driver-gcn.cc b/gcc/config/gcn/driver-gcn.cc
deleted file mode 100644
index 837633a6e37..00000000000
--- a/gcc/config/gcn/driver-gcn.cc
+++ /dev/null
@@ -1,32 +0,0 @@
-/* Subroutines for the gcc driver.
-   Copyright (C) 2018-2023 Free Software Foundation, Inc.
-
-This file is part of GCC.
-
-GCC is free software; you can redistribute it and/or modify
-it under the terms of the GNU General Public License as published by
-the Free Software Foundation; either version 3, or (at your option)
-any later version.
-
-GCC is distributed in the hope that it will be useful,
-but WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
-
-You should have received a copy of the GNU General Public License
-along with GCC; see the file COPYING3.  If not see
-<http://www.gnu.org/licenses/>.  */
-
-#include "config.h"
-#include "system.h"
-#include "coretypes.h"
-#include "tm.h"
-
-const char *
-last_arg_spec_function (int argc, const char **argv)
-{
-  if (argc == 0)
-    return NULL;
-
-  return argv[argc-1];
-}
diff --git a/gcc/config/gcn/gcn-hsa.h b/gcc/config/gcn/gcn-hsa.h
index d986ad2ce22..09f8cbbaadf 100644
--- a/gcc/config/gcn/gcn-hsa.h
+++ b/gcc/config/gcn/gcn-hsa.h
@@ -88,7 +88,7 @@ extern unsigned int gcn_local_sym_hash (const char *name);
 
 /* Use LLVM assembler and linker options.  */
 #define ASM_SPEC  "-triple=amdgcn--amdhsa "  \
-		  "%:last_arg(%{march=*:-mcpu=%*}) " \
+		  "%{march=*:-mcpu=%*} " \
 		  "%{!march=*|march=fiji:--amdhsa-code-object-version=3} " \
 		  "%{" NO_XNACK XNACKOPT "}" \
 		  "%{" NO_SRAM_ECC SRAMOPT "} " \
@@ -111,12 +111,6 @@ extern unsigned int gcn_local_sym_hash (const char *name);
   "%{foffload-abi=lp64:%eBOO2} "
 #define STANDARD_STARTFILE_PREFIX_2 ""
 
-/* The LLVM assembler rejects multiple -mcpu options, so we must drop
-   all but the last.  */
-extern const char *last_arg_spec_function (int argc, const char **argv);
-#define EXTRA_SPEC_FUNCTIONS	\
-    { "last_arg", last_arg_spec_function },
-
 #undef LOCAL_INCLUDE_DIR
 
 /* FIXME: Review debug info settings.
diff --git a/gcc/config/gcn/t-gcn-hsa b/gcc/config/gcn/t-gcn-hsa
index 18db7075356..e2aec71b161 100644
--- a/gcc/config/gcn/t-gcn-hsa
+++ b/gcc/config/gcn/t-gcn-hsa
@@ -16,10 +16,6 @@
 
 GTM_H += $(HASH_TABLE_H)
 
-driver-gcn.o: $(srcdir)/config/gcn/driver-gcn.cc
-	$(COMPILE) $<
-	$(POSTCOMPILE)
-
 CFLAGS-mkoffload.o += $(DRIVER_DEFINES) \
 	-DGCC_INSTALL_NAME=\"$(GCC_INSTALL_NAME)\"
 mkoffload.o: $(srcdir)/config/gcn/mkoffload.cc
-- 
2.34.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669]
  2023-11-24 14:55       ` GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669] (was: [gcn][patch] Add -mgpu option and plumb in assembler/linker) Thomas Schwinge
  2023-11-24 15:06         ` GCN: Remove 'last_arg' spec function (was: GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669]) Thomas Schwinge
@ 2023-11-24 15:55         ` Andrew Stubbs
  1 sibling, 0 replies; 13+ messages in thread
From: Andrew Stubbs @ 2023-11-24 15:55 UTC (permalink / raw)
  To: Thomas Schwinge, Julian Brown, gcc-patches, Joseph Myers

On 24/11/2023 14:55, Thomas Schwinge wrote:
> Hi!
> 
> On 2017-06-21T11:06:24+0100, Andrew Stubbs <ams@codesourcery.com> wrote:
>> --- a/gcc/config/gcn/gcn.opt
>> +++ b/gcc/config/gcn/gcn.opt
> 
>> +march=
>> +Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_arch) Init(PROCESSOR_CARRIZO)
>> +Specify the name of the target GPU.
>> +
>> +mtune=
>> +Target RejectNegative Joined ToLower Enum(gpu_type) Var(gcn_tune) Init(PROCESSOR_CARRIZO)
>> +Specify the name of the target GPU.
> 
> OK to push the attached
> "GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669]"?
> 
> For reference, see also:
> 
>    - [...]
>    - commit 718a3eab2906217c70f27077446fa0e8ddb1bf7b "[arm] Mark -marm and -mthumb as being inverse options"
>    - commit aebe10d48ccc217273ee8a4e2c3805ed1e173a78 "driver: Also prune joined switches with negation" ['gcc/config/i386/i386.opt']
>    - commit 6fdbe41963a7aecad80f27e9805c7e18cbd4ab48 "driver: Also prune joined switches with negation" ['gcc/config/aarch64/aarch64.opt', 'gcc/config/arm/arm.opt']
>    - commit 17f2908fcf058e145cff275966e34f8c7f57c2c5 "RISC-V: For '-march' and '-mabi' options, add 'Negative' property mentions itself"
>    - commit 69c426b89579312af91035c26fb1e270bfbcad00 "doc/options.texi: Fix the description of 'Negative'"
> 
> 
> (..., and yes, GCC/nvptx does need similar treatment...)

OK.

Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: GCN: Remove 'last_arg' spec function
  2023-11-24 15:06         ` GCN: Remove 'last_arg' spec function (was: GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669]) Thomas Schwinge
@ 2023-11-24 15:55           ` Andrew Stubbs
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Stubbs @ 2023-11-24 15:55 UTC (permalink / raw)
  To: Thomas Schwinge, Julian Brown, gcc-patches; +Cc: Joseph Myers

On 24/11/2023 15:06, Thomas Schwinge wrote:
> Hi!
> 
> On 2023-11-24T15:55:52+0100, I wrote:
>> OK to push the attached
>> "GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669]"?
> 
> With that in place, we may then "GCN: Remove 'last_arg' spec function",
> see attached; OK to push?

OK.

Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-11-24 15:55 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-28 17:23 [gcn][patch] Add -mgpu option and plumb in assembler/linker Andrew Stubbs
2017-04-28 17:27 ` Joseph Myers
2017-05-02 17:12 ` Martin Jambor
2017-05-03  9:30   ` Andrew Stubbs
2017-05-16 10:37   ` Andrew Stubbs
2017-05-29 17:48 ` Martin Jambor
2017-06-06 19:53   ` Andrew Stubbs
2017-06-21 10:06     ` Andrew Stubbs
2023-11-24 14:55       ` GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669] (was: [gcn][patch] Add -mgpu option and plumb in assembler/linker) Thomas Schwinge
2023-11-24 15:06         ` GCN: Remove 'last_arg' spec function (was: GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669]) Thomas Schwinge
2023-11-24 15:55           ` GCN: Remove 'last_arg' spec function Andrew Stubbs
2023-11-24 15:55         ` GCN: Tag '-march=[...]', '-mtune=[...]' as 'Negative' of themselves [PR112669] Andrew Stubbs
2017-06-01  8:52 ` [gcn][patch] Add -mgpu option and plumb in assembler/linker Thomas Schwinge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).