[PATCH] gcc/configure.ac: fix register issue for global_load assembler functions

[PATCH] gcc/configure.ac: fix register issue for global_load assembler functions

[Marcel Vollweiler]

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

This patch fixes an issue with global_load assembler functions leading
to a "invalid operand for instruction" error since in different LLVM
versions those functions use either one or two registers.

In this patch a compatibility check is added to the configure.ac.

Marcel
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

[-- Attachment #2: global_load.diff --]
[-- Type: text/plain, Size: 4685 bytes --]

gcc/ChangeLog: adapt configuration according to assembler fix of global_load functions.

	* config.in: Regenerate.
	* config/gcn/gcn.c (print_operand_address): Fix for global_load assembler
	functions.
	* configure: Regenerate.
	* configure.ac: Fix for global_load assembler functions. 

diff --git a/gcc/config.in b/gcc/config.in
index e54f59c..18e6271 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -1431,6 +1431,12 @@
 #endif
 
 
+/* Define if your assembler has fixed global_load functions. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_GCN_ASM_GLOBAL_LOAD_FIXED
+#endif
+
+
 /* Define to 1 if you have the `getchar_unlocked' function. */
 #ifndef USED_FOR_TARGET
 #undef HAVE_GETCHAR_UNLOCKED
diff --git a/gcc/config/gcn/gcn.c b/gcc/config/gcn/gcn.c
index 283a91f..2d27296 100644
--- a/gcc/config/gcn/gcn.c
+++ b/gcc/config/gcn/gcn.c
@@ -5481,13 +5481,24 @@ print_operand_address (FILE *file, rtx mem)
 	      if (vgpr_offset == NULL_RTX)
 		/* In this case, the vector offset is zero, so we use the first
 		   lane of v1, which is initialized to zero.  */
-		fprintf (file, "v[1:2]");
+		{
+#if HAVE_GCN_ASM_GLOBAL_LOAD_FIXED == 1
+		    fprintf (file, "v1"); 
+#else
+		    fprintf (file, "v[1:2]");
+#endif
+		}
 	      else if (REG_P (vgpr_offset)
 		       && VGPR_REGNO_P (REGNO (vgpr_offset)))
 		{
-		  fprintf (file, "v[%d:%d]",
-			   REGNO (vgpr_offset) - FIRST_VGPR_REG,
-			   REGNO (vgpr_offset) - FIRST_VGPR_REG + 1);
+#if HAVE_GCN_ASM_GLOBAL_LOAD_FIXED == 1
+		    fprintf (file, "v%d",
+			     REGNO (vgpr_offset) - FIRST_VGPR_REG);
+#else
+		    fprintf (file, "v[%d:%d]",
+			     REGNO (vgpr_offset) - FIRST_VGPR_REG,
+			     REGNO (vgpr_offset) - FIRST_VGPR_REG + 1);
+#endif
 		}
 	      else
 		output_operand_lossage ("bad ADDR_SPACE_GLOBAL address");
diff --git a/gcc/configure b/gcc/configure
index 4a9e4fa..8e044c3 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -28909,6 +28909,36 @@ case "$target" in
     ;;
 esac
 
+# This tests if the assembler supports two registers for global_load functions
+# (like in LLVM versions <12) or one register (like in LLVM 12).
+case "$target" in
+  amdgcn-* | gcn-*)
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler fix for global_load functions" >&5
+$as_echo_n "checking assembler fix for global_load functions... " >&6; }
+    gcc_cv_as_global_load_fixed=yes
+    if test x$gcc_cv_as != x; then
+      cat > conftest.s <<EOF
+	global_store_dwordx2    v[1:2], v[4:5], s[14:15]
+EOF
+      if $gcc_cv_as -triple=amdgcn--amdhsa -filetype=obj -mcpu=gfx900 -o conftest.o conftest.s > /dev/null 2>&1; then
+        gcc_cv_as_global_load_fixed=no
+      fi
+      rm -f conftest.s conftest.o conftest
+    fi
+    if test x$gcc_cv_as_global_load_fixed = xyes; then
+
+$as_echo "#define HAVE_GCN_ASM_GLOBAL_LOAD_FIXED 1" >>confdefs.h
+
+    else
+
+$as_echo "#define HAVE_GCN_ASM_GLOBAL_LOAD_FIXED 0" >>confdefs.h
+
+    fi
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_global_load_fixed" >&5
+$as_echo "$gcc_cv_as_global_load_fixed" >&6; }
+    ;;
+esac
+
 # ??? Not all targets support dwarf2 debug_line, even within a version
 # of gas.  Moreover, we need to emit a valid instruction to trigger any
 # info to the output file.  So, as supported targets are added to gas 2.11,
diff --git a/gcc/configure.ac b/gcc/configure.ac
index d9fc3c2..d7ea224 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -5357,6 +5357,30 @@ case "$target" in
     ;;
 esac
 
+# This tests if the assembler supports two registers for global_load functions
+# (like in LLVM versions <12) or one register (like in LLVM 12).
+case "$target" in
+  amdgcn-* | gcn-*)
+    AC_MSG_CHECKING(assembler fix for global_load functions)
+    gcc_cv_as_global_load_fixed=yes
+    if test x$gcc_cv_as != x; then
+      cat > conftest.s <<EOF
+	global_store_dwordx2    v[[1:2]], v[[4:5]], s[[14:15]]
+EOF
+      if $gcc_cv_as -triple=amdgcn--amdhsa -filetype=obj -mcpu=gfx900 -o conftest.o conftest.s > /dev/null 2>&1; then
+        gcc_cv_as_global_load_fixed=no
+      fi
+      rm -f conftest.s conftest.o conftest
+    fi
+    if test x$gcc_cv_as_global_load_fixed = xyes; then
+      AC_DEFINE(HAVE_GCN_ASM_GLOBAL_LOAD_FIXED, 1, [Define if your assembler has fixed global_load functions.])
+    else
+      AC_DEFINE(HAVE_GCN_ASM_GLOBAL_LOAD_FIXED, 0, [Define if your assembler has fixed global_load functions.])
+    fi
+    AC_MSG_RESULT($gcc_cv_as_global_load_fixed)
+    ;;
+esac
+
 # ??? Not all targets support dwarf2 debug_line, even within a version
 # of gas.  Moreover, we need to emit a valid instruction to trigger any
 # info to the output file.  So, as supported targets are added to gas 2.11,

Re: [PATCH] gcc/configure.ac: fix register issue for global_load assembler functions

[Julian Brown]

On Wed, 9 Jun 2021 16:47:21 +0200
Marcel Vollweiler <marcel@codesourcery.com> wrote:

> This patch fixes an issue with global_load assembler functions leading
> to a "invalid operand for instruction" error since in different LLVM
> versions those functions use either one or two registers.

LLVM is neither forward- nor backward-compatible with regards to those
registers then, I guess? That's unfortunate...

> In this patch a compatibility check is added to the configure.ac.

The implementation of the solution looks fine, but I worry it's the
wrong approach. What would someone packing GCC for a distribution use
for the configuration setting? It'd mean having a dependency on the
exact LLVM version for a given offloading-compiler build -- so LLVM
couldn't be upgraded separately from the offloading compiler. Maybe
that's OK in practice?

I wonder if the LLVM assembler has a macro system we could abuse
instead? Perhaps not. Or another (very ugly) alternative that would work
with either assembler is giving up and emitting the instruction bit
patterns directly (as we have done elsewhere for certain "SCC"-setting
instructions).

Julian

Re: [PATCH] gcc/configure.ac: fix register issue for global_load assembler functions

[Tobias Burnus]

On 14.06.21 14:36, Julian Brown wrote:
> On Wed, 9 Jun 2021 16:47:21 +0200
> Marcel Vollweiler <marcel@codesourcery.com> wrote:
>> This patch fixes an issue with global_load assembler functions leading
>> to a "invalid operand for instruction" error since in different LLVM
>> versions those functions use either one or two registers.
> LLVM is neither forward- nor backward-compatible with regards to those
> registers then, I guess? That's unfortunate...
The old two-register variant was a bug – which was finally fixed, but it
broke the workaround.
>> In this patch a compatibility check is added to the configure.ac.
> The implementation of the solution looks fine, but I worry it's the
> wrong approach. What would someone packing GCC for a distribution use
> for the configuration setting? It'd mean having a dependency on the
> exact LLVM version for a given offloading-compiler build -- so LLVM
> couldn't be upgraded separately from the offloading compiler. Maybe
> that's OK in practice?

At the end, GCC uses 'as' which is a separate file – which in distros
is usually a symbolic link to the LLVM used in the system.

* Debian uses, https://salsa.debian.org/toolchain-team/gcc

ifneq (,$(filter $(distrelease),buster xenial bionic focal groovy))
   gcn_tools_llvm_version = 9
else ifneq (,$(filter $(distrelease),focal groovy))
   gcn_tools_llvm_version = 11
else
   gcn_tools_llvm_version = tools
endif

which is then used as
   ln -sf /usr/lib/llvm-$(gcn_tools_llvm_version)/bin/llvm-mc bin-gcn/as

Hence, for Debian it encodes the version and we should be fine.
(Not implying that the fixed version dependence is nice.)

* SUSE uses:
ln -s /usr/bin/llvm-mc target-tools/bin/amdgcn-amdhsa-as

I do not quickly see how SUSE solves the LLVM12 version issue.
Richard/Martin and Matthias can answer this better.

I know that for gcn, only, some GCC patches avoid issues like the
.section issue with LLVM 11 but I fail to see how those help with
the assembler problem.

> I wonder if the LLVM assembler has a macro system we could abuse
> instead?

as -defsym=MC_VERSION=`as -v|grep version|sed -e 's/.*version //'` ? ;-)

> Perhaps not. Or another (very ugly) alternative that would work
> with either assembler is giving up and emitting the instruction bit
> patterns directly (as we have done elsewhere for certain "SCC"-setting
> instructions).
Tobias
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

Re: [PATCH] gcc/configure.ac: fix register issue for global_load assembler functions

[Andrew Stubbs]

On 14/06/2021 13:36, Julian Brown wrote:
> On Wed, 9 Jun 2021 16:47:21 +0200
> Marcel Vollweiler <marcel@codesourcery.com> wrote:
> 
>> This patch fixes an issue with global_load assembler functions leading
>> to a "invalid operand for instruction" error since in different LLVM
>> versions those functions use either one or two registers.
> 
> LLVM is neither forward- nor backward-compatible with regards to those
> registers then, I guess? That's unfortunate...
> 
>> In this patch a compatibility check is added to the configure.ac.
> 
> The implementation of the solution looks fine, but I worry it's the
> wrong approach. What would someone packing GCC for a distribution use
> for the configuration setting? It'd mean having a dependency on the
> exact LLVM version for a given offloading-compiler build -- so LLVM
> couldn't be upgraded separately from the offloading compiler. Maybe
> that's OK in practice?

Isn't that true of every configure test? They configure the built binary 
to work with what's there right now?

This will only be a problem, in practice, when upgrading from LLVM 11 to 
LLVM 12, and most distros will do that *before* upgrading to GCC 12. The 
real problem is that GCC 11 will be broken in the meantime (I guess we 
have to backport this to GCC 11.2, too.)

> I wonder if the LLVM assembler has a macro system we could abuse
> instead? Perhaps not. Or another (very ugly) alternative that would work
> with either assembler is giving up and emitting the instruction bit
> patterns directly (as we have done elsewhere for certain "SCC"-setting
> instructions).

I'm not aware of anything.

Andrew

Re: [PATCH] gcc/configure.ac: fix register issue for global_load assembler functions

[Julian Brown]

On Mon, 14 Jun 2021 13:36:54 +0100
Julian Brown <julian@codesourcery.com> wrote:

> On Wed, 9 Jun 2021 16:47:21 +0200
> Marcel Vollweiler <marcel@codesourcery.com> wrote:
> > In this patch a compatibility check is added to the configure.ac.  
> 
> The implementation of the solution looks fine, but I worry it's the
> wrong approach. What would someone packing GCC for a distribution use
> for the configuration setting? It'd mean having a dependency on the
> exact LLVM version for a given offloading-compiler build -- so LLVM
> couldn't be upgraded separately from the offloading compiler. Maybe
> that's OK in practice?

After discussing this offline, distribution-builders seem to have
workarounds for this kind of problems that work fine in practice, and
the patch leaves us better off than the status quo (i.e. newer versions
of LLVM not working at all). So, I withdraw my objection (but I can't
approve the patch).

> I wonder if the LLVM assembler has a macro system we could abuse
> instead? Perhaps not.

(Apparently there's nothing suitable.)

Thanks,

Julian

Re: [PATCH] gcc/configure.ac: fix register issue for global_load assembler functions

[Marcel Vollweiler]

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

Changed the variable "gcc_cv_as_global_load_fixed" into
"gcc_cv_as_gcn_global_load_fixed" in order to have the "gcn" substring
also in the config.patch file.


Am 09.06.2021 um 16:47 schrieb Marcel Vollweiler:
> This patch fixes an issue with global_load assembler functions leading
> to a "invalid operand for instruction" error since in different LLVM
> versions those functions use either one or two registers.
>
> In this patch a compatibility check is added to the configure.ac.
>
> Marcel
> -----------------
> Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München
> Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung,
> Frank Thürauf

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

[-- Attachment #2: global_load.diff --]
[-- Type: text/plain, Size: 4740 bytes --]

gcc/configure.ac: Adapt configuration according to assembler fix of global_load functions.

gcc/ChangeLog:

	* config.in: Regenerate.
	* config/gcn/gcn.c (print_operand_address): Fix for global_load assembler
	functions.
	* configure: Regenerate.
	* configure.ac: Fix for global_load assembler functions. 

diff --git a/gcc/config.in b/gcc/config.in
index e54f59c..18e6271 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -1431,6 +1431,12 @@
 #endif
 
 
+/* Define if your assembler has fixed global_load functions. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_GCN_ASM_GLOBAL_LOAD_FIXED
+#endif
+
+
 /* Define to 1 if you have the `getchar_unlocked' function. */
 #ifndef USED_FOR_TARGET
 #undef HAVE_GETCHAR_UNLOCKED
diff --git a/gcc/config/gcn/gcn.c b/gcc/config/gcn/gcn.c
index 283a91f..2d27296 100644
--- a/gcc/config/gcn/gcn.c
+++ b/gcc/config/gcn/gcn.c
@@ -5481,13 +5481,24 @@ print_operand_address (FILE *file, rtx mem)
 	      if (vgpr_offset == NULL_RTX)
 		/* In this case, the vector offset is zero, so we use the first
 		   lane of v1, which is initialized to zero.  */
-		fprintf (file, "v[1:2]");
+		{
+#if HAVE_GCN_ASM_GLOBAL_LOAD_FIXED == 1
+		    fprintf (file, "v1"); 
+#else
+		    fprintf (file, "v[1:2]");
+#endif
+		}
 	      else if (REG_P (vgpr_offset)
 		       && VGPR_REGNO_P (REGNO (vgpr_offset)))
 		{
-		  fprintf (file, "v[%d:%d]",
-			   REGNO (vgpr_offset) - FIRST_VGPR_REG,
-			   REGNO (vgpr_offset) - FIRST_VGPR_REG + 1);
+#if HAVE_GCN_ASM_GLOBAL_LOAD_FIXED == 1
+		    fprintf (file, "v%d",
+			     REGNO (vgpr_offset) - FIRST_VGPR_REG);
+#else
+		    fprintf (file, "v[%d:%d]",
+			     REGNO (vgpr_offset) - FIRST_VGPR_REG,
+			     REGNO (vgpr_offset) - FIRST_VGPR_REG + 1);
+#endif
 		}
 	      else
 		output_operand_lossage ("bad ADDR_SPACE_GLOBAL address");
diff --git a/gcc/configure b/gcc/configure
index 4a9e4fa..8843a8f 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -28909,6 +28909,36 @@ case "$target" in
     ;;
 esac
 
+# This tests if the assembler supports two registers for global_load functions
+# (like in LLVM versions <12) or one register (like in LLVM 12).
+case "$target" in
+  amdgcn-* | gcn-*)
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler fix for global_load functions" >&5
+$as_echo_n "checking assembler fix for global_load functions... " >&6; }
+    gcc_cv_as_gcn_global_load_fixed=yes
+    if test x$gcc_cv_as != x; then
+      cat > conftest.s <<EOF
+	global_store_dwordx2    v[1:2], v[4:5], s[14:15]
+EOF
+      if $gcc_cv_as -triple=amdgcn--amdhsa -filetype=obj -mcpu=gfx900 -o conftest.o conftest.s > /dev/null 2>&1; then
+        gcc_cv_as_gcn_global_load_fixed=no
+      fi
+      rm -f conftest.s conftest.o conftest
+    fi
+    if test x$gcc_cv_as_gcn_global_load_fixed = xyes; then
+
+$as_echo "#define HAVE_GCN_ASM_GLOBAL_LOAD_FIXED 1" >>confdefs.h
+
+    else
+
+$as_echo "#define HAVE_GCN_ASM_GLOBAL_LOAD_FIXED 0" >>confdefs.h
+
+    fi
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_gcn_global_load_fixed" >&5
+$as_echo "$gcc_cv_as_gcn_global_load_fixed" >&6; }
+    ;;
+esac
+
 # ??? Not all targets support dwarf2 debug_line, even within a version
 # of gas.  Moreover, we need to emit a valid instruction to trigger any
 # info to the output file.  So, as supported targets are added to gas 2.11,
diff --git a/gcc/configure.ac b/gcc/configure.ac
index d9fc3c2..e179ce1 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -5357,6 +5357,30 @@ case "$target" in
     ;;
 esac
 
+# This tests if the assembler supports two registers for global_load functions
+# (like in LLVM versions <12) or one register (like in LLVM 12).
+case "$target" in
+  amdgcn-* | gcn-*)
+    AC_MSG_CHECKING(assembler fix for global_load functions)
+    gcc_cv_as_gcn_global_load_fixed=yes
+    if test x$gcc_cv_as != x; then
+      cat > conftest.s <<EOF
+	global_store_dwordx2    v[[1:2]], v[[4:5]], s[[14:15]]
+EOF
+      if $gcc_cv_as -triple=amdgcn--amdhsa -filetype=obj -mcpu=gfx900 -o conftest.o conftest.s > /dev/null 2>&1; then
+        gcc_cv_as_gcn_global_load_fixed=no
+      fi
+      rm -f conftest.s conftest.o conftest
+    fi
+    if test x$gcc_cv_as_gcn_global_load_fixed = xyes; then
+      AC_DEFINE(HAVE_GCN_ASM_GLOBAL_LOAD_FIXED, 1, [Define if your assembler has fixed global_load functions.])
+    else
+      AC_DEFINE(HAVE_GCN_ASM_GLOBAL_LOAD_FIXED, 0, [Define if your assembler has fixed global_load functions.])
+    fi
+    AC_MSG_RESULT($gcc_cv_as_gcn_global_load_fixed)
+    ;;
+esac
+
 # ??? Not all targets support dwarf2 debug_line, even within a version
 # of gas.  Moreover, we need to emit a valid instruction to trigger any
 # info to the output file.  So, as supported targets are added to gas 2.11,

Re: [PATCH] gcc/configure.ac: fix register issue for global_load assembler functions

[Julian Brown]

At the risk of overstepping my GCN backend review remit...

On Wed, 16 Jun 2021 11:34:53 +0200
Marcel Vollweiler <marcel@codesourcery.com> wrote:

> index d9fc3c2..e179ce1 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -5357,6 +5357,30 @@ case "$target" in
>      ;;
>  esac
>  
> +# This tests if the assembler supports two registers for global_load
> functions +# (like in LLVM versions <12) or one register (like in
> LLVM 12). +case "$target" in
> +  amdgcn-* | gcn-*)
> +    AC_MSG_CHECKING(assembler fix for global_load functions)
> +    gcc_cv_as_gcn_global_load_fixed=yes
> +    if test x$gcc_cv_as != x; then
> +      cat > conftest.s <<EOF
> +	global_store_dwordx2    v[[1:2]], v[[4:5]], s[[14:15]]
> +EOF
> +      if $gcc_cv_as -triple=amdgcn--amdhsa -filetype=obj
> -mcpu=gfx900 -o conftest.o conftest.s > /dev/null 2>&1; then
> +        gcc_cv_as_gcn_global_load_fixed=no
> +      fi
> +      rm -f conftest.s conftest.o conftest
> +    fi
> +    if test x$gcc_cv_as_gcn_global_load_fixed = xyes; then
> +      AC_DEFINE(HAVE_GCN_ASM_GLOBAL_LOAD_FIXED, 1, [Define if your
> assembler has fixed global_load functions.])
> +    else
> +      AC_DEFINE(HAVE_GCN_ASM_GLOBAL_LOAD_FIXED, 0, [Define if your
> assembler has fixed global_load functions.])
> +    fi
> +    AC_MSG_RESULT($gcc_cv_as_gcn_global_load_fixed)
> +    ;;
> +esac

I think the more-common idiom seems to be just having a single
AC_DEFINE if the feature is present -- like (as a random example)
HAVE_AS_IX86_REP_LOCK_PREFIX, which omits the "define ... 0" case you
have here. (You'd use "#ifdef ..." instead of "#if ... == 1" to check
the feature then, of course).

Then OK with that change (as long as a global maintainer doesn't object
in, say, the next 24 hours?) -- but please watch the mailing list for
configuration problems that might spring up on other targets.

Thanks,

Julian

Re: [PATCH] gcc/configure.ac: fix register issue for global_load assembler functions

[Joseph Myers]

On Wed, 16 Jun 2021, Julian Brown wrote:

> > +    if test x$gcc_cv_as_gcn_global_load_fixed = xyes; then
> > +      AC_DEFINE(HAVE_GCN_ASM_GLOBAL_LOAD_FIXED, 1, [Define if your
> > assembler has fixed global_load functions.])
> > +    else
> > +      AC_DEFINE(HAVE_GCN_ASM_GLOBAL_LOAD_FIXED, 0, [Define if your
> > assembler has fixed global_load functions.])
> > +    fi
> > +    AC_MSG_RESULT($gcc_cv_as_gcn_global_load_fixed)
> > +    ;;
> > +esac
> 
> I think the more-common idiom seems to be just having a single
> AC_DEFINE if the feature is present -- like (as a random example)
> HAVE_AS_IX86_REP_LOCK_PREFIX, which omits the "define ... 0" case you
> have here. (You'd use "#ifdef ..." instead of "#if ... == 1" to check
> the feature then, of course).

Actually I think what's preferable is the approach used with e.g. 
GATHER_STATISTICS - define to 0 or 1 using a single AC_DEFINE_UNQUOTED 
call (via a shell variable that's set to 0 or 1 as appropriate), then test 
in "if" conditions, not #if, as far as possible, so that both alternatives 
in the conditional code always get syntax-checked when compiling GCC (for 
this target).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] gcc/configure.ac: fix register issue for global_load assembler functions

[Marcel Vollweiler]

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

Am 16.06.2021 um 19:19 schrieb Joseph Myers:
> On Wed, 16 Jun 2021, Julian Brown wrote:
>
>>> +    if test x$gcc_cv_as_gcn_global_load_fixed = xyes; then
>>> +      AC_DEFINE(HAVE_GCN_ASM_GLOBAL_LOAD_FIXED, 1, [Define if your
>>> assembler has fixed global_load functions.])
>>> +    else
>>> +      AC_DEFINE(HAVE_GCN_ASM_GLOBAL_LOAD_FIXED, 0, [Define if your
>>> assembler has fixed global_load functions.])
>>> +    fi
>>> +    AC_MSG_RESULT($gcc_cv_as_gcn_global_load_fixed)
>>> +    ;;
>>> +esac
>>
>> I think the more-common idiom seems to be just having a single
>> AC_DEFINE if the feature is present -- like (as a random example)
>> HAVE_AS_IX86_REP_LOCK_PREFIX, which omits the "define ... 0" case you
>> have here. (You'd use "#ifdef ..." instead of "#if ... == 1" to check
>> the feature then, of course).
>
> Actually I think what's preferable is the approach used with e.g.
> GATHER_STATISTICS - define to 0 or 1 using a single AC_DEFINE_UNQUOTED
> call (via a shell variable that's set to 0 or 1 as appropriate), then test
> in "if" conditions, not #if, as far as possible, so that both alternatives
> in the conditional code always get syntax-checked when compiling GCC (for
> this target).
>

Thank you for your proposals. I adapted configure.ac and gcn.c
accordingly (similar to the GATHER_STATISTICS example).

Marcel
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

[-- Attachment #2: global_load.diff --]
[-- Type: text/plain, Size: 4657 bytes --]

gcc/configure.ac: Adapt configuration according to assembler fix of global_load functions.

gcc/ChangeLog:

	* config.in: Regenerate.
	* config/gcn/gcn.c (print_operand_address): Fix for global_load assembler
	functions.
	* configure: Regenerate.
	* configure.ac: Fix for global_load assembler functions. 

diff --git a/gcc/config.in b/gcc/config.in
index e54f59c..18e6271 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -1431,6 +1431,12 @@
 #endif
 
 
+/* Define if your assembler has fixed global_load functions. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_GCN_ASM_GLOBAL_LOAD_FIXED
+#endif
+
+
 /* Define to 1 if you have the `getchar_unlocked' function. */
 #ifndef USED_FOR_TARGET
 #undef HAVE_GETCHAR_UNLOCKED
diff --git a/gcc/config/gcn/gcn.c b/gcc/config/gcn/gcn.c
index 283a91f..54a1c0b 100644
--- a/gcc/config/gcn/gcn.c
+++ b/gcc/config/gcn/gcn.c
@@ -5481,13 +5481,22 @@ print_operand_address (FILE *file, rtx mem)
 	      if (vgpr_offset == NULL_RTX)
 		/* In this case, the vector offset is zero, so we use the first
 		   lane of v1, which is initialized to zero.  */
-		fprintf (file, "v[1:2]");
+		{
+		  if (HAVE_GCN_ASM_GLOBAL_LOAD_FIXED)
+		    fprintf (file, "v1");
+		  else
+		    fprintf (file, "v[1:2]");
+		}
 	      else if (REG_P (vgpr_offset)
 		       && VGPR_REGNO_P (REGNO (vgpr_offset)))
 		{
-		  fprintf (file, "v[%d:%d]",
-			   REGNO (vgpr_offset) - FIRST_VGPR_REG,
-			   REGNO (vgpr_offset) - FIRST_VGPR_REG + 1);
+		  if (HAVE_GCN_ASM_GLOBAL_LOAD_FIXED)
+		    fprintf (file, "v%d",
+			     REGNO (vgpr_offset) - FIRST_VGPR_REG);
+		  else
+		    fprintf (file, "v[%d:%d]",
+			     REGNO (vgpr_offset) - FIRST_VGPR_REG,
+			     REGNO (vgpr_offset) - FIRST_VGPR_REG + 1);
 		}
 	      else
 		output_operand_lossage ("bad ADDR_SPACE_GLOBAL address");
diff --git a/gcc/configure b/gcc/configure
index 4a9e4fa..dd0194a 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -28909,6 +28909,33 @@ case "$target" in
     ;;
 esac
 
+# This tests if the assembler supports two registers for global_load functions
+# (like in LLVM versions <12) or one register (like in LLVM 12).
+case "$target" in
+  amdgcn-* | gcn-*)
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler fix for global_load functions" >&5
+$as_echo_n "checking assembler fix for global_load functions... " >&6; }
+    gcc_cv_as_gcn_global_load_fixed=yes
+    if test x$gcc_cv_as != x; then
+      cat > conftest.s <<EOF
+	global_store_dwordx2    v[1:2], v[4:5], s[14:15]
+EOF
+      if $gcc_cv_as -triple=amdgcn--amdhsa -filetype=obj -mcpu=gfx900 -o conftest.o conftest.s > /dev/null 2>&1; then
+        gcc_cv_as_gcn_global_load_fixed=no
+      fi
+      rm -f conftest.s conftest.o conftest
+    fi
+    global_load_fixed=`if test x$gcc_cv_as_gcn_global_load_fixed = xyes; then echo 1; else echo 0; fi`
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_GCN_ASM_GLOBAL_LOAD_FIXED $global_load_fixed
+_ACEOF
+
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_gcn_global_load_fixed" >&5
+$as_echo "$gcc_cv_as_gcn_global_load_fixed" >&6; }
+    ;;
+esac
+
 # ??? Not all targets support dwarf2 debug_line, even within a version
 # of gas.  Moreover, we need to emit a valid instruction to trigger any
 # info to the output file.  So, as supported targets are added to gas 2.11,
diff --git a/gcc/configure.ac b/gcc/configure.ac
index d9fc3c2..5f30f80 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -5357,6 +5357,28 @@ case "$target" in
     ;;
 esac
 
+# This tests if the assembler supports two registers for global_load functions
+# (like in LLVM versions <12) or one register (like in LLVM 12).
+case "$target" in
+  amdgcn-* | gcn-*)
+    AC_MSG_CHECKING(assembler fix for global_load functions)
+    gcc_cv_as_gcn_global_load_fixed=yes
+    if test x$gcc_cv_as != x; then
+      cat > conftest.s <<EOF
+	global_store_dwordx2    v[[1:2]], v[[4:5]], s[[14:15]]
+EOF
+      if $gcc_cv_as -triple=amdgcn--amdhsa -filetype=obj -mcpu=gfx900 -o conftest.o conftest.s > /dev/null 2>&1; then
+        gcc_cv_as_gcn_global_load_fixed=no
+      fi
+      rm -f conftest.s conftest.o conftest
+    fi
+    global_load_fixed=`if test x$gcc_cv_as_gcn_global_load_fixed = xyes; then echo 1; else echo 0; fi`
+    AC_DEFINE_UNQUOTED(HAVE_GCN_ASM_GLOBAL_LOAD_FIXED, $global_load_fixed,
+      [Define if your assembler has fixed global_load functions.])
+    AC_MSG_RESULT($gcc_cv_as_gcn_global_load_fixed)
+    ;;
+esac
+
 # ??? Not all targets support dwarf2 debug_line, even within a version
 # of gas.  Moreover, we need to emit a valid instruction to trigger any
 # info to the output file.  So, as supported targets are added to gas 2.11,

Re: [PATCH] gcc/configure.ac: fix register issue for global_load assembler functions

[Joseph Myers]

On Thu, 17 Jun 2021, Marcel Vollweiler wrote:

> Thank you for your proposals. I adapted configure.ac and gcn.c
> accordingly (similar to the GATHER_STATISTICS example).

Thanks, the configure changes in this version are OK.

-- 
Joseph S. Myers
joseph@codesourcery.com