public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [PATCH][fortran] Use a TRANSLATION_UNIT_DECL for DECL_CONTEXT
@ 2010-10-02 13:45 Dominique Dhumieres
  0 siblings, 0 replies; 8+ messages in thread
From: Dominique Dhumieres @ 2010-10-02 13:45 UTC (permalink / raw)
  To: fortran; +Cc: tobias.burnus, stevenb.gcc, rguenther, gcc-patches

Tested without new regression on x86_64-apple-darwin10.4 and powerpc-apple-darwin9
(with revision 164591 reverted).

Dominique

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][fortran] Use a TRANSLATION_UNIT_DECL for DECL_CONTEXT
  2010-10-02 16:45         ` Mikael Morin
@ 2010-10-04  9:20           ` Richard Guenther
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Guenther @ 2010-10-04  9:20 UTC (permalink / raw)
  To: Mikael Morin; +Cc: fortran, Steven Bosscher, gcc-patches

On Sat, 2 Oct 2010, Mikael Morin wrote:

> On Wednesday 29 September 2010 15:16:30 Richard Guenther wrote:
> > On Fri, 24 Sep 2010, Richard Guenther wrote:
> > > On Wed, 22 Sep 2010, Steven Bosscher wrote:
> > > > On Wed, Sep 22, 2010 at 1:08 PM, Richard Guenther <rguenther@suse.de> 
> wrote:
> > > > > On Fri, 17 Sep 2010, Richard Guenther wrote:
> > > > >> This transitions the Fortran frontend to use a TRANSLATION_UNIT_DECL
> > > > >> for DECL_CONTEXT of global entities.  This will allow LTO to produce
> > > > >> proper language specific debug information.
> > > > >> 
> > > > >> The patch leaves some DECL_CONTEXT tests for NULL untouched, as the
> > > > >> FE seems to have both test-for-globalness and
> > > > >> test-for-not-yet-initialized tests - I'd appreciate if somebody
> > > > >> could go over those in the Frontend and use DECL_FILE_SCOPE_P for
> > > > >> those that really mean it.
> > > > >> 
> > > > >> I'm also somewhat confused about binding-level use - for modules
> > > > >> its state seems to be strange.  Maybe somebody can shed a light
> > > > >> on this.  (Possibly module decls want to use a different T_U_D
> > > > >> in the future as well)
> > > > >> 
> > > > >> As of the ???s in the patch:
> > > > >> 
> > > > >>   /* Create the variable.  */
> > > > >>   pushdecl (decl);
> > > > >>   /* Why is current_binding_level == global_binding_level here?
> > > > >>   gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);  */
> > > > >>   gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
> > > > >>   DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
> > > > >> 
> > > > >> this (and the other two similar cases) seems to assert that
> > > > >> pushdecl doesn't set DECL_CONTEXT (well, before my patch
> > > > >> it set it to current_function_decl which is NULL).  So, does
> > > > >> this check that the decl is at file-scope or does it want to
> > > > >> test that current_function_decl is NULL?
> > > > >> 
> > > > >> Bootstrap and regtest re-running after some minor changes on
> > > > >> x86_64-unknown-linux-gnu.  Any idea about the above?
> > > > > 
> > > > > Ping.
> > > > 
> > > > The whole module stuff looks just wrong, but it's not worse or better
> > > > before or after the patch. And your patch is a valid cleanup So OK.
> > > > 
> > > > If I can find a clone of me, I'll put him to work to figure out what's
> > > > going on in the places you marked with "???"...
> > > 
> > > Ok, so the following patch simply removes the asserts on DECL_CONTEXT
> > > when it is set to sth else in the following statement.
> > > 
> > > Bootstrap & regtest state shouldn't have changed with that change,
> > > but I'll re-bootstrap and test before committing.
> > > 
> > > The patch still allows you to debug Fortran code with Fortran
> > > languge when using LTO.
> > > 
> > > Thus, ok for trunk?
> > 
> > Ping.
> 
> Steven already approved the previous version of the patch, and this version 
> has just the ??? comments removed (as far as I know) so I guess you can 
> commit. 
> I didn't see anything clearly wrong in the patch, but honestly I can't tell 
> whether it's good or bad. 
> OK.

Ah, I didn't recognize Stevens comment as approval.

Bootstrapped and tested on x86_64-unknown-linux-gnu, committed
as rev 164928.

Thanks,
Richard.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][fortran] Use a TRANSLATION_UNIT_DECL for DECL_CONTEXT
  2010-09-29 17:44       ` Richard Guenther
@ 2010-10-02 16:45         ` Mikael Morin
  2010-10-04  9:20           ` Richard Guenther
  0 siblings, 1 reply; 8+ messages in thread
From: Mikael Morin @ 2010-10-02 16:45 UTC (permalink / raw)
  To: fortran; +Cc: Richard Guenther, Steven Bosscher, gcc-patches

On Wednesday 29 September 2010 15:16:30 Richard Guenther wrote:
> On Fri, 24 Sep 2010, Richard Guenther wrote:
> > On Wed, 22 Sep 2010, Steven Bosscher wrote:
> > > On Wed, Sep 22, 2010 at 1:08 PM, Richard Guenther <rguenther@suse.de> 
wrote:
> > > > On Fri, 17 Sep 2010, Richard Guenther wrote:
> > > >> This transitions the Fortran frontend to use a TRANSLATION_UNIT_DECL
> > > >> for DECL_CONTEXT of global entities.  This will allow LTO to produce
> > > >> proper language specific debug information.
> > > >> 
> > > >> The patch leaves some DECL_CONTEXT tests for NULL untouched, as the
> > > >> FE seems to have both test-for-globalness and
> > > >> test-for-not-yet-initialized tests - I'd appreciate if somebody
> > > >> could go over those in the Frontend and use DECL_FILE_SCOPE_P for
> > > >> those that really mean it.
> > > >> 
> > > >> I'm also somewhat confused about binding-level use - for modules
> > > >> its state seems to be strange.  Maybe somebody can shed a light
> > > >> on this.  (Possibly module decls want to use a different T_U_D
> > > >> in the future as well)
> > > >> 
> > > >> As of the ???s in the patch:
> > > >> 
> > > >>   /* Create the variable.  */
> > > >>   pushdecl (decl);
> > > >>   /* Why is current_binding_level == global_binding_level here?
> > > >>   gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);  */
> > > >>   gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
> > > >>   DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
> > > >> 
> > > >> this (and the other two similar cases) seems to assert that
> > > >> pushdecl doesn't set DECL_CONTEXT (well, before my patch
> > > >> it set it to current_function_decl which is NULL).  So, does
> > > >> this check that the decl is at file-scope or does it want to
> > > >> test that current_function_decl is NULL?
> > > >> 
> > > >> Bootstrap and regtest re-running after some minor changes on
> > > >> x86_64-unknown-linux-gnu.  Any idea about the above?
> > > > 
> > > > Ping.
> > > 
> > > The whole module stuff looks just wrong, but it's not worse or better
> > > before or after the patch. And your patch is a valid cleanup So OK.
> > > 
> > > If I can find a clone of me, I'll put him to work to figure out what's
> > > going on in the places you marked with "???"...
> > 
> > Ok, so the following patch simply removes the asserts on DECL_CONTEXT
> > when it is set to sth else in the following statement.
> > 
> > Bootstrap & regtest state shouldn't have changed with that change,
> > but I'll re-bootstrap and test before committing.
> > 
> > The patch still allows you to debug Fortran code with Fortran
> > languge when using LTO.
> > 
> > Thus, ok for trunk?
> 
> Ping.

Steven already approved the previous version of the patch, and this version 
has just the ??? comments removed (as far as I know) so I guess you can 
commit. 
I didn't see anything clearly wrong in the patch, but honestly I can't tell 
whether it's good or bad. 
OK.

Mikael

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][fortran] Use a TRANSLATION_UNIT_DECL for DECL_CONTEXT
  2010-09-24 18:25     ` Richard Guenther
@ 2010-09-29 17:44       ` Richard Guenther
  2010-10-02 16:45         ` Mikael Morin
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Guenther @ 2010-09-29 17:44 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: gcc-patches, fortran

[-- Attachment #1: Type: TEXT/PLAIN, Size: 11449 bytes --]

On Fri, 24 Sep 2010, Richard Guenther wrote:

> On Wed, 22 Sep 2010, Steven Bosscher wrote:
> 
> > On Wed, Sep 22, 2010 at 1:08 PM, Richard Guenther <rguenther@suse.de> wrote:
> > > On Fri, 17 Sep 2010, Richard Guenther wrote:
> > >
> > >>
> > >> This transitions the Fortran frontend to use a TRANSLATION_UNIT_DECL
> > >> for DECL_CONTEXT of global entities.  This will allow LTO to produce
> > >> proper language specific debug information.
> > >>
> > >> The patch leaves some DECL_CONTEXT tests for NULL untouched, as the
> > >> FE seems to have both test-for-globalness and test-for-not-yet-initialized
> > >> tests - I'd appreciate if somebody could go over those in the Frontend
> > >> and use DECL_FILE_SCOPE_P for those that really mean it.
> > >>
> > >> I'm also somewhat confused about binding-level use - for modules
> > >> its state seems to be strange.  Maybe somebody can shed a light
> > >> on this.  (Possibly module decls want to use a different T_U_D
> > >> in the future as well)
> > >>
> > >> As of the ???s in the patch:
> > >>
> > >>   /* Create the variable.  */
> > >>   pushdecl (decl);
> > >>   /* Why is current_binding_level == global_binding_level here?
> > >>   gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);  */
> > >>   gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
> > >>   DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
> > >>
> > >> this (and the other two similar cases) seems to assert that
> > >> pushdecl doesn't set DECL_CONTEXT (well, before my patch
> > >> it set it to current_function_decl which is NULL).  So, does
> > >> this check that the decl is at file-scope or does it want to
> > >> test that current_function_decl is NULL?
> > >>
> > >> Bootstrap and regtest re-running after some minor changes on
> > >> x86_64-unknown-linux-gnu.  Any idea about the above?
> > >
> > > Ping.
> > 
> > The whole module stuff looks just wrong, but it's not worse or better
> > before or after the patch. And your patch is a valid cleanup So OK.
> > 
> > If I can find a clone of me, I'll put him to work to figure out what's
> > going on in the places you marked with "???"...
> 
> Ok, so the following patch simply removes the asserts on DECL_CONTEXT
> when it is set to sth else in the following statement.
> 
> Bootstrap & regtest state shouldn't have changed with that change,
> but I'll re-bootstrap and test before committing.
> 
> The patch still allows you to debug Fortran code with Fortran
> languge when using LTO.
> 
> Thus, ok for trunk?

Ping.

> Thanks,
> Richard.
> 
> 2010-09-24  Richard Guenther  <rguenther@suse.de>
> 
> 	* f95-lang.c (current_translation_unit): New global variable.
> 	(gfc_create_decls): Build a translation-unit decl.
> 	(pushdecl): In the global binding-level use the
> 	translation-unit decl as DECL_CONTEXT.
> 	* trans-decl.c (gfc_get_symbol_decl): Use DECL_FILE_SCOPE_P.
> 	(build_function_decl): Likewise.  Delay setting the assembler
> 	name, leave setting of DECL_CONTEXT to pushdecl.
> 	(trans_function_start): Use DECL_FILE_SCOPE_P.
> 	(gfc_create_module_variable): Likewise.  Remove questionable
> 	asserts.
> 	* trans.c (gfc_generate_module_code): Likewise.
> 
> Index: gcc/fortran/f95-lang.c
> ===================================================================
> *** gcc/fortran/f95-lang.c.orig	2010-09-17 15:55:55.000000000 +0200
> --- gcc/fortran/f95-lang.c	2010-09-24 15:21:44.000000000 +0200
> *************** tree *ridpointers = NULL;
> *** 172,177 ****
> --- 172,180 ----
>   /* True means we've initialized exception handling.  */
>   bool gfc_eh_initialized_p;
>   
> + /* The current translation unit.  */
> + static GTY(()) tree current_translation_unit;
> + 
>   
>   /* Prepare expr to be an argument of a TRUTH_NOT_EXPR,
>      or validate its data type for an `if' or `while' statement or ?..: exp.
> *************** gfc_create_decls (void)
> *** 229,234 ****
> --- 232,240 ----
>     gfc_build_builtin_function_decls ();
>   
>     gfc_init_constants ();
> + 
> +   /* Build our translation-unit decl.  */
> +   current_translation_unit = build_translation_unit_decl (NULL_TREE);
>   }
>   
>   
> *************** tree
> *** 491,498 ****
>   pushdecl (tree decl)
>   {
>     /* External objects aren't nested, other objects may be.  */
> !   if ((DECL_EXTERNAL (decl)) || (decl == current_function_decl))
> !     DECL_CONTEXT (decl) = 0;
>     else
>       DECL_CONTEXT (decl) = current_function_decl;
>   
> --- 497,506 ----
>   pushdecl (tree decl)
>   {
>     /* External objects aren't nested, other objects may be.  */
> !   if (DECL_EXTERNAL (decl))
> !     DECL_CONTEXT (decl) = NULL_TREE;
> !   else if (global_bindings_p ())
> !     DECL_CONTEXT (decl) = current_translation_unit;
>     else
>       DECL_CONTEXT (decl) = current_function_decl;
>   
> Index: gcc/fortran/trans-decl.c
> ===================================================================
> *** gcc/fortran/trans-decl.c.orig	2010-09-24 11:34:48.000000000 +0200
> --- gcc/fortran/trans-decl.c	2010-09-24 15:22:14.000000000 +0200
> *************** gfc_get_symbol_decl (gfc_symbol * sym)
> *** 1090,1096 ****
>   	  else
>   	    length = sym->ts.u.cl->backend_decl;
>   	  if (TREE_CODE (length) == VAR_DECL
> ! 	      && DECL_CONTEXT (length) == NULL_TREE)
>   	    {
>   	      /* Add the string length to the same context as the symbol.  */
>   	      if (DECL_CONTEXT (sym->backend_decl) == current_function_decl)
> --- 1090,1096 ----
>   	  else
>   	    length = sym->ts.u.cl->backend_decl;
>   	  if (TREE_CODE (length) == VAR_DECL
> ! 	      && DECL_FILE_SCOPE_P (length))
>   	    {
>   	      /* Add the string length to the same context as the symbol.  */
>   	      if (DECL_CONTEXT (sym->backend_decl) == current_function_decl)
> *************** build_function_decl (gfc_symbol * sym, b
> *** 1645,1653 ****
>   
>     /* Allow only one nesting level.  Allow public declarations.  */
>     gcc_assert (current_function_decl == NULL_TREE
> ! 	      || DECL_CONTEXT (current_function_decl) == NULL_TREE
> ! 	      || TREE_CODE (DECL_CONTEXT (current_function_decl))
> ! 		 == NAMESPACE_DECL);
>   
>     type = gfc_get_function_type (sym);
>     fndecl = build_decl (input_location,
> --- 1645,1653 ----
>   
>     /* Allow only one nesting level.  Allow public declarations.  */
>     gcc_assert (current_function_decl == NULL_TREE
> ! 	      || DECL_FILE_SCOPE_P (current_function_decl)
> ! 	      || (TREE_CODE (DECL_CONTEXT (current_function_decl))
> ! 		  == NAMESPACE_DECL));
>   
>     type = gfc_get_function_type (sym);
>     fndecl = build_decl (input_location,
> *************** build_function_decl (gfc_symbol * sym, b
> *** 1658,1667 ****
>     attributes = add_attributes_to_decl (attr, NULL_TREE);
>     decl_attributes (&fndecl, attributes, 0);
>   
> -   /* Perform name mangling if this is a top level or module procedure.  */
> -   if (current_function_decl == NULL_TREE)
> -     gfc_set_decl_assembler_name (fndecl, gfc_sym_mangled_function_id (sym));
> - 
>     /* Figure out the return type of the declared function, and build a
>        RESULT_DECL for it.  If this is a subroutine with alternate
>        returns, build a RESULT_DECL for it.  */
> --- 1658,1663 ----
> *************** build_function_decl (gfc_symbol * sym, b
> *** 1709,1720 ****
>        layout_decl (result_decl, 0);  */
>   
>     /* Set up all attributes for the function.  */
> -   DECL_CONTEXT (fndecl) = current_function_decl;
>     DECL_EXTERNAL (fndecl) = 0;
>   
>     /* This specifies if a function is globally visible, i.e. it is
>        the opposite of declaring static in C.  */
> !   if (DECL_CONTEXT (fndecl) == NULL_TREE
>         && !sym->attr.entry_master && !sym->attr.is_main_program)
>       TREE_PUBLIC (fndecl) = 1;
>   
> --- 1705,1715 ----
>        layout_decl (result_decl, 0);  */
>   
>     /* Set up all attributes for the function.  */
>     DECL_EXTERNAL (fndecl) = 0;
>   
>     /* This specifies if a function is globally visible, i.e. it is
>        the opposite of declaring static in C.  */
> !   if (!current_function_decl
>         && !sym->attr.entry_master && !sym->attr.is_main_program)
>       TREE_PUBLIC (fndecl) = 1;
>   
> *************** build_function_decl (gfc_symbol * sym, b
> *** 1743,1748 ****
> --- 1738,1747 ----
>     else
>       pushdecl (fndecl);
>   
> +   /* Perform name mangling if this is a top level or module procedure.  */
> +   if (current_function_decl == NULL_TREE)
> +     gfc_set_decl_assembler_name (fndecl, gfc_sym_mangled_function_id (sym));
> + 
>     sym->backend_decl = fndecl;
>   }
>   
> *************** trans_function_start (gfc_symbol * sym)
> *** 1990,1996 ****
>     /* Let the world know what we're about to do.  */
>     announce_function (fndecl);
>   
> !   if (DECL_CONTEXT (fndecl) == NULL_TREE)
>       {
>         /* Create RTL for function declaration.  */
>         rest_of_decl_compilation (fndecl, 1, 0);
> --- 1989,1995 ----
>     /* Let the world know what we're about to do.  */
>     announce_function (fndecl);
>   
> !   if (DECL_FILE_SCOPE_P (fndecl))
>       {
>         /* Create RTL for function declaration.  */
>         rest_of_decl_compilation (fndecl, 1, 0);
> *************** gfc_create_module_variable (gfc_symbol *
> *** 3597,3603 ****
>     if ((sym->attr.in_common || sym->attr.in_equivalence) && sym->backend_decl)
>       {
>         decl = sym->backend_decl;
> !       gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);
>         gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
>         DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
>         gfc_module_add_decl (cur_module, decl);
> --- 3596,3602 ----
>     if ((sym->attr.in_common || sym->attr.in_equivalence) && sym->backend_decl)
>       {
>         decl = sym->backend_decl;
> !       gcc_assert (DECL_FILE_SCOPE_P (decl));
>         gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
>         DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
>         gfc_module_add_decl (cur_module, decl);
> *************** gfc_create_module_variable (gfc_symbol *
> *** 3624,3630 ****
>   
>     /* Create the variable.  */
>     pushdecl (decl);
> -   gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);
>     gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
>     DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
>     rest_of_decl_compilation (decl, 1, 0);
> --- 3623,3628 ----
> Index: gcc/fortran/trans.c
> ===================================================================
> *** gcc/fortran/trans.c.orig	2010-09-17 15:55:55.000000000 +0200
> --- gcc/fortran/trans.c	2010-09-24 15:23:08.000000000 +0200
> *************** gfc_generate_module_code (gfc_namespace
> *** 1414,1425 ****
>           continue;
>   
>         gfc_create_function_decl (n, false);
> -       gcc_assert (DECL_CONTEXT (n->proc_name->backend_decl) == NULL_TREE);
>         DECL_CONTEXT (n->proc_name->backend_decl) = ns->proc_name->backend_decl;
>         gfc_module_add_decl (entry, n->proc_name->backend_decl);
>         for (el = ns->entries; el; el = el->next)
>   	{
> - 	  gcc_assert (DECL_CONTEXT (el->sym->backend_decl) == NULL_TREE);
>   	  DECL_CONTEXT (el->sym->backend_decl) = ns->proc_name->backend_decl;
>   	  gfc_module_add_decl (entry, el->sym->backend_decl);
>   	}
> --- 1414,1423 ----

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][fortran] Use a TRANSLATION_UNIT_DECL for DECL_CONTEXT
  2010-09-23  0:28   ` Steven Bosscher
@ 2010-09-24 18:25     ` Richard Guenther
  2010-09-29 17:44       ` Richard Guenther
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Guenther @ 2010-09-24 18:25 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: gcc-patches, fortran

[-- Attachment #1: Type: TEXT/PLAIN, Size: 10696 bytes --]

On Wed, 22 Sep 2010, Steven Bosscher wrote:

> On Wed, Sep 22, 2010 at 1:08 PM, Richard Guenther <rguenther@suse.de> wrote:
> > On Fri, 17 Sep 2010, Richard Guenther wrote:
> >
> >>
> >> This transitions the Fortran frontend to use a TRANSLATION_UNIT_DECL
> >> for DECL_CONTEXT of global entities.  This will allow LTO to produce
> >> proper language specific debug information.
> >>
> >> The patch leaves some DECL_CONTEXT tests for NULL untouched, as the
> >> FE seems to have both test-for-globalness and test-for-not-yet-initialized
> >> tests - I'd appreciate if somebody could go over those in the Frontend
> >> and use DECL_FILE_SCOPE_P for those that really mean it.
> >>
> >> I'm also somewhat confused about binding-level use - for modules
> >> its state seems to be strange.  Maybe somebody can shed a light
> >> on this.  (Possibly module decls want to use a different T_U_D
> >> in the future as well)
> >>
> >> As of the ???s in the patch:
> >>
> >>   /* Create the variable.  */
> >>   pushdecl (decl);
> >>   /* Why is current_binding_level == global_binding_level here?
> >>   gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);  */
> >>   gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
> >>   DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
> >>
> >> this (and the other two similar cases) seems to assert that
> >> pushdecl doesn't set DECL_CONTEXT (well, before my patch
> >> it set it to current_function_decl which is NULL).  So, does
> >> this check that the decl is at file-scope or does it want to
> >> test that current_function_decl is NULL?
> >>
> >> Bootstrap and regtest re-running after some minor changes on
> >> x86_64-unknown-linux-gnu.  Any idea about the above?
> >
> > Ping.
> 
> The whole module stuff looks just wrong, but it's not worse or better
> before or after the patch. And your patch is a valid cleanup So OK.
> 
> If I can find a clone of me, I'll put him to work to figure out what's
> going on in the places you marked with "???"...

Ok, so the following patch simply removes the asserts on DECL_CONTEXT
when it is set to sth else in the following statement.

Bootstrap & regtest state shouldn't have changed with that change,
but I'll re-bootstrap and test before committing.

The patch still allows you to debug Fortran code with Fortran
languge when using LTO.

Thus, ok for trunk?

Thanks,
Richard.

2010-09-24  Richard Guenther  <rguenther@suse.de>

	* f95-lang.c (current_translation_unit): New global variable.
	(gfc_create_decls): Build a translation-unit decl.
	(pushdecl): In the global binding-level use the
	translation-unit decl as DECL_CONTEXT.
	* trans-decl.c (gfc_get_symbol_decl): Use DECL_FILE_SCOPE_P.
	(build_function_decl): Likewise.  Delay setting the assembler
	name, leave setting of DECL_CONTEXT to pushdecl.
	(trans_function_start): Use DECL_FILE_SCOPE_P.
	(gfc_create_module_variable): Likewise.  Remove questionable
	asserts.
	* trans.c (gfc_generate_module_code): Likewise.

Index: gcc/fortran/f95-lang.c
===================================================================
*** gcc/fortran/f95-lang.c.orig	2010-09-17 15:55:55.000000000 +0200
--- gcc/fortran/f95-lang.c	2010-09-24 15:21:44.000000000 +0200
*************** tree *ridpointers = NULL;
*** 172,177 ****
--- 172,180 ----
  /* True means we've initialized exception handling.  */
  bool gfc_eh_initialized_p;
  
+ /* The current translation unit.  */
+ static GTY(()) tree current_translation_unit;
+ 
  
  /* Prepare expr to be an argument of a TRUTH_NOT_EXPR,
     or validate its data type for an `if' or `while' statement or ?..: exp.
*************** gfc_create_decls (void)
*** 229,234 ****
--- 232,240 ----
    gfc_build_builtin_function_decls ();
  
    gfc_init_constants ();
+ 
+   /* Build our translation-unit decl.  */
+   current_translation_unit = build_translation_unit_decl (NULL_TREE);
  }
  
  
*************** tree
*** 491,498 ****
  pushdecl (tree decl)
  {
    /* External objects aren't nested, other objects may be.  */
!   if ((DECL_EXTERNAL (decl)) || (decl == current_function_decl))
!     DECL_CONTEXT (decl) = 0;
    else
      DECL_CONTEXT (decl) = current_function_decl;
  
--- 497,506 ----
  pushdecl (tree decl)
  {
    /* External objects aren't nested, other objects may be.  */
!   if (DECL_EXTERNAL (decl))
!     DECL_CONTEXT (decl) = NULL_TREE;
!   else if (global_bindings_p ())
!     DECL_CONTEXT (decl) = current_translation_unit;
    else
      DECL_CONTEXT (decl) = current_function_decl;
  
Index: gcc/fortran/trans-decl.c
===================================================================
*** gcc/fortran/trans-decl.c.orig	2010-09-24 11:34:48.000000000 +0200
--- gcc/fortran/trans-decl.c	2010-09-24 15:22:14.000000000 +0200
*************** gfc_get_symbol_decl (gfc_symbol * sym)
*** 1090,1096 ****
  	  else
  	    length = sym->ts.u.cl->backend_decl;
  	  if (TREE_CODE (length) == VAR_DECL
! 	      && DECL_CONTEXT (length) == NULL_TREE)
  	    {
  	      /* Add the string length to the same context as the symbol.  */
  	      if (DECL_CONTEXT (sym->backend_decl) == current_function_decl)
--- 1090,1096 ----
  	  else
  	    length = sym->ts.u.cl->backend_decl;
  	  if (TREE_CODE (length) == VAR_DECL
! 	      && DECL_FILE_SCOPE_P (length))
  	    {
  	      /* Add the string length to the same context as the symbol.  */
  	      if (DECL_CONTEXT (sym->backend_decl) == current_function_decl)
*************** build_function_decl (gfc_symbol * sym, b
*** 1645,1653 ****
  
    /* Allow only one nesting level.  Allow public declarations.  */
    gcc_assert (current_function_decl == NULL_TREE
! 	      || DECL_CONTEXT (current_function_decl) == NULL_TREE
! 	      || TREE_CODE (DECL_CONTEXT (current_function_decl))
! 		 == NAMESPACE_DECL);
  
    type = gfc_get_function_type (sym);
    fndecl = build_decl (input_location,
--- 1645,1653 ----
  
    /* Allow only one nesting level.  Allow public declarations.  */
    gcc_assert (current_function_decl == NULL_TREE
! 	      || DECL_FILE_SCOPE_P (current_function_decl)
! 	      || (TREE_CODE (DECL_CONTEXT (current_function_decl))
! 		  == NAMESPACE_DECL));
  
    type = gfc_get_function_type (sym);
    fndecl = build_decl (input_location,
*************** build_function_decl (gfc_symbol * sym, b
*** 1658,1667 ****
    attributes = add_attributes_to_decl (attr, NULL_TREE);
    decl_attributes (&fndecl, attributes, 0);
  
-   /* Perform name mangling if this is a top level or module procedure.  */
-   if (current_function_decl == NULL_TREE)
-     gfc_set_decl_assembler_name (fndecl, gfc_sym_mangled_function_id (sym));
- 
    /* Figure out the return type of the declared function, and build a
       RESULT_DECL for it.  If this is a subroutine with alternate
       returns, build a RESULT_DECL for it.  */
--- 1658,1663 ----
*************** build_function_decl (gfc_symbol * sym, b
*** 1709,1720 ****
       layout_decl (result_decl, 0);  */
  
    /* Set up all attributes for the function.  */
-   DECL_CONTEXT (fndecl) = current_function_decl;
    DECL_EXTERNAL (fndecl) = 0;
  
    /* This specifies if a function is globally visible, i.e. it is
       the opposite of declaring static in C.  */
!   if (DECL_CONTEXT (fndecl) == NULL_TREE
        && !sym->attr.entry_master && !sym->attr.is_main_program)
      TREE_PUBLIC (fndecl) = 1;
  
--- 1705,1715 ----
       layout_decl (result_decl, 0);  */
  
    /* Set up all attributes for the function.  */
    DECL_EXTERNAL (fndecl) = 0;
  
    /* This specifies if a function is globally visible, i.e. it is
       the opposite of declaring static in C.  */
!   if (!current_function_decl
        && !sym->attr.entry_master && !sym->attr.is_main_program)
      TREE_PUBLIC (fndecl) = 1;
  
*************** build_function_decl (gfc_symbol * sym, b
*** 1743,1748 ****
--- 1738,1747 ----
    else
      pushdecl (fndecl);
  
+   /* Perform name mangling if this is a top level or module procedure.  */
+   if (current_function_decl == NULL_TREE)
+     gfc_set_decl_assembler_name (fndecl, gfc_sym_mangled_function_id (sym));
+ 
    sym->backend_decl = fndecl;
  }
  
*************** trans_function_start (gfc_symbol * sym)
*** 1990,1996 ****
    /* Let the world know what we're about to do.  */
    announce_function (fndecl);
  
!   if (DECL_CONTEXT (fndecl) == NULL_TREE)
      {
        /* Create RTL for function declaration.  */
        rest_of_decl_compilation (fndecl, 1, 0);
--- 1989,1995 ----
    /* Let the world know what we're about to do.  */
    announce_function (fndecl);
  
!   if (DECL_FILE_SCOPE_P (fndecl))
      {
        /* Create RTL for function declaration.  */
        rest_of_decl_compilation (fndecl, 1, 0);
*************** gfc_create_module_variable (gfc_symbol *
*** 3597,3603 ****
    if ((sym->attr.in_common || sym->attr.in_equivalence) && sym->backend_decl)
      {
        decl = sym->backend_decl;
!       gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);
        gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
        DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
        gfc_module_add_decl (cur_module, decl);
--- 3596,3602 ----
    if ((sym->attr.in_common || sym->attr.in_equivalence) && sym->backend_decl)
      {
        decl = sym->backend_decl;
!       gcc_assert (DECL_FILE_SCOPE_P (decl));
        gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
        DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
        gfc_module_add_decl (cur_module, decl);
*************** gfc_create_module_variable (gfc_symbol *
*** 3624,3630 ****
  
    /* Create the variable.  */
    pushdecl (decl);
-   gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);
    gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
    DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
    rest_of_decl_compilation (decl, 1, 0);
--- 3623,3628 ----
Index: gcc/fortran/trans.c
===================================================================
*** gcc/fortran/trans.c.orig	2010-09-17 15:55:55.000000000 +0200
--- gcc/fortran/trans.c	2010-09-24 15:23:08.000000000 +0200
*************** gfc_generate_module_code (gfc_namespace
*** 1414,1425 ****
          continue;
  
        gfc_create_function_decl (n, false);
-       gcc_assert (DECL_CONTEXT (n->proc_name->backend_decl) == NULL_TREE);
        DECL_CONTEXT (n->proc_name->backend_decl) = ns->proc_name->backend_decl;
        gfc_module_add_decl (entry, n->proc_name->backend_decl);
        for (el = ns->entries; el; el = el->next)
  	{
- 	  gcc_assert (DECL_CONTEXT (el->sym->backend_decl) == NULL_TREE);
  	  DECL_CONTEXT (el->sym->backend_decl) = ns->proc_name->backend_decl;
  	  gfc_module_add_decl (entry, el->sym->backend_decl);
  	}
--- 1414,1423 ----

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][fortran] Use a TRANSLATION_UNIT_DECL for DECL_CONTEXT
  2010-09-22 20:06 ` Richard Guenther
@ 2010-09-23  0:28   ` Steven Bosscher
  2010-09-24 18:25     ` Richard Guenther
  0 siblings, 1 reply; 8+ messages in thread
From: Steven Bosscher @ 2010-09-23  0:28 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, fortran

On Wed, Sep 22, 2010 at 1:08 PM, Richard Guenther <rguenther@suse.de> wrote:
> On Fri, 17 Sep 2010, Richard Guenther wrote:
>
>>
>> This transitions the Fortran frontend to use a TRANSLATION_UNIT_DECL
>> for DECL_CONTEXT of global entities.  This will allow LTO to produce
>> proper language specific debug information.
>>
>> The patch leaves some DECL_CONTEXT tests for NULL untouched, as the
>> FE seems to have both test-for-globalness and test-for-not-yet-initialized
>> tests - I'd appreciate if somebody could go over those in the Frontend
>> and use DECL_FILE_SCOPE_P for those that really mean it.
>>
>> I'm also somewhat confused about binding-level use - for modules
>> its state seems to be strange.  Maybe somebody can shed a light
>> on this.  (Possibly module decls want to use a different T_U_D
>> in the future as well)
>>
>> As of the ???s in the patch:
>>
>>   /* Create the variable.  */
>>   pushdecl (decl);
>>   /* Why is current_binding_level == global_binding_level here?
>>   gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);  */
>>   gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
>>   DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
>>
>> this (and the other two similar cases) seems to assert that
>> pushdecl doesn't set DECL_CONTEXT (well, before my patch
>> it set it to current_function_decl which is NULL).  So, does
>> this check that the decl is at file-scope or does it want to
>> test that current_function_decl is NULL?
>>
>> Bootstrap and regtest re-running after some minor changes on
>> x86_64-unknown-linux-gnu.  Any idea about the above?
>
> Ping.

The whole module stuff looks just wrong, but it's not worse or better
before or after the patch. And your patch is a valid cleanup So OK.

If I can find a clone of me, I'll put him to work to figure out what's
going on in the places you marked with "???"...

Ciao!
Steven

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][fortran] Use a TRANSLATION_UNIT_DECL for DECL_CONTEXT
  2010-09-17 12:51 Richard Guenther
@ 2010-09-22 20:06 ` Richard Guenther
  2010-09-23  0:28   ` Steven Bosscher
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Guenther @ 2010-09-22 20:06 UTC (permalink / raw)
  To: gcc-patches; +Cc: fortran

On Fri, 17 Sep 2010, Richard Guenther wrote:

> 
> This transitions the Fortran frontend to use a TRANSLATION_UNIT_DECL
> for DECL_CONTEXT of global entities.  This will allow LTO to produce
> proper language specific debug information.
> 
> The patch leaves some DECL_CONTEXT tests for NULL untouched, as the
> FE seems to have both test-for-globalness and test-for-not-yet-initialized
> tests - I'd appreciate if somebody could go over those in the Frontend
> and use DECL_FILE_SCOPE_P for those that really mean it.
> 
> I'm also somewhat confused about binding-level use - for modules
> its state seems to be strange.  Maybe somebody can shed a light
> on this.  (Possibly module decls want to use a different T_U_D
> in the future as well)
> 
> As of the ???s in the patch:
> 
>   /* Create the variable.  */
>   pushdecl (decl);
>   /* Why is current_binding_level == global_binding_level here?  
>   gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);  */
>   gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
>   DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
> 
> this (and the other two similar cases) seems to assert that
> pushdecl doesn't set DECL_CONTEXT (well, before my patch
> it set it to current_function_decl which is NULL).  So, does
> this check that the decl is at file-scope or does it want to
> test that current_function_decl is NULL?
> 
> Bootstrap and regtest re-running after some minor changes on
> x86_64-unknown-linux-gnu.  Any idea about the above?

Ping.

> Thanks,
> Richard.
> 
> 2010-09-17  Richard Guenther  <rguenther@suse.de>
> 
> 	* f95-lang.c (current_translation_unit): New global variable.
> 	(gfc_create_decls): Build a translation-unit decl.
> 	(pushdecl): In the global binding-level use the
> 	translation-unit decl as DECL_CONTEXT.
> 	* trans-decl.c (gfc_get_symbol_decl): Use DECL_FILE_SCOPE_P.
> 	(build_function_decl): Likewise.  Delay setting the assembler
> 	name, leave setting of DECL_CONTEXT to pushdecl.
> 	(trans_function_start): Use DECL_FILE_SCOPE_P.
> 	(gfc_create_module_variable): Likewise.  Add some ???s
> 	* trans.c (gfc_generate_module_code): Likewise.
> 
> Index: gcc/fortran/f95-lang.c
> ===================================================================
> *** gcc/fortran/f95-lang.c.orig	2010-09-17 12:40:00.000000000 +0200
> --- gcc/fortran/f95-lang.c	2010-09-17 13:42:18.000000000 +0200
> *************** tree *ridpointers = NULL;
> *** 172,177 ****
> --- 172,180 ----
>   /* True means we've initialized exception handling.  */
>   bool gfc_eh_initialized_p;
>   
> + /* The current translation unit.  */
> + static GTY(()) tree current_translation_unit;
> + 
>   
>   /* Prepare expr to be an argument of a TRUTH_NOT_EXPR,
>      or validate its data type for an `if' or `while' statement or ?..: exp.
> *************** gfc_create_decls (void)
> *** 229,234 ****
> --- 232,240 ----
>     gfc_build_builtin_function_decls ();
>   
>     gfc_init_constants ();
> + 
> +   /* Build our translation-unit decl.  */
> +   current_translation_unit = build_translation_unit_decl (NULL_TREE);
>   }
>   
>   
> *************** tree
> *** 491,498 ****
>   pushdecl (tree decl)
>   {
>     /* External objects aren't nested, other objects may be.  */
> !   if ((DECL_EXTERNAL (decl)) || (decl == current_function_decl))
> !     DECL_CONTEXT (decl) = 0;
>     else
>       DECL_CONTEXT (decl) = current_function_decl;
>   
> --- 497,506 ----
>   pushdecl (tree decl)
>   {
>     /* External objects aren't nested, other objects may be.  */
> !   if (DECL_EXTERNAL (decl))
> !     DECL_CONTEXT (decl) = NULL_TREE;
> !   else if (global_bindings_p ())
> !     DECL_CONTEXT (decl) = current_translation_unit;
>     else
>       DECL_CONTEXT (decl) = current_function_decl;
>   
> Index: gcc/fortran/trans-decl.c
> ===================================================================
> *** gcc/fortran/trans-decl.c.orig	2010-09-17 12:40:00.000000000 +0200
> --- gcc/fortran/trans-decl.c	2010-09-17 13:40:49.000000000 +0200
> *************** gfc_get_symbol_decl (gfc_symbol * sym)
> *** 1089,1095 ****
>   	  else
>   	    length = sym->ts.u.cl->backend_decl;
>   	  if (TREE_CODE (length) == VAR_DECL
> ! 	      && DECL_CONTEXT (length) == NULL_TREE)
>   	    {
>   	      /* Add the string length to the same context as the symbol.  */
>   	      if (DECL_CONTEXT (sym->backend_decl) == current_function_decl)
> --- 1089,1095 ----
>   	  else
>   	    length = sym->ts.u.cl->backend_decl;
>   	  if (TREE_CODE (length) == VAR_DECL
> ! 	      && DECL_FILE_SCOPE_P (length))
>   	    {
>   	      /* Add the string length to the same context as the symbol.  */
>   	      if (DECL_CONTEXT (sym->backend_decl) == current_function_decl)
> *************** build_function_decl (gfc_symbol * sym, b
> *** 1631,1639 ****
>   
>     /* Allow only one nesting level.  Allow public declarations.  */
>     gcc_assert (current_function_decl == NULL_TREE
> ! 	      || DECL_CONTEXT (current_function_decl) == NULL_TREE
> ! 	      || TREE_CODE (DECL_CONTEXT (current_function_decl))
> ! 		 == NAMESPACE_DECL);
>   
>     type = gfc_get_function_type (sym);
>     fndecl = build_decl (input_location,
> --- 1631,1639 ----
>   
>     /* Allow only one nesting level.  Allow public declarations.  */
>     gcc_assert (current_function_decl == NULL_TREE
> ! 	      || DECL_FILE_SCOPE_P (current_function_decl)
> ! 	      || (TREE_CODE (DECL_CONTEXT (current_function_decl))
> ! 		  == NAMESPACE_DECL));
>   
>     type = gfc_get_function_type (sym);
>     fndecl = build_decl (input_location,
> *************** build_function_decl (gfc_symbol * sym, b
> *** 1644,1653 ****
>     attributes = add_attributes_to_decl (attr, NULL_TREE);
>     decl_attributes (&fndecl, attributes, 0);
>   
> -   /* Perform name mangling if this is a top level or module procedure.  */
> -   if (current_function_decl == NULL_TREE)
> -     gfc_set_decl_assembler_name (fndecl, gfc_sym_mangled_function_id (sym));
> - 
>     /* Figure out the return type of the declared function, and build a
>        RESULT_DECL for it.  If this is a subroutine with alternate
>        returns, build a RESULT_DECL for it.  */
> --- 1644,1649 ----
> *************** build_function_decl (gfc_symbol * sym, b
> *** 1695,1706 ****
>        layout_decl (result_decl, 0);  */
>   
>     /* Set up all attributes for the function.  */
> -   DECL_CONTEXT (fndecl) = current_function_decl;
>     DECL_EXTERNAL (fndecl) = 0;
>   
>     /* This specifies if a function is globally visible, i.e. it is
>        the opposite of declaring static in C.  */
> !   if (DECL_CONTEXT (fndecl) == NULL_TREE
>         && !sym->attr.entry_master && !sym->attr.is_main_program)
>       TREE_PUBLIC (fndecl) = 1;
>   
> --- 1691,1701 ----
>        layout_decl (result_decl, 0);  */
>   
>     /* Set up all attributes for the function.  */
>     DECL_EXTERNAL (fndecl) = 0;
>   
>     /* This specifies if a function is globally visible, i.e. it is
>        the opposite of declaring static in C.  */
> !   if (!current_function_decl
>         && !sym->attr.entry_master && !sym->attr.is_main_program)
>       TREE_PUBLIC (fndecl) = 1;
>   
> *************** build_function_decl (gfc_symbol * sym, b
> *** 1729,1734 ****
> --- 1724,1733 ----
>     else
>       pushdecl (fndecl);
>   
> +   /* Perform name mangling if this is a top level or module procedure.  */
> +   if (current_function_decl == NULL_TREE)
> +     gfc_set_decl_assembler_name (fndecl, gfc_sym_mangled_function_id (sym));
> + 
>     sym->backend_decl = fndecl;
>   }
>   
> *************** trans_function_start (gfc_symbol * sym)
> *** 1976,1982 ****
>     /* Let the world know what we're about to do.  */
>     announce_function (fndecl);
>   
> !   if (DECL_CONTEXT (fndecl) == NULL_TREE)
>       {
>         /* Create RTL for function declaration.  */
>         rest_of_decl_compilation (fndecl, 1, 0);
> --- 1975,1981 ----
>     /* Let the world know what we're about to do.  */
>     announce_function (fndecl);
>   
> !   if (DECL_FILE_SCOPE_P (fndecl))
>       {
>         /* Create RTL for function declaration.  */
>         rest_of_decl_compilation (fndecl, 1, 0);
> *************** gfc_create_module_variable (gfc_symbol *
> *** 3583,3589 ****
>     if ((sym->attr.in_common || sym->attr.in_equivalence) && sym->backend_decl)
>       {
>         decl = sym->backend_decl;
> !       gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);
>         gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
>         DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
>         gfc_module_add_decl (cur_module, decl);
> --- 3582,3588 ----
>     if ((sym->attr.in_common || sym->attr.in_equivalence) && sym->backend_decl)
>       {
>         decl = sym->backend_decl;
> !       gcc_assert (DECL_FILE_SCOPE_P (decl));
>         gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
>         DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
>         gfc_module_add_decl (cur_module, decl);
> *************** gfc_create_module_variable (gfc_symbol *
> *** 3610,3616 ****
>   
>     /* Create the variable.  */
>     pushdecl (decl);
> !   gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);
>     gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
>     DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
>     rest_of_decl_compilation (decl, 1, 0);
> --- 3609,3616 ----
>   
>     /* Create the variable.  */
>     pushdecl (decl);
> !   /* Why is current_binding_level == global_binding_level here?
> !   gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);  */
>     gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
>     DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
>     rest_of_decl_compilation (decl, 1, 0);
> Index: gcc/fortran/trans.c
> ===================================================================
> *** gcc/fortran/trans.c.orig	2010-09-17 12:40:00.000000000 +0200
> --- gcc/fortran/trans.c	2010-09-17 13:34:29.000000000 +0200
> *************** gfc_generate_module_code (gfc_namespace
> *** 1413,1425 ****
>         if (!n->proc_name)
>           continue;
>   
>         gfc_create_function_decl (n, false);
> !       gcc_assert (DECL_CONTEXT (n->proc_name->backend_decl) == NULL_TREE);
>         DECL_CONTEXT (n->proc_name->backend_decl) = ns->proc_name->backend_decl;
>         gfc_module_add_decl (entry, n->proc_name->backend_decl);
>         for (el = ns->entries; el; el = el->next)
>   	{
> ! 	  gcc_assert (DECL_CONTEXT (el->sym->backend_decl) == NULL_TREE);
>   	  DECL_CONTEXT (el->sym->backend_decl) = ns->proc_name->backend_decl;
>   	  gfc_module_add_decl (entry, el->sym->backend_decl);
>   	}
> --- 1413,1426 ----
>         if (!n->proc_name)
>           continue;
>   
> +       /* ???  Why's the binding level not that of the module here?  */
>         gfc_create_function_decl (n, false);
> !       /* gcc_assert (DECL_CONTEXT (n->proc_name->backend_decl) == NULL_TREE); */
>         DECL_CONTEXT (n->proc_name->backend_decl) = ns->proc_name->backend_decl;
>         gfc_module_add_decl (entry, n->proc_name->backend_decl);
>         for (el = ns->entries; el; el = el->next)
>   	{
> ! 	  /* gcc_assert (DECL_CONTEXT (el->sym->backend_decl) == NULL_TREE); */
>   	  DECL_CONTEXT (el->sym->backend_decl) = ns->proc_name->backend_decl;
>   	  gfc_module_add_decl (entry, el->sym->backend_decl);
>   	}
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH][fortran] Use a TRANSLATION_UNIT_DECL for DECL_CONTEXT
@ 2010-09-17 12:51 Richard Guenther
  2010-09-22 20:06 ` Richard Guenther
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Guenther @ 2010-09-17 12:51 UTC (permalink / raw)
  To: gcc-patches; +Cc: fortran


This transitions the Fortran frontend to use a TRANSLATION_UNIT_DECL
for DECL_CONTEXT of global entities.  This will allow LTO to produce
proper language specific debug information.

The patch leaves some DECL_CONTEXT tests for NULL untouched, as the
FE seems to have both test-for-globalness and test-for-not-yet-initialized
tests - I'd appreciate if somebody could go over those in the Frontend
and use DECL_FILE_SCOPE_P for those that really mean it.

I'm also somewhat confused about binding-level use - for modules
its state seems to be strange.  Maybe somebody can shed a light
on this.  (Possibly module decls want to use a different T_U_D
in the future as well)

As of the ???s in the patch:

  /* Create the variable.  */
  pushdecl (decl);
  /* Why is current_binding_level == global_binding_level here?  
  gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);  */
  gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
  DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;

this (and the other two similar cases) seems to assert that
pushdecl doesn't set DECL_CONTEXT (well, before my patch
it set it to current_function_decl which is NULL).  So, does
this check that the decl is at file-scope or does it want to
test that current_function_decl is NULL?

Bootstrap and regtest re-running after some minor changes on
x86_64-unknown-linux-gnu.  Any idea about the above?

Thanks,
Richard.

2010-09-17  Richard Guenther  <rguenther@suse.de>

	* f95-lang.c (current_translation_unit): New global variable.
	(gfc_create_decls): Build a translation-unit decl.
	(pushdecl): In the global binding-level use the
	translation-unit decl as DECL_CONTEXT.
	* trans-decl.c (gfc_get_symbol_decl): Use DECL_FILE_SCOPE_P.
	(build_function_decl): Likewise.  Delay setting the assembler
	name, leave setting of DECL_CONTEXT to pushdecl.
	(trans_function_start): Use DECL_FILE_SCOPE_P.
	(gfc_create_module_variable): Likewise.  Add some ???s
	* trans.c (gfc_generate_module_code): Likewise.

Index: gcc/fortran/f95-lang.c
===================================================================
*** gcc/fortran/f95-lang.c.orig	2010-09-17 12:40:00.000000000 +0200
--- gcc/fortran/f95-lang.c	2010-09-17 13:42:18.000000000 +0200
*************** tree *ridpointers = NULL;
*** 172,177 ****
--- 172,180 ----
  /* True means we've initialized exception handling.  */
  bool gfc_eh_initialized_p;
  
+ /* The current translation unit.  */
+ static GTY(()) tree current_translation_unit;
+ 
  
  /* Prepare expr to be an argument of a TRUTH_NOT_EXPR,
     or validate its data type for an `if' or `while' statement or ?..: exp.
*************** gfc_create_decls (void)
*** 229,234 ****
--- 232,240 ----
    gfc_build_builtin_function_decls ();
  
    gfc_init_constants ();
+ 
+   /* Build our translation-unit decl.  */
+   current_translation_unit = build_translation_unit_decl (NULL_TREE);
  }
  
  
*************** tree
*** 491,498 ****
  pushdecl (tree decl)
  {
    /* External objects aren't nested, other objects may be.  */
!   if ((DECL_EXTERNAL (decl)) || (decl == current_function_decl))
!     DECL_CONTEXT (decl) = 0;
    else
      DECL_CONTEXT (decl) = current_function_decl;
  
--- 497,506 ----
  pushdecl (tree decl)
  {
    /* External objects aren't nested, other objects may be.  */
!   if (DECL_EXTERNAL (decl))
!     DECL_CONTEXT (decl) = NULL_TREE;
!   else if (global_bindings_p ())
!     DECL_CONTEXT (decl) = current_translation_unit;
    else
      DECL_CONTEXT (decl) = current_function_decl;
  
Index: gcc/fortran/trans-decl.c
===================================================================
*** gcc/fortran/trans-decl.c.orig	2010-09-17 12:40:00.000000000 +0200
--- gcc/fortran/trans-decl.c	2010-09-17 13:40:49.000000000 +0200
*************** gfc_get_symbol_decl (gfc_symbol * sym)
*** 1089,1095 ****
  	  else
  	    length = sym->ts.u.cl->backend_decl;
  	  if (TREE_CODE (length) == VAR_DECL
! 	      && DECL_CONTEXT (length) == NULL_TREE)
  	    {
  	      /* Add the string length to the same context as the symbol.  */
  	      if (DECL_CONTEXT (sym->backend_decl) == current_function_decl)
--- 1089,1095 ----
  	  else
  	    length = sym->ts.u.cl->backend_decl;
  	  if (TREE_CODE (length) == VAR_DECL
! 	      && DECL_FILE_SCOPE_P (length))
  	    {
  	      /* Add the string length to the same context as the symbol.  */
  	      if (DECL_CONTEXT (sym->backend_decl) == current_function_decl)
*************** build_function_decl (gfc_symbol * sym, b
*** 1631,1639 ****
  
    /* Allow only one nesting level.  Allow public declarations.  */
    gcc_assert (current_function_decl == NULL_TREE
! 	      || DECL_CONTEXT (current_function_decl) == NULL_TREE
! 	      || TREE_CODE (DECL_CONTEXT (current_function_decl))
! 		 == NAMESPACE_DECL);
  
    type = gfc_get_function_type (sym);
    fndecl = build_decl (input_location,
--- 1631,1639 ----
  
    /* Allow only one nesting level.  Allow public declarations.  */
    gcc_assert (current_function_decl == NULL_TREE
! 	      || DECL_FILE_SCOPE_P (current_function_decl)
! 	      || (TREE_CODE (DECL_CONTEXT (current_function_decl))
! 		  == NAMESPACE_DECL));
  
    type = gfc_get_function_type (sym);
    fndecl = build_decl (input_location,
*************** build_function_decl (gfc_symbol * sym, b
*** 1644,1653 ****
    attributes = add_attributes_to_decl (attr, NULL_TREE);
    decl_attributes (&fndecl, attributes, 0);
  
-   /* Perform name mangling if this is a top level or module procedure.  */
-   if (current_function_decl == NULL_TREE)
-     gfc_set_decl_assembler_name (fndecl, gfc_sym_mangled_function_id (sym));
- 
    /* Figure out the return type of the declared function, and build a
       RESULT_DECL for it.  If this is a subroutine with alternate
       returns, build a RESULT_DECL for it.  */
--- 1644,1649 ----
*************** build_function_decl (gfc_symbol * sym, b
*** 1695,1706 ****
       layout_decl (result_decl, 0);  */
  
    /* Set up all attributes for the function.  */
-   DECL_CONTEXT (fndecl) = current_function_decl;
    DECL_EXTERNAL (fndecl) = 0;
  
    /* This specifies if a function is globally visible, i.e. it is
       the opposite of declaring static in C.  */
!   if (DECL_CONTEXT (fndecl) == NULL_TREE
        && !sym->attr.entry_master && !sym->attr.is_main_program)
      TREE_PUBLIC (fndecl) = 1;
  
--- 1691,1701 ----
       layout_decl (result_decl, 0);  */
  
    /* Set up all attributes for the function.  */
    DECL_EXTERNAL (fndecl) = 0;
  
    /* This specifies if a function is globally visible, i.e. it is
       the opposite of declaring static in C.  */
!   if (!current_function_decl
        && !sym->attr.entry_master && !sym->attr.is_main_program)
      TREE_PUBLIC (fndecl) = 1;
  
*************** build_function_decl (gfc_symbol * sym, b
*** 1729,1734 ****
--- 1724,1733 ----
    else
      pushdecl (fndecl);
  
+   /* Perform name mangling if this is a top level or module procedure.  */
+   if (current_function_decl == NULL_TREE)
+     gfc_set_decl_assembler_name (fndecl, gfc_sym_mangled_function_id (sym));
+ 
    sym->backend_decl = fndecl;
  }
  
*************** trans_function_start (gfc_symbol * sym)
*** 1976,1982 ****
    /* Let the world know what we're about to do.  */
    announce_function (fndecl);
  
!   if (DECL_CONTEXT (fndecl) == NULL_TREE)
      {
        /* Create RTL for function declaration.  */
        rest_of_decl_compilation (fndecl, 1, 0);
--- 1975,1981 ----
    /* Let the world know what we're about to do.  */
    announce_function (fndecl);
  
!   if (DECL_FILE_SCOPE_P (fndecl))
      {
        /* Create RTL for function declaration.  */
        rest_of_decl_compilation (fndecl, 1, 0);
*************** gfc_create_module_variable (gfc_symbol *
*** 3583,3589 ****
    if ((sym->attr.in_common || sym->attr.in_equivalence) && sym->backend_decl)
      {
        decl = sym->backend_decl;
!       gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);
        gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
        DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
        gfc_module_add_decl (cur_module, decl);
--- 3582,3588 ----
    if ((sym->attr.in_common || sym->attr.in_equivalence) && sym->backend_decl)
      {
        decl = sym->backend_decl;
!       gcc_assert (DECL_FILE_SCOPE_P (decl));
        gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
        DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
        gfc_module_add_decl (cur_module, decl);
*************** gfc_create_module_variable (gfc_symbol *
*** 3610,3616 ****
  
    /* Create the variable.  */
    pushdecl (decl);
!   gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);
    gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
    DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
    rest_of_decl_compilation (decl, 1, 0);
--- 3609,3616 ----
  
    /* Create the variable.  */
    pushdecl (decl);
!   /* Why is current_binding_level == global_binding_level here?
!   gcc_assert (DECL_CONTEXT (decl) == NULL_TREE);  */
    gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
    DECL_CONTEXT (decl) = sym->ns->proc_name->backend_decl;
    rest_of_decl_compilation (decl, 1, 0);
Index: gcc/fortran/trans.c
===================================================================
*** gcc/fortran/trans.c.orig	2010-09-17 12:40:00.000000000 +0200
--- gcc/fortran/trans.c	2010-09-17 13:34:29.000000000 +0200
*************** gfc_generate_module_code (gfc_namespace
*** 1413,1425 ****
        if (!n->proc_name)
          continue;
  
        gfc_create_function_decl (n, false);
!       gcc_assert (DECL_CONTEXT (n->proc_name->backend_decl) == NULL_TREE);
        DECL_CONTEXT (n->proc_name->backend_decl) = ns->proc_name->backend_decl;
        gfc_module_add_decl (entry, n->proc_name->backend_decl);
        for (el = ns->entries; el; el = el->next)
  	{
! 	  gcc_assert (DECL_CONTEXT (el->sym->backend_decl) == NULL_TREE);
  	  DECL_CONTEXT (el->sym->backend_decl) = ns->proc_name->backend_decl;
  	  gfc_module_add_decl (entry, el->sym->backend_decl);
  	}
--- 1413,1426 ----
        if (!n->proc_name)
          continue;
  
+       /* ???  Why's the binding level not that of the module here?  */
        gfc_create_function_decl (n, false);
!       /* gcc_assert (DECL_CONTEXT (n->proc_name->backend_decl) == NULL_TREE); */
        DECL_CONTEXT (n->proc_name->backend_decl) = ns->proc_name->backend_decl;
        gfc_module_add_decl (entry, n->proc_name->backend_decl);
        for (el = ns->entries; el; el = el->next)
  	{
! 	  /* gcc_assert (DECL_CONTEXT (el->sym->backend_decl) == NULL_TREE); */
  	  DECL_CONTEXT (el->sym->backend_decl) = ns->proc_name->backend_decl;
  	  gfc_module_add_decl (entry, el->sym->backend_decl);
  	}

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2010-10-04  9:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-02 13:45 [PATCH][fortran] Use a TRANSLATION_UNIT_DECL for DECL_CONTEXT Dominique Dhumieres
  -- strict thread matches above, loose matches on Subject: below --
2010-09-17 12:51 Richard Guenther
2010-09-22 20:06 ` Richard Guenther
2010-09-23  0:28   ` Steven Bosscher
2010-09-24 18:25     ` Richard Guenther
2010-09-29 17:44       ` Richard Guenther
2010-10-02 16:45         ` Mikael Morin
2010-10-04  9:20           ` Richard Guenther

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).