| Message ID | 2eb3f34c-d4e4-ce44-1c98-22bff6a9e654@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | musl: use correct long double abi by default | expand |
On Fri, Nov 15, 2019 at 05:26:41PM +0000, Szabolcs Nagy wrote: > On powerpc and s390x the musl ABI requires 64 bit and 128 bit long > double respectively, so adjust the default. > > gcc/ChangeLog: > > 2019-11-15 Szabolcs Nagy <szabolcs.nagy@arm.com> > > * configure.ac: Set gcc_cv_target_ldbl128 for musl targets. > * configure: Regenerate. Please correct the changelog, then? > diff --git a/gcc/configure.ac b/gcc/configure.ac > index 1a0d68208e4..330d5fcaf68 100644 > --- a/gcc/configure.ac > +++ b/gcc/configure.ac > @@ -6161,13 +6161,25 @@ case "$target" in > AC_ARG_WITH(long-double-128, > [AS_HELP_STRING([--with-long-double-128], > [use 128-bit long double by default])], > - gcc_cv_target_ldbl128="$with_long_double_128", > + gcc_cv_target_ldbl128="$with_long_double_128", [ > + case "$target" in > + s390*-*-linux-musl*) > + gcc_cv_target_ldbl128=yes > + ;; > + powerpc*-*-linux-musl*) > + gcc_cv_target_ldbl128=no > + ;; > + *)] I think it would be much more readable and maintainable if you did something like case "$target" in powerpc*-*-linux-musl) gcc_cv_target_ldbl128=no ;; s390*-*-linux-musl) gcc_cv_target_ldbl128=yes ;; powerpc*-*-linux* | sparc*-*-linux* | s390*-*-linux* | alpha*-*-linux*) AC_ARG_WITH(long-double-128, etc., i.e. add the musl targets to the main switch here? Segher
On 15/11/2019 17:48, Segher Boessenkool wrote: > On Fri, Nov 15, 2019 at 05:26:41PM +0000, Szabolcs Nagy wrote: >> On powerpc and s390x the musl ABI requires 64 bit and 128 bit long >> double respectively, so adjust the default. >> >> gcc/ChangeLog: >> >> 2019-11-15 Szabolcs Nagy <szabolcs.nagy@arm.com> >> >> * configure.ac: Set gcc_cv_target_ldbl128 for musl targets. >> * configure: Regenerate. > > Please correct the changelog, then? > >> diff --git a/gcc/configure.ac b/gcc/configure.ac >> index 1a0d68208e4..330d5fcaf68 100644 >> --- a/gcc/configure.ac >> +++ b/gcc/configure.ac >> @@ -6161,13 +6161,25 @@ case "$target" in >> AC_ARG_WITH(long-double-128, >> [AS_HELP_STRING([--with-long-double-128], >> [use 128-bit long double by default])], >> - gcc_cv_target_ldbl128="$with_long_double_128", >> + gcc_cv_target_ldbl128="$with_long_double_128", [ >> + case "$target" in >> + s390*-*-linux-musl*) >> + gcc_cv_target_ldbl128=yes >> + ;; >> + powerpc*-*-linux-musl*) >> + gcc_cv_target_ldbl128=no >> + ;; >> + *)] > > I think it would be much more readable and maintainable if you did > something like > > case "$target" in > powerpc*-*-linux-musl) > gcc_cv_target_ldbl128=no > ;; > s390*-*-linux-musl) > gcc_cv_target_ldbl128=yes > ;; > powerpc*-*-linux* | > sparc*-*-linux* | > s390*-*-linux* | > alpha*-*-linux*) > AC_ARG_WITH(long-double-128, > > etc., i.e. add the musl targets to the main switch here? i'm fine with that, it means the --with-long-double-128 switch does not work on *-musl, which is not a huge loss as the musl abi won't change, but i thought one should be able to configure a non-default-abi toolchain if it's a documented configure option for the target. i'll try your simplified solution.
On Fri, Nov 15, 2019 at 06:03:25PM +0000, Szabolcs Nagy wrote: > On 15/11/2019 17:48, Segher Boessenkool wrote: > > I think it would be much more readable and maintainable if you did > > something like > > > > case "$target" in > > powerpc*-*-linux-musl) > > gcc_cv_target_ldbl128=no > > ;; > > s390*-*-linux-musl) > > gcc_cv_target_ldbl128=yes > > ;; > > powerpc*-*-linux* | > > sparc*-*-linux* | > > s390*-*-linux* | > > alpha*-*-linux*) > > AC_ARG_WITH(long-double-128, > > > > etc., i.e. add the musl targets to the main switch here? > > i'm fine with that, it means the --with-long-double-128 > switch does not work on *-musl, I thought that is what you wanted to do. If you just want to change the default, do that *before* this block, not *in* it? Segher
On 15/11/2019 18:15, Segher Boessenkool wrote: > On Fri, Nov 15, 2019 at 06:03:25PM +0000, Szabolcs Nagy wrote: >> i'm fine with that, it means the --with-long-double-128 >> switch does not work on *-musl, > > I thought that is what you wanted to do. If you just want to change the > default, do that *before* this block, not *in* it? ok, i'll just hard code the abi: On powerpc and s390x the musl ABI requires 64 bit and 128 bit long double respectively, so set the long double abi accordingly instead of requiring correct use of --with-long-double-128. gcc/ChangeLog: 2019-11-15 Szabolcs Nagy <szabolcs.nagy@arm.com> * configure.ac (gcc_cv_target_ldbl128): Set for *-musl* targets. * configure: Regenerate.
On Fri, Nov 15, 2019 at 06:58:24PM +0000, Szabolcs Nagy wrote: > 2019-11-15 Szabolcs Nagy <szabolcs.nagy@arm.com> > > * configure.ac (gcc_cv_target_ldbl128): Set for *-musl* targets. That is not what the patch does. It sets it to yes for s390*-linux-musl*, it sets it to no for powerpc*-*-linux-musl*, and it doesn't do anything for other *-musl* configurations. The powerpc part is okay for trunk, if this is what musl wants to do. (musl has no OS port maintainer listed in MAINTAINERS, maybe that should be fixed?) Segher
On Fri, Nov 15, 2019 at 01:22:20PM -0600, Segher Boessenkool wrote: > On Fri, Nov 15, 2019 at 06:58:24PM +0000, Szabolcs Nagy wrote: > > 2019-11-15 Szabolcs Nagy <szabolcs.nagy@arm.com> > > > > * configure.ac (gcc_cv_target_ldbl128): Set for *-musl* targets. > > That is not what the patch does. It sets it to yes for s390*-linux-musl*, > it sets it to no for powerpc*-*-linux-musl*, and it doesn't do anything for > other *-musl* configurations. > > The powerpc part is okay for trunk, if this is what musl wants to do. > (musl has no OS port maintainer listed in MAINTAINERS, maybe that should > be fixed?) This patch is in line with the ABIs presently supported by musl for powerpc[64] and s390x: ld is ieee-double for powerpc*, ieee-quad for s390x. Rich
On 15/11/2019 19:22, Segher Boessenkool wrote: > On Fri, Nov 15, 2019 at 06:58:24PM +0000, Szabolcs Nagy wrote: >> 2019-11-15 Szabolcs Nagy <szabolcs.nagy@arm.com> >> >> * configure.ac (gcc_cv_target_ldbl128): Set for *-musl* targets. > > That is not what the patch does. It sets it to yes for s390*-linux-musl*, > it sets it to no for powerpc*-*-linux-musl*, and it doesn't do anything for > other *-musl* configurations. ok i can fix the changelog to: * configure.ac (gcc_cv_target_ldbl128): Set for powerpc*-*-linux-musl* and s390*-*-linux-musl* targets. (but i think there is no other *-musl* target that uses gcc_cv_target_ldbl128 so the original was not entirely wrong either) > > The powerpc part is okay for trunk, if this is what musl wants to do. > (musl has no OS port maintainer listed in MAINTAINERS, maybe that should > be fixed?) > > > Segher >
On 15.11.19 19:58, Szabolcs Nagy wrote: > On 15/11/2019 18:15, Segher Boessenkool wrote: >> On Fri, Nov 15, 2019 at 06:03:25PM +0000, Szabolcs Nagy wrote: >>> i'm fine with that, it means the --with-long-double-128 >>> switch does not work on *-musl, >> >> I thought that is what you wanted to do. If you just want to change the >> default, do that *before* this block, not *in* it? > > ok, i'll just hard code the abi: > > > On powerpc and s390x the musl ABI requires 64 bit and 128 bit long > double respectively, so set the long double abi accordingly instead > of requiring correct use of --with-long-double-128. > > gcc/ChangeLog: > > 2019-11-15 Szabolcs Nagy <szabolcs.nagy@arm.com> > > * configure.ac (gcc_cv_target_ldbl128): Set for *-musl* targets. > * configure: Regenerate. > S/390 part is ok. Thanks! Andreas
diff --git a/gcc/configure b/gcc/configure index 6808c23d26b..cf4d009dcfa 100755 --- a/gcc/configure +++ b/gcc/configure @@ -29721,6 +29721,15 @@ if test "${with_long_double_128+set}" = set; then : withval=$with_long_double_128; gcc_cv_target_ldbl128="$with_long_double_128" else + case "$target" in + s390*-*-linux-musl*) + gcc_cv_target_ldbl128=yes + ;; + powerpc*-*-linux-musl*) + gcc_cv_target_ldbl128=no + ;; + *) + if test $glibc_version_major -gt 2 \ || ( test $glibc_version_major -eq 2 && test $glibc_version_minor -ge 4 ); then : gcc_cv_target_ldbl128=yes @@ -29732,6 +29741,10 @@ else && gcc_cv_target_ldbl128=yes fi + + ;; + esac + fi ;; diff --git a/gcc/configure.ac b/gcc/configure.ac index 1a0d68208e4..330d5fcaf68 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -6161,13 +6161,25 @@ case "$target" in AC_ARG_WITH(long-double-128, [AS_HELP_STRING([--with-long-double-128], [use 128-bit long double by default])], - gcc_cv_target_ldbl128="$with_long_double_128", + gcc_cv_target_ldbl128="$with_long_double_128", [ + case "$target" in + s390*-*-linux-musl*) + gcc_cv_target_ldbl128=yes + ;; + powerpc*-*-linux-musl*) + gcc_cv_target_ldbl128=no + ;; + *)] [GCC_GLIBC_VERSION_GTE_IFELSE([2], [4], [gcc_cv_target_ldbl128=yes], [ [gcc_cv_target_ldbl128=no grep '^[ ]*#[ ]*define[ ][ ]*__LONG_DOUBLE_MATH_OPTIONAL' \ $target_header_dir/bits/wordsize.h > /dev/null 2>&1 \ && gcc_cv_target_ldbl128=yes - ]])]) + ]])] + [ + ;; + esac + ]) ;; esac if test x$gcc_cv_target_ldbl128 = xyes; then