public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Re: Initial MIPS patch for GOLD - version 3
@ 2012-02-03 15:40 Simeonov, Aleksandar (c)
  2012-03-07 14:26 ` Simeonov, Aleksandar (c)
  0 siblings, 1 reply; 10+ messages in thread
From: Simeonov, Aleksandar (c) @ 2012-02-03 15:40 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: Fuhler, Rich, mips-compiler, binutils

Hi Ian,
I agree with you that processor specific stuff should be in CPU.cc. Because of that I would like to suggest following:

- reloc.cc (Sized_relobj_file<size, big_endian>::write_sections)

Instead of having direct compare of section types in code:
// For MIPS .reginfo section there is no need to do anything
if (shdr.get_sh_type() == elfcpp::SHT_MIPS_REGINFO)
  continue;

To have something like:
// Sections that need special handling (target specific)
if(parameters->target().section_needs_spec_handling(shdr.get_sh_type()))
 continue;

Where section_needs_spec_handling should be defined in Target as a virtual function that returns false by default and can be implemented as needed.


- layout.cc (Layout::segment_precedes)

Instead of having function segment_precedes in Layout class, to move it to Target class. Make virtual default implementation as it was originally and allow different architectures to make their own implementation.

Maybe some other proposals?

Greetings,
Aleksandar 

On 28/01/2012 02:54, Ian Lance Taylor wrote:
> Aleksandar Simeonov <Aleksandar.Simeonov@RT-RK.com> writes:
> 
>> * reloc.cc (Sized_relobj_file<size, big_endian>::write_sections): Special
>> handling of MIPS .reginfo section.
>> - .reginfo section is generated by linker and needs special handling.
> 
> I haven't thought about everything, but I can see that this patch is not
> going to work as is.  It will fail when linking a non-MIPS object which
> happens to have a section type == SHT_MIPS_REGINFO.  We can't use
> processor-specific values like SHT_MIPS_REGINFO outside of the CPU.cc
> file.
> 
>> * layout.cc (Layout::segment_precedes): Fixed order of MIPS specific
>> segments.
>> - MIPS needs to have PT_MIPS_REGINFO segment before any loadable segment.
> 
> This is similarly troubling, though probably less serious.
> 
>> - MIPS needs to have PT_NULL segment to be last in list of segments.
> 
> Why would we ever have a PT_NULL segment?
> 
> Ian
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: Initial MIPS patch for GOLD - version 3
  2012-02-03 15:40 Initial MIPS patch for GOLD - version 3 Simeonov, Aleksandar (c)
@ 2012-03-07 14:26 ` Simeonov, Aleksandar (c)
  2012-03-07 14:54   ` Ian Lance Taylor
  0 siblings, 1 reply; 10+ messages in thread
From: Simeonov, Aleksandar (c) @ 2012-03-07 14:26 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: Fuhler, Rich, mips-compiler, binutils

Hi Ian,
Do you have any comments on proposals from my previous mail?

Are there any other comments about patch or you think it is ready for trunk?

Thanks in advance for your answers,
Aleksandar

________________________________________
From: Simeonov, Aleksandar (c)
Sent: Friday, February 03, 2012 4:40 PM
To: Ian Lance Taylor
Cc: Fuhler, Rich; mips-compiler@rt-rk.com; binutils@sourceware.org
Subject: Re: Initial MIPS patch for GOLD - version 3

Hi Ian,
I agree with you that processor specific stuff should be in CPU.cc. Because of that I would like to suggest following:

- reloc.cc (Sized_relobj_file<size, big_endian>::write_sections)

Instead of having direct compare of section types in code:
// For MIPS .reginfo section there is no need to do anything
if (shdr.get_sh_type() == elfcpp::SHT_MIPS_REGINFO)
  continue;

To have something like:
// Sections that need special handling (target specific)
if(parameters->target().section_needs_spec_handling(shdr.get_sh_type()))
 continue;

Where section_needs_spec_handling should be defined in Target as a virtual function that returns false by default and can be implemented as needed.


- layout.cc (Layout::segment_precedes)

Instead of having function segment_precedes in Layout class, to move it to Target class. Make virtual default implementation as it was originally and allow different architectures to make their own implementation.

Maybe some other proposals?

Greetings,
Aleksandar

On 28/01/2012 02:54, Ian Lance Taylor wrote:
> Aleksandar Simeonov <Aleksandar.Simeonov@RT-RK.com> writes:
>
>> * reloc.cc (Sized_relobj_file<size, big_endian>::write_sections): Special
>> handling of MIPS .reginfo section.
>> - .reginfo section is generated by linker and needs special handling.
>
> I haven't thought about everything, but I can see that this patch is not
> going to work as is.  It will fail when linking a non-MIPS object which
> happens to have a section type == SHT_MIPS_REGINFO.  We can't use
> processor-specific values like SHT_MIPS_REGINFO outside of the CPU.cc
> file.
>
>> * layout.cc (Layout::segment_precedes): Fixed order of MIPS specific
>> segments.
>> - MIPS needs to have PT_MIPS_REGINFO segment before any loadable segment.
>
> This is similarly troubling, though probably less serious.
>
>> - MIPS needs to have PT_NULL segment to be last in list of segments.
>
> Why would we ever have a PT_NULL segment?
>
> Ian
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Initial MIPS patch for GOLD - version 3
  2012-03-07 14:26 ` Simeonov, Aleksandar (c)
@ 2012-03-07 14:54   ` Ian Lance Taylor
  2012-03-07 19:46     ` Fuhler, Rich
  0 siblings, 1 reply; 10+ messages in thread
From: Ian Lance Taylor @ 2012-03-07 14:54 UTC (permalink / raw)
  To: Simeonov, Aleksandar (c); +Cc: Fuhler, Rich, mips-compiler, binutils

"Simeonov, Aleksandar (c)" <asimeonov@mips.com> writes:

> Do you have any comments on proposals from my previous mail?

I'm not crazy about those proposals, because they in effect add highly
specialized cases to the Target structure.  One of the problems with the
GNU linker is the function pointers in the elf_backend_data structure.
Many of the function pointers are very specific and hard to understand
and use correctly, which becomes an issue when the code changes.
Unfortunately I have to some extent recreated that in gold's Target
structure.  I'm not sure what to do in this area.

I've been postponing more serious consideration of your patch until the
coypright issues are sorted out.  Also I'm in the middle of overlapping
release cycles and simply haven't had time to do any gold work.  Sorry.

Ian

> ________________________________________
> From: Simeonov, Aleksandar (c)
> Sent: Friday, February 03, 2012 4:40 PM
> To: Ian Lance Taylor
> Cc: Fuhler, Rich; mips-compiler@rt-rk.com; binutils@sourceware.org
> Subject: Re: Initial MIPS patch for GOLD - version 3
>
> Hi Ian,
> I agree with you that processor specific stuff should be in CPU.cc. Because of that I would like to suggest following:
>
> - reloc.cc (Sized_relobj_file<size, big_endian>::write_sections)
>
> Instead of having direct compare of section types in code:
> // For MIPS .reginfo section there is no need to do anything
> if (shdr.get_sh_type() == elfcpp::SHT_MIPS_REGINFO)
>   continue;
>
> To have something like:
> // Sections that need special handling (target specific)
> if(parameters->target().section_needs_spec_handling(shdr.get_sh_type()))
>  continue;
>
> Where section_needs_spec_handling should be defined in Target as a virtual function that returns false by default and can be implemented as needed.
>
>
> - layout.cc (Layout::segment_precedes)
>
> Instead of having function segment_precedes in Layout class, to move it to Target class. Make virtual default implementation as it was originally and allow different architectures to make their own implementation.
>
> Maybe some other proposals?
>
> Greetings,
> Aleksandar
>
> On 28/01/2012 02:54, Ian Lance Taylor wrote:
>> Aleksandar Simeonov <Aleksandar.Simeonov@RT-RK.com> writes:
>>
>>> * reloc.cc (Sized_relobj_file<size, big_endian>::write_sections): Special
>>> handling of MIPS .reginfo section.
>>> - .reginfo section is generated by linker and needs special handling.
>>
>> I haven't thought about everything, but I can see that this patch is not
>> going to work as is.  It will fail when linking a non-MIPS object which
>> happens to have a section type == SHT_MIPS_REGINFO.  We can't use
>> processor-specific values like SHT_MIPS_REGINFO outside of the CPU.cc
>> file.
>>
>>> * layout.cc (Layout::segment_precedes): Fixed order of MIPS specific
>>> segments.
>>> - MIPS needs to have PT_MIPS_REGINFO segment before any loadable segment.
>>
>> This is similarly troubling, though probably less serious.
>>
>>> - MIPS needs to have PT_NULL segment to be last in list of segments.
>>
>> Why would we ever have a PT_NULL segment?
>>
>> Ian
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: Initial MIPS patch for GOLD - version 3
  2012-03-07 14:54   ` Ian Lance Taylor
@ 2012-03-07 19:46     ` Fuhler, Rich
  0 siblings, 0 replies; 10+ messages in thread
From: Fuhler, Rich @ 2012-03-07 19:46 UTC (permalink / raw)
  To: Ian Lance Taylor, Simeonov, Aleksandar (c); +Cc: mips-compiler, binutils

Hi Ian, I've been out with pneumonia and the corporate assignment wasn't updated while I was out. I'll get it done next day or so.
________________________________________
From: Ian Lance Taylor [iant@google.com]
Sent: Wednesday, March 07, 2012 06:54
To: Simeonov, Aleksandar (c)
Cc: Fuhler, Rich; mips-compiler@rt-rk.com; binutils@sourceware.org
Subject: Re: Initial MIPS patch for GOLD - version 3

"Simeonov, Aleksandar (c)" <asimeonov@mips.com> writes:

> Do you have any comments on proposals from my previous mail?

I'm not crazy about those proposals, because they in effect add highly
specialized cases to the Target structure.  One of the problems with the
GNU linker is the function pointers in the elf_backend_data structure.
Many of the function pointers are very specific and hard to understand
and use correctly, which becomes an issue when the code changes.
Unfortunately I have to some extent recreated that in gold's Target
structure.  I'm not sure what to do in this area.

I've been postponing more serious consideration of your patch until the
coypright issues are sorted out.  Also I'm in the middle of overlapping
release cycles and simply haven't had time to do any gold work.  Sorry.

Ian

> ________________________________________
> From: Simeonov, Aleksandar (c)
> Sent: Friday, February 03, 2012 4:40 PM
> To: Ian Lance Taylor
> Cc: Fuhler, Rich; mips-compiler@rt-rk.com; binutils@sourceware.org
> Subject: Re: Initial MIPS patch for GOLD - version 3
>
> Hi Ian,
> I agree with you that processor specific stuff should be in CPU.cc. Because of that I would like to suggest following:
>
> - reloc.cc (Sized_relobj_file<size, big_endian>::write_sections)
>
> Instead of having direct compare of section types in code:
> // For MIPS .reginfo section there is no need to do anything
> if (shdr.get_sh_type() == elfcpp::SHT_MIPS_REGINFO)
>   continue;
>
> To have something like:
> // Sections that need special handling (target specific)
> if(parameters->target().section_needs_spec_handling(shdr.get_sh_type()))
>  continue;
>
> Where section_needs_spec_handling should be defined in Target as a virtual function that returns false by default and can be implemented as needed.
>
>
> - layout.cc (Layout::segment_precedes)
>
> Instead of having function segment_precedes in Layout class, to move it to Target class. Make virtual default implementation as it was originally and allow different architectures to make their own implementation.
>
> Maybe some other proposals?
>
> Greetings,
> Aleksandar
>
> On 28/01/2012 02:54, Ian Lance Taylor wrote:
>> Aleksandar Simeonov <Aleksandar.Simeonov@RT-RK.com> writes:
>>
>>> * reloc.cc (Sized_relobj_file<size, big_endian>::write_sections): Special
>>> handling of MIPS .reginfo section.
>>> - .reginfo section is generated by linker and needs special handling.
>>
>> I haven't thought about everything, but I can see that this patch is not
>> going to work as is.  It will fail when linking a non-MIPS object which
>> happens to have a section type == SHT_MIPS_REGINFO.  We can't use
>> processor-specific values like SHT_MIPS_REGINFO outside of the CPU.cc
>> file.
>>
>>> * layout.cc (Layout::segment_precedes): Fixed order of MIPS specific
>>> segments.
>>> - MIPS needs to have PT_MIPS_REGINFO segment before any loadable segment.
>>
>> This is similarly troubling, though probably less serious.
>>
>>> - MIPS needs to have PT_NULL segment to be last in list of segments.
>>
>> Why would we ever have a PT_NULL segment?
>>
>> Ian
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Initial MIPS patch for GOLD - version 3
  2012-01-28  2:11   ` John Reiser
@ 2012-01-29 12:13     ` Richard Sandiford
  0 siblings, 0 replies; 10+ messages in thread
From: Richard Sandiford @ 2012-01-29 12:13 UTC (permalink / raw)
  To: John Reiser; +Cc: Binutils

John Reiser <jreiser@bitwagon.com> writes:
> On 01/27/2012 05:54 PM, Ian Lance Taylor wrote:
>> Aleksandar Simeonov <Aleksandar.Simeonov@RT-RK.com> writes:
>
>>> - MIPS needs to have PT_NULL segment to be last in list of segments.
>
>> Why would we ever have a PT_NULL segment?
>
> It's a way to insert a "comment" into the Phdrs.  Such a comment
> can be convenient for extensions or special cases that the usual
> tools did not anticipate.

In this case it's for the prelinker:

http://sourceware.org/ml/binutils/2006-10/msg00174.html

Richard

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Initial MIPS patch for GOLD - version 3
  2012-01-28  1:55 ` Ian Lance Taylor
@ 2012-01-28  2:11   ` John Reiser
  2012-01-29 12:13     ` Richard Sandiford
  0 siblings, 1 reply; 10+ messages in thread
From: John Reiser @ 2012-01-28  2:11 UTC (permalink / raw)
  To: Binutils

On 01/27/2012 05:54 PM, Ian Lance Taylor wrote:
> Aleksandar Simeonov <Aleksandar.Simeonov@RT-RK.com> writes:

>> - MIPS needs to have PT_NULL segment to be last in list of segments.

> Why would we ever have a PT_NULL segment?

It's a way to insert a "comment" into the Phdrs.  Such a comment
can be convenient for extensions or special cases that the usual
tools did not anticipate.

-- 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Initial MIPS patch for GOLD - version 3
  2012-01-24 15:39 Aleksandar Simeonov
  2012-01-28  1:06 ` Ian Lance Taylor
  2012-01-28  1:48 ` Ian Lance Taylor
@ 2012-01-28  1:55 ` Ian Lance Taylor
  2012-01-28  2:11   ` John Reiser
  2 siblings, 1 reply; 10+ messages in thread
From: Ian Lance Taylor @ 2012-01-28  1:55 UTC (permalink / raw)
  To: Aleksandar Simeonov; +Cc: Fuhler, Rich, mips-compiler, binutils

Aleksandar Simeonov <Aleksandar.Simeonov@RT-RK.com> writes:

> * reloc.cc (Sized_relobj_file<size, big_endian>::write_sections): Special
> handling of MIPS .reginfo section.
> - .reginfo section is generated by linker and needs special handling.

I haven't thought about everything, but I can see that this patch is not
going to work as is.  It will fail when linking a non-MIPS object which
happens to have a section type == SHT_MIPS_REGINFO.  We can't use
processor-specific values like SHT_MIPS_REGINFO outside of the CPU.cc
file.

> * layout.cc (Layout::segment_precedes): Fixed order of MIPS specific
> segments.
> - MIPS needs to have PT_MIPS_REGINFO segment before any loadable segment.

This is similarly troubling, though probably less serious.

> - MIPS needs to have PT_NULL segment to be last in list of segments.

Why would we ever have a PT_NULL segment?

Ian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Initial MIPS patch for GOLD - version 3
  2012-01-24 15:39 Aleksandar Simeonov
  2012-01-28  1:06 ` Ian Lance Taylor
@ 2012-01-28  1:48 ` Ian Lance Taylor
  2012-01-28  1:55 ` Ian Lance Taylor
  2 siblings, 0 replies; 10+ messages in thread
From: Ian Lance Taylor @ 2012-01-28  1:48 UTC (permalink / raw)
  To: Aleksandar Simeonov; +Cc: Fuhler, Rich, mips-compiler, binutils

[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]

Aleksandar Simeonov <Aleksandar.Simeonov@RT-RK.com> writes:

> * utils.h: New file that contains utilities for manipulating integers of
> up to 32-bits.
> * arm.cc (namespace utils): Removed common code to separate file (utils.h).
> * arm.cc (utils::sign_extend): Removed.
> * arm.cc (utils::has_overflow): Likewise.
> * arm.cc (utils::has_signed_unsigned_overflow): Likewise.
> * arm.cc (utils::bit_select): Likewise.
> - Functions that are common for MIPS and ARM architectures moved to
> separate file to avoid duplication of code.

I implemented this in a different way.  I moved the functions from
arm.cc into reloc.h.  I put them in a template class Bits and gave them
slightly different names.  Please update your sources as per the arm.cc
patch below, now committed to mainline.

Ian


2012-01-27  Ian Lance Taylor  <iant@google.com>

	* reloc.h (Bits): New class with static functions, copied from
	namespace utils in arm.cc.
	* arm.cc (namespace utils): Remove.  Rewrite all uses to use Bits
	instead.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 19708 bytes --]

Index: arm.cc
===================================================================
RCS file: /cvs/src/src/gold/arm.cc,v
retrieving revision 1.143
diff -u -r1.143 arm.cc
--- arm.cc	11 Nov 2011 21:49:36 -0000	1.143
+++ arm.cc	28 Jan 2012 01:46:22 -0000
@@ -1,6 +1,6 @@
 // arm.cc -- arm target support for gold.
 
-// Copyright 2009, 2010, 2011 Free Software Foundation, Inc.
+// Copyright 2009, 2010, 2011, 2012 Free Software Foundation, Inc.
 // Written by Doug Kwan <dougkwan@google.com> based on the i386 code
 // by Ian Lance Taylor <iant@google.com>.
 // This file also contains borrowed and adapted code from
@@ -2103,65 +2103,6 @@
   }
 };
 
-// Utilities for manipulating integers of up to 32-bits
-
-namespace utils
-{
-  // Sign extend an n-bit unsigned integer stored in an uint32_t into
-  // an int32_t.  NO_BITS must be between 1 to 32.
-  template<int no_bits>
-  static inline int32_t
-  sign_extend(uint32_t bits)
-  {
-    gold_assert(no_bits >= 0 && no_bits <= 32);
-    if (no_bits == 32)
-      return static_cast<int32_t>(bits);
-    uint32_t mask = (~((uint32_t) 0)) >> (32 - no_bits);
-    bits &= mask;
-    uint32_t top_bit = 1U << (no_bits - 1);
-    int32_t as_signed = static_cast<int32_t>(bits);
-    return (bits & top_bit) ? as_signed + (-top_bit * 2) : as_signed;
-  }
-
-  // Detects overflow of an NO_BITS integer stored in a uint32_t.
-  template<int no_bits>
-  static inline bool
-  has_overflow(uint32_t bits)
-  {
-    gold_assert(no_bits >= 0 && no_bits <= 32);
-    if (no_bits == 32)
-      return false;
-    int32_t max = (1 << (no_bits - 1)) - 1;
-    int32_t min = -(1 << (no_bits - 1));
-    int32_t as_signed = static_cast<int32_t>(bits);
-    return as_signed > max || as_signed < min;
-  }
-
-  // Detects overflow of an NO_BITS integer stored in a uint32_t when it
-  // fits in the given number of bits as either a signed or unsigned value.
-  // For example, has_signed_unsigned_overflow<8> would check
-  // -128 <= bits <= 255
-  template<int no_bits>
-  static inline bool
-  has_signed_unsigned_overflow(uint32_t bits)
-  {
-    gold_assert(no_bits >= 2 && no_bits <= 32);
-    if (no_bits == 32)
-      return false;
-    int32_t max = static_cast<int32_t>((1U << no_bits) - 1);
-    int32_t min = -(1 << (no_bits - 1));
-    int32_t as_signed = static_cast<int32_t>(bits);
-    return as_signed > max || as_signed < min;
-  }
-
-  // Select bits from A and B using bits in MASK.  For each n in [0..31],
-  // the n-th bit in the result is chosen from the n-th bits of A and B.
-  // A zero selects A and a one selects B.
-  static inline uint32_t
-  bit_select(uint32_t a, uint32_t b, uint32_t mask)
-  { return (a & ~mask) | (b & mask); }
-};
-
 template<bool big_endian>
 class Target_arm : public Sized_target<32, big_endian>
 {
@@ -3015,7 +2956,7 @@
   {
     // According to the Elf ABI for ARM Architecture the immediate
     // field is sign-extended to form the addend.
-    return utils::sign_extend<16>(((val >> 4) & 0xf000) | (val & 0xfff));
+    return Bits<16>::sign_extend32(((val >> 4) & 0xf000) | (val & 0xfff));
   }
 
   // Insert X into VAL based on the ARM instruction encoding described
@@ -3049,10 +2990,10 @@
   {
     // According to the Elf ABI for ARM Architecture the immediate
     // field is sign-extended to form the addend.
-    return utils::sign_extend<16>(((val >> 4) & 0xf000)
-				  | ((val >> 15) & 0x0800)
-				  | ((val >> 4) & 0x0700)
-				  | (val & 0x00ff));
+    return Bits<16>::sign_extend32(((val >> 4) & 0xf000)
+				   | ((val >> 15) & 0x0800)
+				   | ((val >> 4) & 0x0700)
+				   | (val & 0x00ff));
   }
 
   // Insert X into VAL based on the Thumb2 instruction encoding
@@ -3160,8 +3101,8 @@
     uint32_t i1 = j1 ^ s ? 0 : 1;
     uint32_t i2 = j2 ^ s ? 0 : 1;
 
-    return utils::sign_extend<25>((s << 24) | (i1 << 23) | (i2 << 22)
-				  | (upper << 12) | (lower << 1));
+    return Bits<25>::sign_extend32((s << 24) | (i1 << 23) | (i2 << 22)
+				   | (upper << 12) | (lower << 1));
   }
 
   // Insert OFFSET to a 32-bit THUMB branch and return the upper instruction.
@@ -3199,7 +3140,7 @@
     uint32_t lower = (lower_insn & 0x07ffU);
     uint32_t upper = (s << 8) | (j2 << 7) | (j1 << 6) | (upper_insn & 0x003fU);
 
-    return utils::sign_extend<21>((upper << 12) | (lower << 1));
+    return Bits<21>::sign_extend32((upper << 12) | (lower << 1));
   }
 
   // Insert OFFSET to a 32-bit THUMB conditional branch and return the upper
@@ -3236,9 +3177,9 @@
     typedef typename elfcpp::Swap<8, big_endian>::Valtype Valtype;
     Valtype* wv = reinterpret_cast<Valtype*>(view);
     Valtype val = elfcpp::Swap<8, big_endian>::readval(wv);
-    int32_t addend = utils::sign_extend<8>(val);
+    int32_t addend = Bits<8>::sign_extend32(val);
     Arm_address x = psymval->value(object, addend);
-    val = utils::bit_select(val, x, 0xffU);
+    val = Bits<32>::bit_select32(val, x, 0xffU);
     elfcpp::Swap<8, big_endian>::writeval(wv, val);
 
     // R_ARM_ABS8 permits signed or unsigned results.
@@ -3260,7 +3201,7 @@
     Valtype val = elfcpp::Swap<16, big_endian>::readval(wv);
     Reltype addend = (val & 0x7e0U) >> 6;
     Reltype x = psymval->value(object, addend);
-    val = utils::bit_select(val, x << 6, 0x7e0U);
+    val = Bits<32>::bit_select32(val, x << 6, 0x7e0U);
     elfcpp::Swap<16, big_endian>::writeval(wv, val);
 
     // R_ARM_ABS16 permits signed or unsigned results.
@@ -3282,9 +3223,9 @@
     Valtype val = elfcpp::Swap<32, big_endian>::readval(wv);
     Reltype addend = val & 0x0fffU;
     Reltype x = psymval->value(object, addend);
-    val = utils::bit_select(val, x, 0x0fffU);
+    val = Bits<32>::bit_select32(val, x, 0x0fffU);
     elfcpp::Swap<32, big_endian>::writeval(wv, val);
-    return (utils::has_overflow<12>(x)
+    return (Bits<12>::has_overflow32(x)
 	    ? This::STATUS_OVERFLOW
 	    : This::STATUS_OKAY);
   }
@@ -3298,9 +3239,9 @@
     typedef typename elfcpp::Swap_unaligned<16, big_endian>::Valtype Valtype;
     typedef typename elfcpp::Swap<32, big_endian>::Valtype Reltype;
     Valtype val = elfcpp::Swap_unaligned<16, big_endian>::readval(view);
-    int32_t addend = utils::sign_extend<16>(val);
+    int32_t addend = Bits<16>::sign_extend32(val);
     Arm_address x = psymval->value(object, addend);
-    val = utils::bit_select(val, x, 0xffffU);
+    val = Bits<32>::bit_select32(val, x, 0xffffU);
     elfcpp::Swap_unaligned<16, big_endian>::writeval(view, val);
 
     // R_ARM_ABS16 permits signed or unsigned results.
@@ -3377,12 +3318,12 @@
     typedef typename elfcpp::Swap<16, big_endian>::Valtype Valtype;
     Valtype* wv = reinterpret_cast<Valtype*>(view);
     Valtype val = elfcpp::Swap<16, big_endian>::readval(wv);
-    int32_t addend = utils::sign_extend<8>((val & 0x00ff) << 1);
+    int32_t addend = Bits<8>::sign_extend32((val & 0x00ff) << 1);
     int32_t x = (psymval->value(object, addend) - address);
     elfcpp::Swap<16, big_endian>::writeval(wv, ((val & 0xff00)
                                                 | ((x & 0x01fe) >> 1)));
     // We do a 9-bit overflow check because x is right-shifted by 1 bit.
-    return (utils::has_overflow<9>(x)
+    return (Bits<9>::has_overflow32(x)
 	    ? This::STATUS_OVERFLOW
 	    : This::STATUS_OKAY);
   }
@@ -3397,12 +3338,12 @@
     typedef typename elfcpp::Swap<16, big_endian>::Valtype Valtype;
     Valtype* wv = reinterpret_cast<Valtype*>(view);
     Valtype val = elfcpp::Swap<16, big_endian>::readval(wv);
-    int32_t addend = utils::sign_extend<11>((val & 0x07ff) << 1);
+    int32_t addend = Bits<11>::sign_extend32((val & 0x07ff) << 1);
     int32_t x = (psymval->value(object, addend) - address);
     elfcpp::Swap<16, big_endian>::writeval(wv, ((val & 0xf800)
                                                 | ((x & 0x0ffe) >> 1)));
     // We do a 12-bit overflow check because x is right-shifted by 1 bit.
-    return (utils::has_overflow<12>(x)
+    return (Bits<12>::has_overflow32(x)
 	    ? This::STATUS_OVERFLOW
 	    : This::STATUS_OKAY);
   }
@@ -3455,12 +3396,13 @@
   {
     typedef typename elfcpp::Swap_unaligned<32, big_endian>::Valtype Valtype;
     Valtype val = elfcpp::Swap_unaligned<32, big_endian>::readval(view);
-    Valtype addend = utils::sign_extend<31>(val);
+    Valtype addend = Bits<31>::sign_extend32(val);
     Valtype x = (psymval->value(object, addend) | thumb_bit) - address;
-    val = utils::bit_select(val, x, 0x7fffffffU);
+    val = Bits<32>::bit_select32(val, x, 0x7fffffffU);
     elfcpp::Swap_unaligned<32, big_endian>::writeval(view, val);
-    return (utils::has_overflow<31>(x) ?
-	    This::STATUS_OVERFLOW : This::STATUS_OKAY);
+    return (Bits<31>::has_overflow32(x)
+	    ? This::STATUS_OVERFLOW
+	    : This::STATUS_OKAY);
   }
 
   // R_ARM_MOVW_ABS_NC: (S + A) | T	(relative address base is )
@@ -3483,7 +3425,7 @@
 		 - relative_address_base);
     val = This::insert_val_arm_movw_movt(val, x);
     elfcpp::Swap<32, big_endian>::writeval(wv, val);
-    return ((check_overflow && utils::has_overflow<16>(x))
+    return ((check_overflow && Bits<16>::has_overflow32(x))
 	    ? This::STATUS_OVERFLOW
 	    : This::STATUS_OKAY);
   }
@@ -3531,7 +3473,7 @@
     val = This::insert_val_thumb_movw_movt(val, x);
     elfcpp::Swap<16, big_endian>::writeval(wv, val >> 16);
     elfcpp::Swap<16, big_endian>::writeval(wv + 1, val & 0xffff);
-    return ((check_overflow && utils::has_overflow<16>(x))
+    return ((check_overflow && Bits<16>::has_overflow32(x))
     	    ? This::STATUS_OVERFLOW
 	    : This::STATUS_OKAY);
   }
@@ -3963,7 +3905,7 @@
       return This::STATUS_OKAY;
     }
  
-  Valtype addend = utils::sign_extend<26>(val << 2);
+  Valtype addend = Bits<26>::sign_extend32(val << 2);
   Valtype branch_target = psymval->value(object, addend);
   int32_t branch_offset = branch_target - address;
 
@@ -3973,7 +3915,7 @@
   Reloc_stub* stub = NULL;
 
   if (!parameters->options().relocatable()
-      && (utils::has_overflow<26>(branch_offset)
+      && (Bits<26>::has_overflow32(branch_offset)
 	  || ((thumb_bit != 0)
 	      && !(may_use_blx && r_type == elfcpp::R_ARM_CALL))))
     {
@@ -3995,7 +3937,7 @@
 	  thumb_bit = stub->stub_template()->entry_in_thumb_mode() ? 1 : 0;
 	  branch_target = stub_table->address() + stub->offset() + addend;
 	  branch_offset = branch_target - address;
-	  gold_assert(!utils::has_overflow<26>(branch_offset));
+	  gold_assert(!Bits<26>::has_overflow32(branch_offset));
 	}
     }
 
@@ -4008,10 +3950,11 @@
       val = (val & 0xffffff) | 0xfa000000 | ((branch_offset & 2) << 23);
     }
 
-  val = utils::bit_select(val, (branch_offset >> 2), 0xffffffUL);
+  val = Bits<32>::bit_select32(val, (branch_offset >> 2), 0xffffffUL);
   elfcpp::Swap<32, big_endian>::writeval(wv, val);
-  return (utils::has_overflow<26>(branch_offset)
-	  ? This::STATUS_OVERFLOW : This::STATUS_OKAY);
+  return (Bits<26>::has_overflow32(branch_offset)
+	  ? This::STATUS_OVERFLOW
+	  : This::STATUS_OKAY);
 }
 
 // Relocate THUMB long branches.  This handles relocation types
@@ -4102,7 +4045,7 @@
   // For BLX, bit 1 of target address comes from bit 1 of base address.
   bool may_use_blx = arm_target->may_use_v5t_interworking();
   if (thumb_bit == 0 && may_use_blx)
-    branch_target = utils::bit_select(branch_target, address, 0x2);
+    branch_target = Bits<32>::bit_select32(branch_target, address, 0x2);
 
   int32_t branch_offset = branch_target - address;
 
@@ -4110,8 +4053,8 @@
   // to switch mode.
   bool thumb2 = arm_target->using_thumb2();
   if (!parameters->options().relocatable()
-      && ((!thumb2 && utils::has_overflow<23>(branch_offset))
-	  || (thumb2 && utils::has_overflow<25>(branch_offset))
+      && ((!thumb2 && Bits<23>::has_overflow32(branch_offset))
+	  || (thumb2 && Bits<25>::has_overflow32(branch_offset))
 	  || ((thumb_bit == 0)
 	      && (((r_type == elfcpp::R_ARM_THM_CALL) && !may_use_blx)
 		  || r_type == elfcpp::R_ARM_THM_JUMP24))))
@@ -4135,7 +4078,7 @@
 	  thumb_bit = stub->stub_template()->entry_in_thumb_mode() ? 1 : 0;
 	  branch_target = stub_table->address() + stub->offset() + addend;
 	  if (thumb_bit == 0 && may_use_blx) 
-	    branch_target = utils::bit_select(branch_target, address, 0x2);
+	    branch_target = Bits<32>::bit_select32(branch_target, address, 0x2);
 	  branch_offset = branch_target - address;
 	}
     }
@@ -4172,11 +4115,11 @@
   elfcpp::Swap<16, big_endian>::writeval(wv, upper_insn);
   elfcpp::Swap<16, big_endian>::writeval(wv + 1, lower_insn);
 
-  gold_assert(!utils::has_overflow<25>(branch_offset));
+  gold_assert(!Bits<25>::has_overflow32(branch_offset));
 
   return ((thumb2
-	   ? utils::has_overflow<25>(branch_offset)
-	   : utils::has_overflow<23>(branch_offset))
+	   ? Bits<25>::has_overflow32(branch_offset)
+	   : Bits<23>::has_overflow32(branch_offset))
 	  ? This::STATUS_OVERFLOW
 	  : This::STATUS_OKAY);
 }
@@ -4221,7 +4164,7 @@
   elfcpp::Swap<16, big_endian>::writeval(wv, upper_insn);
   elfcpp::Swap<16, big_endian>::writeval(wv + 1, lower_insn);
 
-  return (utils::has_overflow<21>(branch_offset)
+  return (Bits<21>::has_overflow32(branch_offset)
 	  ? This::STATUS_OVERFLOW
 	  : This::STATUS_OKAY);
 }
@@ -4504,7 +4447,7 @@
       // For THUMB BLX instruction, bit 1 of target comes from bit 1 of the
       // base address (instruction address + 4).
       if ((r_type == elfcpp::R_ARM_THM_CALL) && may_use_blx && !target_is_thumb)
-	destination = utils::bit_select(destination, location, 0x2);
+	destination = Bits<32>::bit_select32(destination, location, 0x2);
       branch_offset = static_cast<int64_t>(destination) - location;
 	
       // Handle cases where:
@@ -5277,7 +5220,7 @@
   // or after the end of a text section.  The second word is the special
   // EXIDX_CANTUNWIND value.
   uint32_t prel31_offset = output_address - this->address();
-  if (utils::has_overflow<31>(offset))
+  if (Bits<31>::has_overflow32(offset))
     gold_error(_("PREL31 overflow in EXIDX_CANTUNWIND entry"));
   elfcpp::Swap_unaligned<32, big_endian>::writeval(oview,
 						   prel31_offset & 0x7fffffffU);
@@ -7050,7 +6993,7 @@
 	typedef typename elfcpp::Swap<32, big_endian>::Valtype Valtype;
 	const Valtype* wv = reinterpret_cast<const Valtype*>(view);
 	Valtype val = elfcpp::Swap<32, big_endian>::readval(wv);
-	return utils::sign_extend<26>(val << 2);
+	return Bits<26>::sign_extend32(val << 2);
       }
 
     case elfcpp::R_ARM_THM_CALL:
@@ -11901,7 +11844,7 @@
 	branch_offset = (branch_offset + 2) & ~3;
 
       // Put BRANCH_OFFSET back into the insn.
-      gold_assert(!utils::has_overflow<25>(branch_offset));
+      gold_assert(!Bits<25>::has_overflow32(branch_offset));
       upper_insn = RelocFuncs::thumb32_branch_upper(upper_insn, branch_offset);
       lower_insn = RelocFuncs::thumb32_branch_lower(lower_insn, branch_offset);
       break;
Index: reloc.h
===================================================================
RCS file: /cvs/src/src/gold/reloc.h,v
retrieving revision 1.33
diff -u -r1.33 reloc.h
--- reloc.h	10 Nov 2011 20:53:36 -0000	1.33
+++ reloc.h	28 Jan 2012 01:46:22 -0000
@@ -1,6 +1,7 @@
 // reloc.h -- relocate input files for gold   -*- C++ -*-
 
-// Copyright 2006, 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
+// Copyright 2006, 2007, 2008, 2009, 2010, 2011, 2012
+// Free Software Foundation, Inc.
 // Written by Ian Lance Taylor <iant@google.com>.
 
 // This file is part of gold.
@@ -715,6 +716,122 @@
   { This::template pcrela<64>(view, object, psymval, addend, address); }
 };
 
+// Integer manipulation functions used by various targets when
+// performing relocations.
+
+template<int bits>
+class Bits
+{
+ public:
+  // Sign extend an n-bit unsigned integer stored in a uint32_t into
+  // an int32_t.  BITS must be between 0 and 32.
+  static inline int32_t
+  sign_extend32(uint32_t val)
+  {
+    gold_assert(bits >= 0 && bits <= 32);
+    if (bits == 32)
+      return static_cast<int32_t>(val);
+    uint32_t mask = (~static_cast<uint32_t>(0)) >> (32 - bits);
+    val &= mask;
+    uint32_t top_bit = 1U << (bits - 1);
+    int32_t as_signed = static_cast<int32_t>(val);
+    if ((val & top_bit) != 0)
+      as_signed -= static_cast<int32_t>(top_bit * 2);
+    return as_signed;    
+  }
+
+  // Return true if VAL (stored in a uint32_t) has overflowed a signed
+  // value with BITS bits.
+  static inline bool
+  has_overflow32(uint32_t val)
+  {
+    gold_assert(bits >= 0 && bits <= 32);
+    if (bits == 32)
+      return false;
+    int32_t max = (1 << (bits - 1)) - 1;
+    int32_t min = -(1 << (bits - 1));
+    int32_t as_signed = static_cast<int32_t>(val);
+    return as_signed > max || as_signed < min;
+  }
+
+  // Return true if VAL (stored in a uint32_t) has overflowed both a
+  // signed and an unsigned value.  E.g.,
+  // Bits<8>::has_signed_unsigned_overflow32 would check -128 <= VAL <
+  // 255.
+  static inline bool
+  has_signed_unsigned_overflow32(uint32_t val)
+  {
+    gold_assert(bits >= 0 && bits <= 32);
+    if (bits == 32)
+      return false;
+    int32_t max = static_cast<int32_t>((1U << bits) - 1);
+    int32_t min = -(1 << (bits - 1));
+    int32_t as_signed = static_cast<int32_t>(val);
+    return as_signed > max || as_signed < min;
+  }
+
+  // Select bits from A and B using bits in MASK.  For each n in
+  // [0..31], the n-th bit in the result is chosen from the n-th bits
+  // of A and B.  A zero selects A and a one selects B.
+  static inline uint32_t
+  bit_select32(uint32_t a, uint32_t b, uint32_t mask)
+  { return (a & ~mask) | (b & mask); }
+
+  // Sign extend an n-bit unsigned integer stored in a uint64_t into
+  // an int64_t.  BITS must be between 0 and 64.
+  static inline int64_t
+  sign_extend(uint64_t val)
+  {
+    gold_assert(bits >= 0 && bits <= 64);
+    if (bits == 64)
+      return static_cast<int64_t>(val);
+    uint64_t mask = (~static_cast<uint64_t>(0)) >> (64 - bits);
+    val &= mask;
+    uint64_t top_bit = static_cast<uint64_t>(1) << (bits - 1);
+    int64_t as_signed = static_cast<int64_t>(val);
+    if ((val & top_bit) != 0)
+      as_signed -= static_cast<int64_t>(top_bit * 2);
+    return as_signed;    
+  }
+
+  // Return true if VAL (stored in a uint64_t) has overflowed a signed
+  // value with BITS bits.
+  static inline bool
+  has_overflow(uint64_t val)
+  {
+    gold_assert(bits >= 0 && bits <= 64);
+    if (bits == 64)
+      return false;
+    int64_t max = (static_cast<int64_t>(1) << (bits - 1)) - 1;
+    int64_t min = -(static_cast<int64_t>(1) << (bits - 1));
+    int64_t as_signed = static_cast<int64_t>(val);
+    return as_signed > max || as_signed < min;
+  }
+
+  // Return true if VAL (stored in a uint64_t) has overflowed both a
+  // signed and an unsigned value.  E.g.,
+  // Bits<8>::has_signed_unsigned_overflow would check -128 <= VAL <
+  // 255.
+  static inline bool
+  has_signed_unsigned_overflow64(uint64_t val)
+  {
+    gold_assert(bits >= 0 && bits <= 64);
+    if (bits == 64)
+      return false;
+    int64_t max = static_cast<int64_t>((static_cast<uint64_t>(1) << bits) - 1);
+    int64_t min = -(static_cast<int64_t>(1) << (bits - 1));
+    int64_t as_signed = static_cast<int64_t>(val);
+    return as_signed > max || as_signed < min;
+  }
+
+  // Select bits from A and B using bits in MASK.  For each n in
+  // [0..31], the n-th bit in the result is chosen from the n-th bits
+  // of A and B.  A zero selects A and a one selects B.
+  static inline uint64_t
+  bit_select64(uint64_t a, uint64_t b, uint64_t mask)
+  { return (a & ~mask) | (b & mask); }
+};
+
 // Track relocations while reading a section.  This lets you ask for
 // the relocation at a certain offset, and see how relocs occur
 // between points of interest.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Initial MIPS patch for GOLD - version 3
  2012-01-24 15:39 Aleksandar Simeonov
@ 2012-01-28  1:06 ` Ian Lance Taylor
  2012-01-28  1:48 ` Ian Lance Taylor
  2012-01-28  1:55 ` Ian Lance Taylor
  2 siblings, 0 replies; 10+ messages in thread
From: Ian Lance Taylor @ 2012-01-28  1:06 UTC (permalink / raw)
  To: Aleksandar Simeonov; +Cc: Fuhler, Rich, mips-compiler, binutils

Aleksandar Simeonov <Aleksandar.Simeonov@RT-RK.com> writes:

> Please find patch for MIPS version of Gold.

I've committed the patches to elfcpp.  Those are straightforward and it
means that we don't have to worry about them any further.

Thanks.

(Thanks for including ChangeLog entries, but in the future please just
put them in the text of the e-mail, not in the patch.  It's the nature
of ChangeLog files that patches to them rarely apply cleanly.)

Ian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Initial MIPS patch for GOLD - version 3
@ 2012-01-24 15:39 Aleksandar Simeonov
  2012-01-28  1:06 ` Ian Lance Taylor
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Aleksandar Simeonov @ 2012-01-24 15:39 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: Fuhler, Rich, mips-compiler, binutils

[-- Attachment #1: Type: text/plain, Size: 4557 bytes --]

Hi Ian,
Please find patch for MIPS version of Gold. It still contains just
initial version of code for MIPS, but this time I added all changes that
are done in common code. Since those changes have effects on all
architectures I would like to ask you to check if you approve them, and
if not, to give some suggestions how to change them.

List of changes in common code:
* parameters.cc (Parameters::entry) : Default entry symbol for MIPS is
"__start".
- Unlike other architectures, default entry symbol for MIPS is "__start",
not "_start".

* output.cc (Output_file_header::entry) : Start address is not defaulting
to zero but rather to starting address of .text section (if exists).
* target.h (Target::text_section_address) : New function.
- If there are no start symbol or start address is not set from command
line, MIPS linker has to set start of .text section, not 0 (if section
exists).

* output.h (Output_data_reloc::add_absolute_null) : New function. Needed
for generation of R_MIPS_NONE relocation.
* output.cc (Output_reloc::compare) :  R_MIPS_NONE relocation have to be
first.
- First dynamic relocation in MIPS executable or shared object has to be
null relocation (absolute R_MIPS_NONE type to address 0). Function for
creating that entry is added and there were changes in function for
sorting dynamic relocations in order to have this relocation first.

* symtab.h (Symbol::set_nonvis) : New function. Set the non-visibility
part of the st_other field.
- Needed to set STO_MIPS_PLT flag that represents undefined functions (in
file) that are PLT entries.

* layout.cc (Layout::finalize) : Added additional fixing of section data
after all necessary information are known (for MIPS data about dynamic
symbol table are needed).
* target.h (Target::fix_sections) : New function.
* target.h (Target::do_fix_sections) : New function.
- If there are more then 65536 entries in dynamic symbol table entries in
.MIPS.stubs section have size of 5 32-bit words (4 32-bits words is size
when there are less or equal then 65536 entries). Since this value is only
known at the end of Layout::finalize function, additional fixing of data
is needed at the end of that function.

* symtab.cc (got_offset_compare) : New function.
* symtab.cc (Symbol_table::set_dynsym_indexes) : Order of symbols in
dynamic symbol table changed. New order is local symbols, global symbols
without got entry, global symbols with got entry.
- MIPS has strict order of entries in dynamic symbol table. First come
local symbols, then global symbols that has no entry in got, then global
symbols that has got entry.

* symtab.h (Symbol_table::global_got_index_): New data member.
* symtab.h (Symbol_table::global_got_index): New function.
- First entry in dynamic symbol table that has got entry. Needed for
.dynamic section tag (DT_MIPS_GOTSYM).

* utils.h: New file that contains utilities for manipulating integers of
up to 32-bits.
* arm.cc (namespace utils): Removed common code to separate file (utils.h).
* arm.cc (utils::sign_extend): Removed.
* arm.cc (utils::has_overflow): Likewise.
* arm.cc (utils::has_signed_unsigned_overflow): Likewise.
* arm.cc (utils::bit_select): Likewise.
- Functions that are common for MIPS and ARM architectures moved to
separate file to avoid duplication of code.

* output.cc (Output_data_dynamic::Dynamic_entry::write): Added code for
target specific dynamic tags.
* output.h (Output_data_dynamic::add_target_specific): Added function for
adding target specific dynamic tag.
* output.h (Output_data_dynamic::add_target_specific): New method.
* output.h (Output_data_dynamic::Dynamic_entry): Add support for target
specific dynamic table entries.
* output.h (DYNAMIC_TARGET): New enum for target specific dynamic tags.
- MIPS has several dynamic tags that are mandatory. This code is added to
support them.

* layout.cc (Layout::segment_precedes): Fixed order of MIPS specific
segments.
- MIPS needs to have PT_MIPS_REGINFO segment before any loadable segment.
- MIPS needs to have PT_NULL segment to be last in list of segments.

* layout.h (Layout::segment_list): Returns segment list.
* layout.h (Layout::segment_list): New function.
- MIPS dynamic tag DT_MIPS_BASE_ADDRESS needs virtual address of first
loadable segment.

* reloc.cc (Sized_relobj_file<size, big_endian>::write_sections): Special
handling of MIPS .reginfo section.
- .reginfo section is generated by linker and needs special handling.

Rest of the changes are or in mips specific part or in make and configure
files (new target is added).

Best regards,
Aleksandar


[-- Attachment #2: mips_gold.tar.gz --]
[-- Type: application/gzip, Size: 30252 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-03-07 19:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-03 15:40 Initial MIPS patch for GOLD - version 3 Simeonov, Aleksandar (c)
2012-03-07 14:26 ` Simeonov, Aleksandar (c)
2012-03-07 14:54   ` Ian Lance Taylor
2012-03-07 19:46     ` Fuhler, Rich
  -- strict thread matches above, loose matches on Subject: below --
2012-01-24 15:39 Aleksandar Simeonov
2012-01-28  1:06 ` Ian Lance Taylor
2012-01-28  1:48 ` Ian Lance Taylor
2012-01-28  1:55 ` Ian Lance Taylor
2012-01-28  2:11   ` John Reiser
2012-01-29 12:13     ` Richard Sandiford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).