public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: James Greenhalgh <james.greenhalgh@arm.com>
To: gcc-patches@gcc.gnu.org
Cc: marcus.shawcroft@arm.com,	fortran@gcc.gnu.org,	sellcey@mips.com,
	ramana.radhakrishnan@arm.com,	burnus@net-b.de,
	richard.earnshaw@arm.com
Subject: Re: [Patch] Add to the libgfortran/newlib bodge to "detect" ftruncate support in ARM/AArch64/SH
Date: Tue, 25 Aug 2015 14:14:00 -0000	[thread overview]
Message-ID: <1440511998-15345-1-git-send-email-james.greenhalgh@arm.com> (raw)
In-Reply-To: <20150821100546.GA15842@e107456-lin.cambridge.arm.com>

[-- Attachment #1: Type: text/plain, Size: 2762 bytes --]


On Fri, Aug 21, 2015 at 11:05:47AM +0100, James Greenhalgh wrote:
> On Thu, Aug 20, 2015 at 10:50:47AM +0100, Marcus Shawcroft wrote:
> > On 20 August 2015 at 09:31, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> > >
> > > Hi,
> > >
> > > Steve's patch in 2013 [1] to fix the MIPS newlib/libgfortran build
> > > causes subtle issues for an ARM/AArch64 newlib/libgfortran build. The
> > > problem is that ARM/AArch64 (and SH) define a stub function for
> > > ftruncate, which we would previously have auto-detected, but which is not
> > > part of the hardwiring Steve added.
> > >
> > > Continuing the tradition of building bodge on bodge on bodge, this patch
> > > hardwires HAVE_FTRUNCATE on for ARM/AArch64/SH, which does fix the issue
> > > I was seeing.
> >
> > This is the second breakage I'm aware of due to the introduction of
> > this hardwire code, the first being related to strtold.  My
> > recollection is that it is only the mips target that requires the
> > newlib API hardwiring. Ideally we should rely only on the
> > AC_CHECK_FUNCS_ONCE probe code and avoid the hardwire entirely.
> >
> > Perhaps a better approach for trunk would be something along the lines of:
> >
> > case "${host}--x${with_newlib}" in
> > mips*--xyes)
> >   hardwire_newlib=1;;
> > esac
> > if test "${hardwire_newlib:-0}" -eq 1; then
> >   ... existing AC_DEFINES hardwire code
> > else
> >   ... existing AC_CHECK_FUNCS_ONCE probe code
> > fi
> >
> > In effect limiting the hardwire to just the target which is unable to
> > probe.  For backport to 4.9 and 5 I think James' more conservative
> > patch is probably more appropriate.
> >
> > What do folks think?
>
> (+CC fortran@gcc.gnu.org - who I should have CCed from the start).
>
> This runs in to issues with a newlib build [1] (newlib provides a 'kill'
> symbol for linking, but does not provide a declaration in signal.h, so
> we take a -Werror=implicit-function-declaration).

This is what the patch you suggested would look like.

I've sent a patch to the newlib list [1] which unconditionally declares
'kill'. With that in place, we can then autodetect the presence of the
functions newlib provides. I'd expect that you would need to apply that
newlib patch if you were testing this patch locally.

I've tested this with a build of arm-none-eabi and aarch64-none-elf to
check that I now get HAVE_FTRUNCATE defined, and that the build completes.

OK?

Thanks,
James

---
2015-08-25  James Greenhalgh  <james.greenhalgh@arm.com>

	* configure.ac: Auto-detect newlib function support unless we
	know there are issues when configuring for a host.
	* configure: Regenerate.

---
[1]: https://sourceware.org/ml/newlib/2015/msg00632.html


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Re-Patch-Add-to-the-libgfortran-newlib-bodge-to-dete.patch --]
[-- Type: text/x-patch;  name=0001-Re-Patch-Add-to-the-libgfortran-newlib-bodge-to-dete.patch, Size: 687 bytes --]

diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
index 35a8b39..1e9914c 100644
--- a/libgfortran/configure.ac
+++ b/libgfortran/configure.ac
@@ -273,8 +273,13 @@ GCC_HEADER_STDINT(gstdint.h)
 
 AC_CHECK_MEMBERS([struct stat.st_blksize, struct stat.st_blocks, struct stat.st_rdev])
 
+case "${host}--x${with_newlib}" in
+  mips*--xyes)
+    hardwire_newlib=1;;
+esac
+
 # Check for library functions.
-if test "x${with_newlib}" = "xyes"; then
+if test "${hardwire_newlib:-0}" -eq 1; 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.

  reply	other threads:[~2015-08-25 14:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-20  8:32 James Greenhalgh
2015-08-20 10:02 ` Marcus Shawcroft
2015-08-21 10:49   ` James Greenhalgh
2015-08-25 14:14     ` James Greenhalgh [this message]
2015-08-25 14:48       ` FX
2015-08-28 10:07         ` James Greenhalgh
2015-08-28 15:34           ` James Greenhalgh
2015-08-30 16:03             ` Hans-Peter Nilsson
2015-09-03  9:40               ` James Greenhalgh
2015-09-03 11:31                 ` Hans-Peter Nilsson
2015-08-20 21:06 ` Steve Ellcey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1440511998-15345-1-git-send-email-james.greenhalgh@arm.com \
    --to=james.greenhalgh@arm.com \
    --cc=burnus@net-b.de \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=marcus.shawcroft@arm.com \
    --cc=ramana.radhakrishnan@arm.com \
    --cc=richard.earnshaw@arm.com \
    --cc=sellcey@mips.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).