Re: [PATCH,FORTRAN] Fix memory leak in finalization wrappers
Le 07/11/2021 à 00:56, Bernhard Reutner-Fischer a écrit : On Sat, 6 Nov 2021 13:04:07 +0100 Mikael Morin wrote: Le 05/11/2021 à 23:08, Bernhard Reutner-Fischer a écrit : On Fri, 5 Nov 2021 19:46:16 +0100 Mikael Morin wrote: I’m a bit concerned by the loss of the null_expr’s type interface. I can’t convince myself that it’s either absolutely necessary or completely useless. It's a delicate spot, yes, but i do think they are completely useless. If we do NOT need a finalization, the initializer can (and has to be AFAIU) be a null_expr and AFAICS then does not need an interface. Well, the null pointer itself doesn’t need a type, but I think it’s better if the pointer it’s assigned to has a type different from void*. It will (hopefully) help the middle-end optimizers downstream. I would not expect this to help all that much or at all TBH. So i compiled for i in $(grep -li final $(grep -L dg-error /scratch/src/gcc-12.mine/gcc/testsuite/gfortran.dg/*.f*)); do gfortran -O2 -fcoarray=single -c $i -g -g3 -ggdb3 -fdump-tree-original -fdump-tree-optimized;done and diffed all .original and .optimized dumps against pristine trunk and they are identical. I inspected and ran the binary from finalize_14 and there is no change in the leaks compared to pristine trunk. The 3 shape_w in p leak as they used to. I do remember that finalize_14 was a good testcase, in sum i glared at it for quite some time ;) In fact, the interface is not used. the type is built in gfc_get_ppc_type which has the following. /* Explicit interface. */ if (c->attr.if_source != IFSRC_UNKNOWN && c->ts.interface) return build_pointer_type (gfc_get_function_type (c->ts.interface)); As components have no if_source attribute set, the type is not built here and a default function type is built further down without interface information. This is probably unintended as the components’ initializers carefully set an if_source attribute. The problem has been identified before ; see vaguely related posts from FX in september 2020. Anyway, I don’t think your changes will have negative impact then, and it makes things more readable, so I’m fine with it after all; OK. I will see if I can manage to create a testcase where it makes a difference (don’t hold your breath, I don’t even have a bootstrapped compiler ready yet). That'd be great, TIA! [] I’ve given up eventually. btw.. Just because it's vagely related. I think f8add009ce300f24b75e9c2e2cc5dd944a020c28 for PR fortran/88009 (ICE in find_intrinsic_vtab, at fortran/class.c:2761) is incomplete in that i think all the internal class helpers should be flagged artificial. All these symbols built in gfc_build_class_symbol, generate_finalization_wrapper, gfc_find_derived_vtab etc. Looking at the history it seems the artificial bit often was forgotten. I guess so, yes... And most importantly i think it is not correct to ignore artificial in gfc_check_conflict! Well, it’s not correct to throw errors at users for things they haven’t written and that they don’t control.
[PATCH v3 1/5] fortran: Tiny sort_actual internal refactoring
Preliminary refactoring to make further changes more obvious.
No functional change.
gcc/fortran/ChangeLog:
* intrinsic.c (sort_actual): initialise variable and use it earlier.
---
gcc/fortran/intrinsic.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c
index a6a18a471e3..49ef3b2a3d2 100644
--- a/gcc/fortran/intrinsic.c
+++ b/gcc/fortran/intrinsic.c
@@ -4378,19 +4378,18 @@ do_sort:
for (f = formal; f; f = f->next)
{
- if (f->actual && f->actual->label != NULL && f->ts.type)
+ a = f->actual;
+ if (a && a->label != NULL && f->ts.type)
{
gfc_error ("ALTERNATE RETURN not permitted at %L", where);
return false;
}
- if (f->actual == NULL)
+ if (a == NULL)
{
a = gfc_get_actual_arglist ();
a->missing_arg_type = f->ts.type;
}
- else
- a = f->actual;
if (actual == NULL)
*ap = a;
[PATCH v3 0/5] fortran: Ignore unused arguments for scalarisation [PR97896]
Hello, This is the third submit of this patch series. After submitting the v2 [2] for master, and a somewhat different variant for backport [3], I thought it was defeating the purpose of the backporting process. So I have decided to rebase the master patches on the backport patches, so that the backport patchs can get some testing on master first. The problematic case is intrinsic procedures where an argument is actually not used in the code generated (KIND argument of INDEX in the testcase), which confuses the scalariser. Thomas König comitted a change to workaround the problem, but it regressed in PR97896. These patches put the workaround where I think it is more appropriate, namely at the beginning of the scalarisation procedure. That’s what is done by the series [3] initially intended for backport only, and now for master too. This series is a followup to them. What are left in this series are a couple of refactoring for the master branch only. They aim at being able to identify the KIND argument of the INDEX intrinsic by its name, rather than counting the right number of next->next->next indirections starting with the first argument. It may seem overkill for just this use case, but I think it’s worth having that facility in the long term. Regression-tested on x86_64-linux-gnu. Ok for master? Changes from v2 [2]: Rebase on the backport variant of the series. Changes from v1 [1]: Use C structs and enums instead of C++ classes. [1] https://gcc.gnu.org/pipermail/fortran/2021-August/056303.html [2] https://gcc.gnu.org/pipermail/fortran/2021-August/056317.html [3] https://gcc.gnu.org/pipermail/fortran/2021-August/056329.html Mikael Morin (5): fortran: Tiny sort_actual internal refactoring fortran: Reverse actual vs dummy argument mapping fortran: simplify elemental arguments walking fortran: Delete redundant missing_arg_type field fortran: Identify arguments by their names gcc/fortran/gfortran.h| 41 +++ gcc/fortran/interface.c | 77 +++ gcc/fortran/intrinsic.c | 53 gcc/fortran/trans-array.c | 35 +--- gcc/fortran/trans-array.h | 2 +- gcc/fortran/trans-expr.c | 9 +++- gcc/fortran/trans-intrinsic.c | 2 +- gcc/fortran/trans-stmt.c | 22 -- gcc/fortran/trans.h | 4 +- 9 files changed, 161 insertions(+), 84 deletions(-) -- 2.33.0
[PATCH v3 2/5] fortran: Reverse actual vs dummy argument mapping
There was originally no way from an actual argument to get
to the corresponding dummy argument, even if the job of sorting
and matching actual with dummy arguments was done.
The closest was a field named actual in gfc_intrinsic_arg that was
used as scratch data when sorting arguments of one specific call.
However that value was overwritten later on as arguments of another
call to the same procedure were sorted and matched.
This change removes that field from gfc_intrinsic_arg and adds instead
a new field associated_dummy in gfc_actual_arglist.
The new field has as type a new wrapper struct gfc_dummy_arg that provides
a common interface to both dummy arguments of user-defined procedures
(which have type gfc_formal_arglist) and dummy arguments of intrinsic procedures
(which have type gfc_intrinsic_arg).
As the removed field was used in the code sorting and matching arguments,
that code has to be updated. Two local vectors with matching indices
are introduced for respectively dummy and actual arguments, and the
loops are modified to use indices and update those argument vectors.
gcc/fortran/ChangeLog:
* gfortran.h (gfc_dummy_arg_kind, gfc_dummy_arg): New.
(gfc_actual_arglist): New field associated_dummy.
(gfc_intrinsic_arg): Remove field actual.
* interface.c (get_nonintrinsic_dummy_arg): New.
(gfc_compare_actual): Initialize associated_dummy.
* intrinsic.c (get_intrinsic_dummy_arg): New.
(sort_actual): Add argument vectors.
Use loops with indices on argument vectors.
Initialize associated_dummy.
---
gcc/fortran/gfortran.h | 31 +++--
gcc/fortran/interface.c | 21 ++--
gcc/fortran/intrinsic.c | 43 ++---
3 files changed, 80 insertions(+), 15 deletions(-)
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 8c11cf6d18d..d678c6b56dc 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -1199,6 +1199,9 @@ gfc_formal_arglist;
#define gfc_get_formal_arglist() XCNEW (gfc_formal_arglist)
+struct gfc_dummy_arg;
+
+
/* The gfc_actual_arglist structure is for actual arguments and
for type parameter specification lists. */
typedef struct gfc_actual_arglist
@@ -1215,6 +1218,11 @@ typedef struct gfc_actual_arglist
gfc_param_spec_type spec_type;
struct gfc_expr *expr;
+
+ /* The dummy arg this actual arg is associated with, if the interface
+ is explicit. NULL otherwise. */
+ gfc_dummy_arg *associated_dummy;
+
struct gfc_actual_arglist *next;
}
gfc_actual_arglist;
@@ -2298,14 +2306,33 @@ typedef struct gfc_intrinsic_arg
gfc_typespec ts;
unsigned optional:1, value:1;
ENUM_BITFIELD (sym_intent) intent:2;
- gfc_actual_arglist *actual;
struct gfc_intrinsic_arg *next;
-
}
gfc_intrinsic_arg;
+typedef enum {
+ GFC_UNDEFINED_DUMMY_ARG = 0,
+ GFC_INTRINSIC_DUMMY_ARG,
+ GFC_NON_INTRINSIC_DUMMY_ARG
+}
+gfc_dummy_arg_intrinsicness;
+
+/* dummy arg of either an intrinsic or a user-defined procedure. */
+struct gfc_dummy_arg
+{
+ gfc_dummy_arg_intrinsicness intrinsicness;
+
+ union {
+gfc_intrinsic_arg *intrinsic;
+gfc_formal_arglist *non_intrinsic;
+ } u;
+};
+
+#define gfc_get_dummy_arg() XCNEW (gfc_dummy_arg)
+
+
/* Specifies the various kinds of check functions used to verify the
argument lists of intrinsic functions. fX with X an integer refer
to check functions of intrinsics with X arguments. f1m is used for
diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
index 24698be8364..c4ec0d89a58 100644
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -3043,6 +3043,18 @@ lookup_arg_fuzzy (const char *arg, gfc_formal_arglist *arguments)
}
+static gfc_dummy_arg *
+get_nonintrinsic_dummy_arg (gfc_formal_arglist *formal)
+{
+ gfc_dummy_arg * const dummy_arg = gfc_get_dummy_arg ();
+
+ dummy_arg->intrinsicness = GFC_NON_INTRINSIC_DUMMY_ARG;
+ dummy_arg->u.non_intrinsic = formal;
+
+ return dummy_arg;
+}
+
+
/* Given formal and actual argument lists, see if they are compatible.
If they are compatible, the actual argument list is sorted to
correspond with the formal list, and elements for missing optional
@@ -3150,6 +3162,8 @@ gfc_compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal,
"call at %L", where);
return false;
}
+ else
+ a->associated_dummy = get_nonintrinsic_dummy_arg (f);
if (a->expr == NULL)
{
@@ -3646,9 +3660,12 @@ gfc_compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal,
/* The argument lists are compatible. We now relink a new actual
argument list with null arguments in the right places. The head
of the list remains the head. */
- for (i = 0; i < n; i++)
+ for (f = formal, i = 0; f; f = f->next, i++)
if (new_arg[i] == NULL)
- new_arg[i] = gfc_get_actual_arglist ();
+ {
+ new_arg[i] = gfc_get_actual_arglist ();
+ new_arg[
[PATCH v3 3/5] fortran: simplify elemental arguments walking
This adds two functions working with the wrapper struct gfc_dummy_arg
and makes usage of them to simplify a bit the walking of elemental
procedure arguments for scalarization. As information about dummy arguments
can be obtained from the actual argument through the just-introduced
associated_dummy field, there is no need to carry around the procedure
interface and walk dummy arguments manually together with actual arguments.
gcc/fortran/ChangeLog:
* interface.c (gfc_dummy_arg_get_typespec,
gfc_dummy_arg_is_optional): New functions.
* gfortran.h (gfc_dummy_arg_get_typespec,
gfc_dummy_arg_is_optional): Declare them.
* trans.h (gfc_ss_info::dummy_arg): Use the wrapper type
as declaration type.
* trans-array.c (gfc_scalar_elemental_arg_saved_as_reference):
use gfc_dummy_arg_get_typespec function to get the type.
(gfc_walk_elemental_function_args): Remove proc_ifc argument.
Get info about the dummy arg using the associated_dummy field.
* trans-array.h (gfc_walk_elemental_function_args): Update declaration.
* trans-intrinsic.c (gfc_walk_intrinsic_function):
Update call to gfc_walk_elemental_function_args.
* trans-stmt.c (gfc_trans_call): Ditto.
(get_proc_ifc_for_call): Remove.
---
gcc/fortran/gfortran.h| 4
gcc/fortran/interface.c | 34 ++
gcc/fortran/trans-array.c | 19 ++-
gcc/fortran/trans-array.h | 2 +-
gcc/fortran/trans-intrinsic.c | 2 +-
gcc/fortran/trans-stmt.c | 22 --
gcc/fortran/trans.h | 4 ++--
7 files changed, 48 insertions(+), 39 deletions(-)
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index d678c6b56dc..7e76e482b98 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2333,6 +2333,10 @@ struct gfc_dummy_arg
#define gfc_get_dummy_arg() XCNEW (gfc_dummy_arg)
+const gfc_typespec & gfc_dummy_arg_get_typespec (gfc_dummy_arg &);
+bool gfc_dummy_arg_is_optional (gfc_dummy_arg &);
+
+
/* Specifies the various kinds of check functions used to verify the
argument lists of intrinsic functions. fX with X an integer refer
to check functions of intrinsics with X arguments. f1m is used for
diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
index c4ec0d89a58..db0b3b01b8c 100644
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -5503,3 +5503,37 @@ gfc_get_formal_from_actual_arglist (gfc_symbol *sym,
f = &((*f)->next);
}
}
+
+
+const gfc_typespec &
+gfc_dummy_arg_get_typespec (gfc_dummy_arg & dummy_arg)
+{
+ switch (dummy_arg.intrinsicness)
+{
+case GFC_INTRINSIC_DUMMY_ARG:
+ return dummy_arg.u.intrinsic->ts;
+
+case GFC_NON_INTRINSIC_DUMMY_ARG:
+ return dummy_arg.u.non_intrinsic->sym->ts;
+
+default:
+ gcc_unreachable ();
+}
+}
+
+
+bool
+gfc_dummy_arg_is_optional (gfc_dummy_arg & dummy_arg)
+{
+ switch (dummy_arg.intrinsicness)
+{
+case GFC_INTRINSIC_DUMMY_ARG:
+ return dummy_arg.u.intrinsic->optional;
+
+case GFC_NON_INTRINSIC_DUMMY_ARG:
+ return dummy_arg.u.non_intrinsic->sym->attr.optional;
+
+default:
+ gcc_unreachable ();
+}
+}
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 79321854498..d37c1e7ad7f 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -3010,7 +3010,8 @@ gfc_scalar_elemental_arg_saved_as_reference (gfc_ss_info * ss_info)
/* If the expression is of polymorphic type, it's actual size is not known,
so we avoid copying it anywhere. */
if (ss_info->data.scalar.dummy_arg
- && ss_info->data.scalar.dummy_arg->ts.type == BT_CLASS
+ && gfc_dummy_arg_get_typespec (*ss_info->data.scalar.dummy_arg).type
+ == BT_CLASS
&& ss_info->expr->ts.type == BT_CLASS)
return true;
@@ -11521,9 +11522,8 @@ arg_evaluated_for_scalarization (gfc_intrinsic_sym *function,
gfc_ss *
gfc_walk_elemental_function_args (gfc_ss * ss, gfc_actual_arglist *arg,
gfc_intrinsic_sym *intrinsic_sym,
- gfc_symbol *proc_ifc, gfc_ss_type type)
+ gfc_ss_type type)
{
- gfc_formal_arglist *dummy_arg;
int scalar;
gfc_ss *head;
gfc_ss *tail;
@@ -11532,15 +11532,11 @@ gfc_walk_elemental_function_args (gfc_ss * ss, gfc_actual_arglist *arg,
head = gfc_ss_terminator;
tail = NULL;
- if (proc_ifc)
-dummy_arg = gfc_sym_get_dummy_args (proc_ifc);
- else
-dummy_arg = NULL;
-
int arg_num = 0;
scalar = 1;
for (; arg; arg = arg->next)
{
+ gfc_dummy_arg * const dummy_arg = arg->associated_dummy;
if (!arg->expr
|| arg->expr->expr_type == EXPR_NULL
|| !arg_evaluated_for_scalarization (intrinsic_sym, *arg, arg_num))
@@ -11554,13 +11550,13 @@ gfc_walk_elemental_function_args (gfc_ss * ss, gfc_actual_arglist *arg,
newss = gfc_get_scalar_ss (head, arg->expr);
newss->info->type = type;
if (dummy_arg)
-
[PATCH v3 4/5] fortran: Delete redundant missing_arg_type field
Now that we can get information about an actual arg's associated
dummy using the associated_dummy attribute, the field missing_arg_type
contains redundant information.
This removes it.
gcc/fortran/ChangeLog:
* gfortran.h (gfc_actual_arglist::missing_arg_type): Remove.
* interface.c (gfc_compare_actual_formal): Remove
missing_arg_type initialization.
* intrinsic.c (sort_actual): Ditto.
* trans-expr.c (gfc_conv_procedure_call): Use associated_dummy
and gfc_dummy_arg_get_typespec to get the dummy argument type.
---
gcc/fortran/gfortran.h | 5 -
gcc/fortran/interface.c | 5 -
gcc/fortran/intrinsic.c | 5 +
gcc/fortran/trans-expr.c | 9 +++--
4 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 7e76e482b98..4879805ff0b 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -1210,11 +1210,6 @@ typedef struct gfc_actual_arglist
/* Alternate return label when the expr member is null. */
struct gfc_st_label *label;
- /* This is set to the type of an eventual omitted optional
- argument. This is used to determine if a hidden string length
- argument has to be added to a function call. */
- bt missing_arg_type;
-
gfc_param_spec_type spec_type;
struct gfc_expr *expr;
diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
index db0b3b01b8c..36b7a852066 100644
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -3681,11 +3681,6 @@ gfc_compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal,
if (*ap == NULL && n > 0)
*ap = new_arg[0];
- /* Note the types of omitted optional arguments. */
- for (a = *ap, f = formal; a; a = a->next, f = f->next)
-if (a->expr == NULL && a->label == NULL)
- a->missing_arg_type = f->sym->ts.type;
-
return true;
}
diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c
index f6d061a847c..3018315ed78 100644
--- a/gcc/fortran/intrinsic.c
+++ b/gcc/fortran/intrinsic.c
@@ -4405,10 +4405,7 @@ do_sort:
}
if (a == NULL)
- {
- a = gfc_get_actual_arglist ();
- a->missing_arg_type = f->ts.type;
- }
+ a = gfc_get_actual_arglist ();
a->associated_dummy = get_intrinsic_dummy_arg (f);
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index e7aec3845d3..bc502c0f43c 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -6157,7 +6157,10 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
{
/* Pass a NULL pointer for an absent arg. */
parmse.expr = null_pointer_node;
- if (arg->missing_arg_type == BT_CHARACTER)
+ gfc_dummy_arg * const dummy_arg = arg->associated_dummy;
+ if (dummy_arg
+ && gfc_dummy_arg_get_typespec (*dummy_arg).type
+ == BT_CHARACTER)
parmse.string_length = build_int_cst (gfc_charlen_type_node,
0);
}
@@ -6174,7 +6177,9 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
|| !CLASS_DATA (fsym)->attr.allocatable));
gfc_init_se (&parmse, NULL);
parmse.expr = null_pointer_node;
- if (arg->missing_arg_type == BT_CHARACTER)
+ if (arg->associated_dummy
+ && gfc_dummy_arg_get_typespec (*arg->associated_dummy).type
+ == BT_CHARACTER)
parmse.string_length = build_int_cst (gfc_charlen_type_node, 0);
}
else if (fsym && fsym->ts.type == BT_CLASS
[PATCH 0/2] fortran: Ignore unused arguments for scalarisation [PR97896]
Hello, I repost this patch series initially targetted at the 11 branch only [1], and that I now would like to commit to master as well before. The problematic case is intrinsic procedures where an argument is actually not used in the code generated (KIND argument of INDEX in the testcase), which confuses the scalariser. Thomas König comitted a change to workaround the problem, but it regressed in PR97896. These patch put the workaround where I think it is more appropriate, namely at the beginning of the scalarisation procedure. This is the patch 2 of the series, preceded with the revert in patch 1. I intend to commit both of them squashed together. Regression-tested on x86_64-linux-gnu. Ok for master and 11 branch? Changes from v1: Rebase on master. [1] https://gcc.gnu.org/pipermail/fortran/2021-August/056329.html Mikael Morin (2): Revert "Remove KIND argument from INDEX so it does not mess up scalarization." fortran: Ignore unused args in scalarization [PR97896] gcc/fortran/intrinsic.c | 48 +++-- gcc/fortran/intrinsic.h | 3 +- gcc/fortran/iresolve.c| 21 ++--- gcc/fortran/trans-array.c | 61 ++- gcc/fortran/trans-array.h | 3 ++ gcc/fortran/trans-decl.c | 24 +-- gcc/fortran/trans-intrinsic.c | 1 + gcc/fortran/trans-stmt.c | 20 + gcc/testsuite/gfortran.dg/index_5.f90 | 23 ++ 9 files changed, 121 insertions(+), 83 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/index_5.f90 -- 2.33.0
[PATCH 1/2] Revert "Remove KIND argument from INDEX so it does not mess up scalarization."
This reverts commit d09847357b965a2c2cda063827ce362d4c9c86f2 except for
its testcase.
gcc/fortran/ChangeLog:
* intrinsic.c (add_sym_4ind): Remove.
(add_functions): Use add_sym4 instead of add_sym4ind.
Don’t special case the index intrinsic.
* iresolve.c (gfc_resolve_index_func): Use the individual arguments
directly instead of the full argument list.
* intrinsic.h (gfc_resolve_index_func): Update the declaration
accordingly.
* trans-decl.c (gfc_get_extern_function_decl): Don’t modify the
list of arguments in the case of the index intrinsic.
---
gcc/fortran/intrinsic.c | 48 ++--
gcc/fortran/intrinsic.h | 3 ++-
gcc/fortran/iresolve.c | 21 --
gcc/fortran/trans-decl.c | 24 +---
4 files changed, 14 insertions(+), 82 deletions(-)
diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c
index f5c88d98cc9..a6a18a471e3 100644
--- a/gcc/fortran/intrinsic.c
+++ b/gcc/fortran/intrinsic.c
@@ -888,39 +888,6 @@ add_sym_4 (const char *name, gfc_isym_id id, enum klass cl, int actual_ok, bt ty
(void *) 0);
}
-/* Add a symbol to the function list where the function takes 4
- arguments and resolution may need to change the number or
- arrangement of arguments. This is the case for INDEX, which needs
- its KIND argument removed. */
-
-static void
-add_sym_4ind (const char *name, gfc_isym_id id, enum klass cl, int actual_ok,
- bt type, int kind, int standard,
- bool (*check) (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *),
- gfc_expr *(*simplify) (gfc_expr *, gfc_expr *, gfc_expr *,
- gfc_expr *),
- void (*resolve) (gfc_expr *, gfc_actual_arglist *),
- const char *a1, bt type1, int kind1, int optional1,
- const char *a2, bt type2, int kind2, int optional2,
- const char *a3, bt type3, int kind3, int optional3,
- const char *a4, bt type4, int kind4, int optional4 )
-{
- gfc_check_f cf;
- gfc_simplify_f sf;
- gfc_resolve_f rf;
-
- cf.f4 = check;
- sf.f4 = simplify;
- rf.f1m = resolve;
-
- add_sym (name, id, cl, actual_ok, type, kind, standard, cf, sf, rf,
- a1, type1, kind1, optional1, INTENT_IN,
- a2, type2, kind2, optional2, INTENT_IN,
- a3, type3, kind3, optional3, INTENT_IN,
- a4, type4, kind4, optional4, INTENT_IN,
- (void *) 0);
-}
-
/* Add a symbol to the subroutine list where the subroutine takes
4 arguments. */
@@ -2223,11 +2190,11 @@ add_functions (void)
/* The resolution function for INDEX is called gfc_resolve_index_func
because the name gfc_resolve_index is already used in resolve.c. */
- add_sym_4ind ("index", GFC_ISYM_INDEX, CLASS_ELEMENTAL, ACTUAL_YES,
- BT_INTEGER, di, GFC_STD_F77,
- gfc_check_index, gfc_simplify_index, gfc_resolve_index_func,
- stg, BT_CHARACTER, dc, REQUIRED, ssg, BT_CHARACTER, dc, REQUIRED,
- bck, BT_LOGICAL, dl, OPTIONAL, kind, BT_INTEGER, di, OPTIONAL);
+ add_sym_4 ("index", GFC_ISYM_INDEX, CLASS_ELEMENTAL, ACTUAL_YES,
+ BT_INTEGER, di, GFC_STD_F77,
+ gfc_check_index, gfc_simplify_index, gfc_resolve_index_func,
+ stg, BT_CHARACTER, dc, REQUIRED, ssg, BT_CHARACTER, dc, REQUIRED,
+ bck, BT_LOGICAL, dl, OPTIONAL, kind, BT_INTEGER, di, OPTIONAL);
make_generic ("index", GFC_ISYM_INDEX, GFC_STD_F77);
@@ -4530,10 +4497,9 @@ resolve_intrinsic (gfc_intrinsic_sym *specific, gfc_expr *e)
arg = e->value.function.actual;
- /* Special case hacks for MIN, MAX and INDEX. */
+ /* Special case hacks for MIN and MAX. */
if (specific->resolve.f1m == gfc_resolve_max
- || specific->resolve.f1m == gfc_resolve_min
- || specific->resolve.f1m == gfc_resolve_index_func)
+ || specific->resolve.f1m == gfc_resolve_min)
{
(*specific->resolve.f1m) (e, arg);
return;
diff --git a/gcc/fortran/intrinsic.h b/gcc/fortran/intrinsic.h
index 7511df1..fb655fb078a 100644
--- a/gcc/fortran/intrinsic.h
+++ b/gcc/fortran/intrinsic.h
@@ -519,7 +519,8 @@ void gfc_resolve_ibits (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *);
void gfc_resolve_ibset (gfc_expr *, gfc_expr *, gfc_expr *);
void gfc_resolve_image_index (gfc_expr *, gfc_expr *, gfc_expr *);
void gfc_resolve_image_status (gfc_expr *, gfc_expr *, gfc_expr *);
-void gfc_resolve_index_func (gfc_expr *, gfc_actual_arglist *);
+void gfc_resolve_index_func (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *,
+ gfc_expr *);
void gfc_resolve_ierrno (gfc_expr *);
void gfc_resolve_ieor (gfc_expr *, gfc_expr *, gfc_expr *);
void gfc_resolve_ichar (gfc_expr *, gfc_expr *, gfc_expr *);
diff --git a/gcc/fortran/iresolve.c b/gcc/fortran/iresolve.c
index e17fe45f080..598c0409b66 100644
--- a/gcc/fortran/iresolve.c
+++ b/gcc/fortran/iresolve.c
@@ -1276,27 +1276,16 @@ gfc_resolve_ior (gfc_expr *f, gfc_expr *i, gfc_expr *j)
void
-gfc_resolve_index_func (gfc_expr *f, gfc_actual_arglist *a)
+gfc_resolve_index_func (gfc_expr *f, gfc_exp
[PATCH 2/2] fortran: Ignore unused args in scalarization [PR97896]
The KIND argument of the INDEX intrinsic is a compile time constant
that is used at compile time only to resolve to a kind-specific library
function. That argument is otherwise completely ignored at runtime, and there
is
no code generated for it as the library procedure has no kind argument.
This confuses the scalarizer which expects to see every argument
of elemental functions used when calling a procedure.
This change removes the argument from the scalarization lists
at the beginning of the scalarization process, so that the argument
is completely ignored.
PR fortran/97896
gcc/fortran/ChangeLog:
* trans-array.h (gfc_get_intrinsic_for_expr,
gfc_get_proc_ifc_for_expr): New.
* trans-array.c (gfc_get_intrinsic_for_expr,
arg_evaluated_for_scalarization): New.
(gfc_walk_elemental_function_args): Add intrinsic procedure
as argument. Count arguments. Check arg_evaluated_for_scalarization.
* trans-intrinsic.c (gfc_walk_intrinsic_function): Update call.
* trans-stmt.c (get_intrinsic_for_code): New.
(gfc_trans_call): Update call.
gcc/testsuite/ChangeLog:
* gfortran.dg/index_5.f90: New.
---
gcc/fortran/trans-array.c | 61 ++-
gcc/fortran/trans-array.h | 3 ++
gcc/fortran/trans-intrinsic.c | 1 +
gcc/fortran/trans-stmt.c | 20 +
gcc/testsuite/gfortran.dg/index_5.f90 | 23 ++
5 files changed, 107 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gfortran.dg/index_5.f90
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 5ceb261b698..79321854498 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -11460,6 +11460,59 @@ gfc_get_proc_ifc_for_expr (gfc_expr *procedure_ref)
}
+/* Given an expression referring to an intrinsic function call,
+ return the intrinsic symbol. */
+
+gfc_intrinsic_sym *
+gfc_get_intrinsic_for_expr (gfc_expr *call)
+{
+ if (call == NULL)
+return NULL;
+
+ /* Normal procedure case. */
+ if (call->expr_type == EXPR_FUNCTION)
+return call->value.function.isym;
+ else
+return NULL;
+}
+
+
+/* Indicates whether an argument to an intrinsic function should be used in
+ scalarization. It is usually the case, except for some intrinsics
+ requiring the value to be constant, and using the value at compile time only.
+ As the value is not used at runtime in those cases, we donât produce code
+ for it, and it should not be visible to the scalarizer.
+ FUNCTION is the intrinsic function being called, ACTUAL_ARG is the actual
+ argument being examined in that call, and ARG_NUM the index number
+ of ACTUAL_ARG in the list of arguments.
+ The intrinsic procedureâs dummy argument associated with ACTUAL_ARG is
+ identified using the name in ACTUAL_ARG if it is present (that is: if itâs
+ a keyword argument), otherwise using ARG_NUM. */
+
+static bool
+arg_evaluated_for_scalarization (gfc_intrinsic_sym *function,
+ gfc_actual_arglist &actual_arg, int arg_num)
+{
+ if (function != NULL)
+{
+ switch (function->id)
+ {
+ case GFC_ISYM_INDEX:
+ if ((actual_arg.name == NULL && arg_num == 3)
+ || (actual_arg.name != NULL
+ && strcmp ("kind", actual_arg.name) == 0))
+ return false;
+ /* Fallthrough. */
+
+ default:
+ break;
+ }
+}
+
+ return true;
+}
+
+
/* Walk the arguments of an elemental function.
PROC_EXPR is used to check whether an argument is permitted to be absent. If
it is NULL, we don't do the check and the argument is assumed to be present.
@@ -11467,6 +11520,7 @@ gfc_get_proc_ifc_for_expr (gfc_expr *procedure_ref)
gfc_ss *
gfc_walk_elemental_function_args (gfc_ss * ss, gfc_actual_arglist *arg,
+ gfc_intrinsic_sym *intrinsic_sym,
gfc_symbol *proc_ifc, gfc_ss_type type)
{
gfc_formal_arglist *dummy_arg;
@@ -11483,10 +11537,13 @@ gfc_walk_elemental_function_args (gfc_ss * ss, gfc_actual_arglist *arg,
else
dummy_arg = NULL;
+ int arg_num = 0;
scalar = 1;
for (; arg; arg = arg->next)
{
- if (!arg->expr || arg->expr->expr_type == EXPR_NULL)
+ if (!arg->expr
+ || arg->expr->expr_type == EXPR_NULL
+ || !arg_evaluated_for_scalarization (intrinsic_sym, *arg, arg_num))
goto loop_continue;
newss = gfc_walk_subexpr (head, arg->expr);
@@ -11519,6 +11576,7 @@ gfc_walk_elemental_function_args (gfc_ss * ss, gfc_actual_arglist *arg,
}
loop_continue:
+ arg_num++;
if (dummy_arg != NULL)
dummy_arg = dummy_arg->next;
}
@@ -11579,6 +11637,7 @@ gfc_walk_function_expr (gfc_ss * ss, gfc_expr * expr)
ss = gfc_walk_elemental_function_args (old_ss,
expr->value.function.actual,
+ gfc_get_intrinsic_for_expr (expr),
gfc_get_proc_ifc_for_expr (expr),
GFC_SS_REFERENCE);
if (ss != old_ss
diff --git a/gcc/fortran/trans-array.h b/gcc/fortran/trans-array.
[PATCH v3 5/5] fortran: Identify arguments by their names
This provides a new function to get the name of a dummy argument,
so that identifying an argument can be made using just its name
instead of a mix of name matching (for keyword actual arguments)
and argument counting (for other actual arguments).
gcc/fortran/ChangeLog:
* interface.c (gfc_dummy_arg_get_name): New function.
* gfortran.h (gfc_dummy_arg_get_name): Declare it.
* trans-array.c (arg_evaluated_for_scalarization): Pass a dummy
argument wrapper as argument instead of an actual argument
and an index number. Check it’s non-NULL. Use its name
to identify it.
(gfc_walk_elemental_function_args): Update call to
arg_evaluated for scalarization. Remove argument counting.
---
gcc/fortran/gfortran.h| 1 +
gcc/fortran/interface.c | 17 +
gcc/fortran/trans-array.c | 16 +---
3 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 4879805ff0b..ac4b3a8b6d4 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2328,6 +2328,7 @@ struct gfc_dummy_arg
#define gfc_get_dummy_arg() XCNEW (gfc_dummy_arg)
+const char * gfc_dummy_arg_get_name (gfc_dummy_arg &);
const gfc_typespec & gfc_dummy_arg_get_typespec (gfc_dummy_arg &);
bool gfc_dummy_arg_is_optional (gfc_dummy_arg &);
diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
index 36b7a852066..d87088f988d 100644
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -5500,6 +5500,23 @@ gfc_get_formal_from_actual_arglist (gfc_symbol *sym,
}
+const char *
+gfc_dummy_arg_get_name (gfc_dummy_arg & dummy_arg)
+{
+ switch (dummy_arg.intrinsicness)
+{
+case GFC_INTRINSIC_DUMMY_ARG:
+ return dummy_arg.u.intrinsic->name;
+
+case GFC_NON_INTRINSIC_DUMMY_ARG:
+ return dummy_arg.u.non_intrinsic->sym->name;
+
+default:
+ gcc_unreachable ();
+}
+}
+
+
const gfc_typespec &
gfc_dummy_arg_get_typespec (gfc_dummy_arg & dummy_arg)
{
diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index d37c1e7ad7f..2090adf01e7 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -11492,16 +11492,14 @@ gfc_get_intrinsic_for_expr (gfc_expr *call)
static bool
arg_evaluated_for_scalarization (gfc_intrinsic_sym *function,
- gfc_actual_arglist &actual_arg, int arg_num)
+ gfc_dummy_arg *dummy_arg)
{
- if (function != NULL)
+ if (function != NULL && dummy_arg != NULL)
{
switch (function->id)
{
case GFC_ISYM_INDEX:
- if ((actual_arg.name == NULL && arg_num == 3)
- || (actual_arg.name != NULL
- && strcmp ("kind", actual_arg.name) == 0))
+ if (strcmp ("kind", gfc_dummy_arg_get_name (*dummy_arg)) == 0)
return false;
/* Fallthrough. */
@@ -11532,15 +11530,14 @@ gfc_walk_elemental_function_args (gfc_ss * ss, gfc_actual_arglist *arg,
head = gfc_ss_terminator;
tail = NULL;
- int arg_num = 0;
scalar = 1;
for (; arg; arg = arg->next)
{
gfc_dummy_arg * const dummy_arg = arg->associated_dummy;
if (!arg->expr
|| arg->expr->expr_type == EXPR_NULL
- || !arg_evaluated_for_scalarization (intrinsic_sym, *arg, arg_num))
- goto loop_continue;
+ || !arg_evaluated_for_scalarization (intrinsic_sym, dummy_arg))
+ continue;
newss = gfc_walk_subexpr (head, arg->expr);
if (newss == head)
@@ -11570,9 +11567,6 @@ gfc_walk_elemental_function_args (gfc_ss * ss, gfc_actual_arglist *arg,
while (tail->next != gfc_ss_terminator)
tail = tail->next;
}
-
-loop_continue:
- arg_num++;
}
if (scalar)
[patch, fortran, wwwdocs] Fix name of argument to CO_REDUCE
Hello world,
the attached patches fix the name of the function argument to CO_REDUCE
to conform to Fortran 2018 instead of the TR.
This is a user-visible change, so I have put this both into changes.html
and porting_to.html.
Regression-tested. OK for trunk?
Best regards
Thomas
Author: Thomas Koenig
Date: 2021-11-07 15:38:35 +0100
Fix keyword name for co_reduce.
gcc/fortran/ChangeLog:
* intrinsic.c (add_subroutines): Change keyword "operator"
to the correct one, "operation".
* check.c (gfc_check_co_reduce): Change OPERATOR to
OPERATION in error messages.
gcc/testsuite/ChangeLog:
* gfortran.dg/co_reduce_2.f90: New test.
* gfortran.dg/coarray_collective_16.f90: Change OPERATOR
to OPERATION.
* gfortran.dg/coarray_collective_9.f90: Likewise.
Co-authored by: Steve Kargl
diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
index 6ea6e136d4f..15772009af4 100644
--- a/gcc/fortran/check.c
+++ b/gcc/fortran/check.c
@@ -2265,7 +2265,7 @@ gfc_check_co_reduce (gfc_expr *a, gfc_expr *op, gfc_expr *result_image,
attr = gfc_expr_attr (op);
if (!attr.pure || !attr.function)
{
- gfc_error ("OPERATOR argument at %L must be a PURE function",
+ gfc_error ("OPERATION argument at %L must be a PURE function",
&op->where);
return false;
}
@@ -2292,7 +2292,7 @@ gfc_check_co_reduce (gfc_expr *a, gfc_expr *op, gfc_expr *result_image,
if (!formal || !formal->next || formal->next->next)
{
- gfc_error ("The function passed as OPERATOR at %L shall have two "
+ gfc_error ("The function passed as OPERATION at %L shall have two "
"arguments", &op->where);
return false;
}
@@ -2303,7 +2303,7 @@ gfc_check_co_reduce (gfc_expr *a, gfc_expr *op, gfc_expr *result_image,
if (!gfc_compare_types (&a->ts, &sym->result->ts))
{
gfc_error ("The A argument at %L has type %s but the function passed as "
- "OPERATOR at %L returns %s",
+ "OPERATION at %L returns %s",
&a->where, gfc_typename (a), &op->where,
gfc_typename (&sym->result->ts));
return false;
@@ -2311,7 +2311,7 @@ gfc_check_co_reduce (gfc_expr *a, gfc_expr *op, gfc_expr *result_image,
if (!gfc_compare_types (&a->ts, &formal->sym->ts)
|| !gfc_compare_types (&a->ts, &formal->next->sym->ts))
{
- gfc_error ("The function passed as OPERATOR at %L has arguments of type "
+ gfc_error ("The function passed as OPERATION at %L has arguments of type "
"%s and %s but shall have type %s", &op->where,
gfc_typename (&formal->sym->ts),
gfc_typename (&formal->next->sym->ts), gfc_typename (a));
@@ -2322,7 +2322,7 @@ gfc_check_co_reduce (gfc_expr *a, gfc_expr *op, gfc_expr *result_image,
|| formal->next->sym->attr.allocatable || formal->sym->attr.pointer
|| formal->next->sym->attr.pointer)
{
- gfc_error ("The function passed as OPERATOR at %L shall have scalar "
+ gfc_error ("The function passed as OPERATION at %L shall have scalar "
"nonallocatable nonpointer arguments and return a "
"nonallocatable nonpointer scalar", &op->where);
return false;
@@ -2330,21 +2330,21 @@ gfc_check_co_reduce (gfc_expr *a, gfc_expr *op, gfc_expr *result_image,
if (formal->sym->attr.value != formal->next->sym->attr.value)
{
- gfc_error ("The function passed as OPERATOR at %L shall have the VALUE "
+ gfc_error ("The function passed as OPERATION at %L shall have the VALUE "
"attribute either for none or both arguments", &op->where);
return false;
}
if (formal->sym->attr.target != formal->next->sym->attr.target)
{
- gfc_error ("The function passed as OPERATOR at %L shall have the TARGET "
+ gfc_error ("The function passed as OPERATION at %L shall have the TARGET "
"attribute either for none or both arguments", &op->where);
return false;
}
if (formal->sym->attr.asynchronous != formal->next->sym->attr.asynchronous)
{
- gfc_error ("The function passed as OPERATOR at %L shall have the "
+ gfc_error ("The function passed as OPERATION at %L shall have the "
"ASYNCHRONOUS attribute either for none or both arguments",
&op->where);
return false;
@@ -2352,7 +2352,7 @@ gfc_check_co_reduce (gfc_expr *a, gfc_expr *op, gfc_expr *result_image,
if (formal->sym->attr.optional || formal->next->sym->attr.optional)
{
- gfc_error ("The function passed as OPERATOR at %L shall not have the "
+ gfc_error ("The function passed as OPERATION at %L shall not have the "
"OPTIONAL attribute for either of the arguments", &op->where);
return false;
}
@@ -2383,14 +2383,14 @@ gfc_check_co_reduce (gfc_expr *a, gfc_expr *op, gfc_expr *result_image,
|| (formal_size2 && actual_size != formal_size2)))
{
gfc_error ("The character length of the A argument at %L and of the "
- "arguments of the OP
Re: [patch, fortran, wwwdocs] Fix name of argument to CO_REDUCE
Hi Thomas, Am 07.11.21 um 19:18 schrieb Thomas Koenig via Fortran: Hello world, the attached patches fix the name of the function argument to CO_REDUCE to conform to Fortran 2018 instead of the TR. This is a user-visible change, so I have put this both into changes.html and porting_to.html. Regression-tested. OK for trunk? Best regards Thomas Author: Thomas Koenig Date: 2021-11-07 15:38:35 +0100 Fix keyword name for co_reduce. gcc/fortran/ChangeLog: * intrinsic.c (add_subroutines): Change keyword "operator" to the correct one, "operation". * check.c (gfc_check_co_reduce): Change OPERATOR to OPERATION in error messages. gcc/testsuite/ChangeLog: * gfortran.dg/co_reduce_2.f90: New test. * gfortran.dg/coarray_collective_16.f90: Change OPERATOR to OPERATION. * gfortran.dg/coarray_collective_9.f90: Likewise. Co-authored by: Steve Kargl Steve added an update to gcc/fortran/intrinsic.texi in the PR which is missing here. OK if it is committed with the above. Thanks for the patch! Harald
Re: [PATCH 0/2] fortran: Ignore unused arguments for scalarisation [PR97896]
Hi Mikael, thanks for working on this! Am 07.11.21 um 17:17 schrieb Mikael Morin via Gcc-patches: Hello, I repost this patch series initially targetted at the 11 branch only [1], and that I now would like to commit to master as well before. The problematic case is intrinsic procedures where an argument is actually not used in the code generated (KIND argument of INDEX in the testcase), which confuses the scalariser. Thomas König comitted a change to workaround the problem, but it regressed in PR97896. These patch put the workaround where I think it is more appropriate, namely at the beginning of the scalarisation procedure. This is the patch 2 of the series, preceded with the revert in patch 1. I intend to commit both of them squashed together. Regression-tested on x86_64-linux-gnu. Ok for master and 11 branch? Changes from v1: Rebase on master. [1] https://gcc.gnu.org/pipermail/fortran/2021-August/056329.html Mikael Morin (2): Revert "Remove KIND argument from INDEX so it does not mess up scalarization." fortran: Ignore unused args in scalarization [PR97896] gcc/fortran/intrinsic.c | 48 +++-- gcc/fortran/intrinsic.h | 3 +- gcc/fortran/iresolve.c| 21 ++--- gcc/fortran/trans-array.c | 61 ++- gcc/fortran/trans-array.h | 3 ++ gcc/fortran/trans-decl.c | 24 +-- gcc/fortran/trans-intrinsic.c | 1 + gcc/fortran/trans-stmt.c | 20 + gcc/testsuite/gfortran.dg/index_5.f90 | 23 ++ 9 files changed, 121 insertions(+), 83 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/index_5.f90 LGTM at first sight. But you may want to wait for Tobias or Thomas to take a second look. Thanks for the patch! Harald
