This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [patch][gold] Remove empty Merge_map class
- From: Cary Coutant <ccoutant at google dot com>
- To: Rafael EspÃndola <rafael dot espindola at gmail dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Mon, 2 Mar 2015 11:08:54 -0800
- Subject: Re: [patch][gold] Remove empty Merge_map class
- Authentication-results: sourceware.org; auth=none
- References: <CAG3jReJM-x=kpdPfb=KswWrZ+hRa3AiaN4Tv_MjqFi7rqxd58Q at mail dot gmail dot com> <CAG3jReLo1fAieCU-5wNVFUrvRy5-AujvBm+96eDQLBJ2n5mSLA at mail dot gmail dot com> <CAG3jReJpJOgFvG5BAWE0NMDsacNn+hL-4D20wWNKMP9Ctgys5w at mail dot gmail dot com> <CAG3jReK2DXyw1pS8nwQbBQh-=Catr=xTt2MZr=0coFbFGkoqEw at mail dot gmail dot com>
> Ping. Lets focus on just this patch first. It is just deleting dead
> code. To see it, noticed that:
>
> * Merge_map is empty, so its methods (add_mapping, get_output_offset,
> is_merge_section_for) don't use "this" and are really just free
> standing functions. In this patch they are converted 1:1 into Relobj
> methods since they all currently take a Relobj* as the first argument.
>
> * The only use of a pointer to a Merge_map is in the comparison
> "map->merge_map == merge_map" in
> Object_merge_map::is_merge_section_for, which indicates that Merge_map
> is now really just an ID.
>
> * Merge_map is never allocated on its own, just as a member variable
> of Eh_frame and Output_merge_base. Given that, we can just use a
> pointer to Eh_frame/Output_merge_base and save the member. The common
> base class of Eh_frame and Output_merge_base is Output_section_data,
> so this patch s/Merge_map/Output_section_data/.
Thanks. This looks good, but let's take it one step further. The only
exposure that Relobj::merge_map_ has outside the class is in reloc.cc:
template<int size>
void
Merged_symbol_value<size>::initialize_input_to_output_map(
const Relobj* object,
unsigned int input_shndx)
{
Object_merge_map* map = object->merge_map();
map->initialize_input_to_output_map<size>(input_shndx,
this->output_start_address_,
&this->output_addresses_);
}
// Get the output value corresponding to an input offset if we
// couldn't find it in the hash table.
template<int size>
typename elfcpp::Elf_types<size>::Elf_Addr
Merged_symbol_value<size>::value_from_output_section(
const Relobj* object,
unsigned int input_shndx,
typename elfcpp::Elf_types<size>::Elf_Addr input_offset) const
{
section_offset_type output_offset;
bool found = object->merge_map()->get_output_offset(input_shndx,
input_offset,
&output_offset);
...
In the first case, let's just add an initialize_input_to_output_map()
method to Relobj, and call it instead of first getting the merge_map.
In the second case, you can now call object->merge_output_offset().
Now you can remove Relobj::merge_map(), and you can also remove
Relobj::set_merge_map(), instead just setting merge_map_ directly in
Relobj::add_merge_mapping().
Also, please write a ChangeLog entry.
(Sorry I've been so slow.)
-cary