Re: [PATCHv2][PR 57371] Remove useless floating point casts in comparisons
On Mon, 10 Jul 2017, Yuri Gribov wrote: > On Mon, Jul 10, 2017 at 11:06 AM, Joseph Myers > wrote: > > On Sat, 8 Jul 2017, Yuri Gribov wrote: > > > >> On Fri, Jul 7, 2017 at 11:51 PM, Joseph Myers > >> wrote: > >> > On Fri, 7 Jul 2017, Yuri Gribov wrote: > >> > > >> >> > I suspect infinities would already work with the patch as-is (the > >> >> > logic > >> >> > dealing with constants outside the range of the integer type). I'm > >> >> > less > >> >> > clear that NaNs would work properly. (If the comparison is == or != > >> >> > you > >> >> > can optimize it for quiet NaNs, to false and true respectively. If > >> >> > it's a > >> >> > signaling NaN, or < <= > >=, optimizing to false is only valid with > >> >> > -fno-trapping-math, as it would lose an "invalid" exception.) > >> >> > >> >> It's actually under -fsignaling-nans (which if off by default). I've > >> > > >> > No, ordered comparisons with qNaNs should result in exceptions, > >> > >> I assume you mean sNaNs. > > > > No, I mean qNaNs, as I said. Any of < <= > >= with a NaN argument, > > whether quiet or signaling, raise "invalid"; == and != only raise > > "invalid" for sNaNs, not qNaNs. (For a few architectures this is broken > > in GCC; see bug 52451 for x86, 58684 for powerpc, 77918 for s390. We > > should not introduce more instances of such breakage, and should fix it > > where it exists.) > > Oh, I see. Assuming that I fix this (in obvious way, by changing > real_issignaling_nan to real_is_nan) and boostrap/regtest, is the rest > of the patch ok? Please send the updated patch (including any testcase updates needed) for review. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCHv2][PR 57371] Remove useless floating point casts in comparisons
On Mon, Jul 10, 2017 at 11:06 AM, Joseph Myers wrote: > On Sat, 8 Jul 2017, Yuri Gribov wrote: > >> On Fri, Jul 7, 2017 at 11:51 PM, Joseph Myers >> wrote: >> > On Fri, 7 Jul 2017, Yuri Gribov wrote: >> > >> >> > I suspect infinities would already work with the patch as-is (the logic >> >> > dealing with constants outside the range of the integer type). I'm less >> >> > clear that NaNs would work properly. (If the comparison is == or != you >> >> > can optimize it for quiet NaNs, to false and true respectively. If >> >> > it's a >> >> > signaling NaN, or < <= > >=, optimizing to false is only valid with >> >> > -fno-trapping-math, as it would lose an "invalid" exception.) >> >> >> >> It's actually under -fsignaling-nans (which if off by default). I've >> > >> > No, ordered comparisons with qNaNs should result in exceptions, >> >> I assume you mean sNaNs. > > No, I mean qNaNs, as I said. Any of < <= > >= with a NaN argument, > whether quiet or signaling, raise "invalid"; == and != only raise > "invalid" for sNaNs, not qNaNs. (For a few architectures this is broken > in GCC; see bug 52451 for x86, 58684 for powerpc, 77918 for s390. We > should not introduce more instances of such breakage, and should fix it > where it exists.) Oh, I see. Assuming that I fix this (in obvious way, by changing real_issignaling_nan to real_is_nan) and boostrap/regtest, is the rest of the patch ok? -Y
Re: [PATCHv2][PR 57371] Remove useless floating point casts in comparisons
On Sat, 8 Jul 2017, Yuri Gribov wrote: > On Fri, Jul 7, 2017 at 11:51 PM, Joseph Myers wrote: > > On Fri, 7 Jul 2017, Yuri Gribov wrote: > > > >> > I suspect infinities would already work with the patch as-is (the logic > >> > dealing with constants outside the range of the integer type). I'm less > >> > clear that NaNs would work properly. (If the comparison is == or != you > >> > can optimize it for quiet NaNs, to false and true respectively. If it's > >> > a > >> > signaling NaN, or < <= > >=, optimizing to false is only valid with > >> > -fno-trapping-math, as it would lose an "invalid" exception.) > >> > >> It's actually under -fsignaling-nans (which if off by default). I've > > > > No, ordered comparisons with qNaNs should result in exceptions, > > I assume you mean sNaNs. No, I mean qNaNs, as I said. Any of < <= > >= with a NaN argument, whether quiet or signaling, raise "invalid"; == and != only raise "invalid" for sNaNs, not qNaNs. (For a few architectures this is broken in GCC; see bug 52451 for x86, 58684 for powerpc, 77918 for s390. We should not introduce more instances of such breakage, and should fix it where it exists.) -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCHv2][PR 57371] Remove useless floating point casts in comparisons
On Sat, Jul 8, 2017 at 5:30 AM, Yuri Gribov wrote:
> On Fri, Jul 7, 2017 at 11:51 PM, Joseph Myers wrote:
>> On Fri, 7 Jul 2017, Yuri Gribov wrote:
>>
>>> > I suspect infinities would already work with the patch as-is (the logic
>>> > dealing with constants outside the range of the integer type). I'm less
>>> > clear that NaNs would work properly. (If the comparison is == or != you
>>> > can optimize it for quiet NaNs, to false and true respectively. If it's a
>>> > signaling NaN, or < <= > >=, optimizing to false is only valid with
>>> > -fno-trapping-math, as it would lose an "invalid" exception.)
>>>
>>> It's actually under -fsignaling-nans (which if off by default). I've
>>
>> No, ordered comparisons with qNaNs should result in exceptions,
>
> I assume you mean sNaNs.
>
>> so it's not valid by default to optimize them to false (whereas it is valid
>> for
>> equality comparisons, as those only raise exceptions for signaling NaNs).
>
> I'm afraid this is default GCC behavior atm - e.g. check existing
>/* a CMP (-0) -> a CMP 0 */
>...
>(if (REAL_VALUE_ISNAN (TREE_REAL_CST (@1))
> && ! HONOR_SNANS (@1))
> { constant_boolean_node (cmp == NE_EXPR, type); })
> (this pattern causes testcase in my patch pr53731-5.c to be optimized).
>
> Or documentation for -fsignaling-nans which says that "the default is
> -fno-signaling-nans" and it "may change the number of exceptions
> visible with signaling NaNs".
>
> In any case, decision to optimize sNaNs is done in HONOR_NANS macro
> which my code duly checks
Actually I should probly change this to be HONOR_SNANS to enable sNaN
optimization by default (like other matchers do).
> so I'm not really sure what else needs to be
> done about discussed patch in this regards.
>
> -Y
Re: [PATCHv2][PR 57371] Remove useless floating point casts in comparisons
On Fri, Jul 7, 2017 at 11:51 PM, Joseph Myers wrote:
> On Fri, 7 Jul 2017, Yuri Gribov wrote:
>
>> > I suspect infinities would already work with the patch as-is (the logic
>> > dealing with constants outside the range of the integer type). I'm less
>> > clear that NaNs would work properly. (If the comparison is == or != you
>> > can optimize it for quiet NaNs, to false and true respectively. If it's a
>> > signaling NaN, or < <= > >=, optimizing to false is only valid with
>> > -fno-trapping-math, as it would lose an "invalid" exception.)
>>
>> It's actually under -fsignaling-nans (which if off by default). I've
>
> No, ordered comparisons with qNaNs should result in exceptions,
I assume you mean sNaNs.
> so it's not valid by default to optimize them to false (whereas it is valid
> for
> equality comparisons, as those only raise exceptions for signaling NaNs).
I'm afraid this is default GCC behavior atm - e.g. check existing
/* a CMP (-0) -> a CMP 0 */
...
(if (REAL_VALUE_ISNAN (TREE_REAL_CST (@1))
&& ! HONOR_SNANS (@1))
{ constant_boolean_node (cmp == NE_EXPR, type); })
(this pattern causes testcase in my patch pr53731-5.c to be optimized).
Or documentation for -fsignaling-nans which says that "the default is
-fno-signaling-nans" and it "may change the number of exceptions
visible with signaling NaNs".
In any case, decision to optimize sNaNs is done in HONOR_NANS macro
which my code duly checks so I'm not really sure what else needs to be
done about discussed patch in this regards.
-Y
Re: [PATCHv2][PR 57371] Remove useless floating point casts in comparisons
On Fri, 7 Jul 2017, Yuri Gribov wrote: > > I suspect infinities would already work with the patch as-is (the logic > > dealing with constants outside the range of the integer type). I'm less > > clear that NaNs would work properly. (If the comparison is == or != you > > can optimize it for quiet NaNs, to false and true respectively. If it's a > > signaling NaN, or < <= > >=, optimizing to false is only valid with > > -fno-trapping-math, as it would lose an "invalid" exception.) > > It's actually under -fsignaling-nans (which if off by default). I've No, ordered comparisons with qNaNs should result in exceptions, so it's not valid by default to optimize them to false (whereas it is valid for equality comparisons, as those only raise exceptions for signaling NaNs). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCHv2][PR 57371] Remove useless floating point casts in comparisons
On Fri, Jul 7, 2017 at 6:07 PM, Joseph Myers wrote: > On Fri, 7 Jul 2017, Yuri Gribov wrote: > >> Hi all, >> >> This is an updated version of patch in >> https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00034.html . It should >> be much more complete, both in functionality and in tests. > > I think there should be tests when the constant is an infinity (of either > sign) or NaN (quiet or signaling). Indeed. > I suspect infinities would already work with the patch as-is (the logic > dealing with constants outside the range of the integer type). I'm less > clear that NaNs would work properly. (If the comparison is == or != you > can optimize it for quiet NaNs, to false and true respectively. If it's a > signaling NaN, or < <= > >=, optimizing to false is only valid with > -fno-trapping-math, as it would lose an "invalid" exception.) It's actually under -fsignaling-nans (which if off by default). I've attached updated patch, tests are still running but perhaps you could take a look? -Y pr57371-3.patch Description: Binary data
Re: [PATCHv2][PR 57371] Remove useless floating point casts in comparisons
On Fri, 7 Jul 2017, Yuri Gribov wrote: > Hi all, > > This is an updated version of patch in > https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00034.html . It should > be much more complete, both in functionality and in tests. I think there should be tests when the constant is an infinity (of either sign) or NaN (quiet or signaling). I suspect infinities would already work with the patch as-is (the logic dealing with constants outside the range of the integer type). I'm less clear that NaNs would work properly. (If the comparison is == or != you can optimize it for quiet NaNs, to false and true respectively. If it's a signaling NaN, or < <= > >=, optimizing to false is only valid with -fno-trapping-math, as it would lose an "invalid" exception.) -- Joseph S. Myers jos...@codesourcery.com
[PATCHv2][PR 57371] Remove useless floating point casts in comparisons
Hi all, This is an updated version of patch in https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00034.html . It should be much more complete, both in functionality and in tests. Bootstrapped and regtested on x64. Ok for trunk? -Y pr57371-2.patch Description: Binary data
