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 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
