diff mbox series

amdgcn: config.gcc - enable gfx1100 multilib; add gfx1100 to docs (was: [PATCH] amdgcn: additional gfx1100 support)

Message ID 14019d71-7ffb-40e6-892f-5d8168242614@baylibre.com
State New
Headers show
Series amdgcn: config.gcc - enable gfx1100 multilib; add gfx1100 to docs (was: [PATCH] amdgcn: additional gfx1100 support) | expand

Commit Message

Tobias Burnus Jan. 24, 2024, 4:01 p.m. UTC 
This patch obviously depends on Andrew's; he wrote in the previous email 
of this thread regarding his patch:

Andrew Stubbs wrote:
> This is enough to get gfx1100 working for most purposes, on top of the
> patch that Tobias committed a week or so ago; there are still some test
> failures to investigate, and probably some tuning to do.
> 
> It might also get gfx1030 working too. @Richi, could you test it,
> please?
> 
> I can't test the other multilibs right now. @PA, can you test it please?
> 
> I can self-approve the patch, but I'll hold off the commit until the
> test results come back.

Okay to enable gfx1100 multilib building and to document gfx1100 in the 
manual?

(I mean, obviously, only after Andrew committed his patch. For gfx1030, 
we might eventually also enable gfx1030 multilib support; if Richard 
confirms that collaterally fixes gfx1030, we probably should - and 
depending on the number/kinds of testsuite, we could then document it
or not, I guess.)

Tobias

Comments

Tobias Burnus Jan. 26, 2024, 12:26 p.m. UTC | #1 
Hi all, hi Richard & Andrew,

Am 24.01.24 um 17:01 schrieb Tobias Burnus:
> This patch obviously depends on Andrew's; he wrote in the previous 
> email of this thread regarding his patch:
>
> Andrew Stubbs wrote:
>> This is enough to get gfx1100 working for most purposes, on top of the
>> patch that Tobias committed a week or so ago; there are still some test
>> failures to investigate, and probably some tuning to do.
>>
>> It might also get gfx1030 working too. @Richi, could you test it,
>> please?

If gfx1030 doesn't work, I would propose my patch previously in the 
thread, https://gcc.gnu.org/pipermail/gcc-patches/2024-January/643835.html

This patch assumes that both gfx1100 and gfx1030 are now working:

> Okay to enable gfx1100 multilib building and to document gfx1100 in 
> the manual?
and, with this patch, additionally gfx1030?

OK for mainline, once Andrew's patch is in?

Tobias
Tobias Burnus Jan. 26, 2024, 12:32 p.m. UTC | #2 
Now with patch ...

Tobias Burnus wrote:
> Hi all, hi Richard & Andrew,
>
> Am 24.01.24 um 17:01 schrieb Tobias Burnus:
>> This patch obviously depends on Andrew's; he wrote in the previous 
>> email of this thread regarding his patch:
>>
>> Andrew Stubbs wrote:
>>> This is enough to get gfx1100 working for most purposes, on top of the
>>> patch that Tobias committed a week or so ago; there are still some test
>>> failures to investigate, and probably some tuning to do.
>>>
>>> It might also get gfx1030 working too. @Richi, could you test it,
>>> please?
>
> If gfx1030 doesn't work, I would propose my patch previously in the 
> thread, 
> https://gcc.gnu.org/pipermail/gcc-patches/2024-January/643835.html
>
> This patch assumes that both gfx1100 and gfx1030 are now working:
>
>> Okay to enable gfx1100 multilib building and to document gfx1100 in 
>> the manual?
> and, with this patch, additionally gfx1030?
>
> OK for mainline, once Andrew's patch is in? 
Tobias
Richard Biener Jan. 26, 2024, 12:40 p.m. UTC | #3 
On Fri, 26 Jan 2024, Tobias Burnus wrote:

> Now with patch ...
> 
> Tobias Burnus wrote:
> > Hi all, hi Richard & Andrew,
> >
> > Am 24.01.24 um 17:01 schrieb Tobias Burnus:
> >> This patch obviously depends on Andrew's; he wrote in the previous email of
> >> this thread regarding his patch:
> >>
> >> Andrew Stubbs wrote:
> >>> This is enough to get gfx1100 working for most purposes, on top of the
> >>> patch that Tobias committed a week or so ago; there are still some test
> >>> failures to investigate, and probably some tuning to do.
> >>>
> >>> It might also get gfx1030 working too. @Richi, could you test it,
> >>> please?
> >
> > If gfx1030 doesn't work, I would propose my patch previously in the thread,
> > https://gcc.gnu.org/pipermail/gcc-patches/2024-January/643835.html
> >
> > This patch assumes that both gfx1100 and gfx1030 are now working:
> >
> >> Okay to enable gfx1100 multilib building and to document gfx1100 in the
> >> manual?
> > and, with this patch, additionally gfx1030?
> >
> > OK for mainline, once Andrew's patch is in? 
> Tobias

Looks good to me.

+@item gfx1030
+Compile for RDNA2 gfx1030 devices (GFX10 series).
+
+@item gfx1100
+Compile for RDNA3 gfx1100 devices (GFX11 series).

Btw, "GFX10" series isn't precise as it's only the high-end parts
that are covered by gfx1030, there's gfx103[0-6] where hopefully
at least gfx1031, gfx1032 and gfx1034 (the dGPU variants) are
trivial to support as well(?).  Using gfx103x might be better
that way, OTOH if APU vs dGPU will make a compilation target
difference then gfx103d vs gfx103a maybe?  "GFX10" series might
be also not know to users, but I'm unsure we can list AMD
product names here?

Richard.
Tobias Burnus Jan. 26, 2024, 12:59 p.m. UTC | #4 
Hi Richard,

Richard Biener wrote:
> Looks good to me.
Thanks - I will commit it after lunch to see whether someone else has 
additional comments.
> +@item gfx1030
> +Compile for RDNA2 gfx1030 devices (GFX10 series).
> +
> +@item gfx1100
> +Compile for RDNA3 gfx1100 devices (GFX11 series).
>
> Btw, "GFX10" series isn't precise as it's only the high-end parts
> that are covered by gfx1030, there's gfx103[0-6] where hopefully
> at least gfx1031, gfx1032 and gfx1034 (the dGPU variants) are
> trivial to support as well(?).
>
> Using gfx103x might be better
> that way, OTOH if APU vs dGPU will make a compilation target
> difference then gfx103d vs gfx103a maybe?  "GFX10" series might

On the LLVM side (and also for the llvm-mc assembler), they distinguish 
all of the gfx* for the -mcpu= argument. See 
https://llvm.org/docs/AMDGPUUsage.html#id26 for that list.

Thus, I think it makes sense to do the same here.  The last column on 
that page lists the supported hardware but is it neither really up to 
date nor complete.

Thus, I found it easier to just mention gfx1100 as that's unique. On the 
ROCm side, AMD has:

https://rocm.docs.amd.com/projects/install-on-linux/en/latest/reference/system-requirements.html

Tobias
Thomas Schwinge Jan. 26, 2024, 4:21 p.m. UTC | #5 
Hi!

Great progress that you've made!  :-)

On 2024-01-26T13:32:02+0100, Tobias Burnus <tburnus@baylibre.com> wrote:
> Tobias Burnus wrote:
>> Am 24.01.24 um 17:01 schrieb Tobias Burnus:
>>> Okay to enable gfx1100 multilib building and to document gfx1100 in 
>>> the manual?
>>
>> and, with this patch, additionally gfx1030?

> amdgcn: config.gcc - enable gfx1030 and gfx1100 multilib; add them to the docs

> --- a/gcc/doc/install.texi
> +++ b/gcc/doc/install.texi
> @@ -1258,12 +1258,12 @@ default set of libraries is selected based on the value of
>  
>  @item amdgcn*-*-*
>  @var{list} is a comma separated list of ISA names (allowed values: @code{fiji},
> -@code{gfx900}, @code{gfx906}, @code{gfx908}, @code{gfx90a}). It ought not
> -include the name of the default ISA, specified via @option{--with-arch}.  If
> -@var{list} is empty, then there will be no multilibs and only the default
> -run-time library will be built.  If @var{list} is @code{default} or
> -@option{--with-multilib-list=} is not specified, then the default set of
> -libraries is selected.
> +@code{gfx900}, @code{gfx906}, @code{gfx908}, @code{gfx90a}, @code{gfx1030},
> +@code{gfx1100}).  It ought not include the name of the default ISA, specified
> +via @option{--with-arch}.  If @var{list} is empty, then there will be no
> +multilibs and only the default run-time library will be built.  If @var{list}
> +is @code{default} or @option{--with-multilib-list=} is not specified, then
> +the default set of libraries is selected.

Further down in that file, we state:

    @anchor{amdgcn-x-amdhsa}
    @heading amdgcn-*-amdhsa
    AMD GCN GPU target.
    
    Instead of GNU Binutils, you will need to install LLVM 13.0.1, or later, [...]

LLVM 13.0.1 may still be fine for gfx1030
('[...]/amdgcn-amdhsa/gfx1030/libgcc' does get built; I've not further
tested), but it's not sufficient for gfx1100 anymore:

    [...]
    checking for suffix of object files... configure: error: in `[...]/amdgcn-amdhsa/gfx1100/libgcc':
    configure: error: cannot compute suffix of object files: cannot compile
    See `config.log' for more details
    make[1]: *** [Makefile:14105: configure-target-libgcc] Error 1
    [...]

'[...]/amdgcn-amdhsa/gfx1100/libgcc/config.log':

    [...]
    'gfx1100' is not a recognized processor for this target (ignoring processor)
    'gfx1100' is not a recognized processor for this target (ignoring processor)
    /tmp/ccZdohcj.s:1:17: error: .amdgcn_target directive's target id amdgcn-unknown-amdhsa--gfx1100 does not match the specified target id amdgcn-unknown-amdhsa--gfx000
            .amdgcn_target "amdgcn-unknown-amdhsa--gfx1100"
                           ^
    [...]

Which version of LLVM should we be recommending?


Grüße
 Thomas
Richard Biener Jan. 26, 2024, 4:36 p.m. UTC | #6 
> Am 26.01.2024 um 17:22 schrieb Thomas Schwinge <tschwinge@baylibre.com>:
> 
> Hi!
> 
> Great progress that you've made!  :-)
> 
>> On 2024-01-26T13:32:02+0100, Tobias Burnus <tburnus@baylibre.com> wrote:
>> Tobias Burnus wrote:
>>> Am 24.01.24 um 17:01 schrieb Tobias Burnus:
>>>> Okay to enable gfx1100 multilib building and to document gfx1100 in
>>>> the manual?
>>> 
>>> and, with this patch, additionally gfx1030?
> 
>> amdgcn: config.gcc - enable gfx1030 and gfx1100 multilib; add them to the docs
> 
>> --- a/gcc/doc/install.texi
>> +++ b/gcc/doc/install.texi
>> @@ -1258,12 +1258,12 @@ default set of libraries is selected based on the value of
>> 
>> @item amdgcn*-*-*
>> @var{list} is a comma separated list of ISA names (allowed values: @code{fiji},
>> -@code{gfx900}, @code{gfx906}, @code{gfx908}, @code{gfx90a}). It ought not
>> -include the name of the default ISA, specified via @option{--with-arch}.  If
>> -@var{list} is empty, then there will be no multilibs and only the default
>> -run-time library will be built.  If @var{list} is @code{default} or
>> -@option{--with-multilib-list=} is not specified, then the default set of
>> -libraries is selected.
>> +@code{gfx900}, @code{gfx906}, @code{gfx908}, @code{gfx90a}, @code{gfx1030},
>> +@code{gfx1100}).  It ought not include the name of the default ISA, specified
>> +via @option{--with-arch}.  If @var{list} is empty, then there will be no
>> +multilibs and only the default run-time library will be built.  If @var{list}
>> +is @code{default} or @option{--with-multilib-list=} is not specified, then
>> +the default set of libraries is selected.
> 
> Further down in that file, we state:
> 
>    @anchor{amdgcn-x-amdhsa}
>    @heading amdgcn-*-amdhsa
>    AMD GCN GPU target.
> 
>    Instead of GNU Binutils, you will need to install LLVM 13.0.1, or later, [...]
> 
> LLVM 13.0.1 may still be fine for gfx1030
> ('[...]/amdgcn-amdhsa/gfx1030/libgcc' does get built; I've not further
> tested), but it's not sufficient for gfx1100 anymore:
> 
>    [...]
>    checking for suffix of object files... configure: error: in `[...]/amdgcn-amdhsa/gfx1100/libgcc':
>    configure: error: cannot compute suffix of object files: cannot compile
>    See `config.log' for more details
>    make[1]: *** [Makefile:14105: configure-target-libgcc] Error 1
>    [...]
> 
> '[...]/amdgcn-amdhsa/gfx1100/libgcc/config.log':
> 
>    [...]
>    'gfx1100' is not a recognized processor for this target (ignoring processor)
>    'gfx1100' is not a recognized processor for this target (ignoring processor)
>    /tmp/ccZdohcj.s:1:17: error: .amdgcn_target directive's target id amdgcn-unknown-amdhsa--gfx1100 does not match the specified target id amdgcn-unknown-amdhsa--gfx000
>            .amdgcn_target "amdgcn-unknown-amdhsa--gfx1100"
>                           ^
>    [...]
> 
> Which version of LLVM should we be recommending?

A more recent one ;)  unless we know of any fixed bugs in the assembler we’d rely on I‘d day the oldest that works „or later“

Richard 

> 
> Grüße
> Thomas
diff mbox series

Patch

amdgcn: config.gcc - enable gfx1100 multilib; add gfx1100 to docs

gcc/ChangeLog:

	* config.gcc (amdgcn-*-*): Add gfx1100 to TM_MULTILIB_CONFIG.
	* doc/install.texi (Configuration amdgcn-*-*): Mention gfx1100.
	* doc/invoke.texi (AMD GCN Options): Add gfx1100 to -march/-mtune.

libgomp/ChangeLog:

	* testsuite/libgomp.c/declare-variant-4.h: Add variant functions
	for gfx1030 and gfx1100.
	* testsuite/libgomp.c/declare-variant-4-gfx1100.c: New test.

Signed-off-by: Tobias Burnus <tburnus@baylibre.com>

 gcc/config.gcc                                          |  2 +-
 gcc/doc/install.texi                                    | 12 ++++++------
 gcc/doc/invoke.texi                                     |  3 +++
 libgomp/testsuite/libgomp.c/declare-variant-4-gfx1100.c |  8 ++++++++
 libgomp/testsuite/libgomp.c/declare-variant-4.h         | 16 ++++++++++++++++
 5 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index b2d7d7dd475..2343e98ebe6 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4564,7 +4564,7 @@  case "${target}" in
 			TM_MULTILIB_CONFIG=
 			;;
 		xdefault | xyes)
-			TM_MULTILIB_CONFIG=`echo "gfx900,gfx906,gfx908,gfx90a" | sed "s/${with_arch},\?//;s/,$//"`
+			TM_MULTILIB_CONFIG=`echo "gfx900,gfx906,gfx908,gfx90a,gfx1100" | sed "s/${with_arch},\?//;s/,$//"`
 			;;
 		*)
 			TM_MULTILIB_CONFIG="${with_multilib_list}"
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 71593919389..5304ebd36a9 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -1258,12 +1258,12 @@  default set of libraries is selected based on the value of
 
 @item amdgcn*-*-*
 @var{list} is a comma separated list of ISA names (allowed values: @code{fiji},
-@code{gfx900}, @code{gfx906}, @code{gfx908}, @code{gfx90a}). It ought not
-include the name of the default ISA, specified via @option{--with-arch}.  If
-@var{list} is empty, then there will be no multilibs and only the default
-run-time library will be built.  If @var{list} is @code{default} or
-@option{--with-multilib-list=} is not specified, then the default set of
-libraries is selected.
+@code{gfx900}, @code{gfx906}, @code{gfx908}, @code{gfx90a}, @code{gfx1100}).
+It ought not include the name of the default ISA, specified
+via @option{--with-arch}.  If @var{list} is empty, then there will be no
+multilibs and only the default run-time library will be built.  If @var{list}
+is @code{default} or @option{--with-multilib-list=} is not specified, then
+the default set of libraries is selected.
 
 @item arm*-*-*
 @var{list} is a comma separated list of @code{aprofile} and
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 5f904cf1ef2..d1b2c284e2b 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -21723,6 +21723,9 @@  Compile for CDNA1 Instinct MI100 series devices (gfx908).
 @item gfx90a
 Compile for CDNA2 Instinct MI200 series devices (gfx90a).
 
+@item gfx1100
+Compile for RDNA3 gfx1100 devices (GFX11 series).
+
 @end table
 
 @opindex msram-ecc
diff --git a/libgomp/testsuite/libgomp.c/declare-variant-4-gfx1100.c b/libgomp/testsuite/libgomp.c/declare-variant-4-gfx1100.c
new file mode 100644
index 00000000000..6ade35224cc
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/declare-variant-4-gfx1100.c
@@ -0,0 +1,8 @@ 
+/* { dg-do link { target { offload_target_amdgcn } } } */
+/* { dg-additional-options -foffload=amdgcn-amdhsa } */
+/* { dg-additional-options -foffload=-march=gfx1100 } */
+/* { dg-additional-options "-foffload=-fdump-tree-optimized" } */
+
+#include "declare-variant-4.h"
+
+/* { dg-final { only_for_offload_target amdgcn-amdhsa scan-offload-tree-dump "= gfx1100 \\(\\);" "optimized" } } */
diff --git a/libgomp/testsuite/libgomp.c/declare-variant-4.h b/libgomp/testsuite/libgomp.c/declare-variant-4.h
index a70352430c2..393a5e295cc 100644
--- a/libgomp/testsuite/libgomp.c/declare-variant-4.h
+++ b/libgomp/testsuite/libgomp.c/declare-variant-4.h
@@ -35,6 +35,20 @@  gfx90a (void)
   return 0x90a;
 }
 
+__attribute__ ((noipa))
+int
+gfx1030 (void)
+{
+  return 0x1030;
+}
+
+__attribute__ ((noipa))
+int
+gfx1100 (void)
+{
+  return 0x1100;
+}
+
 #ifdef USE_FIJI_FOR_GFX803
 #pragma omp declare variant(gfx803) match(device = {isa("fiji")})
 #else
@@ -44,6 +58,8 @@  gfx90a (void)
 #pragma omp declare variant(gfx906) match(device = {isa("gfx906")})
 #pragma omp declare variant(gfx908) match(device = {isa("gfx908")})
 #pragma omp declare variant(gfx90a) match(device = {isa("gfx90a")})
+#pragma omp declare variant(gfx90a) match(device = {isa("gfx1030")})
+#pragma omp declare variant(gfx90a) match(device = {isa("gfx1100")})
 __attribute__ ((noipa))
 int
 f (void)