[PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[VandeVondele Joost]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

Hi,

the Fortran FE sets flag_errno_math and flag_associative_math

https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=94447
https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=159620

Seemingly it (now?) needs to communicate that it is doing so, since otherwise this is overwritten later on and ignored.

The attached patch does so and comes with two testcases to verify the expected effect. 

The patch has been bootstrapped and regtested on x86_64-unknown-linux-gnu. If OK, please apply to trunk.

Thanks in advance,

Joost

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-math-errno-associative.diff --]
[-- Type: text/x-patch; name="patch-math-errno-associative.diff", Size: 2053 bytes --]

gcc/fortran/ChangeLog:

2014-05-30 Joost VandeVondele  <Joost.VandeVondele@mat.ethz.ch>

	* options.c (gfc_init_options_struct): assert that the frontend sets
        flag_errno_math and flag_associative_math.

gcc/testsuite/ChangeLog:

2014-05-30 Joost VandeVondele  <Joost.VandeVondele@mat.ethz.ch>

	* gfortran.dg/errnocheck_1.f90: New test.
	* gfortran.dg/associative_1.f90: New test.


Index: gcc/fortran/options.c
===================================================================
--- gcc/fortran/options.c	(revision 211022)
+++ gcc/fortran/options.c	(working copy)
@@ -66,7 +66,9 @@ void
 gfc_init_options_struct (struct gcc_options *opts)
 {
   opts->x_flag_errno_math = 0;
+  opts->frontend_set_flag_errno_math = true;
   opts->x_flag_associative_math = -1;
+  opts->frontend_set_flag_associative_math = true;
 }
 
 /* Get ready for options handling. Keep in sync with
Index: gcc/testsuite/gfortran.dg/errnocheck_1.f90
===================================================================
--- gcc/testsuite/gfortran.dg/errnocheck_1.f90	(revision 0)
+++ gcc/testsuite/gfortran.dg/errnocheck_1.f90	(revision 0)
@@ -0,0 +1,8 @@
+! { dg-do compile { target x86_64-*-* } }
+! Fortran should default to -fno-math-errno
+! and thus no call to sqrt in asm
+subroutine mysqrt(a)
+ real(KIND=KIND(0.0D0)) :: a
+ a=sqrt(a)
+end subroutine
+! { dg-final { scan-assembler-times "call" 0 } }
Index: gcc/testsuite/gfortran.dg/associative_1.f90
===================================================================
--- gcc/testsuite/gfortran.dg/associative_1.f90	(revision 0)
+++ gcc/testsuite/gfortran.dg/associative_1.f90	(revision 0)
@@ -0,0 +1,10 @@
+! { dg-do compile }
+! { dg-options "-O1 -fno-signed-zeros -fno-trapping-math -fdump-tree-optimized" }
+! Fortran defaults to associative by default,
+! with -fno-signed-zeros -fno-trapping-math this should optimize away all additions
+SUBROUTINE S1(a)
+ REAL :: a
+ a=1+a-1
+END SUBROUTINE S1
+! { dg-final { scan-tree-dump-times " \\\+ " 0 "optimized" } }
+! { dg-final { cleanup-tree-dump "optimized" } }

RE: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[VandeVondele Joost]

ping ?

https://gcc.gnu.org/ml/fortran/2014-05/msg00162.html

Re: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[Janne Blomqvist]

On Mon, Aug 18, 2014 at 6:48 PM, VandeVondele  Joost
<joost.vandevondele@mat.ethz.ch> wrote:
> ping ?
>
> https://gcc.gnu.org/ml/fortran/2014-05/msg00162.html

Ok, again. If Dominique comes up with some reason why the patch should
be modified, it can be done then. AFAICT your patch is better than the
status quo.


-- 
Janne Blomqvist

RE: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[VandeVondele Joost]

Thanks, can somebody with svn write access commit ?

Re: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[Janne Blomqvist]

On Tue, Aug 19, 2014 at 1:07 PM, VandeVondele  Joost
<joost.vandevondele@mat.ethz.ch> wrote:
> Thanks, can somebody with svn write access commit ?

Please get yourself write-after-approval access per instructions at
https://gcc.gnu.org/svnwrite.html . You can use me as your sponsor.
Thanks.

-- 
Janne Blomqvist

RE: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[VandeVondele Joost]

Committed revision 214211.

RE: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[VandeVondele Joost]

Dominque, 

after Janne's OK, and FX judgement, I wonder if you have reached a conclusion. If so, the fsf assignment is now complete, and the patch could be applied.

Joost

> Ok for trunk. Thanks!

Please don't rush! The behavior of -fno-signed-zeros -fno-trapping-math
implying associative math has been changed (as in reverted) between r165758
(implied associative math) and r165930 (lost associative math). AFAICT
it could be due to 165823. Investigating! I am also lookinf for the introduction
of *frontend_set*.

TIA

Dominique

Re: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[Dominique Dhumieres]

> Ok for trunk. Thanks!

Please don't rush! The behavior of -fno-signed-zeros -fno-trapping-math
implying associative math has been changed (as in reverted) between r165758
(implied associative math) and r165930 (lost associative math). AFAICT
it could be due to 165823. Investigating! I am also lookinf for the introduction
of *frontend_set*.

TIA

Dominique

Re: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[Janne Blomqvist]

On Thu, Jun 5, 2014 at 3:54 PM, VandeVondele  Joost
<joost.vandevondele@mat.ethz.ch> wrote:
> I have now verified that both new testcases indeed pass with
>
> gcc version 4.6.0 20100520 (experimental) [trunk revision 159620] (GCC)
>
> that is the revision where Tobias enabled associative math by default. So that this patch fixes a regression.

Ok for trunk. Thanks!


-- 
Janne Blomqvist

RE: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[VandeVondele Joost]

I have now verified that both new testcases indeed pass with

gcc version 4.6.0 20100520 (experimental) [trunk revision 159620] (GCC)

that is the revision where Tobias enabled associative math by default. So that this patch fixes a regression.  

Re: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[FX]

> Why do you want -fno-math-errno to be the default for gfortran?
> AFAICT such default is not documented and the flag works
> (checked on your test gfortran.dg/errnocheck_1.f90).

It should be the default, and it used to be. Fortran doesn’t care about math routines setting errno (its existence is specified in C & C++ standards, but not in the Fortran standard). If I understand correctly, Joost’s patch just adapts this to a new flag handling mechanism.

FX

RE: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[VandeVondele Joost]

> Why do you want -fno-math-errno to be the default for gfortran?

because it was with rth's commit? This makes sense also because errno is a variable that is defined for C, and not accessible from Fortran. Why would you want -fmath-errno to be the default ?

>  if (flag_associative_math == -1)
>    flag_associative_math = (!flag_trapping_math && !flag_signed_zeros);

> Why this is not working is beyond my understanding, but I am not sure that your fix is the right one.

while the details of the option handling are rather opaque to me to, at the point of reaching this statement, flag_associative_math currently equals 0 or 1 even if the user didn't specify the flag. I believe as set by common_handle_option. With my patch, the value of -1 is seen when the user did not specify the flag explicitly.

Note that the go frontend uses a similar approach for errno.

Joost

Re: [PATCH, Fortan] fix initialization of flag_errno_math and flag_associative_math

[Dominique Dhumieres]

Hi Joost,

Why do you want -fno-math-errno to be the default for gfortran?
AFAICT such default is not documented and the flag works
(checked on your test gfortran.dg/errnocheck_1.f90).

For -fassociative-math, the manual says

For Fortran the option is automatically enabled when both -fno-signed-zeros and -fno-trapping-math are in effect.

and again AFAICT this is not working (checked on your test
gfortran.dg/associative_1.f90) while I see

  /* Fortran allows associative math - but we cannot reassociate if
     we want traps or signed zeros. Cf. also flag_protect_parens.  */
  if (flag_associative_math == -1)
    flag_associative_math = (!flag_trapping_math && !flag_signed_zeros);

in gcc/fortran/options.c. Why this is not working is beyond my understanding,
but I am not sure that your fix is the right one.

Cheers,

Dominique