Hi Paul,
to me this looks fine. Thanks for the patch. Me having been away for some time
from gfortran, I recommend you wait for Harald's ok, too.
Regards,
Andre
On Thu, 13 Jun 2024 22:43:03 +0100
Paul Richard Thomas <paul.richard.tho...@gmail.com> wrote:
> Hi Both,
>
> Thanks for the highly constructive comments. I think that I have
> incorporated them fully in the attached.
>
> OK for mainline and ...?
>
> Paul
>
>
> On Mon, 10 Jun 2024 at 08:19, Andre Vehreschild <ve...@gmx.de> wrote:
>
> > Hi Paul,
> >
> > while looking at your patch I see calls to gfc_add_init_cleanup (...,
> > back),
> > while the function signature is gfc_add_init_cleanup (..., bool front).
> > This
> > slightly confuses me. I would at least expect to see
> > gfc_add_init_cleanup(...,
> > !back) calls. Just to get the semantics right.
> >
> > Then I wonder why not doing:
> >
> > diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
> > index bafe8cbc5bc..97ace8c778e 100644
> > --- a/gcc/fortran/dependency.cc
> > +++ b/gcc/fortran/dependency.cc
> > @@ -2497,3 +2497,63 @@ gfc_omp_expr_prefix_same (gfc_expr *lexpr, gfc_expr
> > *rexpr)
> > return true;
> > }
> > +
> > +
> > +/* gfc_function_dependency returns true for non-dummy symbols with
> > dependencies
> > + on an old-fashioned function result (ie. proc_name =
> > proc_name->result).
> > + This is used to ensure that initialization code appears after the
> > function
> > + result is treated and that any mutual dependencies between these
> > symbols are
> > + respected. */
> > +
> > +static bool
> > +dependency_fcn (gfc_expr *e, gfc_symbol *sym,
> > + int *f ATTRIBUTE_UNUSED)
> > +{
> > + return (e && e->expr_type == EXPR_VARIABLE
> > + && e->symtree
> > + && e->symtree->n.sym == sym);
> > +}
> >
> > Instead of the multiple if-statements?
> >
> > +
> > +bool
> > +gfc_function_dependency (gfc_symbol *sym, gfc_symbol *proc_name)
> > +{
> > + bool front = false;
> > +
> > + if (proc_name && proc_name->attr.function
> > + && proc_name == proc_name->result
> > + && !(sym->attr.dummy || sym->attr.result))
> > + {
> > + if (sym->as && sym->as->type == AS_EXPLICIT)
> > + {
> > + for (int dim = 0; dim < sym->as->rank; dim++)
> > + {
> > + if (sym->as->lower[dim]
> > + && sym->as->lower[dim]->expr_type != EXPR_CONSTANT)
> > + front = gfc_traverse_expr (sym->as->lower[dim], proc_name,
> > + dependency_fcn, 0);
> > + if (front)
> > + break;
> > + if (sym->as->upper[dim]
> > + && sym->as->upper[dim]->expr_type != EXPR_CONSTANT)
> > + front = gfc_traverse_expr (sym->as->upper[dim], proc_name,
> > + dependency_fcn, 0);
> > + if (front)
> > + break;
> > + }
> > + }
> > +
> > + if (sym->ts.type == BT_CHARACTER
> > + && sym->ts.u.cl && sym->ts.u.cl->length
> > + && sym->ts.u.cl->length->expr_type != EXPR_CONSTANT)
> > + front = gfc_traverse_expr (sym->ts.u.cl->length, proc_name,
> > + dependency_fcn, 0);
> >
> > This can overwrite a previous front == true, right? Is this intended?
> >
> > + }
> > + return front;
> > + }
> >
> > The rest - besides the front-back confusion - looks fine to me. Thanks for
> > the
> > patch.
> >
> > Regards,
> > Andre
> >
> > On Sun, 9 Jun 2024 07:14:39 +0100
> > Paul Richard Thomas <paul.richard.tho...@gmail.com> wrote:
> >
> > > Hi All,
> > >
> > > The attached fixes a problem that, judging by the comments, has been
> > looked
> > > at periodically over the last ten years but just looked to be too
> > > fiendishly complicated to fix. This is not in small part because of the
> > > confusing ordering of dummies in the tlink chain and the unintuitive
> > > placement of all deferred initializations to the front of the init chain
> > in
> > > the wrapped block.
> > >
> > > The result of the existing ordering is that the initialization code for
> > > non-dummy variables that depends on the function result occurs before any
> > > initialization code for the function result itself. The fix ensures that:
> > > (i) These variables are placed correctly in the tlink chain, respecting
> > > inter-dependencies; and (ii) The dependent initializations are placed at
> > > the end of the wrapped block init chain. The details appear in the
> > > comments in the patch. It is entirely possible that a less clunky fix
> > > exists but I failed to find it.
> > >
> > > OK for mainline?
> > >
> > > Regards
> > >
> > > Paul
> >
> >
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de
> >
--
Andre Vehreschild * Email: vehre ad gmx dot de
