This is the mail archive of the binutils@sourceware.org mailing list for the binutils 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: [patch][gold] Remove empty Merge_map class


> 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


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