[PATCH][RFC]Overloading intrinsics

[Martin Liška]

Hi.

Unlike the C and C++ front-ends, GNU Fortran does not know about
vector implementations of math routines provided by GLIBC. This
prevents vectorization of many loops which is a frequent cause of
performance that is worse than compilers with their own math library
such as ICC.

The purpose of the patch is to provide a mechanism that will tell Fotran FE
which intrinsics have simd cloned in math library.

I've been cooperating with Paul and we came to a proof-of-concept that consists
of 2 parts (patches):

The first patch adds support for inclusion a module via
command line. The module will be provided by glibc in order to synchronize which functions
are provided by a glibc version. Paul is suggesting to maybe include that into load machinery
of IEEE module.

Second patch propagates information about newly introduced attribute simd_notinbranch into
gfc_intrinsic_sym. That is later used to modify a corresponding __bultin_*.

Definition of the intrinsics module and it's usage can look like:

cat ~/Programming/testcases/use.f90
module overload
interface
function sin(arg)
!GCC$ attributes simd_notinbranch :: sin
real, intent(in) :: arg
real :: sin
end function sin
end interface
end module

program test_overloaded_intrinsic
real(4) :: x(3200), y(3200), z(3200)

! this should be using simd clone
y = sin(x)
print *, y

! this not
z = cos(x)
print *, z
end

Then using my patches one can see:
$ ./xgcc -B. ~/Programming/testcases/use.f90 -c -Ofast -fdump-tree-optimized=/dev/stdout -c

;; Function test_overloaded_intrinsic (MAIN__, funcdef_no=0, decl_uid=3815, cgraph_uid=1, symbol_order=0) (executed once)

test_overloaded_intrinsic ()
{
...
vect__3.14_58 = sinf.simdclone.0 (vect__2.13_60);
MEM[symbol: y, index: ivtmp.35_47, offset: 0B] = vect__3.14_58;
...
_6 = __builtin_cosf (_5);
MEM[symbol: z, index: ivtmp.30_46, offset: 0B] = _6;
...
}

That's what I have. I would like to ask Fortran folks about their opinion? I know
the part in gfc_match_gcc_attributes is bit tricky, but apart from that the rest
should be well formed.

Thoughts?
Thanks,
Martin

[Richard Biener]

Post by Martin Liška
Hi.
Unlike the C and C++ front-ends, GNU Fortran does not know about
vector implementations of math routines provided by GLIBC. This
prevents vectorization of many loops which is a frequent cause of
performance that is worse than compilers with their own math library
such as ICC.
The purpose of the patch is to provide a mechanism that will tell Fotran FE
which intrinsics have simd cloned in math library.
I've been cooperating with Paul and we came to a proof-of-concept that consists
The first patch adds support for inclusion a module via
command line. The module will be provided by glibc in order to synchronize which functions
are provided by a glibc version. Paul is suggesting to maybe include that into load machinery
of IEEE module.
Second patch propagates information about newly introduced attribute simd_notinbranch into
gfc_intrinsic_sym. That is later used to modify a corresponding __bultin_*.
cat ~/Programming/testcases/use.f90
module overload
interface
function sin(arg)
!GCC$ attributes simd_notinbranch :: sin
real, intent(in) :: arg
real :: sin
end function sin
end interface
end module
program test_overloaded_intrinsic
real(4) :: x(3200), y(3200), z(3200)
! this should be using simd clone
y = sin(x)
print *, y
! this not
z = cos(x)
print *, z
end
$ ./xgcc -B. ~/Programming/testcases/use.f90 -c -Ofast -fdump-tree-optimized=/dev/stdout -c
;; Function test_overloaded_intrinsic (MAIN__, funcdef_no=0, decl_uid=3815, cgraph_uid=1, symbol_order=0) (executed once)
test_overloaded_intrinsic ()
{
...
vect__3.14_58 = sinf.simdclone.0 (vect__2.13_60);
MEM[symbol: y, index: ivtmp.35_47, offset: 0B] = vect__3.14_58;
...
_6 = __builtin_cosf (_5);
MEM[symbol: z, index: ivtmp.30_46, offset: 0B] = _6;
...
}
That's what I have. I would like to ask Fortran folks about their opinion? I know
the part in gfc_match_gcc_attributes is bit tricky, but apart from that the rest
should be well formed.
Thoughts?

The gfc_conv_intrinsic_lib_function should be in
gfc_get_intrinsic_lib_fndecl instead I think
as you are adding the attribute once for each call it seems. I think
that it would be more
forward-looking to make the gfc_intrinsics_sym->simd flag a 'tree
attributes' list instead.

That you do the matching in gfc_match_gcc_attributes is quite odd IMHO
but you said
that already. I'd say a more proper place would be where the FE
"ends" parsing of
the specification part of an interface.

Of course simd_notinbranch isn't exactly supporting all simd variants,
but not sure
how difficult it is to add this as 'simd' with arguments...

Some bikeshedding on the other part:

+module-include
+Fortran Joined Separate
+Use implicitelly a module
+

module-use or use-module or simply use? Or import? include sounds so C-ish.

+void
+gfc_add_implicit_use (const char *module_name)
+{
+ implicit_module_name = module_name;
+}

so this works for exactly one module...

I guess the part that is missing is to add target specific specs
fragments adding
-use FOO for fortran invocations, allowing FOO to not exist(?).

Richard.

Post by Martin Liška
Thanks,
Martin

[Martin Liška]

Post by Richard Biener

Post by Martin Liška
Hi.
Unlike the C and C++ front-ends, GNU Fortran does not know about
vector implementations of math routines provided by GLIBC. This
prevents vectorization of many loops which is a frequent cause of
performance that is worse than compilers with their own math library
such as ICC.
The purpose of the patch is to provide a mechanism that will tell Fotran FE
which intrinsics have simd cloned in math library.
I've been cooperating with Paul and we came to a proof-of-concept that consists
The first patch adds support for inclusion a module via
command line. The module will be provided by glibc in order to synchronize which functions
are provided by a glibc version. Paul is suggesting to maybe include that into load machinery
of IEEE module.
Second patch propagates information about newly introduced attribute simd_notinbranch into
gfc_intrinsic_sym. That is later used to modify a corresponding __bultin_*.
cat ~/Programming/testcases/use.f90
module overload
interface
function sin(arg)
!GCC$ attributes simd_notinbranch :: sin
real, intent(in) :: arg
real :: sin
end function sin
end interface
end module
program test_overloaded_intrinsic
real(4) :: x(3200), y(3200), z(3200)
! this should be using simd clone
y = sin(x)
print *, y
! this not
z = cos(x)
print *, z
end
$ ./xgcc -B. ~/Programming/testcases/use.f90 -c -Ofast -fdump-tree-optimized=/dev/stdout -c
;; Function test_overloaded_intrinsic (MAIN__, funcdef_no=0, decl_uid=3815, cgraph_uid=1, symbol_order=0) (executed once)
test_overloaded_intrinsic ()
{
...
vect__3.14_58 = sinf.simdclone.0 (vect__2.13_60);
MEM[symbol: y, index: ivtmp.35_47, offset: 0B] = vect__3.14_58;
...
_6 = __builtin_cosf (_5);
MEM[symbol: z, index: ivtmp.30_46, offset: 0B] = _6;
...
}
That's what I have. I would like to ask Fortran folks about their opinion? I know
the part in gfc_match_gcc_attributes is bit tricky, but apart from that the rest
should be well formed.
Thoughts?

The gfc_conv_intrinsic_lib_function should be in
gfc_get_intrinsic_lib_fndecl instead I think
as you are adding the attribute once for each call it seems. I think
that it would be more

Hi.

I fixed that by clearing of simd attribute. But yes, it makes sense to set
it just once.

Post by Richard Biener
forward-looking to make the gfc_intrinsics_sym->simd flag a 'tree
attributes' list instead.

Well, I believe all the flags gfc_intrinsic_sym flags are used in very similar way
as I use it for simd flag.

Post by Richard Biener
That you do the matching in gfc_match_gcc_attributes is quite odd IMHO
but you said
that already. I'd say a more proper place would be where the FE
"ends" parsing of
the specification part of an interface.

I improved that and I also do it only for a module with 'vector_math' name.
Will we want that?

Post by Richard Biener
Of course simd_notinbranch isn't exactly supporting all simd variants,
but not sure
how difficult it is to add this as 'simd' with arguments...

Will we need multiple variants?

Post by Richard Biener
+module-include
+Fortran Joined Separate
+Use implicitelly a module
+
module-use or use-module or simply use? Or import? include sounds so C-ish.

Yep, I prefer module-use.

Post by Richard Biener
+void
+gfc_add_implicit_use (const char *module_name)
+{
+ implicit_module_name = module_name;
+}
so this works for exactly one module...

I extended that to support multiple ones.

Post by Richard Biener
I guess the part that is missing is to add target specific specs
fragments adding
-use FOO for fortran invocations, allowing FOO to not exist(?).

Will take a look at that.

Now I've tried to separate real module and it's usage and I see strange error:

$ cat ~/Programming/testcases/module.f90
module vector_math
interface
function sin(arg)
!GCC$ attributes simd_notinbranch :: sin
real, intent(in) :: arg
real :: sin
end function sin
end interface
end module

$ ./xgcc -B. -Ofast -c ~/Programming/testcases/module.f90

$ cat ~/Programming/testcases/use.f90
program test_overloaded_intrinsic
use vector_math
real(4) :: x(3200), y(3200), z(3200)

! this should be using simd clone
y = sin(x)
print *, y

! this not
z = cos(x)
print *, z

z = sin (z)
print *, z
end

$ ./xgcc -B. -Ofast -c ~/Programming/testcases/use.f90
/home/marxin/Programming/testcases/use.f90:6:10:

6 | y = sin(x)
| 1
Error: Rank mismatch in argument ‘arg’ at (1) (scalar and rank-1)
/home/marxin/Programming/testcases/use.f90:13:11:

13 | z = sin (z)
| 1
Error: Rank mismatch in argument ‘arg’ at (1) (scalar and rank-1)

Thanks,
Martin

Post by Richard Biener
Richard.

Post by Martin Liška
Thanks,
Martin

[Richard Biener]

Post by Martin Liška

Post by Richard Biener

Post by Martin Liška
Hi.
Unlike the C and C++ front-ends, GNU Fortran does not know about
vector implementations of math routines provided by GLIBC. This
prevents vectorization of many loops which is a frequent cause of
performance that is worse than compilers with their own math library
such as ICC.
The purpose of the patch is to provide a mechanism that will tell Fotran FE
which intrinsics have simd cloned in math library.
I've been cooperating with Paul and we came to a proof-of-concept that consists
The first patch adds support for inclusion a module via
command line. The module will be provided by glibc in order to synchronize which functions
are provided by a glibc version. Paul is suggesting to maybe include that into load machinery
of IEEE module.
Second patch propagates information about newly introduced attribute simd_notinbranch into
gfc_intrinsic_sym. That is later used to modify a corresponding __bultin_*.
cat ~/Programming/testcases/use.f90
module overload
interface
function sin(arg)
!GCC$ attributes simd_notinbranch :: sin
real, intent(in) :: arg
real :: sin
end function sin
end interface
end module
program test_overloaded_intrinsic
real(4) :: x(3200), y(3200), z(3200)
! this should be using simd clone
y = sin(x)
print *, y
! this not
z = cos(x)
print *, z
end
$ ./xgcc -B. ~/Programming/testcases/use.f90 -c -Ofast -fdump-tree-optimized=/dev/stdout -c
;; Function test_overloaded_intrinsic (MAIN__, funcdef_no=0, decl_uid=3815, cgraph_uid=1, symbol_order=0) (executed once)
test_overloaded_intrinsic ()
{
...
vect__3.14_58 = sinf.simdclone.0 (vect__2.13_60);
MEM[symbol: y, index: ivtmp.35_47, offset: 0B] = vect__3.14_58;
...
_6 = __builtin_cosf (_5);
MEM[symbol: z, index: ivtmp.30_46, offset: 0B] = _6;
...
}
That's what I have. I would like to ask Fortran folks about their opinion? I know
the part in gfc_match_gcc_attributes is bit tricky, but apart from that the rest
should be well formed.
Thoughts?

The gfc_conv_intrinsic_lib_function should be in
gfc_get_intrinsic_lib_fndecl instead I think
as you are adding the attribute once for each call it seems. I think
that it would be more

Hi.
I fixed that by clearing of simd attribute. But yes, it makes sense to set
it just once.

Post by Richard Biener
forward-looking to make the gfc_intrinsics_sym->simd flag a 'tree
attributes' list instead.

Well, I believe all the flags gfc_intrinsic_sym flags are used in very similar way
as I use it for simd flag.

Post by Richard Biener
That you do the matching in gfc_match_gcc_attributes is quite odd IMHO
but you said
that already. I'd say a more proper place would be where the FE
"ends" parsing of
the specification part of an interface.

I improved that and I also do it only for a module with 'vector_math' name.
Will we want that?

I don't think so.

Post by Martin Liška

Post by Richard Biener
Of course simd_notinbranch isn't exactly supporting all simd variants,
but not sure
how difficult it is to add this as 'simd' with arguments...

Will we need multiple variants?

The simd attribute can be used on regular functions as well to get vectorization
without -fopenmp-simd. But yes, for a standard vectorized math libary full
functionality isn't needed.

Post by Martin Liška

Post by Richard Biener
+module-include
+Fortran Joined Separate
+Use implicitelly a module
+
module-use or use-module or simply use? Or import? include sounds so C-ish.

Yep, I prefer module-use.

Post by Richard Biener
+void
+gfc_add_implicit_use (const char *module_name)
+{
+ implicit_module_name = module_name;
+}
so this works for exactly one module...

I extended that to support multiple ones.

Post by Richard Biener
I guess the part that is missing is to add target specific specs
fragments adding
-use FOO for fortran invocations, allowing FOO to not exist(?).

Will take a look at that.
$ cat ~/Programming/testcases/module.f90
module vector_math
interface
function sin(arg)
!GCC$ attributes simd_notinbranch :: sin
real, intent(in) :: arg
real :: sin
end function sin
end interface
end module
$ ./xgcc -B. -Ofast -c ~/Programming/testcases/module.f90
$ cat ~/Programming/testcases/use.f90
program test_overloaded_intrinsic
use vector_math
real(4) :: x(3200), y(3200), z(3200)
! this should be using simd clone
y = sin(x)
print *, y
! this not
z = cos(x)
print *, z
z = sin (z)
print *, z
end
$ ./xgcc -B. -Ofast -c ~/Programming/testcases/use.f90
6 | y = sin(x)
| 1
Error: Rank mismatch in argument ‘arg’ at (1) (scalar and rank-1)
13 | z = sin (z)
| 1
Error: Rank mismatch in argument ‘arg’ at (1) (scalar and rank-1)
Thanks,
Martin

Post by Richard Biener
Richard.

Post by Martin Liška
Thanks,
Martin

[Toon Moene]

On 10/30/2018 04:40 PM, Martin Liška wrote:

Did anyone ever helped you with this error message ? I haven't seen a
reply to the list to that effect ...

Post by Martin Liška
$ cat ~/Programming/testcases/module.f90
module vector_math
interface
function sin(arg)
!GCC$ attributes simd_notinbranch :: sin
real, intent(in) :: arg
real :: sin
end function sin
end interface
end module
$ ./xgcc -B. -Ofast -c ~/Programming/testcases/module.f90
$ cat ~/Programming/testcases/use.f90
program test_overloaded_intrinsic
use vector_math
real(4) :: x(3200), y(3200), z(3200)
! this should be using simd clone
y = sin(x)
print *, y
! this not
z = cos(x)
print *, z
z = sin (z)
print *, z
end
$ ./xgcc -B. -Ofast -c ~/Programming/testcases/use.f90
6 | y = sin(x)
| 1
Error: Rank mismatch in argument ‘arg’ at (1) (scalar and rank-1)
13 | z = sin (z)
| 1
Error: Rank mismatch in argument ‘arg’ at (1) (scalar and rank-1)

What is happening here is that you define an EXTERNAL function "sin"

Post by Martin Liška
function sin(arg)
real, intent(in) :: arg
real :: sin

is a real function of a scalar argument returning a scalar result.

However, in using it, you pass it 32000 element arrays of reals (you
don't need, and should omit, the (4) on the real declaration).

This works for the INTRINSIC functions "sin", "cos", etc., because the
Fortran language declares them to be ELEMENTAL.

This means that, while you can pass them a scalar argument and get a
scalar result, you can also pass them a rank-N array and get a rank-N
array result OF THE SAME SHAPE.

Post by Martin Liška
elemental real function sin(arg)
real, intent(in) :: arg
real :: sin

which declares a user-defined, EXTERNAL, ELEMENTAL function called "sin".

Hope this helps,

[Martin Liška]

Did anyone ever helped you with this error message ? I haven't seen a reply to the list to that effect ...

Hello.

No, you're the first one who replied. Thanks for it.

Post by Martin Liška
$ cat ~/Programming/testcases/module.f90
module vector_math
   interface
     function sin(arg)
       !GCC$ attributes simd_notinbranch :: sin
       real, intent(in) :: arg
       real :: sin
     end function sin
   end interface
end module
$ ./xgcc -B. -Ofast -c ~/Programming/testcases/module.f90
$ cat ~/Programming/testcases/use.f90
program test_overloaded_intrinsic
   use vector_math
   real(4) :: x(3200), y(3200), z(3200)
   ! this should be using simd clone
   y = sin(x)
   print *, y
   ! this not
   z = cos(x)
   print *, z
   z = sin (z)
   print *, z
end
$ ./xgcc -B. -Ofast -c ~/Programming/testcases/use.f90
     6 |   y = sin(x)
       |          1
Error: Rank mismatch in argument ‘arg’ at (1) (scalar and rank-1)
    13 |   z = sin (z)
       |           1
Error: Rank mismatch in argument ‘arg’ at (1) (scalar and rank-1)
      function sin(arg)
        real, intent(in) :: arg
        real :: sin

is a real function of a scalar argument returning a scalar result.
However, in using it, you pass it 32000 element arrays of reals (you don't need, and should omit, the (4) on the real declaration).
This works for the INTRINSIC functions "sin", "cos", etc., because the Fortran language declares them to be ELEMENTAL.

I see!

This means that, while you can pass them a scalar argument and get a scalar result, you can also pass them a rank-N array and get a rank-N array result OF THE SAME SHAPE.

Post by Martin Liška
      elemental real function sin(arg)
        real, intent(in) :: arg
        real :: sin

which declares a user-defined, EXTERNAL, ELEMENTAL function called "sin".

I can confirm that adding ELEMENTAL removes the compilation error.

Now I slightly adjusted the 0001-Support-simd-attribute-propagation-for-a-vector_math.patch patch:
- I do not math match module by name
- I mark simd flag in mio_symbol, which is the place where a module symbol is created.

Now I'm able to add 'use vector_math' directive to my test:

module vector_math
interface
elemental function sin(arg)
!GCC$ attributes simd_notinbranch :: sin
real, intent(in) :: arg
real :: sin
end function sin
end interface
end module

program test_overloaded_intrinsic
use vector_math
real :: x(3200), y(3200), z(3200)

! this should be using simd clone
y = sin(x)
print *, y

! this not
z = cos(x)
print *, z

z = sin (z)
print *, z
end

However, now I need significant hacking in resolve.c in order to persuade Fortran FE
that my "sin" function (being external module function)
can really be seen as intrinsic (please see 0002-* patch).
Any ideas how to make the resolution working without hacking?

Thank you,
Martin

Hope this helps,

[Thomas Koenig]

Hi Martin,

Post by Martin Liška
module vector_math
interface
elemental function sin(arg)
!GCC$ attributes simd_notinbranch :: sin
real, intent(in) :: arg
real :: sin
end function sin
end interface
end module

I'm glad that this project we discussed at the GNU Cauldron is
moving forward!

Regarding the syntax: I am not sure that using an external function
and then trying to coerce it to an intrinsic is something that is
easily or cleanly done (as you yourself noted).

Would it be possible to use something like

module x
!GCC$ attributes simd_notinbranch, real(4) :: sin
!GCC$ attributes simd_notinbranch, real(8) :: sin
end module x

Attribute parsing might have to be extended a little, but
this would create one clear point where to create the
specific version of the library routine.

What do you think?

Regards

Thomas

[Toon Moene]

Post by Thomas Koenig
Would it be possible to use something like
module x
  !GCC$ attributes simd_notinbranch, real(4) :: sin
  !GCC$ attributes simd_notinbranch, real(8) :: sin
end module x
Attribute parsing might have to be extended a little, but
this would create one clear point where to create the
specific version of the library routine.

Wouldn't it be much simpler to simply add the attribute *in the Front
End* while generating the call to the INTRINSIC sin ?

Or is there something I miss here ...

[Martin Liška]

Post by Thomas Koenig
Would it be possible to use something like
module x
   !GCC$ attributes simd_notinbranch, real(4) :: sin
   !GCC$ attributes simd_notinbranch, real(8) :: sin
end module x
Attribute parsing might have to be extended a little, but
this would create one clear point where to create the
specific version of the library routine.

Wouldn't it be much simpler to simply add the attribute *in the Front End* while generating the call to the INTRINSIC sin ?
Or is there something I miss here ...

Hi.

Can you please explain that more?

Martin

[Thomas Koenig]

Hi Toon,

Post by Toon Moene
Wouldn't it be much simpler to simply add the attribute *in the Front
End* while generating the call to the INTRINSIC sin ?

The problem is that it is not clear, when building the compiler,
if the target system supplies the vectorized version of the intrinsics,
as modern glibc does, but there are people using other libraries
or older versions.

So, the idea is that glibc supplies a configuration file (we can't
really use a header file, like we can in C) which is read in on
compiler startup. And it would be easiest to make this
some sort of module, consisting of Fortran code.

Regards

Thomas

[Martin Liška]

Post by Thomas Koenig
Hi Martin,

Post by Martin Liška
module vector_math
   interface
     elemental function sin(arg)
       !GCC$ attributes simd_notinbranch :: sin
       real, intent(in) :: arg
       real :: sin
     end function sin
   end interface
end module

I'm glad that this project we discussed at the GNU Cauldron is
moving forward!

Hello.

Agree with that. So far I've been provided very nice guidance and
I hope we can get it into GCC 9.1.

Post by Thomas Koenig
Regarding the syntax: I am not sure that using an external function
and then trying to coerce it to an intrinsic is something that is
easily or cleanly done (as you yourself noted).
Would it be possible to use something like
module x
  !GCC$ attributes simd_notinbranch, real(4) :: sin
  !GCC$ attributes simd_notinbranch, real(8) :: sin
end module x
Attribute parsing might have to be extended a little, but
this would create one clear point where to create the
specific version of the library routine.
What do you think?

Sounds reasonable for me, I can play with the idea tomorrow.

Thank you,
Martin

Post by Thomas Koenig
Regards
    Thomas

[Richard Biener]

Post by Thomas Koenig
Hi Martin,

Post by Martin Liška
module vector_math
interface
elemental function sin(arg)
!GCC$ attributes simd_notinbranch :: sin
real, intent(in) :: arg
real :: sin
end function sin
end interface
end module

I'm glad that this project we discussed at the GNU Cauldron is
moving forward!
Regarding the syntax: I am not sure that using an external function
and then trying to coerce it to an intrinsic is something that is
easily or cleanly done (as you yourself noted).
Would it be possible to use something like
module x
!GCC$ attributes simd_notinbranch, real(4) :: sin
!GCC$ attributes simd_notinbranch, real(8) :: sin
end module x

That might very well work better indeed. Are there any
math library routines that are _not_ intrinsics? Are all
builtins we initialize through mathbuiltins.def also
Fortran intrinsics? The important implementation detail
is that we need to amend those builtin declarations via
the module - so one of my thoughts (for future improvement...)
would be to re-organize the mathbuiltins.def thing more
like an intrinsic module that is unconditionally USEd. Of
course if there's no way to "declare" an intrinsic in fortran
syntax that becomes somewhat difficult.

Post by Thomas Koenig
Attribute parsing might have to be extended a little, but
this would create one clear point where to create the
specific version of the library routine.
What do you think?
Regards
Thomas

[Thomas Koenig]

Post by Richard Biener

Post by Thomas Koenig
Hi Martin,

Post by Martin Liška
module vector_math
interface
elemental function sin(arg)
!GCC$ attributes simd_notinbranch :: sin
real, intent(in) :: arg
real :: sin
end function sin
end interface
end module

I'm glad that this project we discussed at the GNU Cauldron is
moving forward!
Regarding the syntax: I am not sure that using an external function
and then trying to coerce it to an intrinsic is something that is
easily or cleanly done (as you yourself noted).
Would it be possible to use something like
module x
!GCC$ attributes simd_notinbranch, real(4) :: sin
!GCC$ attributes simd_notinbranch, real(8) :: sin
end module x

That might very well work better indeed.

Mabe it would not even need a module - just having encountered
the attributes line could be enough to modify the declaration.

Post by Richard Biener
Are there any
math library routines that are _not_ intrinsics?

If you mean glibc math functions, there are a few missing -
Fortran does not have cbrt, copysign and exp2, just to mention the
first few that strike me reading the list in alphabetical order.

Post by Richard Biener
Are all
builtins we initialize through mathbuiltins.def also
Fortran intrinsics?

You have to be a bit careful here about generic functions (which many
of the intrinsics are). For example, in Fortran, SIN is a generic
function which can take different argument types. This is then
mapped to the specific function, which may be something like
__builtin_sinf.

Post by Richard Biener
The important implementation detail
is that we need to amend those builtin declarations via
the module - so one of my thoughts (for future improvement...)
would be to re-organize the mathbuiltins.def thing more
like an intrinsic module that is unconditionally USEd. Of
course if there's no way to "declare" an intrinsic in fortran
syntax that becomes somewhat difficult.

I'm not sure we really need a module. Just modifying the
declarations (changing whatever generates the declaration
of __builtin_sinf to use something else) directly might
be enough.

Actually, you can affirm that something is an intrinsic
function, via the INTRINSIC statement or attribute.
You can't do that twice, though, but something like

intrinsic :: sin
!GCC$ attributes simd_notinbranch, real(4) :: sin
!GCC$ attributes simd_notinbranch, real(8) :: sin

would be legal.

(The intended use of INTRINSIC is something else:
A compiler can provide additional intrinsics, more than
what the languages specifies. This ist mostly harmless because
a user can still use the name for his own variables or
procedures. However, if you use these extensions, it
is sometimes a good idea to specify them as INTRINSIC,
so that if you try to compile the code with another
compiler, you get at least a clean error message).

Regards

Thomas

[Richard Biener]

Post by Thomas Koenig

Post by Richard Biener

Post by Thomas Koenig
Hi Martin,

Post by Martin Liška
module vector_math
interface
elemental function sin(arg)
!GCC$ attributes simd_notinbranch :: sin
real, intent(in) :: arg
real :: sin
end function sin
end interface
end module

I'm glad that this project we discussed at the GNU Cauldron is
moving forward!
Regarding the syntax: I am not sure that using an external function
and then trying to coerce it to an intrinsic is something that is
easily or cleanly done (as you yourself noted).
Would it be possible to use something like
module x
!GCC$ attributes simd_notinbranch, real(4) :: sin
!GCC$ attributes simd_notinbranch, real(8) :: sin
end module x

That might very well work better indeed.

Mabe it would not even need a module - just having encountered
the attributes line could be enough to modify the declaration.

Post by Richard Biener
Are there any
math library routines that are _not_ intrinsics?

If you mean glibc math functions, there are a few missing -
Fortran does not have cbrt, copysign and exp2, just to mention the
first few that strike me reading the list in alphabetical order.

Post by Richard Biener
Are all
builtins we initialize through mathbuiltins.def also
Fortran intrinsics?

You have to be a bit careful here about generic functions (which many
of the intrinsics are). For example, in Fortran, SIN is a generic
function which can take different argument types. This is then
mapped to the specific function, which may be something like
__builtin_sinf.

Post by Richard Biener
The important implementation detail
is that we need to amend those builtin declarations via
the module - so one of my thoughts (for future improvement...)
would be to re-organize the mathbuiltins.def thing more
like an intrinsic module that is unconditionally USEd. Of
course if there's no way to "declare" an intrinsic in fortran
syntax that becomes somewhat difficult.

I'm not sure we really need a module. Just modifying the
declarations (changing whatever generates the declaration
of __builtin_sinf to use something else) directly might
be enough.

Hmm, OK. So of course I originally thought of doing
an "include file" but a module looked more "modern"...

So if we'd instead do sth like the C -include command-line
option and have glibc just produce a textual file with
!GCC$ attributes lines then would that work when being
pre-"included" to each fortran source? That would even
solve the issue of having multiple .mod variants for
several GCC versions...

Post by Thomas Koenig
Actually, you can affirm that something is an intrinsic
function, via the INTRINSIC statement or attribute.
You can't do that twice, though, but something like
intrinsic :: sin
!GCC$ attributes simd_notinbranch, real(4) :: sin
!GCC$ attributes simd_notinbranch, real(8) :: sin
would be legal.
A compiler can provide additional intrinsics, more than
what the languages specifies. This ist mostly harmless because
a user can still use the name for his own variables or
procedures. However, if you use these extensions, it
is sometimes a good idea to specify them as INTRINSIC,
so that if you try to compile the code with another
compiler, you get at least a clean error message).
Regards
Thomas

[Martin Liška]

Post by Richard Biener

Post by Thomas Koenig

Post by Richard Biener

Post by Thomas Koenig
Hi Martin,

Post by Martin Liška
module vector_math
interface
elemental function sin(arg)
!GCC$ attributes simd_notinbranch :: sin
real, intent(in) :: arg
real :: sin
end function sin
end interface
end module

I'm glad that this project we discussed at the GNU Cauldron is
moving forward!
Regarding the syntax: I am not sure that using an external function
and then trying to coerce it to an intrinsic is something that is
easily or cleanly done (as you yourself noted).
Would it be possible to use something like
module x
!GCC$ attributes simd_notinbranch, real(4) :: sin
!GCC$ attributes simd_notinbranch, real(8) :: sin
end module x

That might very well work better indeed.

Mabe it would not even need a module - just having encountered
the attributes line could be enough to modify the declaration.

Post by Richard Biener
Are there any
math library routines that are _not_ intrinsics?

If you mean glibc math functions, there are a few missing -
Fortran does not have cbrt, copysign and exp2, just to mention the
first few that strike me reading the list in alphabetical order.

Post by Richard Biener
Are all
builtins we initialize through mathbuiltins.def also
Fortran intrinsics?

You have to be a bit careful here about generic functions (which many
of the intrinsics are). For example, in Fortran, SIN is a generic
function which can take different argument types. This is then
mapped to the specific function, which may be something like
__builtin_sinf.

Post by Richard Biener
The important implementation detail
is that we need to amend those builtin declarations via
the module - so one of my thoughts (for future improvement...)
would be to re-organize the mathbuiltins.def thing more
like an intrinsic module that is unconditionally USEd. Of
course if there's no way to "declare" an intrinsic in fortran
syntax that becomes somewhat difficult.

I'm not sure we really need a module. Just modifying the
declarations (changing whatever generates the declaration
of __builtin_sinf to use something else) directly might
be enough.

Hmm, OK. So of course I originally thought of doing
an "include file" but a module looked more "modern"...
So if we'd instead do sth like the C -include command-line
option and have glibc just produce a textual file with
!GCC$ attributes lines then would that work when being
pre-"included" to each fortran source? That would even
solve the issue of having multiple .mod variants for
several GCC versions...

Post by Thomas Koenig
Actually, you can affirm that something is an intrinsic
function, via the INTRINSIC statement or attribute.
You can't do that twice, though, but something like
intrinsic :: sin
!GCC$ attributes simd_notinbranch, real(4) :: sin
!GCC$ attributes simd_notinbranch, real(8) :: sin
would be legal.
A compiler can provide additional intrinsics, more than
what the languages specifies. This ist mostly harmless because
a user can still use the name for his own variables or
procedures. However, if you use these extensions, it
is sometimes a good idea to specify them as INTRINSIC,
so that if you try to compile the code with another
compiler, you get at least a clean error message).
Regards
Thomas

Hi.

Thank you all for comments. I'm sending patch that works with following code snippets:

$ cat usage.F90
program test_overloaded_intrinsic
#include "vector-math.f90"

real :: x(3200), y(3200), z(3200)

! this should be using simd clone
y = sin(x)
print *, y

! this not
z = cos(x)
print *, z

z = sin (z)
print *, z
end

$ cat vector-math.f90
intrinsic :: sin
!GCC$ attributes simd_notinbranch :: sin

$ ./xgcc -B. -Ofast -c ~/Programming/testcases/usage.F90 -fdump-tree-optimized=/dev/stdout | grep sin
vect__3.23_101 = sinf.simdclone.0 (vect__2.22_100);
vect__9.17_87 = sinf.simdclone.0 (vect__8.16_10);

Now the question is how to add a new argument that will implicitly include additional file.
Should that be added to Fortran pre-processor? Hints welcomed.

Note that I removed the ', real(4)' from attribute definition as there's just single gfc_intrinsic_sym
for sin* functions in Fortran FE. Is it fine?

About the math functions that are not intrinsics. If I see correctly current list of vectorized
math functions is here:
https://sourceware.org/git/?p=glibc.git;a=blob_plain;f=sysdeps/x86/fpu/bits/math-vector.h;hb=HEAD

Martin

[Thomas Koenig]

Hi Martin,

Post by Martin Liška
$ cat usage.F90
program test_overloaded_intrinsic
#include "vector-math.f90"
real :: x(3200), y(3200), z(3200)
! this should be using simd clone
y = sin(x)
print *, y
! this not
z = cos(x)
print *, z
z = sin (z)
print *, z
end

This is looking good, in principle.

A few points:

If we want to add this to the top of each translation unit, the
!GCC$ attributes simd_notinbranch :: sin

work if it is put into the first lines of program code.

Post by Martin Liška
Note that I removed the ', real(4)' from attribute definition as there's just single gfc_intrinsic_sym
for sin* functions in Fortran FE. Is it fine?

No. The problem is that "sin" is generic and overloaded with different
versions for different real types.

We may have vectorization for real(kind=4) and real(kind=8), but not for
real (kind=16), for example.

So, you could use either the intrinsic name with the precision or (maybe
that is an even better idea) the name of the C function that is
overloaded.

Post by Martin Liška
About the math functions that are not intrinsics. If I see correctly current list of vectorized
https://sourceware.org/git/?p=glibc.git;a=blob_plain;f=sysdeps/x86/fpu/bits/math-vector.h;hb=HEAD

I see sincos and sincosf there. There is no sincos intrinsic in Fortran,
but there could be a lot of value in having a vecorized sincos if
the middle end detects an opportunity for it.

So, putting the attributes on the middle end declarations might actually
be the better way handling this.

Regards

Thomas

[Thomas Koenig]

Post by Thomas Koenig
If we want to add this to the top of each translation unit, the
!GCC$ attributes simd_notinbranch :: sin
work if it is put into the first lines of program code.

Or even more radical:

Could we just add a
-fvectorized-function=sin,cos,sinf,cosf,sincos,sincosf, ...

option and read that from a predefined place via @FILE ?

This could be even more straightforward than implementnig this
as an extension to the Fortran parser.

Regards

Thomas

[Martin Liška]

Post by Thomas Koenig
If we want to add this to the top of each translation unit, the
!GCC$ attributes simd_notinbranch :: sin
work if it is put into the first lines of program code.

Could we just add a -fvectorized-function=sin,cos,sinf,cosf,sincos,sincosf, ...

Hi.

To be honest, that approach was my first idea how to implement that. But the
module approach looks more saner. Anyway I'm attaching patch that does that.

Having that, one can see:

$ cat ~/Programming/testcases/f.f90
program test_overloaded_intrinsic
real(4) :: x1(3200), x2(3200)
real(8) :: z1(3200), z2(3200)

x2 = sin(x1)
print *, x2
z2 = sin(z1)
print *, z2
end

$ ./xgcc -B. ~/Programming/testcases/f.f90 -c -fvectorized-functions=sinf -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
vect__3.14_7 = sinf.simdclone.0 (vect__2.13_8);
_6 = __builtin_sin (_5);

$ ./xgcc -B. ~/Programming/testcases/f.f90 -c -fvectorized-functions=sin -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
_3 = __builtin_sinf (_2);
vect__6.14_7 = sin.simdclone.0 (vect__5.13_8);

Richi can you please express your opinion?

Martin

This could be even more straightforward than implementnig this
as an extension to the Fortran parser.
Regards
    Thomas

[Richard Biener]

Post by Martin Liška

Post by Thomas Koenig
If we want to add this to the top of each translation unit, the
!GCC$ attributes simd_notinbranch :: sin
work if it is put into the first lines of program code.

Could we just add a -fvectorized-function=sin,cos,sinf,cosf,sincos,sincosf, ...

Hi.
To be honest, that approach was my first idea how to implement that. But the
module approach looks more saner. Anyway I'm attaching patch that does that.
$ cat ~/Programming/testcases/f.f90
program test_overloaded_intrinsic
real(4) :: x1(3200), x2(3200)
real(8) :: z1(3200), z2(3200)
x2 = sin(x1)
print *, x2
z2 = sin(z1)
print *, z2
end
$ ./xgcc -B. ~/Programming/testcases/f.f90 -c -fvectorized-functions=sinf -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
vect__3.14_7 = sinf.simdclone.0 (vect__2.13_8);
_6 = __builtin_sin (_5);
$ ./xgcc -B. ~/Programming/testcases/f.f90 -c -fvectorized-functions=sin -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
_3 = __builtin_sinf (_2);
vect__6.14_7 = sin.simdclone.0 (vect__5.13_8);
Richi can you please express your opinion?

I don't think this scales well. With OMP you can also declare
variants that work on arrays
and variants with a mask (inbranch). I also don't think we have a decent way to
add @foo in specs processing but we do have precedence for
pre-including things in
the C frontend (stdc-predef.h, also coming from glibc!). There is a
TARGET_C_PREINCLUDE
hook for this, used from push_command_line_include () using libcpps
cpp_push_default_include
feature. I could imagine a similar mechanism used for Fortran, not
sure if the FE uses the
preprocessor unconditionally though.

Richard.

Post by Martin Liška
Martin

This could be even more straightforward than implementnig this
as an extension to the Fortran parser.
Regards
Thomas

[Martin Liška]

Post by Richard Biener

Post by Martin Liška

Post by Thomas Koenig
If we want to add this to the top of each translation unit, the
!GCC$ attributes simd_notinbranch :: sin
work if it is put into the first lines of program code.

Could we just add a -fvectorized-function=sin,cos,sinf,cosf,sincos,sincosf, ...

Hi.
To be honest, that approach was my first idea how to implement that. But the
module approach looks more saner. Anyway I'm attaching patch that does that.
$ cat ~/Programming/testcases/f.f90
program test_overloaded_intrinsic
real(4) :: x1(3200), x2(3200)
real(8) :: z1(3200), z2(3200)
x2 = sin(x1)
print *, x2
z2 = sin(z1)
print *, z2
end
$ ./xgcc -B. ~/Programming/testcases/f.f90 -c -fvectorized-functions=sinf -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
vect__3.14_7 = sinf.simdclone.0 (vect__2.13_8);
_6 = __builtin_sin (_5);
$ ./xgcc -B. ~/Programming/testcases/f.f90 -c -fvectorized-functions=sin -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
_3 = __builtin_sinf (_2);
vect__6.14_7 = sin.simdclone.0 (vect__5.13_8);
Richi can you please express your opinion?

I don't think this scales well. With OMP you can also declare
variants that work on arrays
and variants with a mask (inbranch).

Ok, but these are not subject of currently exposed functions:
https://sourceware.org/git/?p=glibc.git;a=blob_plain;f=sysdeps/x86/fpu/bits/math-vector.h;hb=HEAD

if I'm correct? For user-defined Fortran functions we can still come up
with '!GCC$ attributes simd_* :: myfunction' attribute.

Post by Richard Biener
I also don't think we have a decent way to
pre-including things in
the C frontend (stdc-predef.h, also coming from glibc!). There is a
TARGET_C_PREINCLUDE
hook for this, used from push_command_line_include () using libcpps
cpp_push_default_include
feature. I could imagine a similar mechanism used for Fortran, not
sure if the FE uses the
preprocessor unconditionally though.

Isn't that the same I suggested in previous version: '#include "vector-math.f90"'.
Note that Thomas mentioned that some of functions listed in math-vector.h are
not Fortran intrinsics. That's why he recommended to base the implementation
on middle-end builtins.

Martin

Post by Richard Biener
Richard.

Post by Martin Liška
Martin

This could be even more straightforward than implementnig this
as an extension to the Fortran parser.
Regards
Thomas

[Paul Richard Thomas]

Hi All,

If I had realised which way this was where we were going to go when we
were in Manchester, I would have kept off the subject of .mod files to
enable the simd intrinsics :-) That said, I didn't know anything about
how the intrinsics are handled in fortran so it was rather a case of
the blind leading the blind.

IMHO the simd functions should be tested for availability and declared
when -ftree-vectorize is set (default for -O3), with an option to
disable them. The declarations can all be done in f95-lang.c,
mathbuiltins.def and trans-decl.c. The choice between the scalar and
simd versions can be made either on the basis of the expression having
an gfc_ss (ie. a scalarized loop) or the call being made within a do
loop. In these cases, the simd version should be used. In that way,
gfortran would behave like ifort and, I presume, would give gfortran a
boost with the polyhedron testsuite, for example.

The reason why I was asking about the interface (in the fortran sense)
of the simd functions was that I was wondering how expressions
involving simd functions would work; eg what happens with z(:) =
x(:)*sin(x(:)) ? If -ftree-vectorize does this just happen
automatically within the scalarization/do loop? ie. is the indexing
detected and all the magic done automatically?

While being able to set attributes C-style would be nice and should be
implemented, I don't think that it makes for an especially transparent
API. "Works out of the box" should be the gfortran motto. Somehow
ifort is doing just this.

Regards

Paul

Post by Martin Liška

Post by Richard Biener

Post by Martin Liška

Post by Thomas Koenig
If we want to add this to the top of each translation unit, the
!GCC$ attributes simd_notinbranch :: sin
work if it is put into the first lines of program code.

Could we just add a -fvectorized-function=sin,cos,sinf,cosf,sincos,sincosf, ...

Hi.
To be honest, that approach was my first idea how to implement that. But the
module approach looks more saner. Anyway I'm attaching patch that does that.
$ cat ~/Programming/testcases/f.f90
program test_overloaded_intrinsic
real(4) :: x1(3200), x2(3200)
real(8) :: z1(3200), z2(3200)
x2 = sin(x1)
print *, x2
z2 = sin(z1)
print *, z2
end
$ ./xgcc -B. ~/Programming/testcases/f.f90 -c -fvectorized-functions=sinf -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
vect__3.14_7 = sinf.simdclone.0 (vect__2.13_8);
_6 = __builtin_sin (_5);
$ ./xgcc -B. ~/Programming/testcases/f.f90 -c -fvectorized-functions=sin -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
_3 = __builtin_sinf (_2);
vect__6.14_7 = sin.simdclone.0 (vect__5.13_8);
Richi can you please express your opinion?

I don't think this scales well. With OMP you can also declare
variants that work on arrays
and variants with a mask (inbranch).

https://sourceware.org/git/?p=glibc.git;a=blob_plain;f=sysdeps/x86/fpu/bits/math-vector.h;hb=HEAD
if I'm correct? For user-defined Fortran functions we can still come up
with '!GCC$ attributes simd_* :: myfunction' attribute.

Post by Richard Biener
I also don't think we have a decent way to
pre-including things in
the C frontend (stdc-predef.h, also coming from glibc!). There is a
TARGET_C_PREINCLUDE
hook for this, used from push_command_line_include () using libcpps
cpp_push_default_include
feature. I could imagine a similar mechanism used for Fortran, not
sure if the FE uses the
preprocessor unconditionally though.

Isn't that the same I suggested in previous version: '#include "vector-math.f90"'.
Note that Thomas mentioned that some of functions listed in math-vector.h are
not Fortran intrinsics. That's why he recommended to base the implementation
on middle-end builtins.
Martin

Post by Richard Biener
Richard.

Post by Martin Liška
Martin

This could be even more straightforward than implementnig this
as an extension to the Fortran parser.
Regards
Thomas

[Richard Biener]

On Wed, Nov 7, 2018 at 5:20 PM Paul Richard Thomas

Post by Paul Richard Thomas
Hi All,
If I had realised which way this was where we were going to go when we
were in Manchester, I would have kept off the subject of .mod files to
enable the simd intrinsics :-) That said, I didn't know anything about
how the intrinsics are handled in fortran so it was rather a case of
the blind leading the blind.
IMHO the simd functions should be tested for availability and declared
when -ftree-vectorize is set (default for -O3), with an option to
disable them. The declarations can all be done in f95-lang.c,
mathbuiltins.def and trans-decl.c. The choice between the scalar and
simd versions can be made either on the basis of the expression having
an gfc_ss (ie. a scalarized loop) or the call being made within a do
loop. In these cases, the simd version should be used. In that way,
gfortran would behave like ifort and, I presume, would give gfortran a
boost with the polyhedron testsuite, for example.
The reason why I was asking about the interface (in the fortran sense)
of the simd functions was that I was wondering how expressions
involving simd functions would work; eg what happens with z(:) =
x(:)*sin(x(:)) ? If -ftree-vectorize does this just happen
automatically within the scalarization/do loop? ie. is the indexing
detected and all the magic done automatically?
While being able to set attributes C-style would be nice and should be
implemented, I don't think that it makes for an especially transparent
API. "Works out of the box" should be the gfortran motto. Somehow
ifort is doing just this.

The reason ifort can do this is that it comes with its vectorized math
library. If GCC shipped such then it would know exactly which functions
it has vectorized versions for and which not ...

So somehow we have to interface with glibc here (or other vendors
libraries). One variant is to hard-code some state into GCC
(see -mveclibabi= we have for ACML and SVML). The other variant
is do what glibc does for C - simply annotate functions appropriately
in the header. So the whole idea is to make it possible for glibc
to provide such header to GFortran - a header that specifies that
certain _C ABI_ math functions are also available in vectorized form.

Richard.

Post by Paul Richard Thomas
Regards
Paul

Post by Martin Liška

Post by Richard Biener

Post by Martin Liška

Post by Thomas Koenig
If we want to add this to the top of each translation unit, the
!GCC$ attributes simd_notinbranch :: sin
work if it is put into the first lines of program code.

Could we just add a -fvectorized-function=sin,cos,sinf,cosf,sincos,sincosf, ...

Hi.
To be honest, that approach was my first idea how to implement that. But the
module approach looks more saner. Anyway I'm attaching patch that does that.
$ cat ~/Programming/testcases/f.f90
program test_overloaded_intrinsic
real(4) :: x1(3200), x2(3200)
real(8) :: z1(3200), z2(3200)
x2 = sin(x1)
print *, x2
z2 = sin(z1)
print *, z2
end
$ ./xgcc -B. ~/Programming/testcases/f.f90 -c -fvectorized-functions=sinf -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
vect__3.14_7 = sinf.simdclone.0 (vect__2.13_8);
_6 = __builtin_sin (_5);
$ ./xgcc -B. ~/Programming/testcases/f.f90 -c -fvectorized-functions=sin -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
_3 = __builtin_sinf (_2);
vect__6.14_7 = sin.simdclone.0 (vect__5.13_8);
Richi can you please express your opinion?

I don't think this scales well. With OMP you can also declare
variants that work on arrays
and variants with a mask (inbranch).

https://sourceware.org/git/?p=glibc.git;a=blob_plain;f=sysdeps/x86/fpu/bits/math-vector.h;hb=HEAD
if I'm correct? For user-defined Fortran functions we can still come up
with '!GCC$ attributes simd_* :: myfunction' attribute.

Post by Richard Biener
I also don't think we have a decent way to
pre-including things in
the C frontend (stdc-predef.h, also coming from glibc!). There is a
TARGET_C_PREINCLUDE
hook for this, used from push_command_line_include () using libcpps
cpp_push_default_include
feature. I could imagine a similar mechanism used for Fortran, not
sure if the FE uses the
preprocessor unconditionally though.

Isn't that the same I suggested in previous version: '#include "vector-math.f90"'.
Note that Thomas mentioned that some of functions listed in math-vector.h are
not Fortran intrinsics. That's why he recommended to base the implementation
on middle-end builtins.
Martin

Post by Richard Biener
Richard.

Post by Martin Liška
Martin

This could be even more straightforward than implementnig this
as an extension to the Fortran parser.
Regards
Thomas

--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

[Richard Biener]

Post by Martin Liška

Post by Richard Biener

Post by Martin Liška

Post by Thomas Koenig
If we want to add this to the top of each translation unit, the
!GCC$ attributes simd_notinbranch :: sin
work if it is put into the first lines of program code.

Could we just add a -fvectorized-function=sin,cos,sinf,cosf,sincos,sincosf, ...

Hi.
To be honest, that approach was my first idea how to implement that. But the
module approach looks more saner. Anyway I'm attaching patch that does that.
$ cat ~/Programming/testcases/f.f90
program test_overloaded_intrinsic
real(4) :: x1(3200), x2(3200)
real(8) :: z1(3200), z2(3200)
x2 = sin(x1)
print *, x2
z2 = sin(z1)
print *, z2
end
$ ./xgcc -B. ~/Programming/testcases/f.f90 -c -fvectorized-functions=sinf -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
vect__3.14_7 = sinf.simdclone.0 (vect__2.13_8);
_6 = __builtin_sin (_5);
$ ./xgcc -B. ~/Programming/testcases/f.f90 -c -fvectorized-functions=sin -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
_3 = __builtin_sinf (_2);
vect__6.14_7 = sin.simdclone.0 (vect__5.13_8);
Richi can you please express your opinion?

I don't think this scales well. With OMP you can also declare
variants that work on arrays
and variants with a mask (inbranch).

https://sourceware.org/git/?p=glibc.git;a=blob_plain;f=sysdeps/x86/fpu/bits/math-vector.h;hb=HEAD
if I'm correct? For user-defined Fortran functions we can still come up
with '!GCC$ attributes simd_* :: myfunction' attribute.

Post by Richard Biener
I also don't think we have a decent way to
pre-including things in
the C frontend (stdc-predef.h, also coming from glibc!). There is a
TARGET_C_PREINCLUDE
hook for this, used from push_command_line_include () using libcpps
cpp_push_default_include
feature. I could imagine a similar mechanism used for Fortran, not
sure if the FE uses the
preprocessor unconditionally though.

Isn't that the same I suggested in previous version: '#include "vector-math.f90"'.
Note that Thomas mentioned that some of functions listed in math-vector.h are
not Fortran intrinsics. That's why he recommended to base the implementation
on middle-end builtins.

Sure, fortran can declare interfaces to C functions. IIRC via sth like
bind(c, name="sinf"). So we can very well adjust the proposed syntax
to sth like

!GCC$ attributes simd_notinbranch, bind(c, name="sinf") :: sin

? Of course the FE then would need to do name-lookup for 'sinf' and
find its own builtin declaration as generated from math-builtins.def.

Richard.

Post by Martin Liška
Martin

Post by Richard Biener
Richard.

Post by Martin Liška
Martin

This could be even more straightforward than implementnig this
as an extension to the Fortran parser.
Regards
Thomas

[Martin Liška]

Post by Thomas Koenig
Hi Martin,

Post by Martin Liška
$ cat usage.F90
program test_overloaded_intrinsic
#include "vector-math.f90"
   real :: x(3200), y(3200), z(3200)
   ! this should be using simd clone
   y = sin(x)
   print *, y
   ! this not
   z = cos(x)
   print *, z
   z = sin (z)
   print *, z
end

This is looking good, in principle.

Hi.

As Richi mentioned, the list of middle-end built-ins does not scale well.
So the way discussed in this sub-thread looks preferred.

Post by Thomas Koenig
If we want to add this to the top of each translation unit, the
!GCC$ attributes simd_notinbranch :: sin
work if it is put into the first lines of program code.

Does it mean that the '#include "vector-math.f90"' should be put before
a program directive? If so, then I see following error for:

#include "vector-math.f90"
program test_overloaded_intrinsic
...

$ ./xgcc -B. -Ofast -c ~/Programming/testcases/usage.F90
/home/marxin/Programming/testcases/usage.F90:2:33:

2 | program test_overloaded_intrinsic
| 1
Error: Unexpected PROGRAM statement at (1)

Post by Thomas Koenig

Post by Martin Liška
Note that I removed the ', real(4)' from attribute definition as there's just single gfc_intrinsic_sym
for sin* functions in Fortran FE. Is it fine?

No. The problem is that "sin" is generic and overloaded with different
versions for different real types.
We may have vectorization for real(kind=4) and real(kind=8), but not for
real (kind=16), for example.
So, you could use either the intrinsic name with the precision or (maybe
that is an even better idea) the name of the C function that is
overloaded.

I've got it. It's implemented in attached patch.

Post by Thomas Koenig

Post by Martin Liška
About the math functions that are not intrinsics. If I see correctly current list of vectorized
https://sourceware.org/git/?p=glibc.git;a=blob_plain;f=sysdeps/x86/fpu/bits/math-vector.h;hb=HEAD

I see sincos and sincosf there. There is no sincos intrinsic in Fortran,
but there could be a lot of value in having a vecorized sincos if
the middle end detects an opportunity for it.

Btw. I'm curios how can one use these built-ins from *.f90 file so that they will
be converted into corresponding __builtin_* functions when CALL_EXPRs are created?

Example of what I have:

$ cat vector-math.f90
intrinsic :: sin
!GCC$ attributes simd_notinbranch, type=real(4) :: sin

Here I invented 'type=*' parsing.

$ cat usage.F90
program test_overloaded_intrinsic
#include "vector-math.f90"
real(4) :: x4(3200), y4(3200)
real(8) :: x8(3200), y8(3200)

! this should be using simd clone
y4 = sin(x4)
print *, y4

! this should not be using simd clone
y4 = sin(x8)
print *, y8
end

$ ./xgcc -B. -Ofast -c ~/Programming/testcases/usage.F90 -fdump-tree-optimized=/dev/stdout | grep sin
vect__3.14_11 = sinf.simdclone.0 (vect__2.13_61);
_6 = __builtin_sin (_5);

Martin

Post by Thomas Koenig
So, putting the attributes on the middle end declarations might actually
be the better way handling this.
Regards
    Thomas

[Thomas Koenig]

Post by Martin Liška
As Richi mentioned, the list of middle-end built-ins does not scale well.

It is not clear to me what that means. Could you elaborate?
At the moment, this would still be my preferred solution.

(If it is a problem of too-long lines: Repeated uses of the
option with different names should just work, or can
be made to work).

Post by Martin Liška
So the way discussed in this sub-thread looks preferred.

I have given this some more thought.

First, any solution should include sincos and other functions
that are not Fortran intrnisics.

Second, if you include any additional lines from an external
file, they have to be syntactially valid outside any translation
unit, or you will run into the exact problem that you

Post by Martin Liška
Does it mean that the '#include "vector-math.f90"' should be put before
a program directive?

The problem is that you can start a valid program without a PROGRAM
statement, so

intrnisic :: sin
end

is a valid program, and for

intrinsic :: sin
program foo

the compiler correctly complains. So, you need to take out the
need for the intrinsic:: sin line.

(Also, the user is free to declare a variable sin, although it
is not advisable).

Post by Martin Liška

Post by Thomas Koenig
So, you could use either the intrinsic name with the precision or (maybe
that is an even better idea) the name of the C function that is
overloaded.

I've got it. It's implemented in attached patch.

It's not quite clear to me Which of the two options you mean :-)
Regards

Thomas

[Richard Biener]

Post by Thomas Koenig

Post by Martin Liška
As Richi mentioned, the list of middle-end built-ins does not scale well.

It is not clear to me what that means. Could you elaborate?
At the moment, this would still be my preferred solution.
(If it is a problem of too-long lines: Repeated uses of the
option with different names should just work, or can
be made to work).

Post by Martin Liška
So the way discussed in this sub-thread looks preferred.

I have given this some more thought.
First, any solution should include sincos and other functions
that are not Fortran intrnisics.

As I said I agree with the fact that the Fortran FE needs to annotate
the middle-end builtins. That means we are talking about a way
to declare/amend the C ABI math functions with additional attributes.

This means that technically using

intrinsic :: ...

isn't correct. That leaves us with the choice of adding a custom
"pragma" where we can of course choose the syntax freely.
I guess

!GCC$ ...

at the start of a translation unit doesn't affect any behavior you
quote below.

Whether the proposed syntax needs to mimic fortran syntax
remains a question but a simplistic

!GCC$ builtin "sinf" attributes omp-simd-notinbranch

would work up to the point where when parsing this the FE needs
to lookup the corresponding builtin for "sinf". math-builtins.def
seems to only contain the unsuffixed names and we likely build
variants with f and l somewhere during processing. But eventually
we should be able to process math-builtins.def for this lookup
(and ignore ones that are not listed given the user would have to
declare a fortran function with appropriate C binding himself).

Note that the sincos vectorized variant from glibc is useless
(it uses the wrong ABI)

Post by Thomas Koenig
Second, if you include any additional lines from an external
file, they have to be syntactially valid outside any translation
unit, or you will run into the exact problem that you

Post by Martin Liška
Does it mean that the '#include "vector-math.f90"' should be put before
a program directive?

The problem is that you can start a valid program without a PROGRAM
statement, so
intrnisic :: sin
end
is a valid program, and for
intrinsic :: sin
program foo
the compiler correctly complains. So, you need to take out the
need for the intrinsic:: sin line.
(Also, the user is free to declare a variable sin, although it
is not advisable).

Post by Martin Liška

Post by Thomas Koenig
So, you could use either the intrinsic name with the precision or (maybe
that is an even better idea) the name of the C function that is
overloaded.

I've got it. It's implemented in attached patch.

It's not quite clear to me Which of the two options you mean :-)
Regards
Thomas

[Martin Liška]

Post by Richard Biener

Post by Thomas Koenig

Post by Martin Liška
As Richi mentioned, the list of middle-end built-ins does not scale well.

It is not clear to me what that means. Could you elaborate?
At the moment, this would still be my preferred solution.
(If it is a problem of too-long lines: Repeated uses of the
option with different names should just work, or can
be made to work).

Post by Martin Liška
So the way discussed in this sub-thread looks preferred.

I have given this some more thought.
First, any solution should include sincos and other functions
that are not Fortran intrnisics.

As I said I agree with the fact that the Fortran FE needs to annotate
the middle-end builtins. That means we are talking about a way
to declare/amend the C ABI math functions with additional attributes.
This means that technically using
intrinsic :: ...
isn't correct. That leaves us with the choice of adding a custom
"pragma" where we can of course choose the syntax freely.
I guess
!GCC$ ...
at the start of a translation unit doesn't affect any behavior you
quote below.
Whether the proposed syntax needs to mimic fortran syntax
remains a question but a simplistic
!GCC$ builtin "sinf" attributes omp-simd-notinbranch
would work up to the point where when parsing this the FE needs
to lookup the corresponding builtin for "sinf". math-builtins.def
seems to only contain the unsuffixed names and we likely build
variants with f and l somewhere during processing. But eventually
we should be able to process math-builtins.def for this lookup
(and ignore ones that are not listed given the user would have to
declare a fortran function with appropriate C binding himself).
Note that the sincos vectorized variant from glibc is useless
(it uses the wrong ABI)

Post by Thomas Koenig
Second, if you include any additional lines from an external
file, they have to be syntactially valid outside any translation
unit, or you will run into the exact problem that you

Post by Martin Liška
Does it mean that the '#include "vector-math.f90"' should be put before
a program directive?

The problem is that you can start a valid program without a PROGRAM
statement, so
intrnisic :: sin
end
is a valid program, and for
intrinsic :: sin
program foo
the compiler correctly complains. So, you need to take out the
need for the intrinsic:: sin line.
(Also, the user is free to declare a variable sin, although it
is not advisable).

Post by Martin Liška

Post by Thomas Koenig
So, you could use either the intrinsic name with the precision or (maybe
that is an even better idea) the name of the C function that is
overloaded.

I've got it. It's implemented in attached patch.

It's not quite clear to me Which of the two options you mean :-)
Regards
Thomas

Hi.

I'm attaching patch draft that can leverage defined syntax:

$ cat vector-math.f90
!GCC$ builtin sinf attributes omp_simd_notinbranch
!GCC$ builtin cosf attributes omp_simd_notinbranch

$ cat usage.F90
#include "vector-math.f90"
program test_overloaded_intrinsic
real(4) :: x4(3200), y4(3200)
real(8) :: x8(3200), y8(3200)

! this should be using simd clone
y4 = sin(x4)
print *, y4

! this should not be using simd clone
y4 = sin(x8)
print *, y8
end

$ ./xgcc -B. ~/Programming/testcases/usage.F90 -c -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
vect__3.14_11 = sinf.simdclone.0 (vect__2.13_61);
_6 = __builtin_sin (_5);

The question still remains which way use to provide automatic inclusion of the "header" file?

Martin

[Richard Biener]

Post by Martin Liška

Post by Richard Biener

Post by Thomas Koenig

Post by Martin Liška
As Richi mentioned, the list of middle-end built-ins does not scale well.

It is not clear to me what that means. Could you elaborate?
At the moment, this would still be my preferred solution.
(If it is a problem of too-long lines: Repeated uses of the
option with different names should just work, or can
be made to work).

Post by Martin Liška
So the way discussed in this sub-thread looks preferred.

I have given this some more thought.
First, any solution should include sincos and other functions
that are not Fortran intrnisics.

As I said I agree with the fact that the Fortran FE needs to annotate
the middle-end builtins. That means we are talking about a way
to declare/amend the C ABI math functions with additional attributes.
This means that technically using
intrinsic :: ...
isn't correct. That leaves us with the choice of adding a custom
"pragma" where we can of course choose the syntax freely.
I guess
!GCC$ ...
at the start of a translation unit doesn't affect any behavior you
quote below.
Whether the proposed syntax needs to mimic fortran syntax
remains a question but a simplistic
!GCC$ builtin "sinf" attributes omp-simd-notinbranch
would work up to the point where when parsing this the FE needs
to lookup the corresponding builtin for "sinf". math-builtins.def
seems to only contain the unsuffixed names and we likely build
variants with f and l somewhere during processing. But eventually
we should be able to process math-builtins.def for this lookup
(and ignore ones that are not listed given the user would have to
declare a fortran function with appropriate C binding himself).
Note that the sincos vectorized variant from glibc is useless
(it uses the wrong ABI)

Post by Thomas Koenig
Second, if you include any additional lines from an external
file, they have to be syntactially valid outside any translation
unit, or you will run into the exact problem that you

Post by Martin Liška
Does it mean that the '#include "vector-math.f90"' should be put before
a program directive?

The problem is that you can start a valid program without a PROGRAM
statement, so
intrnisic :: sin
end
is a valid program, and for
intrinsic :: sin
program foo
the compiler correctly complains. So, you need to take out the
need for the intrinsic:: sin line.
(Also, the user is free to declare a variable sin, although it
is not advisable).

Post by Martin Liška

Post by Thomas Koenig
So, you could use either the intrinsic name with the precision or (maybe
that is an even better idea) the name of the C function that is
overloaded.

I've got it. It's implemented in attached patch.

It's not quite clear to me Which of the two options you mean :-)
Regards
Thomas

Hi.
$ cat vector-math.f90
!GCC$ builtin sinf attributes omp_simd_notinbranch
!GCC$ builtin cosf attributes omp_simd_notinbranch
$ cat usage.F90
#include "vector-math.f90"
program test_overloaded_intrinsic
real(4) :: x4(3200), y4(3200)
real(8) :: x8(3200), y8(3200)
! this should be using simd clone
y4 = sin(x4)
print *, y4
! this should not be using simd clone
y4 = sin(x8)
print *, y8
end
$ ./xgcc -B. ~/Programming/testcases/usage.F90 -c -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
vect__3.14_11 = sinf.simdclone.0 (vect__2.13_61);
_6 = __builtin_sin (_5);
The question still remains which way use to provide automatic inclusion of the "header" file?

I'd say go the C family way - add a target hook telling you if and
what to pre-include.

Richard.

Post by Martin Liška
Martin

[Martin Liška]

Post by Richard Biener

Post by Martin Liška

Post by Richard Biener

Post by Thomas Koenig

Post by Martin Liška
As Richi mentioned, the list of middle-end built-ins does not scale well.

It is not clear to me what that means. Could you elaborate?
At the moment, this would still be my preferred solution.
(If it is a problem of too-long lines: Repeated uses of the
option with different names should just work, or can
be made to work).

Post by Martin Liška
So the way discussed in this sub-thread looks preferred.

I have given this some more thought.
First, any solution should include sincos and other functions
that are not Fortran intrnisics.

As I said I agree with the fact that the Fortran FE needs to annotate
the middle-end builtins. That means we are talking about a way
to declare/amend the C ABI math functions with additional attributes.
This means that technically using
intrinsic :: ...
isn't correct. That leaves us with the choice of adding a custom
"pragma" where we can of course choose the syntax freely.
I guess
!GCC$ ...
at the start of a translation unit doesn't affect any behavior you
quote below.
Whether the proposed syntax needs to mimic fortran syntax
remains a question but a simplistic
!GCC$ builtin "sinf" attributes omp-simd-notinbranch
would work up to the point where when parsing this the FE needs
to lookup the corresponding builtin for "sinf". math-builtins.def
seems to only contain the unsuffixed names and we likely build
variants with f and l somewhere during processing. But eventually
we should be able to process math-builtins.def for this lookup
(and ignore ones that are not listed given the user would have to
declare a fortran function with appropriate C binding himself).
Note that the sincos vectorized variant from glibc is useless
(it uses the wrong ABI)

Post by Thomas Koenig
Second, if you include any additional lines from an external
file, they have to be syntactially valid outside any translation
unit, or you will run into the exact problem that you

Post by Martin Liška
Does it mean that the '#include "vector-math.f90"' should be put before
a program directive?

The problem is that you can start a valid program without a PROGRAM
statement, so
intrnisic :: sin
end
is a valid program, and for
intrinsic :: sin
program foo
the compiler correctly complains. So, you need to take out the
need for the intrinsic:: sin line.
(Also, the user is free to declare a variable sin, although it
is not advisable).

Post by Martin Liška

Post by Thomas Koenig
So, you could use either the intrinsic name with the precision or (maybe
that is an even better idea) the name of the C function that is
overloaded.

I've got it. It's implemented in attached patch.

It's not quite clear to me Which of the two options you mean :-)
Regards
Thomas

Hi.
$ cat vector-math.f90
!GCC$ builtin sinf attributes omp_simd_notinbranch
!GCC$ builtin cosf attributes omp_simd_notinbranch
$ cat usage.F90
#include "vector-math.f90"
program test_overloaded_intrinsic
real(4) :: x4(3200), y4(3200)
real(8) :: x8(3200), y8(3200)
! this should be using simd clone
y4 = sin(x4)
print *, y4
! this should not be using simd clone
y4 = sin(x8)
print *, y8
end
$ ./xgcc -B. ~/Programming/testcases/usage.F90 -c -Ofast -fdump-tree-optimized=/dev/stdout | grep sin
vect__3.14_11 = sinf.simdclone.0 (vect__2.13_61);
_6 = __builtin_sin (_5);
The question still remains which way use to provide automatic inclusion of the "header" file?

I'd say go the C family way - add a target hook telling you if and
what to pre-include.

Good, you already mentioned that in previous emails.

I'm sending another version of the patch that implements that. I know we are quite
close to the end of stage1, but I would be very happy to have it included in GCC 9.1.
Is it doable?

Thanks,
Martin

Post by Richard Biener
Richard.

Post by Martin Liška
Martin

[Thomas Koenig]

Hi Martin,

Post by Martin Liška
I'm sending another version of the patch that implements that. I know we are quite
close to the end of stage1, but I would be very happy to have it included in GCC 9.1.
Is it doable?

First, I thinkt he concept is fine.

If possible, I would prefer a different name - this is really an include
fine, not a free-form Fortran file. There is no common convention
what to call Fortran include files, but *.inc seems to be common,
and we are also using *.h for other stuff. So maybe *.h would
be best.

With your patch, exactly where would the file in question need
to be located in order to be found? I suppose under the
finclude directory might be a good idea.

Finally, how do we generate the file to be included? My
suggestion would be to use something like

$ cat tst.c
#include <math.h>
$ cat c2f.pl
#! /usr/bin/perl

while (<>) {
next unless /notinbranch/;
@arr = split(';');
foreach $a (@arr) {
next unless $a =~ /notinbranch/;
$a =~ s/__attribute__ \(\(.*?\)\)*//g;
$a =~ s/\(.*\)//;
@a = split(" ",$a);
print '!GCC$ builtin ',$a[2]," attributes omp_simd_notinbranch\n";
}
}
$ gcc -E -Ofast tst.c | ./c2f.pl
!GCC$ builtin cos attributes omp_simd_notinbranch
!GCC$ builtin sin attributes omp_simd_notinbranch
!GCC$ builtin exp attributes omp_simd_notinbranch
!GCC$ builtin log attributes omp_simd_notinbranch
!GCC$ builtin pow attributes omp_simd_notinbranch
!GCC$ builtin cosf attributes omp_simd_notinbranch
!GCC$ builtin sinf attributes omp_simd_notinbranch
!GCC$ builtin expf attributes omp_simd_notinbranch
!GCC$ builtin logf attributes omp_simd_notinbranch
!GCC$ builtin powf attributes omp_simd_notinbranch

but I am not sure how (or if) this would work with
cross-compilation or if it is stable enough.

Regards

Thomas

[Martin Liška]

Post by Thomas Koenig
Hi Martin,

Post by Martin Liška
I'm sending another version of the patch that implements that. I know we are quite
close to the end of stage1, but I would be very happy to have it included in GCC 9.1.
Is it doable?

First, I thinkt he concept is fine.

Hi.

Thanks.

Post by Thomas Koenig
If possible, I would prefer a different name - this is really an include
fine, not a free-form Fortran file.  There is no common convention
what to call Fortran include files, but *.inc seems to be common,
and we are also using *.h for other stuff.  So maybe *.h would
be best.

Works for me, then I'm suggesting following name: math-vector-fortran.h.

Post by Thomas Koenig
With your patch, exactly where would the file in question need
to be located in order to be found?  I suppose under the
finclude directory might be a good idea.

Well, I would generally copy what we do for stdc-predef.h. The file
lives in glibc repository (./include/stdc-predef.h) and is then
distributed via a glibc-devel package. Then the file is installed
into /usr/include/stdc-predef.h. So a standard include directory.
We would also need to come up with a default empty file for non-x86_64
targets.

Post by Thomas Koenig
Finally, how do we generate the file to be included?  My
suggestion would be to use something like
$ cat tst.c
#include <math.h>
$ cat c2f.pl
#! /usr/bin/perl
while (<>) {
    next unless /notinbranch/;
        next unless $a =~ /notinbranch/;
        $a =~ s/__attribute__ \(\(.*?\)\)*//g;
        $a =~ s/\(.*\)//;
        print '!GCC$ builtin ',$a[2]," attributes omp_simd_notinbranch\n";
    }
}
$ gcc -E -Ofast tst.c | ./c2f.pl
!GCC$ builtin cos attributes omp_simd_notinbranch
!GCC$ builtin sin attributes omp_simd_notinbranch
!GCC$ builtin exp attributes omp_simd_notinbranch
!GCC$ builtin log attributes omp_simd_notinbranch
!GCC$ builtin pow attributes omp_simd_notinbranch
!GCC$ builtin cosf attributes omp_simd_notinbranch
!GCC$ builtin sinf attributes omp_simd_notinbranch
!GCC$ builtin expf attributes omp_simd_notinbranch
!GCC$ builtin logf attributes omp_simd_notinbranch
!GCC$ builtin powf attributes omp_simd_notinbranch

Works for me, glibc has already some perl scripts, so that can be hopefully
included.

Post by Thomas Koenig
but I am not sure how (or if) this would work with
cross-compilation or if it is stable enough.

If I'm correct that should be resolved by inclusion of corresponding
cross header file for math-vector-fortran.h.

Martin

Post by Thomas Koenig
Regards
    Thomas

[Toon Moene]

Post by Martin Liška
I'm sending another version of the patch that implements that. I know we are quite
close to the end of stage1, but I would be very happy to have it included in GCC 9.1.
Is it doable?

In the past we got some extra leeway from the Release Managers for issue
entirely contained inside the Fortran Front End and run time library.

So I hope they will give you some extra time.

Kind regards,

[Bernhard Reutner-Fischer]

Post by Martin Liška
!GCC$ builtin sinf attributes omp_simd_notinbranch

I would expect "attribute" singular, like in C.

I'm not opposed to your approach, but did we consider using -include glibc-simd-math.h via spec if lets say -O3 or  -mveclibabi=glibc ?

+ /* Make a guard to prevent recursive inclusion. */
+ static bool preinclude_done = false;
+ const char *preinc = targetcm.c_preinclude ();

Hm. Why don't you call the target hook only if !preinclude_done?

+ if (!preinclude_done && preinc != NULL)
+ {
+ preinclude_done = true;
+ if (!load_file (preinc, NULL, false))
+ exit (FATAL_EXIT_CODE);
+ }
+

thanks,

[Martin Liška]

Post by Bernhard Reutner-Fischer

Post by Martin Liška
!GCC$ builtin sinf attributes omp_simd_notinbranch

Hello.

Post by Bernhard Reutner-Fischer
I would expect "attribute" singular, like in C.

Plural form is more aligned with e.g. '!GCC$ attributes NO_ARG_CHECK :: buf' ?

Post by Bernhard Reutner-Fischer
I'm not opposed to your approach, but did we consider using -include glibc-simd-math.h via spec if lets say -O3 or  -mveclibabi=glibc ?

Richi?

Post by Bernhard Reutner-Fischer
+ /* Make a guard to prevent recursive inclusion. */
+ static bool preinclude_done = false;
+ const char *preinc = targetcm.c_preinclude ();
Hm. Why don't you call the target hook only if !preinclude_done?

Because otherwise I would see following error:
math-vector-fortran.h:1: Error: File 'math-vector-fortran.h' is being included recursively

Thanks,
Martin

Post by Bernhard Reutner-Fischer
+ if (!preinclude_done && preinc != NULL)
+ {
+ preinclude_done = true;
+ if (!load_file (preinc, NULL, false))
+ exit (FATAL_EXIT_CODE);
+ }
+
thanks,

[Richard Biener]

Post by Martin Liška

Post by Bernhard Reutner-Fischer

Post by Martin Liška
!GCC$ builtin sinf attributes omp_simd_notinbranch

Hello.

Post by Bernhard Reutner-Fischer
I would expect "attribute" singular, like in C.

Plural form is more aligned with e.g. '!GCC$ attributes NO_ARG_CHECK :: buf' ?

Post by Bernhard Reutner-Fischer
I'm not opposed to your approach, but did we consider using -include glibc-simd-math.h via spec if lets say -O3 or -mveclibabi=glibc ?

Richi?

Using specs was the original idea.

You need to add -include support to the Fortran FE for that, of
course. And you need to
add a default spec fragment somewhere given we want to include the
glibc variant by
default for glibc targets. And you need to allow the file to be not
present (easy to check
with the target hook...), either by making -include not error for not
existing ones or
by making spec processing add -include X only when X exists [in the
finclude path].
So in the end the target hook way seemed easier...

Yes, if we want to support -mveclibabi=foobar and then automatically include
foobar-vec.h if present that looks like specs processing is a better
fit for the job.
But I'm not sure if we should optimize for that scenario.

Richard.

Post by Martin Liška

Post by Bernhard Reutner-Fischer
+ /* Make a guard to prevent recursive inclusion. */
+ static bool preinclude_done = false;
+ const char *preinc = targetcm.c_preinclude ();
Hm. Why don't you call the target hook only if !preinclude_done?

math-vector-fortran.h:1: Error: File 'math-vector-fortran.h' is being included recursively
Thanks,
Martin

Post by Bernhard Reutner-Fischer
+ if (!preinclude_done && preinc != NULL)
+ {
+ preinclude_done = true;
+ if (!load_file (preinc, NULL, false))
+ exit (FATAL_EXIT_CODE);
+ }
+
thanks,

[Bernhard Reutner-Fischer]

On Mon, 12 Nov 2018 15:51:07 +0100

Post by Richard Biener

Post by Martin Liška

Post by Bernhard Reutner-Fischer

Post by Martin Liška
!GCC$ builtin sinf attributes omp_simd_notinbranch

Hello.

Post by Bernhard Reutner-Fischer
I would expect "attribute" singular, like in C.

Plural form is more aligned with e.g. '!GCC$ attributes NO_ARG_CHECK :: buf' ?

oh, ok then.

Post by Richard Biener

Post by Martin Liška

Post by Bernhard Reutner-Fischer
I'm not opposed to your approach, but did we consider using -include glibc-simd-math.h via spec if lets say -O3 or -mveclibabi=glibc ?

Richi?

Using specs was the original idea.
You need to add -include support to the Fortran FE for that, of
course. And you need to
add a default spec fragment somewhere given we want to include the
glibc variant by
default for glibc targets. And you need to allow the file to be not

... by default *when optimizing* i suppose. Or do you envision to do
this even at -O0 ?

Post by Richard Biener
present (easy to check
with the target hook...), either by making -include not error for not
existing ones or
by making spec processing add -include X only when X exists [in the
finclude path].
So in the end the target hook way seemed easier...
Yes, if we want to support -mveclibabi=foobar and then automatically include
foobar-vec.h if present that looks like specs processing is a better
fit for the job.
But I'm not sure if we should optimize for that scenario.

Ok, fair enough.

Post by Richard Biener

Post by Martin Liška

Post by Bernhard Reutner-Fischer
+ /* Make a guard to prevent recursive inclusion. */
+ static bool preinclude_done = false;
+ const char *preinc = targetcm.c_preinclude ();
Hm. Why don't you call the target hook only if !preinclude_done?

math-vector-fortran.h:1: Error: File 'math-vector-fortran.h' is being included recursively

How so?
Why would you need to call the target macro even if preinclude_done was
true like in your current patch?
Why isn't it sufficient to

static bool preinclude_done = false;
if (!preinclude_done)
{
const char *preinc = targetcm.c_preinclude ();
preinclude_done = true;
if (preinc != NULL && !load_file (preinc, NULL, false))
exit (FATAL_EXIT_CODE);
}

i.e. only call the target macro if !preinclude_done ?

AFAIU your current code would call the target macro needlessly for
every file included from the main_input_filename but not attempt to do
anything with it?

[btw, brig_init could use main_input_basename instead of manually
redoing the conversion from main_input_filename to main_input_basename,
it seems]

thanks,

Post by Richard Biener

Post by Martin Liška

Post by Bernhard Reutner-Fischer
+ if (!preinclude_done && preinc != NULL)
+ {
+ preinclude_done = true;
+ if (!load_file (preinc, NULL, false))
+ exit (FATAL_EXIT_CODE);
+ }
+
thanks,

[Richard Biener]

On Mon, Nov 12, 2018 at 4:33 PM Bernhard Reutner-Fischer

Post by Bernhard Reutner-Fischer
On Mon, 12 Nov 2018 15:51:07 +0100

Post by Richard Biener

Post by Martin Liška

Post by Bernhard Reutner-Fischer

Post by Martin Liška
!GCC$ builtin sinf attributes omp_simd_notinbranch

Hello.

Post by Bernhard Reutner-Fischer
I would expect "attribute" singular, like in C.

Plural form is more aligned with e.g. '!GCC$ attributes NO_ARG_CHECK :: buf' ?

oh, ok then.

Post by Richard Biener

Post by Martin Liška

Post by Bernhard Reutner-Fischer
I'm not opposed to your approach, but did we consider using -include glibc-simd-math.h via spec if lets say -O3 or -mveclibabi=glibc ?

Richi?

Using specs was the original idea.
You need to add -include support to the Fortran FE for that, of
course. And you need to
add a default spec fragment somewhere given we want to include the
glibc variant by
default for glibc targets. And you need to allow the file to be not

... by default *when optimizing* i suppose. Or do you envision to do
this even at -O0 ?

It shouldn't make any difference there (extra parsing time I guess).

Post by Bernhard Reutner-Fischer

Post by Richard Biener
present (easy to check
with the target hook...), either by making -include not error for not
existing ones or
by making spec processing add -include X only when X exists [in the
finclude path].
So in the end the target hook way seemed easier...
Yes, if we want to support -mveclibabi=foobar and then automatically include
foobar-vec.h if present that looks like specs processing is a better
fit for the job.
But I'm not sure if we should optimize for that scenario.

Ok, fair enough.

Post by Richard Biener

Post by Martin Liška

Post by Bernhard Reutner-Fischer
+ /* Make a guard to prevent recursive inclusion. */
+ static bool preinclude_done = false;
+ const char *preinc = targetcm.c_preinclude ();
Hm. Why don't you call the target hook only if !preinclude_done?

math-vector-fortran.h:1: Error: File 'math-vector-fortran.h' is being included recursively

How so?
Why would you need to call the target macro even if preinclude_done was
true like in your current patch?
Why isn't it sufficient to
static bool preinclude_done = false;
if (!preinclude_done)
{
const char *preinc = targetcm.c_preinclude ();
preinclude_done = true;
if (preinc != NULL && !load_file (preinc, NULL, false))
exit (FATAL_EXIT_CODE);
}
i.e. only call the target macro if !preinclude_done ?
AFAIU your current code would call the target macro needlessly for
every file included from the main_input_filename but not attempt to do
anything with it?
[btw, brig_init could use main_input_basename instead of manually
redoing the conversion from main_input_filename to main_input_basename,
it seems]
thanks,

Post by Richard Biener

Post by Martin Liška

Post by Bernhard Reutner-Fischer
+ if (!preinclude_done && preinc != NULL)
+ {
+ preinclude_done = true;
+ if (!load_file (preinc, NULL, false))
+ exit (FATAL_EXIT_CODE);
+ }
+
thanks,

[Martin Liška]

Hi.

I'm CCing gcc-patches ML and Jakub, who significantly helped me yesterday. The
idea now is to have a driver search for the math header file. When it exists,
Fortran FE loads the file (via new -fpre-include option).

I can confirm that e.g. roms_r CPU2017 benchmark utilizes couple of simd clones:

$ grep simd *.optimized
analytical.fppized.f90.229t.optimized: vect__77.555_90 = exp.simdclone.12 (vect__76.554_94);
analytical.fppized.f90.229t.optimized: vect__322.866_1061 = sin.simdclone.4 (vect__321.865_1062);
bulk_flux.fppized.f90.229t.optimized: vect__506.784_1887 = pow.simdclone.2 (vect__1057.783_1889, { 1.939999999999999946709294817992486059665679931640625e+0, 1.939999999999999946709294817992486059665679931640625e+0 });
gasdev.fppized.f90.229t.optimized: vect__66.139_93 = log.simdclone.0 (vect__65.138_94);
lmd_swfrac.fppized.f90.229t.optimized: vect__84.60_182 = exp.simdclone.0 (vect__83.59_183);
lmd_swfrac.fppized.f90.229t.optimized: vect__89.69_161 = exp.simdclone.0 (vect__88.68_162);
***@marxinbox:~/Programming/cpu2017/benchspec/CPU/554.roms_r/build/build_peak_gcc7-m64.0000> grep simd *.optimized
analytical.fppized.f90.229t.optimized: vect__77.555_90 = exp.simdclone.12 (vect__76.554_94);
analytical.fppized.f90.229t.optimized: vect__322.866_1061 = sin.simdclone.4 (vect__321.865_1062);
bulk_flux.fppized.f90.229t.optimized: vect__506.784_1887 = pow.simdclone.2 (vect__1057.783_1889, { 1.939999999999999946709294817992486059665679931640625e+0, 1.939999999999999946709294817992486059665679931640625e+0 });
gasdev.fppized.f90.229t.optimized: vect__66.139_93 = log.simdclone.0 (vect__65.138_94);
lmd_swfrac.fppized.f90.229t.optimized: vect__84.60_182 = exp.simdclone.0 (vect__83.59_183);
lmd_swfrac.fppized.f90.229t.optimized: vect__89.69_161 = exp.simdclone.0 (vect__88.68_162);

and it also finishes successfully.

Question I have is about default search locations for the header file. On my machine I can
see:
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../x86_64-pc-linux-gnu/lib/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../x86_64-pc-linux-gnu/lib/../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/lib/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/lib/../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/usr/lib/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/usr/lib/../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../x86_64-pc-linux-gnu/lib/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/lib/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/usr/lib/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)

Aren't these locations desired for libraries, instead of include locations?

Thoughts?
Thanks,
Martin

[Jakub Jelinek]

Post by Martin Liška
Question I have is about default search locations for the header file. On my machine I can
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../x86_64-pc-linux-gnu/lib/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../x86_64-pc-linux-gnu/lib/../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/lib/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/lib/../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/usr/lib/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/usr/lib/../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../x86_64-pc-linux-gnu/lib/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/lib/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/usr/lib/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
Aren't these locations desired for libraries, instead of include locations?

That isn't correct indeed.
What about find_a_file (&include_prefixes, ... )?
Though, in the design where to put the file we really need to have multilib
(and multiarch on Debian/Ubuntu) in mind, because e.g. on x86_64-linux you
want to find a m64 version of the header for -m64, but a different for -m32
and there is always the possibility somebody installs a 32-bit gfortran on x86_64.

Post by Martin Liška
--- a/gcc/config/gnu-user.h
+++ b/gcc/config/gnu-user.h
@@ -170,3 +170,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
LD_STATIC_OPTION " --whole-archive -llsan --no-whole-archive " \
LD_DYNAMIC_OPTION "}}%{!static-liblsan:-llsan}"
#endif
+
+#undef TARGET_F951_NOSTDINC_OPTIONS
+#define TARGET_F951_NOSTDINC_OPTIONS "%:fortran-header-file(-fpre-include= math-vector-fortran.h)"

Too long line, use some \s to split it up.

Post by Martin Liška
+ - omp-simd-notinbranch.

So omp-simd-notinbranch or omp_simd_notinbranch?
Any particular reason for this weird syntax and for not also
supporting inbranch or just simd?

Post by Martin Liška
+
+ When we come here, we have already matched the !GCC$ builtin string. */
+match
+gfc_match_gcc_builtin (void)
+{
+ char builtin[GFC_MAX_SYMBOL_LEN + 1];
+
+ if (gfc_match_name (builtin) != MATCH_YES)
+ return MATCH_ERROR;
+
+ gfc_gobble_whitespace ();
+ if (gfc_match ("attributes") != MATCH_YES)
+ return MATCH_ERROR;
+
+ gfc_gobble_whitespace ();
+ if (gfc_match ("omp_simd_notinbranch") != MATCH_YES)
+ return MATCH_ERROR;

Why so many gfc_match calls? Can't you e.g. just do
if (gfc_match ("%n attributes simd", builtin) != MATCH_YES)
return MATCH_ERROR;

int builtin_kind = 0; /* Or whatever, just want to show the parsing here. */
if (gfc_match ("(notinbranch)") == MATCH_YES)
builtin_kind = -1;
else if (gfc_match ("(inbranch)") == MATCH_YES)
builtin_kind = 1;

The space in gfc_match indicates gfc_gobble_whitespace (), i.e. optionally
eating whitespace (note, in fixed form white space is optionally eaten
pretty much always). If you want a mandatory space, there is "% ".
So it depends in if in fixed form we want to support e.g.
!GCC$ BUI L TIN SINf ATTRI BUT ESSIMD(NOT IN BRANCH)
and in free form e.g.
!gcc$ builtin sinf attributessimd (notinbranch)
I wouldn't use omp_simd because in C/C++ the attribute is called simd.

Post by Martin Liška
+
+ char *r = XNEWVEC (char, strlen (builtin) + 32);
+ sprintf (r, "__builtin_%s", builtin);
+ vectorized_builtins.safe_push (r);

Perhaps make it vector of const char *, int pairs, so that you can
encode no argument (aka inbranch + notinbranch) vs. inbranch vs. notinbranch?

Post by Martin Liška
#define F951_OPTIONS "%(cc1_options) %{J*} \
- %{!nostdinc:-fintrinsic-modules-path finclude%s}\
+ %{!nostdinc:-fintrinsic-modules-path finclude%s " \
+ TARGET_F951_NOSTDINC_OPTIONS "}\

I wouldn't stick it inside of %{!nostdinc:, but let config/gnu-user.h decide
about that (i.e. wrap it into %{!nostdinc: ... } there).

Post by Martin Liška
--- a/gcc/fortran/lang.opt
+++ b/gcc/fortran/lang.opt
@@ -662,6 +662,10 @@ fprotect-parens
Fortran Var(flag_protect_parens) Init(-1)
Protect parentheses in expressions.
+fpre-include=
+Fortran RejectNegative JoinedOrMissing Var(flag_pre_include) Undocumented
+Path to header file that should be pre-included before each compilation unit.

Why OrMissing? If the argument is missing, that should be an error, you
can't load "".

Post by Martin Liška
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -2365,6 +2365,16 @@ load_file (const char *realfilename, const char *displayedname, bool initial)
}
}
+ /* Make a guard to prevent recursive inclusion. */
+ static bool preinclude_done = false;
+ if (!preinclude_done)
+ {
+ preinclude_done = true;
+ if (flag_pre_include != NULL && !load_file (flag_pre_include, NULL,
+ false))
+ exit (FATAL_EXIT_CODE);
+ }

Why in load_file? I'd handle this in gfc_new_file, where it is called just
once. Formatting - would be nicer to put && on the next line and not wrap
load_file args.

Post by Martin Liška
+static const char *
+find_fortran_header_file (int argc, const char **argv)
+{
+ if (argc != 2)
+ return NULL;
+
+ const char *path = find_file (argv[1]);
+ if (path != argv[1])
+ return concat (argv[0], path, NULL);

I wouldn't call it fortran_header_file but fortran_preinclude_file
(both in what appears in spec and the name of the callback).

Plus, the patch lacks testcases. I'd use dg-options "-nostdinc"
so that in that testcase it isn't preincluded, and have one free form and
one fixed form testcase that has various forms of the !gcc$ builtin
for a couple of builtins + some calls to them and check how they are handled
(perhaps limited to the one or two ABIs that support those)?

Jakub

[Jakub Jelinek]

Post by Jakub Jelinek

Post by Martin Liška
+
+ When we come here, we have already matched the !GCC$ builtin string. */
+match
+gfc_match_gcc_builtin (void)
+{
+ char builtin[GFC_MAX_SYMBOL_LEN + 1];
+
+ if (gfc_match_name (builtin) != MATCH_YES)
+ return MATCH_ERROR;
+
+ gfc_gobble_whitespace ();
+ if (gfc_match ("attributes") != MATCH_YES)
+ return MATCH_ERROR;
+
+ gfc_gobble_whitespace ();
+ if (gfc_match ("omp_simd_notinbranch") != MATCH_YES)
+ return MATCH_ERROR;

Why so many gfc_match calls? Can't you e.g. just do
if (gfc_match ("%n attributes simd", builtin) != MATCH_YES)
return MATCH_ERROR;
int builtin_kind = 0; /* Or whatever, just want to show the parsing here. */
if (gfc_match ("(notinbranch)") == MATCH_YES)
builtin_kind = -1;
else if (gfc_match ("(inbranch)") == MATCH_YES)
builtin_kind = 1;
The space in gfc_match indicates gfc_gobble_whitespace (), i.e. optionally
eating whitespace (note, in fixed form white space is optionally eaten
pretty much always). If you want a mandatory space, there is "% ".
So it depends in if in fixed form we want to support e.g.
!GCC$ BUI L TIN SINf ATTRI BUT ESSIMD(NOT IN BRANCH)
and in free form e.g.
!gcc$ builtin sinf attributessimd (notinbranch)
I wouldn't use omp_simd because in C/C++ the attribute is called simd.

One more thing, for fixed form spaces are insignificant generally,
gfc_next_char has:
do
{
c = gfc_next_char_literal (NONSTRING);
}
while (gfc_current_form == FORM_FIXED && gfc_is_whitespace (c));
so a gfc_name in fixed form needs to be separated by something different
from whitespace I'd say. Because in
!gcc$ builtin sinf attribute simd
or
!GCC$ BUILTINSINFATTRIBUTESIMD
you'd get in both cases sinfattributesimd as the builtin's name and nothing
afterwards (so gfc_match would fail).
One possibility is
if (gfc_match ("(%n) attributes simd", name) == MATCH_YES)
where you'd require parens around the builtin's name. Or it could go last
in the syntax, i.e.
!gcc$ builtin attributes simd sinf
!gcc$ builtin attributes simd (notinbranch) sinf
But I think ()s are better.

Jakub

[Jakub Jelinek]

Post by Jakub Jelinek
One possibility is
if (gfc_match ("(%n) attributes simd", name) == MATCH_YES)

" (%n) attributes simd" to be precise, so that whitespace is allowed
in free form between builtin and (. And the following parsing might be
" ( notinbranch )"
etc.

Jakub

[Martin Liška]

Post by Jakub Jelinek

Post by Martin Liška
Question I have is about default search locations for the header file. On my machine I can
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../x86_64-pc-linux-gnu/lib/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../x86_64-pc-linux-gnu/lib/../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/lib/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/lib/../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/usr/lib/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/usr/lib/../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../x86_64-pc-linux-gnu/lib/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/lib/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/usr/lib/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
Aren't these locations desired for libraries, instead of include locations?

That isn't correct indeed.
What about find_a_file (&include_prefixes, ... )?

Thanks, so setting last argument to true should handle here the multilib support.

Post by Jakub Jelinek
Though, in the design where to put the file we really need to have multilib
(and multiarch on Debian/Ubuntu) in mind, because e.g. on x86_64-linux you
want to find a m64 version of the header for -m64, but a different for -m32
and there is always the possibility somebody installs a 32-bit gfortran on x86_64.

Post by Martin Liška
--- a/gcc/config/gnu-user.h
+++ b/gcc/config/gnu-user.h
@@ -170,3 +170,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
LD_STATIC_OPTION " --whole-archive -llsan --no-whole-archive " \
LD_DYNAMIC_OPTION "}}%{!static-liblsan:-llsan}"
#endif
+
+#undef TARGET_F951_NOSTDINC_OPTIONS
+#define TARGET_F951_NOSTDINC_OPTIONS "%:fortran-header-file(-fpre-include= math-vector-fortran.h)"

Too long line, use some \s to split it up.

Post by Martin Liška
+ - omp-simd-notinbranch.

So omp-simd-notinbranch or omp_simd_notinbranch?
Any particular reason for this weird syntax and for not also
supporting inbranch or just simd?

Questionable whether to support as current glibc vector ABI only uses notinbranch?

Post by Jakub Jelinek

Post by Martin Liška
+
+ When we come here, we have already matched the !GCC$ builtin string. */
+match
+gfc_match_gcc_builtin (void)
+{
+ char builtin[GFC_MAX_SYMBOL_LEN + 1];
+
+ if (gfc_match_name (builtin) != MATCH_YES)
+ return MATCH_ERROR;
+
+ gfc_gobble_whitespace ();
+ if (gfc_match ("attributes") != MATCH_YES)
+ return MATCH_ERROR;
+
+ gfc_gobble_whitespace ();
+ if (gfc_match ("omp_simd_notinbranch") != MATCH_YES)
+ return MATCH_ERROR;

Why so many gfc_match calls? Can't you e.g. just do
if (gfc_match ("%n attributes simd", builtin) != MATCH_YES)
return MATCH_ERROR;
int builtin_kind = 0; /* Or whatever, just want to show the parsing here. */
if (gfc_match ("(notinbranch)") == MATCH_YES)
builtin_kind = -1;
else if (gfc_match ("(inbranch)") == MATCH_YES)
builtin_kind = 1;
The space in gfc_match indicates gfc_gobble_whitespace (), i.e. optionally
eating whitespace (note, in fixed form white space is optionally eaten
pretty much always). If you want a mandatory space, there is "% ".
So it depends in if in fixed form we want to support e.g.
!GCC$ BUI L TIN SINf ATTRI BUT ESSIMD(NOT IN BRANCH)
and in free form e.g.
!gcc$ builtin sinf attributessimd (notinbranch)
I wouldn't use omp_simd because in C/C++ the attribute is called simd.

Post by Martin Liška
+
+ char *r = XNEWVEC (char, strlen (builtin) + 32);
+ sprintf (r, "__builtin_%s", builtin);
+ vectorized_builtins.safe_push (r);

Perhaps make it vector of const char *, int pairs, so that you can
encode no argument (aka inbranch + notinbranch) vs. inbranch vs. notinbranch?

If we want to support the variants, then yes.

Post by Jakub Jelinek

Post by Martin Liška
#define F951_OPTIONS "%(cc1_options) %{J*} \
- %{!nostdinc:-fintrinsic-modules-path finclude%s}\
+ %{!nostdinc:-fintrinsic-modules-path finclude%s " \
+ TARGET_F951_NOSTDINC_OPTIONS "}\

I wouldn't stick it inside of %{!nostdinc:, but let config/gnu-user.h decide
about that (i.e. wrap it into %{!nostdinc: ... } there).

Sure, works for me now.

Post by Jakub Jelinek

Post by Martin Liška
--- a/gcc/fortran/lang.opt
+++ b/gcc/fortran/lang.opt
@@ -662,6 +662,10 @@ fprotect-parens
Fortran Var(flag_protect_parens) Init(-1)
Protect parentheses in expressions.
+fpre-include=
+Fortran RejectNegative JoinedOrMissing Var(flag_pre_include) Undocumented
+Path to header file that should be pre-included before each compilation unit.

Why OrMissing? If the argument is missing, that should be an error, you
can't load "".

Ok.

Post by Jakub Jelinek

Post by Martin Liška
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -2365,6 +2365,16 @@ load_file (const char *realfilename, const char *displayedname, bool initial)
}
}
+ /* Make a guard to prevent recursive inclusion. */
+ static bool preinclude_done = false;
+ if (!preinclude_done)
+ {
+ preinclude_done = true;
+ if (flag_pre_include != NULL && !load_file (flag_pre_include, NULL,
+ false))
+ exit (FATAL_EXIT_CODE);
+ }

Why in load_file? I'd handle this in gfc_new_file, where it is called just
once. Formatting - would be nicer to put && on the next line and not wrap
load_file args.

Works for me.

Post by Jakub Jelinek

Post by Martin Liška
+static const char *
+find_fortran_header_file (int argc, const char **argv)
+{
+ if (argc != 2)
+ return NULL;
+
+ const char *path = find_file (argv[1]);
+ if (path != argv[1])
+ return concat (argv[0], path, NULL);

I wouldn't call it fortran_header_file but fortran_preinclude_file
(both in what appears in spec and the name of the callback).

Done.

Post by Jakub Jelinek
Plus, the patch lacks testcases. I'd use dg-options "-nostdinc"
so that in that testcase it isn't preincluded, and have one free form and
one fixed form testcase that has various forms of the !gcc$ builtin
for a couple of builtins + some calls to them and check how they are handled
(perhaps limited to the one or two ABIs that support those)?

Sure, will do as soon as final version of the patch will be done.

Thanks,
Martin

Post by Jakub Jelinek
Jakub

[Jakub Jelinek]

Post by Martin Liška

Post by Jakub Jelinek
So omp-simd-notinbranch or omp_simd_notinbranch?
Any particular reason for this weird syntax and for not also
supporting inbranch or just simd?

Questionable whether to support as current glibc vector ABI only uses notinbranch?

simd attribute supports all those three cases, i.e.
__attribute__((simd)), __attribute__((simd ("notinbranch"))) and
__attribute__((simd ("inbranch"))).
So at least for consistency it would be nice to support the same thing.
Some users could e.g. provide a vectorized definition for other builtins
and have e.g. inbranch support there only (the vectorizer can use those
by providing all ones masks).

Jakub

[Bernhard Reutner-Fischer]

Post by Martin Liška

Post by Martin Liška
--- a/gcc/config/gnu-user.h
+++ b/gcc/config/gnu-user.h
@@ -170,3 +170,6 @@ see the files COPYING3 and COPYING.RUNTIME

respectively. If not, see

Post by Martin Liška
LD_STATIC_OPTION " --whole-archive -llsan --no-whole-archive " \
LD_DYNAMIC_OPTION "}}%{!static-liblsan:-llsan}"
#endif
+
+#undef TARGET_F951_NOSTDINC_OPTIONS
+#define TARGET_F951_NOSTDINC_OPTIONS

"%:fortran-header-file(-fpre-include= math-vector-fortran.h)"
Too long line, use some \s to split it up.

Can we use plain -include like in C?
This has an established meaning and is already documented.

Thanks,

[Jakub Jelinek]

Post by Bernhard Reutner-Fischer

Post by Martin Liška

Post by Martin Liška
--- a/gcc/config/gnu-user.h
+++ b/gcc/config/gnu-user.h
@@ -170,3 +170,6 @@ see the files COPYING3 and COPYING.RUNTIME

respectively. If not, see

Post by Martin Liška
LD_STATIC_OPTION " --whole-archive -llsan --no-whole-archive " \
LD_DYNAMIC_OPTION "}}%{!static-liblsan:-llsan}"
#endif
+
+#undef TARGET_F951_NOSTDINC_OPTIONS
+#define TARGET_F951_NOSTDINC_OPTIONS

"%:fortran-header-file(-fpre-include= math-vector-fortran.h)"
Too long line, use some \s to split it up.

Can we use plain -include like in C?

Wouldn't that be confusing whether it is included in preprocessor only or if
it is included as a magic fortran include line at the beginning?

Jakub

[Martin Liška]

Post by Jakub Jelinek

Post by Bernhard Reutner-Fischer

Post by Martin Liška

Post by Martin Liška
--- a/gcc/config/gnu-user.h
+++ b/gcc/config/gnu-user.h
@@ -170,3 +170,6 @@ see the files COPYING3 and COPYING.RUNTIME

respectively. If not, see

Post by Martin Liška
LD_STATIC_OPTION " --whole-archive -llsan --no-whole-archive " \
LD_DYNAMIC_OPTION "}}%{!static-liblsan:-llsan}"
#endif
+
+#undef TARGET_F951_NOSTDINC_OPTIONS
+#define TARGET_F951_NOSTDINC_OPTIONS

"%:fortran-header-file(-fpre-include= math-vector-fortran.h)"
Too long line, use some \s to split it up.

Can we use plain -include like in C?

Wouldn't that be confusing whether it is included in preprocessor only or if
it is included as a magic fortran include line at the beginning?
Jakub

Agree with that, I'm sending updated version of the patch. It's tested on x86_64-linux-gnu,
where it survives regression tests and bootstraps.

I hope I addressed all notes that Jakub provided.

Thanks,
Martin

[Jakub Jelinek]

+ if (gfc_match (" (%n) attributes simd", builtin) != MATCH_YES)
+ return MATCH_ERROR;
+
+ int builtin_kind = 0;
+ if (gfc_match (" (notinbranch)") == MATCH_YES)

I think you need " ( notinbranch )" here.

+ builtin_kind = -1;
+ else if (gfc_match (" (inbranch)") == MATCH_YES)
+ builtin_kind = 1;

And similarly here (+ testsuite coverage for whether you can in free form
insert spaces in all the spots that should be allowed).
!gcc$ builtin ( sinf ) attributes simd ( notinbranch ) ! comment
e.g. should be valid in free form (and fixed form too).

--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2764,6 +2764,18 @@ bool gfc_in_match_data (void);
match gfc_match_char_spec (gfc_typespec *);
extern int directive_unroll;
+/* Tuple for parsing of vectorized built-ins. */
+struct vect_builtin_tuple
+{
+ vect_builtin_tuple (const char *n, int t): name (n), simd_type (t)

gfc_vect_builtin_tuple ?
+ document what the simd_type is (or make it enum or whatever).
One option would be enum omp_clause_code and use OMP_CLAUSE_ERROR for
the case where the argument isn't specified, but I think generally
gfortran.h doesn't depend on tree* stuff and wants to have its own
enums etc.

+extern vec<vect_builtin_tuple> vectorized_builtins;

gfc_vectorized_builtins ?

--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -597,7 +597,61 @@ define_quad_builtin (const char *name, tree type, bool is_const)
return fndecl;
}
+/* Add SIMD attribute for FNDECL built-in if the built-in
+ name is in VECTORIZED_BUILTINS. */
+#include "print-tree.h"

If you need to include a header, include it at the start of the file.

+static void
+add_simd_flag_for_built_in (tree fndecl)
+{
+ if (fndecl == NULL_TREE)
+ return;
+
+ const char *name = IDENTIFIER_POINTER (DECL_NAME (fndecl));
+ for (unsigned i = 0; i < vectorized_builtins.length (); i++)
+ if (strcmp (vectorized_builtins[i].name, name) == 0)

How many add_simd_flag_for_built_in calls are we expecting and how many
vectorized_builtins.length ()? If it is too much, perhaps e.g. sort
the vector by name and do a binary search. At least if it turns out to be
non-trivial compile time.

+
+ vectorized_builtins.truncate (0);

That is a memory leak, right? The names are malloced.
And why truncate rather than release?

+ const char *path = find_a_file (&include_prefixes, argv[1], R_OK, true);
+ if (path != NULL)
+ return concat (argv[0], path, NULL);

Formatting.

--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/simd-builtins-1.h
@@ -0,0 +1,4 @@
+!GCC$ builtin (sinf) attributes simd
+!GCC$ builtin (sinf) attributes simd (inbranch)
+!GCC$ builtin (sinf) attributes simd (notinbranch)
+!GCC$ builtin (cosf) attributes simd (notinbranch)

Are you sure it is a good idea to have the 3 first lines for the same
builtin, rather than different?

It should be testsuite covered what we do in that case, but with the above
you don't cover what happens e.g. with notinbranch alone, or no argument.

Plus, as I said, I think you should have one *.f and one *.f90 test where
you just use many of those !gcc$ builtin lines with spaces in various spots
to verify it is parsed properly.

Jakub

[Martin Liška]

Post by Jakub Jelinek

+ if (gfc_match (" (%n) attributes simd", builtin) != MATCH_YES)
+ return MATCH_ERROR;
+
+ int builtin_kind = 0;
+ if (gfc_match (" (notinbranch)") == MATCH_YES)

I think you need " ( notinbranch )" here.

+ builtin_kind = -1;
+ else if (gfc_match (" (inbranch)") == MATCH_YES)
+ builtin_kind = 1;

And similarly here (+ testsuite coverage for whether you can in free form
insert spaces in all the spots that should be allowed).
!gcc$ builtin ( sinf ) attributes simd ( notinbranch ) ! comment
e.g. should be valid in free form (and fixed form too).

--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2764,6 +2764,18 @@ bool gfc_in_match_data (void);
match gfc_match_char_spec (gfc_typespec *);
extern int directive_unroll;
+/* Tuple for parsing of vectorized built-ins. */
+struct vect_builtin_tuple
+{
+ vect_builtin_tuple (const char *n, int t): name (n), simd_type (t)

gfc_vect_builtin_tuple ?
+ document what the simd_type is (or make it enum or whatever).
One option would be enum omp_clause_code and use OMP_CLAUSE_ERROR for
the case where the argument isn't specified, but I think generally
gfortran.h doesn't depend on tree* stuff and wants to have its own
enums etc.

+extern vec<vect_builtin_tuple> vectorized_builtins;

gfc_vectorized_builtins ?

--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -597,7 +597,61 @@ define_quad_builtin (const char *name, tree type, bool is_const)
return fndecl;
}
+/* Add SIMD attribute for FNDECL built-in if the built-in
+ name is in VECTORIZED_BUILTINS. */
+#include "print-tree.h"

If you need to include a header, include it at the start of the file.

+static void
+add_simd_flag_for_built_in (tree fndecl)
+{
+ if (fndecl == NULL_TREE)
+ return;
+
+ const char *name = IDENTIFIER_POINTER (DECL_NAME (fndecl));
+ for (unsigned i = 0; i < vectorized_builtins.length (); i++)
+ if (strcmp (vectorized_builtins[i].name, name) == 0)

How many add_simd_flag_for_built_in calls are we expecting and how many
vectorized_builtins.length ()? If it is too much, perhaps e.g. sort
the vector by name and do a binary search. At least if it turns out to be
non-trivial compile time.

+
+ vectorized_builtins.truncate (0);

That is a memory leak, right? The names are malloced.
And why truncate rather than release?

+ const char *path = find_a_file (&include_prefixes, argv[1], R_OK, true);
+ if (path != NULL)
+ return concat (argv[0], path, NULL);

Formatting.

--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/simd-builtins-1.h
@@ -0,0 +1,4 @@
+!GCC$ builtin (sinf) attributes simd
+!GCC$ builtin (sinf) attributes simd (inbranch)
+!GCC$ builtin (sinf) attributes simd (notinbranch)
+!GCC$ builtin (cosf) attributes simd (notinbranch)

Are you sure it is a good idea to have the 3 first lines for the same
builtin, rather than different?
It should be testsuite covered what we do in that case, but with the above
you don't cover what happens e.g. with notinbranch alone, or no argument.
Plus, as I said, I think you should have one *.f and one *.f90 test where
you just use many of those !gcc$ builtin lines with spaces in various spots
to verify it is parsed properly.
Jakub

Hi.

I'm sending version, I changed the container to hash_map that should provide
faster look up.

I've been testing the patch right now.

Martin

[Paul Richard Thomas]

Hi All,

I took a few moments away from what I really must be doing to try out
an earlier version of the patch. There are quite a few CRs in the
patch and the third chunk in gcc.c was rejected, although I cannot see
why. I made a change to scanner.c to prevent the segfault that results
from not having the pre-include file in the right directory - see
attached.

Everything works fine with the testcases and a slightly evolved
version shows reasonable speed-ups:
program test_overloaded_intrinsic
real(4) :: x4(3200), y4(3200, 10000)
real(8) :: x8(3200), y8(3200)

do i = 1,10000
y4(:,i) = cos(x4)
y4(:,i) = x4*y4(:,i)
y4(:,i) = sqrt(y4(:,i))
end do
print *, y4(1,1)
end

Disappointingly, though, one of the worst cases in
https://www.fortran.uk/fortran-compiler-comparisons/ of a big
difference between gfortran and ifort with vectorization,
mp_prop_design.f90, doesn't seem to pick up any of the vectorization
opportunities, even though it is peppered with trig functions. Should
I be expecting this? Yes, I did add the other trig functions to the
pre-include file :-)

Best regards and thanks for taking care of this

Paul

Post by Martin Liška

Post by Jakub Jelinek

+ if (gfc_match (" (%n) attributes simd", builtin) != MATCH_YES)
+ return MATCH_ERROR;
+
+ int builtin_kind = 0;
+ if (gfc_match (" (notinbranch)") == MATCH_YES)

I think you need " ( notinbranch )" here.

+ builtin_kind = -1;
+ else if (gfc_match (" (inbranch)") == MATCH_YES)
+ builtin_kind = 1;

And similarly here (+ testsuite coverage for whether you can in free form
insert spaces in all the spots that should be allowed).
!gcc$ builtin ( sinf ) attributes simd ( notinbranch ) ! comment
e.g. should be valid in free form (and fixed form too).

--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2764,6 +2764,18 @@ bool gfc_in_match_data (void);
match gfc_match_char_spec (gfc_typespec *);
extern int directive_unroll;
+/* Tuple for parsing of vectorized built-ins. */
+struct vect_builtin_tuple
+{
+ vect_builtin_tuple (const char *n, int t): name (n), simd_type (t)

gfc_vect_builtin_tuple ?
+ document what the simd_type is (or make it enum or whatever).
One option would be enum omp_clause_code and use OMP_CLAUSE_ERROR for
the case where the argument isn't specified, but I think generally
gfortran.h doesn't depend on tree* stuff and wants to have its own
enums etc.

+extern vec<vect_builtin_tuple> vectorized_builtins;

gfc_vectorized_builtins ?

--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -597,7 +597,61 @@ define_quad_builtin (const char *name, tree type, bool is_const)
return fndecl;
}
+/* Add SIMD attribute for FNDECL built-in if the built-in
+ name is in VECTORIZED_BUILTINS. */
+#include "print-tree.h"

If you need to include a header, include it at the start of the file.

+static void
+add_simd_flag_for_built_in (tree fndecl)
+{
+ if (fndecl == NULL_TREE)
+ return;
+
+ const char *name = IDENTIFIER_POINTER (DECL_NAME (fndecl));
+ for (unsigned i = 0; i < vectorized_builtins.length (); i++)
+ if (strcmp (vectorized_builtins[i].name, name) == 0)

How many add_simd_flag_for_built_in calls are we expecting and how many
vectorized_builtins.length ()? If it is too much, perhaps e.g. sort
the vector by name and do a binary search. At least if it turns out to be
non-trivial compile time.

+
+ vectorized_builtins.truncate (0);

That is a memory leak, right? The names are malloced.
And why truncate rather than release?

+ const char *path = find_a_file (&include_prefixes, argv[1], R_OK, true);
+ if (path != NULL)
+ return concat (argv[0], path, NULL);

Formatting.

--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/simd-builtins-1.h
@@ -0,0 +1,4 @@
+!GCC$ builtin (sinf) attributes simd
+!GCC$ builtin (sinf) attributes simd (inbranch)
+!GCC$ builtin (sinf) attributes simd (notinbranch)
+!GCC$ builtin (cosf) attributes simd (notinbranch)

Are you sure it is a good idea to have the 3 first lines for the same
builtin, rather than different?
It should be testsuite covered what we do in that case, but with the above
you don't cover what happens e.g. with notinbranch alone, or no argument.
Plus, as I said, I think you should have one *.f and one *.f90 test where
you just use many of those !gcc$ builtin lines with spaces in various spots
to verify it is parsed properly.
Jakub

Hi.
I'm sending version, I changed the container to hash_map that should provide
faster look up.
I've been testing the patch right now.
Martin

[Paul Richard Thomas]

Hi All,

Forget my remark about mp_prop_design.f90. ifort -parallel means just
that and has nothing to do with vectorization.

Sorry for the noise.

Paul

On Sat, 17 Nov 2018 at 13:29, Paul Richard Thomas

Post by Paul Richard Thomas
Hi All,
I took a few moments away from what I really must be doing to try out
an earlier version of the patch. There are quite a few CRs in the
patch and the third chunk in gcc.c was rejected, although I cannot see
why. I made a change to scanner.c to prevent the segfault that results
from not having the pre-include file in the right directory - see
attached.
Everything works fine with the testcases and a slightly evolved
program test_overloaded_intrinsic
real(4) :: x4(3200), y4(3200, 10000)
real(8) :: x8(3200), y8(3200)
do i = 1,10000
y4(:,i) = cos(x4)
y4(:,i) = x4*y4(:,i)
y4(:,i) = sqrt(y4(:,i))
end do
print *, y4(1,1)
end
Disappointingly, though, one of the worst cases in
https://www.fortran.uk/fortran-compiler-comparisons/ of a big
difference between gfortran and ifort with vectorization,
mp_prop_design.f90, doesn't seem to pick up any of the vectorization
opportunities, even though it is peppered with trig functions. Should
I be expecting this? Yes, I did add the other trig functions to the
pre-include file :-)
Best regards and thanks for taking care of this
Paul

Post by Martin Liška

Post by Jakub Jelinek

+ if (gfc_match (" (%n) attributes simd", builtin) != MATCH_YES)
+ return MATCH_ERROR;
+
+ int builtin_kind = 0;
+ if (gfc_match (" (notinbranch)") == MATCH_YES)

I think you need " ( notinbranch )" here.

+ builtin_kind = -1;
+ else if (gfc_match (" (inbranch)") == MATCH_YES)
+ builtin_kind = 1;

And similarly here (+ testsuite coverage for whether you can in free form
insert spaces in all the spots that should be allowed).
!gcc$ builtin ( sinf ) attributes simd ( notinbranch ) ! comment
e.g. should be valid in free form (and fixed form too).

--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2764,6 +2764,18 @@ bool gfc_in_match_data (void);
match gfc_match_char_spec (gfc_typespec *);
extern int directive_unroll;
+/* Tuple for parsing of vectorized built-ins. */
+struct vect_builtin_tuple
+{
+ vect_builtin_tuple (const char *n, int t): name (n), simd_type (t)

gfc_vect_builtin_tuple ?
+ document what the simd_type is (or make it enum or whatever).
One option would be enum omp_clause_code and use OMP_CLAUSE_ERROR for
the case where the argument isn't specified, but I think generally
gfortran.h doesn't depend on tree* stuff and wants to have its own
enums etc.

+extern vec<vect_builtin_tuple> vectorized_builtins;

gfc_vectorized_builtins ?

--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -597,7 +597,61 @@ define_quad_builtin (const char *name, tree type, bool is_const)
return fndecl;
}
+/* Add SIMD attribute for FNDECL built-in if the built-in
+ name is in VECTORIZED_BUILTINS. */
+#include "print-tree.h"

If you need to include a header, include it at the start of the file.

+static void
+add_simd_flag_for_built_in (tree fndecl)
+{
+ if (fndecl == NULL_TREE)
+ return;
+
+ const char *name = IDENTIFIER_POINTER (DECL_NAME (fndecl));
+ for (unsigned i = 0; i < vectorized_builtins.length (); i++)
+ if (strcmp (vectorized_builtins[i].name, name) == 0)

How many add_simd_flag_for_built_in calls are we expecting and how many
vectorized_builtins.length ()? If it is too much, perhaps e.g. sort
the vector by name and do a binary search. At least if it turns out to be
non-trivial compile time.

+
+ vectorized_builtins.truncate (0);

That is a memory leak, right? The names are malloced.
And why truncate rather than release?

+ const char *path = find_a_file (&include_prefixes, argv[1], R_OK, true);
+ if (path != NULL)
+ return concat (argv[0], path, NULL);

Formatting.

--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/simd-builtins-1.h
@@ -0,0 +1,4 @@
+!GCC$ builtin (sinf) attributes simd
+!GCC$ builtin (sinf) attributes simd (inbranch)
+!GCC$ builtin (sinf) attributes simd (notinbranch)
+!GCC$ builtin (cosf) attributes simd (notinbranch)

Are you sure it is a good idea to have the 3 first lines for the same
builtin, rather than different?
It should be testsuite covered what we do in that case, but with the above
you don't cover what happens e.g. with notinbranch alone, or no argument.
Plus, as I said, I think you should have one *.f and one *.f90 test where
you just use many of those !gcc$ builtin lines with spaces in various spots
to verify it is parsed properly.
Jakub

Hi.
I'm sending version, I changed the container to hash_map that should provide
faster look up.
I've been testing the patch right now.
Martin

--
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

[Bernhard Reutner-Fischer]

Hi

Post by Martin Liška
I'm sending version, I changed the container to hash_map that should provide
faster look up.
I've been testing the patch right now.

In find_fortran_preinclude_file() you allocate the filename.

diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index 55d6dafdb5d..4e500f88174 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -2428,6 +2428,10 @@ gfc_new_file (void)
{
bool result;

+ if (flag_pre_include != NULL
+ && !load_file (flag_pre_include, NULL, false))
+ exit (FATAL_EXIT_CODE);
+
if (gfc_cpp_enabled ())
{
result = gfc_cpp_preprocess (gfc_source_file);


Don't you leak the filename here?
I.e.
else free()

thanks,

[Martin Liška]

Post by Bernhard Reutner-Fischer
Hi

Post by Martin Liška
I'm sending version, I changed the container to hash_map that should provide
faster look up.
I've been testing the patch right now.

In find_fortran_preinclude_file() you allocate the filename.

Sure, but that's driver allocation and apparently valgrind
is not complaining about the memory allocation.

Post by Bernhard Reutner-Fischer
diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index 55d6dafdb5d..4e500f88174 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -2428,6 +2428,10 @@ gfc_new_file (void)
{
bool result;
+ if (flag_pre_include != NULL
+ && !load_file (flag_pre_include, NULL, false))
+ exit (FATAL_EXIT_CODE);
+
if (gfc_cpp_enabled ())
{
result = gfc_cpp_preprocess (gfc_source_file);
Don't you leak the filename here?

This string is in FE, which lives in a option string pool.

Martin

Post by Bernhard Reutner-Fischer
I.e.
else free()
thanks,

[Martin Liška]

Post by Martin Liška

Post by Jakub Jelinek

+ if (gfc_match (" (%n) attributes simd", builtin) != MATCH_YES)
+ return MATCH_ERROR;
+
+ int builtin_kind = 0;
+ if (gfc_match (" (notinbranch)") == MATCH_YES)

I think you need " ( notinbranch )" here.

+ builtin_kind = -1;
+ else if (gfc_match (" (inbranch)") == MATCH_YES)
+ builtin_kind = 1;

And similarly here (+ testsuite coverage for whether you can in free form
insert spaces in all the spots that should be allowed).
!gcc$ builtin ( sinf ) attributes simd ( notinbranch ) ! comment
e.g. should be valid in free form (and fixed form too).

--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2764,6 +2764,18 @@ bool gfc_in_match_data (void);
match gfc_match_char_spec (gfc_typespec *);
extern int directive_unroll;
+/* Tuple for parsing of vectorized built-ins. */
+struct vect_builtin_tuple
+{
+ vect_builtin_tuple (const char *n, int t): name (n), simd_type (t)

gfc_vect_builtin_tuple ?
+ document what the simd_type is (or make it enum or whatever).
One option would be enum omp_clause_code and use OMP_CLAUSE_ERROR for
the case where the argument isn't specified, but I think generally
gfortran.h doesn't depend on tree* stuff and wants to have its own
enums etc.

+extern vec<vect_builtin_tuple> vectorized_builtins;

gfc_vectorized_builtins ?

--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -597,7 +597,61 @@ define_quad_builtin (const char *name, tree type, bool is_const)
return fndecl;
}
+/* Add SIMD attribute for FNDECL built-in if the built-in
+ name is in VECTORIZED_BUILTINS. */
+#include "print-tree.h"

If you need to include a header, include it at the start of the file.

+static void
+add_simd_flag_for_built_in (tree fndecl)
+{
+ if (fndecl == NULL_TREE)
+ return;
+
+ const char *name = IDENTIFIER_POINTER (DECL_NAME (fndecl));
+ for (unsigned i = 0; i < vectorized_builtins.length (); i++)
+ if (strcmp (vectorized_builtins[i].name, name) == 0)

How many add_simd_flag_for_built_in calls are we expecting and how many
vectorized_builtins.length ()? If it is too much, perhaps e.g. sort
the vector by name and do a binary search. At least if it turns out to be
non-trivial compile time.

+
+ vectorized_builtins.truncate (0);

That is a memory leak, right? The names are malloced.
And why truncate rather than release?

+ const char *path = find_a_file (&include_prefixes, argv[1], R_OK, true);
+ if (path != NULL)
+ return concat (argv[0], path, NULL);

Formatting.

--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/simd-builtins-1.h
@@ -0,0 +1,4 @@
+!GCC$ builtin (sinf) attributes simd
+!GCC$ builtin (sinf) attributes simd (inbranch)
+!GCC$ builtin (sinf) attributes simd (notinbranch)
+!GCC$ builtin (cosf) attributes simd (notinbranch)

Are you sure it is a good idea to have the 3 first lines for the same
builtin, rather than different?
It should be testsuite covered what we do in that case, but with the above
you don't cover what happens e.g. with notinbranch alone, or no argument.
Plus, as I said, I think you should have one *.f and one *.f90 test where
you just use many of those !gcc$ builtin lines with spaces in various spots
to verify it is parsed properly.
Jakub

Hi.
I'm sending version, I changed the container to hash_map that should provide
faster look up.
I've been testing the patch right now.
Martin

Hi.

I'm sending one another tested version on x86_64-linux-gnu. I fixed issues spotted
by Jakub.

Martin

[Jakub Jelinek]

* config/gnu-user.h (TARGET_F951_OPTIONS): New.
* gcc.c (find_fortran_preinclude_file): New function
to handle Fortran pre-include.
* decl.c (gfc_match_gcc_builtin): New function.
* gfortran.h (struct vect_builtin_tuple): New.
(gfc_adjust_builtins): Likewise.
* lang-specs.h (TARGET_F951_OPTIONS): New.
(F951_OPTIONS): Use it.
* lang.opt: Add new option -fpre-include.
* match.h (gfc_match_gcc_builtin): Declare new function.
* parse.c (decode_gcc_attribute): Handle builtin.
(parse_progunit): Call gfc_adjust_builtins.
* scanner.c (gfc_new_file): Load pre-included header file
when provided.
* trans-intrinsic.c (add_simd_flag_for_built_in): New.
(gfc_adjust_builtins): Likewise.
* gfortran.dg/simd-builtins-1.f90: New test.
* gfortran.dg/simd-builtins-1.h: New test.
* gfortran.dg/simd-builtins-2.f90: New test.
* gfortran.dg/simd-builtins-3.f90: New test.
* gfortran.dg/simd-builtins-3.h: New test.
* gfortran.dg/simd-builtins-4.f: New test.
* gfortran.dg/simd-builtins-4.h: New test.
* gfortran.dg/simd-builtins-5.f: New test.
* gfortran.dg/simd-builtins-6.f90: New test.

LGTM. Please give Fortran maintainers 2 days so that they can comment on it
before committing.

Jakub

[Martin Liška]

Hi.

Following patch is a follow up of the discussion we had on IRC about
locations where a Fortran pre-include should be searched.

With the patch applied I see now:
$ strace -f -s512 ./xgcc -B. ~/Programming/testcases/usage.F90 -c 2>&1 | grep math-vector
access("./x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("./math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/bin/../include/finclude/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/bin/../include/finclude/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/usr/include/finclude/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/usr/include/finclude/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)

Martin

[Jakub Jelinek]

Post by Martin Liška
Following patch is a follow up of the discussion we had on IRC about
locations where a Fortran pre-include should be searched.
$ strace -f -s512 ./xgcc -B. ~/Programming/testcases/usage.F90 -c 2>&1 | grep math-vector
access("./x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("./math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/bin/../include/finclude/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/home/marxin/bin/gcc2/bin/../include/finclude/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/usr/include/finclude/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
access("/usr/include/finclude/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or directory)
static const char *
find_fortran_preinclude_file (int argc, const char **argv)
{
+ char *result = NULL;
if (argc != 2)
return NULL;

it doesn't search the same directory as is normally searched for Fortran
modules; that is finclude%s and thus would need to be yet another
argument to this spec internal function and you'd check if
a file exists with that prefix

Post by Martin Liška
+ struct path_prefix prefixes = { 0, 0, "preinclude" };
+ add_prefix (&prefixes, STANDARD_BINDIR_PREFIX "../include/finclude/", NULL,
+ 0, 0, 0);
+ add_prefix (&prefixes, "/usr/include/finclude/", NULL, 0, 0, 0);

hardcoding /usr/include looks just very wrong here. That should always be
dependent on the configured prefix or better be relative from the driver,
gcc should be relocatable. Or at least come from configure. It should e.g.
honor the sysroot stuff etc.

That said, I think you need somebody familiar with the driver, perhaps
Joseph?

Jakub

[Joseph Myers]

Post by Jakub Jelinek
hardcoding /usr/include looks just very wrong here. That should always be
dependent on the configured prefix or better be relative from the driver,
gcc should be relocatable. Or at least come from configure. It should e.g.
honor the sysroot stuff etc.
That said, I think you need somebody familiar with the driver, perhaps
Joseph?

I'd sort of expect structures like those in cppdefault.[ch] to describe
the relevant Fortran directories and their properties (such as being
sysrooted or not - and if sysrooted, I suppose you'll want to make sure
SYSROOT_HEADERS_SUFFIX_SPEC is properly applied).

If this preinclude doesn't pass through the C preprocessor, directories in
which it is searched for will need multilib or multiarch suffixes.
(Multilib suffixes on include directories for C are more or less an
implementation detail of how fixed headers are arranged in the case where
sysroot headers suffixes are used; they aren't really expected to be a
stable interface such that third-party software might install anything
using them, but I'm not sure if this preinclude is meant to come from
external software or be installed by GCC. Multiarch suffixes, for systems
using Debian/Ubuntu-style multiarch directory arrangements, *are* intended
as a stable interface. And multilib *OS* suffixes
(-print-multi-os-directory) are a stable interface, but only really
suitable for libraries, not headers, because they are paths relative to
lib/ such as ../lib64.)

[Martin Liška]

Post by Joseph Myers

Post by Jakub Jelinek
hardcoding /usr/include looks just very wrong here. That should always be
dependent on the configured prefix or better be relative from the driver,
gcc should be relocatable. Or at least come from configure. It should e.g.
honor the sysroot stuff etc.
That said, I think you need somebody familiar with the driver, perhaps
Joseph?

Hi Joseph.

Post by Joseph Myers
I'd sort of expect structures like those in cppdefault.[ch] to describe
the relevant Fortran directories and their properties (such as being
sysrooted or not - and if sysrooted, I suppose you'll want to make sure
SYSROOT_HEADERS_SUFFIX_SPEC is properly applied).

We probably forgot to mention that the search mechanism happens in GCC driver.
It's because we want to include the file (via a Fortran -fpre-include) conditionally
only when found.

Post by Joseph Myers
If this preinclude doesn't pass through the C preprocessor, directories in
which it is searched for will need multilib or multiarch suffixes.

No, it's not C preprocessor.

Post by Joseph Myers
(Multilib suffixes on include directories for C are more or less an
implementation detail of how fixed headers are arranged in the case where
sysroot headers suffixes are used; they aren't really expected to be a
stable interface such that third-party software might install anything
using them, but I'm not sure if this preinclude is meant to come from
external software or be installed by GCC.

It will come from glibc-devel package, and it's expected to be installed in:
usr/include/finclude/ for 64-bit header
and /usr/include/finclude/32/ for the 32-bit header.

Multiarch suffixes, for systems

Post by Joseph Myers
using Debian/Ubuntu-style multiarch directory arrangements, *are* intended
as a stable interface. And multilib *OS* suffixes
(-print-multi-os-directory) are a stable interface, but only really
suitable for libraries, not headers, because they are paths relative to
lib/ such as ../lib64.)

If I understand correctly, Jakub wanted to have it working with -m32 being used
for 64-bit GCC compiler. So yes, multi os for header files.

So the question is what can we really do in the GCC driver?

Martin

[Joseph Myers]

Post by Martin Liška

Post by Joseph Myers
(Multilib suffixes on include directories for C are more or less an
implementation detail of how fixed headers are arranged in the case where
sysroot headers suffixes are used; they aren't really expected to be a
stable interface such that third-party software might install anything
using them, but I'm not sure if this preinclude is meant to come from
external software or be installed by GCC.

usr/include/finclude/ for 64-bit header
and /usr/include/finclude/32/ for the 32-bit header.

So, to be clear, is that

<sysroot><sysroot-headers-suffix><native-system-header-dir>/finclude/$($CC $CFLAGS $CPPFLAGS -print-multi-directory)

? (Where glibc would be what uses the $CC $CFLAGS $CPPFLAGS
-print-multi-directory to determine where to install the file.)

If so, you need to make sure that all of those pieces are properly used.

* The sysroot and headers suffix in the case of a sysrooted toolchain.
(Sysroot headers suffixes are for e.g. the case of a toolchain with both
glibc and uClibc multilibs, so needing multiple sets of headers. Most
toolchains with multiple sysroots using the same libc only need a single
set of headers and don't use sysroot headers suffixes, only sysroot
suffixes (non-headers), with appropriate arrangements being made for all
the per-multilib headers, such as gnu/lib-names-*.h and gnu/stubs-*.h, to
be copied into the common include directory.)

* The native system header directory (which is /include not /usr/include
for GNU Hurd, for example; see config.gcc).

* Then finclude with the multilib (non-OS) suffix.

And you need to consider what's right for non-sysrooted toolchains. If
native, the above is right, but without the sysroot-related components.
But what about a non-sysrooted cross toolchain? Certainly using the
native directory would be wrong there.

Also, what's right in the multiarch directory arrangements case - should
it be
<sysroot><sysroot-headers-suffix><native-system-header-dir>/<multiarch>/finclude
instead? Would one of the Debian / Ubuntu GCC maintainers like to
comment?

Are there corresponding versions with /usr/local/include
(LOCAL_INCLUDE_DIR, in general), before those with
NATIVE_SYSTEM_HEADER_DIR? Even in the driver, the list of directories in
cppdefault.c should at least serve as a guide to which directories you
want to search and which get sysroots added (of course, the C++-specific
ones are irrelevant here).

[Martin Liška]

Post by Joseph Myers

Post by Martin Liška

Post by Joseph Myers
(Multilib suffixes on include directories for C are more or less an
implementation detail of how fixed headers are arranged in the case where
sysroot headers suffixes are used; they aren't really expected to be a
stable interface such that third-party software might install anything
using them, but I'm not sure if this preinclude is meant to come from
external software or be installed by GCC.

usr/include/finclude/ for 64-bit header
and /usr/include/finclude/32/ for the 32-bit header.

So, to be clear, is that
<sysroot><sysroot-headers-suffix><native-system-header-dir>/finclude/$($CC $CFLAGS $CPPFLAGS -print-multi-directory)
? (Where glibc would be what uses the $CC $CFLAGS $CPPFLAGS
-print-multi-directory to determine where to install the file.)
If so, you need to make sure that all of those pieces are properly used.
* The sysroot and headers suffix in the case of a sysrooted toolchain.
(Sysroot headers suffixes are for e.g. the case of a toolchain with both
glibc and uClibc multilibs, so needing multiple sets of headers. Most
toolchains with multiple sysroots using the same libc only need a single
set of headers and don't use sysroot headers suffixes, only sysroot
suffixes (non-headers), with appropriate arrangements being made for all
the per-multilib headers, such as gnu/lib-names-*.h and gnu/stubs-*.h, to
be copied into the common include directory.)
* The native system header directory (which is /include not /usr/include
for GNU Hurd, for example; see config.gcc).
* Then finclude with the multilib (non-OS) suffix.
And you need to consider what's right for non-sysrooted toolchains. If
native, the above is right, but without the sysroot-related components.
But what about a non-sysrooted cross toolchain? Certainly using the
native directory would be wrong there.
Also, what's right in the multiarch directory arrangements case - should
it be
<sysroot><sysroot-headers-suffix><native-system-header-dir>/<multiarch>/finclude
instead? Would one of the Debian / Ubuntu GCC maintainers like to
comment?
Are there corresponding versions with /usr/local/include
(LOCAL_INCLUDE_DIR, in general), before those with
NATIVE_SYSTEM_HEADER_DIR? Even in the driver, the list of directories in
cppdefault.c should at least serve as a guide to which directories you
want to search and which get sysroots added (of course, the C++-specific
ones are irrelevant here).

Hi.

Looks the problematic is quite complex as I can understand. I prepared a patch
that should hopefully follow advises provided.

Thoughts?
Thanks,
Martin

[Joseph Myers]

Post by Martin Liška
Looks the problematic is quite complex as I can understand. I prepared a patch
that should hopefully follow advises provided.

I don't see how this version ensures that NATIVE_SYSTEM_HEADER_DIR is
properly sysrooted. Note there's add_sysrooted_prefix separate from
add_prefix (but that's *not* the correct thing to use here because it uses
target_sysroot_suffix whereas you need target_sysroot_hdrs_suffix).

The last argument to add_prefix, which you're setting to true, is
os_multilib. But using the OS multilib scheme seems wrong here, because
the OS multilib names are names like "../lib64", which only make sense in
directories called lib - you don't want to end up searching a directory
<something>/include/finclude/../lib64.

In the multiarch case, do you want
<something>/include/finclude/<multiarch> or
<something>/include/<multiarch>/finclude? (This is where I'd hope Debian
/ Ubuntu GCC people would comment.)

[Martin Liška]

Post by Joseph Myers
In the multiarch case, do you want
<something>/include/finclude/<multiarch> or
<something>/include/<multiarch>/finclude? (This is where I'd hope Debian
/ Ubuntu GCC people would comment.)

Mathias can you please reply to this?

Thanks,
Martin

[Matthias Klose]

Post by Martin Liška

Post by Joseph Myers
In the multiarch case, do you want
<something>/include/finclude/<multiarch> or
<something>/include/<multiarch>/finclude? (This is where I'd hope Debian
/ Ubuntu GCC people would comment.)

Mathias can you please reply to this?

this should not matter, as long as you use the multilib name, and the correct
directory is used with the -m32/-m64/-mabi options. Are other compilers like a
clang based compiler supposed to access this directory as well? In that case
the include directories should be documented.

One more question: Will GCC itself install header files in these directories, or
will you have different directories depending on the GCC version (or gfortran
module version)?

Matthias

[Martin Liška]

Post by Matthias Klose

Post by Martin Liška

Post by Joseph Myers
In the multiarch case, do you want
<something>/include/finclude/<multiarch> or
<something>/include/<multiarch>/finclude? (This is where I'd hope Debian
/ Ubuntu GCC people would comment.)

Mathias can you please reply to this?

this should not matter, as long as you use the multilib name, and the correct
directory is used with the -m32/-m64/-mabi options. Are other compilers like a
clang based compiler supposed to access this directory as well?

I don't think so.

Post by Matthias Klose
In that case
the include directories should be documented.
One more question: Will GCC itself install header files in these directories, or
will you have different directories depending on the GCC version (or gfortran
module version)?

The header file will be install by glibc (glibc-devel package).

Martin

Post by Matthias Klose
Matthias

[Joseph Myers]

Post by Martin Liška
The header file will be install by glibc (glibc-devel package).

To confirm: you intend to submit a patch to glibc upstream to install this
file (rather than it only being something in distribution packaging)?

[Martin Liška]

Post by Joseph Myers

Post by Martin Liška
The header file will be install by glibc (glibc-devel package).

To confirm: you intend to submit a patch to glibc upstream to install this
file (rather than it only being something in distribution packaging)?

Yes, I've already ask glibc people about it:
https://sourceware.org/ml/libc-help/2018-11/msg00015.html

I would appreciate a help with that.

Martin

[Thomas Koenig]

Hi Martin,

Post by Martin Liška
he header file will be install by glibc (glibc-devel package).

Why actually glibc-devel? Needing glibc-devel for fast compilation
of Fortran seems to be counter-intuitive. Maybe glibc would be better.

Regards

Thomas

[Martin Liška]

Post by Thomas Koenig
Hi Martin,

Post by Martin Liška
he header file will be install by glibc (glibc-devel package).

Why actually glibc-devel?  Needing glibc-devel for fast compilation
of Fortran seems to be counter-intuitive. Maybe glibc would be better.

Works for me.

Martin

Post by Thomas Koenig
Regards
    Thomas

[Steve Ellcey]

Post by Richard Biener

Post by Matthias Klose

Post by Martin Liška

Post by Joseph Myers
In the multiarch case, do you want
<something>/include/finclude/<multiarch> or
<something>/include/<multiarch>/finclude? (This is where I'd hope Debian
/ Ubuntu GCC people would comment.)

Mathias can you please reply to this?

this should not matter, as long as you use the multilib name, and the correct
directory is used with the -m32/-m64/-mabi options. Are other compilers like a
clang based compiler supposed to access this directory as well?

I don't think so.

Post by Matthias Klose
In that case
the include directories should be documented.

Why wouldn't clang (flang) want to use the same mechanism as
GCC/gfortran? I know there is some interest/work going on here for
flang and we would like a consistent way to use pre-includes to define
SIMD vector functions in both gfortran and flang. I think this should
be documented so flang and other compilers can use it. Even if no
other compilers did use it I think it should be documented because it
crosses project/package boundries, i.e. it is created by glibc and used
by gfortran.

Steve Ellcey
sel

[Thomas Koenig]

Post by Steve Ellcey
Why wouldn't clang (flang) want to use the same mechanism as
GCC/gfortran? I know there is some interest/work going on here for
flang and we would like a consistent way to use pre-includes to define
SIMD vector functions in both gfortran and flang. I think this should
be documented so flang and other compilers can use it. Even if no
other compilers did use it I think it should be documented because it
crosses project/package boundries, i.e. it is created by glibc and used
by gfortran.

Absolutely.

As soon as this is committed, we should document all the
specifics in the gfortran manual.

Regards

Thomas

[Martin Liška]

Post by Joseph Myers

Post by Martin Liška
Looks the problematic is quite complex as I can understand. I prepared a patch
that should hopefully follow advises provided.

Hello.

Post by Joseph Myers
I don't see how this version ensures that NATIVE_SYSTEM_HEADER_DIR is
properly sysrooted. Note there's add_sysrooted_prefix separate from
add_prefix (but that's *not* the correct thing to use here because it uses
target_sysroot_suffix whereas you need target_sysroot_hdrs_suffix).

I address that in updated version of the patch.

Post by Joseph Myers
The last argument to add_prefix, which you're setting to true, is
os_multilib. But using the OS multilib scheme seems wrong here, because
the OS multilib names are names like "../lib64", which only make sense in
directories called lib - you don't want to end up searching a directory
<something>/include/finclude/../lib64.

Agree, does not make sense.

Post by Joseph Myers
In the multiarch case, do you want
<something>/include/finclude/<multiarch> or
<something>/include/<multiarch>/finclude? (This is where I'd hope Debian
/ Ubuntu GCC people would comment.)

As Mathias wrote he's fine with both of these variants. I've just configure a compiler
with --enable-multiarch=yes and I see that the file is eventually searched in:

/usr/include/finclude/i386-linux-gnu/math-vector-fortran.h (with -m32)

and

/usr/include/finclude/x86_64-linux-gnu/math-vector-fortran.h (with -m64)

Which looks fine to me?

Thanks,
Martin

[Joseph Myers]

Post by Martin Liška

Post by Joseph Myers
I don't see how this version ensures that NATIVE_SYSTEM_HEADER_DIR is
properly sysrooted. Note there's add_sysrooted_prefix separate from
add_prefix (but that's *not* the correct thing to use here because it uses
target_sysroot_suffix whereas you need target_sysroot_hdrs_suffix).

I address that in updated version of the patch.

However, this version seems to make TOOL_INCLUDE_DIR sysrooted as well.
I don't think that's correct; TOOL_INCLUDE_DIR ($prefix/$target/include,
roughly) is a non-sysroot location for headers. Note that it's not
sysrooted in cppdefault.c, which is a good guide to which directories
should or should not be sysrooted, and what order they should come in
(though as discussed, various of the directories there are not relevant
for the present issue).

The patch appears to be against some tree other than current trunk. At
least, it shows a function find_fortran_preinclude_file in gcc.c as
already existing in the diff context, but I see no such function in the
current sources.

[Martin Liška]

Post by Joseph Myers

Post by Martin Liška

Post by Joseph Myers
I don't see how this version ensures that NATIVE_SYSTEM_HEADER_DIR is
properly sysrooted. Note there's add_sysrooted_prefix separate from
add_prefix (but that's *not* the correct thing to use here because it uses
target_sysroot_suffix whereas you need target_sysroot_hdrs_suffix).

I address that in updated version of the patch.

However, this version seems to make TOOL_INCLUDE_DIR sysrooted as well.
I don't think that's correct; TOOL_INCLUDE_DIR ($prefix/$target/include,
roughly) is a non-sysroot location for headers. Note that it's not
sysrooted in cppdefault.c, which is a good guide to which directories
should or should not be sysrooted, and what order they should come in
(though as discussed, various of the directories there are not relevant
for the present issue).
The patch appears to be against some tree other than current trunk. At
least, it shows a function find_fortran_preinclude_file in gcc.c as
already existing in the diff context, but I see no such function in the
current sources.

I've just installed a prerequisite patch and now you should be able
to apply the patch on top of current trunk.

Thanks,
Martin

[Martin Liška]

Post by Martin Liška

Post by Joseph Myers

Post by Martin Liška

Post by Joseph Myers
I don't see how this version ensures that NATIVE_SYSTEM_HEADER_DIR is
properly sysrooted. Note there's add_sysrooted_prefix separate from
add_prefix (but that's *not* the correct thing to use here because it uses
target_sysroot_suffix whereas you need target_sysroot_hdrs_suffix).

I address that in updated version of the patch.

However, this version seems to make TOOL_INCLUDE_DIR sysrooted as well.
I don't think that's correct; TOOL_INCLUDE_DIR ($prefix/$target/include,
roughly) is a non-sysroot location for headers. Note that it's not
sysrooted in cppdefault.c, which is a good guide to which directories
should or should not be sysrooted, and what order they should come in
(though as discussed, various of the directories there are not relevant
for the present issue).
The patch appears to be against some tree other than current trunk. At
least, it shows a function find_fortran_preinclude_file in gcc.c as
already existing in the diff context, but I see no such function in the
current sources.

I've just installed a prerequisite patch and now you should be able
to apply the patch on top of current trunk.
Thanks,
Martin

Hi Joseph.

About this: I'll be away for next 3 weeks and I'm planning to return to
this once I'm back. If you find a spare cycles and help me with the
location which should be searched in find_fortran_preinclude_file, I would
be happy ;)

Thanks,
Martin

[Bernhard Reutner-Fischer]

Post by Jakub Jelinek

Post by Bernhard Reutner-Fischer
Can we use plain -include like in C?

Wouldn't that be confusing whether it is included in preprocessor only or if
it is included as a magic fortran include line at the beginning?

Yes, of course. Forgot that its a cpp argument.
So -fpre-include is fine.
Thanks,

[Thomas Koenig]

Hi Richi,

Post by Richard Biener
So if we'd instead do sth like the C -include command-line
option and have glibc just produce a textual file with
!GCC$ attributes lines then would that work when being
pre-"included" to each fortran source? That would even
solve the issue of having multiple .mod variants for
several GCC versions...

Yes, I think this would work.

We would have to make sure that the attribute lines work before
the first translation unit (so no INTRINSIC statement
can be there).

Regards

Thomas

[Janne Blomqvist]

Post by Martin Liška
Hi.
Unlike the C and C++ front-ends, GNU Fortran does not know about
vector implementations of math routines provided by GLIBC. This
prevents vectorization of many loops which is a frequent cause of
performance that is worse than compilers with their own math library
such as ICC.
The purpose of the patch is to provide a mechanism that will tell Fotran FE
which intrinsics have simd cloned in math library.
I've been cooperating with Paul and we came to a proof-of-concept that consists
The first patch adds support for inclusion a module via
command line. The module will be provided by glibc in order to synchronize which functions
are provided by a glibc version. Paul is suggesting to maybe include that
into load machinery
of IEEE module.

Would this module be provided by glibc in source form, and included, or
would glibc provide a .mod file?

The problem is that the gfortran module format tends to change for every
release, so the glibc provided module would then only be compatible with
the version glibc was compiled with. I think we should try to avoid such a
situation.

Also, I suppose it would also be possible to use this same mechanism to
provide a similar module for svml, or other vector math libs? If not
shipped as part of gcc, or said math library, maybe as a separate 3rd party
project?

[Richard Biener]

On Wed, Oct 31, 2018 at 9:29 AM Janne Blomqvist

Post by Martin Liška
Hi.
Unlike the C and C++ front-ends, GNU Fortran does not know about
vector implementations of math routines provided by GLIBC. This
prevents vectorization of many loops which is a frequent cause of
performance that is worse than compilers with their own math library
such as ICC.
The purpose of the patch is to provide a mechanism that will tell Fotran FE
which intrinsics have simd cloned in math library.
I've been cooperating with Paul and we came to a proof-of-concept that consists
The first patch adds support for inclusion a module via
command line. The module will be provided by glibc in order to synchronize which functions
are provided by a glibc version. Paul is suggesting to maybe include that into load machinery
of IEEE module.

Would this module be provided by glibc in source form, and included, or would glibc provide a .mod file?

I think glibc will provide the module in source form and it's build
machinery or distributors build machinery
will likely build a .mod file. An alternative variant would be to
provide the GCC build with a path to the
module source and build that into a GCC specific path (like where the
omp module sits).

The problem is that the gfortran module format tends to change for every release, so the glibc provided module would then only be compatible with the version glibc was compiled with. I think we should try to avoid such a situation.

Yes, of course.

Also, I suppose it would also be possible to use this same mechanism to provide a similar module for svml, or other vector math libs? If not shipped as part of gcc, or said math library, maybe as a separate 3rd party project?

I'm not sure that would work unless they provide the functions with
"standard" mangling for OpenMP SIMD.
If they do then yes.

Richard.

--
Janne Blomqvist