[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |olegendo at gcc dot gnu.org

--- Comment #1 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-07-15 20:08:49 UTC ---
(In reply to comment #0)
> The documentation of -ffinite-math-only states
> "This option should never be turned on by any '-O' option"
> 
> but for sh's -mieee it is said "the default is set to -ffinite-math-only"
> (-mieee is unset by default)
> 
> as a result the -ffinite-math-only is turned on by default for the sh for all
> optimisation levels.
> 
> on chapter 2 it was reenforced that: ... "*By default*, it (gcc) will act as
> the compiler for a hosted implementation" ... and a conforming hosted
> implementation 
> supports the whole standard.
> 
> So either the documentation is false or infinite float values should be
> supported (by default).
> The ability to perform optimum floating point code is preserved by explicitly
> setting it.

The SH -mieee option documentation is wrong.
-ffinite-math-only is not enabled by default (maybe it used to be some time
ago?).  Thus, -mieee is enabled by default (and -mno-ieee has no effect).

Moreover, the documentation part 
"... getting IEEE-conforming results for comparisons of NANs / infinities
incurs extra overhead in every floating-point comparison..."

is confusing, because this is not what the compiler actually does.  If I
observe correctly, the only cases where two fcmp instructions are generated are
when doing '>=' or '<=' comparisons.

I would suggest to make the -mieee option control only the FPU comparisons and
not make it change the flag_finite_math_only.  Also support for -mno-ieee
should be added.

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kkojima at gcc dot gnu.org

--- Comment #2 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-07-15 21:32:17 UTC ---
Kaz, do you have any thoughts on this?

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[kkojima at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

--- Comment #3 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2012-07-16 00:18:11 UTC ---
(In reply to comment #2)
You are right.  Our sh_option_override has code like this:

  if (flag_finite_math_only == 2)
    flag_finite_math_only
      = !flag_signaling_nans && TARGET_SH2E && ! TARGET_IEEE;
  if (TARGET_SH2E && !flag_finite_math_only)
    target_flags |= MASK_IEEE;

Before 4.6, we set flag_finite_math_only to 2 in sh_optimization_options
and tried to set it to 2 with sh_option_init_struct in 4.6, though
the latter looks failed to work.   Thus before 4.6, the 2nd if
close in the above code doesn't effect but now MASK_IEEE is set
with it.  Ugh.
I guess that the better default would be -mno-ieee for bare metals
and -mieee for linux.

Maybe your proposal + removing the above lines and the patch like

--- ORIG/trunk/gcc/config/sh/linux.h    2012-03-08 09:57:48.000000000 +0900
+++ trunk/gcc/config/sh/linux.h    2012-07-16 08:59:57.000000000 +0900
@@ -41,7 +41,7 @@ along with GCC; see the file COPYING3.  

 #undef TARGET_DEFAULT
 #define TARGET_DEFAULT \
-  (TARGET_CPU_DEFAULT | MASK_USERMODE | TARGET_ENDIAN_DEFAULT \
+  (TARGET_CPU_DEFAULT | MASK_USERMODE | MASK_IEEE | TARGET_ENDIAN_DEFAULT \
    | TARGET_OPT_DEFAULT)

 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack

will be reasonable and the most unsurprising way to go.

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

--- Comment #4 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-07-16 20:50:50 UTC ---
(In reply to comment #3)

> I guess that the better default would be -mno-ieee for bare metals

Hm ... Is there a particular reason to distinguish between linux and non-linux?

I was actually thinking of tying -mno-ieee to -ffinite-math-only in the
following way:

no options                -> implicit -fno-finite-math-only -mieee
-ffinite-math-only        -> disable ieee fcmp, allow other optimizations
-ffinite-math-only -mieee -> enable ieee fcmp, allow other optimizations
-mno-ieee                 -> disable ieee fcmp, disallow other optimizations 

I think if faster FPU code is desired, most likely the -ffast-math (or some
sub-option like -ffinite-math-only) will be consulted in the first place.  In
case of bare metal configurations, users will probably tweak their specific
options.
Thus, I did not want to distinguish between linux or non-linux here.

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[kkojima at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

--- Comment #5 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2012-07-16 22:15:40 UTC ---
(In reply to comment #4)
> Thus, I did not want to distinguish between linux or non-linux here.

I have no strong feelings about the bare metal case.
We didn't catch any complaints for defaulting -mieee in 4.6
and 4.7 compilers.  It could be a proof of your argument.

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

--- Comment #6 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-07-18 07:49:56 UTC ---
Author: olegendo
Date: Wed Jul 18 07:49:50 2012
New Revision: 189602

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189602
Log:
    PR target/33135
    * config/sh/sh.opt (mieee): Use Var instead of Mask.  Correct
    description.
    * config/sh/sh.c (sh_option_override): Do not change 
    flag_finite_math_only.  Set TARGET_IEEE to complement of
    flag_finite_math_only.
    * doc/invoke.texi (SH options): Add mno-ieee.  Correct description
    of mieee and mno-ieee behavior.

    PR target/33135
    * gcc.target/sh/pr33135-1.c: New.
    * gcc.target/sh/pr33135-2.c: New.
    * gcc.target/sh/pr33135-3.c: New.
    * gcc.target/sh/pr33135-4.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/sh/pr33135-1.c
    trunk/gcc/testsuite/gcc.target/sh/pr33135-2.c
    trunk/gcc/testsuite/gcc.target/sh/pr33135-3.c
    trunk/gcc/testsuite/gcc.target/sh/pr33135-4.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh.c
    trunk/gcc/config/sh/sh.opt
    trunk/gcc/doc/invoke.texi
    trunk/gcc/testsuite/ChangeLog

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

--- Comment #7 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-07-18 09:16:28 UTC ---
Should this be backported to 4.6 or 4.7?

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[kkojima at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

--- Comment #8 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2012-07-18 09:32:08 UTC ---
(In reply to comment #7)
> Should this be backported to 4.6 or 4.7?

Maybe.  It could be counted as a regression from 4.5 because currently
we have no way to disable mieee on those branches.

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

--- Comment #9 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-07-22 23:44:49 UTC ---
Author: olegendo
Date: Sun Jul 22 23:44:45 2012
New Revision: 189761

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189761
Log:
    PR target/33135
    * config/sh/sh.opt (mieee): Use Var instead of Mask.  Correct
    description.
    * config/sh/sh.c (sh_option_override): Do not change 
    flag_finite_math_only.  Set TARGET_IEEE to complement of
    flag_finite_math_only.
    * doc/invoke.texi (SH options): Add mno-ieee.  Correct
    description of mieee and mno-ieee behavior.


Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/config/sh/sh.c
    branches/gcc-4_7-branch/gcc/config/sh/sh.opt
    branches/gcc-4_7-branch/gcc/doc/invoke.texi

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

--- Comment #10 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-07-22 23:50:00 UTC ---
Author: olegendo
Date: Sun Jul 22 23:49:56 2012
New Revision: 189762

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189762
Log:
    PR target/33135
    * config/sh/sh.opt (mieee): Use Var instead of Mask.  Correct
    description.
    * config/sh/sh.c (sh_option_override): Do not change 
    flag_finite_math_only.  Set TARGET_IEEE to complement of
    flag_finite_math_only.
    * doc/invoke.texi (SH options): Add mno-ieee.  Correct
    description of mieee and mno-ieee behavior.


Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/config/sh/sh.c
    branches/gcc-4_6-branch/gcc/config/sh/sh.opt
    branches/gcc-4_6-branch/gcc/doc/invoke.texi

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #11 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-07-22 23:52:58 UTC ---
Fixed on 4.8, 4.7.2, 4.6.4.

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
   Last reconfirmed|                            |2012-09-01
         Resolution|FIXED                       |
     Ever Confirmed|0                           |1

--- Comment #12 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-09-01 20:15:17 UTC ---
Although this has been closed, I've just discovered something that I believe
can go away:

gcc/common/sh/sh-common.c (sh_option_init_struct):

---
  /* We can't meaningfully test TARGET_SH2E / TARGET_IEEE
     here, so leave it to TARGET_OPTION_OVERRIDE to set
     flag_finite_math_only.  We set it to 2 here so we know if the user
     explicitly requested this to be on or off.  */
  opts->x_flag_finite_math_only = 2;

---- 

TARGET_OPTION_OVERRIDE does not set 'flag_finite_math_only' any longer.
Kaz, would it be OK to remove the whole function 'sh_option_init_struct' from 
gcc/common/sh/sh-common.c ?

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[kkojima at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

--- Comment #13 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2012-09-02 22:43:07 UTC ---
(In reply to comment #12)
> Kaz, would it be OK to remove the whole function 'sh_option_init_struct' from 
> gcc/common/sh/sh-common.c ?

Definitely.

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

--- Comment #14 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-09-02 23:18:11 UTC ---
Author: olegendo
Date: Sun Sep  2 23:18:08 2012
New Revision: 190865

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190865
Log:
    PR target/33135
    * common/config/sh/sh-common.c: Update copyright years.
    (sh_option_init_struct): Delete.
    (TARGET_OPTION_INIT_STRUCT): Likewise.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/common/config/sh/sh-common.c

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #15 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-09-03 09:46:40 UTC ---
Leftover of this issue in gcc/common/sh/sh-common.c has been removed.  Closed
again.

[Bug target/33135] [SH] -ffinite-math-only should not be on by default

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33135

--- Comment #16 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-04 18:32:26 UTC ---
Author: olegendo
Date: Thu Oct  4 18:32:20 2012
New Revision: 192097

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=192097
Log:
    PR target/33135
    * config/sh/t-sh (HOST_LIBGCC2_CFLAGS): Delete.
    * config/sh/t-netbsd (HOST_LIBGCC2_CFLAGS): Delete.
    * config/sh/t-linux (HOST_LIBGCC2_CFLAGS): Remove mieee option.


Modified:
    trunk/gcc/config/sh/sh.md
    trunk/gcc/testsuite/gcc.target/sh/pr52933-1.c
    trunk/libgcc/ChangeLog
    trunk/libgcc/config/sh/t-linux
    trunk/libgcc/config/sh/t-netbsd
    trunk/libgcc/config/sh/t-sh