Re: [PATCH 2/2] Fortran: add attribute target_clones
Le 21/11/2022 à 23:26, Bernhard Reutner-Fischer a écrit :
On Mon, 21 Nov 2022 20:13:40 +0100
Mikael Morin wrote:
Hello,
Le 09/11/2022 à 20:02, Bernhard Reutner-Fischer via Fortran a écrit :
Hi!
(...)
+ if (allow_multiple && gfc_match_char (')') != MATCH_YES)
+{
+ gfc_error ("expected ')' at %C");
+ return NULL_TREE;
+}
+
+ return attr_args;
+}
I'm not sure this function need to do all the parsing manually.
I would rather use gfc_match_actual_arglist, or maybe implement the
function as a wrapper around it.
What is allowed here? Are non-literal constants allowed, for example
parameter variables? Is line continuation supported ?
Line continuation is supported i think.
Parameter variables supposedly are or should not be supported. Why would
you do that in the context of an attribute target decl? > Either way, if the ME
does not find such an fndecl, it will complain
and ignore the attribute.
I don't understand non-literal constants in this context.
This very attribute applies to decls, so the existing code supposedly
matches a comma separated list of identifiers. The usual dollar-ok
caveats apply.
No, my comment and my questions were about your function, which, as I
understand it, matches the arguments to the attribute: it matches open
and closing parenthesis, double quotes, etc.
Matching of decl names comes after that.
I asked the question about non-literal constant (and the other as well),
because I saw it as a possible reason to not reuse the existing parsing
functions.
As to gfc_match_actual_arglist, probably.
target_clones has
+ { "target_clones", 1, -1, true, false, false, false,
+ dummy, NULL },
with tree-core.h struct attribute_spec, so
name, min=1, max=unbounded, decl_required=yes, ...ignore...
hence applies to functions and subroutines and the like. It does take an
unbounded list of strings, isa1, isa2, isa4, default. We could add
"default" unless seen, but i'd rather want it spelled out by the user
for the user is supposed to know what she's doing, as in c or c++.
The ME has code to sanity-check the attributes, including conflicting
(ME) attributes.
The reason why i contemplated with a separate parser was that for stuff
like regparm or sseregparm, you would want to require a single number
for the equivalent of
__attribute__((regparm(3),stdcall)
which you would provide in 2 separate !GCC$ attributes i assume.
Well, the check could as easily be enforced after parsing with the
existing parsing functions.
Nothing (bad) to say about the rest, but there is enough to change with
the above comments.
Yes, many thanks for your comments.
I think there is no other non-intrusive way to pass the data through the
frontend. So for an acceptable way this means touching quite some spots
for every single ME attribute anybody would like to add in the future.
I'm not sure I understand. Please let's just add what is necessary for
this attribute, not more.
Re: [PATCH 2/2] Fortran: add attribute target_clones
On Mon, 21 Nov 2022 20:13:40 +0100
Mikael Morin wrote:
> Hello,
>
> Le 09/11/2022 à 20:02, Bernhard Reutner-Fischer via Fortran a écrit :
> > Hi!
> >
> > Add support for attribute target_clones:
> > !GCC$ ATTRIBUTES target_clones("arch1", "arch3","default") :: mysubroutine
> > +/* Internal helper to parse attribute argument list.
> > + If REQUIRE_STRING is true, then require a string.
> > + If ALLOW_MULTIPLE is true, allow more than one arg.
> > + If multiple arguments are passed, require braces around them.
> > + Returns a tree_list of arguments or NULL_TREE. */
> > +static tree
> > +gfc_match_gcc_attribute_args (bool require_string, bool allow_multiple)
> > + do {
> > + } while (num_quotes % 2 && gfc_match_eos () != MATCH_YES);
> The do-while loops are wrongly indented.
> It should be:
>do
> {
>...
> }
>while (...)
oops, right.
> > + tree str = build_string (pos, name);
> > + /* Compare with c-family/c-common.cc: fix_string_type. */
> > + tree i_type = build_index_type (size_int (pos));
> > + tree a_type = build_array_type (char_type_node, i_type);
> > + TREE_TYPE (str) = a_type;
> > + TREE_READONLY (str) = 1;
> > + TREE_STATIC (str) = 1;
> > + attr_arg = build_tree_list (NULL_TREE, str);
> > + attr_args = chainon (attr_args, attr_arg);
> Same comment as for the flatten attribute:
> please no tree stuff out of the trans-*.cc files.
yes ok, noted. It's a pity in this context, where we purely pass a blob
on to the ME but ok.
> This includes gfortran.h, so the attribute arguments need to be carried
> around using the front-end structures (gfc_actual_arglist for example).
That's a sane rule of thumb, yes.
Usually, the parser deals with language grammar and not with pure
passthrough remarks, so that's fair. Not so much in the case of such
attribs but i see your point :)
> > + if (allow_multiple && gfc_match_char (')') != MATCH_YES)
> > +{
> > + gfc_error ("expected ')' at %C");
> > + return NULL_TREE;
> > +}
> > +
> > + return attr_args;
> > +}
> I'm not sure this function need to do all the parsing manually.
> I would rather use gfc_match_actual_arglist, or maybe implement the
> function as a wrapper around it.
> What is allowed here? Are non-literal constants allowed, for example
> parameter variables? Is line continuation supported ?
Line continuation is supported i think.
Parameter variables supposedly are or should not be supported. Why would
you do that in the context of an attribute target decl?
Either way, if the ME does not find such an fndecl, it will complain
and ignore the attribute.
I don't understand non-literal constants in this context.
This very attribute applies to decls, so the existing code supposedly
matches a comma separated list of identifiers. The usual dollar-ok
caveats apply.
As to gfc_match_actual_arglist, probably.
target_clones has
+ { "target_clones", 1, -1, true, false, false, false,
+ dummy, NULL },
with tree-core.h struct attribute_spec, so
name, min=1, max=unbounded, decl_required=yes, ...ignore...
hence applies to functions and subroutines and the like. It does take an
unbounded list of strings, isa1, isa2, isa4, default. We could add
"default" unless seen, but i'd rather want it spelled out by the user
for the user is supposed to know what she's doing, as in c or c++.
The ME has code to sanity-check the attributes, including conflicting
(ME) attributes.
The reason why i contemplated with a separate parser was that for stuff
like regparm or sseregparm, you would want to require a single number
for the equivalent of
__attribute__((regparm(3),stdcall)
which you would provide in 2 separate !GCC$ attributes i assume.
>
> Nothing (bad) to say about the rest, but there is enough to change with
> the above comments.
Yes, many thanks for your comments.
I think there is no other non-intrusive way to pass the data through the
frontend. So for an acceptable way this means touching quite some spots
for every single ME attribute anybody would like to add in the future.
But that's how it is.
Re: [PATCH 2/2] Fortran: add attribute target_clones
Hello,
Le 09/11/2022 à 20:02, Bernhard Reutner-Fischer via Fortran a écrit :
Hi!
Add support for attribute target_clones:
!GCC$ ATTRIBUTES target_clones("arch1", "arch3","default") :: mysubroutine
Bootstrapped and regtested on x86_64-unknown-linux with
--target_board=unix'{-m32,-m64}'.
OK for trunk?
gcc/fortran/ChangeLog:
* decl.cc: Include fold-const.h for size_int.
(gfc_match_gcc_attribute_args): New internal helper function.
(gfc_match_gcc_attributes): Handle target_clones.
* f95-lang.cc (struct attribute_spec): Add target and
target_clones entries.
* gfortran.h (ext_attr_id_t): Add EXT_ATTR_TARGET_CLONES.
(struct symbol_attribute): Add field ext_attr_args.
* trans-decl.cc (add_attributes_to_decl): Also add ext_attr_args
to the decl's attributes.
* gfortran.texi: Document attribute target_clones.
gcc/testsuite/ChangeLog:
* gfortran.dg/attr_target_clones-1.F90: New test.
Cc: gfortran ML
---
gcc/fortran/decl.cc | 104 ++
gcc/fortran/f95-lang.cc | 4 +
gcc/fortran/gfortran.h| 2 +
gcc/fortran/gfortran.texi | 31 ++
gcc/fortran/trans-decl.cc | 3 +
.../gfortran.dg/attr_target_clones-1.F90 | 30 +
6 files changed, 174 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/attr_target_clones-1.F90
diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc
index 0f9b2ced4c2..3a619dbdd34 100644
--- a/gcc/fortran/decl.cc
+++ b/gcc/fortran/decl.cc
(...)
@@ -11709,6 +11710,96 @@ gfc_match_final_decl (void)
return MATCH_YES;
}
+/* Internal helper to parse attribute argument list.
+ If REQUIRE_STRING is true, then require a string.
+ If ALLOW_MULTIPLE is true, allow more than one arg.
+ If multiple arguments are passed, require braces around them.
+ Returns a tree_list of arguments or NULL_TREE. */
+static tree
+gfc_match_gcc_attribute_args (bool require_string, bool allow_multiple)
+{
+ tree attr_args = NULL_TREE, attr_arg;
+ char name[GFC_MAX_SYMBOL_LEN + 1];
+ unsigned pos = 0;
+ gfc_char_t c;
+
+ /* When we get here, we already parsed
+ !GCC$ ATTRIBUTES ATTRIBUTE_NAME
+ Now parse the arguments. These could be one of
+ "single_string_literal"
+ ( "str_literal_1" , "str_literal_2" )
+ */
+
+ gfc_gobble_whitespace ();
+
+ if (allow_multiple && gfc_match_char ('(') != MATCH_YES)
+{
+ gfc_error ("expected '(' at %C");
+ return NULL_TREE;
+}
+
+ if (require_string)
+{
+ do {
+ if (pos)
+ {
+ if (!allow_multiple)
+ {
+ gfc_error ("surplus argument at %C");
+ return NULL_TREE;
+ }
+ gfc_next_ascii_char (); /* Consume the comma. */
+ }
+ pos = 0;
+ gfc_gobble_whitespace ();
+ unsigned char num_quotes = 0;
+ do {
+ c = gfc_next_char_literal (NONSTRING);
+ if (c == '"')
+ {
+ num_quotes++;
+ continue; /* Skip the quote */
+ }
+ name[pos++] = c;
+ if (pos >= GFC_MAX_SYMBOL_LEN)
+ {
+ gfc_error ("attribute argument truncated at %C");
+ return NULL_TREE;
+ }
+ } while (num_quotes % 2 && gfc_match_eos () != MATCH_YES);
The do-while loops are wrongly indented.
It should be:
do
{
...
}
while (...)
+ if (pos < 1)
+ {
+ gfc_error ("expected argument at %C");
+ return NULL_TREE;
+ }
+ if (num_quotes != 2)
+ {
+ gfc_error ("invalid string literal at %C");
+ return NULL_TREE;
+ }
+ name[pos] = '\0'; /* Redundant wrt build_string. */
+ tree str = build_string (pos, name);
+ /* Compare with c-family/c-common.cc: fix_string_type. */
+ tree i_type = build_index_type (size_int (pos));
+ tree a_type = build_array_type (char_type_node, i_type);
+ TREE_TYPE (str) = a_type;
+ TREE_READONLY (str) = 1;
+ TREE_STATIC (str) = 1;
+ attr_arg = build_tree_list (NULL_TREE, str);
+ attr_args = chainon (attr_args, attr_arg);
Same comment as for the flatten attribute:
please no tree stuff out of the trans-*.cc files.
This includes gfortran.h, so the attribute arguments need to be carried
around using the front-end structures (gfc_actual_arglist for example).
+
+ gfc_gobble_whitespace ();
+ } while (gfc_peek_ascii_char () == ',');
+}
+
+ if (allow_multiple && gfc_match_char (')') != MATCH_YES)
+{
+ gfc_error ("expected ')' at %C");
+ return NULL_TREE;
+}
+
+ return attr_args;
+}
I'm not sure this function need to do all the parsing manually.
I would rather use gfc_match_actual_arglist, or maybe implement the
function as a wrapper around it.
What is allowed here? Are
[PATCH 2/2] Fortran: add attribute target_clones
Hi!
Add support for attribute target_clones:
!GCC$ ATTRIBUTES target_clones("arch1", "arch3","default") :: mysubroutine
Bootstrapped and regtested on x86_64-unknown-linux with
--target_board=unix'{-m32,-m64}'.
OK for trunk?
gcc/fortran/ChangeLog:
* decl.cc: Include fold-const.h for size_int.
(gfc_match_gcc_attribute_args): New internal helper function.
(gfc_match_gcc_attributes): Handle target_clones.
* f95-lang.cc (struct attribute_spec): Add target and
target_clones entries.
* gfortran.h (ext_attr_id_t): Add EXT_ATTR_TARGET_CLONES.
(struct symbol_attribute): Add field ext_attr_args.
* trans-decl.cc (add_attributes_to_decl): Also add ext_attr_args
to the decl's attributes.
* gfortran.texi: Document attribute target_clones.
gcc/testsuite/ChangeLog:
* gfortran.dg/attr_target_clones-1.F90: New test.
Cc: gfortran ML
---
gcc/fortran/decl.cc | 104 ++
gcc/fortran/f95-lang.cc | 4 +
gcc/fortran/gfortran.h| 2 +
gcc/fortran/gfortran.texi | 31 ++
gcc/fortran/trans-decl.cc | 3 +
.../gfortran.dg/attr_target_clones-1.F90 | 30 +
6 files changed, 174 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/attr_target_clones-1.F90
diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc
index 0f9b2ced4c2..3a619dbdd34 100644
--- a/gcc/fortran/decl.cc
+++ b/gcc/fortran/decl.cc
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3. If not see
#include "coretypes.h"
#include "options.h"
#include "tree.h"
+#include "fold-const.h"
#include "gfortran.h"
#include "stringpool.h"
#include "match.h"
@@ -11709,6 +11710,96 @@ gfc_match_final_decl (void)
return MATCH_YES;
}
+/* Internal helper to parse attribute argument list.
+ If REQUIRE_STRING is true, then require a string.
+ If ALLOW_MULTIPLE is true, allow more than one arg.
+ If multiple arguments are passed, require braces around them.
+ Returns a tree_list of arguments or NULL_TREE. */
+static tree
+gfc_match_gcc_attribute_args (bool require_string, bool allow_multiple)
+{
+ tree attr_args = NULL_TREE, attr_arg;
+ char name[GFC_MAX_SYMBOL_LEN + 1];
+ unsigned pos = 0;
+ gfc_char_t c;
+
+ /* When we get here, we already parsed
+ !GCC$ ATTRIBUTES ATTRIBUTE_NAME
+ Now parse the arguments. These could be one of
+ "single_string_literal"
+ ( "str_literal_1" , "str_literal_2" )
+ */
+
+ gfc_gobble_whitespace ();
+
+ if (allow_multiple && gfc_match_char ('(') != MATCH_YES)
+{
+ gfc_error ("expected '(' at %C");
+ return NULL_TREE;
+}
+
+ if (require_string)
+{
+ do {
+ if (pos)
+ {
+ if (!allow_multiple)
+ {
+ gfc_error ("surplus argument at %C");
+ return NULL_TREE;
+ }
+ gfc_next_ascii_char (); /* Consume the comma. */
+ }
+ pos = 0;
+ gfc_gobble_whitespace ();
+ unsigned char num_quotes = 0;
+ do {
+ c = gfc_next_char_literal (NONSTRING);
+ if (c == '"')
+ {
+ num_quotes++;
+ continue; /* Skip the quote */
+ }
+ name[pos++] = c;
+ if (pos >= GFC_MAX_SYMBOL_LEN)
+ {
+ gfc_error ("attribute argument truncated at %C");
+ return NULL_TREE;
+ }
+ } while (num_quotes % 2 && gfc_match_eos () != MATCH_YES);
+ if (pos < 1)
+ {
+ gfc_error ("expected argument at %C");
+ return NULL_TREE;
+ }
+ if (num_quotes != 2)
+ {
+ gfc_error ("invalid string literal at %C");
+ return NULL_TREE;
+ }
+ name[pos] = '\0'; /* Redundant wrt build_string. */
+ tree str = build_string (pos, name);
+ /* Compare with c-family/c-common.cc: fix_string_type. */
+ tree i_type = build_index_type (size_int (pos));
+ tree a_type = build_array_type (char_type_node, i_type);
+ TREE_TYPE (str) = a_type;
+ TREE_READONLY (str) = 1;
+ TREE_STATIC (str) = 1;
+ attr_arg = build_tree_list (NULL_TREE, str);
+ attr_args = chainon (attr_args, attr_arg);
+
+ gfc_gobble_whitespace ();
+ } while (gfc_peek_ascii_char () == ',');
+}
+
+ if (allow_multiple && gfc_match_char (')') != MATCH_YES)
+{
+ gfc_error ("expected ')' at %C");
+ return NULL_TREE;
+}
+
+ return attr_args;
+}
const ext_attr_t ext_attr_list[] = {
{ "dllimport",EXT_ATTR_DLLIMPORT,"dllimport" },
@@ -11718,6 +11809,7 @@ const ext_attr_t ext_attr_list[] = {
{ "fastcall", EXT_ATTR_FASTCALL, "fastcall" },
{ "no_arg_check", EXT_ATTR_NO_ARG_CHECK, NULL},
{ "deprecated", EXT_ATTR_DEPRECATED, NULL },
+ { "target_clones",EXT_ATTR_TARGET_CLONES,NULL },
{
