On Fri, Nov 16, 2018 at 02:24:42PM +0100, Martin Liška wrote:
> + 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
