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: [gold patch] incremental 5/18: support --start-lib/--end-lib


Cary Coutant <ccoutant@google.com> writes:

> I've got a series of 14 more patches lined up now for incremental
> linking support, including the four that are still pending from last
> September. I've updated the pending patches a bit and rebased them to
> the current head. I'll send them to you a few at a time.

Just checking--are you going to resend the patches from last September?
(I think there are only three, as one was approved.)

> 2011-03-28 Cary Coutant  <ccoutant@google.com>
>
> 	* archive.cc (Library_base::should_include_member): Move
> 	method here from class Archive.
> 	(Archive::Archive): Initialize base class.
> 	(Archive::should_include_member): Move to base class.
> 	(Archive::do_for_all_unused_symbols): New function.
> 	(Add_archive_symbols::run): Remove redundant access to
> 	incremental_inputs.
> 	(Lib_group::Lib_group): Initialize base class.
> 	(Lib_group::do_filename): New function.
> 	(Lib_group::include_member): Pass pointer to Lib_group to
> 	report_object.
> 	(Lib_group::do_for_all_unused_symbols): New function.
> 	(Add_lib_group_symbols::run): Report archive information for
> 	incremental links.
> 	* archive.h (class Library_base): New base class.
> 	(class Archive): Derive from Library_base.
> 	(Archive::filename): Move to base class.
> 	(Archive::set_incremental_info): Likewise.
> 	(Archive::incremental_info): Likewise.
> 	(Archive::Should_include): Likewise.
> 	(Archive::should_include_member): Likewise.
> 	(Archive::Armap_entry): Remove.
> 	(Archive::Unused_symbol_iterator): Remove.
> 	(Archive::unused_symbols_begin): Remove.
> 	(Archive::unused_symbols_end): Remove.
> 	(Archive::do_filename): New function.
> 	(Archive::do_get_mtime): New function.
> 	(Archive::do_for_all_unused_symbols): New function.
> 	(Archive::task_): Move to base class.
> 	(Archive::incremental_info_): Likewise.
> 	(class Lib_group): Derive from Library_base.
> 	(Lib_group::do_filename): New function.
> 	(Lib_group::do_get_mtime): New function.
> 	(Lib_group::do_for_all_unused_symbols): New function.
> 	(Lib_group::task_): Move to base class.
> 	* dynobj.cc (Sized_dynobj::do_for_all_global_symbols): New
> 	function.
> 	* dynobj.h (Sized_dynobj::do_for_all_global_symbols): New
> 	function.
> 	* incremental.cc (Incremental_inputs::report_archive_begin):
> 	Use Library_base; call library's get_mtime; add incremental inputs
> 	entry before members.
> 	(class Unused_symbol_visitor): New class.
> 	(Incremental_inputs::report_archive_end): Use Library_base; use
> 	visitor class to record unused symbols; don't add incremental inputs
> 	entry after members.
> 	(Incremental_inputs::report_object): Use Library_base.
> 	* incremental.h
> 	(Incremental_archive_entry::Incremental_archive_entry): Remove
> 	unused Archive parameter.
> 	(Incremental_inputs::report_archive_begin): Use Library_base.
> 	(Incremental_inputs::report_archive_end): Likewise.
> 	(Incremental_inputs::report_object): Likewise.
> 	* object.cc (Sized_relobj::do_for_all_global_symbols): New
> 	function.
> 	* object.h (Object::for_all_global_symbols): New function.
> 	(Object::do_for_all_global_symbols): New function.
> 	(Sized_relobj::do_for_all_global_symbols): New function.
> 	* plugin.cc (Sized_pluginobj::do_for_all_global_symbols):  New
> 	function.
> 	* plugin.h (Sized_pluginobj::do_for_all_global_symbols):  New
> 	function.

> +// Iterate over all unused symbols, and call the visitor class V for each.
> +
> +void
> +Archive::do_for_all_unused_symbols(Symbol_visitor_base& v) const
> +{
> +  for (std::vector<Armap_entry>::const_iterator p = this->armap_.begin();
> +       p != this->armap_.end();
> +       ++p)
> +    {
> +      if (this->seen_offsets_.find(p->file_offset)
> +          == this->seen_offsets_.end())
> +        v.visit(this->armap_names_.data() + p->name_offset);
> +    }
> +}

In gold we should avoid non-const references.  Make v a pointer instead.

> +  // Abstract base class for processing unused symbols.
> +  class Symbol_visitor_base
> +  {
> +   public:
> +    Symbol_visitor_base()
> +    { }
> +
> +    virtual
> +    ~Symbol_visitor_base()
> +    { }
> +
> +    virtual void
> +    visit(const char*) = 0;
> +  };

The visit member function should have a comment saying that the
parameter is.

> +  // Iterator for unused global symbols in the library.
> +  void
> +  for_all_unused_symbols(Symbol_visitor_base& v) const
> +  { this->do_for_all_unused_symbols(v); }

Clarify: unused symbol definitions or unused symbol references?

> +// Iterate over global symbols, calling a visitor class V for each.

s/global symbol/global defined symbols/

> +// Visitor class for processing the unused global symbols in a library.
> +
> +class Unused_symbol_visitor : public Library_base::Symbol_visitor_base
> +{

Expand comment: what sort of processing?

> diff --git a/gold/object.cc b/gold/object.cc
> index e2c0113..954961a 100644
> --- a/gold/object.cc
> +++ b/gold/object.cc
> @@ -1691,6 +1691,31 @@ Sized_relobj<size, big_endian>::do_should_include_member(Symbol_table* symtab,
>    return Archive::SHOULD_INCLUDE_UNKNOWN;
>  }
>  
> +// Iterate over global symbols, calling a visitor class V for each.
> +
> +template<int size, bool big_endian>
> +void
> +Sized_relobj<size, big_endian>::do_for_all_global_symbols(
> +    Read_symbols_data* sd,
> +    Library_base::Symbol_visitor_base& v)
> +{

s/global symbols/global defined symbols/


This is OK with those changes.

Thanks.

Ian


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