On 11/12/19 11:28 PM, Strager Neds wrote: > Sometimes, we need to copy a section name from one decl or symtab node > to another. Currently, this is done by getting the source's section > name and setting the destination's section name. For example: > > set_decl_section_name (dest, DECL_SECTION_NAME (source)); > dest->set_section (source->get_section ()); > > This code could be more efficient. Section names are stored in an > interning hash table, but the current interfaces of > set_decl_section_name and symtab_node::set_section force unnecessary > indirections (to get the section name) and hashing (to find the section > name in the hash table). > > Overload set_decl_section_name and symtab_node::set_section to accept an > existing symtab_node to copy the section name from: > > set_decl_section_name (dest, source); > dest->set_section (*source); > > For now, implement these new functions as a simple wrapper around the > existing functions. In the future, these functions can be implemented > using just a pointer copy and an increment (for the reference count). > > This patch should not change behavior. > > Testing: Bootstrap on x86_64-linux-gnu with --disable-multilib > --enable-checking=release --enable-languages=c,c++. Observe no change in > test results. > > 2019-11-12 Matthew Glazar <strager....@gmail.com> > > * gcc/cgraph.h (symtab_node::get_section): Constify. > (symtab_node::set_section): Declare new overload. > * gcc/symtab.c (symtab_node::set_section): Define new overload. > (symtab_node::copy_visibility_from): Use new overload of > symtab_node::set_section. > (symtab_node::resolve_alias): Same. > * gcc/tree.h (set_decl_section_name): Declare new overload. > * gcc/tree.c (set_decl_section_name): Define new overload. > * gcc/c/c-decl.c (merge_decls): Use new overload of > set_decl_section_name. > * gcc/cp/decl.c (duplicate_decls): Same. > * gcc/cp/method.c (use_thunk): Same. > * gcc/cp/optimize.c (maybe_clone_body): Same. > * gcc/d/decl.cc (finish_thunk): Same. > * gcc/tree-emutls.c (get_emutls_init_templ_addr): Same. > * gcc/cgraphclones.c (cgraph_node::create_virtual_clone): Use new > overload of symtab_node::set_section. > (cgraph_node::create_version_clone_with_body): Same. > * gcc/trans-mem.c (ipa_tm_create_version): Same.
So only a year after the original post.... Presumably given that you're just adding an overload, there is no need or intent to change all the callers to set_decl_section_name. Right? I scanned the various remaining calls, some just use a string, or build up a string based on a decl or something else. I did find one other call that should be converted in cp/coroutines.cc which almost certainly went onto the trunk after your patch was submitted. I've adjusted that one. Also note, your mailer seemed to mangle whitespace in your message. It makes it slightly harder to deal with, so if you plan on making regular contributions, we should probably work to get that fixed. If the refactoring + subsequent bugfixes are all you're planning to do, then I'll fix the whitespace issues manually. Given the age of the patch, the need to manually apply bits (due to whitespace issues) and the desire to fix coroutines.cc, I'm giving it a fresh test spin. Jeff
