| Message ID | 20221110102031.1366016-2-aldot@gcc.gnu.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fortran: Add attribute flatten | expand |
Hello, Le 10/11/2022 à 11:20, Bernhard Reutner-Fischer via Fortran a écrit : > Tiny cleanup opportunity since we now have ext_attr_args in > struct symbol_attribute. > Bootstrapped and regtested on x86_64-unknown-linux with no new > regressions. > Ok for trunk if the prerequisite was approved ([PATCH 2/2] Fortran: add > attribute target_clones) ? > > gcc/fortran/ChangeLog: > > * gfortran.h (struct ext_attr_t): Remove middle_end_name. > * trans-decl.cc (add_attributes_to_decl): Move building > tree_list to ... > * decl.cc (gfc_match_gcc_attributes): ... here. Add the attribute to > the tree_list for the middle end. > I prefer to not do any middle-end stuff at parsing time, so I would rather not do this change. Not OK.
On Mon, 21 Nov 2022 12:08:20 +0100 Mikael Morin <morin-mikael@orange.fr> wrote: > > * gfortran.h (struct ext_attr_t): Remove middle_end_name. > > * trans-decl.cc (add_attributes_to_decl): Move building > > tree_list to ... > > * decl.cc (gfc_match_gcc_attributes): ... here. Add the attribute to > > the tree_list for the middle end. > > > I prefer to not do any middle-end stuff at parsing time, so I would > rather not do this change. > Not OK. Ok, that means we should filter-out those bits that we don't want to write to the module (right?). We've plenty of bits left, more than Dave Love would want to have added, i hope, so that should not be much of a concern. What that table really wants to say is whether or not this attribute should be passed to the ME. Would it be acceptable to remove these duplicate strings and just have a bool/char/int that is true if it should be lowered (in trans-decl, as before)? But now i admit it's just bikeshedding and we can as well leave it alone for now.. It was just a though. thanks,
Le 21/11/2022 à 21:34, Bernhard Reutner-Fischer a écrit : > On Mon, 21 Nov 2022 12:08:20 +0100 > Mikael Morin <morin-mikael@orange.fr> wrote: > >>> * gfortran.h (struct ext_attr_t): Remove middle_end_name. >>> * trans-decl.cc (add_attributes_to_decl): Move building >>> tree_list to ... >>> * decl.cc (gfc_match_gcc_attributes): ... here. Add the attribute to >>> the tree_list for the middle end. >>> >> I prefer to not do any middle-end stuff at parsing time, so I would >> rather not do this change. >> Not OK. > > Ok, that means we should filter-out those bits that we don't want to > write to the module (right?). We've plenty of bits left, more than Dave > Love would want to have added, i hope, so that should not be much of a > concern. > I didn't think of modules. Yes, that means we have to store (in memory) the attribute we have parsed, and we can filter-out the attributes at the time the attributes are written to the module. I don't think it is strictly necessary (for flatten, at least) though. > What that table really wants to say is whether or not this attribute > should be passed to the ME. Would it be acceptable to remove these > duplicate strings and just have a bool/char/int that is true if it > should be lowered (in trans-decl, as before)? But now i admit it's just > bikeshedding and we can as well leave it alone for now.. It was just a > though. > Yes, that would be acceptable.
diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index 3a619dbdd34..d312d4812b6 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc @@ -11802,15 +11802,15 @@ gfc_match_gcc_attribute_args (bool require_string, bool allow_multiple) } const ext_attr_t ext_attr_list[] = { - { "dllimport", EXT_ATTR_DLLIMPORT, "dllimport" }, - { "dllexport", EXT_ATTR_DLLEXPORT, "dllexport" }, - { "cdecl", EXT_ATTR_CDECL, "cdecl" }, - { "stdcall", EXT_ATTR_STDCALL, "stdcall" }, - { "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 }, - { NULL, EXT_ATTR_LAST, NULL } + { "dllimport", EXT_ATTR_DLLIMPORT }, + { "dllexport", EXT_ATTR_DLLEXPORT }, + { "cdecl", EXT_ATTR_CDECL }, + { "stdcall", EXT_ATTR_STDCALL }, + { "fastcall", EXT_ATTR_FASTCALL, }, + { "no_arg_check", EXT_ATTR_NO_ARG_CHECK }, + { "deprecated", EXT_ATTR_DEPRECATED }, + { "target_clones",EXT_ATTR_TARGET_CLONES }, + { NULL, EXT_ATTR_LAST } }; /* Match a !GCC$ ATTRIBUTES statement of the form: @@ -11854,6 +11854,20 @@ gfc_match_gcc_attributes (void) gfc_error ("Unknown attribute in !GCC$ ATTRIBUTES statement at %C"); return MATCH_ERROR; } + + /* Check for errors. + If everything is fine, add attributes the middle-end has to know about. + */ + if (!gfc_add_ext_attribute (&attr, (ext_attr_id_t)id, &gfc_current_locus)) + return MATCH_ERROR; + else if (id == EXT_ATTR_DLLIMPORT + || id == EXT_ATTR_DLLEXPORT + || id == EXT_ATTR_CDECL + || id == EXT_ATTR_STDCALL + || id == EXT_ATTR_FASTCALL) + attr.ext_attr_args + = chainon (attr.ext_attr_args, + build_tree_list (get_identifier (name), NULL_TREE)); else if (id == EXT_ATTR_TARGET_CLONES) { attr_args @@ -11864,9 +11878,6 @@ gfc_match_gcc_attributes (void) build_tree_list (get_identifier (name), attr_args)); } - if (!gfc_add_ext_attribute (&attr, (ext_attr_id_t)id, &gfc_current_locus)) - return MATCH_ERROR; - gfc_gobble_whitespace (); ch = gfc_next_ascii_char (); if (ch == ':') diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index ce0cb61e647..c4deec0d5b8 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -847,7 +847,6 @@ typedef struct { const char *name; unsigned id; - const char *middle_end_name; } ext_attr_t; diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc index 24cbd4cda28..7d5d2bdbb37 100644 --- a/gcc/fortran/trans-decl.cc +++ b/gcc/fortran/trans-decl.cc @@ -1436,18 +1436,7 @@ gfc_add_assign_aux_vars (gfc_symbol * sym) static tree add_attributes_to_decl (symbol_attribute sym_attr, tree list) { - unsigned id; - tree attr; - - for (id = 0; id < EXT_ATTR_NUM; id++) - if (sym_attr.ext_attr & (1 << id) && ext_attr_list[id].middle_end_name) - { - attr = build_tree_list ( - get_identifier (ext_attr_list[id].middle_end_name), - NULL_TREE); - list = chainon (list, attr); - } - /* Add attribute args. */ + /* Add attributes and their arguments. */ if (sym_attr.ext_attr_args != NULL_TREE) list = chainon (list, sym_attr.ext_attr_args);