GCN: Restore lost '__gfx90a__' target CPU definition (was: [Patch] GCN: Add pre-initial support for gfx1100)
Hi!
On 2024-01-07T20:20:19+0100, Tobias Burnus wrote:
> --- a/gcc/config/gcn/gcn.h
> +++ b/gcc/config/gcn/gcn.h
> @@ -30,6 +30,8 @@
> builtin_define ("__CDNA2__"); \
>else if (TARGET_RDNA2)
> \
> builtin_define ("__RDNA2__"); \
> + else if (TARGET_RDNA3)
> \
> + builtin_define ("__RDNA3__"); \
>if (TARGET_FIJI)
> \
> { \
> builtin_define ("__fiji__"); \
> @@ -41,11 +43,13 @@
> builtin_define ("__gfx906__"); \
>else if (TARGET_GFX908)
> \
> builtin_define ("__gfx908__"); \
> - else if (TARGET_GFX90a)
> \
> - builtin_define ("__gfx90a__"); \
> + else if (TARGET_GFX1030)
> \
> + builtin_define ("__gfx1030"); \
> + else if (TARGET_GFX1100)
> \
> + builtin_define ("__gfx1100__");\
>} while (0)
Supposedly it wasn't intentional that we lost gfx90a here -- I've pushed
to master branch commit 159174f25716c18a74a915cb01b9a28024ea7a3d
"GCN: Restore lost '__gfx90a__' target CPU definition", see attached.
Grüße
Thomas
>From 159174f25716c18a74a915cb01b9a28024ea7a3d Mon Sep 17 00:00:00 2001
From: Thomas Schwinge
Date: Thu, 8 Feb 2024 23:27:19 +0100
Subject: [PATCH] GCN: Restore lost '__gfx90a__' target CPU definition
Also, add some safeguards for the future.
Fix-up for commit 52a2c659ae6c21f84b6acce0afcb9b93b9dc71a0
"GCN: Add pre-initial support for gfx1100".
gcc/
* config/gcn/gcn.h (TARGET_CPU_CPP_BUILTINS): Restore lost
'__gfx90a__' target CPU definition. Add some safeguards for the future.
---
gcc/config/gcn/gcn.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/gcc/config/gcn/gcn.h b/gcc/config/gcn/gcn.h
index a17f16aacc40..c314c7b4ae8e 100644
--- a/gcc/config/gcn/gcn.h
+++ b/gcc/config/gcn/gcn.h
@@ -32,6 +32,8 @@
builtin_define ("__RDNA2__"); \
else if (TARGET_RDNA3) \
builtin_define ("__RDNA3__"); \
+ else \
+ gcc_unreachable ();\
if (TARGET_FIJI) \
{ \
builtin_define ("__fiji__"); \
@@ -43,10 +45,14 @@
builtin_define ("__gfx906__"); \
else if (TARGET_GFX908) \
builtin_define ("__gfx908__"); \
+ else if (TARGET_GFX90a) \
+ builtin_define ("__gfx90a__"); \
else if (TARGET_GFX1030) \
builtin_define ("__gfx1030"); \
else if (TARGET_GFX1100) \
builtin_define ("__gfx1100__");\
+ else \
+ gcc_unreachable ();\
} while (0)
#define ASSEMBLER_DIALECT (TARGET_RDNA2_PLUS ? 1 : 0)
--
2.43.0
Re: [Patch] GCN: Add pre-initial support for gfx1100
Hi!
On 2024-01-08T15:30:06+0100, Tobias Burnus wrote:
> Andrew Stubbs wrote:
>> I know there will be things that need fixing for
>> both experimental architectures.
>
> Indeed. [...]
..., like, making it even build? ;-P
>> P.S. Apologies, but I think my commits today conflict a little; you
>> should be able to drop the hunks that patch deleted code.
>
> I did so - but I then realized that I should have also added gfx1100 to
> the new chunk.
>
> Committed as r14-7006-g97a52f69d209f6 (see attachment) - as follow up to
> the original r14-7005-g52a2c659ae6c21
Pushed to master branch commit f9290cdf4697f467fd0fb7c710f58cc12e497889
"GCN: Add pre-initial support for gfx1100: 'EF_AMDGPU_MACH_AMDGCN_GFX1100'",
see attached.
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
>From f9290cdf4697f467fd0fb7c710f58cc12e497889 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge
Date: Mon, 8 Jan 2024 20:35:27 +0100
Subject: [PATCH] GCN: Add pre-initial support for gfx1100:
'EF_AMDGPU_MACH_AMDGCN_GFX1100'
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
../../../source-gcc/libgomp/plugin/plugin-gcn.c: In function ‘isa_hsa_name’:
../../../source-gcc/libgomp/plugin/plugin-gcn.c:1666:10: error: ‘EF_AMDGPU_MACH_AMDGCN_GFX1100’ undeclared (first use in this function); did you mean ‘EF_AMDGPU_MACH_AMDGCN_GFX1030’?
1666 | case EF_AMDGPU_MACH_AMDGCN_GFX1100:
| ^
| EF_AMDGPU_MACH_AMDGCN_GFX1030
../../../source-gcc/libgomp/plugin/plugin-gcn.c:1666:10: note: each undeclared identifier is reported only once for each function it appears in
../../../source-gcc/libgomp/plugin/plugin-gcn.c: In function ‘isa_code’:
../../../source-gcc/libgomp/plugin/plugin-gcn.c:1711:12: error: ‘EF_AMDGPU_MACH_AMDGCN_GFX1100’ undeclared (first use in this function); did you mean ‘EF_AMDGPU_MACH_AMDGCN_GFX1030’?
1711 | return EF_AMDGPU_MACH_AMDGCN_GFX1100;
|^
|EF_AMDGPU_MACH_AMDGCN_GFX1030
../../../source-gcc/libgomp/plugin/plugin-gcn.c: In function ‘max_isa_vgprs’:
../../../source-gcc/libgomp/plugin/plugin-gcn.c:1728:10: error: ‘EF_AMDGPU_MACH_AMDGCN_GFX1100’ undeclared (first use in this function); did you mean ‘EF_AMDGPU_MACH_AMDGCN_GFX1030’?
1728 | case EF_AMDGPU_MACH_AMDGCN_GFX1100:
| ^
| EF_AMDGPU_MACH_AMDGCN_GFX1030
make[4]: *** [Makefile:813: libgomp_plugin_gcn_la-plugin-gcn.lo] Error 1
Fix-up for commit 52a2c659ae6c21f84b6acce0afcb9b93b9dc71a0
"GCN: Add pre-initial support for gfx1100".
libgomp/
* plugin/plugin-gcn.c (EF_AMDGPU_MACH): Add
'EF_AMDGPU_MACH_AMDGCN_GFX1100'.
---
libgomp/plugin/plugin-gcn.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libgomp/plugin/plugin-gcn.c b/libgomp/plugin/plugin-gcn.c
index f24a28faa22..0339848451e 100644
--- a/libgomp/plugin/plugin-gcn.c
+++ b/libgomp/plugin/plugin-gcn.c
@@ -389,7 +389,8 @@ typedef enum {
EF_AMDGPU_MACH_AMDGCN_GFX906 = 0x02f,
EF_AMDGPU_MACH_AMDGCN_GFX908 = 0x030,
EF_AMDGPU_MACH_AMDGCN_GFX90a = 0x03f,
- EF_AMDGPU_MACH_AMDGCN_GFX1030 = 0x036
+ EF_AMDGPU_MACH_AMDGCN_GFX1030 = 0x036,
+ EF_AMDGPU_MACH_AMDGCN_GFX1100 = 0x041
} EF_AMDGPU_MACH;
const static int EF_AMDGPU_MACH_MASK = 0x00ff;
--
2.34.1
Re: [Patch] GCN: Add pre-initial support for gfx1100
Hi Andrew, Andrew Stubbs wrote: OK for mainline ? This looks fine to me. I know there will be things that need fixing for both experimental architectures. Indeed. I tried to be a bit more verbose also to avoid too high expectations by occasional gcc-patches@ readers. P.S. Apologies, but I think my commits today conflict a little; you should be able to drop the hunks that patch deleted code. I did so - but I then realized that I should have also added gfx1100 to the new chunk. Committed as r14-7006-g97a52f69d209f6 (see attachment) - as follow up to the original r14-7005-g52a2c659ae6c21 Tobiascommit 97a52f69d209f69e755ffad6897c7176da9ac686 Author: Tobias Burnus Date: Mon Jan 8 15:18:10 2024 +0100 amdgcn: Add gfx1100 to new XNACK defaults in mkoffload Commit r14-6997-g78dff4c25c1b95 added an arch-dependent SET_XNACK_OFF vs. SET_XNACK_ANY check; that was added between writing and committing the add-gfx1100 commit r14-7005-g52a2c659ae6c21 - and I missed to add it there. gcc/ChangeLog: * config/gcn/mkoffload.cc (main): Handle gfx1100 when setting the default XNACK. --- gcc/config/gcn/mkoffload.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/config/gcn/mkoffload.cc b/gcc/config/gcn/mkoffload.cc index 2cd201d56ca..d4cd509089e 100644 --- a/gcc/config/gcn/mkoffload.cc +++ b/gcc/config/gcn/mkoffload.cc @@ -1018,6 +1018,7 @@ main (int argc, char **argv) case EF_AMDGPU_MACH_AMDGCN_GFX906: case EF_AMDGPU_MACH_AMDGCN_GFX908: case EF_AMDGPU_MACH_AMDGCN_GFX1030: +case EF_AMDGPU_MACH_AMDGCN_GFX1100: SET_XNACK_OFF (elf_flags); break; case EF_AMDGPU_MACH_AMDGCN_GFX90a:
Re: [Patch] GCN: Add pre-initial support for gfx1100
On 07/01/2024 19:20, Tobias Burnus wrote: ROCm meanwhile supports also some consumer cards; besides the semi-new gfx1030, support for gfx1100 was added more recently (in ROCm 5.7.1 for "Ubuntu 22.04 only" and without parenthesis since ROCm 6.0.0). GCC has already very limited support for gfx1030 - whose multlib support is - on purpose - not yet enabled by default and is WIP. The attached patch now adds gfx1100 on top of it, assuming that it mostly behaves the same as gfx1030. This is really WIP as there are known build (assembly) issues (see below) and not only "just" runtime issues. gfx1100 differs at least in the following aspects from the previously supported cards: * gfx1100 has an 'architected flat scratch' which is different from 'absolute flat scratch' which all others (but fiji: 'offset flat scratch') have. Hence, '.amdhsa_reserve_flat_scratch 0' has to be excluded to avoid assembly errors. * gfx1100 also does not support 'v_mov_b32_sdwa', failing to assembly libc/argz/libc_a-argz_stringify.o with: "sdwa variant of this instruction is not supported" → This has not been address in the patch, hence, specifying gfx1100 in --with-multilib-list= will fail to build when an in-tree newlib is build. * * * The attached patch fixes in addition one issue in libgomp (string-length len constant is too short for gfx1030 (and gfx1100) = 7 characters) and it includes the fix that __gfx1030__ is not defined, which I have submitted separately (yesterday). With the caveat that gfx1100 is even less usable than gfx1030 and it won't build newlib, is it nonetheless OK for mainline ? (As gfx1100 is not enabled by default in multilib, a regular build will will not fail and I think the *.md issue can be addressed separately.) This looks fine to me. I know there will be things that need fixing for both experimental architectures. Andrew P.S. Apologies, but I think my commits today conflict a little; you should be able to drop the hunks that patch deleted code.
[Patch] GCN: Add pre-initial support for gfx1100
ROCm meanwhile supports also some consumer cards; besides the semi-new
gfx1030, support for gfx1100 was added more recently (in ROCm 5.7.1 for
"Ubuntu 22.04 only" and without parenthesis since ROCm 6.0.0).
GCC has already very limited support for gfx1030 - whose multlib support
is - on purpose - not yet enabled by default and is WIP.
The attached patch now adds gfx1100 on top of it, assuming that it
mostly behaves the same as gfx1030. This is really WIP as there are
known build (assembly) issues (see below) and not only "just" runtime
issues.
gfx1100 differs at least in the following aspects from the previously
supported cards:
* gfx1100 has an 'architected flat scratch' which is different from
'absolute flat scratch' which all others (but fiji: 'offset flat
scratch') have. Hence, '.amdhsa_reserve_flat_scratch 0'
has to be excluded to avoid assembly errors.
* gfx1100 also does not support 'v_mov_b32_sdwa', failing to assembly
libc/argz/libc_a-argz_stringify.o with:
"sdwa variant of this instruction is not supported"
→ This has not been address in the patch, hence, specifying gfx1100 in
--with-multilib-list= will fail to build when an in-tree newlib is build.
* * *
The attached patch fixes in addition one issue in libgomp (string-length
len constant is too short for gfx1030 (and gfx1100) = 7 characters) and
it includes the fix that __gfx1030__ is not defined, which I have
submitted separately (yesterday).
With the caveat that gfx1100 is even less usable than gfx1030 and it
won't build newlib, is it nonetheless
OK for mainline ?
(As gfx1100 is not enabled by default in multilib, a regular build will
will not fail and I think the *.md issue can be addressed separately.)
TobiasGCN: Add pre-initial support for gfx1100
ROCm since 5.7.1 supports gfx1100 (RDNA3) cards. This commit adds support
for it, mostly by assuming gfx1100 behaves identical to gfx1030. Like gfx1030,
gfx1100 support is neither documented nor the build of the multilib enabled by
default.
But contrary to gfx1030, gfx1100 has a known issue causing some libraries not
to build, including newlib: The sdwa variant of v_mov_b32_sdwa is not supported
by the hardware but GCC current does generates this instruction.
This will be addressed in a later commit.
gcc/ChangeLog:
* config.gcc (amdgcn-*-amdhsa): Accept --with-arch=gfx1100.
* config/gcn/gcn-hsa.h (NO_XNACK): Add gfx1100:
(ASM_SPEC): Handle gfx1100.
* config/gcn/gcn-opts.h (enum processor_type): Add PROCESSOR_GFX1100.
(enum gcn_isa): Add ISA_RDNA3.
(TARGET_GFX1100, TARGET_RDNA2_PLUS, TARGET_RDNA3): Define.
* config/gcn/gcn-valu.md: Change TARGET_RDNA2 to TARGET_RDNA2_PLUS.
* config/gcn/gcn.cc (gcn_option_override,
gcn_omp_device_kind_arch_isa, output_file_start): Handle gfx1100.
(gcn_global_address_p, gcn_addr_space_legitimate_address_p): Change
TARGET_RDNA2 to TARGET_RDNA2_PLUS.
(gcn_hsa_declare_function_name): Don't use '.amdhsa_reserve_flat_scratch'
with gfx1100.
* config/gcn/gcn.h (ASSEMBLER_DIALECT): Likewise.
(TARGET_CPU_CPP_BUILTINS): Define __RDNA3__, __gfx1030__ and
__gfx1100__.
* config/gcn/gcn.md: Change TARGET_RDNA2 to TARGET_RDNA2_PLUS.
* config/gcn/gcn.opt (Enum gpu_type): Add gfx1100.
* config/gcn/mkoffload.cc (EF_AMDGPU_MACH_AMDGCN_GFX1100): Define.
(isa_has_combined_avgprs, main): Handle gfx1100.
* config/gcn/t-omp-device (isa): Add gfx1100.
libgomp/ChangeLog:
* plugin/plugin-gcn.c (gcn_gfx1100_s): New const string.
(gcn_isa_name_len): Fix length.
(isa_hsa_name, isa_code, max_isa_vgprs): Handle gfx1100.
gcc/config.gcc | 2 +-
gcc/config/gcn/gcn-hsa.h| 4 ++--
gcc/config/gcn/gcn-opts.h | 7 +-
gcc/config/gcn/gcn-valu.md | 10
gcc/config/gcn/gcn.cc | 29 ---
gcc/config/gcn/gcn.h| 10 +---
gcc/config/gcn/gcn.md | 32 -
gcc/config/gcn/gcn.opt | 3 +++
gcc/config/gcn/mkoffload.cc | 5
gcc/config/gcn/t-omp-device | 2 +-
gcc/tree-vect-loop-manip.cc | 16 +
gcc/tree-vect-loop.cc | 58 ++---
libgomp/plugin/plugin-gcn.c | 9 ++-
13 files changed, 119 insertions(+), 68 deletions(-)
diff --git a/gcc/config.gcc b/gcc/config.gcc
index ce40b7758dd..7e583390024 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4548,7 +4548,7 @@ case "${target}" in
for which in arch tune; do
eval "val=\$with_$which"
case ${val} in
- "" | fiji | gfx900 | gfx906 | gfx908 | gfx90a | gfx1030)
+ "" | fiji | gfx900 | gfx906 | gfx908 | gfx90a | gfx1030 | gfx1100)
# OK
;;
*)
diff --git a/gcc/config/gcn/gcn-hsa.h b/gcc/config/gcn/gcn-hsa.h
index 43bbe0411a3..bf7079fbbc6 100644
--- a/gcc/config/gcn/gcn-hsa.h
+++ b/gcc/config/gcn/gcn-hsa.h
@@ -75,7 +75,7 @@ extern unsigned int gcn_local_sym_hash (const char *name);
supported for gcn. */
#define GOMP_SELF_SPECS ""
-#define NO_XNACK "march=fiji:;march=gfx1030:;" \
+#define NO_XNACK "marc
