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


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

Re: [RFC, PATCH 1/n] IPA C++ refactoring


> I tried to mark it as protected, by there's usage that blocks that:
> 
> In file included from ../../gcc/symtab.c:40:0:
> ../../gcc/cgraph.h: In member function ?void symtab_node::unregister()?:
> ../../gcc/cgraph.h:1178:16: error: ?cgraph_node* cgraph_node::find_replacement()? is protected
>    cgraph_node *find_replacement (void);
>                 ^
> ../../gcc/symtab.c:462:46: error: within this context
>   replacement_node = cnode->find_replacement ();

OK, lets keep it public for now then.

> >Likewise
> create_version_clone_with_body calls: new_version_node->call_function_insertion_hooks ();

OK.
> 
> Usages from outside of cgraph_node:
> 
> cgraph.c:cgraph_node::create_empty (void)
> cgraph.c:  struct cgraph_node *node = cgraph_node::create_empty ();
> cgraphclones.c:  struct cgraph_node *new_node = cgraph_node::create_empty ();
> lto-cgraph.c:      node = cgraph_node::create_empty ();
> 
> I will go through class members and check grouping one more.
> What do  you think about 'static' class functions, should be placed after all member functions?

Sounds fine for me.

Honza
> 
> Thanks,
> Martin
> >+
> >+  /* Try to find a call graph node for declaration DECL and if it does not
> >+     exist or if it corresponds to an inline clone, create a new one.  */
> >+  static cgraph_node * get_create (tree);
> >
> >Also to begginig, next to get and create.
> >+
> >+  /* Return the cgraph node that has ASMNAME for its DECL_ASSEMBLER_NAME.
> >+     Return NULL if there's no such node.  */
> >+  static cgraph_node *get_for_asmname (tree asmname);
> >
> >Likewise
> >+
> >+  /* Attempt to mark ALIAS as an alias to DECL.  Return alias node if
> >+     successful and NULL otherwise.
> >+     Same body aliases are output whenever the body of DECL is output,
> >+     and cgraph_node::get (ALIAS) transparently
> >+     returns cgraph_node::get (DECL).  */
> >+  static cgraph_node * create_same_body_alias (tree alias, tree decl);
> >
> >To alias API
> >+
> >+  /* Worker for cgraph_can_remove_if_no_direct_calls_p.  */
> >+  static bool used_from_object_file_p_worker (cgraph_node *node,
> >+					      void *data ATTRIBUTE_UNUSED)
> >+  {
> >+    return node->used_from_object_file_p ();
> >+  }
> >+
> >+  /* Return true when cgraph_node can not be local.
> >+     Worker for cgraph_local_node_p.  */
> >cgraph_local_node_p was probably renamed, but we should sanify predicates here.
> >Please group all functions dealing with local functions togehter, too.
> >+  static bool non_local_p (cgraph_node *node, void *data ATTRIBUTE_UNUSED);
> >+
> >+  /* Verify whole cgraph structure.  */
> >+  static void DEBUG_FUNCTION verify_cgraph_nodes (void);
> >+
> >+  /* Worker to bring NODE local.  */
> >+  static bool make_local (cgraph_node *node, void *data ATTRIBUTE_UNUSED);
> >+
> >+  /* Mark ALIAS as an alias to DECL.  DECL_NODE is cgraph node representing
> >+     the function body is associated
> >+     with (not necessarily cgraph_node (DECL).  */
> >+  static cgraph_node *create_alias (tree alias, tree target);
> >+
> >+  static cgraph_edge * create_edge (cgraph_node *caller, cgraph_node *callee,
> >+				    gimple call_stmt, gcov_type count,
> >+				    int freq,
> >+				    bool indir_unknown_callee);
> >
> >Also edges and aliases should be grouped.
> >
> >With these changes patch looks good. Probably we will need one additional cleanup pass, but it would be better
> >to do it incrementally.  Please write changelog that carefuly records what was renamed to what.
> >
> >Honza


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