This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Implement C _FloatN, _FloatNx types [version 6]
- From: David Malcolm <dmalcolm at redhat dot com>
- To: Joseph Myers <joseph at codesourcery dot com>, Richard Biener <richard dot guenther at gmail dot com>
- Cc: James Greenhalgh <james dot greenhalgh at arm dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, "fortran at gcc dot gnu dot org" <fortran at gcc dot gnu dot org>, Jason Merrill <jason at redhat dot com>, Richard Earnshaw <richard dot earnshaw at arm dot com>, Nick Clifton <nickc at redhat dot com>, Ramana Radhakrishnan <ramana dot radhakrishnan at arm dot com>, Marcus Shawcroft <marcus dot shawcroft at arm dot com>, David Edelsohn <dje dot gcc at gmail dot com>, Segher Boessenkool <segher at kernel dot crashing dot org>, Michael Meissner <meissner at linux dot vnet dot ibm dot com>, murphyp at linux dot vnet dot ibm dot com, nd at arm dot com, Per Bothner <per at bothner dot com>
- Date: Fri, 19 Aug 2016 11:52:41 -0400
- Subject: Re: Implement C _FloatN, _FloatNx types [version 6]
- Authentication-results: sourceware.org; auth=none
- References: <alpine.DEB.2.20.1606211202040.4526@digraph.polyomino.org.uk> <alpine.DEB.2.20.1606211738200.31330@digraph.polyomino.org.uk> <alpine.DEB.2.20.1606231418120.21240@digraph.polyomino.org.uk> <alpine.DEB.2.20.1606271720270.7438@digraph.polyomino.org.uk> <alpine.DEB.2.20.1607191347340.9265@digraph.polyomino.org.uk> <alpine.DEB.2.20.1607222158330.22448@digraph.polyomino.org.uk> <20160817154244.GA39270@arm.com> <alpine.DEB.2.20.1608171641000.7156@digraph.polyomino.org.uk> <alpine.DEB.2.20.1608172015080.27199@digraph.polyomino.org.uk> <CAFiYyc3xqcqJ1rK2X0rC+wwpx3akHbULVG1G47PRmtk4wTk=7A@mail.gmail.com> <alpine.DEB.2.20.1608191101110.30687@digraph.polyomino.org.uk> <CAFiYyc3r-TA2PNwsXSabtaKroYgRh0hjgRbe7u=wR949BetpXQ@mail.gmail.com> <alpine.DEB.2.20.1608191439230.21166@digraph.polyomino.org.uk>
On Fri, 2016-08-19 at 14:40 +0000, Joseph Myers wrote:
> On Fri, 19 Aug 2016, Richard Biener wrote:
>
> > > > Can you quickly verify if LTO works with the new types? I
> > > > don't see anything
> > > > that would prevent it but having new global trees and backends
> > > > initializing them
> > > > might come up with surprises (see tree
> > > > -streamer.c:preload_common_nodes)
> > >
> > > Well, the execution tests are in gcc.dg/torture, which is run
> > > with various
> > > options including -flto (and I've checked the testsuite logs to
> > > confirm
> > > these tests are indeed run with such options). Is there
> > > something else
> > > you think should be tested?
> >
> > No, I think that's enough.
>
> Then I'll commit the patch later today in the absence of comments
> from
> other libcpp maintainers (and then go on to update and retest the
> built-in
> functions patch).
[For reference, the latest version of the patch seems to be here:
https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01290.html ]
Although I'm listed as a libcpp maintainer, I believe today is the
first time I've looked at libcpp/expr.c:interpret_float_suffix. Also,
my attempts to search for the standard you're referring to are failing,
which isn't helping, so my apologies in advance.
I attempted to review your changes to interpret_float_suffix, and I'm
having a hard time understanding what the supported suffixes now are.
Please could you take this opportunity to add some examples to the
header comment for that function, both for the common cases e.g. "f",
and for the new suffixes; nothing in the patch body appears to document
them. (ideally, referencing the standard).
Also, it would be good to add some more comments to the function body.
For example, in this hunk:
- if (f + d + l + w + q > 1 || i > 1)
+ if (f + d + l + w + q + fn + fnx > 1 || i > 1)
should it read something like :
/* Reject duplicate suffixes, contradictory suffixes [...] */
where "[...]" is something relating to fn + fnx, which I can't figure
out in the absence of the standard you're referring to.
Sorry about my ignorance, hope this is constructive feedback
Dave