This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch, fortran] PR29397 and PR29400 - parameter arrays with scalar initializers


:REVIEWMAIL:

2006-05-07 Paul Thomas <pault@gcc.gnu.org>

	PR fortran/29397
	PR fortran/29400
	* decl.c (add_init_expr_to_sym): Expand a scalar initializer
	for a parameter array into an array expression with the right
	shape.
	* array.c (spec_dimen_size): Remove static attribute.
	* gfortran.h : Prototype for spec_dimen_size.

I'm concerned that we create a string of constructors for each single element of the constant array. This scales very badly, and even more badly if you consider how gfc_append_constructor() appears to work: for each element, we start from the start of the constructor chain, and go down to the last element to add the new one. I'd suggest to change the code to something like the following:


Index: decl.c
===================================================================
--- decl.c (revision 124460)
+++ decl.c (working copy)
@@ -974,7 +974,44 @@ add_init_expr_to_sym (const char *name,
/* Add initializer. Make sure we keep the ranks sane. */
if (sym->attr.dimension && init->rank == 0)
- init->rank = sym->as->rank;
+ {
+ mpz_t size;
+ if (sym->attr.flavor == FL_PARAMETER
+ && init->expr_type == EXPR_CONSTANT
+ && spec_size (sym->as, &size) == SUCCESS
+ && mpz_cmp_si (size, 0) > 0)
+ {
+ gfc_expr *array;
+ gfc_constructor *c = NULL;
+ int n;
+
+ array = gfc_start_constructor (init->ts.type, init- >ts.kind,
+ &init->where);
+ array->value.constructor = gfc_get_constructor ();
+
+ for (n = 0; n < (int)mpz_get_si (size); n++)
+ {
+ if (c == NULL)
+ c = array->value.constructor;
+ else
+ {
+ c->next = gfc_get_constructor ();
+ c = c->next;
+ }
+
+ c->expr = gfc_copy_expr (init);
+ }
+
+ array->shape = gfc_get_shape (sym->as->rank);
+ for (n = 0; n < sym->as->rank; n++)
+ spec_dimen_size (sym->as, n, &array->shape[n]);
+
+ gfc_free_expr (init);
+ init = array;
+ mpz_clear (size);
+ }
+ init->rank = sym->as->rank;
+ }
sym->value = init;
*initp = NULL;



I didn't regtest it, but simple tests appear to pass and be way faster for large arrays.


So, my question is: why can't we use the repeat element of the constructor structure for that? (I tried, but it appears to be ignored later, as if the array had only one element; the comment on top of the structure definition in gfortran.h hints that it's only used for data initialization).

To summarize: the patch is OK, with the modification I suggest above or something similar.

Thanks,
FX


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]