Re: [LTO][PATCH] splitting lto annotations and functions for LGEN

[Diego Novillo <dnovillo at google dot com>]

Doug Kwan (éæå) wrote:

> +}
> +
> +/* Add cgraph_node NODE from cgraph_node_set SET. */

s/from/to/

> +
> +void
> +cgraph_node_set_add (cgraph_node_set set, struct cgraph_node *node)
> +{
> +  void **slot;
> +  struct cgraph_node_set_element_def temp;
> +  cgraph_node_set_element element;
> +
> +  temp.node = node;
> +  slot = htab_find_slot (set->hashtab, &temp, INSERT);
> +
> +  if (*slot != HTAB_EMPTY_ENTRY)
> +    {
> +#ifdef ENABLE_CHECKING
> +      element = (cgraph_node_set_element) *slot;
> +      gcc_assert (element->node == node);
> +#endif
> +      return;
> +    }
> +
> +  element =
> +    (cgraph_node_set_element) GGC_NEW (struct cgraph_node_set_element_def);
> +  element->node = node;
> +
> +  /* Insert into node list. */
> +  element->next = NULL;

How about making this a VEC to simplify handling?  This is used for 
sequential traversals and the hash table for quick lookups, right?  Each 
cgraph_node_set_element would only need to have its associated index 
into this VEC.

> @@ -359,6 +398,69 @@ bool cgraph_is_master_clone (struct cgra
>  struct cgraph_node *cgraph_master_clone (struct cgraph_node *, bool);
>  void cgraph_add_new_function (tree, bool);
>  
> +cgraph_node_set cgraph_node_set_new (void);
> +cgraph_node_set_iterator cgraph_node_set_find (cgraph_node_set set,
> +					       struct cgraph_node *node);
> +void cgraph_node_set_add (cgraph_node_set, struct cgraph_node *);
> +void cgraph_node_set_remove (cgraph_node_set, struct cgraph_node *);
> +void dump_cgraph_node_set (FILE *f, cgraph_node_set);
> +void debug_cgraph_node_set (cgraph_node_set);
> +
> +/* Return ture if interator CPI points to nothing. */

s/ture/true/
s/interator/iterator/

> +
> +static inline void
> +csi_next (cgraph_node_set_iterator *csi)
> +{
> +  if (csi->element)
> +    csi->element = csi->element->next;
> +}
> +
> +/* Return the node pointed to by CPI. */

s/CPI/CSI/

> +
> +static inline struct cgraph_node *
> +csi_node (cgraph_node_set_iterator csi)
> +{
> +  return csi.element->node;
> +}
> +
> +/* Return an iterator to the first node in SET */

End comment with '.  */'

>  
>  
> +/* Return true if we need to write summary of NODE. */
> +static bool

Blank line after comment.

>  
> +#ifdef ENABLE_CHECKING
> +/* Just a little sanity check to keep Honza honest.  At the

s/Honza/the cgraph machinery/

> +   point where we stream out the functions there must only be
> +   master_clone nodes or nodes that have no function bodies.  */
> +static void

Blank line after comment.

> @@ -525,7 +663,27 @@ lto_main (int debug_p ATTRIBUTE_UNUSED)
>    /* Let the middle end know that we have read and merged all of the
>       input files.  */ 
>    /*cgraph_finalize_compilation_unit ();*/
> -  cgraph_optimize ();
> +  if (!flag_wpa)
> +    cgraph_optimize ();
> +  else
> +    {
> +      /* FIXME-lto: Hack. We should use the IPA passes. */

What is involved here?  We should probably get this into our TODO list 
if it's too convoluted.

> @@ -536,4 +694,14 @@ lto_main (int debug_p ATTRIBUTE_UNUSED)
>        file = lto_set_current_out_file (NULL);
>        lto_elf_file_close (file);
>      }
> +
> +  if (flag_wpa)
> +    {
> +      lto_wpa_write_files ();
> +    }
> +
> +  bitmap_obstack_release (&lto_bitmap_obstack);
>  }
> +
> +#include "ggc.h"

This is already included at the top.


> +fwpa
> +Common Report Var(flag_wpa) Init(0) Optimization
> +Run LTO in WPA mode
> +

Also need an entry in doc/invoke.texi.

Looks OK otherwise.  It certainly needs some more work, but it's better 
to add it as a WIP now and refine it later.


Diego.