| Message ID | 20180312172809.10870-1-krebbel@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | S/390: Set ABI default based on uname | expand |
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
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 >
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
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 >
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.
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-
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.
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}"