[Patch, fortran] PR87477 - (associate) - [meta-bug] [F03] issues concerning the ASSOCIATE statement
Hi All, Three more fixes for PR87477. Please note that PR99350 was a blocker but, as pointed out in comment #5 of the PR, this has nothing to do with the associate construct. All three fixes are straight forward and the .diff + ChangeLog suffice to explain them. 'rankguessed' was made redundant by the last PR87477 fix. Regtests on x86_64 - good for mainline? Paul Fortran: Fix some more blockers in associate meta-bug [PR87477] 2023-06-07 Paul Thomas gcc/fortran PR fortran/99350 * decl.cc (char_len_param_value): Simplify a copy of the expr and replace the original if there is no error. * gfortran.h : Remove the redundant field 'rankguessed' from 'gfc_association_list'. * resolve.cc (resolve_assoc_var): Remove refs to 'rankguessed'. PR fortran/107281 * resolve.cc (resolve_variable): Associate names with constant or structure constructor targets cannot have array refs. PR fortran/109451 * trans-array.cc (gfc_conv_expr_descriptor): Guard expression character length backend decl before using it. Suppress the assignment if lhs equals rhs. * trans-io.cc (gfc_trans_transfer): Scalarize transfer of associate variables pointing to a variable. Add comment. * trans-stmt.cc (trans_associate_var): Remove requirement that the character length be deferred before assigning the value returned by gfc_conv_expr_descriptor. Also, guard the backend decl before testing with VAR_P. gcc/testsuite/ PR fortran/99350 * gfortran.dg/pr99350.f90 : New test. PR fortran/107281 * gfortran.dg/associate_5.f03 : Changed error message. * gfortran.dg/pr107281.f90 : New test. PR fortran/109451 * gfortran.dg/associate_61.f90 : New test diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index f5d39e2a3d8..d09c8bc97d9 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc @@ -1056,6 +1056,7 @@ static match char_len_param_value (gfc_expr **expr, bool *deferred) { match m; + gfc_expr *p; *expr = NULL; *deferred = false; @@ -1081,10 +1082,10 @@ char_len_param_value (gfc_expr **expr, bool *deferred) if (!gfc_expr_check_typed (*expr, gfc_current_ns, false)) return MATCH_ERROR; - /* If gfortran gets an EXPR_OP, try to simplify it. This catches things - like CHARACTER(([1])). */ - if ((*expr)->expr_type == EXPR_OP) -gfc_simplify_expr (*expr, 1); + /* Try to simplify the expression to catch things like CHARACTER(([1])). */ + p = gfc_copy_expr (*expr); + if (gfc_is_constant_expr (p) && gfc_simplify_expr (p, 1)) +gfc_replace_expr (*expr, p); if ((*expr)->expr_type == EXPR_FUNCTION) { diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 3e5f942d7fd..a65dd571591 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -2914,9 +2914,6 @@ typedef struct gfc_association_list for memory handling. */ unsigned dangling:1; - /* True when the rank of the target expression is guessed during parsing. */ - unsigned rankguessed:1; - char name[GFC_MAX_SYMBOL_LEN + 1]; gfc_symtree *st; /* Symtree corresponding to name. */ locus where; diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index 2ba3101f1fe..f2604314570 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc @@ -5872,7 +5872,15 @@ resolve_variable (gfc_expr *e) if (sym->ts.type == BT_CLASS) gfc_fix_class_refs (e); if (!sym->attr.dimension && e->ref && e->ref->type == REF_ARRAY) - return false; + { + /* Unambiguously scalar! */ + if (sym->assoc->target + && (sym->assoc->target->expr_type == EXPR_CONSTANT + || sym->assoc->target->expr_type == EXPR_STRUCTURE)) + gfc_error ("Scalar variable %qs has an array reference at %L", + sym->name, &e->where); + return false; + } else if (sym->attr.dimension && (!e->ref || e->ref->type != REF_ARRAY)) { /* This can happen because the parser did not detect that the @@ -9279,7 +9287,7 @@ resolve_assoc_var (gfc_symbol* sym, bool resolve_target) gfc_array_spec *as; /* The rank may be incorrectly guessed at parsing, therefore make sure it is corrected now. */ - if (sym->ts.type != BT_CLASS && (!sym->as || sym->assoc->rankguessed)) + if (sym->ts.type != BT_CLASS && !sym->as) { if (!sym->as) sym->as = gfc_get_array_spec (); @@ -9292,8 +9300,7 @@ resolve_assoc_var (gfc_symbol* sym, bool resolve_target) sym->attr.codimension = 1; } else if (sym->ts.type == BT_CLASS - && CLASS_DATA (sym) - && (!CLASS_DATA (sym)->as || sym->assoc->rankguessed)) + && CLASS_DATA (sym) && !CLASS_DATA (sym)->as) { if (!CLASS_DATA (sym)->as) CLASS_DATA (sym)->as = gfc_get_array_spec (); diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 1c7ea900ea1..e1c75e9fe02 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -7934,7 +7934,8 @@ gfc_conv_expr_descriptor (gfc_se *se, gfc_expr *expr) else tmp = se->string_length; - if (expr->ts.deferred && VAR_P (expr->ts.u.cl->b
Re: [PATCH] Fortran: add Fortran 2018 IEEE_{MIN,MAX} functions
Hi FX, On 6/6/23 21:11, FX Coudert via Gcc-patches wrote: Hi, I cannot see if there is proper support for kind=17 in your patch; at least the libgfortran/ieee/ieee_arithmetic.F90 part does not seem to have any related code. Can real(kind=17) ever be an IEEE mode? If so, something seriously wrong happened, because the IEEE modules have no kind=17 mention in them anywhere. Actually, where is the kind=17 documented? FX I was hoping for Thomas to come forward with some comment, as he was quite involved in related work. There are several threads on IEEE128 for Power on the fortran ML e.g. around November/December 2021, January 2022. I wasn't meaning to block your work, just wondering if the Power platform needs more attention here. Harald
Re: [Patch, fortran] PR87477 - (associate) - [meta-bug] [F03] issues concerning the ASSOCIATE statement
Hi Paul! On 6/7/23 18:10, Paul Richard Thomas via Gcc-patches wrote: Hi All, Three more fixes for PR87477. Please note that PR99350 was a blocker but, as pointed out in comment #5 of the PR, this has nothing to do with the associate construct. All three fixes are straight forward and the .diff + ChangeLog suffice to explain them. 'rankguessed' was made redundant by the last PR87477 fix. Regtests on x86_64 - good for mainline? Paul Fortran: Fix some more blockers in associate meta-bug [PR87477] 2023-06-07 Paul Thomas gcc/fortran PR fortran/99350 * decl.cc (char_len_param_value): Simplify a copy of the expr and replace the original if there is no error. This seems to lack a gfc_free_expr (p) in case the gfc_replace_expr is not executed, leading to a possible memleak. Can you check? @@ -1081,10 +1082,10 @@ char_len_param_value (gfc_expr **expr, bool *deferred) if (!gfc_expr_check_typed (*expr, gfc_current_ns, false)) return MATCH_ERROR; - /* If gfortran gets an EXPR_OP, try to simplify it. This catches things - like CHARACTER(([1])). */ - if ((*expr)->expr_type == EXPR_OP) -gfc_simplify_expr (*expr, 1); + /* Try to simplify the expression to catch things like CHARACTER(([1])). */ + p = gfc_copy_expr (*expr); + if (gfc_is_constant_expr (p) && gfc_simplify_expr (p, 1)) +gfc_replace_expr (*expr, p); else gfc_free_expr (p); * gfortran.h : Remove the redundant field 'rankguessed' from 'gfc_association_list'. * resolve.cc (resolve_assoc_var): Remove refs to 'rankguessed'. PR fortran/107281 * resolve.cc (resolve_variable): Associate names with constant or structure constructor targets cannot have array refs. PR fortran/109451 * trans-array.cc (gfc_conv_expr_descriptor): Guard expression character length backend decl before using it. Suppress the assignment if lhs equals rhs. * trans-io.cc (gfc_trans_transfer): Scalarize transfer of associate variables pointing to a variable. Add comment. * trans-stmt.cc (trans_associate_var): Remove requirement that the character length be deferred before assigning the value returned by gfc_conv_expr_descriptor. Also, guard the backend decl before testing with VAR_P. gcc/testsuite/ PR fortran/99350 * gfortran.dg/pr99350.f90 : New test. PR fortran/107281 * gfortran.dg/associate_5.f03 : Changed error message. * gfortran.dg/pr107281.f90 : New test. PR fortran/109451 * gfortran.dg/associate_61.f90 : New test Otherwise LGTM. Thanks for the patch! Harald
Re: [PATCH] Fortran: add Fortran 2018 IEEE_{MIN,MAX} functions
On Wed, Jun 07, 2023 at 08:31:35PM +0200, Harald Anlauf via Fortran wrote: > Hi FX, > > On 6/6/23 21:11, FX Coudert via Gcc-patches wrote: > > Hi, > > > > > I cannot see if there is proper support for kind=17 in your patch; > > > at least the libgfortran/ieee/ieee_arithmetic.F90 part does not > > > seem to have any related code. > > > > Can real(kind=17) ever be an IEEE mode? If so, something seriously wrong > > happened, because the IEEE modules have no kind=17 mention in them anywhere. > > > > Actually, where is the kind=17 documented? > > > > FX > > I was hoping for Thomas to come forward with some comment, as > he was quite involved in related work. > > There are several threads on IEEE128 for Power on the fortran ML > e.g. around November/December 2021, January 2022. > > I wasn't meaning to block your work, just wondering if the Power > platform needs more attention here. > % cd gcc/gccx/libgfortran % grep HAVE_GFC_REAL_17 ieee/* % troutmask:sgk[219] ls ieee % ieee_arithmetic.F90 ieee_features.F90 % ieee_exceptions.F90 ieee_helper.c There are zero hits for REAL(17) in the IEEE code. If REAL(17) is intended to be an IEEE-754 type, then it seems gfortran's support was never added for it. If anyone has access to a power system, it's easy to test program foo use ieee_arithmetic print *, ieee_support_datatype(1.e_17) end program foo -- Steve
Re: Possible funding of gfortran work
On 6/5/23 13:07, Andre Vehreschild wrote: > Hi Benson, > > thank you for your input. Comments are inline: > >> Maybe add Quantum Espresso: >> https://www.quantum-espresso.org/ > Another code: https://github.com/openmopac/mopac currently being supported by https://molssi.org/ > done > >> R and Octave may also be good examples of use cases. > > Mhhh, both are not written in Fortran, right? I don't feel tempted to > include other programming languages into the references list. It feels odd to > me. Any thoughts, anyone? > In addition to use of Lapack, many subroutines are written in Fortran. They have many users in a variety of sectors. Path to parallelization is unclear, even multicore parallelization will benefit many users. People in these projects may be willing to give input if asked. > >> Some gfortran work has been done as company sponsored in that >> individuals using the compiler needed it for company work and could work >> on the compiler on company time. If a large proportion is voluntary and >> companies only sponsor small extensions and bug fixes, one might assume >> that if the funding is given, once it is finished, the chances of >> further work will be very limited. Maybe one can tie into the GNU >> compiler collection as well, emphasizing the longevity of the project >> and usefulness of the funding in adding additional capabilities and >> cleaning up code contributions. Then indicate that new parts that this >> proposal addresses have primarily been voluntary because they are not >> yet ready for production use, and this project would make them ready for >> production use so that in future maintenance efforts can be made by the >> community (both voluntary and sponsored). > > I have added a paragraph about sponsoring of general gfortran work: > > GFortran in general stems from a merge of projects that have been supported by > academic research, commercial needs and in large parts volunteers. Funding by > companies was mostly done by allowing employees to work on features required > for > the company and donating the code. > > Is that what you were trying to add? > That seems good. Maybe something like: GFortran is a portion of the long lived GCC compiler suite and has gotten contributions due to academic research needs, commercial needs and volunteer interest. Industry funding primarily enables employees to work on features required by the company, for example to support new processors or ensure performance in a critical company code section. > Regards, > Andre > -- > Andre Vehreschild * Email: vehre ad gmx dot de
Re: [Patch, fortran] PR87477 - (associate) - [meta-bug] [F03] issues concerning the ASSOCIATE statement
Hi Harald, In answer to your question: void gfc_replace_expr (gfc_expr *dest, gfc_expr *src) { free_expr0 (dest); *dest = *src; free (src); } So it does indeed do the job. I should perhaps have remarked that, following the divide error, gfc_simplify_expr was returning a mutilated version of the expression and this was somehow connected with successfully simplifying the parentheses. Copying and replacing on no errors deals with the problem. Thanks Paul On Wed, 7 Jun 2023 at 19:38, Harald Anlauf wrote: > > Hi Paul! > > On 6/7/23 18:10, Paul Richard Thomas via Gcc-patches wrote: > > Hi All, > > > > Three more fixes for PR87477. Please note that PR99350 was a blocker > > but, as pointed out in comment #5 of the PR, this has nothing to do > > with the associate construct. > > > > All three fixes are straight forward and the .diff + ChangeLog suffice > > to explain them. 'rankguessed' was made redundant by the last PR87477 > > fix. > > > > Regtests on x86_64 - good for mainline? > > > > Paul > > > > Fortran: Fix some more blockers in associate meta-bug [PR87477] > > > > 2023-06-07 Paul Thomas > > > > gcc/fortran > > PR fortran/99350 > > * decl.cc (char_len_param_value): Simplify a copy of the expr > > and replace the original if there is no error. > > This seems to lack a gfc_free_expr (p) in case the gfc_replace_expr > is not executed, leading to a possible memleak. Can you check? > > @@ -1081,10 +1082,10 @@ char_len_param_value (gfc_expr **expr, bool > *deferred) > if (!gfc_expr_check_typed (*expr, gfc_current_ns, false)) > return MATCH_ERROR; > > - /* If gfortran gets an EXPR_OP, try to simplify it. This catches things > - like CHARACTER(([1])). */ > - if ((*expr)->expr_type == EXPR_OP) > -gfc_simplify_expr (*expr, 1); > + /* Try to simplify the expression to catch things like > CHARACTER(([1])). */ > + p = gfc_copy_expr (*expr); > + if (gfc_is_constant_expr (p) && gfc_simplify_expr (p, 1)) > +gfc_replace_expr (*expr, p); > else > gfc_free_expr (p); > > > * gfortran.h : Remove the redundant field 'rankguessed' from > > 'gfc_association_list'. > > * resolve.cc (resolve_assoc_var): Remove refs to 'rankguessed'. > > > > PR fortran/107281 > > * resolve.cc (resolve_variable): Associate names with constant > > or structure constructor targets cannot have array refs. > > > > PR fortran/109451 > > * trans-array.cc (gfc_conv_expr_descriptor): Guard expression > > character length backend decl before using it. Suppress the > > assignment if lhs equals rhs. > > * trans-io.cc (gfc_trans_transfer): Scalarize transfer of > > associate variables pointing to a variable. Add comment. > > * trans-stmt.cc (trans_associate_var): Remove requirement that > > the character length be deferred before assigning the value > > returned by gfc_conv_expr_descriptor. Also, guard the backend > > decl before testing with VAR_P. > > > > gcc/testsuite/ > > PR fortran/99350 > > * gfortran.dg/pr99350.f90 : New test. > > > > PR fortran/107281 > > * gfortran.dg/associate_5.f03 : Changed error message. > > * gfortran.dg/pr107281.f90 : New test. > > > > PR fortran/109451 > > * gfortran.dg/associate_61.f90 : New test > > Otherwise LGTM. > > Thanks for the patch! > > Harald > > -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein