S/390: Set ABI default based on uname

Message ID 20180312172809.10870-1-krebbel@linux.vnet.ibm.com
State New
Headers show
Series
  • S/390: Set ABI default based on uname
Related show

Commit Message

Andreas Krebbel March 12, 2018, 5:28 p.m.
Currently the default ABI option for a GCC built on a 64 bit system is
always -m64.  This is inconvenient when e.g. building 32 bit libraries
on a 64 bit system.  The usual way to do this is to set the personality
to s390 (32 bit) and let the configure script do the right thing.
Having a GCC which defaults to -m64 also requires to get a -m31 somehow
into the option list.

With that patch the GCC driver checks the current personality setting in
order to figure out at runtime what the default is supposed to be.

Bootstrapped and regtested on s390x.

I will commit the patch after waiting a few days for comments.

gcc/ChangeLog:

2018-03-12  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>

	* config/s390/driver-native.c (s390_host_detect_target_bits): New
	function.
	* config/s390/s390.h: Invoke s390_host_detect_target_bits if
	neither -m31 nor -m64 has been specified.
---
 gcc/config/s390/driver-native.c | 26 ++++++++++++++++++++++++++
 gcc/config/s390/s390.h          | 14 +++++---------
 2 files changed, 31 insertions(+), 9 deletions(-)

-- 
2.9.1

Comments

Jakub Jelinek March 12, 2018, 5:31 p.m. | #1
On Mon, Mar 12, 2018 at 06:28:09PM +0100, Andreas Krebbel wrote:
> Currently the default ABI option for a GCC built on a 64 bit system is

> always -m64.  This is inconvenient when e.g. building 32 bit libraries

> on a 64 bit system.  The usual way to do this is to set the personality

> to s390 (32 bit) and let the configure script do the right thing.

> Having a GCC which defaults to -m64 also requires to get a -m31 somehow

> into the option list.

> 

> With that patch the GCC driver checks the current personality setting in

> order to figure out at runtime what the default is supposed to be.

> 

> Bootstrapped and regtested on s390x.

> 

> I will commit the patch after waiting a few days for comments.


I don't like this, it is inconsistent with how it is handled on all other
targets and how it worked for years.  Having setarch magically change the
default is dangerous.

> 2018-03-12  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>

> 

> 	* config/s390/driver-native.c (s390_host_detect_target_bits): New

> 	function.

> 	* config/s390/s390.h: Invoke s390_host_detect_target_bits if

> 	neither -m31 nor -m64 has been specified.


	Jakub
Andreas Krebbel March 12, 2018, 5:42 p.m. | #2
On 03/12/2018 06:31 PM, Jakub Jelinek wrote:
> On Mon, Mar 12, 2018 at 06:28:09PM +0100, Andreas Krebbel wrote:

>> Currently the default ABI option for a GCC built on a 64 bit system is

>> always -m64.  This is inconvenient when e.g. building 32 bit libraries

>> on a 64 bit system.  The usual way to do this is to set the personality

>> to s390 (32 bit) and let the configure script do the right thing.

>> Having a GCC which defaults to -m64 also requires to get a -m31 somehow

>> into the option list.

>>

>> With that patch the GCC driver checks the current personality setting in

>> order to figure out at runtime what the default is supposed to be.

>>

>> Bootstrapped and regtested on s390x.

>>

>> I will commit the patch after waiting a few days for comments.

> 

> I don't like this, it is inconsistent with how it is handled on all other

> targets and how it worked for years.  Having setarch magically change the

> default is dangerous.


Wow, that was quick ;)

I didn't expect this to break things. Could you please elaborate where this might cause trouble?

-Andreas-


> 

>> 2018-03-12  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>

>>

>> 	* config/s390/driver-native.c (s390_host_detect_target_bits): New

>> 	function.

>> 	* config/s390/s390.h: Invoke s390_host_detect_target_bits if

>> 	neither -m31 nor -m64 has been specified.

> 

> 	Jakub

>
Jakub Jelinek March 12, 2018, 5:50 p.m. | #3
On Mon, Mar 12, 2018 at 06:42:15PM +0100, Andreas Krebbel wrote:
> On 03/12/2018 06:31 PM, Jakub Jelinek wrote:

> > On Mon, Mar 12, 2018 at 06:28:09PM +0100, Andreas Krebbel wrote:

> >> Currently the default ABI option for a GCC built on a 64 bit system is

> >> always -m64.  This is inconvenient when e.g. building 32 bit libraries

> >> on a 64 bit system.  The usual way to do this is to set the personality

> >> to s390 (32 bit) and let the configure script do the right thing.

> >> Having a GCC which defaults to -m64 also requires to get a -m31 somehow

> >> into the option list.

> >>

> >> With that patch the GCC driver checks the current personality setting in

> >> order to figure out at runtime what the default is supposed to be.

> >>

> >> Bootstrapped and regtested on s390x.

> >>

> >> I will commit the patch after waiting a few days for comments.

> > 

> > I don't like this, it is inconsistent with how it is handled on all other

> > targets and how it worked for years.  Having setarch magically change the

> > default is dangerous.

> 

> Wow, that was quick ;)

> 

> I didn't expect this to break things. Could you please elaborate where this might cause trouble?


It is an unexpected major behavior change for environments that are this way
used for 1-2 decades.  When people want -m32 they just specify it, or if
they want a compiler that defaults to -m32 they configure it that way;
when gcc 7.x works one way and 8.x works differently, it will just mean
people will need to use -m32 or -m64 all the time, otherwise they won't be
sure what will happen.

Plus as I said, it is about consistency, why should s390x work in this
regard any differently in this regard from x86_64, powerpc64, sparc64,
mips64, which don't change the default even when setarch changes uname.

	Jakub
Andreas Krebbel March 13, 2018, 9:22 a.m. | #4
On 03/12/2018 06:50 PM, Jakub Jelinek wrote:
> On Mon, Mar 12, 2018 at 06:42:15PM +0100, Andreas Krebbel wrote:

>> On 03/12/2018 06:31 PM, Jakub Jelinek wrote:

>>> On Mon, Mar 12, 2018 at 06:28:09PM +0100, Andreas Krebbel wrote:

>>>> Currently the default ABI option for a GCC built on a 64 bit system is

>>>> always -m64.  This is inconvenient when e.g. building 32 bit libraries

>>>> on a 64 bit system.  The usual way to do this is to set the personality

>>>> to s390 (32 bit) and let the configure script do the right thing.

>>>> Having a GCC which defaults to -m64 also requires to get a -m31 somehow

>>>> into the option list.

>>>>

>>>> With that patch the GCC driver checks the current personality setting in

>>>> order to figure out at runtime what the default is supposed to be.

>>>>

>>>> Bootstrapped and regtested on s390x.

>>>>

>>>> I will commit the patch after waiting a few days for comments.

>>>

>>> I don't like this, it is inconsistent with how it is handled on all other

>>> targets and how it worked for years.  Having setarch magically change the

>>> default is dangerous.

>>

>> Wow, that was quick ;)

>>

>> I didn't expect this to break things. Could you please elaborate where this might cause trouble?

> 

> It is an unexpected major behavior change for environments that are this way

> used for 1-2 decades.


Ok. I agree that changing the behavior unconditionally probably isn't a good idea.  The
default-default should stay as is.  I'll try to hide this behind a configure switch.

> When people want -m32 they just specify it, or if

> they want a compiler that defaults to -m32 they configure it that way;

> when gcc 7.x works one way and 8.x works differently, it will just mean

> people will need to use -m32 or -m64 all the time, otherwise they won't be

> sure what will happen.


The only case I see where an additional -m option would be required is when trying to generate 64
bit code while running in a 32 bit setarch. Should this be something to care about?

Leaving history aside don't you agree that it would have been more sensible to require a -m option
only if you want to build for an ABI different from what is currently mandated by the personality
setting?

> Plus as I said, it is about consistency, why should s390x work in this

> regard any differently in this regard from x86_64, powerpc64, sparc64,

> mips64, which don't change the default even when setarch changes uname.


Perhaps other platforms might want that as well?! ;)

Background of that change is that while 32 bit support is starting to fade out of the distros we
internally have to continue to build 32 bit stuff for a while.  Currently the RPM spec files from
the distros take care of supporting 32 bit builds in 64 bit environments. E.g. for the Glibc Fedora
spec file I recently did send a patch adding -m31 to the compile options depending on the
personality setting. The spec file already did that for many other biarch architectures. With biarch
builds probably getting less attention in the future the GCC mechanism provides us with a simple way
to do 32 bit builds for libs which used to support s390-32bit before but don't add -m31 depending on
the current personality.

It is especially annoying on S/390 with our -m31 switch (instead of -m32). This certainly wasn't a
great choice in the very beginning of the S/390 port. I never dared to also support -m32
expecting that this would make the whole thing even more messy.


Andreas

> 

> 	Jakub

>
Michael Matz March 13, 2018, 3:53 p.m. | #5
Hi,

On Tue, 13 Mar 2018, Andreas Krebbel wrote:

> Leaving history aside don't you agree that it would have been more 

> sensible to require a -m option only if you want to build for an ABI 

> different from what is currently mandated by the personality setting?


I agree with you.  But we can't leave history aside, so I'm having the 
same sentiment as Jakub.  The current situation is not good, but changing 
behaviour would be even worse.


Ciao,
Michael.
Andreas Krebbel March 20, 2018, 4:32 p.m. | #6
On 03/13/2018 04:53 PM, Michael Matz wrote:
> Hi,

> 

> On Tue, 13 Mar 2018, Andreas Krebbel wrote:

> 

>> Leaving history aside don't you agree that it would have been more 

>> sensible to require a -m option only if you want to build for an ABI 

>> different from what is currently mandated by the personality setting?

> 

> I agree with you.  But we can't leave history aside, so I'm having the 

> same sentiment as Jakub.  The current situation is not good, but changing 

> behaviour would be even worse.


Ok. I'm not convinced that this change will actually hurt somewhere but having downvotes from you
guys is enough to drop that patch.

If I still have to build 32 bit stuff after your distros dropped 32 bit support entirely I'll
probably come back with it ;)

-Andreas-
Michael Matz March 20, 2018, 5:15 p.m. | #7
Hi,

On Tue, 20 Mar 2018, Andreas Krebbel wrote:

> On 03/13/2018 04:53 PM, Michael Matz wrote:

> > Hi,

> > 

> > On Tue, 13 Mar 2018, Andreas Krebbel wrote:

> > 

> >> Leaving history aside don't you agree that it would have been more 

> >> sensible to require a -m option only if you want to build for an ABI 

> >> different from what is currently mandated by the personality setting?

> > 

> > I agree with you.  But we can't leave history aside, so I'm having the 

> > same sentiment as Jakub.  The current situation is not good, but changing 

> > behaviour would be even worse.

> 

> Ok. I'm not convinced that this change will actually hurt somewhere but 

> having downvotes from you guys is enough to drop that patch.

> 

> If I still have to build 32 bit stuff after your distros dropped 32 bit 

> support entirely I'll probably come back with it ;)


Heh, maybe in 15 years ;)


Ciao,
Michael.

Patch

diff --git a/gcc/config/s390/driver-native.c b/gcc/config/s390/driver-native.c
index 3793f8a..666ae48 100644
--- a/gcc/config/s390/driver-native.c
+++ b/gcc/config/s390/driver-native.c
@@ -19,6 +19,8 @@  along with GCC; see the file COPYING3.  If not see
 
 #define IN_TARGET_CODE 1
 
+#include <sys/utsname.h>
+
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
@@ -184,3 +186,27 @@  s390_host_detect_local_cpu (int argc, const char **argv)
 
   return concat ("-m", argv[0], "=", cpu, options, NULL);
 }
+
+/* This function is invoked by the gcc spec parser via
+   target_bits_detect.  It returns either "64" or "31" depending on
+   the personality currently being used.  */
+
+const char *
+s390_host_detect_target_bits (int argc ATTRIBUTE_UNUSED,
+			      const char **argv ATTRIBUTE_UNUSED)
+{
+  struct utsname uname_struct;
+
+  if (uname (&uname_struct) == 0)
+    {
+      if (strcmp (uname_struct.machine, "s390x") == 0)
+	return "64";
+      else if (strcmp (uname_struct.machine, "s390") == 0)
+	return "31";
+    }
+#ifdef DEFAULT_TARGET_64BIT
+  return "64";
+#else
+  return "31";
+#endif
+}
diff --git a/gcc/config/s390/s390.h b/gcc/config/s390/s390.h
index de71fd9..eee6158 100644
--- a/gcc/config/s390/s390.h
+++ b/gcc/config/s390/s390.h
@@ -209,8 +209,10 @@  enum processor_flags
 
 #ifdef __s390__
 extern const char *s390_host_detect_local_cpu (int argc, const char **argv);
-# define EXTRA_SPEC_FUNCTIONS \
-  { "local_cpu_detect", s390_host_detect_local_cpu },
+extern const char *s390_host_detect_target_bits (int argc, const char **argv);
+# define EXTRA_SPEC_FUNCTIONS				\
+  { "local_cpu_detect", s390_host_detect_local_cpu },	\
+  { "target_bits_detect", s390_host_detect_target_bits },
 
 #define MARCH_MTUNE_NATIVE_SPECS				\
   "%{mtune=native:%<mtune=native %:local_cpu_detect(tune)} "	\
@@ -220,16 +222,10 @@  extern const char *s390_host_detect_local_cpu (int argc, const char **argv);
 # define MARCH_MTUNE_NATIVE_SPECS ""
 #endif
 
-#ifdef DEFAULT_TARGET_64BIT
-#define S390_TARGET_BITS_STRING "64"
-#else
-#define S390_TARGET_BITS_STRING "31"
-#endif
-
 /* Defaulting rules.  */
 #define DRIVER_SELF_SPECS					\
   MARCH_MTUNE_NATIVE_SPECS,					\
-  "%{!m31:%{!m64:-m" S390_TARGET_BITS_STRING "}}",		\
+  "%{!m31:%{!m64:-m%:target_bits_detect()}}",			\
   "%{!mesa:%{!mzarch:%{m31:-mesa}%{m64:-mzarch}}}",		\
   "%{!march=*:-march=z900}"