This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fix 61441
- From: Sujoy Saraswati <ssaraswati at gmail dot com>
- To: Joseph Myers <joseph at codesourcery dot com>
- Cc: Richard Biener <richard dot guenther at gmail dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 6 Nov 2015 10:26:29 +0530
- Subject: Re: Fix 61441
- Authentication-results: sourceware.org; auth=none
- References: <CA+ZXfhXZWO8eOCYGPv3nomrfK7zzWHkXSJE++jw0heHxbQoqaA at mail dot gmail dot com> <CAFiYyc0rwpQ_v1x4WzbrzaPfiFQetfq_hLur44otKwceqo6CDg at mail dot gmail dot com> <CA+ZXfhV3KKizefovNp+cu2G_yhL7nK7E41gocnRWaX67TuMq-w at mail dot gmail dot com> <CAFiYyc2N0PEiw12PoFkj-nq-WhG26Tezb40ZrBqtCr3Rwbk7Pw at mail dot gmail dot com> <CA+ZXfhVZx61N50nydQfDtMdD77329efLKpYmw+7g0rXvRHKvjQ at mail dot gmail dot com> <CAFiYyc0qur5=4TywfbgYGGHUpsvtV-f0-Dn+9VtrK0EYyjAnQg at mail dot gmail dot com> <alpine dot DEB dot 2 dot 10 dot 1509142023440 dot 22006 at digraph dot polyomino dot org dot uk> <CA+ZXfhU_JckpXiFNKjsVS3kiaW1dVmnwH-YggzVn8h=_WOJy3w at mail dot gmail dot com> <alpine dot DEB dot 2 dot 10 dot 1509161637180 dot 9502 at digraph dot polyomino dot org dot uk> <CA+ZXfhWh9qmyNACyYDzakHv1qvd-JLWZGr2eeg--KL4OyU15xg at mail dot gmail dot com> <alpine dot DEB dot 2 dot 10 dot 1510281649240 dot 14980 at digraph dot polyomino dot org dot uk> <CA+ZXfhXPwqoUN-NvMzPDKbAoLuZUJNR5Qupx300_KeoVwmq4fg at mail dot gmail dot com> <alpine dot DEB dot 2 dot 10 dot 1511051647390 dot 26133 at digraph dot polyomino dot org dot uk>
Hi,
> This is looking pretty close to ready to go in, but the patch also seems
> to be doing several things at once and it might be easier to review if
> split into separate logical pieces. For example (this is an illustration,
> not necessarily saying it should be exactly this way): (1) Add
> REAL_VALUE_ISSIGNALING_NAN and convert existing logic that already does
> something equivalent (so no semantic change, just cleanup). (2) Convert
> existing code that checks for all NaNs in cases where
> REAL_VALUE_ISSIGNALING_NAN would be enough. (3) Treat rint and nearbyint
> the same. (4) Make real.c operations properly produce quiet NaNs for
> operations with signaling NaN input. (5) Disable various transformations
> for signaling NaN operands, where folding them loses exceptions.
Thanks, I will break it up into multiple patches as you proposed.
> This patch is also getting big enough that it would be a good idea to
> complete the copyright assignment paperwork, certainly if you plan to make
> more contributions in future, if you're not covered by a corporate
> assignment.
I have a corporate assignment, I will post the patches from my
corporate email id.
>> @@ -1943,7 +1965,17 @@ fold_convert_const_real_from_real (tree type, cons
>> REAL_VALUE_TYPE value;
>> tree t;
>>
>> + /* Don't perform the operation if flag_signaling_nans is on
>> + and the operand is a NaN. */
>> + if (HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg1)))
>> + && REAL_VALUE_ISSIGNALING_NAN (TREE_REAL_CST (arg1)))
>> + return NULL_TREE;
>> +
>> real_convert (&value, TYPE_MODE (type), &TREE_REAL_CST (arg1));
>> + /* Make resulting NaN value to be qNaN when flag_signaling_nans
>> + is off. */
>> + if (REAL_VALUE_ISSIGNALING_NAN (value))
>> + value.signalling = 0;
>
> Shouldn't real_convert do this rather than the caller needing to do it?
Yes, it should be. I had started by doing this within real_convert but
then saw that there are quite a few callers where I should add the
check for flag_signaling_nans. This was making the patch bigger, so
instead decided to change the caller in this particular case. I will
try to make the change in real_convert now that we are planning to
break the patch.
Regards,
Sujoy
> --
> Joseph S. Myers
> joseph@codesourcery.com