| Message ID | 3d4ad562-5532-f4e1-0311-b708133c22ff@netcologne.de |
|---|---|
| State | New |
| Headers | show |
| Series | [fortran] Fix PR 96018, wrong code caused by implicit_pure | expand |
Ping? > the attached patch fixes a 9/10/11 regression where we left over > an implicit_pure attribute when a non-implicit_pure procedure was > called. > > The fix is explained in the ChangeLog.
Hi Thomas, The patch looks fine to me but I have two questions: (i) Why is this not done in resolve.c? (ii) Is Martin's reduced reproducer not the basis for a testcase? Regards Paul On Sat, 18 Jul 2020 at 11:58, Thomas Koenig via Fortran <fortran@gcc.gnu.org> wrote: > Ping? > > > the attached patch fixes a 9/10/11 regression where we left over > > an implicit_pure attribute when a non-implicit_pure procedure was > > called. > > > > The fix is explained in the ChangeLog. > > >
Hi Paul, > The patch looks fine to me but I have two questions: > > (i) Why is this not done in resolve.c? Of course it doesn't matter where the function resides :-) I put it into frontend-passes.c because it makes heavy use of gfc_code_walker, and out of habit. If you prefer, I can of course move the code to resolve.c. If your question is more like "why is that not done during normal resolution" - while fixing this bug, I began to understand why the standard has an explicit PURE attribute. If the compiler is chasing after a procedure which may or may not be implicit_pure, then everything else needs to have been resolved beforehand. And in normal resolution, something needs to be the last. > (ii) Is Martin's reduced reproducer not the basis for a testcase? There, git got the better of me. I thought it was included in the patch, but, as I reread it and found that this wasn't the case. I have attache the test case now. OK for trunk and backport once gcc 10 is open again? Regards Thomas
Hi Thomas, I am fine with this being in frontend-passes.c - it was just a question :-) resolve.c has become too large anyway. The testcase looks familiar! Don't forget to commit and push the additional source too. OK for 10-- and all the affected branches. Cheers Paul On Sat, 18 Jul 2020 at 18:57, Thomas Koenig <tkoenig@netcologne.de> wrote: > Hi Paul, > > > The patch looks fine to me but I have two questions: > > > > (i) Why is this not done in resolve.c? > > Of course it doesn't matter where the function resides :-) I put it > into frontend-passes.c because it makes heavy use of gfc_code_walker, > and out of habit. If you prefer, I can of course move the code to > resolve.c. > > If your question is more like "why is that not done during > normal resolution" - while fixing this bug, I began to understand > why the standard has an explicit PURE attribute. If the compiler > is chasing after a procedure which may or may not be implicit_pure, then > everything else needs to have been resolved beforehand. And in normal > resolution, something needs to be the last. > > > (ii) Is Martin's reduced reproducer not the basis for a testcase? > > There, git got the better of me. I thought it was included in the > patch, but, as I reread it and found that this wasn't the case. > > I have attache the test case now. > > OK for trunk and backport once gcc 10 is open again? > > Regards > > Thomas >
diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c index 69f9ca64c97..4b1d3f2feda 100644 --- a/gcc/fortran/frontend-passes.c +++ b/gcc/fortran/frontend-passes.c @@ -5550,7 +5550,8 @@ gfc_check_externals0 (gfc_namespace *ns) /* Called routine. */ -void gfc_check_externals (gfc_namespace *ns) +void +gfc_check_externals (gfc_namespace *ns) { gfc_clear_error (); @@ -5565,3 +5566,76 @@ void gfc_check_externals (gfc_namespace *ns) gfc_errors_to_warnings (false); } +/* Callback function. If there is a call to a subroutine which is + neither pure nor implicit_pure, unset the implicit_pure flag for + the caller and return -1. */ + +static int +implicit_pure_call (gfc_code **c, int *walk_subtrees ATTRIBUTE_UNUSED, + void *sym_data) +{ + gfc_code *co = *c; + gfc_symbol *caller_sym; + symbol_attribute *a; + + if (co->op != EXEC_CALL || co->resolved_sym == NULL) + return 0; + + a = &co->resolved_sym->attr; + if (a->intrinsic || a->pure || a->implicit_pure) + return 0; + + caller_sym = (gfc_symbol *) sym_data; + gfc_unset_implicit_pure (caller_sym); + return 1; +} + +/* Callback function. If there is a call to a function which is + neither pure nor implicit_pure, unset the implicit_pure flag for + the caller and return 1. */ + +static int +implicit_pure_expr (gfc_expr **e, int *walk ATTRIBUTE_UNUSED, void *sym_data) +{ + gfc_expr *expr = *e; + gfc_symbol *caller_sym; + gfc_symbol *sym; + symbol_attribute *a; + + if (expr->expr_type != EXPR_FUNCTION || expr->value.function.isym) + return 0; + + sym = expr->symtree->n.sym; + a = &sym->attr; + if (a->pure || a->implicit_pure) + return 0; + + caller_sym = (gfc_symbol *) sym_data; + gfc_unset_implicit_pure (caller_sym); + return 1; +} + +/* Go through all procedures in the namespace and unset the + implicit_pure attribute for any procedure that calls something not + pure or implicit pure. */ + +bool +gfc_fix_implicit_pure (gfc_namespace *ns) +{ + bool changed = false; + gfc_symbol *proc = ns->proc_name; + + if (proc && proc->attr.flavor == FL_PROCEDURE && proc->attr.implicit_pure + && ns->code + && gfc_code_walker (&ns->code, implicit_pure_call, implicit_pure_expr, + (void *) ns->proc_name)) + changed = true; + + for (ns = ns->contained; ns; ns = ns->sibling) + { + if (gfc_fix_implicit_pure (ns)) + changed = true; + } + + return changed; +} diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 24c5101c4cb..264822ef9f8 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -3623,6 +3623,7 @@ int gfc_expr_walker (gfc_expr **, walk_expr_fn_t, void *); int gfc_code_walker (gfc_code **, walk_code_fn_t, walk_expr_fn_t, void *); bool gfc_has_dimen_vector_ref (gfc_expr *e); void gfc_check_externals (gfc_namespace *); +bool gfc_fix_implicit_pure (gfc_namespace *); /* simplify.c */ diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c index 36715134a2c..d30208febb1 100644 --- a/gcc/fortran/parse.c +++ b/gcc/fortran/parse.c @@ -6447,6 +6447,11 @@ loop: gfc_resolve (gfc_current_ns); + /* Fix the implicit_pure attribute for those procedures who should + not have it. */ + while (gfc_fix_implicit_pure (gfc_current_ns)) + ; + /* Dump the parse tree if requested. */ if (flag_dump_fortran_original) gfc_dump_parse_tree (gfc_current_ns, stdout); @@ -6492,6 +6497,23 @@ done: /* Do the resolution. */ resolve_all_program_units (gfc_global_ns_list); + /* Go through all top-level namespaces and unset the implicit_pure + attribute for any procedures that call something not pure or + implicit_pure. Because the a procedure marked as not implicit_pure + in one sweep may be called by another routine, we repeat this + process until there are no more changes. */ + bool changed; + do + { + changed = false; + for (gfc_current_ns = gfc_global_ns_list; gfc_current_ns; + gfc_current_ns = gfc_current_ns->sibling) + { + if (gfc_fix_implicit_pure (gfc_current_ns)) + changed = true; + } + } + while (changed); /* Fixup for external procedures. */ for (gfc_current_ns = gfc_global_ns_list; gfc_current_ns;