Hi Andre,
Thanks - I will wait for Harald, if for no other reason than I just don't
have time today to do the commit :-)
BTW Note the commented out lines in the testcase. They fail in the front
end for reasons that I am not sure are correct. Interestingly, nagfor does
the same. Ifort fails on many of the lines that the patched gfortran and
nagfor accept. Any insights from anybody? The declaration order looks OK to
me and, I would have thought, they could be fixed up in resolution.
Cheers
Paul
On Fri, 14 Jun 2024 at 08:48, Andre Vehreschild <ve...@gmx.de> wrote:
> 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
>
