Optimization of add_dt_to_dt_list() in resolve.c

Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

[-- Attachment #1: Type: text/plain, Size: 1604 bytes --]

I've been attempting to track down some of the causes of very long compile 
times for files which use modules that contain a large number of symbols. The 
worst case offender in my code takes around 12 minutes to compile. 

After profiling f951 for this source file it turns out that the majority of the 
time is spent in add_dt_to_dt_list() in resolve.c. In cases where the number 
of symbols imported becomes very large (~10,000 in some cases in this code), 
the O(N) search in this function becomes inefficient.

A simple optimization for this problem seems to be to just have the gfc_symbol 
struct include a pointer back to the corresponding entry in the 
gfc_derived_types list. It's then fast to check if a symbol is already on that 
list by checking if this pointer is non-null. (It could just as easily be an 
int in gfc_symbol which indicates if the symbol is already added to the list - 
I don't know if having a pointer to the list entry is useful for any other 
reason.)

With this change in place compile times are much faster - my worst case 
offender now takes just under 1 minute to compile.

My patch is attached. I freely admit that I have only a very shallow 
understanding of the inner workings of the compiler, so I would not be 
surprised if there are good reasons not to do this. I did "make check-fortran" 
and did not see any failures. If any one wants to try this out and/or provide 
any feedback I'd be happy to hear it.

Thanks,
Andrew

-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1202 bytes --]

Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 260682)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -1611,6 +1611,9 @@ typedef struct gfc_symbol
 
   /* Link to corresponding association-list if this is an associate name.  */
   struct gfc_association_list *assoc;
+
+  /* Link to derived type list */
+  struct gfc_dt_list *dt_list;
 }
 gfc_symbol;
 
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 260682)
+++ gcc/fortran/resolve.c	(working copy)
@@ -13437,14 +13437,13 @@ add_dt_to_dt_list (gfc_symbol *derived)
 {
   gfc_dt_list *dt_list;
 
-  for (dt_list = gfc_derived_types; dt_list; dt_list = dt_list->next)
-    if (derived == dt_list->derived)
-      return;
-
-  dt_list = gfc_get_dt_list ();
-  dt_list->next = gfc_derived_types;
-  dt_list->derived = derived;
-  gfc_derived_types = dt_list;
+  if (derived->dt_list == NULL) {
+    dt_list = gfc_get_dt_list ();
+    dt_list->next = gfc_derived_types;
+    dt_list->derived = derived;
+    derived->dt_list = dt_list;  
+    gfc_derived_types = dt_list;
+  }
 }
 
 

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Richard Biener]

On Fri, May 25, 2018 at 12:53 AM Andrew Benson <abenson@carnegiescience.edu>
wrote:

> I've been attempting to track down some of the causes of very long compile
> times for files which use modules that contain a large number of symbols.
The
> worst case offender in my code takes around 12 minutes to compile.

> After profiling f951 for this source file it turns out that the majority
of the
> time is spent in add_dt_to_dt_list() in resolve.c. In cases where the
number
> of symbols imported becomes very large (~10,000 in some cases in this
code),
> the O(N) search in this function becomes inefficient.

> A simple optimization for this problem seems to be to just have the
gfc_symbol
> struct include a pointer back to the corresponding entry in the
> gfc_derived_types list. It's then fast to check if a symbol is already on
that
> list by checking if this pointer is non-null. (It could just as easily be
an
> int in gfc_symbol which indicates if the symbol is already added to the
list -
> I don't know if having a pointer to the list entry is useful for any other
> reason.)

> With this change in place compile times are much faster - my worst case
> offender now takes just under 1 minute to compile.

> My patch is attached. I freely admit that I have only a very shallow
> understanding of the inner workings of the compiler, so I would not be
> surprised if there are good reasons not to do this. I did "make
check-fortran"
> and did not see any failures. If any one wants to try this out and/or
provide
> any feedback I'd be happy to hear it.

It looks like it would be cheaper to simply embed gtc_dt_list *next in
gfc_symbol?
(in case a gfc_symbol can be only on one gfc_dt_list which your patch
assumes as well)

Richard.

> Thanks,
> Andrew

> --

> * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

> * Galacticus: https://bitbucket.org/abensonca/galacticus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Richard Biener]

On Fri, May 25, 2018 at 9:06 AM Richard Biener <richard.guenther@gmail.com>
wrote:

> On Fri, May 25, 2018 at 12:53 AM Andrew Benson <
abenson@carnegiescience.edu>
> wrote:

> > I've been attempting to track down some of the causes of very long
compile
> > times for files which use modules that contain a large number of
symbols.
> The
> > worst case offender in my code takes around 12 minutes to compile.

> > After profiling f951 for this source file it turns out that the majority
> of the
> > time is spent in add_dt_to_dt_list() in resolve.c. In cases where the
> number
> > of symbols imported becomes very large (~10,000 in some cases in this
> code),
> > the O(N) search in this function becomes inefficient.

> > A simple optimization for this problem seems to be to just have the
> gfc_symbol
> > struct include a pointer back to the corresponding entry in the
> > gfc_derived_types list. It's then fast to check if a symbol is already
on
> that
> > list by checking if this pointer is non-null. (It could just as easily
be
> an
> > int in gfc_symbol which indicates if the symbol is already added to the
> list -
> > I don't know if having a pointer to the list entry is useful for any
other
> > reason.)

> > With this change in place compile times are much faster - my worst case
> > offender now takes just under 1 minute to compile.

> > My patch is attached. I freely admit that I have only a very shallow
> > understanding of the inner workings of the compiler, so I would not be
> > surprised if there are good reasons not to do this. I did "make
> check-fortran"
> > and did not see any failures. If any one wants to try this out and/or
> provide
> > any feedback I'd be happy to hear it.

> It looks like it would be cheaper to simply embed gtc_dt_list *next in
> gfc_symbol?
> (in case a gfc_symbol can be only on one gfc_dt_list which your patch
> assumes as well)

Note you'll need to make the list cyclic to make the member test work
as sym->next_dt != NULL.

Also note that gfc_get_derived_type has another linear list walk so it
looks like derived type processing is still quadratic in the number of
derived types after the change.  That suggests that eventually
making the derived_types list a hash-map of gfc_symbol to
the common derived type (symbol) would be a better fix, thus doing
the merging at dt-"list" insertion time.

Of course that's likely a bigger change that needs more thorough
understanding
of how derived types work.  A fix along your idea looks simple enough
and has obvious benefits as you show so it might be even appropriate for
release branches.

Richard.

> Richard.

> > Thanks,
> > Andrew

> > --

> > * Andrew Benson:
http://users.obs.carnegiescience.edu/abenson/contact.html

> > * Galacticus: https://bitbucket.org/abensonca/galacticus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

[-- Attachment #1: Type: text/plain, Size: 2843 bytes --]

Richard:

Thanks for the suggestion. I changed my patch (new version attached) so that 
there's a *dt_next in gfc_symbol, which is now used to construct the 
(circular) linked list. There were a couple places where I had to change the 
order in which clean up of symbols and derived type lists were done - it's now 
necessary to free the derived type list before its associated symbols (since 
the symbols carry the links for the derived type list).

This passes "make check-fortran" and seems to retain the speed-up from my 
original patch.

Thanks,
Andrew

On Friday, May 25, 2018 9:06:22 AM PDT Richard Biener wrote:
> On Fri, May 25, 2018 at 12:53 AM Andrew Benson <abenson@carnegiescience.edu>
> wrote:
> > I've been attempting to track down some of the causes of very long compile
> > times for files which use modules that contain a large number of symbols.
> 
> The
> 
> > worst case offender in my code takes around 12 minutes to compile.
> > 
> > After profiling f951 for this source file it turns out that the majority
> 
> of the
> 
> > time is spent in add_dt_to_dt_list() in resolve.c. In cases where the
> 
> number
> 
> > of symbols imported becomes very large (~10,000 in some cases in this
> 
> code),
> 
> > the O(N) search in this function becomes inefficient.
> > 
> > A simple optimization for this problem seems to be to just have the
> 
> gfc_symbol
> 
> > struct include a pointer back to the corresponding entry in the
> > gfc_derived_types list. It's then fast to check if a symbol is already on
> 
> that
> 
> > list by checking if this pointer is non-null. (It could just as easily be
> 
> an
> 
> > int in gfc_symbol which indicates if the symbol is already added to the
> 
> list -
> 
> > I don't know if having a pointer to the list entry is useful for any other
> > reason.)
> > 
> > With this change in place compile times are much faster - my worst case
> > offender now takes just under 1 minute to compile.
> > 
> > My patch is attached. I freely admit that I have only a very shallow
> > understanding of the inner workings of the compiler, so I would not be
> > surprised if there are good reasons not to do this. I did "make
> 
> check-fortran"
> 
> > and did not see any failures. If any one wants to try this out and/or
> 
> provide
> 
> > any feedback I'd be happy to hear it.
> 
> It looks like it would be cheaper to simply embed gtc_dt_list *next in
> gfc_symbol?
> (in case a gfc_symbol can be only on one gfc_dt_list which your patch
> assumes as well)
> 
> Richard.
> 
> > Thanks,
> > Andrew
> > 
> > --
> > 
> > * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html
> > 
> > * Galacticus: https://bitbucket.org/abensonca/galacticus


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 7020 bytes --]

Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 260682)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -1611,6 +1611,9 @@ typedef struct gfc_symbol
 
   /* Link to corresponding association-list if this is an associate name.  */
   struct gfc_association_list *assoc;
+
+  /* Link to next entry in derived type list */
+  struct gfc_dt_list *dt_next;
 }
 gfc_symbol;
 
@@ -1716,7 +1719,6 @@ gfc_symtree;
 typedef struct gfc_dt_list
 {
   struct gfc_symbol *derived;
-  struct gfc_dt_list *next;
 }
 gfc_dt_list;
 
Index: gcc/fortran/parse.c
===================================================================
--- gcc/fortran/parse.c	(revision 260682)
+++ gcc/fortran/parse.c	(working copy)
@@ -6123,6 +6123,9 @@ translate_all_program_units (gfc_namespace *gfc_gl
     }
 
   /* Clean up all the namespaces after translation.  */
+
+  clean_up_modules (gfc_gsym_root);
+
   gfc_current_ns = gfc_global_ns_list;
   for (;gfc_current_ns;)
     {
@@ -6141,7 +6144,6 @@ translate_all_program_units (gfc_namespace *gfc_gl
       gfc_current_ns = ns;
     }
 
-  clean_up_modules (gfc_gsym_root);
 }
 
 
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 260682)
+++ gcc/fortran/resolve.c	(working copy)
@@ -13437,14 +13437,18 @@ add_dt_to_dt_list (gfc_symbol *derived)
 {
   gfc_dt_list *dt_list;
 
-  for (dt_list = gfc_derived_types; dt_list; dt_list = dt_list->next)
-    if (derived == dt_list->derived)
-      return;
-
-  dt_list = gfc_get_dt_list ();
-  dt_list->next = gfc_derived_types;
-  dt_list->derived = derived;
-  gfc_derived_types = dt_list;
+  if (!derived->dt_next) {
+    dt_list = gfc_get_dt_list ();
+    dt_list->derived = derived;
+    if (gfc_derived_types) {
+      derived->dt_next = gfc_derived_types->derived->dt_next;
+      gfc_derived_types->derived->dt_next = dt_list;
+    } else {
+      derived->dt_next = dt_list;
+    }
+    gfc_derived_types = dt_list;
+  }
+  
 }
 
 
Index: gcc/fortran/symbol.c
===================================================================
--- gcc/fortran/symbol.c	(revision 260682)
+++ gcc/fortran/symbol.c	(working copy)
@@ -3885,11 +3885,15 @@ gfc_free_dt_list (void)
 {
   gfc_dt_list *dt, *n;
 
-  for (dt = gfc_derived_types; dt; dt = n)
-    {
-      n = dt->next;
-      free (dt);
+  if (gfc_derived_types) {
+    dt = gfc_derived_types;
+    while (dt->derived->dt_next != dt) {
+      n = dt->derived->dt_next;
+      dt->derived->dt_next = n->derived->dt_next;
+      free(n);
     }
+    free(dt);
+  }
 
   gfc_derived_types = NULL;
 }
@@ -4072,6 +4076,7 @@ gfc_symbol_init_2 (void)
 void
 gfc_symbol_done_2 (void)
 {
+  gfc_free_dt_list ();
   if (gfc_current_ns != NULL)
     {
       /* free everything from the root.  */
@@ -4080,7 +4085,6 @@ gfc_symbol_done_2 (void)
       gfc_free_namespace (gfc_current_ns);
       gfc_current_ns = NULL;
     }
-  gfc_free_dt_list ();
 
   enforce_single_undo_checkpoint ();
   free_undo_change_set_data (*latest_undo_chgset);
@@ -4350,15 +4354,17 @@ get_iso_c_binding_dt (int sym_id)
   /* Loop through the derived types in the name list, searching for
      the desired symbol from iso_c_binding.  Search the parent namespaces
      if necessary and requested to (parent_flag).  */
-  while (dt_list != NULL)
-    {
-      if (dt_list->derived->from_intmod != INTMOD_NONE
-	  && dt_list->derived->intmod_sym_id == sym_id)
-        return dt_list->derived;
+  if (dt_list) {
+    while (dt_list->derived->dt_next != gfc_derived_types)
+      {
+	if (dt_list->derived->from_intmod != INTMOD_NONE
+	    && dt_list->derived->intmod_sym_id == sym_id)
+	  return dt_list->derived;
+	
+	dt_list = dt_list->derived->dt_next;
+      }
+  }
 
-      dt_list = dt_list->next;
-    }
-
   return NULL;
 }
 
@@ -4765,8 +4771,13 @@ generate_isocbinding_symbol (const char *mod_name,
 	  gfc_dt_list *dt_list;
 	  dt_list = gfc_get_dt_list ();
 	  dt_list->derived = tmp_sym;
-	  dt_list->next = gfc_derived_types;
-  	  gfc_derived_types = dt_list;
+	  if (gfc_derived_types) {
+	    tmp_sym->dt_next = gfc_derived_types->derived->dt_next;
+	    gfc_derived_types->derived->dt_next = dt_list;
+	  } else {
+	    tmp_sym->dt_next = dt_list;
+	  }
+	  gfc_derived_types = dt_list;
         }
 
       return tmp_symtree;
@@ -4874,7 +4885,7 @@ generate_isocbinding_symbol (const char *mod_name,
       case ISOCBINDING_FUNPTR:
 	{
 	  gfc_symbol *dt_sym;
-	  gfc_dt_list **dt_list_ptr = NULL;
+	  gfc_dt_list *dt_list = NULL;
 	  gfc_component *tmp_comp = NULL;
 
 	  /* Generate real derived type.  */
@@ -4936,18 +4947,16 @@ generate_isocbinding_symbol (const char *mod_name,
 	  dt_sym->ts.u.derived = dt_sym;
 
 	  /* Add the symbol created for the derived type to the current ns.  */
-	  dt_list_ptr = &(gfc_derived_types);
-	  while (*dt_list_ptr != NULL && (*dt_list_ptr)->next != NULL)
-	    dt_list_ptr = &((*dt_list_ptr)->next);
-
-	  /* There is already at least one derived type in the list, so append
-	     the one we're currently building for c_ptr or c_funptr.  */
-	  if (*dt_list_ptr != NULL)
-	    dt_list_ptr = &((*dt_list_ptr)->next);
-	  (*dt_list_ptr) = gfc_get_dt_list ();
-	  (*dt_list_ptr)->derived = dt_sym;
-	  (*dt_list_ptr)->next = NULL;
-
+	  dt_list = gfc_get_dt_list ();
+	  if (gfc_derived_types) {
+	    dt_sym->dt_next = gfc_derived_types->derived->dt_next;
+	    gfc_derived_types->derived->dt_next = dt_list;
+	  } else {
+	    dt_sym->dt_next = dt_list;
+	  }
+	  dt_list->derived = dt_sym;
+	  gfc_derived_types = dt_list;
+      	  
 	  gfc_add_component (dt_sym, "c_address", &tmp_comp);
 	  if (tmp_comp == NULL)
 	    gcc_unreachable ();
Index: gcc/fortran/trans-types.c
===================================================================
--- gcc/fortran/trans-types.c	(revision 260682)
+++ gcc/fortran/trans-types.c	(working copy)
@@ -2599,13 +2599,15 @@ gfc_get_derived_type (gfc_symbol * derived, int co
 	   ns->translated && !got_canonical;
 	   ns = ns->sibling)
 	{
-	  dt = ns->derived_types;
-	  for (; dt && !canonical; dt = dt->next)
-	    {
-	      gfc_copy_dt_decls_ifequal (dt->derived, derived, true);
-	      if (derived->backend_decl)
-		got_canonical = true;
-	    }
+	  if (ns->derived_types) {
+	    dt = ns->derived_types;
+	    for (; dt->derived->dt_next != ns->derived_types && !canonical; dt = dt->derived->dt_next)
+	      {
+		gfc_copy_dt_decls_ifequal (dt->derived, derived, true);
+		if (derived->backend_decl)
+		  got_canonical = true;
+	      }
+	  }
 	}
     }
 
@@ -2867,9 +2869,11 @@ copy_derived_types:
 	}
     }
 
-  for (dt = gfc_derived_types; dt; dt = dt->next)
-    gfc_copy_dt_decls_ifequal (derived, dt->derived, false);
-
+  if (gfc_derived_types) {
+    for (dt = gfc_derived_types; dt->derived->dt_next != gfc_derived_types; dt = dt->derived->dt_next)
+      gfc_copy_dt_decls_ifequal (derived, dt->derived, false);
+  }
+  
   return derived->backend_decl;
 }
 

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Richard Biener]

On Fri, May 25, 2018 at 11:54 PM Andrew Benson <abenson@carnegiescience.edu>
wrote:

> Richard:

> Thanks for the suggestion. I changed my patch (new version attached) so
that
> there's a *dt_next in gfc_symbol, which is now used to construct the
> (circular) linked list. There were a couple places where I had to change
the
> order in which clean up of symbols and derived type lists were done -
it's now
> necessary to free the derived type list before its associated symbols
(since
> the symbols carry the links for the derived type list).

Hmm, it still has the indirection via gfc_dt_list.  I think it should be
possible
to do away with gfc_dt_list objects alltogether by no doing
sym->dt_next->derived
but sym->derived thus

@@ -1611,6 +1611,9 @@ typedef struct gfc_symbol

    /* Link to corresponding association-list if this is an associate name.
  */
    struct gfc_association_list *assoc;
+
+  /* Link to next entry in derived type list */
+  gfc_symbol *dt_next;
  }
  gfc_symbol;

that means for example gfc_free_dt_list can be simply removed.  The
gfc_derived_types global would then point to the first derived type
directly.

Richard.

> This passes "make check-fortran" and seems to retain the speed-up from my
> original patch.

> Thanks,
> Andrew

> On Friday, May 25, 2018 9:06:22 AM PDT Richard Biener wrote:
> > On Fri, May 25, 2018 at 12:53 AM Andrew Benson <
abenson@carnegiescience.edu>
> > wrote:
> > > I've been attempting to track down some of the causes of very long
compile
> > > times for files which use modules that contain a large number of
symbols.
> >
> > The
> >
> > > worst case offender in my code takes around 12 minutes to compile.
> > >
> > > After profiling f951 for this source file it turns out that the
majority
> >
> > of the
> >
> > > time is spent in add_dt_to_dt_list() in resolve.c. In cases where the
> >
> > number
> >
> > > of symbols imported becomes very large (~10,000 in some cases in this
> >
> > code),
> >
> > > the O(N) search in this function becomes inefficient.
> > >
> > > A simple optimization for this problem seems to be to just have the
> >
> > gfc_symbol
> >
> > > struct include a pointer back to the corresponding entry in the
> > > gfc_derived_types list. It's then fast to check if a symbol is
already on
> >
> > that
> >
> > > list by checking if this pointer is non-null. (It could just as
easily be
> >
> > an
> >
> > > int in gfc_symbol which indicates if the symbol is already added to
the
> >
> > list -
> >
> > > I don't know if having a pointer to the list entry is useful for any
other
> > > reason.)
> > >
> > > With this change in place compile times are much faster - my worst
case
> > > offender now takes just under 1 minute to compile.
> > >
> > > My patch is attached. I freely admit that I have only a very shallow
> > > understanding of the inner workings of the compiler, so I would not be
> > > surprised if there are good reasons not to do this. I did "make
> >
> > check-fortran"
> >
> > > and did not see any failures. If any one wants to try this out and/or
> >
> > provide
> >
> > > any feedback I'd be happy to hear it.
> >
> > It looks like it would be cheaper to simply embed gtc_dt_list *next in
> > gfc_symbol?
> > (in case a gfc_symbol can be only on one gfc_dt_list which your patch
> > assumes as well)
> >
> > Richard.
> >
> > > Thanks,
> > > Andrew
> > >
> > > --
> > >
> > > * Andrew Benson:
http://users.obs.carnegiescience.edu/abenson/contact.html
> > >
> > > * Galacticus: https://bitbucket.org/abensonca/galacticus


> --

> * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

> * Galacticus: https://bitbucket.org/abensonca/galacticus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

[-- Attachment #1: Type: text/plain, Size: 4317 bytes --]

Yes - definitely possible to remove gfc_dt_list entirely - new patch is 
attached.

Thanks,
Andrew

On Monday, May 28, 2018 11:54:41 AM PDT Richard Biener wrote:
> On Fri, May 25, 2018 at 11:54 PM Andrew Benson <abenson@carnegiescience.edu>
> wrote:
> > Richard:
> > 
> > Thanks for the suggestion. I changed my patch (new version attached) so
> 
> that
> 
> > there's a *dt_next in gfc_symbol, which is now used to construct the
> > (circular) linked list. There were a couple places where I had to change
> 
> the
> 
> > order in which clean up of symbols and derived type lists were done -
> 
> it's now
> 
> > necessary to free the derived type list before its associated symbols
> 
> (since
> 
> > the symbols carry the links for the derived type list).
> 
> Hmm, it still has the indirection via gfc_dt_list.  I think it should be
> possible
> to do away with gfc_dt_list objects alltogether by no doing
> sym->dt_next->derived
> but sym->derived thus
> 
> @@ -1611,6 +1611,9 @@ typedef struct gfc_symbol
> 
>     /* Link to corresponding association-list if this is an associate name.
>   */
>     struct gfc_association_list *assoc;
> +
> +  /* Link to next entry in derived type list */
> +  gfc_symbol *dt_next;
>   }
>   gfc_symbol;
> 
> that means for example gfc_free_dt_list can be simply removed.  The
> gfc_derived_types global would then point to the first derived type
> directly.
> 
> Richard.
> 
> > This passes "make check-fortran" and seems to retain the speed-up from my
> > original patch.
> > 
> > Thanks,
> > Andrew
> > 
> > On Friday, May 25, 2018 9:06:22 AM PDT Richard Biener wrote:
> > > On Fri, May 25, 2018 at 12:53 AM Andrew Benson <
> 
> abenson@carnegiescience.edu>
> 
> > > wrote:
> > > > I've been attempting to track down some of the causes of very long
> 
> compile
> 
> > > > times for files which use modules that contain a large number of
> 
> symbols.
> 
> > > The
> > > 
> > > > worst case offender in my code takes around 12 minutes to compile.
> > > > 
> > > > After profiling f951 for this source file it turns out that the
> 
> majority
> 
> > > of the
> > > 
> > > > time is spent in add_dt_to_dt_list() in resolve.c. In cases where the
> > > 
> > > number
> > > 
> > > > of symbols imported becomes very large (~10,000 in some cases in this
> > > 
> > > code),
> > > 
> > > > the O(N) search in this function becomes inefficient.
> > > > 
> > > > A simple optimization for this problem seems to be to just have the
> > > 
> > > gfc_symbol
> > > 
> > > > struct include a pointer back to the corresponding entry in the
> > > > gfc_derived_types list. It's then fast to check if a symbol is
> 
> already on
> 
> > > that
> > > 
> > > > list by checking if this pointer is non-null. (It could just as
> 
> easily be
> 
> > > an
> > > 
> > > > int in gfc_symbol which indicates if the symbol is already added to
> 
> the
> 
> > > list -
> > > 
> > > > I don't know if having a pointer to the list entry is useful for any
> 
> other
> 
> > > > reason.)
> > > > 
> > > > With this change in place compile times are much faster - my worst
> 
> case
> 
> > > > offender now takes just under 1 minute to compile.
> > > > 
> > > > My patch is attached. I freely admit that I have only a very shallow
> > > > understanding of the inner workings of the compiler, so I would not be
> > > > surprised if there are good reasons not to do this. I did "make
> > > 
> > > check-fortran"
> > > 
> > > > and did not see any failures. If any one wants to try this out and/or
> > > 
> > > provide
> > > 
> > > > any feedback I'd be happy to hear it.
> > > 
> > > It looks like it would be cheaper to simply embed gtc_dt_list *next in
> > > gfc_symbol?
> > > (in case a gfc_symbol can be only on one gfc_dt_list which your patch
> > > assumes as well)
> > > 
> > > Richard.
> > > 
> > > > Thanks,
> > > > Andrew
> > > > 
> > > > --
> 
> > > > * Andrew Benson:
> http://users.obs.carnegiescience.edu/abenson/contact.html
> 
> > > > * Galacticus: https://bitbucket.org/abensonca/galacticus
> > 
> > --
> > 
> > * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html
> > 
> > * Galacticus: https://bitbucket.org/abensonca/galacticus


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 8423 bytes --]

Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 260682)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -1611,6 +1611,9 @@ typedef struct gfc_symbol
 
   /* Link to corresponding association-list if this is an associate name.  */
   struct gfc_association_list *assoc;
+
+  /* Link to next entry in derived type list */
+  struct gfc_symbol *dt_next;
 }
 gfc_symbol;
 
@@ -1712,19 +1715,9 @@ typedef struct gfc_symtree
 }
 gfc_symtree;
 
-/* A linked list of derived types in the namespace.  */
-typedef struct gfc_dt_list
-{
-  struct gfc_symbol *derived;
-  struct gfc_dt_list *next;
-}
-gfc_dt_list;
+/* A list of all derived types.  */
+extern gfc_symbol *gfc_derived_types;
 
-#define gfc_get_dt_list() XCNEW (gfc_dt_list)
-
-  /* A list of all derived types.  */
-  extern gfc_dt_list *gfc_derived_types;
-
 typedef struct gfc_oacc_routine_name
 {
   struct gfc_symbol *sym;
@@ -1809,7 +1802,7 @@ typedef struct gfc_namespace
 
   gfc_charlen *cl_list;
 
-  gfc_dt_list *derived_types;
+  gfc_symbol *derived_types;
 
   int save_all, seen_save, seen_implicit_none;
 
Index: gcc/fortran/parse.c
===================================================================
--- gcc/fortran/parse.c	(revision 260682)
+++ gcc/fortran/parse.c	(working copy)
@@ -6047,7 +6047,7 @@ add_global_program (void)
 static void
 resolve_all_program_units (gfc_namespace *gfc_global_ns_list)
 {
-  gfc_free_dt_list ();
+  gfc_derived_types = NULL;
   gfc_current_ns = gfc_global_ns_list;
   for (; gfc_current_ns; gfc_current_ns = gfc_current_ns->sibling)
     {
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 260682)
+++ gcc/fortran/resolve.c	(working copy)
@@ -2504,7 +2504,7 @@ resolve_global_procedure (gfc_symbol *sym, locus *
       /* Resolve the gsymbol namespace if needed.  */
       if (!gsym->ns->resolved)
 	{
-	  gfc_dt_list *old_dt_list;
+	  gfc_symbol *old_dt_list;
 
 	  /* Stash away derived types so that the backend_decls do not
 	     get mixed up.  */
@@ -13435,16 +13435,17 @@ resolve_typebound_procedures (gfc_symbol* derived)
 static void
 add_dt_to_dt_list (gfc_symbol *derived)
 {
-  gfc_dt_list *dt_list;
 
-  for (dt_list = gfc_derived_types; dt_list; dt_list = dt_list->next)
-    if (derived == dt_list->derived)
-      return;
-
-  dt_list = gfc_get_dt_list ();
-  dt_list->next = gfc_derived_types;
-  dt_list->derived = derived;
-  gfc_derived_types = dt_list;
+  if (!derived->dt_next) {
+    if (gfc_derived_types) {
+      derived->dt_next = gfc_derived_types->dt_next;
+      gfc_derived_types->dt_next = derived;
+    } else {
+      derived->dt_next = derived;
+    }
+    gfc_derived_types = derived;
+  }
+  
 }
 
 
Index: gcc/fortran/symbol.c
===================================================================
--- gcc/fortran/symbol.c	(revision 260682)
+++ gcc/fortran/symbol.c	(working copy)
@@ -107,7 +107,7 @@ gfc_namespace *gfc_global_ns_list;
 
 gfc_gsymbol *gfc_gsym_root = NULL;
 
-gfc_dt_list *gfc_derived_types;
+gfc_symbol *gfc_derived_types;
 
 static gfc_undo_change_set default_undo_chgset_var = { vNULL, vNULL, NULL };
 static gfc_undo_change_set *latest_undo_chgset = &default_undo_chgset_var;
@@ -3119,6 +3119,7 @@ gfc_new_symbol (const char *name, gfc_namespace *n
   p->common_block = NULL;
   p->f2k_derived = NULL;
   p->assoc = NULL;
+  p->dt_next = NULL;
   p->fn_result_spec = 0;
 
   return p;
@@ -3878,23 +3879,6 @@ free_sym_tree (gfc_symtree *sym_tree)
 }
 
 
-/* Free the derived type list.  */
-
-void
-gfc_free_dt_list (void)
-{
-  gfc_dt_list *dt, *n;
-
-  for (dt = gfc_derived_types; dt; dt = n)
-    {
-      n = dt->next;
-      free (dt);
-    }
-
-  gfc_derived_types = NULL;
-}
-
-
 /* Free the gfc_equiv_info's.  */
 
 static void
@@ -4080,7 +4064,7 @@ gfc_symbol_done_2 (void)
       gfc_free_namespace (gfc_current_ns);
       gfc_current_ns = NULL;
     }
-  gfc_free_dt_list ();
+  gfc_derived_types = NULL;
 
   enforce_single_undo_checkpoint ();
   free_undo_change_set_data (*latest_undo_chgset);
@@ -4343,7 +4327,7 @@ gfc_get_gsymbol (const char *name)
 static gfc_symbol *
 get_iso_c_binding_dt (int sym_id)
 {
-  gfc_dt_list *dt_list;
+  gfc_symbol *dt_list;
 
   dt_list = gfc_derived_types;
 
@@ -4350,15 +4334,17 @@ get_iso_c_binding_dt (int sym_id)
   /* Loop through the derived types in the name list, searching for
      the desired symbol from iso_c_binding.  Search the parent namespaces
      if necessary and requested to (parent_flag).  */
-  while (dt_list != NULL)
-    {
-      if (dt_list->derived->from_intmod != INTMOD_NONE
-	  && dt_list->derived->intmod_sym_id == sym_id)
-        return dt_list->derived;
+  if (dt_list) {
+    while (dt_list->dt_next != gfc_derived_types)
+      {
+	if (dt_list->from_intmod != INTMOD_NONE
+	    && dt_list->intmod_sym_id == sym_id)
+	  return dt_list;
+	
+	dt_list = dt_list->dt_next;
+      }
+  }
 
-      dt_list = dt_list->next;
-    }
-
   return NULL;
 }
 
@@ -4762,11 +4748,13 @@ generate_isocbinding_symbol (const char *mod_name,
       if (tmp_sym->attr.flavor == FL_DERIVED
 	  && !get_iso_c_binding_dt (tmp_sym->intmod_sym_id))
 	{
-	  gfc_dt_list *dt_list;
-	  dt_list = gfc_get_dt_list ();
-	  dt_list->derived = tmp_sym;
-	  dt_list->next = gfc_derived_types;
-  	  gfc_derived_types = dt_list;
+	  if (gfc_derived_types) {
+	    tmp_sym->dt_next = gfc_derived_types->dt_next;
+	    gfc_derived_types->dt_next = tmp_sym;
+	  } else {
+	    tmp_sym->dt_next = tmp_sym;
+	  }
+	  gfc_derived_types = tmp_sym;
         }
 
       return tmp_symtree;
@@ -4874,7 +4862,6 @@ generate_isocbinding_symbol (const char *mod_name,
       case ISOCBINDING_FUNPTR:
 	{
 	  gfc_symbol *dt_sym;
-	  gfc_dt_list **dt_list_ptr = NULL;
 	  gfc_component *tmp_comp = NULL;
 
 	  /* Generate real derived type.  */
@@ -4936,18 +4923,14 @@ generate_isocbinding_symbol (const char *mod_name,
 	  dt_sym->ts.u.derived = dt_sym;
 
 	  /* Add the symbol created for the derived type to the current ns.  */
-	  dt_list_ptr = &(gfc_derived_types);
-	  while (*dt_list_ptr != NULL && (*dt_list_ptr)->next != NULL)
-	    dt_list_ptr = &((*dt_list_ptr)->next);
-
-	  /* There is already at least one derived type in the list, so append
-	     the one we're currently building for c_ptr or c_funptr.  */
-	  if (*dt_list_ptr != NULL)
-	    dt_list_ptr = &((*dt_list_ptr)->next);
-	  (*dt_list_ptr) = gfc_get_dt_list ();
-	  (*dt_list_ptr)->derived = dt_sym;
-	  (*dt_list_ptr)->next = NULL;
-
+	  if (gfc_derived_types) {
+	    dt_sym->dt_next = gfc_derived_types->dt_next;
+	    gfc_derived_types->dt_next = dt_sym;
+	  } else {
+	    dt_sym->dt_next = dt_sym;
+	  }
+	  gfc_derived_types = dt_sym;
+      	  
 	  gfc_add_component (dt_sym, "c_address", &tmp_comp);
 	  if (tmp_comp == NULL)
 	    gcc_unreachable ();
Index: gcc/fortran/trans-types.c
===================================================================
--- gcc/fortran/trans-types.c	(revision 260682)
+++ gcc/fortran/trans-types.c	(working copy)
@@ -2534,7 +2534,7 @@ gfc_get_derived_type (gfc_symbol * derived, int co
   bool got_canonical = false;
   bool unlimited_entity = false;
   gfc_component *c;
-  gfc_dt_list *dt;
+  gfc_symbol *dt;
   gfc_namespace *ns;
   tree tmp;
   bool coarray_flag;
@@ -2599,13 +2599,18 @@ gfc_get_derived_type (gfc_symbol * derived, int co
 	   ns->translated && !got_canonical;
 	   ns = ns->sibling)
 	{
-	  dt = ns->derived_types;
-	  for (; dt && !canonical; dt = dt->next)
-	    {
-	      gfc_copy_dt_decls_ifequal (dt->derived, derived, true);
-	      if (derived->backend_decl)
-		got_canonical = true;
-	    }
+	  if (ns->derived_types) {
+	    dt = ns->derived_types;
+	    for (;;)
+	      {
+		gfc_copy_dt_decls_ifequal (dt, derived, true);
+		if (derived->backend_decl)
+		  got_canonical = true;
+		if (dt->dt_next == ns->derived_types)
+		  break;
+		dt = dt->dt_next;
+	      }
+	  }
 	}
     }
 
@@ -2867,9 +2872,16 @@ copy_derived_types:
 	}
     }
 
-  for (dt = gfc_derived_types; dt; dt = dt->next)
-    gfc_copy_dt_decls_ifequal (derived, dt->derived, false);
-
+  if (gfc_derived_types) {
+    dt = gfc_derived_types;
+    for (;;)
+      {
+	gfc_copy_dt_decls_ifequal (derived, dt, false);
+	if (dt->dt_next == gfc_derived_types)
+	  break;
+	dt = dt->dt_next;
+      }
+  }
   return derived->backend_decl;
 }
 

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Richard Biener]

On Tue, May 29, 2018 at 10:24 PM Andrew Benson <abenson@carnegiescience.edu>
wrote:

> Yes - definitely possible to remove gfc_dt_list entirely - new patch is
> attached.

This looks good to me but it still requires review/ack from fortran people.

Note another trick commonly used for cyclic lists is to make the "head"
always present which could be done by making gfc_derived_types a
gfc_symbol  (non-pointer), chaining to itself.  Then you can elide
the if (gfc_derived_types) checks and iterate via

for (gfc_sybol *sym = gfc_derived_types.dt_next; sym != &gfc_derived_types;
sym = sym->dt_next)
   ...

similar appending and removing lose some special cases.

Having a global gfc_symbol with just one pointer in it used may seem a
litte ugly though
(and you have to init it somewhere suitable unless you want to write up a
huge mostly
zero static initializer).

Richard.

> Thanks,
> Andrew

> On Monday, May 28, 2018 11:54:41 AM PDT Richard Biener wrote:
> > On Fri, May 25, 2018 at 11:54 PM Andrew Benson <
abenson@carnegiescience.edu>
> > wrote:
> > > Richard:
> > >
> > > Thanks for the suggestion. I changed my patch (new version attached)
so
> >
> > that
> >
> > > there's a *dt_next in gfc_symbol, which is now used to construct the
> > > (circular) linked list. There were a couple places where I had to
change
> >
> > the
> >
> > > order in which clean up of symbols and derived type lists were done -
> >
> > it's now
> >
> > > necessary to free the derived type list before its associated symbols
> >
> > (since
> >
> > > the symbols carry the links for the derived type list).
> >
> > Hmm, it still has the indirection via gfc_dt_list.  I think it should be
> > possible
> > to do away with gfc_dt_list objects alltogether by no doing
> > sym->dt_next->derived
> > but sym->derived thus
> >
> > @@ -1611,6 +1611,9 @@ typedef struct gfc_symbol
> >
> >     /* Link to corresponding association-list if this is an associate
name.
> >   */
> >     struct gfc_association_list *assoc;
> > +
> > +  /* Link to next entry in derived type list */
> > +  gfc_symbol *dt_next;
> >   }
> >   gfc_symbol;
> >
> > that means for example gfc_free_dt_list can be simply removed.  The
> > gfc_derived_types global would then point to the first derived type
> > directly.
> >
> > Richard.
> >
> > > This passes "make check-fortran" and seems to retain the speed-up
from my
> > > original patch.
> > >
> > > Thanks,
> > > Andrew
> > >
> > > On Friday, May 25, 2018 9:06:22 AM PDT Richard Biener wrote:
> > > > On Fri, May 25, 2018 at 12:53 AM Andrew Benson <
> >
> > abenson@carnegiescience.edu>
> >
> > > > wrote:
> > > > > I've been attempting to track down some of the causes of very long
> >
> > compile
> >
> > > > > times for files which use modules that contain a large number of
> >
> > symbols.
> >
> > > > The
> > > >
> > > > > worst case offender in my code takes around 12 minutes to compile.
> > > > >
> > > > > After profiling f951 for this source file it turns out that the
> >
> > majority
> >
> > > > of the
> > > >
> > > > > time is spent in add_dt_to_dt_list() in resolve.c. In cases where
the
> > > >
> > > > number
> > > >
> > > > > of symbols imported becomes very large (~10,000 in some cases in
this
> > > >
> > > > code),
> > > >
> > > > > the O(N) search in this function becomes inefficient.
> > > > >
> > > > > A simple optimization for this problem seems to be to just have
the
> > > >
> > > > gfc_symbol
> > > >
> > > > > struct include a pointer back to the corresponding entry in the
> > > > > gfc_derived_types list. It's then fast to check if a symbol is
> >
> > already on
> >
> > > > that
> > > >
> > > > > list by checking if this pointer is non-null. (It could just as
> >
> > easily be
> >
> > > > an
> > > >
> > > > > int in gfc_symbol which indicates if the symbol is already added
to
> >
> > the
> >
> > > > list -
> > > >
> > > > > I don't know if having a pointer to the list entry is useful for
any
> >
> > other
> >
> > > > > reason.)
> > > > >
> > > > > With this change in place compile times are much faster - my worst
> >
> > case
> >
> > > > > offender now takes just under 1 minute to compile.
> > > > >
> > > > > My patch is attached. I freely admit that I have only a very
shallow
> > > > > understanding of the inner workings of the compiler, so I would
not be
> > > > > surprised if there are good reasons not to do this. I did "make
> > > >
> > > > check-fortran"
> > > >
> > > > > and did not see any failures. If any one wants to try this out
and/or
> > > >
> > > > provide
> > > >
> > > > > any feedback I'd be happy to hear it.
> > > >
> > > > It looks like it would be cheaper to simply embed gtc_dt_list *next
in
> > > > gfc_symbol?
> > > > (in case a gfc_symbol can be only on one gfc_dt_list which your
patch
> > > > assumes as well)
> > > >
> > > > Richard.
> > > >
> > > > > Thanks,
> > > > > Andrew
> > > > >
> > > > > --
> >
> > > > > * Andrew Benson:
> > http://users.obs.carnegiescience.edu/abenson/contact.html
> >
> > > > > * Galacticus: https://bitbucket.org/abensonca/galacticus
> > >
> > > --
> > >
> > > * Andrew Benson:
http://users.obs.carnegiescience.edu/abenson/contact.html
> > >
> > > * Galacticus: https://bitbucket.org/abensonca/galacticus


> --

> * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

> * Galacticus: https://bitbucket.org/abensonca/galacticus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

On Wednesday, May 30, 2018 11:43:58 AM PDT Richard Biener wrote:
> On Tue, May 29, 2018 at 10:24 PM Andrew Benson <abenson@carnegiescience.edu>
> wrote:
> > Yes - definitely possible to remove gfc_dt_list entirely - new patch is
> > attached.
> 
> This looks good to me but it still requires review/ack from fortran people.

Thanks - I'll wait on input from the fortran devs.

> Note another trick commonly used for cyclic lists is to make the "head"
> always present which could be done by making gfc_derived_types a
> gfc_symbol  (non-pointer), chaining to itself.  Then you can elide
> the if (gfc_derived_types) checks and iterate via
> 
> for (gfc_sybol *sym = gfc_derived_types.dt_next; sym != &gfc_derived_types;
> sym = sym->dt_next)
>    ...
> 
> similar appending and removing lose some special cases.
> 
> Having a global gfc_symbol with just one pointer in it used may seem a
> litte ugly though
> (and you have to init it somewhere suitable unless you want to write up a
> huge mostly
> zero static initializer).

That would definitely make the code cleaner. I'll wait on input from the 
fortran devs to see if any of them have an opinion on this - and I'd probably 
need some guidance on where to do the init.

-Andrew

-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Steve Kargl]

On Wed, May 30, 2018 at 10:01:12AM -0700, Andrew Benson wrote:
> On Wednesday, May 30, 2018 11:43:58 AM PDT Richard Biener wrote:
> > On Tue, May 29, 2018 at 10:24 PM Andrew Benson <abenson@carnegiescience.edu>
> > wrote:
> > > Yes - definitely possible to remove gfc_dt_list entirely - new patch is
> > > attached.
> > 
> > This looks good to me but it still requires review/ack from fortran people.
> 
> Thanks - I'll wait on input from the fortran devs.
> 

Andrew,

You and Richard were having what appeared to be a productive
exchange, so I thought I would sit on the side and let it
reach a conclusion.  I'll take a look at what you've done
this weekend.  Hopefully, others can also cast an eye over
the patch as well.

PS: Welcome to the wonderful wonder of gfortran hacking.

-- 
Steve

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

On Wednesday, May 30, 2018 11:25:41 AM PDT Steve Kargl wrote:
> On Wed, May 30, 2018 at 10:01:12AM -0700, Andrew Benson wrote:
> > On Wednesday, May 30, 2018 11:43:58 AM PDT Richard Biener wrote:
> > > On Tue, May 29, 2018 at 10:24 PM Andrew Benson
> > > <abenson@carnegiescience.edu>> > 
> > > wrote:
> > > > Yes - definitely possible to remove gfc_dt_list entirely - new patch
> > > > is
> > > > attached.
> > > 
> > > This looks good to me but it still requires review/ack from fortran
> > > people.
> > 
> > Thanks - I'll wait on input from the fortran devs.
>
> You and Richard were having what appeared to be a productive
> exchange, so I thought I would sit on the side and let it
> reach a conclusion.  I'll take a look at what you've done
> this weekend.  Hopefully, others can also cast an eye over
> the patch as well.

Thanks - greatly appreciated!

> PS: Welcome to the wonderful wonder of gfortran hacking.

I suspect this is a slippery slope.... but so far it's been fun and 
instructive for me at least!

-Andrew

-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Janus Weil]

Hi Andrew,

2018-05-29 22:24 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
> Yes - definitely possible to remove gfc_dt_list entirely - new patch is
> attached.

since you already got some 'contentual' comments, I'll give you some
more 'formal' ones ...


+  if (!derived->dt_next) {
+    if (gfc_derived_types) {
+      derived->dt_next = gfc_derived_types->dt_next;
+      gfc_derived_types->dt_next = derived;
+    } else {
+      derived->dt_next = derived;
+    }
+    gfc_derived_types = derived;
+  }

Here and in some other hunks you're not conforming with the GNU coding
style, which demands that opening and closing braces should be on
separate lines (and at a new indentation level).


-  for (dt = gfc_derived_types; dt; dt = dt->next)
-    gfc_copy_dt_decls_ifequal (derived, dt->derived, false);
-
+  if (gfc_derived_types) {
+    dt = gfc_derived_types;
+    for (;;)
+      {
+    gfc_copy_dt_decls_ifequal (derived, dt, false);
+    if (dt->dt_next == gfc_derived_types)
+      break;
+    dt = dt->dt_next;
+      }
+  }

Is there a particular reason why you changed the loop to "for (;;)" ?
I find the old style a bit clearer and more compact. Also I think it's
more common in gfortran.

Btw, do you already have a copyright assignment on file? If not,
you'll probably need one (see https://gcc.gnu.org/contribute.html).

Thanks for your contribution and welcome to the gfortran team :)

Cheers,
Janus



> On Monday, May 28, 2018 11:54:41 AM PDT Richard Biener wrote:
>> On Fri, May 25, 2018 at 11:54 PM Andrew Benson <abenson@carnegiescience.edu>
>> wrote:
>> > Richard:
>> >
>> > Thanks for the suggestion. I changed my patch (new version attached) so
>>
>> that
>>
>> > there's a *dt_next in gfc_symbol, which is now used to construct the
>> > (circular) linked list. There were a couple places where I had to change
>>
>> the
>>
>> > order in which clean up of symbols and derived type lists were done -
>>
>> it's now
>>
>> > necessary to free the derived type list before its associated symbols
>>
>> (since
>>
>> > the symbols carry the links for the derived type list).
>>
>> Hmm, it still has the indirection via gfc_dt_list.  I think it should be
>> possible
>> to do away with gfc_dt_list objects alltogether by no doing
>> sym->dt_next->derived
>> but sym->derived thus
>>
>> @@ -1611,6 +1611,9 @@ typedef struct gfc_symbol
>>
>>     /* Link to corresponding association-list if this is an associate name.
>>   */
>>     struct gfc_association_list *assoc;
>> +
>> +  /* Link to next entry in derived type list */
>> +  gfc_symbol *dt_next;
>>   }
>>   gfc_symbol;
>>
>> that means for example gfc_free_dt_list can be simply removed.  The
>> gfc_derived_types global would then point to the first derived type
>> directly.
>>
>> Richard.
>>
>> > This passes "make check-fortran" and seems to retain the speed-up from my
>> > original patch.
>> >
>> > Thanks,
>> > Andrew
>> >
>> > On Friday, May 25, 2018 9:06:22 AM PDT Richard Biener wrote:
>> > > On Fri, May 25, 2018 at 12:53 AM Andrew Benson <
>>
>> abenson@carnegiescience.edu>
>>
>> > > wrote:
>> > > > I've been attempting to track down some of the causes of very long
>>
>> compile
>>
>> > > > times for files which use modules that contain a large number of
>>
>> symbols.
>>
>> > > The
>> > >
>> > > > worst case offender in my code takes around 12 minutes to compile.
>> > > >
>> > > > After profiling f951 for this source file it turns out that the
>>
>> majority
>>
>> > > of the
>> > >
>> > > > time is spent in add_dt_to_dt_list() in resolve.c. In cases where the
>> > >
>> > > number
>> > >
>> > > > of symbols imported becomes very large (~10,000 in some cases in this
>> > >
>> > > code),
>> > >
>> > > > the O(N) search in this function becomes inefficient.
>> > > >
>> > > > A simple optimization for this problem seems to be to just have the
>> > >
>> > > gfc_symbol
>> > >
>> > > > struct include a pointer back to the corresponding entry in the
>> > > > gfc_derived_types list. It's then fast to check if a symbol is
>>
>> already on
>>
>> > > that
>> > >
>> > > > list by checking if this pointer is non-null. (It could just as
>>
>> easily be
>>
>> > > an
>> > >
>> > > > int in gfc_symbol which indicates if the symbol is already added to
>>
>> the
>>
>> > > list -
>> > >
>> > > > I don't know if having a pointer to the list entry is useful for any
>>
>> other
>>
>> > > > reason.)
>> > > >
>> > > > With this change in place compile times are much faster - my worst
>>
>> case
>>
>> > > > offender now takes just under 1 minute to compile.
>> > > >
>> > > > My patch is attached. I freely admit that I have only a very shallow
>> > > > understanding of the inner workings of the compiler, so I would not be
>> > > > surprised if there are good reasons not to do this. I did "make
>> > >
>> > > check-fortran"
>> > >
>> > > > and did not see any failures. If any one wants to try this out and/or
>> > >
>> > > provide
>> > >
>> > > > any feedback I'd be happy to hear it.
>> > >
>> > > It looks like it would be cheaper to simply embed gtc_dt_list *next in
>> > > gfc_symbol?
>> > > (in case a gfc_symbol can be only on one gfc_dt_list which your patch
>> > > assumes as well)
>> > >
>> > > Richard.
>> > >
>> > > > Thanks,
>> > > > Andrew
>> > > >
>> > > > --
>>
>> > > > * Andrew Benson:
>> http://users.obs.carnegiescience.edu/abenson/contact.html
>>
>> > > > * Galacticus: https://bitbucket.org/abensonca/galacticus
>> >
>> > --
>> >
>> > * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html
>> >
>> > * Galacticus: https://bitbucket.org/abensonca/galacticus
>
>
> --
>
> * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html
>
> * Galacticus: https://bitbucket.org/abensonca/galacticus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

Hi Janus,

On Wednesday, May 30, 2018 10:42:46 PM PDT Janus Weil wrote:
> Hi Andrew,
> 
> 2018-05-29 22:24 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
> > Yes - definitely possible to remove gfc_dt_list entirely - new patch is
> > attached.
> 
> since you already got some 'contentual' comments, I'll give you some
> more 'formal' ones ...
> 
> 
> +  if (!derived->dt_next) {
> +    if (gfc_derived_types) {
> +      derived->dt_next = gfc_derived_types->dt_next;
> +      gfc_derived_types->dt_next = derived;
> +    } else {
> +      derived->dt_next = derived;
> +    }
> +    gfc_derived_types = derived;
> +  }
>
> Here and in some other hunks you're not conforming with the GNU coding
> style, which demands that opening and closing braces should be on
> separate lines (and at a new indentation level).

Thanks - I'll fix those cases.
 
> -  for (dt = gfc_derived_types; dt; dt = dt->next)
> -    gfc_copy_dt_decls_ifequal (derived, dt->derived, false);
> -
> +  if (gfc_derived_types) {
> +    dt = gfc_derived_types;
> +    for (;;)
> +      {
> +    gfc_copy_dt_decls_ifequal (derived, dt, false);
> +    if (dt->dt_next == gfc_derived_types)
> +      break;
> +    dt = dt->dt_next;
> +      }
> +  }
> 
> Is there a particular reason why you changed the loop to "for (;;)" ?
> I find the old style a bit clearer and more compact. Also I think it's
> more common in gfortran.

I changed the original for loop since it wasn't possible (as far as I could 
see) to have an exit condition that worked now that list is circularly linked 
(i.e. "dt" never becomes NULL, and testing for dt->dt_next == 
gfc_derived_types doesn't work as it would miss the final entry in the list). 
So, I used for(;;) and added the exit condition explicitly into the loop.

But, I agree, it's not very clear. An alternative would be something such as:
 
-  for (dt = gfc_derived_types; dt; dt = dt->next)
-    gfc_copy_dt_decls_ifequal (derived, dt->derived, false);
-
+  for (dt = gfc_derived_types; dt; dt = dt->dt_next)
+    {
+      gfc_copy_dt_decls_ifequal (derived, dt, false);
+      if (dt->dt_next == gfc_derived_types)
+	break;
+    }
+  

which is more compact, and has the advantage that if doesn't require an "if 
(gfc_derived_types)". Do you think that's a better approach?

> Btw, do you already have a copyright assignment on file? If not,
> you'll probably need one (see https://gcc.gnu.org/contribute.html).

I don't, so will need to get that figured out.

> Thanks for your contribution and welcome to the gfortran team :)

Thanks!

-Andrew

-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Janus Weil]

2018-05-31 0:22 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
>> -  for (dt = gfc_derived_types; dt; dt = dt->next)
>> -    gfc_copy_dt_decls_ifequal (derived, dt->derived, false);
>> -
>> +  if (gfc_derived_types) {
>> +    dt = gfc_derived_types;
>> +    for (;;)
>> +      {
>> +    gfc_copy_dt_decls_ifequal (derived, dt, false);
>> +    if (dt->dt_next == gfc_derived_types)
>> +      break;
>> +    dt = dt->dt_next;
>> +      }
>> +  }
>>
>> Is there a particular reason why you changed the loop to "for (;;)" ?
>> I find the old style a bit clearer and more compact. Also I think it's
>> more common in gfortran.
>
> I changed the original for loop since it wasn't possible (as far as I could
> see) to have an exit condition that worked now that list is circularly linked
> (i.e. "dt" never becomes NULL, and testing for dt->dt_next ==
> gfc_derived_types doesn't work as it would miss the final entry in the list).
> So, I used for(;;) and added the exit condition explicitly into the loop.
>
> But, I agree, it's not very clear. An alternative would be something such as:
>
> -  for (dt = gfc_derived_types; dt; dt = dt->next)
> -    gfc_copy_dt_decls_ifequal (derived, dt->derived, false);
> -
> +  for (dt = gfc_derived_types; dt; dt = dt->dt_next)
> +    {
> +      gfc_copy_dt_decls_ifequal (derived, dt, false);
> +      if (dt->dt_next == gfc_derived_types)
> +       break;
> +    }
> +
>
> which is more compact, and has the advantage that if doesn't require an "if
> (gfc_derived_types)". Do you think that's a better approach?

Yes, definitely looks better to me. Note that there is another such
case further up in gfc_get_derived_type. Actually I'd also move the
declaration of dt ("gfc_symbol *dt") into the loops, in order to make
it more local (it's not used outside).

Btw, another thing that you'll need is a ChangeLog entry that lists
every file and function touched by your patch and gives a short
description of the modifications. You'll find plenty of examples for
this in gcc/fortran/ChangeLog.

Cheers,
Janus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

[-- Attachment #1: Type: text/plain, Size: 4140 bytes --]

On Thursday, May 31, 2018 10:56:47 AM PDT Janus Weil wrote:
> 2018-05-31 0:22 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
> >> -  for (dt = gfc_derived_types; dt; dt = dt->next)
> >> -    gfc_copy_dt_decls_ifequal (derived, dt->derived, false);
> >> -
> >> +  if (gfc_derived_types) {
> >> +    dt = gfc_derived_types;
> >> +    for (;;)
> >> +      {
> >> +    gfc_copy_dt_decls_ifequal (derived, dt, false);
> >> +    if (dt->dt_next == gfc_derived_types)
> >> +      break;
> >> +    dt = dt->dt_next;
> >> +      }
> >> +  }
> >> 
> >> Is there a particular reason why you changed the loop to "for (;;)" ?
> >> I find the old style a bit clearer and more compact. Also I think it's
> >> more common in gfortran.
> > 
> > I changed the original for loop since it wasn't possible (as far as I
> > could
> > see) to have an exit condition that worked now that list is circularly
> > linked (i.e. "dt" never becomes NULL, and testing for dt->dt_next ==
> > gfc_derived_types doesn't work as it would miss the final entry in the
> > list). So, I used for(;;) and added the exit condition explicitly into
> > the loop.
> > 
> > But, I agree, it's not very clear. An alternative would be something such
> > as:
> > 
> > -  for (dt = gfc_derived_types; dt; dt = dt->next)
> > -    gfc_copy_dt_decls_ifequal (derived, dt->derived, false);
> > -
> > +  for (dt = gfc_derived_types; dt; dt = dt->dt_next)
> > +    {
> > +      gfc_copy_dt_decls_ifequal (derived, dt, false);
> > +      if (dt->dt_next == gfc_derived_types)
> > +       break;
> > +    }
> > +
> > 
> > which is more compact, and has the advantage that if doesn't require an
> > "if
> > (gfc_derived_types)". Do you think that's a better approach?
> 
> Yes, definitely looks better to me. Note that there is another such
> case further up in gfc_get_derived_type. Actually I'd also move the
> declaration of dt ("gfc_symbol *dt") into the loops, in order to make
> it more local (it's not used outside).

Thanks - an updated patch is attached with those changes.

While staring at the patch again there's something about the following in 
trans-types.c that seems possibly wrong:

@@ -2599,16 +2598,20 @@ gfc_get_derived_type (gfc_symbol * derived, int co
 	   ns->translated && !got_canonical;
 	   ns = ns->sibling)
 	{
-	  dt = ns->derived_types;
-	  for (; dt && !canonical; dt = dt->next)
+	  if (ns->derived_types)
 	    {
-	      gfc_copy_dt_decls_ifequal (dt->derived, derived, true);
-	      if (derived->backend_decl)
-		got_canonical = true;
+	      for (gfc_symbol *dt = ns->derived_types; dt; dt = dt->dt_next)
+		{
+		  gfc_copy_dt_decls_ifequal (dt, derived, true);
+		  if (derived->backend_decl)
+		    got_canonical = true;
+		  if (dt->dt_next == ns->derived_types)
+		    break;
+		}
 	    }
 	}
     }

In the original, the loop exits if "dt" becomes NULL (i.e. end of linked list 
found) or if "canonical" becomes non-null. I don't see any way for "canonical" 
to change within the loop though, so I suspect that the original should have 
been:

   for (; dt && !got_canonical; dt = dt->next)

i.e. tested "got_canonical" instead of "canonical". 

Currently my patch just removes the test of "canonical", and passes all tests 
cleanly. If I'm correct about this, the original, wrong test didn't cause any 
problems (since "canonical" is always NULL in this loop), but just means that 
there's no early exit from the loop once the canonical type is found.

If you agree with that I can add a "!got_canonical" test back in to my patch 
(I've already checked that it passes tests cleanly with that test in place).

> Btw, another thing that you'll need is a ChangeLog entry that lists
> every file and function touched by your patch and gives a short
> description of the modifications. You'll find plenty of examples for
> this in gcc/fortran/ChangeLog.

I've also attached a ChangeLog entry.

One other question: for copyright assignment, who do I need to talk to to get 
the relevant form(s)? 

Thanks,
Andrew
-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 8567 bytes --]

Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 260682)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -1611,6 +1611,9 @@ typedef struct gfc_symbol
 
   /* Link to corresponding association-list if this is an associate name.  */
   struct gfc_association_list *assoc;
+
+  /* Link to next entry in derived type list */
+  struct gfc_symbol *dt_next;
 }
 gfc_symbol;
 
@@ -1712,19 +1715,9 @@ typedef struct gfc_symtree
 }
 gfc_symtree;
 
-/* A linked list of derived types in the namespace.  */
-typedef struct gfc_dt_list
-{
-  struct gfc_symbol *derived;
-  struct gfc_dt_list *next;
-}
-gfc_dt_list;
+/* A list of all derived types.  */
+extern gfc_symbol *gfc_derived_types;
 
-#define gfc_get_dt_list() XCNEW (gfc_dt_list)
-
-  /* A list of all derived types.  */
-  extern gfc_dt_list *gfc_derived_types;
-
 typedef struct gfc_oacc_routine_name
 {
   struct gfc_symbol *sym;
@@ -1809,7 +1802,7 @@ typedef struct gfc_namespace
 
   gfc_charlen *cl_list;
 
-  gfc_dt_list *derived_types;
+  gfc_symbol *derived_types;
 
   int save_all, seen_save, seen_implicit_none;
 
Index: gcc/fortran/parse.c
===================================================================
--- gcc/fortran/parse.c	(revision 260682)
+++ gcc/fortran/parse.c	(working copy)
@@ -6047,7 +6047,7 @@ add_global_program (void)
 static void
 resolve_all_program_units (gfc_namespace *gfc_global_ns_list)
 {
-  gfc_free_dt_list ();
+  gfc_derived_types = NULL;
   gfc_current_ns = gfc_global_ns_list;
   for (; gfc_current_ns; gfc_current_ns = gfc_current_ns->sibling)
     {
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 260682)
+++ gcc/fortran/resolve.c	(working copy)
@@ -2504,7 +2504,7 @@ resolve_global_procedure (gfc_symbol *sym, locus *
       /* Resolve the gsymbol namespace if needed.  */
       if (!gsym->ns->resolved)
 	{
-	  gfc_dt_list *old_dt_list;
+	  gfc_symbol *old_dt_list;
 
 	  /* Stash away derived types so that the backend_decls do not
 	     get mixed up.  */
@@ -13435,16 +13435,21 @@ resolve_typebound_procedures (gfc_symbol* derived)
 static void
 add_dt_to_dt_list (gfc_symbol *derived)
 {
-  gfc_dt_list *dt_list;
 
-  for (dt_list = gfc_derived_types; dt_list; dt_list = dt_list->next)
-    if (derived == dt_list->derived)
-      return;
-
-  dt_list = gfc_get_dt_list ();
-  dt_list->next = gfc_derived_types;
-  dt_list->derived = derived;
-  gfc_derived_types = dt_list;
+  if (!derived->dt_next)
+    {
+      if (gfc_derived_types)
+	{
+	  derived->dt_next = gfc_derived_types->dt_next;
+	  gfc_derived_types->dt_next = derived;
+	}
+      else
+	{
+	  derived->dt_next = derived;
+	}
+      gfc_derived_types = derived;
+    }
+  
 }
 
 
Index: gcc/fortran/symbol.c
===================================================================
--- gcc/fortran/symbol.c	(revision 260682)
+++ gcc/fortran/symbol.c	(working copy)
@@ -107,7 +107,7 @@ gfc_namespace *gfc_global_ns_list;
 
 gfc_gsymbol *gfc_gsym_root = NULL;
 
-gfc_dt_list *gfc_derived_types;
+gfc_symbol *gfc_derived_types;
 
 static gfc_undo_change_set default_undo_chgset_var = { vNULL, vNULL, NULL };
 static gfc_undo_change_set *latest_undo_chgset = &default_undo_chgset_var;
@@ -3119,6 +3119,7 @@ gfc_new_symbol (const char *name, gfc_namespace *n
   p->common_block = NULL;
   p->f2k_derived = NULL;
   p->assoc = NULL;
+  p->dt_next = NULL;
   p->fn_result_spec = 0;
 
   return p;
@@ -3878,23 +3879,6 @@ free_sym_tree (gfc_symtree *sym_tree)
 }
 
 
-/* Free the derived type list.  */
-
-void
-gfc_free_dt_list (void)
-{
-  gfc_dt_list *dt, *n;
-
-  for (dt = gfc_derived_types; dt; dt = n)
-    {
-      n = dt->next;
-      free (dt);
-    }
-
-  gfc_derived_types = NULL;
-}
-
-
 /* Free the gfc_equiv_info's.  */
 
 static void
@@ -4080,7 +4064,7 @@ gfc_symbol_done_2 (void)
       gfc_free_namespace (gfc_current_ns);
       gfc_current_ns = NULL;
     }
-  gfc_free_dt_list ();
+  gfc_derived_types = NULL;
 
   enforce_single_undo_checkpoint ();
   free_undo_change_set_data (*latest_undo_chgset);
@@ -4343,7 +4327,7 @@ gfc_get_gsymbol (const char *name)
 static gfc_symbol *
 get_iso_c_binding_dt (int sym_id)
 {
-  gfc_dt_list *dt_list;
+  gfc_symbol *dt_list;
 
   dt_list = gfc_derived_types;
 
@@ -4350,13 +4334,16 @@ get_iso_c_binding_dt (int sym_id)
   /* Loop through the derived types in the name list, searching for
      the desired symbol from iso_c_binding.  Search the parent namespaces
      if necessary and requested to (parent_flag).  */
-  while (dt_list != NULL)
+  if (dt_list)
     {
-      if (dt_list->derived->from_intmod != INTMOD_NONE
-	  && dt_list->derived->intmod_sym_id == sym_id)
-        return dt_list->derived;
-
-      dt_list = dt_list->next;
+      while (dt_list->dt_next != gfc_derived_types)
+	{
+	  if (dt_list->from_intmod != INTMOD_NONE
+	      && dt_list->intmod_sym_id == sym_id)
+	    return dt_list;
+	
+	  dt_list = dt_list->dt_next;
+	}
     }
 
   return NULL;
@@ -4762,11 +4749,16 @@ generate_isocbinding_symbol (const char *mod_name,
       if (tmp_sym->attr.flavor == FL_DERIVED
 	  && !get_iso_c_binding_dt (tmp_sym->intmod_sym_id))
 	{
-	  gfc_dt_list *dt_list;
-	  dt_list = gfc_get_dt_list ();
-	  dt_list->derived = tmp_sym;
-	  dt_list->next = gfc_derived_types;
-  	  gfc_derived_types = dt_list;
+	  if (gfc_derived_types)
+	    {
+	      tmp_sym->dt_next = gfc_derived_types->dt_next;
+	      gfc_derived_types->dt_next = tmp_sym;
+	    }
+	  else
+	    {
+	      tmp_sym->dt_next = tmp_sym;
+	    }
+	  gfc_derived_types = tmp_sym;
         }
 
       return tmp_symtree;
@@ -4874,7 +4866,6 @@ generate_isocbinding_symbol (const char *mod_name,
       case ISOCBINDING_FUNPTR:
 	{
 	  gfc_symbol *dt_sym;
-	  gfc_dt_list **dt_list_ptr = NULL;
 	  gfc_component *tmp_comp = NULL;
 
 	  /* Generate real derived type.  */
@@ -4936,18 +4927,17 @@ generate_isocbinding_symbol (const char *mod_name,
 	  dt_sym->ts.u.derived = dt_sym;
 
 	  /* Add the symbol created for the derived type to the current ns.  */
-	  dt_list_ptr = &(gfc_derived_types);
-	  while (*dt_list_ptr != NULL && (*dt_list_ptr)->next != NULL)
-	    dt_list_ptr = &((*dt_list_ptr)->next);
-
-	  /* There is already at least one derived type in the list, so append
-	     the one we're currently building for c_ptr or c_funptr.  */
-	  if (*dt_list_ptr != NULL)
-	    dt_list_ptr = &((*dt_list_ptr)->next);
-	  (*dt_list_ptr) = gfc_get_dt_list ();
-	  (*dt_list_ptr)->derived = dt_sym;
-	  (*dt_list_ptr)->next = NULL;
-
+	  if (gfc_derived_types)
+	    {
+	      dt_sym->dt_next = gfc_derived_types->dt_next;
+	      gfc_derived_types->dt_next = dt_sym;
+	    }
+	  else
+	    {
+	      dt_sym->dt_next = dt_sym;
+	    }
+	  gfc_derived_types = dt_sym;
+      	  
 	  gfc_add_component (dt_sym, "c_address", &tmp_comp);
 	  if (tmp_comp == NULL)
 	    gcc_unreachable ();
Index: gcc/fortran/trans-types.c
===================================================================
--- gcc/fortran/trans-types.c	(revision 260682)
+++ gcc/fortran/trans-types.c	(working copy)
@@ -2534,7 +2534,6 @@ gfc_get_derived_type (gfc_symbol * derived, int co
   bool got_canonical = false;
   bool unlimited_entity = false;
   gfc_component *c;
-  gfc_dt_list *dt;
   gfc_namespace *ns;
   tree tmp;
   bool coarray_flag;
@@ -2599,16 +2598,20 @@ gfc_get_derived_type (gfc_symbol * derived, int co
 	   ns->translated && !got_canonical;
 	   ns = ns->sibling)
 	{
-	  dt = ns->derived_types;
-	  for (; dt && !canonical; dt = dt->next)
+	  if (ns->derived_types)
 	    {
-	      gfc_copy_dt_decls_ifequal (dt->derived, derived, true);
-	      if (derived->backend_decl)
-		got_canonical = true;
+	      for (gfc_symbol *dt = ns->derived_types; dt; dt = dt->dt_next)
+		{
+		  gfc_copy_dt_decls_ifequal (dt, derived, true);
+		  if (derived->backend_decl)
+		    got_canonical = true;
+		  if (dt->dt_next == ns->derived_types)
+		    break;
+		}
 	    }
 	}
     }
-
+  
   /* Store up the canonical type to be added to this one.  */
   if (got_canonical)
     {
@@ -2866,10 +2869,14 @@ copy_derived_types:
 	  TREE_NO_WARNING (c->caf_token) = 1;
 	}
     }
-
-  for (dt = gfc_derived_types; dt; dt = dt->next)
-    gfc_copy_dt_decls_ifequal (derived, dt->derived, false);
-
+  
+  for (gfc_symbol *dt = gfc_derived_types; dt; dt = dt->dt_next)
+    {
+      gfc_copy_dt_decls_ifequal (derived, dt, false);
+      if (dt->dt_next == gfc_derived_types)
+	break;
+    }
+  
   return derived->backend_decl;
 }
 

[-- Attachment #3: ChangeLog --]
[-- Type: text/x-changelog, Size: 808 bytes --]

2018-05-31  Andrew Benson  <abenson@carnegiescience.edu>

	* gfortran.h: Add pointer to next derived type to
	gfc_symbol. Remove gfc_dt_list.
	* parse.c (resolve_all_program_units, resolve_global_procedure)
	(resolve_typebound_procedures): Replace gfc_free_dt_list() with
	simple nullification of gfc_derived_types. Change derived type
	linked list insertion to utilize dt_next pointers in gfc_symbol.
	* symbol.c (gfc_new_symbol, free_sym_tree, gfc_symbol_done2)
	(gfc_get_gsymbol, get_iso_c_binding_dt)
	(generate_isocbinding_symbol): Remove gfc_free_dt_list as
	gfc_dt_list is obsoleted. Change derived type linked list
	search/insertion to utilize dt_next pointers in gfc_symbol.
	* trans-types.c (gfc_get_derived_type): Change derived type linked
	list search to utilize dt_next pointers in gfc_symbol.

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Janus Weil]

2018-05-31 20:04 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
> One other question: for copyright assignment, who do I need to talk to to get
> the relevant form(s)?

I think you need to send a request to assign@gnu.org and
gcc@gcc.gnu.org in order to get the copyright assignment form. After
the form is mailed to you, you sign it and send it back to the FSF.

Cheers,
Janus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Steve Kargl]

On Fri, Jun 01, 2018 at 08:14:04AM +0200, Janus Weil wrote:
> 2018-05-31 20:04 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
> > One other question: for copyright assignment, who do I need to
> > talk to to get the relevant form(s)?
> 
> I think you need to send a request to assign@gnu.org and
> gcc@gcc.gnu.org in order to get the copyright assignment form. After
> the form is mailed to you, you sign it and send it back to the FSF.
> 

Andrew, I see that you've asked on gcc@gcc on June 1 about
Copyright forms.  Has anyone responded?

I ask because my patch for PR fortran/68544 walks the
gfc_derived_types list.

https://gcc.gnu.org/ml/fortran/2018-06/msg00054.html

and Thomas has approved the patch.  In my patch, I
have

+static bool
+is_dt_name (const char *name)
+{
+  gfc_dt_list *dt_list;
+
+  for (dt_list = gfc_derived_types; dt_list; dt_list = dt_list->next)
+    if (strcmp(dt_list->derived->name, name) == 0)
+      return true;
+  return false;
+}

we'll need to update this to deal with your change for a
circular linked list.

-- 
Steve

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

Hi Steve,

Yes - I did get a response from gcc@gcc, and then got the copyright assignment 
forms sent to me. I'm now waiting on my employer signing off on the waiver so 
that I can return the forms. I'll be following up with them today to hopefully 
move the process along. I'll let you know as soon as I get the forms 
submitted.

Thanks,
Andrew

On Monday, June 11, 2018 11:17:10 AM PDT Steve Kargl wrote:
> On Fri, Jun 01, 2018 at 08:14:04AM +0200, Janus Weil wrote:
> > 2018-05-31 20:04 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
> > > One other question: for copyright assignment, who do I need to
> > > talk to to get the relevant form(s)?
> > 
> > I think you need to send a request to assign@gnu.org and
> > gcc@gcc.gnu.org in order to get the copyright assignment form. After
> > the form is mailed to you, you sign it and send it back to the FSF.
> 
> Andrew, I see that you've asked on gcc@gcc on June 1 about
> Copyright forms.  Has anyone responded?
> 
> I ask because my patch for PR fortran/68544 walks the
> gfc_derived_types list.
> 
> https://gcc.gnu.org/ml/fortran/2018-06/msg00054.html
> 
> and Thomas has approved the patch.  In my patch, I
> have
> 
> +static bool
> +is_dt_name (const char *name)
> +{
> +  gfc_dt_list *dt_list;
> +
> +  for (dt_list = gfc_derived_types; dt_list; dt_list = dt_list->next)
> +    if (strcmp(dt_list->derived->name, name) == 0)
> +      return true;
> +  return false;
> +}
> 
> we'll need to update this to deal with your change for a
> circular linked list.


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

Steve,

I have just sent my copyright assignment documents back to the FSF, so I think 
it should be ok to submit my patch as soon as it has approval.

-Andrew

On Monday, June 11, 2018 11:17:10 AM PDT Steve Kargl wrote:
> On Fri, Jun 01, 2018 at 08:14:04AM +0200, Janus Weil wrote:
> > 2018-05-31 20:04 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
> > > One other question: for copyright assignment, who do I need to
> > > talk to to get the relevant form(s)?
> > 
> > I think you need to send a request to assign@gnu.org and
> > gcc@gcc.gnu.org in order to get the copyright assignment form. After
> > the form is mailed to you, you sign it and send it back to the FSF.
> 
> Andrew, I see that you've asked on gcc@gcc on June 1 about
> Copyright forms.  Has anyone responded?
> 
> I ask because my patch for PR fortran/68544 walks the
> gfc_derived_types list.
> 
> https://gcc.gnu.org/ml/fortran/2018-06/msg00054.html
> 
> and Thomas has approved the patch.  In my patch, I
> have
> 
> +static bool
> +is_dt_name (const char *name)
> +{
> +  gfc_dt_list *dt_list;
> +
> +  for (dt_list = gfc_derived_types; dt_list; dt_list = dt_list->next)
> +    if (strcmp(dt_list->derived->name, name) == 0)
> +      return true;
> +  return false;
> +}
> 
> we'll need to update this to deal with your change for a
> circular linked list.


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Steve Kargl]

Ping me when you hear back from FSF.  I'll apply your
current patch on the weekend to my tree and do some
testing.

-- 
steve

On Wed, Jun 13, 2018 at 03:55:59PM -0700, Andrew Benson wrote:
> Steve,
> 
> I have just sent my copyright assignment documents back to the FSF, so I think 
> it should be ok to submit my patch as soon as it has approval.
> 
> -Andrew
> 
> On Monday, June 11, 2018 11:17:10 AM PDT Steve Kargl wrote:
> > On Fri, Jun 01, 2018 at 08:14:04AM +0200, Janus Weil wrote:
> > > 2018-05-31 20:04 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
> > > > One other question: for copyright assignment, who do I need to
> > > > talk to to get the relevant form(s)?
> > > 
> > > I think you need to send a request to assign@gnu.org and
> > > gcc@gcc.gnu.org in order to get the copyright assignment form. After
> > > the form is mailed to you, you sign it and send it back to the FSF.
> > 
> > Andrew, I see that you've asked on gcc@gcc on June 1 about
> > Copyright forms.  Has anyone responded?
> > 
> > I ask because my patch for PR fortran/68544 walks the
> > gfc_derived_types list.
> > 
> > https://gcc.gnu.org/ml/fortran/2018-06/msg00054.html
> > 
> > and Thomas has approved the patch.  In my patch, I
> > have
> > 
> > +static bool
> > +is_dt_name (const char *name)
> > +{
> > +  gfc_dt_list *dt_list;
> > +
> > +  for (dt_list = gfc_derived_types; dt_list; dt_list = dt_list->next)
> > +    if (strcmp(dt_list->derived->name, name) == 0)
> > +      return true;
> > +  return false;
> > +}
> > 
> > we'll need to update this to deal with your change for a
> > circular linked list.
> 
> 
> -- 
> 
> * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html
> 
> * Galacticus: https://bitbucket.org/abensonca/galacticus

-- 
Steve
20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4
20161221 https://www.youtube.com/watch?v=IbCHE-hONow

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

I'll do that.

-Andrew

On Thursday, June 14, 2018 5:10:24 PM PDT Steve Kargl wrote:
> Ping me when you hear back from FSF.  I'll apply your
> current patch on the weekend to my tree and do some
> testing.
> 
> > Steve,
> > 
> > I have just sent my copyright assignment documents back to the FSF, so I
> > think it should be ok to submit my patch as soon as it has approval.
> > 
> > -Andrew
> > 
> > On Monday, June 11, 2018 11:17:10 AM PDT Steve Kargl wrote:
> > > On Fri, Jun 01, 2018 at 08:14:04AM +0200, Janus Weil wrote:
> > > > 2018-05-31 20:04 GMT+02:00 Andrew Benson 
<abenson@carnegiescience.edu>:
> > > > > One other question: for copyright assignment, who do I need to
> > > > > talk to to get the relevant form(s)?
> > > > 
> > > > I think you need to send a request to assign@gnu.org and
> > > > gcc@gcc.gnu.org in order to get the copyright assignment form. After
> > > > the form is mailed to you, you sign it and send it back to the FSF.
> > > 
> > > Andrew, I see that you've asked on gcc@gcc on June 1 about
> > > Copyright forms.  Has anyone responded?
> > > 
> > > I ask because my patch for PR fortran/68544 walks the
> > > gfc_derived_types list.
> > > 
> > > https://gcc.gnu.org/ml/fortran/2018-06/msg00054.html
> > > 
> > > and Thomas has approved the patch.  In my patch, I
> > > have
> > > 
> > > +static bool
> > > +is_dt_name (const char *name)
> > > +{
> > > +  gfc_dt_list *dt_list;
> > > +
> > > +  for (dt_list = gfc_derived_types; dt_list; dt_list = dt_list->next)
> > > +    if (strcmp(dt_list->derived->name, name) == 0)
> > > +      return true;
> > > +  return false;
> > > +}
> > > 
> > > we'll need to update this to deal with your change for a
> > > circular linked list.


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

[-- Attachment #1: Type: text/plain, Size: 2071 bytes --]

I now finally have heard back from the FSF, so my paperwork is all signed and 
recorded.

I've tested my patch on the latest trunk - slightly updated version which 
applies cleanly to trunk is attached (along with the ChangeLog). 

-Andrew 

On Thursday, June 14, 2018 5:10:24 PM PDT Steve Kargl wrote:
> Ping me when you hear back from FSF.  I'll apply your
> current patch on the weekend to my tree and do some
> testing.
> 
> > Steve,
> > 
> > I have just sent my copyright assignment documents back to the FSF, so I
> > think it should be ok to submit my patch as soon as it has approval.
> > 
> > -Andrew
> > 
> > On Monday, June 11, 2018 11:17:10 AM PDT Steve Kargl wrote:
> > > On Fri, Jun 01, 2018 at 08:14:04AM +0200, Janus Weil wrote:
> > > > 2018-05-31 20:04 GMT+02:00 Andrew Benson 
<abenson@carnegiescience.edu>:
> > > > > One other question: for copyright assignment, who do I need to
> > > > > talk to to get the relevant form(s)?
> > > > 
> > > > I think you need to send a request to assign@gnu.org and
> > > > gcc@gcc.gnu.org in order to get the copyright assignment form. After
> > > > the form is mailed to you, you sign it and send it back to the FSF.
> > > 
> > > Andrew, I see that you've asked on gcc@gcc on June 1 about
> > > Copyright forms.  Has anyone responded?
> > > 
> > > I ask because my patch for PR fortran/68544 walks the
> > > gfc_derived_types list.
> > > 
> > > https://gcc.gnu.org/ml/fortran/2018-06/msg00054.html
> > > 
> > > and Thomas has approved the patch.  In my patch, I
> > > have
> > > 
> > > +static bool
> > > +is_dt_name (const char *name)
> > > +{
> > > +  gfc_dt_list *dt_list;
> > > +
> > > +  for (dt_list = gfc_derived_types; dt_list; dt_list = dt_list->next)
> > > +    if (strcmp(dt_list->derived->name, name) == 0)
> > > +      return true;
> > > +  return false;
> > > +}
> > > 
> > > we'll need to update this to deal with your change for a
> > > circular linked list.


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 8599 bytes --]

Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 262545)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -1614,6 +1614,9 @@ typedef struct gfc_symbol
 
   /* Link to corresponding association-list if this is an associate name.  */
   struct gfc_association_list *assoc;
+
+  /* Link to next entry in derived type list */
+  struct gfc_symbol *dt_next;
 }
 gfc_symbol;
 
@@ -1715,19 +1718,9 @@ typedef struct gfc_symtree
 }
 gfc_symtree;
 
-/* A linked list of derived types in the namespace.  */
-typedef struct gfc_dt_list
-{
-  struct gfc_symbol *derived;
-  struct gfc_dt_list *next;
-}
-gfc_dt_list;
+/* A list of all derived types.  */
+extern gfc_symbol *gfc_derived_types;
 
-#define gfc_get_dt_list() XCNEW (gfc_dt_list)
-
-  /* A list of all derived types.  */
-  extern gfc_dt_list *gfc_derived_types;
-
 typedef struct gfc_oacc_routine_name
 {
   struct gfc_symbol *sym;
@@ -1812,7 +1805,7 @@ typedef struct gfc_namespace
 
   gfc_charlen *cl_list;
 
-  gfc_dt_list *derived_types;
+  gfc_symbol *derived_types;
 
   int save_all, seen_save, seen_implicit_none;
 
Index: gcc/fortran/parse.c
===================================================================
--- gcc/fortran/parse.c	(revision 262545)
+++ gcc/fortran/parse.c	(working copy)
@@ -6051,7 +6051,7 @@ add_global_program (void)
 static void
 resolve_all_program_units (gfc_namespace *gfc_global_ns_list)
 {
-  gfc_free_dt_list ();
+  gfc_derived_types = NULL;
   gfc_current_ns = gfc_global_ns_list;
   for (; gfc_current_ns; gfc_current_ns = gfc_current_ns->sibling)
     {
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 262545)
+++ gcc/fortran/resolve.c	(working copy)
@@ -2509,7 +2509,7 @@ resolve_global_procedure (gfc_symbol *sym, locus *
       /* Resolve the gsymbol namespace if needed.  */
       if (!gsym->ns->resolved)
 	{
-	  gfc_dt_list *old_dt_list;
+	  gfc_symbol *old_dt_list;
 
 	  /* Stash away derived types so that the backend_decls do not
 	     get mixed up.  */
@@ -13474,16 +13474,21 @@ resolve_typebound_procedures (gfc_symbol* derived)
 static void
 add_dt_to_dt_list (gfc_symbol *derived)
 {
-  gfc_dt_list *dt_list;
 
-  for (dt_list = gfc_derived_types; dt_list; dt_list = dt_list->next)
-    if (derived == dt_list->derived)
-      return;
-
-  dt_list = gfc_get_dt_list ();
-  dt_list->next = gfc_derived_types;
-  dt_list->derived = derived;
-  gfc_derived_types = dt_list;
+  if (!derived->dt_next)
+    {
+      if (gfc_derived_types)
+	{
+	  derived->dt_next = gfc_derived_types->dt_next;
+	  gfc_derived_types->dt_next = derived;
+	}
+      else
+	{
+	  derived->dt_next = derived;
+	}
+      gfc_derived_types = derived;
+    }
+  
 }
 
 
Index: gcc/fortran/symbol.c
===================================================================
--- gcc/fortran/symbol.c	(revision 262545)
+++ gcc/fortran/symbol.c	(working copy)
@@ -107,7 +107,7 @@ gfc_namespace *gfc_global_ns_list;
 
 gfc_gsymbol *gfc_gsym_root = NULL;
 
-gfc_dt_list *gfc_derived_types;
+gfc_symbol *gfc_derived_types;
 
 static gfc_undo_change_set default_undo_chgset_var = { vNULL, vNULL, NULL };
 static gfc_undo_change_set *latest_undo_chgset = &default_undo_chgset_var;
@@ -3137,6 +3137,7 @@ gfc_new_symbol (const char *name, gfc_namespace *n
   p->common_block = NULL;
   p->f2k_derived = NULL;
   p->assoc = NULL;
+  p->dt_next = NULL;
   p->fn_result_spec = 0;
 
   return p;
@@ -3896,23 +3897,6 @@ free_sym_tree (gfc_symtree *sym_tree)
 }
 
 
-/* Free the derived type list.  */
-
-void
-gfc_free_dt_list (void)
-{
-  gfc_dt_list *dt, *n;
-
-  for (dt = gfc_derived_types; dt; dt = n)
-    {
-      n = dt->next;
-      free (dt);
-    }
-
-  gfc_derived_types = NULL;
-}
-
-
 /* Free the gfc_equiv_info's.  */
 
 static void
@@ -4098,7 +4082,7 @@ gfc_symbol_done_2 (void)
       gfc_free_namespace (gfc_current_ns);
       gfc_current_ns = NULL;
     }
-  gfc_free_dt_list ();
+  gfc_derived_types = NULL;
 
   enforce_single_undo_checkpoint ();
   free_undo_change_set_data (*latest_undo_chgset);
@@ -4361,7 +4345,7 @@ gfc_get_gsymbol (const char *name)
 static gfc_symbol *
 get_iso_c_binding_dt (int sym_id)
 {
-  gfc_dt_list *dt_list;
+  gfc_symbol *dt_list;
 
   dt_list = gfc_derived_types;
 
@@ -4368,13 +4352,16 @@ get_iso_c_binding_dt (int sym_id)
   /* Loop through the derived types in the name list, searching for
      the desired symbol from iso_c_binding.  Search the parent namespaces
      if necessary and requested to (parent_flag).  */
-  while (dt_list != NULL)
+  if (dt_list)
     {
-      if (dt_list->derived->from_intmod != INTMOD_NONE
-	  && dt_list->derived->intmod_sym_id == sym_id)
-        return dt_list->derived;
-
-      dt_list = dt_list->next;
+      while (dt_list->dt_next != gfc_derived_types)
+	{
+	  if (dt_list->from_intmod != INTMOD_NONE
+	      && dt_list->intmod_sym_id == sym_id)
+	    return dt_list;
+	
+	  dt_list = dt_list->dt_next;
+	}
     }
 
   return NULL;
@@ -4780,11 +4767,16 @@ generate_isocbinding_symbol (const char *mod_name,
       if (tmp_sym->attr.flavor == FL_DERIVED
 	  && !get_iso_c_binding_dt (tmp_sym->intmod_sym_id))
 	{
-	  gfc_dt_list *dt_list;
-	  dt_list = gfc_get_dt_list ();
-	  dt_list->derived = tmp_sym;
-	  dt_list->next = gfc_derived_types;
-  	  gfc_derived_types = dt_list;
+	  if (gfc_derived_types)
+	    {
+	      tmp_sym->dt_next = gfc_derived_types->dt_next;
+	      gfc_derived_types->dt_next = tmp_sym;
+	    }
+	  else
+	    {
+	      tmp_sym->dt_next = tmp_sym;
+	    }
+	  gfc_derived_types = tmp_sym;
         }
 
       return tmp_symtree;
@@ -4892,7 +4884,6 @@ generate_isocbinding_symbol (const char *mod_name,
       case ISOCBINDING_FUNPTR:
 	{
 	  gfc_symbol *dt_sym;
-	  gfc_dt_list **dt_list_ptr = NULL;
 	  gfc_component *tmp_comp = NULL;
 
 	  /* Generate real derived type.  */
@@ -4954,18 +4945,17 @@ generate_isocbinding_symbol (const char *mod_name,
 	  dt_sym->ts.u.derived = dt_sym;
 
 	  /* Add the symbol created for the derived type to the current ns.  */
-	  dt_list_ptr = &(gfc_derived_types);
-	  while (*dt_list_ptr != NULL && (*dt_list_ptr)->next != NULL)
-	    dt_list_ptr = &((*dt_list_ptr)->next);
-
-	  /* There is already at least one derived type in the list, so append
-	     the one we're currently building for c_ptr or c_funptr.  */
-	  if (*dt_list_ptr != NULL)
-	    dt_list_ptr = &((*dt_list_ptr)->next);
-	  (*dt_list_ptr) = gfc_get_dt_list ();
-	  (*dt_list_ptr)->derived = dt_sym;
-	  (*dt_list_ptr)->next = NULL;
-
+	  if (gfc_derived_types)
+	    {
+	      dt_sym->dt_next = gfc_derived_types->dt_next;
+	      gfc_derived_types->dt_next = dt_sym;
+	    }
+	  else
+	    {
+	      dt_sym->dt_next = dt_sym;
+	    }
+	  gfc_derived_types = dt_sym;
+      	  
 	  gfc_add_component (dt_sym, "c_address", &tmp_comp);
 	  if (tmp_comp == NULL)
 	    gcc_unreachable ();
Index: gcc/fortran/trans-types.c
===================================================================
--- gcc/fortran/trans-types.c	(revision 262545)
+++ gcc/fortran/trans-types.c	(working copy)
@@ -2542,7 +2542,6 @@ gfc_get_derived_type (gfc_symbol * derived, int co
   bool got_canonical = false;
   bool unlimited_entity = false;
   gfc_component *c;
-  gfc_dt_list *dt;
   gfc_namespace *ns;
   tree tmp;
   bool coarray_flag;
@@ -2607,16 +2606,20 @@ gfc_get_derived_type (gfc_symbol * derived, int co
 	   ns->translated && !got_canonical;
 	   ns = ns->sibling)
 	{
-	  dt = ns->derived_types;
-	  for (; dt && !canonical; dt = dt->next)
+	  if (ns->derived_types)
 	    {
-	      gfc_copy_dt_decls_ifequal (dt->derived, derived, true);
-	      if (derived->backend_decl)
-		got_canonical = true;
-	    }
-	}
+	      for (gfc_symbol *dt = ns->derived_types; dt && !got_canonical; dt = dt->dt_next)
+		{
+		  gfc_copy_dt_decls_ifequal (dt, derived, true);
+		  if (derived->backend_decl)
+		    got_canonical = true;
+		  if (dt->dt_next == ns->derived_types)
+		    break;
+		}
+ 	    }
+ 	}
     }
-
+  
   /* Store up the canonical type to be added to this one.  */
   if (got_canonical)
     {
@@ -2874,10 +2877,14 @@ copy_derived_types:
 	  TREE_NO_WARNING (c->caf_token) = 1;
 	}
     }
-
-  for (dt = gfc_derived_types; dt; dt = dt->next)
-    gfc_copy_dt_decls_ifequal (derived, dt->derived, false);
-
+  
+  for (gfc_symbol *dt = gfc_derived_types; dt; dt = dt->dt_next)
+    {
+      gfc_copy_dt_decls_ifequal (derived, dt, false);
+      if (dt->dt_next == gfc_derived_types)
+	break;
+    }
+  
   return derived->backend_decl;
 }
 

[-- Attachment #3: ChangeLog --]
[-- Type: text/x-changelog, Size: 808 bytes --]

2018-05-31  Andrew Benson  <abenson@carnegiescience.edu>

	* gfortran.h: Add pointer to next derived type to
	gfc_symbol. Remove gfc_dt_list.
	* parse.c (resolve_all_program_units, resolve_global_procedure)
	(resolve_typebound_procedures): Replace gfc_free_dt_list() with
	simple nullification of gfc_derived_types. Change derived type
	linked list insertion to utilize dt_next pointers in gfc_symbol.
	* symbol.c (gfc_new_symbol, free_sym_tree, gfc_symbol_done2)
	(gfc_get_gsymbol, get_iso_c_binding_dt)
	(generate_isocbinding_symbol): Remove gfc_free_dt_list as
	gfc_dt_list is obsoleted. Change derived type linked list
	search/insertion to utilize dt_next pointers in gfc_symbol.
	* trans-types.c (gfc_get_derived_type): Change derived type linked
	list search to utilize dt_next pointers in gfc_symbol.

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

Pinging to see if anyone can take a look at this.

Thanks!

-Andrew

On Tuesday, July 10, 2018 7:32:58 PM PDT Andrew Benson wrote:
> I now finally have heard back from the FSF, so my paperwork is all signed
> and recorded.
> 
> I've tested my patch on the latest trunk - slightly updated version which
> applies cleanly to trunk is attached (along with the ChangeLog).
> 
> -Andrew
> 
> On Thursday, June 14, 2018 5:10:24 PM PDT Steve Kargl wrote:
> > Ping me when you hear back from FSF.  I'll apply your
> > current patch on the weekend to my tree and do some
> > testing.
> > 
> > > Steve,
> > > 
> > > I have just sent my copyright assignment documents back to the FSF, so I
> > > think it should be ok to submit my patch as soon as it has approval.
> > > 
> > > -Andrew
> > > 
> > > On Monday, June 11, 2018 11:17:10 AM PDT Steve Kargl wrote:
> > > > On Fri, Jun 01, 2018 at 08:14:04AM +0200, Janus Weil wrote:
> > > > > 2018-05-31 20:04 GMT+02:00 Andrew Benson
> 
> <abenson@carnegiescience.edu>:
> > > > > > One other question: for copyright assignment, who do I need to
> > > > > > talk to to get the relevant form(s)?
> > > > > 
> > > > > I think you need to send a request to assign@gnu.org and
> > > > > gcc@gcc.gnu.org in order to get the copyright assignment form. After
> > > > > the form is mailed to you, you sign it and send it back to the FSF.
> > > > 
> > > > Andrew, I see that you've asked on gcc@gcc on June 1 about
> > > > Copyright forms.  Has anyone responded?
> > > > 
> > > > I ask because my patch for PR fortran/68544 walks the
> > > > gfc_derived_types list.
> > > > 
> > > > https://gcc.gnu.org/ml/fortran/2018-06/msg00054.html
> > > > 
> > > > and Thomas has approved the patch.  In my patch, I
> > > > have
> > > > 
> > > > +static bool
> > > > +is_dt_name (const char *name)
> > > > +{
> > > > +  gfc_dt_list *dt_list;
> > > > +
> > > > +  for (dt_list = gfc_derived_types; dt_list; dt_list = dt_list->next)
> > > > +    if (strcmp(dt_list->derived->name, name) == 0)
> > > > +      return true;
> > > > +  return false;
> > > > +}
> > > > 
> > > > we'll need to update this to deal with your change for a
> > > > circular linked list.


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Janus Weil]

Hi Andrew,

2018-07-20 18:29 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
> Pinging to see if anyone can take a look at this.

I think your patch has received a reasonable amount of reviewing
already. You seem to have responded to all the points that came up,
and I personally I don't see any further ones.

I had even tested your patch a few weeks ago. It did not bring much
speedup for the codes I've tried it on (probably because the number of
derived types is significantly lower than in your case), but it also
did not show any negative side effects either.

So, I'd say the patch is ok for trunk!

Do you want me to commit it for you, or do you already have a
sourceware account for svn access (see
https://gcc.gnu.org/svnwrite.html#authenticated) and prefer to do it
yourself?

Cheers,
Janus



> On Tuesday, July 10, 2018 7:32:58 PM PDT Andrew Benson wrote:
>> I now finally have heard back from the FSF, so my paperwork is all signed
>> and recorded.
>>
>> I've tested my patch on the latest trunk - slightly updated version which
>> applies cleanly to trunk is attached (along with the ChangeLog).
>>
>> -Andrew
>>
>> On Thursday, June 14, 2018 5:10:24 PM PDT Steve Kargl wrote:
>> > Ping me when you hear back from FSF.  I'll apply your
>> > current patch on the weekend to my tree and do some
>> > testing.
>> >
>> > > Steve,
>> > >
>> > > I have just sent my copyright assignment documents back to the FSF, so I
>> > > think it should be ok to submit my patch as soon as it has approval.
>> > >
>> > > -Andrew
>> > >
>> > > On Monday, June 11, 2018 11:17:10 AM PDT Steve Kargl wrote:
>> > > > On Fri, Jun 01, 2018 at 08:14:04AM +0200, Janus Weil wrote:
>> > > > > 2018-05-31 20:04 GMT+02:00 Andrew Benson
>>
>> <abenson@carnegiescience.edu>:
>> > > > > > One other question: for copyright assignment, who do I need to
>> > > > > > talk to to get the relevant form(s)?
>> > > > >
>> > > > > I think you need to send a request to assign@gnu.org and
>> > > > > gcc@gcc.gnu.org in order to get the copyright assignment form. After
>> > > > > the form is mailed to you, you sign it and send it back to the FSF.
>> > > >
>> > > > Andrew, I see that you've asked on gcc@gcc on June 1 about
>> > > > Copyright forms.  Has anyone responded?
>> > > >
>> > > > I ask because my patch for PR fortran/68544 walks the
>> > > > gfc_derived_types list.
>> > > >
>> > > > https://gcc.gnu.org/ml/fortran/2018-06/msg00054.html
>> > > >
>> > > > and Thomas has approved the patch.  In my patch, I
>> > > > have
>> > > >
>> > > > +static bool
>> > > > +is_dt_name (const char *name)
>> > > > +{
>> > > > +  gfc_dt_list *dt_list;
>> > > > +
>> > > > +  for (dt_list = gfc_derived_types; dt_list; dt_list = dt_list->next)
>> > > > +    if (strcmp(dt_list->derived->name, name) == 0)
>> > > > +      return true;
>> > > > +  return false;
>> > > > +}
>> > > >
>> > > > we'll need to update this to deal with your change for a
>> > > > circular linked list.
>
>
> --
>
> * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html
>
> * Galacticus: https://bitbucket.org/abensonca/galacticus
>

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

Hi Janus,

Thanks!

I don't have a sourceware account yet - I should probably work on getting one. 
But, I'm happy for you to commit the patch if you're willing to do that.

Cheers,
Andrew

On Friday, July 20, 2018 8:59:38 PM PDT Janus Weil wrote:
> Hi Andrew,
> 
> 2018-07-20 18:29 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
> > Pinging to see if anyone can take a look at this.
> 
> I think your patch has received a reasonable amount of reviewing
> already. You seem to have responded to all the points that came up,
> and I personally I don't see any further ones.
> 
> I had even tested your patch a few weeks ago. It did not bring much
> speedup for the codes I've tried it on (probably because the number of
> derived types is significantly lower than in your case), but it also
> did not show any negative side effects either.
> 
> So, I'd say the patch is ok for trunk!
> 
> Do you want me to commit it for you, or do you already have a
> sourceware account for svn access (see
> https://gcc.gnu.org/svnwrite.html#authenticated) and prefer to do it
> yourself?
> 
> Cheers,
> Janus
> 
> > On Tuesday, July 10, 2018 7:32:58 PM PDT Andrew Benson wrote:
> >> I now finally have heard back from the FSF, so my paperwork is all signed
> >> and recorded.
> >> 
> >> I've tested my patch on the latest trunk - slightly updated version which
> >> applies cleanly to trunk is attached (along with the ChangeLog).
> >> 
> >> -Andrew
> >> 
> >> On Thursday, June 14, 2018 5:10:24 PM PDT Steve Kargl wrote:
> >> > Ping me when you hear back from FSF.  I'll apply your
> >> > current patch on the weekend to my tree and do some
> >> > testing.
> >> > 
> >> > > Steve,
> >> > > 
> >> > > I have just sent my copyright assignment documents back to the FSF,
> >> > > so I
> >> > > think it should be ok to submit my patch as soon as it has approval.
> >> > > 
> >> > > -Andrew
> >> > > 
> >> > > On Monday, June 11, 2018 11:17:10 AM PDT Steve Kargl wrote:
> >> > > > On Fri, Jun 01, 2018 at 08:14:04AM +0200, Janus Weil wrote:
> >> > > > > 2018-05-31 20:04 GMT+02:00 Andrew Benson
> >> 
> >> <abenson@carnegiescience.edu>:
> >> > > > > > One other question: for copyright assignment, who do I need to
> >> > > > > > talk to to get the relevant form(s)?
> >> > > > > 
> >> > > > > I think you need to send a request to assign@gnu.org and
> >> > > > > gcc@gcc.gnu.org in order to get the copyright assignment form.
> >> > > > > After
> >> > > > > the form is mailed to you, you sign it and send it back to the
> >> > > > > FSF.
> >> > > > 
> >> > > > Andrew, I see that you've asked on gcc@gcc on June 1 about
> >> > > > Copyright forms.  Has anyone responded?
> >> > > > 
> >> > > > I ask because my patch for PR fortran/68544 walks the
> >> > > > gfc_derived_types list.
> >> > > > 
> >> > > > https://gcc.gnu.org/ml/fortran/2018-06/msg00054.html
> >> > > > 
> >> > > > and Thomas has approved the patch.  In my patch, I
> >> > > > have
> >> > > > 
> >> > > > +static bool
> >> > > > +is_dt_name (const char *name)
> >> > > > +{
> >> > > > +  gfc_dt_list *dt_list;
> >> > > > +
> >> > > > +  for (dt_list = gfc_derived_types; dt_list; dt_list =
> >> > > > dt_list->next)
> >> > > > +    if (strcmp(dt_list->derived->name, name) == 0)
> >> > > > +      return true;
> >> > > > +  return false;
> >> > > > +}
> >> > > > 
> >> > > > we'll need to update this to deal with your change for a
> >> > > > circular linked list.
> > 
> > --
> > 
> > * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html
> > 
> > * Galacticus: https://bitbucket.org/abensonca/galacticus


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Janus Weil]

2018-07-20 21:02 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
> I don't have a sourceware account yet - I should probably work on getting one.

well, that's neither mandatory nor urgent, but if you plan to make
further contributions in the future, it definitely makes sense to get
one.


> But, I'm happy for you to commit the patch if you're willing to do that.

Sure, no problem. Should be able to get that done within the next hour ...

Cheers,
Janus



> On Friday, July 20, 2018 8:59:38 PM PDT Janus Weil wrote:
>> Hi Andrew,
>>
>> 2018-07-20 18:29 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
>> > Pinging to see if anyone can take a look at this.
>>
>> I think your patch has received a reasonable amount of reviewing
>> already. You seem to have responded to all the points that came up,
>> and I personally I don't see any further ones.
>>
>> I had even tested your patch a few weeks ago. It did not bring much
>> speedup for the codes I've tried it on (probably because the number of
>> derived types is significantly lower than in your case), but it also
>> did not show any negative side effects either.
>>
>> So, I'd say the patch is ok for trunk!
>>
>> Do you want me to commit it for you, or do you already have a
>> sourceware account for svn access (see
>> https://gcc.gnu.org/svnwrite.html#authenticated) and prefer to do it
>> yourself?
>>
>> Cheers,
>> Janus
>>
>> > On Tuesday, July 10, 2018 7:32:58 PM PDT Andrew Benson wrote:
>> >> I now finally have heard back from the FSF, so my paperwork is all signed
>> >> and recorded.
>> >>
>> >> I've tested my patch on the latest trunk - slightly updated version which
>> >> applies cleanly to trunk is attached (along with the ChangeLog).
>> >>
>> >> -Andrew
>> >>
>> >> On Thursday, June 14, 2018 5:10:24 PM PDT Steve Kargl wrote:
>> >> > Ping me when you hear back from FSF.  I'll apply your
>> >> > current patch on the weekend to my tree and do some
>> >> > testing.
>> >> >
>> >> > > Steve,
>> >> > >
>> >> > > I have just sent my copyright assignment documents back to the FSF,
>> >> > > so I
>> >> > > think it should be ok to submit my patch as soon as it has approval.
>> >> > >
>> >> > > -Andrew
>> >> > >
>> >> > > On Monday, June 11, 2018 11:17:10 AM PDT Steve Kargl wrote:
>> >> > > > On Fri, Jun 01, 2018 at 08:14:04AM +0200, Janus Weil wrote:
>> >> > > > > 2018-05-31 20:04 GMT+02:00 Andrew Benson
>> >>
>> >> <abenson@carnegiescience.edu>:
>> >> > > > > > One other question: for copyright assignment, who do I need to
>> >> > > > > > talk to to get the relevant form(s)?
>> >> > > > >
>> >> > > > > I think you need to send a request to assign@gnu.org and
>> >> > > > > gcc@gcc.gnu.org in order to get the copyright assignment form.
>> >> > > > > After
>> >> > > > > the form is mailed to you, you sign it and send it back to the
>> >> > > > > FSF.
>> >> > > >
>> >> > > > Andrew, I see that you've asked on gcc@gcc on June 1 about
>> >> > > > Copyright forms.  Has anyone responded?
>> >> > > >
>> >> > > > I ask because my patch for PR fortran/68544 walks the
>> >> > > > gfc_derived_types list.
>> >> > > >
>> >> > > > https://gcc.gnu.org/ml/fortran/2018-06/msg00054.html
>> >> > > >
>> >> > > > and Thomas has approved the patch.  In my patch, I
>> >> > > > have
>> >> > > >
>> >> > > > +static bool
>> >> > > > +is_dt_name (const char *name)
>> >> > > > +{
>> >> > > > +  gfc_dt_list *dt_list;
>> >> > > > +
>> >> > > > +  for (dt_list = gfc_derived_types; dt_list; dt_list =
>> >> > > > dt_list->next)
>> >> > > > +    if (strcmp(dt_list->derived->name, name) == 0)
>> >> > > > +      return true;
>> >> > > > +  return false;
>> >> > > > +}
>> >> > > >
>> >> > > > we'll need to update this to deal with your change for a
>> >> > > > circular linked list.
>> >
>> > --
>> >
>> > * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html
>> >
>> > * Galacticus: https://bitbucket.org/abensonca/galacticus
>
>
> --
>
> * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html
>
> * Galacticus: https://bitbucket.org/abensonca/galacticus
>

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Janus Weil]

2018-07-20 21:10 GMT+02:00 Janus Weil <janus@gcc.gnu.org>:
> 2018-07-20 21:02 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
>> But, I'm happy for you to commit the patch if you're willing to do that.
>
> Sure, no problem. Should be able to get that done within the next hour ...

Ok, I have made some small last-minute adjustments (fixed an overlong
line and some whitespace issues and brushed up the ChangeLog a bit)
and committed the patch as r262909:

https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=262909

Not much left to say, except: Congratulations to your first GCC
contribution, welcome to the gfortran team and thanks for the nice
work! We hope there is more of that to come :)

Cheers,
Janus



>> On Friday, July 20, 2018 8:59:38 PM PDT Janus Weil wrote:
>>> Hi Andrew,
>>>
>>> 2018-07-20 18:29 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
>>> > Pinging to see if anyone can take a look at this.
>>>
>>> I think your patch has received a reasonable amount of reviewing
>>> already. You seem to have responded to all the points that came up,
>>> and I personally I don't see any further ones.
>>>
>>> I had even tested your patch a few weeks ago. It did not bring much
>>> speedup for the codes I've tried it on (probably because the number of
>>> derived types is significantly lower than in your case), but it also
>>> did not show any negative side effects either.
>>>
>>> So, I'd say the patch is ok for trunk!
>>>
>>> Do you want me to commit it for you, or do you already have a
>>> sourceware account for svn access (see
>>> https://gcc.gnu.org/svnwrite.html#authenticated) and prefer to do it
>>> yourself?
>>>
>>> Cheers,
>>> Janus
>>>
>>> > On Tuesday, July 10, 2018 7:32:58 PM PDT Andrew Benson wrote:
>>> >> I now finally have heard back from the FSF, so my paperwork is all signed
>>> >> and recorded.
>>> >>
>>> >> I've tested my patch on the latest trunk - slightly updated version which
>>> >> applies cleanly to trunk is attached (along with the ChangeLog).
>>> >>
>>> >> -Andrew
>>> >>
>>> >> On Thursday, June 14, 2018 5:10:24 PM PDT Steve Kargl wrote:
>>> >> > Ping me when you hear back from FSF.  I'll apply your
>>> >> > current patch on the weekend to my tree and do some
>>> >> > testing.
>>> >> >
>>> >> > > Steve,
>>> >> > >
>>> >> > > I have just sent my copyright assignment documents back to the FSF,
>>> >> > > so I
>>> >> > > think it should be ok to submit my patch as soon as it has approval.
>>> >> > >
>>> >> > > -Andrew
>>> >> > >
>>> >> > > On Monday, June 11, 2018 11:17:10 AM PDT Steve Kargl wrote:
>>> >> > > > On Fri, Jun 01, 2018 at 08:14:04AM +0200, Janus Weil wrote:
>>> >> > > > > 2018-05-31 20:04 GMT+02:00 Andrew Benson
>>> >>
>>> >> <abenson@carnegiescience.edu>:
>>> >> > > > > > One other question: for copyright assignment, who do I need to
>>> >> > > > > > talk to to get the relevant form(s)?
>>> >> > > > >
>>> >> > > > > I think you need to send a request to assign@gnu.org and
>>> >> > > > > gcc@gcc.gnu.org in order to get the copyright assignment form.
>>> >> > > > > After
>>> >> > > > > the form is mailed to you, you sign it and send it back to the
>>> >> > > > > FSF.
>>> >> > > >
>>> >> > > > Andrew, I see that you've asked on gcc@gcc on June 1 about
>>> >> > > > Copyright forms.  Has anyone responded?
>>> >> > > >
>>> >> > > > I ask because my patch for PR fortran/68544 walks the
>>> >> > > > gfc_derived_types list.
>>> >> > > >
>>> >> > > > https://gcc.gnu.org/ml/fortran/2018-06/msg00054.html
>>> >> > > >
>>> >> > > > and Thomas has approved the patch.  In my patch, I
>>> >> > > > have
>>> >> > > >
>>> >> > > > +static bool
>>> >> > > > +is_dt_name (const char *name)
>>> >> > > > +{
>>> >> > > > +  gfc_dt_list *dt_list;
>>> >> > > > +
>>> >> > > > +  for (dt_list = gfc_derived_types; dt_list; dt_list =
>>> >> > > > dt_list->next)
>>> >> > > > +    if (strcmp(dt_list->derived->name, name) == 0)
>>> >> > > > +      return true;
>>> >> > > > +  return false;
>>> >> > > > +}
>>> >> > > >
>>> >> > > > we'll need to update this to deal with your change for a
>>> >> > > > circular linked list.
>>> >
>>> > --
>>> >
>>> > * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html
>>> >
>>> > * Galacticus: https://bitbucket.org/abensonca/galacticus
>>
>>
>> --
>>
>> * Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html
>>
>> * Galacticus: https://bitbucket.org/abensonca/galacticus
>>

Re: Optimization of add_dt_to_dt_list() in resolve.c

[Andrew Benson]

Thanks! I have a few other areas I've identified that look promising for 
similar optimizations, so I'm hoping to look into those soon.

Cheers,
Andrew

On Friday, July 20, 2018 10:04:47 PM PDT Janus Weil wrote:
> 2018-07-20 21:10 GMT+02:00 Janus Weil <janus@gcc.gnu.org>:
> > 2018-07-20 21:02 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
> >> But, I'm happy for you to commit the patch if you're willing to do that.
> > 
> > Sure, no problem. Should be able to get that done within the next hour ...
> 
> Ok, I have made some small last-minute adjustments (fixed an overlong
> line and some whitespace issues and brushed up the ChangeLog a bit)
> and committed the patch as r262909:
> 
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=262909
> 
> Not much left to say, except: Congratulations to your first GCC
> contribution, welcome to the gfortran team and thanks for the nice
> work! We hope there is more of that to come :)
> 
> Cheers,
> Janus
> 
> >> On Friday, July 20, 2018 8:59:38 PM PDT Janus Weil wrote:
> >>> Hi Andrew,
> >>> 
> >>> 2018-07-20 18:29 GMT+02:00 Andrew Benson <abenson@carnegiescience.edu>:
> >>> > Pinging to see if anyone can take a look at this.
> >>> 
> >>> I think your patch has received a reasonable amount of reviewing
> >>> already. You seem to have responded to all the points that came up,
> >>> and I personally I don't see any further ones.
> >>> 
> >>> I had even tested your patch a few weeks ago. It did not bring much
> >>> speedup for the codes I've tried it on (probably because the number of
> >>> derived types is significantly lower than in your case), but it also
> >>> did not show any negative side effects either.
> >>> 
> >>> So, I'd say the patch is ok for trunk!
> >>> 
> >>> Do you want me to commit it for you, or do you already have a
> >>> sourceware account for svn access (see
> >>> https://gcc.gnu.org/svnwrite.html#authenticated) and prefer to do it
> >>> yourself?
> >>> 
> >>> Cheers,
> >>> Janus
> >>> 
> >>> > On Tuesday, July 10, 2018 7:32:58 PM PDT Andrew Benson wrote:
> >>> >> I now finally have heard back from the FSF, so my paperwork is all
> >>> >> signed
> >>> >> and recorded.
> >>> >> 
> >>> >> I've tested my patch on the latest trunk - slightly updated version
> >>> >> which
> >>> >> applies cleanly to trunk is attached (along with the ChangeLog).
> >>> >> 
> >>> >> -Andrew
> >>> >> 
> >>> >> On Thursday, June 14, 2018 5:10:24 PM PDT Steve Kargl wrote:
> >>> >> > Ping me when you hear back from FSF.  I'll apply your
> >>> >> > current patch on the weekend to my tree and do some
> >>> >> > testing.
> >>> >> > 
> >>> >> > > Steve,
> >>> >> > > 
> >>> >> > > I have just sent my copyright assignment documents back to the
> >>> >> > > FSF,
> >>> >> > > so I
> >>> >> > > think it should be ok to submit my patch as soon as it has
> >>> >> > > approval.
> >>> >> > > 
> >>> >> > > -Andrew
> >>> >> > > 
> >>> >> > > On Monday, June 11, 2018 11:17:10 AM PDT Steve Kargl wrote:
> >>> >> > > > On Fri, Jun 01, 2018 at 08:14:04AM +0200, Janus Weil wrote:
> >>> >> > > > > 2018-05-31 20:04 GMT+02:00 Andrew Benson
> >>> >> 
> >>> >> <abenson@carnegiescience.edu>:
> >>> >> > > > > > One other question: for copyright assignment, who do I need
> >>> >> > > > > > to
> >>> >> > > > > > talk to to get the relevant form(s)?
> >>> >> > > > > 
> >>> >> > > > > I think you need to send a request to assign@gnu.org and
> >>> >> > > > > gcc@gcc.gnu.org in order to get the copyright assignment
> >>> >> > > > > form.
> >>> >> > > > > After
> >>> >> > > > > the form is mailed to you, you sign it and send it back to
> >>> >> > > > > the
> >>> >> > > > > FSF.
> >>> >> > > > 
> >>> >> > > > Andrew, I see that you've asked on gcc@gcc on June 1 about
> >>> >> > > > Copyright forms.  Has anyone responded?
> >>> >> > > > 
> >>> >> > > > I ask because my patch for PR fortran/68544 walks the
> >>> >> > > > gfc_derived_types list.
> >>> >> > > > 
> >>> >> > > > https://gcc.gnu.org/ml/fortran/2018-06/msg00054.html
> >>> >> > > > 
> >>> >> > > > and Thomas has approved the patch.  In my patch, I
> >>> >> > > > have
> >>> >> > > > 
> >>> >> > > > +static bool
> >>> >> > > > +is_dt_name (const char *name)
> >>> >> > > > +{
> >>> >> > > > +  gfc_dt_list *dt_list;
> >>> >> > > > +
> >>> >> > > > +  for (dt_list = gfc_derived_types; dt_list; dt_list =
> >>> >> > > > dt_list->next)
> >>> >> > > > +    if (strcmp(dt_list->derived->name, name) == 0)
> >>> >> > > > +      return true;
> >>> >> > > > +  return false;
> >>> >> > > > +}
> >>> >> > > > 
> >>> >> > > > we'll need to update this to deal with your change for a
> >>> >> > > > circular linked list.
> >>> > 
> >>> > --
> >>> > 
> >>> > * Andrew Benson:
> >>> > http://users.obs.carnegiescience.edu/abenson/contact.html
> >>> > 
> >>> > * Galacticus: https://bitbucket.org/abensonca/galacticus
> >> 
> >> --
> >> 
> >> * Andrew Benson:
> >> http://users.obs.carnegiescience.edu/abenson/contact.html
> >> 
> >> * Galacticus: https://bitbucket.org/abensonca/galacticus


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://bitbucket.org/abensonca/galacticus