| Message ID | 524AB4AD.6070508@arm.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 2013-10-01 at 12:40 +0100, Marcus Shawcroft wrote: > On 30/09/13 13:40, Marcus Shawcroft wrote: > > >> Well, I thought this patch would work for me, but it does not. It looks > >> like gcc_no_link is set to 'no' on my target because, technically, I can > >> link even if I don't use a linker script. I just can't find any > >> functions. > >> > > > In which case gating on gcc_no_link could be replaced with a test that > > looks to see if we can link with the library. Perhaps looking for > > exit() or some such that might reasonably be expected to be present. > > > > For example: > > > > AC_CHECK_FUNC(exit) > > if test "x${with_newlib}" = "xyes" -a "x${ac_cv_func_exit}" = "xno"; then > > > > /Marcus This patch works on my mips-mti-elf target. Steve Ellcey sellcey@mips.com
On 1 October 2013 12:40, Marcus Shawcroft <marcus.shawcroft@arm.com> wrote: > Patch attached. > > /Marcus > > 2013-10-01 Marcus Shawcroft <marcus.shawcroft@arm.com> > > * configure.ac (AC_CHECK_FUNCS_ONCE): Add for exit() then make > existing AC_CHECK_FUNCS_ONCE dependent on outcome. Ping.
On 1 October 2013 12:40, Marcus Shawcroft <marcus.shawcroft@arm.com> wrote: > On 30/09/13 13:40, Marcus Shawcroft wrote: > >>> Well, I thought this patch would work for me, but it does not. It looks >>> like gcc_no_link is set to 'no' on my target because, technically, I can >>> link even if I don't use a linker script. I just can't find any >>> functions. >>> > >> In which case gating on gcc_no_link could be replaced with a test that >> looks to see if we can link with the library. Perhaps looking for >> exit() or some such that might reasonably be expected to be present. >> >> For example: >> >> AC_CHECK_FUNC(exit) >> if test "x${with_newlib}" = "xyes" -a "x${ac_cv_func_exit}" = "xno"; then >> >> /Marcus >> >> >> >> > > > Patch attached. > > /Marcus > > 2013-10-01 Marcus Shawcroft <marcus.shawcroft@arm.com> > > * configure.ac (AC_CHECK_FUNCS_ONCE): Add for exit() then make > existing AC_CHECK_FUNCS_ONCE dependent on outcome. Ping^2 /Marcus
On 15/10/13 12:31, Marcus Shawcroft wrote: > On 1 October 2013 12:40, Marcus Shawcroft <marcus.shawcroft@arm.com> wrote: >> On 30/09/13 13:40, Marcus Shawcroft wrote: >> >>>> Well, I thought this patch would work for me, but it does not. It looks >>>> like gcc_no_link is set to 'no' on my target because, technically, I can >>>> link even if I don't use a linker script. I just can't find any >>>> functions. >>>> >> >>> In which case gating on gcc_no_link could be replaced with a test that >>> looks to see if we can link with the library. Perhaps looking for >>> exit() or some such that might reasonably be expected to be present. >>> >>> For example: >>> >>> AC_CHECK_FUNC(exit) >>> if test "x${with_newlib}" = "xyes" -a "x${ac_cv_func_exit}" = "xno"; then >>> >>> /Marcus >>> >>> >>> >>> >> >> >> Patch attached. >> >> /Marcus >> >> 2013-10-01 Marcus Shawcroft <marcus.shawcroft@arm.com> >> >> * configure.ac (AC_CHECK_FUNCS_ONCE): Add for exit() then make >> existing AC_CHECK_FUNCS_ONCE dependent on outcome. > > Ping^2 > > /Marcus > OK provided no Fortran maintainer objects within the next 24 hours. R.
Marcus Shawcroft wrote:>> 2013-10-01 Marcus Shawcroft <marcus.shawcroft@arm.com> >> >> * configure.ac (AC_CHECK_FUNCS_ONCE): Add for exit() then make >> existing AC_CHECK_FUNCS_ONCE dependent on outcome. > > Ping^2 For configure patches, I am never quite sure whether they should be reviewed by a build maintainer or by the library maintainer. In any case, the change looks reasonable to me and as no other newlib user has complained, it is also okay from my side. Tobias
On Oct 15, 2013, at 1:17 PM, Tobias Burnus <burnus@net-b.de> wrote: > Marcus Shawcroft wrote:>> 2013-10-01 Marcus Shawcroft <marcus.shawcroft@arm.com> > >> > >> * configure.ac (AC_CHECK_FUNCS_ONCE): Add for exit() then make > >> existing AC_CHECK_FUNCS_ONCE dependent on outcome. > > > > Ping^2 > > For configure patches, I am never quite sure whether they should be reviewed by a build maintainer or by the library maintainer. In any case, the change looks reasonable to me and as no other newlib user has complained, it is also okay from my side. Would be nice for a build/config person to weigh in or to upgrade and make bullet proof the system against such failures. My take, by default, the compile line should do something useful, and that should be enough for autoconf style tests to smell the library. Now, we can observe that Steve's mips-mti-elf newlib port apparently violates that, but it is lost on me why that is and why that is a good thing. Steve? Is there some reason why by default, a suitable linker script can't be added by the compiler, or, if none suitable, a stub one that isn't suitable in general, but, is complete enough to allow autoconf to function normally?
On 15 October 2013 22:35, Mike Stump <mikestump@comcast.net> wrote:
> Would be nice for a build/config person to weigh in or to upgrade and make bullet proof the system against such failures. My take, by default, the compile line should do something useful, and that should be enough for autoconf style tests to smell the library. Now, we can observe that Steve's mips-mti-elf newlib port apparently violates that, but it is lost on me why that is and why that is a good thing. Steve? Is there some reason why by default, a suitable linker script can't be added by the compiler, or, if none suitable, a stub one that isn't suitable in general, but, is complete enough to allow autoconf to function normally?
Steve,
Can your build be fixed allowing us to back out:
http://gcc.gnu.org/ml/fortran/2013-06/msg00038.html
?
I'd really like to make some progress on this, while my proposed patch
does resolve the regression introduced by the above patch I am
concerned that this is going in the wrong direction and that we
should, as Mike suggests above fix the build issue such that autoconf
behaves, rather than attempting to hardwire configure details of
newlib into libgfortran...
/Marcus
On Thu, 2013-10-24 at 15:53 +0100, Marcus Shawcroft wrote: > Steve, > > Can your build be fixed allowing us to back out: > http://gcc.gnu.org/ml/fortran/2013-06/msg00038.html > > ? > > I'd really like to make some progress on this, while my proposed patch > does resolve the regression introduced by the above patch I am > concerned that this is going in the wrong direction and that we > should, as Mike suggests above fix the build issue such that autoconf > behaves, rather than attempting to hardwire configure details of > newlib into libgfortran... > > /Marcus I am not sure how we would fix the build issue to allow us to not hardcode the newlib configure details into the libgfortran configure script. The linker script that needs to be used to get a good link is different depending on what options one is compiling with on a multilib MIPS system and I have no idea on how one could create (or use) a dummy linker script. Ideally, I think we would want a check that does not depend on linking at all. Note that this problem is not just in libgfortran, it affects libstdc++ and libjava too and those configure scripts also have hardcoded the newlib assumptions. The only difference between libgfortran and the other two libraries is that the newlib assumptions for libgfortran are not static like they are for libstdc++ and libjava. They vary (for one function) based on whether or not long double is supported. If we want to come up with a better solution it should be used for all the libraries and not just for libgfortran. Steve Ellcey sellcey@mips.com
On 24 October 2013 17:47, Steve Ellcey <sellcey@mips.com> wrote: > I am not sure how we would fix the build issue to allow us to not > hardcode the newlib configure details into the libgfortran configure > script. The linker script that needs to be used to get a good link is > different depending on what options one is compiling with on a multilib > MIPS system and I have no idea on how one could create (or use) a dummy > linker script. Ideally, I think we would want a check that does not > depend on linking at all. > > Note that this problem is not just in libgfortran, it affects libstdc++ > and libjava too and those configure scripts also have hardcoded the > newlib assumptions. The only difference between libgfortran and the > other two libraries is that the newlib assumptions for libgfortran are > not static like they are for libstdc++ and libjava. They vary (for one > function) based on whether or not long double is supported. Exactly, hard wiring the newlib interface into the configury of other libraries is questionable, but the patch applied to libgfortran goes beyond hard wiring details of the interface, it hard wires incorrect details of the interface when sizeof(long double) != sizeof(double). The argument that the patch is OK because libjava and libstdc++ also use this idiom, is also rather questionable, because the libgfortran patch goes beyond the idiom used in those other libraries by hard wiring an assumption that does not hold universally. On that basis, I think the the libgfortran patch should be reverted since it caused a regression. /Marcus
On Wed, 2013-11-06 at 17:32 +0000, Marcus Shawcroft wrote: > Exactly, hard wiring the newlib interface into the configury of other > libraries is questionable, but the patch applied to libgfortran goes > beyond hard wiring details of the interface, it hard wires incorrect > details of the interface when sizeof(long double) != sizeof(double). Actually, libstdc++ hardwires incorrect information too. In its configure it checks to see if long_double_math_on_this_cpu is set to 'yes' and then defines the *L math routines if it is not set. But the variable is never set to true anywhere. The comment in configure.ac says: # At some point, we should differentiate between architectures # like x86, which have long double versions, and alpha/powerpc/etc., # which don't. For the time being, punt. I tested to see if this approach would work with libgfortran and MIPS by not defining HAVE_STRTOLD and I could still build and run the testsuite. In fact (at least on MIPS) we never use strtold because GFC_REAL_16_IS_FLOAT128 is defined and that takes precedence and calls a different routine to do the conversion. > The argument that the patch is OK because libjava and libstdc++ also > use this idiom, is also rather questionable, because the libgfortran > patch goes beyond the idiom used in those other libraries by hard > wiring an assumption that does not hold universally. libgfortran is no better or worse then libstdc++ in this regard. > On that basis, I think the the libgfortran patch should be reverted > since it caused a regression. > > /Marcus I understand that you don't like the patch and that you don't think 'libstdc++ does it too' is an adequate rationale, but at some point being able to build is more important then having a clean looking configure script and if we revert my patch, I can't build Fortran on MIPS. If you don't want to check in you patch that checks to see if we can link in the exit routine, I would support removing the HAVE_STRTOLD setting completely or putting it under a long_double_math_on_this_cpu check (that is always false) like the libstdc++ configure script has. Either of these changes would allow me to build. But just removing my original patch leaves me unable to build at all and I don't know of a 'clean' way to fix that and apparently neither did the people working on the libstdc++ and libjava configure scripts so I do object to reverting my patch. Steve Ellcey sellcey@mips.com
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac index 4609eba..ac0c02f 100644 --- a/libgfortran/configure.ac +++ b/libgfortran/configure.ac @@ -261,7 +261,8 @@ GCC_HEADER_STDINT(gstdint.h) AC_CHECK_MEMBERS([struct stat.st_blksize, struct stat.st_blocks, struct stat.st_rdev]) # Check for library functions. -if test "x${with_newlib}" = "xyes"; then +AC_CHECK_FUNC(exit) +if test "x${with_newlib}" = "xyes" -a "x${ac_cv_func_exit}" = "xno"; then # We are being configured with a cross compiler. AC_REPLACE_FUNCS # may not work correctly, because the compiler may not be able to # link executables.