public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] IBM zSystems: Fix TARGET_D_CPU_VERSIONS
@ 2023-01-13 17:54 Stefan Schulze Frielinghaus
  2023-01-23 13:21 ` Iain Buclaw
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Schulze Frielinghaus @ 2023-01-13 17:54 UTC (permalink / raw)
  To: krebbel, gcc-patches; +Cc: Stefan Schulze Frielinghaus

In the context of D the interpretation of S390, S390X, and SystemZ is a
bit fuzzy.  The wording S390X was wrongly deprecated in favour of
SystemZ by commit
https://github.com/dlang/dlang.org/commit/3b50a4c3faf01c32234d0ef8be5f82915a61c23f
Thus, SystemZ is used for 64-bit targets, now, and S390 for 31-bit
targets.  However, in TARGET_D_CPU_VERSIONS depending on TARGET_ZARCH we
set the CPU version to SystemZ.  This is also the case if compiled for
31-bit targets leading to the following error:

libphobos/libdruntime/core/sys/posix/sys/stat.d:967:13: error: static assert:  '96u == 144u' is false
  967 |             static assert(stat_t.sizeof == 144);
      |             ^

Thus in order to keep this patch simple I went for keeping SystemZ for
64-bit targets and S390, as usual, for 31-bit targets and dropped the
distinction between ESA and z/Architecture.

Bootstrapped and regtested on IBM zSystems.  Ok for mainline?

gcc/ChangeLog:

	* config/s390/s390-d.cc (s390_d_target_versions): Fix detection
	of CPU version.
---
 gcc/config/s390/s390-d.cc | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/gcc/config/s390/s390-d.cc b/gcc/config/s390/s390-d.cc
index d10b45f7de4..ced7f49a988 100644
--- a/gcc/config/s390/s390-d.cc
+++ b/gcc/config/s390/s390-d.cc
@@ -30,10 +30,8 @@ along with GCC; see the file COPYING3.  If not see
 void
 s390_d_target_versions (void)
 {
-  if (TARGET_ZARCH)
+  if (TARGET_64BIT)
     d_add_builtin_version ("SystemZ");
-  else if (TARGET_64BIT)
-    d_add_builtin_version ("S390X");
   else
     d_add_builtin_version ("S390");
 
-- 
2.39.0


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

* Re: [PATCH] IBM zSystems: Fix TARGET_D_CPU_VERSIONS
  2023-01-13 17:54 [PATCH] IBM zSystems: Fix TARGET_D_CPU_VERSIONS Stefan Schulze Frielinghaus
@ 2023-01-23 13:21 ` Iain Buclaw
  2023-01-23 18:45   ` Stefan Schulze Frielinghaus
  0 siblings, 1 reply; 6+ messages in thread
From: Iain Buclaw @ 2023-01-23 13:21 UTC (permalink / raw)
  To: gcc-patches, krebbel, Stefan Schulze Frielinghaus

Excerpts from Stefan Schulze Frielinghaus via Gcc-patches's message of Januar 13, 2023 6:54 pm:
> In the context of D the interpretation of S390, S390X, and SystemZ is a
> bit fuzzy.  The wording S390X was wrongly deprecated in favour of
> SystemZ by commit
> https://github.com/dlang/dlang.org/commit/3b50a4c3faf01c32234d0ef8be5f82915a61c23f
> Thus, SystemZ is used for 64-bit targets, now, and S390 for 31-bit
> targets.  However, in TARGET_D_CPU_VERSIONS depending on TARGET_ZARCH we
> set the CPU version to SystemZ.  This is also the case if compiled for
> 31-bit targets leading to the following error:
> 
> libphobos/libdruntime/core/sys/posix/sys/stat.d:967:13: error: static assert:  '96u == 144u' is false
>   967 |             static assert(stat_t.sizeof == 144);
>       |             ^
> 

So that I follow, there are three possible combinations?

ESA 31-bit (S390)
ESA 64-bit (what was S390X)
z/Arch 64-bit (SystemZ)

> Thus in order to keep this patch simple I went for keeping SystemZ for
> 64-bit targets and S390, as usual, for 31-bit targets and dropped the
> distinction between ESA and z/Architecture.
> 
> Bootstrapped and regtested on IBM zSystems.  Ok for mainline?
> 

OK by me.  Maybe keep both S390X and SystemZ for TARGET_64BIT? There's
only ever been a binary distinction as far as I'm aware.

Iain.

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

* Re: [PATCH] IBM zSystems: Fix TARGET_D_CPU_VERSIONS
  2023-01-23 13:21 ` Iain Buclaw
@ 2023-01-23 18:45   ` Stefan Schulze Frielinghaus
  2023-01-24  8:47     ` [PATCH v2] " Stefan Schulze Frielinghaus
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Schulze Frielinghaus @ 2023-01-23 18:45 UTC (permalink / raw)
  To: Iain Buclaw; +Cc: gcc-patches, krebbel

On Mon, Jan 23, 2023 at 02:21:46PM +0100, Iain Buclaw wrote:
> Excerpts from Stefan Schulze Frielinghaus via Gcc-patches's message of Januar 13, 2023 6:54 pm:
> > In the context of D the interpretation of S390, S390X, and SystemZ is a
> > bit fuzzy.  The wording S390X was wrongly deprecated in favour of
> > SystemZ by commit
> > https://github.com/dlang/dlang.org/commit/3b50a4c3faf01c32234d0ef8be5f82915a61c23f
> > Thus, SystemZ is used for 64-bit targets, now, and S390 for 31-bit
> > targets.  However, in TARGET_D_CPU_VERSIONS depending on TARGET_ZARCH we
> > set the CPU version to SystemZ.  This is also the case if compiled for
> > 31-bit targets leading to the following error:
> > 
> > libphobos/libdruntime/core/sys/posix/sys/stat.d:967:13: error: static assert:  '96u == 144u' is false
> >   967 |             static assert(stat_t.sizeof == 144);
> >       |             ^
> > 
> 
> So that I follow, there are three possible combinations?
> 
> ESA 31-bit (S390)
> ESA 64-bit (what was S390X)
> z/Arch 64-bit (SystemZ)

There are three combinations:

- s390:  32-bit ABI and ESA mode
- s390:  32-bit ABI and z/Architecture mode
- s390x: 64-bit ABI and z/Architecture mode

Note, depending on the CPU mode z/Architecture is supported by the
32- and 64-bit ABI whereas ESA is only supported by the 32-bit ABI.

Thus, s390 always refers to the 32-bit ABI but does not fix the
instructions set architecture (ESA or z/Architecture).  Whereas s390x
refers to the 64-bit ABI for which only z/Architecture exists.

While nitpicking, typically the target is written in lower case letters,
i.e., not S390X but s390x and likewise s390 instead of S390.

Hope this clarifies the set of possible combinations.  Let me know if
anything else is unclear.

> 
> > Thus in order to keep this patch simple I went for keeping SystemZ for
> > 64-bit targets and S390, as usual, for 31-bit targets and dropped the
> > distinction between ESA and z/Architecture.
> > 
> > Bootstrapped and regtested on IBM zSystems.  Ok for mainline?
> > 
> 
> OK by me.  Maybe keep both S390X and SystemZ for TARGET_64BIT? There's
> only ever been a binary distinction as far as I'm aware.

Sounds good to me.  I will come up with an updated patch.

Cheers,
Stefan

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

* [PATCH v2] IBM zSystems: Fix TARGET_D_CPU_VERSIONS
  2023-01-23 18:45   ` Stefan Schulze Frielinghaus
@ 2023-01-24  8:47     ` Stefan Schulze Frielinghaus
  2023-01-24 11:05       ` Iain Buclaw
  2023-01-24 16:04       ` Andreas Krebbel
  0 siblings, 2 replies; 6+ messages in thread
From: Stefan Schulze Frielinghaus @ 2023-01-24  8:47 UTC (permalink / raw)
  To: Iain Buclaw, krebbel, gcc-patches; +Cc: Stefan Schulze Frielinghaus

In the context of D the interpretation of S390, S390X, and SystemZ is a
bit fuzzy.  The wording S390X was wrongly deprecated in favour of
SystemZ by commit
https://github.com/dlang/dlang.org/commit/3b50a4c3faf01c32234d0ef8be5f82915a61c23f
Thus, SystemZ is used for 64-bit targets, now, and S390 for 31-bit
targets.  However, in TARGET_D_CPU_VERSIONS depending on TARGET_ZARCH we
set the CPU version to SystemZ.  This is also the case if compiled for
31-bit targets leading to the following error:

libphobos/libdruntime/core/sys/posix/sys/stat.d:967:13: error: static assert:  '96u == 144u' is false
  967 |             static assert(stat_t.sizeof == 144);
      |             ^

Thus in order to keep this patch simple I went for keeping SystemZ for
64-bit targets and S390, as usual, for 31-bit targets and dropped the
distinction between ESA and z/Architecture.

Bootstrapped and regtested on IBM zSystems.  Ok for mainline?

gcc/ChangeLog:

	* config/s390/s390-d.cc (s390_d_target_versions): Fix detection
	of CPU version.
---
 gcc/config/s390/s390-d.cc | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/gcc/config/s390/s390-d.cc b/gcc/config/s390/s390-d.cc
index d10b45f7de4..6e9c80f7283 100644
--- a/gcc/config/s390/s390-d.cc
+++ b/gcc/config/s390/s390-d.cc
@@ -30,10 +30,11 @@ along with GCC; see the file COPYING3.  If not see
 void
 s390_d_target_versions (void)
 {
-  if (TARGET_ZARCH)
-    d_add_builtin_version ("SystemZ");
-  else if (TARGET_64BIT)
-    d_add_builtin_version ("S390X");
+  if (TARGET_64BIT)
+    {
+      d_add_builtin_version ("S390X");
+      d_add_builtin_version ("SystemZ");
+    }
   else
     d_add_builtin_version ("S390");
 
-- 
2.39.0


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

* Re: [PATCH v2] IBM zSystems: Fix TARGET_D_CPU_VERSIONS
  2023-01-24  8:47     ` [PATCH v2] " Stefan Schulze Frielinghaus
@ 2023-01-24 11:05       ` Iain Buclaw
  2023-01-24 16:04       ` Andreas Krebbel
  1 sibling, 0 replies; 6+ messages in thread
From: Iain Buclaw @ 2023-01-24 11:05 UTC (permalink / raw)
  To: gcc-patches, krebbel, Stefan Schulze Frielinghaus

Excerpts from Stefan Schulze Frielinghaus's message of Januar 24, 2023 9:47 am:
> In the context of D the interpretation of S390, S390X, and SystemZ is a
> bit fuzzy.  The wording S390X was wrongly deprecated in favour of
> SystemZ by commit
> https://github.com/dlang/dlang.org/commit/3b50a4c3faf01c32234d0ef8be5f82915a61c23f
> Thus, SystemZ is used for 64-bit targets, now, and S390 for 31-bit
> targets.  However, in TARGET_D_CPU_VERSIONS depending on TARGET_ZARCH we
> set the CPU version to SystemZ.  This is also the case if compiled for
> 31-bit targets leading to the following error:
> 
> libphobos/libdruntime/core/sys/posix/sys/stat.d:967:13: error: static assert:  '96u == 144u' is false
>   967 |             static assert(stat_t.sizeof == 144);
>       |             ^
> 
> Thus in order to keep this patch simple I went for keeping SystemZ for
> 64-bit targets and S390, as usual, for 31-bit targets and dropped the
> distinction between ESA and z/Architecture.
> 
> Bootstrapped and regtested on IBM zSystems.  Ok for mainline?
> 

OK.

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

* Re: [PATCH v2] IBM zSystems: Fix TARGET_D_CPU_VERSIONS
  2023-01-24  8:47     ` [PATCH v2] " Stefan Schulze Frielinghaus
  2023-01-24 11:05       ` Iain Buclaw
@ 2023-01-24 16:04       ` Andreas Krebbel
  1 sibling, 0 replies; 6+ messages in thread
From: Andreas Krebbel @ 2023-01-24 16:04 UTC (permalink / raw)
  To: Stefan Schulze Frielinghaus, Iain Buclaw, gcc-patches

On 1/24/23 09:47, Stefan Schulze Frielinghaus wrote:
> In the context of D the interpretation of S390, S390X, and SystemZ is a
> bit fuzzy.  The wording S390X was wrongly deprecated in favour of
> SystemZ by commit
> https://github.com/dlang/dlang.org/commit/3b50a4c3faf01c32234d0ef8be5f82915a61c23f
> Thus, SystemZ is used for 64-bit targets, now, and S390 for 31-bit
> targets.  However, in TARGET_D_CPU_VERSIONS depending on TARGET_ZARCH we
> set the CPU version to SystemZ.  This is also the case if compiled for
> 31-bit targets leading to the following error:
> 
> libphobos/libdruntime/core/sys/posix/sys/stat.d:967:13: error: static assert:  '96u == 144u' is false
>   967 |             static assert(stat_t.sizeof == 144);
>       |             ^
> 
> Thus in order to keep this patch simple I went for keeping SystemZ for
> 64-bit targets and S390, as usual, for 31-bit targets and dropped the
> distinction between ESA and z/Architecture.
> 
> Bootstrapped and regtested on IBM zSystems.  Ok for mainline?
> 
> gcc/ChangeLog:
> 
> 	* config/s390/s390-d.cc (s390_d_target_versions): Fix detection
> 	of CPU version.

Ok, thanks!

Andreas


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

end of thread, other threads:[~2023-01-24 16:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-13 17:54 [PATCH] IBM zSystems: Fix TARGET_D_CPU_VERSIONS Stefan Schulze Frielinghaus
2023-01-23 13:21 ` Iain Buclaw
2023-01-23 18:45   ` Stefan Schulze Frielinghaus
2023-01-24  8:47     ` [PATCH v2] " Stefan Schulze Frielinghaus
2023-01-24 11:05       ` Iain Buclaw
2023-01-24 16:04       ` Andreas Krebbel

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