[PATCH] Support simd function declarations via a pre-include.

[Martin Liška]

On 11/16/18 4:12 PM, Martin Liška wrote:
> On 11/16/18 2:49 PM, Jakub Jelinek wrote:
>> 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
>>
> 
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Support-simd-function-declarations-via-a-pre-include.patch
Type: text/x-patch
Size: 19480 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20181119/07bcf918/attachment.bin>