gold patch committed: Implement --sort-common

[Ian Lance Taylor <iant at google dot com>]

PR 10931 points out that gold does not implement the --sort-common
option.  Fixed with this patch.  Committed to mainline.

Ian


2009-12-30  Ian Lance Taylor  <iant@google.com>

	PR 10931
	* options.h (class General_options): Add --sort-common option.
	* symtab.h (class Symbol_table): Define Sort_commons_order enum.
	* common.cc (Sort_common): Add sort_order parameter to
	constructor.  Add sort_order_ field.
	(Sort_commons::operator): Check sort_order_.
	(Symbol_table::allocate_commons): Determine the sort order.
	(Symbol_table::do_allocate_commons): Add sort_order parameter.
	Change all callers.
	(Symbol_table::do_allocate_commons_list): Likewise.


Index: common.cc
===================================================================
RCS file: /cvs/src/src/gold/common.cc,v
retrieving revision 1.20
diff -p -u -r1.20 common.cc
--- common.cc	30 Dec 2009 06:57:17 -0000	1.20
+++ common.cc	31 Dec 2009 01:55:07 -0000
@@ -64,21 +64,26 @@ Allocate_commons_task::run(Workqueue*)
   this->symtab_->allocate_commons(this->layout_, this->mapfile_);
 }
 
-// This class is used to sort the common symbol by size.  We put the
-// larger common symbols first.
+// This class is used to sort the common symbol.  We normally put the
+// larger common symbols first.  This can be changed by using
+// --sort-commons, which tells the linker to sort by alignment.
 
 template<int size>
 class Sort_commons
 {
  public:
-  Sort_commons(const Symbol_table* symtab)
-    : symtab_(symtab)
+  Sort_commons(const Symbol_table* symtab,
+	       Symbol_table::Sort_commons_order sort_order)
+    : symtab_(symtab), sort_order_(sort_order)
   { }
 
   bool operator()(const Symbol* a, const Symbol* b) const;
 
  private:
+  // The symbol table.
   const Symbol_table* symtab_;
+  // How to sort.
+  Symbol_table::Sort_commons_order sort_order_;
 };
 
 template<int size>
@@ -94,22 +99,48 @@ Sort_commons<size>::operator()(const Sym
   const Sized_symbol<size>* psa = symtab->get_sized_symbol<size>(pa);
   const Sized_symbol<size>* psb = symtab->get_sized_symbol<size>(pb);
 
-  // Sort by largest size first.
+  // The size.
   typename Sized_symbol<size>::Size_type sa = psa->symsize();
   typename Sized_symbol<size>::Size_type sb = psb->symsize();
+
+  // The alignment.
+  typename Sized_symbol<size>::Value_type aa = psa->value();
+  typename Sized_symbol<size>::Value_type ab = psb->value();
+
+  if (this->sort_order_ == Symbol_table::SORT_COMMONS_BY_ALIGNMENT_DESCENDING)
+    {
+      if (aa < ab)
+	return false;
+      else if (ab < aa)
+	return true;
+    }
+  else if (this->sort_order_
+	   == Symbol_table::SORT_COMMONS_BY_ALIGNMENT_ASCENDING)
+    {
+      if (aa < ab)
+	return true;
+      else if (ab < aa)
+	return false;
+    }
+  else
+    gold_assert(this->sort_order_
+		== Symbol_table::SORT_COMMONS_BY_SIZE_DESCENDING);
+
+  // Sort by descending size.
   if (sa < sb)
     return false;
   else if (sb < sa)
     return true;
 
-  // When the symbols are the same size, we sort them by alignment,
-  // largest alignment first.
-  typename Sized_symbol<size>::Value_type va = psa->value();
-  typename Sized_symbol<size>::Value_type vb = psb->value();
-  if (va < vb)
-    return false;
-  else if (vb < va)
-    return true;
+  if (this->sort_order_ == Symbol_table::SORT_COMMONS_BY_SIZE_DESCENDING)
+    {
+      // When the symbols are the same size, we sort them by
+      // alignment, largest alignment first.
+      if (aa < ab)
+	return false;
+      else if (ab < aa)
+	return true;
+    }
 
   // Otherwise we stabilize the sort by sorting by name.
   return strcmp(psa->name(), psb->name()) < 0;
@@ -120,10 +151,27 @@ Sort_commons<size>::operator()(const Sym
 void
 Symbol_table::allocate_commons(Layout* layout, Mapfile* mapfile)
 {
+  Sort_commons_order sort_order;
+  if (!parameters->options().user_set_sort_common())
+    sort_order = SORT_COMMONS_BY_SIZE_DESCENDING;
+  else
+    {
+      const char* order = parameters->options().sort_common();
+      if (*order == '\0' || strcmp(order, "descending") == 0)
+	sort_order = SORT_COMMONS_BY_ALIGNMENT_DESCENDING;
+      else if (strcmp(order, "ascending") == 0)
+	sort_order = SORT_COMMONS_BY_ALIGNMENT_ASCENDING;
+      else
+	{
+	  gold_error("invalid --sort-common argument: %s", order);
+	  sort_order = SORT_COMMONS_BY_SIZE_DESCENDING;
+	}
+    }
+
   if (parameters->target().get_size() == 32)
     {
 #if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_32_BIG)
-      this->do_allocate_commons<32>(layout, mapfile);
+      this->do_allocate_commons<32>(layout, mapfile, sort_order);
 #else
       gold_unreachable();
 #endif
@@ -131,7 +179,7 @@ Symbol_table::allocate_commons(Layout* l
   else if (parameters->target().get_size() == 64)
     {
 #if defined(HAVE_TARGET_64_LITTLE) || defined(HAVE_TARGET_64_BIG)
-      this->do_allocate_commons<64>(layout, mapfile);
+      this->do_allocate_commons<64>(layout, mapfile, sort_order);
 #else
       gold_unreachable();
 #endif
@@ -144,20 +192,25 @@ Symbol_table::allocate_commons(Layout* l
 
 template<int size>
 void
-Symbol_table::do_allocate_commons(Layout* layout, Mapfile* mapfile)
+Symbol_table::do_allocate_commons(Layout* layout, Mapfile* mapfile,
+				  Sort_commons_order sort_order)
 {
   if (!this->commons_.empty())
     this->do_allocate_commons_list<size>(layout, COMMONS_NORMAL,
-					 &this->commons_, mapfile);
+					 &this->commons_, mapfile,
+					 sort_order);
   if (!this->tls_commons_.empty())
     this->do_allocate_commons_list<size>(layout, COMMONS_TLS,
-					 &this->tls_commons_, mapfile);
+					 &this->tls_commons_, mapfile,
+					 sort_order);
   if (!this->small_commons_.empty())
     this->do_allocate_commons_list<size>(layout, COMMONS_SMALL,
-					 &this->small_commons_, mapfile);
+					 &this->small_commons_, mapfile,
+					 sort_order);
   if (!this->large_commons_.empty())
     this->do_allocate_commons_list<size>(layout, COMMONS_LARGE,
-					 &this->large_commons_, mapfile);
+					 &this->large_commons_, mapfile,
+					 sort_order);
 }
 
 // Allocate the common symbols in a list.  IS_TLS indicates whether
@@ -169,7 +222,8 @@ Symbol_table::do_allocate_commons_list(
     Layout* layout,
     Commons_section_type commons_section_type,
     Commons_type* commons,
-    Mapfile* mapfile)
+    Mapfile* mapfile,
+    Sort_commons_order sort_order)
 {
   typedef typename Sized_symbol<size>::Value_type Value_type;
   typedef typename Sized_symbol<size>::Size_type Size_type;
@@ -202,10 +256,9 @@ Symbol_table::do_allocate_commons_list(
   if (!any)
     return;
 
-  // Sort the common symbols by size, so that they pack better into
-  // memory.
+  // Sort the common symbols.
   std::sort(commons->begin(), commons->end(),
-	    Sort_commons<size>(this));
+	    Sort_commons<size>(this, sort_order));
 
   // Place them in a newly allocated BSS section.
   elfcpp::Elf_Xword flags = elfcpp::SHF_WRITE | elfcpp::SHF_ALLOC;
Index: options.h
===================================================================
RCS file: /cvs/src/src/gold/options.h,v
retrieving revision 1.123
diff -p -u -r1.123 options.h
--- options.h	30 Dec 2009 22:35:48 -0000	1.123
+++ options.h	31 Dec 2009 01:55:08 -0000
@@ -819,6 +819,10 @@ class General_options
                  N_("Add DIR to link time shared library search path"),
                  N_("DIR"));
 
+  DEFINE_optional_string(sort_common, options::TWO_DASHES, '\0', NULL,
+			 N_("Sort common symbols by alignment"),
+			 N_("[={ascending,descending}]"));
+
   DEFINE_bool(strip_all, options::TWO_DASHES, 's', false,
               N_("Strip all symbols"), NULL);
   DEFINE_bool(strip_debug, options::TWO_DASHES, 'S', false,
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gold/symtab.h,v
retrieving revision 1.101
diff -p -u -r1.101 symtab.h
--- symtab.h	30 Dec 2009 07:40:01 -0000	1.101
+++ symtab.h	31 Dec 2009 01:55:08 -0000
@@ -1195,6 +1195,14 @@ class Symbol_table
     PREDEFINED,
   };
 
+  // The order in which we sort common symbols.
+  enum Sort_commons_order
+  {
+    SORT_COMMONS_BY_SIZE_DESCENDING,
+    SORT_COMMONS_BY_ALIGNMENT_DESCENDING,
+    SORT_COMMONS_BY_ALIGNMENT_ASCENDING
+  };
+
   // COUNT is an estimate of how many symbosl will be inserted in the
   // symbol table.  It's ok to put 0 if you don't know; a correct
   // guess will just save some CPU by reducing hashtable resizes.
@@ -1603,13 +1611,13 @@ class Symbol_table
   // Allocate the common symbols, sized version.
   template<int size>
   void
-  do_allocate_commons(Layout*, Mapfile*);
+  do_allocate_commons(Layout*, Mapfile*, Sort_commons_order);
 
   // Allocate the common symbols from one list.
   template<int size>
   void
   do_allocate_commons_list(Layout*, Commons_section_type, Commons_type*,
-			   Mapfile*);
+			   Mapfile*, Sort_commons_order);
 
   // Implement detect_odr_violations.
   template<int size, bool big_endian>