Commonize handling of attr-fnspec

Hi,
this patch adds the simple class for parsing fnspec attribute.  I plan
to add support for generating and modifying it too (it is used by
fortran and I plan to make modref to detect noclobbers and stuff).
Verification is disabled until we fix remaining fortran specifier
(I got a promised help at monday)

Bootstrapped/regtested x86_64-linux, OK?

Honza

gcc/ChangeLog:

2020-10-01  Jan Hubicka  <hubicka@ucw.cz>

	* calls.c (decl_return_flags): Implement using attr_fnspec.
	* gimple.c (gimple_call_arg_flags): Likewise
	(gimple_call_return_flags): Likewise
	* tree-into-ssa.c (pass_build_ssa::execute): Likewise.
	* tree-ssa-alias.c (attr_fnspec::verify): New
	* attr-fnspec.h: New file.

On Thu, 1 Oct 2020, Jan Hubicka wrote:

> Hi,
> this patch adds the simple class for parsing fnspec attribute.  I plan
> to add support for generating and modifying it too (it is used by
> fortran and I plan to make modref to detect noclobbers and stuff).
> Verification is disabled until we fix remaining fortran specifier
> (I got a promised help at monday)
> 
> Bootstrapped/regtested x86_64-linux, OK?

Well, besides many errors, see below...

> Honza
> 
> gcc/ChangeLog:
> 
> 2020-10-01  Jan Hubicka  <hubicka@ucw.cz>
> 
> 	* calls.c (decl_return_flags): Implement using attr_fnspec.
> 	* gimple.c (gimple_call_arg_flags): Likewise
> 	(gimple_call_return_flags): Likewise
> 	* tree-into-ssa.c (pass_build_ssa::execute): Likewise.
> 	* tree-ssa-alias.c (attr_fnspec::verify): New
> 	* attr-fnspec.h: New file.
> 
> diff --git a/gcc/calls.c b/gcc/calls.c
> index ed4363811c8..94f433685b9 100644
> --- a/gcc/calls.c
> +++ b/gcc/calls.c
> @@ -58,6 +58,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "attribs.h"
>  #include "builtins.h"
>  #include "gimple-fold.h"
> +#include "attr-fnspec.h"
>  
>  #include "tree-pretty-print.h"
>  
> @@ -642,25 +643,14 @@ decl_return_flags (tree fndecl)
>    if (!attr)
>      return 0;
>  
> -  attr = TREE_VALUE (TREE_VALUE (attr));
> -  if (!attr || TREE_STRING_LENGTH (attr) < 1)
> -    return 0;
> -
> -  switch (TREE_STRING_POINTER (attr)[0])
> -    {
> -    case '1':
> -    case '2':
> -    case '3':
> -    case '4':
> -      return ERF_RETURNS_ARG | (TREE_STRING_POINTER (attr)[0] - '1');
> +  attr_fnspec fnspec (TREE_VALUE (TREE_VALUE (attr)));
>  
> -    case 'm':
> -      return ERF_NOALIAS;
> +  if (fnspec.returns_arg () >= 0)
> +    return ERF_RETURNS_ARG | fnspec.returns_arg ();
>  
> -    case '.':
> -    default:
> -      return 0;
> -    }
> +  if (fnspec.returns_noalias_p ())
> +    return ERF_NOALIAS;
> +  return 0;
>  }
>  
>  /* Return nonzero when FNDECL represents a call to setjmp.  */
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index fd4e0fac0d4..2f2db309df5 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "attribs.h"
>  #include "asan.h"
>  #include "langhooks.h"
> +#include "attr-fnspec.h"
>  
>  
>  /* All the tuples have their operand vector (if present) at the very bottom
> @@ -1508,31 +1509,26 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
>  {
>    const_tree attr = gimple_call_fnspec (stmt);
>  
> -  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
> +  if (!attr)
>      return 0;
>  
> -  switch (TREE_STRING_POINTER (attr)[1 + arg])
> -    {
> -    case 'x':
> -    case 'X':
> -      return EAF_UNUSED;
> -
> -    case 'R':
> -      return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> -
> -    case 'r':
> -      return EAF_NOCLOBBER | EAF_NOESCAPE;
> -
> -    case 'W':
> -      return EAF_DIRECT | EAF_NOESCAPE;
> -
> -    case 'w':
> -      return EAF_NOESCAPE;
> +  int flags = 0;
> +  attr_fnspec fnspec (attr);
>  
> -    case '.':
> -    default:
> -      return 0;
> +  if (!fnspec.arg_specified_p (arg))
> +    ;
> +  else if (!fnspec.arg_used_p (arg))
> +    flags = EAF_UNUSED;
> +  else
> +    {
> +      if (!fnspec.arg_direct_p (arg))

negated test

> +	flags |= EAF_DIRECT;
> +      if (!fnspec.arg_noescape_p (arg))
> +	flags |= EAF_NOESCAPE;

likewise.

> +      if (!fnspec.arg_readonly_p (arg))
> +	flags |= EAF_NOCLOBBER;


likewise.

>      }
> +  return flags;
>  }
>  
>  /* Detects return flags for the call STMT.  */
> @@ -1546,24 +1542,16 @@ gimple_call_return_flags (const gcall *stmt)
>      return ERF_NOALIAS;
>  
>    attr = gimple_call_fnspec (stmt);
> -  if (!attr || TREE_STRING_LENGTH (attr) < 1)
> +  if (!attr)
>      return 0;
> +  attr_fnspec fnspec (attr);
>  
> -  switch (TREE_STRING_POINTER (attr)[0])
> -    {
> -    case '1':
> -    case '2':
> -    case '3':
> -    case '4':
> -      return ERF_RETURNS_ARG | (TREE_STRING_POINTER (attr)[0] - '1');
> -
> -    case 'm':
> -      return ERF_NOALIAS;
> +  if (fnspec.returns_arg () >= 0)
> +    return ERF_RETURNS_ARG | fnspec.returns_arg ();

hmm, maybe

     if (fnspec.returns_arg_p (&arg))
       return ERF_RETURNS_ARG | arg;

?

>  
> -    case '.':
> -    default:
> -      return 0;
> -    }
> +  if (fnspec.returns_noalias_p ())
> +    return ERF_NOALIAS;
> +  return 0;
>  }
>  
>  
> diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
> index 0d016134774..1493b323956 100644
> --- a/gcc/tree-into-ssa.c
> +++ b/gcc/tree-into-ssa.c
> @@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "stringpool.h"
>  #include "attribs.h"
>  #include "asan.h"
> +#include "attr-fnspec.h"
>  
>  #define PERCENT(x,y) ((float)(x) * 100.0 / (float)(y))
>  
> @@ -2492,19 +2493,19 @@ pass_build_ssa::execute (function *fun)
>      }
>  
>    /* Initialize SSA_NAME_POINTS_TO_READONLY_MEMORY.  */
> -  tree fnspec = lookup_attribute ("fn spec",
> -				  TYPE_ATTRIBUTES (TREE_TYPE (fun->decl)));
> -  if (fnspec)
> +  tree fnspec_tree
> +	 = lookup_attribute ("fn spec",
> +			     TYPE_ATTRIBUTES (TREE_TYPE (fun->decl)));
> +  if (fnspec_tree)
>      {
> -      fnspec = TREE_VALUE (TREE_VALUE (fnspec));
> -      unsigned i = 1;
> +      attr_fnspec fnspec (TREE_VALUE (TREE_VALUE (fnspec_tree)));
> +      unsigned i = 0;
>        for (tree arg = DECL_ARGUMENTS (cfun->decl);
>  	   arg; arg = DECL_CHAIN (arg), ++i)
>  	{
> -	  if (i >= (unsigned) TREE_STRING_LENGTH (fnspec))
> -	    break;
> -	  if (TREE_STRING_POINTER (fnspec)[i]  == 'R'
> -	      || TREE_STRING_POINTER (fnspec)[i] == 'r')
> +	  if (!fnspec.arg_specified_p (i))
> +	   break;
> +	  if (fnspec.arg_readonly_p (i))
>  	    {
>  	      tree name = ssa_default_def (fun, arg);
>  	      if (name)
> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> index fe390d4ffbe..2b2d53ebb88 100644
> --- a/gcc/tree-ssa-alias.c
> +++ b/gcc/tree-ssa-alias.c
> @@ -40,6 +40,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "varasm.h"
>  #include "ipa-modref-tree.h"
>  #include "ipa-modref.h"
> +#include "attr-fnspec.h"
> +#include "errors.h"
>  
>  /* Broad overview of how alias analysis on gimple works:
>  
> @@ -3984,3 +3985,42 @@ walk_aliased_vdefs (ao_ref *ref, tree vdef,
>    return ret;
>  }
>  
> +/* Verify validity of the fnspec string.
> +   See attr-fnspec.h for details.  */
> +
> +void
> +attr_fnspec::verify ()
> +{
> +  /* FIXME: Fortran trans-decl.c contains multiple wrong fnspec strings.
> +     re-enable verification after these are fixed.  */
> +  return;
> +  bool err = false;
> +
> +  /* Check return value specifier.  */
> +  if (len < return_desc_size)
> +    err = true;
> +  else if ((str[0] < '1' || str[0] > '4')
> +	   && str[0] != '.' && str[0] != 'm')
> +    err = true;
> +
> +  /* Now check all parameters.  */
> +  for (unsigned int i = 0; arg_specified_p (i); i++)
> +    {
> +      unsigned int idx = arg_idx (i);
> +      switch (str[idx])
> +	{
> +	  case 'x':
> +	  case 'X':
> +	  case 'r':
> +	  case 'R':
> +	  case 'w':
> +	  case 'W':
> +	  case '.':
> +	    break;
> +	  default:
> +	    err = true;
> +	}
> +    }
> +  if (err)
> +    internal_error ("invalid fn spec attribute %s", str);
> +}
> --- attr-fnspec.h	2020-10-01 14:39:32.565476448 +0200
> +++ attr-fnspec.h	2020-10-01 14:35:02.069814323 +0200
> @@ -0,0 +1,150 @@
> +/* Handling of fnspec attribute specifiers
> +   Copyright (C) 2008-2020 Free Software Foundation, Inc.
> +   Contributed by Richard Guenther  <rguenther@suse.de>
> +
> +   This file is part of GCC.
> +
> +   GCC is free software; you can redistribute it and/or modify
> +   under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   GCC is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with GCC; see the file COPYING3.  If not see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +/* Parse string of attribute "fn spec".  This is an internal attribute
> +   describing side effects of a function as follows:
> +
> +   character 0  specifies properties of return values as follows:
> +     '1'...'4'  specifies number of argument function returns (as in memset)
> +     'm'	specifies that returned value is noalias (as in malloc)
> +     '.'	specifies that nothing is known.
> +
> +   character 1+i specifies properties of argument number i as follows:
> +     'x' or 'X' specifies that parameter is unused.
> +     'r' or 'R' specifies that parameter is only read and memory pointed to is
> +		never dereferenced.
> +     'w' or 'W' specifies that parameter is only written to.
> +     '.'	specifies that nothing is known.
> +   The uppercase letter in addition specifies that parameter
> +   is non-escaping.  */
> +
> +#ifndef ATTR_FNSPEC_H
> +#define ATTR_FNSPEC_H
> +
> +class attr_fnspec
> +{
> +private:
> +  /* fn spec attribute string.  */
> +  const char *str;
> +  /* length of the fn spec string.  */
> +  const unsigned len;
> +  /* Number of characters specifying return value.  */
> +  const unsigned int return_desc_size = 1;
> +  /* Number of characters specifying size.  */
> +  const unsigned int arg_desc_size = 1;
> +
> +  /* Return start of specifier of arg i.  */
> +  unsigned int arg_idx (int i)
> +  {
> +    return return_desc_size + arg_desc_size * i;
> +  }
> +
> +public:
> +  attr_fnspec (const char *str, unsigned len)
> +  : str (str), len (len)
> +  {
> +    if (flag_checking)
> +      verify ();
> +  }
> +  attr_fnspec (const_tree identifier)
> +  : str (TREE_STRING_POINTER (identifier)),
> +    len (TREE_STRING_LENGTH (identifier))
> +  {
> +    if (flag_checking)
> +      verify ();
> +  }
> +
> +  /* Return true if arg I is specified.  */
> +  bool
> +  arg_specified_p (unsigned int i)
> +  {
> +    return len >= arg_idx (i + 1);
> +  }
> +
> +  /* True if the argument is not dereferenced recursively, thus only
> +     directly reachable memory is read or written.  */
> +  bool
> +  arg_direct_p (unsigned int i)
> +  {
> +    unsigned int idx = arg_idx (i);
> +    gcc_checking_assert (arg_specified_p (i));
> +    return str[idx] == 'R' || str[idx] == 'W';
> +  }
> +
> +  /* True if argument is used.  */
> +  bool
> +  arg_used_p (unsigned int i)
> +  {
> +    unsigned int idx = arg_idx (i);
> +    gcc_checking_assert (arg_specified_p (i));
> +    return str[idx] != 'x' && str[idx] != 'X';
> +  }
> +
> +  /* True if memory reached by the argument is readonly (not clobbered).  */
> +  bool
> +  arg_readonly_p (unsigned int i)
> +  {
> +    unsigned int idx = arg_idx (i);
> +    gcc_checking_assert (arg_specified_p (i));
> +    return str[idx] == 'r' || str[idx] == 'R';
> +  }
> +
> +  /* True if memory reached is only written into (but not read).  */
> +  bool
> +  arg_writeonly_p (unsigned int i)

This is actually arg_readwrite_p (), there's no flag for write-only.
wW are merely for noescape & only direct read/write.

> +  {
> +    unsigned int idx = arg_idx (i);
> +    gcc_checking_assert (arg_specified_p (i));
> +    return str[idx] == 'w' || str[idx] == 'W';
> +  }
> +
> +  /* True if the argument does not escape.  */
> +  bool
> +  arg_noescape_p (unsigned int i)
> +  {
> +    unsigned int idx = arg_idx (i);
> +    gcc_checking_assert (arg_specified_p (i));
> +    return str[idx] == 'w' || str[idx] == 'W'
> +	   || str[idx] == 'R' || str[idx] == 'r';
> +  }

Currently all specified args imply noescape btw.

> +  /* If not equal to -1 then it specifies number of argument returned by
> +     the function.  */
> +  int
> +  returns_arg ()
> +  {
> +    if (str[0] >= '1' && str[0] <= '4')
> +      return str[0]-'1';
> +    return -1;
> +  }
> +
> +  /* Nonzero if the return value does not alias with anything.  Functions
> +     with the malloc attribute have this set on their return value.  */
> +  int
> +  returns_noalias_p ()
> +  {
> +    return str[0] == 'm';
> +  }
> +
> +  /* Check validity of the string.  */
> +  void verify ();
> +};
> +
> +#endif /* ATTR_FNSPEC_H  */
>

Hi
> > +  if (!fnspec.arg_specified_p (arg))
> > +    ;
> > +  else if (!fnspec.arg_used_p (arg))
> > +    flags = EAF_UNUSED;
> > +  else
> > +    {
> > +      if (!fnspec.arg_direct_p (arg))
> 
> negated test
> 
> > +	flags |= EAF_DIRECT;
> > +      if (!fnspec.arg_noescape_p (arg))
> > +	flags |= EAF_NOESCAPE;
> 
> likewise.
> 
> > +      if (!fnspec.arg_readonly_p (arg))
> > +	flags |= EAF_NOCLOBBER;
> 
> 
> likewise.

Oops, sorry for that.  I got carried away trying to make sense of
fortran specifiers. Thanks for catching this. I wonder how that chould
have passed testing.
> 
> >      }
> > +  return flags;
> >  }
> >  
> >  /* Detects return flags for the call STMT.  */
> > @@ -1546,24 +1542,16 @@ gimple_call_return_flags (const gcall *stmt)
> >      return ERF_NOALIAS;
> >  
> >    attr = gimple_call_fnspec (stmt);
> > -  if (!attr || TREE_STRING_LENGTH (attr) < 1)
> > +  if (!attr)
> >      return 0;
> > +  attr_fnspec fnspec (attr);
> >  
> > -  switch (TREE_STRING_POINTER (attr)[0])
> > -    {
> > -    case '1':
> > -    case '2':
> > -    case '3':
> > -    case '4':
> > -      return ERF_RETURNS_ARG | (TREE_STRING_POINTER (attr)[0] - '1');
> > -
> > -    case 'm':
> > -      return ERF_NOALIAS;
> > +  if (fnspec.returns_arg () >= 0)
> > +    return ERF_RETURNS_ARG | fnspec.returns_arg ();
> 
> hmm, maybe
> 
>      if (fnspec.returns_arg_p (&arg))
>        return ERF_RETURNS_ARG | arg;

I added arg variable, but I think returning -1 for unknown arg is kind
of more consistent with what we do elsewhere (plus referneces are not
cool)
> > +  /* True if memory reached is only written into (but not read).  */
> > +  bool
> > +  arg_writeonly_p (unsigned int i)
> 
> This is actually arg_readwrite_p (), there's no flag for write-only.
> wW are merely for noescape & only direct read/write.

I dropped this for now, but for tree-ssa-alias we will need to have way
to specify that parameter is only written into.
> 
> Currently all specified args imply noescape btw.

Yep, but I think we may want to change this, so I think it is safer to
list those that do.

This is updated patch I am re-testing. Does it look OK?

Next I would like to proceed by blowing up all specifiers to double of
size (without functional changes) and then add the extra letters.

I was wondering if we want to use ' ' instead of '.' for the second
char.  It may make it easier to read ". . . R " than "......R." but it
also may be bit misleading in a way that the there must be precisely one
space.

Honza

gcc/ChangeLog:

2020-10-01  Jan Hubicka  <hubicka@ucw.cz>

	* attr-fnspec.h: New file.
	* calls.c (decl_return_flags): Implement using attr_fnspec.
	* gimple.c (gimple_call_arg_flags): Likewise
	(gimple_call_return_flags): Likewise
	* tree-into-ssa.c (pass_build_ssa::execute): Likewise.
	* tree-ssa-alias.c (attr_fnspec::verify): New

diff --git a/gcc/attr-fnspec.h b/gcc/attr-fnspec.h
new file mode 100644
index 00000000000..4ad4b8758e0
--- /dev/null
+++ b/gcc/attr-fnspec.h
@@ -0,0 +1,141 @@
+/* Handling of fnspec attribute specifiers
+   Copyright (C) 2008-2020 Free Software Foundation, Inc.
+   Contributed by Richard Guenther  <rguenther@suse.de>
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   GCC is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+/* Parse string of attribute "fn spec".  This is an internal attribute
+   describing side effects of a function as follows:
+
+   character 0  specifies properties of return values as follows:
+     '1'...'4'  specifies number of argument function returns (as in memset)
+     'm'	specifies that returned value is noalias (as in malloc)
+     '.'	specifies that nothing is known.
+
+   character 1+i specifies properties of argument number i as follows:
+     'x' or 'X' specifies that parameter is unused.
+     'r' or 'R' specifies that parameter is only read and memory pointed to is
+		never dereferenced.
+     'w' or 'W' specifies that parameter is only written to.
+     '.'	specifies that nothing is known.
+   The uppercase letter in addition specifies that parameter
+   is non-escaping.  */
+
+#ifndef ATTR_FNSPEC_H
+#define ATTR_FNSPEC_H
+
+class attr_fnspec
+{
+private:
+  /* fn spec attribute string.  */
+  const char *str;
+  /* length of the fn spec string.  */
+  const unsigned len;
+  /* Number of characters specifying return value.  */
+  const unsigned int return_desc_size = 1;
+  /* Number of characters specifying size.  */
+  const unsigned int arg_desc_size = 1;
+
+  /* Return start of specifier of arg i.  */
+  unsigned int arg_idx (int i)
+  {
+    return return_desc_size + arg_desc_size * i;
+  }
+
+public:
+  attr_fnspec (const char *str, unsigned len)
+  : str (str), len (len)
+  {
+    if (flag_checking)
+      verify ();
+  }
+  attr_fnspec (const_tree identifier)
+  : str (TREE_STRING_POINTER (identifier)),
+    len (TREE_STRING_LENGTH (identifier))
+  {
+    if (flag_checking)
+      verify ();
+  }
+
+  /* Return true if arg I is specified.  */
+  bool
+  arg_specified_p (unsigned int i)
+  {
+    return len >= arg_idx (i + 1);
+  }
+
+  /* True if the argument is not dereferenced recursively, thus only
+     directly reachable memory is read or written.  */
+  bool
+  arg_direct_p (unsigned int i)
+  {
+    unsigned int idx = arg_idx (i);
+    gcc_checking_assert (arg_specified_p (i));
+    return str[idx] == 'R' || str[idx] == 'W';
+  }
+
+  /* True if argument is used.  */
+  bool
+  arg_used_p (unsigned int i)
+  {
+    unsigned int idx = arg_idx (i);
+    gcc_checking_assert (arg_specified_p (i));
+    return str[idx] != 'x' && str[idx] != 'X';
+  }
+
+  /* True if memory reached by the argument is readonly (not clobbered).  */
+  bool
+  arg_readonly_p (unsigned int i)
+  {
+    unsigned int idx = arg_idx (i);
+    gcc_checking_assert (arg_specified_p (i));
+    return str[idx] == 'r' || str[idx] == 'R';
+  }
+
+  /* True if the argument does not escape.  */
+  bool
+  arg_noescape_p (unsigned int i)
+  {
+    unsigned int idx = arg_idx (i);
+    gcc_checking_assert (arg_specified_p (i));
+    return str[idx] == 'w' || str[idx] == 'W'
+	   || str[idx] == 'R' || str[idx] == 'r';
+  }
+
+  /* If not equal to -1 then it specifies number of argument returned by
+     the function.  */
+  int
+  returns_arg ()
+  {
+    if (str[0] >= '1' && str[0] <= '4')
+      return str[0]-'1';
+    return -1;
+  }
+
+  /* Nonzero if the return value does not alias with anything.  Functions
+     with the malloc attribute have this set on their return value.  */
+  int
+  returns_noalias_p ()
+  {
+    return str[0] == 'm';
+  }
+
+  /* Check validity of the string.  */
+  void verify ();
+};
+
+#endif /* ATTR_FNSPEC_H  */
diff --git a/gcc/calls.c b/gcc/calls.c
index ed4363811c8..40f4863a89b 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -58,6 +58,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "attribs.h"
 #include "builtins.h"
 #include "gimple-fold.h"
+#include "attr-fnspec.h"
 
 #include "tree-pretty-print.h"
 
@@ -642,25 +643,15 @@ decl_return_flags (tree fndecl)
   if (!attr)
     return 0;
 
-  attr = TREE_VALUE (TREE_VALUE (attr));
-  if (!attr || TREE_STRING_LENGTH (attr) < 1)
-    return 0;
-
-  switch (TREE_STRING_POINTER (attr)[0])
-    {
-    case '1':
-    case '2':
-    case '3':
-    case '4':
-      return ERF_RETURNS_ARG | (TREE_STRING_POINTER (attr)[0] - '1');
+  attr_fnspec fnspec (TREE_VALUE (TREE_VALUE (attr)));
 
-    case 'm':
-      return ERF_NOALIAS;
+  int arg = fnspec.returns_arg ();
+  if (arg >= 0)
+    return ERF_RETURNS_ARG | arg;
 
-    case '.':
-    default:
-      return 0;
-    }
+  if (fnspec.returns_noalias_p ())
+    return ERF_NOALIAS;
+  return 0;
 }
 
 /* Return nonzero when FNDECL represents a call to setjmp.  */
diff --git a/gcc/gimple.c b/gcc/gimple.c
index fd4e0fac0d4..8fb82570374 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "attribs.h"
 #include "asan.h"
 #include "langhooks.h"
+#include "attr-fnspec.h"
 
 
 /* All the tuples have their operand vector (if present) at the very bottom
@@ -1508,31 +1509,26 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
 {
   const_tree attr = gimple_call_fnspec (stmt);
 
-  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
+  if (!attr)
     return 0;
 
-  switch (TREE_STRING_POINTER (attr)[1 + arg])
-    {
-    case 'x':
-    case 'X':
-      return EAF_UNUSED;
-
-    case 'R':
-      return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
-
-    case 'r':
-      return EAF_NOCLOBBER | EAF_NOESCAPE;
-
-    case 'W':
-      return EAF_DIRECT | EAF_NOESCAPE;
-
-    case 'w':
-      return EAF_NOESCAPE;
+  int flags = 0;
+  attr_fnspec fnspec (attr);
 
-    case '.':
-    default:
-      return 0;
+  if (!fnspec.arg_specified_p (arg))
+    ;
+  else if (!fnspec.arg_used_p (arg))
+    flags = EAF_UNUSED;
+  else
+    {
+      if (fnspec.arg_direct_p (arg))
+	flags |= EAF_DIRECT;
+      if (fnspec.arg_noescape_p (arg))
+	flags |= EAF_NOESCAPE;
+      if (fnspec.arg_readonly_p (arg))
+	flags |= EAF_NOCLOBBER;
     }
+  return flags;
 }
 
 /* Detects return flags for the call STMT.  */
@@ -1546,24 +1542,16 @@ gimple_call_return_flags (const gcall *stmt)
     return ERF_NOALIAS;
 
   attr = gimple_call_fnspec (stmt);
-  if (!attr || TREE_STRING_LENGTH (attr) < 1)
+  if (!attr)
     return 0;
+  attr_fnspec fnspec (attr);
 
-  switch (TREE_STRING_POINTER (attr)[0])
-    {
-    case '1':
-    case '2':
-    case '3':
-    case '4':
-      return ERF_RETURNS_ARG | (TREE_STRING_POINTER (attr)[0] - '1');
-
-    case 'm':
-      return ERF_NOALIAS;
+  if (fnspec.returns_arg () >= 0)
+    return ERF_RETURNS_ARG | fnspec.returns_arg ();
 
-    case '.':
-    default:
-      return 0;
-    }
+  if (fnspec.returns_noalias_p ())
+    return ERF_NOALIAS;
+  return 0;
 }
 
 
diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
index 0d016134774..1493b323956 100644
--- a/gcc/tree-into-ssa.c
+++ b/gcc/tree-into-ssa.c
@@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "stringpool.h"
 #include "attribs.h"
 #include "asan.h"
+#include "attr-fnspec.h"
 
 #define PERCENT(x,y) ((float)(x) * 100.0 / (float)(y))
 
@@ -2492,19 +2493,19 @@ pass_build_ssa::execute (function *fun)
     }
 
   /* Initialize SSA_NAME_POINTS_TO_READONLY_MEMORY.  */
-  tree fnspec = lookup_attribute ("fn spec",
-				  TYPE_ATTRIBUTES (TREE_TYPE (fun->decl)));
-  if (fnspec)
+  tree fnspec_tree
+	 = lookup_attribute ("fn spec",
+			     TYPE_ATTRIBUTES (TREE_TYPE (fun->decl)));
+  if (fnspec_tree)
     {
-      fnspec = TREE_VALUE (TREE_VALUE (fnspec));
-      unsigned i = 1;
+      attr_fnspec fnspec (TREE_VALUE (TREE_VALUE (fnspec_tree)));
+      unsigned i = 0;
       for (tree arg = DECL_ARGUMENTS (cfun->decl);
 	   arg; arg = DECL_CHAIN (arg), ++i)
 	{
-	  if (i >= (unsigned) TREE_STRING_LENGTH (fnspec))
-	    break;
-	  if (TREE_STRING_POINTER (fnspec)[i]  == 'R'
-	      || TREE_STRING_POINTER (fnspec)[i] == 'r')
+	  if (!fnspec.arg_specified_p (i))
+	   break;
+	  if (fnspec.arg_readonly_p (i))
 	    {
 	      tree name = ssa_default_def (fun, arg);
 	      if (name)
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index fe390d4ffbe..2b2d53ebb88 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "varasm.h"
 #include "ipa-modref-tree.h"
 #include "ipa-modref.h"
+#include "attr-fnspec.h"
 
 /* Broad overview of how alias analysis on gimple works:
 
@@ -3984,3 +3985,42 @@ walk_aliased_vdefs (ao_ref *ref, tree vdef,
   return ret;
 }
 
+/* Verify validity of the fnspec string.
+   See attr-fnspec.h for details.  */
+
+void
+attr_fnspec::verify ()
+{
+  /* FIXME: Fortran trans-decl.c contains multiple wrong fnspec strings.
+     re-enable verification after these are fixed.  */
+  return;
+  bool err = false;
+
+  /* Check return value specifier.  */
+  if (len < return_desc_size)
+    err = true;
+  else if (str[0] < '1' || str[0] > '4')
+	   && str[0] != '.' && str[0] != 'm')
+    err = true;
+
+  /* Now check all parameters.  */
+  for (unsigned int i = 0; arg_specified_p (i); i++)
+    {
+      unsigned int idx = arg_idx (i);
+      switch (str[idx])
+	{
+	  case 'x':
+	  case 'X':
+	  case 'r':
+	  case 'R':
+	  case 'w':
+	  case 'W':
+	  case '.':
+	    break;
+	  default:
+	    err = true;
+	}
+    }
+  if (err)
+    internal_error ("invalid fn spec attribute %s", str);
+}

On Thu, 1 Oct 2020, Jan Hubicka wrote:

> Hi
> > > +  if (!fnspec.arg_specified_p (arg))
> > > +    ;
> > > +  else if (!fnspec.arg_used_p (arg))
> > > +    flags = EAF_UNUSED;
> > > +  else
> > > +    {
> > > +      if (!fnspec.arg_direct_p (arg))
> > 
> > negated test
> > 
> > > +	flags |= EAF_DIRECT;
> > > +      if (!fnspec.arg_noescape_p (arg))
> > > +	flags |= EAF_NOESCAPE;
> > 
> > likewise.
> > 
> > > +      if (!fnspec.arg_readonly_p (arg))
> > > +	flags |= EAF_NOCLOBBER;
> > 
> > 
> > likewise.
> 
> Oops, sorry for that.  I got carried away trying to make sense of
> fortran specifiers. Thanks for catching this. I wonder how that chould
> have passed testing.
> > 
> > >      }
> > > +  return flags;
> > >  }
> > >  
> > >  /* Detects return flags for the call STMT.  */
> > > @@ -1546,24 +1542,16 @@ gimple_call_return_flags (const gcall *stmt)
> > >      return ERF_NOALIAS;
> > >  
> > >    attr = gimple_call_fnspec (stmt);
> > > -  if (!attr || TREE_STRING_LENGTH (attr) < 1)
> > > +  if (!attr)
> > >      return 0;
> > > +  attr_fnspec fnspec (attr);
> > >  
> > > -  switch (TREE_STRING_POINTER (attr)[0])
> > > -    {
> > > -    case '1':
> > > -    case '2':
> > > -    case '3':
> > > -    case '4':
> > > -      return ERF_RETURNS_ARG | (TREE_STRING_POINTER (attr)[0] - '1');
> > > -
> > > -    case 'm':
> > > -      return ERF_NOALIAS;
> > > +  if (fnspec.returns_arg () >= 0)
> > > +    return ERF_RETURNS_ARG | fnspec.returns_arg ();
> > 
> > hmm, maybe
> > 
> >      if (fnspec.returns_arg_p (&arg))
> >        return ERF_RETURNS_ARG | arg;
> 
> I added arg variable, but I think returning -1 for unknown arg is kind
> of more consistent with what we do elsewhere (plus referneces are not
> cool)
> > > +  /* True if memory reached is only written into (but not read).  */
> > > +  bool
> > > +  arg_writeonly_p (unsigned int i)
> > 
> > This is actually arg_readwrite_p (), there's no flag for write-only.
> > wW are merely for noescape & only direct read/write.
> 
> I dropped this for now, but for tree-ssa-alias we will need to have way
> to specify that parameter is only written into.
> > 
> > Currently all specified args imply noescape btw.
> 
> Yep, but I think we may want to change this, so I think it is safer to
> list those that do.
> 
> This is updated patch I am re-testing. Does it look OK?
> 
> Next I would like to proceed by blowing up all specifiers to double of
> size (without functional changes) and then add the extra letters.
> 
> I was wondering if we want to use ' ' instead of '.' for the second
> char.  It may make it easier to read ". . . R " than "......R." but it
> also may be bit misleading in a way that the there must be precisely one
> space.
> 
> Honza
> 
> gcc/ChangeLog:
> 
> 2020-10-01  Jan Hubicka  <hubicka@ucw.cz>
> 
> 	* attr-fnspec.h: New file.
> 	* calls.c (decl_return_flags): Implement using attr_fnspec.
> 	* gimple.c (gimple_call_arg_flags): Likewise
> 	(gimple_call_return_flags): Likewise
> 	* tree-into-ssa.c (pass_build_ssa::execute): Likewise.
> 	* tree-ssa-alias.c (attr_fnspec::verify): New
> 
> diff --git a/gcc/attr-fnspec.h b/gcc/attr-fnspec.h
> new file mode 100644
> index 00000000000..4ad4b8758e0
> --- /dev/null
> +++ b/gcc/attr-fnspec.h
> @@ -0,0 +1,141 @@
> +/* Handling of fnspec attribute specifiers
> +   Copyright (C) 2008-2020 Free Software Foundation, Inc.
> +   Contributed by Richard Guenther  <rguenther@suse.de>
> +
> +   This file is part of GCC.
> +
> +   GCC is free software; you can redistribute it and/or modify
> +   under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   GCC is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with GCC; see the file COPYING3.  If not see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +/* Parse string of attribute "fn spec".  This is an internal attribute
> +   describing side effects of a function as follows:
> +
> +   character 0  specifies properties of return values as follows:
> +     '1'...'4'  specifies number of argument function returns (as in memset)
> +     'm'	specifies that returned value is noalias (as in malloc)
> +     '.'	specifies that nothing is known.
> +
> +   character 1+i specifies properties of argument number i as follows:
> +     'x' or 'X' specifies that parameter is unused.
> +     'r' or 'R' specifies that parameter is only read and memory pointed to is
> +		never dereferenced.
> +     'w' or 'W' specifies that parameter is only written to.
> +     '.'	specifies that nothing is known.
> +   The uppercase letter in addition specifies that parameter
> +   is non-escaping.  */
> +
> +#ifndef ATTR_FNSPEC_H
> +#define ATTR_FNSPEC_H
> +
> +class attr_fnspec
> +{
> +private:
> +  /* fn spec attribute string.  */
> +  const char *str;
> +  /* length of the fn spec string.  */
> +  const unsigned len;
> +  /* Number of characters specifying return value.  */
> +  const unsigned int return_desc_size = 1;
> +  /* Number of characters specifying size.  */
> +  const unsigned int arg_desc_size = 1;
> +
> +  /* Return start of specifier of arg i.  */
> +  unsigned int arg_idx (int i)
> +  {
> +    return return_desc_size + arg_desc_size * i;
> +  }
> +
> +public:
> +  attr_fnspec (const char *str, unsigned len)
> +  : str (str), len (len)
> +  {
> +    if (flag_checking)
> +      verify ();
> +  }
> +  attr_fnspec (const_tree identifier)
> +  : str (TREE_STRING_POINTER (identifier)),
> +    len (TREE_STRING_LENGTH (identifier))
> +  {
> +    if (flag_checking)
> +      verify ();
> +  }
> +
> +  /* Return true if arg I is specified.  */
> +  bool
> +  arg_specified_p (unsigned int i)
> +  {
> +    return len >= arg_idx (i + 1);
> +  }
> +
> +  /* True if the argument is not dereferenced recursively, thus only
> +     directly reachable memory is read or written.  */
> +  bool
> +  arg_direct_p (unsigned int i)

maybe better arg_direct_only_p, you differ with
arg_readonly_p from EAF_NOCLOBBER as well.  It's all just
names of course.

> +  {
> +    unsigned int idx = arg_idx (i);
> +    gcc_checking_assert (arg_specified_p (i));
> +    return str[idx] == 'R' || str[idx] == 'W';
> +  }
> +
> +  /* True if argument is used.  */
> +  bool
> +  arg_used_p (unsigned int i)
> +  {
> +    unsigned int idx = arg_idx (i);
> +    gcc_checking_assert (arg_specified_p (i));
> +    return str[idx] != 'x' && str[idx] != 'X';
> +  }

Hmm, I guess !arg_used_p also implies arg_noescape_p (not "correctly"
handled by gimple_call_arg_flag currently either).  So the question
is what "used"/"unused" means - does it mean the argument is
'inspected'?  That is, can we have an argument escape but not
read or written to?

Guess this can be sorted out in a followup, your patch doesn't
change current beahvior.

> +  /* True if memory reached by the argument is readonly (not clobbered).  */
> +  bool
> +  arg_readonly_p (unsigned int i)
> +  {
> +    unsigned int idx = arg_idx (i);
> +    gcc_checking_assert (arg_specified_p (i));
> +    return str[idx] == 'r' || str[idx] == 'R';
> +  }
> +
> +  /* True if the argument does not escape.  */
> +  bool
> +  arg_noescape_p (unsigned int i)
> +  {
> +    unsigned int idx = arg_idx (i);
> +    gcc_checking_assert (arg_specified_p (i));
> +    return str[idx] == 'w' || str[idx] == 'W'
> +	   || str[idx] == 'R' || str[idx] == 'r';
> +  }
> +
> +  /* If not equal to -1 then it specifies number of argument returned by
> +     the function.  */
> +  int
> +  returns_arg ()
> +  {
> +    if (str[0] >= '1' && str[0] <= '4')
> +      return str[0]-'1';
> +    return -1;
> +  }
> +
> +  /* Nonzero if the return value does not alias with anything.  Functions
> +     with the malloc attribute have this set on their return value.  */
> +  int

bool?

OK with this change at least.

Richard.

> +  returns_noalias_p ()
> +  {
> +    return str[0] == 'm';
> +  }
> +
> +  /* Check validity of the string.  */
> +  void verify ();
> +};
> +
> +#endif /* ATTR_FNSPEC_H  */
> diff --git a/gcc/calls.c b/gcc/calls.c
> index ed4363811c8..40f4863a89b 100644
> --- a/gcc/calls.c
> +++ b/gcc/calls.c
> @@ -58,6 +58,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "attribs.h"
>  #include "builtins.h"
>  #include "gimple-fold.h"
> +#include "attr-fnspec.h"
>  
>  #include "tree-pretty-print.h"
>  
> @@ -642,25 +643,15 @@ decl_return_flags (tree fndecl)
>    if (!attr)
>      return 0;
>  
> -  attr = TREE_VALUE (TREE_VALUE (attr));
> -  if (!attr || TREE_STRING_LENGTH (attr) < 1)
> -    return 0;
> -
> -  switch (TREE_STRING_POINTER (attr)[0])
> -    {
> -    case '1':
> -    case '2':
> -    case '3':
> -    case '4':
> -      return ERF_RETURNS_ARG | (TREE_STRING_POINTER (attr)[0] - '1');
> +  attr_fnspec fnspec (TREE_VALUE (TREE_VALUE (attr)));
>  
> -    case 'm':
> -      return ERF_NOALIAS;
> +  int arg = fnspec.returns_arg ();
> +  if (arg >= 0)
> +    return ERF_RETURNS_ARG | arg;
>  
> -    case '.':
> -    default:
> -      return 0;
> -    }
> +  if (fnspec.returns_noalias_p ())
> +    return ERF_NOALIAS;
> +  return 0;
>  }
>  
>  /* Return nonzero when FNDECL represents a call to setjmp.  */
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index fd4e0fac0d4..8fb82570374 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "attribs.h"
>  #include "asan.h"
>  #include "langhooks.h"
> +#include "attr-fnspec.h"
>  
>  
>  /* All the tuples have their operand vector (if present) at the very bottom
> @@ -1508,31 +1509,26 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
>  {
>    const_tree attr = gimple_call_fnspec (stmt);
>  
> -  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
> +  if (!attr)
>      return 0;
>  
> -  switch (TREE_STRING_POINTER (attr)[1 + arg])
> -    {
> -    case 'x':
> -    case 'X':
> -      return EAF_UNUSED;
> -
> -    case 'R':
> -      return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> -
> -    case 'r':
> -      return EAF_NOCLOBBER | EAF_NOESCAPE;
> -
> -    case 'W':
> -      return EAF_DIRECT | EAF_NOESCAPE;
> -
> -    case 'w':
> -      return EAF_NOESCAPE;
> +  int flags = 0;
> +  attr_fnspec fnspec (attr);
>  
> -    case '.':
> -    default:
> -      return 0;
> +  if (!fnspec.arg_specified_p (arg))
> +    ;
> +  else if (!fnspec.arg_used_p (arg))
> +    flags = EAF_UNUSED;
> +  else
> +    {
> +      if (fnspec.arg_direct_p (arg))
> +	flags |= EAF_DIRECT;
> +      if (fnspec.arg_noescape_p (arg))
> +	flags |= EAF_NOESCAPE;
> +      if (fnspec.arg_readonly_p (arg))
> +	flags |= EAF_NOCLOBBER;
>      }
> +  return flags;
>  }
>  
>  /* Detects return flags for the call STMT.  */
> @@ -1546,24 +1542,16 @@ gimple_call_return_flags (const gcall *stmt)
>      return ERF_NOALIAS;
>  
>    attr = gimple_call_fnspec (stmt);
> -  if (!attr || TREE_STRING_LENGTH (attr) < 1)
> +  if (!attr)
>      return 0;
> +  attr_fnspec fnspec (attr);
>  
> -  switch (TREE_STRING_POINTER (attr)[0])
> -    {
> -    case '1':
> -    case '2':
> -    case '3':
> -    case '4':
> -      return ERF_RETURNS_ARG | (TREE_STRING_POINTER (attr)[0] - '1');
> -
> -    case 'm':
> -      return ERF_NOALIAS;
> +  if (fnspec.returns_arg () >= 0)
> +    return ERF_RETURNS_ARG | fnspec.returns_arg ();
>  
> -    case '.':
> -    default:
> -      return 0;
> -    }
> +  if (fnspec.returns_noalias_p ())
> +    return ERF_NOALIAS;
> +  return 0;
>  }
>  
>  
> diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
> index 0d016134774..1493b323956 100644
> --- a/gcc/tree-into-ssa.c
> +++ b/gcc/tree-into-ssa.c
> @@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "stringpool.h"
>  #include "attribs.h"
>  #include "asan.h"
> +#include "attr-fnspec.h"
>  
>  #define PERCENT(x,y) ((float)(x) * 100.0 / (float)(y))
>  
> @@ -2492,19 +2493,19 @@ pass_build_ssa::execute (function *fun)
>      }
>  
>    /* Initialize SSA_NAME_POINTS_TO_READONLY_MEMORY.  */
> -  tree fnspec = lookup_attribute ("fn spec",
> -				  TYPE_ATTRIBUTES (TREE_TYPE (fun->decl)));
> -  if (fnspec)
> +  tree fnspec_tree
> +	 = lookup_attribute ("fn spec",
> +			     TYPE_ATTRIBUTES (TREE_TYPE (fun->decl)));
> +  if (fnspec_tree)
>      {
> -      fnspec = TREE_VALUE (TREE_VALUE (fnspec));
> -      unsigned i = 1;
> +      attr_fnspec fnspec (TREE_VALUE (TREE_VALUE (fnspec_tree)));
> +      unsigned i = 0;
>        for (tree arg = DECL_ARGUMENTS (cfun->decl);
>  	   arg; arg = DECL_CHAIN (arg), ++i)
>  	{
> -	  if (i >= (unsigned) TREE_STRING_LENGTH (fnspec))
> -	    break;
> -	  if (TREE_STRING_POINTER (fnspec)[i]  == 'R'
> -	      || TREE_STRING_POINTER (fnspec)[i] == 'r')
> +	  if (!fnspec.arg_specified_p (i))
> +	   break;
> +	  if (fnspec.arg_readonly_p (i))
>  	    {
>  	      tree name = ssa_default_def (fun, arg);
>  	      if (name)
> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> index fe390d4ffbe..2b2d53ebb88 100644
> --- a/gcc/tree-ssa-alias.c
> +++ b/gcc/tree-ssa-alias.c
> @@ -40,6 +40,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "varasm.h"
>  #include "ipa-modref-tree.h"
>  #include "ipa-modref.h"
> +#include "attr-fnspec.h"
>  
>  /* Broad overview of how alias analysis on gimple works:
>  
> @@ -3984,3 +3985,42 @@ walk_aliased_vdefs (ao_ref *ref, tree vdef,
>    return ret;
>  }
>  
> +/* Verify validity of the fnspec string.
> +   See attr-fnspec.h for details.  */
> +
> +void
> +attr_fnspec::verify ()
> +{
> +  /* FIXME: Fortran trans-decl.c contains multiple wrong fnspec strings.
> +     re-enable verification after these are fixed.  */
> +  return;
> +  bool err = false;
> +
> +  /* Check return value specifier.  */
> +  if (len < return_desc_size)
> +    err = true;
> +  else if (str[0] < '1' || str[0] > '4')
> +	   && str[0] != '.' && str[0] != 'm')
> +    err = true;
> +
> +  /* Now check all parameters.  */
> +  for (unsigned int i = 0; arg_specified_p (i); i++)
> +    {
> +      unsigned int idx = arg_idx (i);
> +      switch (str[idx])
> +	{
> +	  case 'x':
> +	  case 'X':
> +	  case 'r':
> +	  case 'R':
> +	  case 'w':
> +	  case 'W':
> +	  case '.':
> +	    break;
> +	  default:
> +	    err = true;
> +	}
> +    }
> +  if (err)
> +    internal_error ("invalid fn spec attribute %s", str);
> +}
>