This is the mail archive of the 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, OOP] PR 45290/45271: pointer initialization / vtab init

Hi Tobias,

>> this patch fixes PR 45271 by giving static initializers to the PPC
>> components of the vtabs. This is made possible by the recent progress
>> on pointer initialization (PR 45290). A prerequisite for vtab
>> initialization was fixing comment #6 of that PR, which is accomplished
>> simply by removing an 'gcc_assert' in 'build_function_decl'.
> I like the patch - the only thing I do not quite understand is the removal
> of the
> - gcc_assert (!sym->backend_decl);
> assert in build_function_decl.
> How does the sym->backend_decl get set? More specifically, I worry about
> generating two declarations for the same procedure as on has later in
> build_function_decl:
> ?sym->backend_decl = fndecl;
> Having two declarations for the same procedures not only wastes memory but
> it causes severe problems as the middle end does not expect this. Using
> -fwhole-file (now default) with -fwhole-program and high optimization
> settings (-O3) causes a lot of wrong optimizations if there are two
> declarations. We had a hard time to fix most of them when enabling
> -fwhole-file by default - thus I do not want to introduce new problems.
> Currently, one has two methods of getting the backend_decl for a procedure:
> a) Via gfc_create_function_decl
> b) Via gfc_get_extern_function_decl
> If in the same file a procedure exists both as external function and is
> defined then a lot of effort is made that both have the share backend_decl.
> Thus, I fear that part of the patch is not correct.

well, I have been wondering about this myself. I think what currently
happens for a module function which is used as an initialization
target in the header of the module is the following:

1) When translating the initializer, we create a backend_decl via
2) When generating the function code, we create another one using
gfc_create_function_decl (overriding the old one).

While the middle end seems to handle this alright, you are right that
we should avoid some of the extra work of doing it twice. Instead of
just removing the assert, what do you think of the following:

@@ -1608,9 +1622,11 @@ build_function_decl (gfc_symbol * sym, bool global
   tree result_decl;
   gfc_formal_arglist *f;

-  gcc_assert (!sym->backend_decl);
   gcc_assert (!sym->attr.external);

+  if (sym->backend_decl)
+    return;
   /* Set the line and filename.  sym->declared_at seems to point to the
      last statement for subroutines, but it'll do for now.  */
   gfc_set_backend_locus (&sym->declared_at);

This will guarantee that we only generate the backend_decl once.
However, this will only work if we use build_function_decl instead of
gfc_get_extern_function_decl when building the decl for the first time
(since they are not equivalent), which can be accomplished by this

@@ -1160,12 +1163,21 @@ gfc_get_symbol_decl (gfc_symbol * sym)

-  /* Catch function declarations.  Only used for actual parameters and
-     procedure pointers.  */
+  /* Catch function declarations.  Only used for actual parameters,
+     procedure pointers and procptr initialization targets.  */
   if (sym->attr.flavor == FL_PROCEDURE)
-      decl = gfc_get_extern_function_decl (sym);
-      gfc_set_decl_location (decl, &sym->declared_at);
+      if (sym->attr.external || sym->attr.use_assoc)
+	{
+	  decl = gfc_get_extern_function_decl (sym);
+	  gfc_set_decl_location (decl, &sym->declared_at);
+	}
+      else
+	{
+	  if (!sym->backend_decl)
+	    build_function_decl (sym, false);
+	  decl = sym->backend_decl;
+	}
       return decl;

Does this look alright? Updated patch attached (regtesting now).


Attachment: vtab_init_v2.diff
Description: Binary data

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