On 12/19/2016 10:18 AM, Tamar Christina wrote:
Hi All,
I've respun the patch with the feedback from Jeff and Joseph.
I think an integer mode should always exist - even in the case of TFmode
on 32-bit systems (32-bit sparc / s390, for example, use TFmode long
double for GNU/Linux, and it's supported as _Float128 and __float128 on
32-bit x86). It just be not be usable for arithmetic or declaring
variables of that type.
You're right, so I test the integer mode I receive with scalar_mode_supported_p.
And this seems to do the right thing.
Thanks for all the comments so far!
Kind Regards,
Tamar
________________________________________
From: Joseph Myers <jos...@codesourcery.com>
Sent: Thursday, December 15, 2016 7:03:27 PM
To: Tamar Christina
Cc: Jeff Law; GCC Patches; Wilco Dijkstra; rguent...@suse.de; Michael Meissner;
nd
Subject: Re: [PATCH][GCC][PATCHv3] Improve fpclassify w.r.t IEEE like numbers
in GIMPLE.
On Thu, 15 Dec 2016, Tamar Christina wrote:
Note that on some systems we even disable 64bit floating point support.
I suspect this check needs a little re-thinking as I don't think that
checking for a specific UNITS_PER_WORD is correct, nor is checking the
width of the type. I'm not offhand sure what the test should be, just
that I think we need something better here.
I think what I really wanted to test here is if there was an integer
mode available which has the exact width as the floating point one. So I
have replaced this with just a call to int_mode_for_mode. Which is
probably more correct.
I think an integer mode should always exist - even in the case of TFmode
on 32-bit systems (32-bit sparc / s390, for example, use TFmode long
double for GNU/Linux, and it's supported as _Float128 and __float128 on
32-bit x86). It just be not be usable for arithmetic or declaring
variables of that type.
I don't know whether TImode bitwise operations, such as generated by this
fpclassify work, will get properly lowered to operations on supported
narrower modes, but I hope so (clearly it's simpler if you can write
things straightforwardly and have them cover this case of TFmode on 32-bit
systems automatically through lowering elsewhere in the compiler, than if
covering that case would require additional code - the more cases you
cover, the more opportunity there is for glibc to use the built-in
functions even with -fsignaling-nans).
--
Joseph S. Myers
jos...@codesourcery.com
fpclassify-patch-gimple-3.patch
diff --git a/gcc/gimple-low.c b/gcc/gimple-low.c
index
64752b67b86b3d01df5f5661e4666df98b7b91d1..ceaee295c6e81531dbe047c569dd18332179ccbf
100644
--- a/gcc/gimple-low.c
+++ b/gcc/gimple-low.c
@@ -30,6 +30,8 @@ along with GCC; see the file COPYING3. If not see
#include "calls.h"
#include "gimple-iterator.h"
#include "gimple-low.h"
+#include "stor-layout.h"
+#include "target.h"
Presumably these are for the endianness & mode checking? It's a bit of
a wart checking those in gimple-low, but I can live with it. We might
consider ways to avoid this layering violation in the future.
+
+/* Validate a single argument ARG against a tree code CODE representing
+ a type. */
I think this comment for gimple_validate_arg is outdated and superseded
by the one immediately following. So I think this comment can simply be
removed.
With that nit fixed, this is OK for the trunk. Thanks for your
patience. I'm really happy with the improvements that were made since
the initial submission of this change.
jeff
