[PATCH 1/2] Fortran: Cleanup struct ext_attr_t
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. Cc: gfortran ML --- gcc/fortran/decl.cc | 35 +++ gcc/fortran/gfortran.h| 1 - gcc/fortran/trans-decl.cc | 13 + 3 files changed, 24 insertions(+), 25 deletions(-) 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); -- 2.38.1
[PATCH 0/2] Fortran: Add attribute flatten
Hi! I could imagine that the flatten attribute might be useful. Do we want to add support for it for gcc-13? Bernhard Reutner-Fischer (2): Fortran: Cleanup struct ext_attr_t Fortran: Add attribute flatten gcc/fortran/decl.cc | 41 +--- gcc/fortran/f95-lang.cc | 2 + gcc/fortran/gfortran.h | 1 - gcc/fortran/gfortran.texi| 8 gcc/fortran/trans-decl.cc| 13 +-- gcc/testsuite/gfortran.dg/attr_flatten-1.f90 | 41 6 files changed, 80 insertions(+), 26 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/attr_flatten-1.f90 -- 2.38.1
[PATCH 2/2] Fortran: Add attribute flatten
Bootstrapped and regtested cleanly on x86_unknown-linux. The document bits will be rewritten for rst. Ok for trunk if the prerequisite target_clones patch is approved? gcc/fortran/ChangeLog: * decl.cc (gfc_match_gcc_attributes): Handle flatten. * f95-lang.cc (gfc_attribute_table): Add flatten. * gfortran.texi: Document attribute flatten. gcc/testsuite/ChangeLog: * gfortran.dg/attr_flatten-1.f90: New test. --- gcc/fortran/decl.cc | 8 +++- gcc/fortran/f95-lang.cc | 2 + gcc/fortran/gfortran.texi| 8 gcc/testsuite/gfortran.dg/attr_flatten-1.f90 | 41 4 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/attr_flatten-1.f90 diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index d312d4812b6..3d210c26eb5 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc @@ -11841,6 +11841,7 @@ gfc_match_gcc_attributes (void) for(;;) { char ch; + bool known_attr0args = false; if (gfc_match_name (name) != MATCH_YES) return MATCH_ERROR; @@ -11849,7 +11850,9 @@ gfc_match_gcc_attributes (void) if (strcmp (name, ext_attr_list[id].name) == 0) break; - if (id == EXT_ATTR_LAST) + if (strcmp (name, "flatten") == 0) + known_attr0args = true; /* Handled below. We do not need a bit. */ + else if (id == EXT_ATTR_LAST) { gfc_error ("Unknown attribute in !GCC$ ATTRIBUTES statement at %C"); return MATCH_ERROR; @@ -11864,7 +11867,8 @@ gfc_match_gcc_attributes (void) || id == EXT_ATTR_DLLEXPORT || id == EXT_ATTR_CDECL || id == EXT_ATTR_STDCALL - || id == EXT_ATTR_FASTCALL) + || id == EXT_ATTR_FASTCALL + || known_attr0args) attr.ext_attr_args = chainon (attr.ext_attr_args, build_tree_list (get_identifier (name), NULL_TREE)); diff --git a/gcc/fortran/f95-lang.cc b/gcc/fortran/f95-lang.cc index 7154568aec5..ddb5b686cf6 100644 --- a/gcc/fortran/f95-lang.cc +++ b/gcc/fortran/f95-lang.cc @@ -101,6 +101,8 @@ static const struct attribute_spec gfc_attribute_table[] = gfc_handle_omp_declare_target_attribute, NULL }, { "target_clones", 1, -1, true, false, false, false, gfc_handle_omp_declare_target_attribute, NULL }, + { "flatten",0, 0, true, false, false, false, + gfc_handle_omp_declare_target_attribute, NULL }, { NULL,0, 0, false, false, false, false, NULL, NULL } }; diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi index 06e4c8c00a1..be650f28b62 100644 --- a/gcc/fortran/gfortran.texi +++ b/gcc/fortran/gfortran.texi @@ -3280,6 +3280,14 @@ contains end module mymod @end smallexample +@node flatten + +Procedures annotated with the @code{flatten} attribute have their +callees inlined, if possible. +Please refer to +@ref{Top,,Common Function Attributes,gcc,Using the GNU Compiler Collection (GCC)} +for details about the respective attribute. + The attributes are specified using the syntax @code{!GCC$ ATTRIBUTES} @var{attribute-list} @code{::} @var{variable-list} diff --git a/gcc/testsuite/gfortran.dg/attr_flatten-1.f90 b/gcc/testsuite/gfortran.dg/attr_flatten-1.f90 new file mode 100644 index 000..0b72f1ba17c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/attr_flatten-1.f90 @@ -0,0 +1,41 @@ +! { dg-do compile } +! { dg-additional-options "-fdump-tree-optimized" } +! Test __attribute__((flatten)) +! +module attr_flttn_1_a + implicit none +contains + subroutine sub1(i) +integer, intent(in) :: i +integer :: n +do n = 1, i + print *, "marker1 ", i, i+n; +enddo + end + subroutine sub2(i) +integer, intent(in) :: i +integer :: n +do n = 1, i + print *, "marker2 ", i, i*i-n; +enddo + end +end module +module attr_flttn_1_b + use attr_flttn_1_a +contains + subroutine sub3 +!GCC$ ATTRIBUTES flatten :: sub3 +print *, "marker3 " +call sub2(4711) +call sub1(42) + end +end module +! Without the attribute flatten we would have 1 character write for each marker. +! That would be 3 _gfortran_transfer_character_write.*marker +! With the attribute, we have one for each sub plus marker1 and marker2 +! which were inlined into sub3. +! So this gives 5 _gfortran_transfer_character_write.*marker +! and there should be no calls to sub1 (); nor sub2 (); +! { dg-final { scan-tree-dump-times { _gfortran_transfer_character_write .*?marker} 5 "optimized" } } +! { dg-final { scan-tree-dump-not { sub1 \([^\)][^\)]*\);} "optimized" } } +! { dg-final { scan-tree-dump-not { sub2 \([^\)][^\)]*\);} "optimized" } } -- 2.38.1
Re: adding attributes
Hi! On Mon, 07 Nov 2022 11:04:17 + Dave Love via Fortran wrote: > Bernhard Reutner-Fischer via Fortran writes: > > > I see. > > So target_clones is one thing. What other attributes would be important? > > At least optimization-related ones could be useful, and possibly others. > I haven't made a list, but could do. Please do. And yes, i can see that __attribute__((__optimize__(...))) would be useful. > dynamic dispatch in libraries. (The worst thing about gfortran for > system management is the lack of backwards-compatibility in module > formats and libgfortran.) yea. IIRC there was discussion a couple of years back what we could do about the module format. Nowadays i'd probably just use JSON, but i did not think too much about it. Needless to say that rewriting the mio (module I/O) would take more than one evening :) I remember some wrinkles there when i played around with the fortran-fe-stringpool idea (which reminds me i should pickup again, maybe). > > But since you cannot mix target_clones across arch-boundaries, > > supporting those for a distro will probably be rather ugly anyway. > > Yes, you need simple pre-processing, as you do for the attributes in C, > unless there was some extra guard facility added. Yes indeed. But this would be much easier to handle if we'd have actual arch defines. Until we have, you'd have to run this through cpp manually which is doable but not all that convenient IMHO. Hm, didn't we have a syntax for arch conditions in the math vec? We could hijack that, but it's probably still better to just fix the arch defines as that's generally useful. > > heh, me neither. Luckily yesterday was a holiday, so what i ended up > > with was the following, fya. > > Gosh; I thought it would take a while even if you knew your way around. > I didn't want to spoil a holiday! (I'd aim to do such things on work > time.) No problem, it was just for fun. I spent most of the time to scratch my head why the attribute didn't work for i had wrapped it in an arch ifdef for the testsuite to cover both i386 and ppc. And of course i only noticed very, very late what was really going on ;) > > I've added a > >/* Attributes set by compiler extensions (!GCC$ ATTRIBUTES). */ > >unsigned ext_attr:EXT_ATTR_NUM; > > + tree ext_attr_args; > > > > to struct symbol_attribute where i can prepare the tree_list for the > > attrs right from the start. The lowering is then rather simple and > > uniform, just chainon the prepared attributes and be done. > > If I understand correctly, I could go through and add ones that look > useful (for debate). I have experience of using several in C (at least > once even for g77 runtime). Yes please, that'd be interesting. > > target_clones does not require a bump in the module format, i'd say, > > because the main entry point does not change. Will have to check if > > the clones do not end up being emitted in the module, they shouldn't be. > > Other attributes _may_ require a change in the module format though. > > These would need checking on a per case basis. > > I don't understand the module format, but I wouldn't have expected > relevant attributes to change interfaces. Well that should probably not be needed indeed for most attributes, yes. But then, i do think we stream out the ext_attr, at least for certain attributes like "cdecl", the dll{im,ex}port, {std,fast}call et al. See module.cc, mio_symbol_attribute. So the bits that change the calling convention have to be brought to the attention of the module consumer probably. Think regparm or sseregparm for example i guess. That said, if i comment out the invalid cases of the test, i get with gcc-12: $ gfortran -c -o /tmp/out0.o /scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90 /scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90:33:15: 33 | cdecl => sub2 | 1 Warning: ‘cdecl’ attribute ignored [-Wattributes] /scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90:33:15: Warning: ‘cdecl’ attribute ignored [-Wattributes] /scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90:34:17: 34 | stdcall => sub3 | 1 Warning: ‘stdcall’ attribute ignored [-Wattributes] /scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90:34:17: Warning: ‘stdcall’ attribute ignored [-Wattributes] /scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90:35:18: 35 | fastcall => sub4 | 1 Warning: ‘fastcall’ attribute ignored [-Wattributes] /scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90:35:18: Warning: ‘fastcall’ attribute ignored [-Wattributes] so i'm not sure what these attributes do or are supposed to do really, but i didn't look. And in addition there is typo s/consitency/consistency/ in the test. > > Anyway, thanks! You're welcome. PS: You might have seen th
[PATCH] Fortran: fix treatment of character, value, optional dummy arguments [PR107444]
Dear Fortranners, the attached patch is a follow-up to the fix for PR107441, as it finally fixes the treatment of character dummy arguments that have the value,optional attribute, and allows for checking of the presence of such arguments. This entails a small ABI clarification, as the previous text was not really clear on the argument passing conventions, and the previously generated code was inconsistent at best, or rather wrong, for this kind of procedure arguments. (E.g. the number of passed arguments was varying...) Testcase cross-checked with NAG 7.1. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From d87e299dd2b7f4be6ca829e80cd94babc53fa12f Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 10 Nov 2022 22:30:27 +0100 Subject: [PATCH] Fortran: fix treatment of character, value, optional dummy arguments [PR107444] Fix handling of character dummy arguments that have the optional+value attribute. Change name of internal symbols that carry the hidden presence status of optional arguments to distinguish them from the internal hidden character length. Update documentation to clarify the gfortran ABI. gcc/fortran/ChangeLog: PR fortran/107444 * trans-decl.cc (create_function_arglist): Extend presence status to all intrinsic types, and change prefix of internal symbol to '.'. * trans-expr.cc (gfc_conv_expr_present): Align to changes in create_function_arglist. (gfc_conv_procedure_call): Fix generation of procedure arguments for the case of character dummy arguments with optional+value attribute. * trans-types.cc (gfc_get_function_type): Synchronize with changes to create_function_arglist. * doc/gfortran/naming-and-argument-passing-conventions.rst: Clarify the gfortran argument passing conventions with regard to OPTIONAL dummy arguments of intrinsic type. gcc/testsuite/ChangeLog: PR fortran/107444 * gfortran.dg/optional_absent_7.f90: Adjust regex. * gfortran.dg/optional_absent_8.f90: New test. --- ...aming-and-argument-passing-conventions.rst | 3 +- gcc/fortran/trans-decl.cc | 10 ++-- gcc/fortran/trans-expr.cc | 25 ++--- gcc/fortran/trans-types.cc| 14 ++--- .../gfortran.dg/optional_absent_7.f90 | 2 +- .../gfortran.dg/optional_absent_8.f90 | 53 +++ 6 files changed, 84 insertions(+), 23 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/optional_absent_8.f90 diff --git a/gcc/fortran/doc/gfortran/naming-and-argument-passing-conventions.rst b/gcc/fortran/doc/gfortran/naming-and-argument-passing-conventions.rst index 4baaee9bfec..fa999fac355 100644 --- a/gcc/fortran/doc/gfortran/naming-and-argument-passing-conventions.rst +++ b/gcc/fortran/doc/gfortran/naming-and-argument-passing-conventions.rst @@ -142,8 +142,7 @@ is used for dummy arguments; with ``VALUE``, those variables are passed by value. For ``OPTIONAL`` dummy arguments, an absent argument is denoted -by a NULL pointer, except for scalar dummy arguments of type -``INTEGER``, ``LOGICAL``, ``REAL`` and ``COMPLEX`` +by a NULL pointer, except for scalar dummy arguments of intrinsic type which have the ``VALUE`` attribute. For those, a hidden Boolean argument (``logical(kind=C_bool),value``) is used to indicate whether the argument is present. diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc index 94988b8690e..217de6b8da0 100644 --- a/gcc/fortran/trans-decl.cc +++ b/gcc/fortran/trans-decl.cc @@ -2708,16 +2708,16 @@ create_function_arglist (gfc_symbol * sym) type = gfc_sym_type (f->sym); } } - /* For noncharacter scalar intrinsic types, VALUE passes the value, + /* For scalar intrinsic types, VALUE passes the value, hence, the optional status cannot be transferred via a NULL pointer. Thus, we will use a hidden argument in that case. */ - else if (f->sym->attr.optional && f->sym->attr.value - && !f->sym->attr.dimension && f->sym->ts.type != BT_CLASS - && !gfc_bt_struct (f->sym->ts.type)) + if (f->sym->attr.optional && f->sym->attr.value + && !f->sym->attr.dimension && f->sym->ts.type != BT_CLASS + && !gfc_bt_struct (f->sym->ts.type)) { tree tmp; strcpy (&name[1], f->sym->name); - name[0] = '_'; + name[0] = '.'; tmp = build_decl (input_location, PARM_DECL, get_identifier (name), boolean_type_node); diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index f3fbb527157..b95c5cf2f96 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -1985,15 +1985,14 @@ gfc_conv_expr_present (gfc_symbol * sym, bool use_saved_desc) /* Intrinsic scalars with VALUE attribute which are passed by value use a hidden argument to denote the present status. */ - if (sym->attr.value && sym->ts.type != BT_CHARACTER - && sym->ts.type != BT_CLASS && sym->ts.type != BT_DERIVED - && !sym->attr.dimension) + if (sym->attr.value && !sym