Discussion:
[PATCH]: Sparc GOTDATA optimizations.
David Miller
2010-02-09 19:36:08 UTC
Permalink
This implements GOTDATA relocation optimizations in
the BFD sparc backend.

Essentially this turns a sequence of the form:

sethi %gdop_hix22(local_sym), %l1
xor %l1, %gdop_lox10(local_sym), %l1
ld [%l7 + %l1], %i0, %gdop(local_sym)

into:

sethi %hi22(local_sym - _GLOBAL_OFFSET_TABLE_), %l1
xor %l1, %lox10(local_sym - _GLOBAL_OFFSET_TABLE_), %l1
add %l7, %l1, %i0

when 'local_sym' is "local", which specifically exludes
functions marked as protected.

It's similar to @GOTOFF on i386, but because the linker makes the
decision instead of the compiler, it can be applied to more cases.

I have a GCC patch which I'll be posting which makes the compiler
start to emit these relocations. Using that combined toolchain I've
done extensive testing with glibc on both 32-bit and 64-bit.

No new testsuite regressions in glibc and binutils after these
changes, even with a gcc which emits the relocs.

Ok to commit?

bfd/

2010-02-07 David S. Miller <***@davemloft.net>

* elfxx-sparc.c (_bfd_sparc_elf_check_relocs): For R_SPARC_GOTDATA_OP_HIX22
and R_SPARC_GOTDATA_OP_LOX10, only bump the GOT refcount for global
symbols.
(_bfd_sparc_elf_gc_sweep_hook): Likewise only decrement the GOT count for
these relocs on global symbols.
(gdopoff): New.
(_bfd_sparc_elf_relocate_section): Perform GOTDATA optimizations on
local symbol references which are not STT_GNU_IFUNC. Handle
relocation of them like R_SPARC_HIX22 and R_SPARC_LOX10 respectively,
and deal with negative vs. non-negative values properly.

ld/testsuite

* ld-sparc/gotop32.s: Add local symbol case.
* ld-sparc/gotop64.s: Likewise.
* ld-sparc/gotop32.rd: Adjust expected results.
* ld-sparc/gotop32.td: Likewise.
* ld-sparc/gotop64.dd: Likewise.
* ld-sparc/gotop64.rd: Likewise.
* ld-sparc/gotop64.td: Likewise.
---
bfd/elfxx-sparc.c | 111 +++++++++++++++++++++++++++++---------
ld/testsuite/ld-sparc/gotop32.dd | 12 +++-
ld/testsuite/ld-sparc/gotop32.rd | 1 +
ld/testsuite/ld-sparc/gotop32.s | 9 +++
ld/testsuite/ld-sparc/gotop32.td | 2 +-
ld/testsuite/ld-sparc/gotop64.dd | 12 +++-
ld/testsuite/ld-sparc/gotop64.rd | 1 +
ld/testsuite/ld-sparc/gotop64.s | 9 +++
ld/testsuite/ld-sparc/gotop64.td | 2 +-
9 files changed, 125 insertions(+), 34 deletions(-)

diff --git a/bfd/elfxx-sparc.c b/bfd/elfxx-sparc.c
index bc36920..1947d1a 100644
--- a/bfd/elfxx-sparc.c
+++ b/bfd/elfxx-sparc.c
@@ -1345,8 +1345,6 @@ _bfd_sparc_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
case R_SPARC_GOT10:
case R_SPARC_GOT13:
case R_SPARC_GOT22:
- case R_SPARC_GOTDATA_HIX22:
- case R_SPARC_GOTDATA_LOX10:
case R_SPARC_GOTDATA_OP_HIX22:
case R_SPARC_GOTDATA_OP_LOX10:
tls_type = GOT_NORMAL;
@@ -1386,7 +1384,16 @@ _bfd_sparc_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
_bfd_sparc_elf_local_got_tls_type (abfd)
= (char *) (local_got_refcounts + symtab_hdr->sh_info);
}
- local_got_refcounts[r_symndx] += 1;
+ switch (r_type)
+ {
+ case R_SPARC_GOTDATA_OP_HIX22:
+ case R_SPARC_GOTDATA_OP_LOX10:
+ break;
+
+ default:
+ local_got_refcounts[r_symndx] += 1;
+ break;
+ }
old_tls_type = _bfd_sparc_elf_local_got_tls_type (abfd) [r_symndx];
}

@@ -1766,8 +1773,17 @@ _bfd_sparc_elf_gc_sweep_hook (bfd *abfd, struct bfd_link_info *info,
}
else
{
- if (local_got_refcounts[r_symndx] > 0)
- local_got_refcounts[r_symndx]--;
+ switch (r_type)
+ {
+ case R_SPARC_GOTDATA_OP_HIX22:
+ case R_SPARC_GOTDATA_OP_LOX10:
+ break;
+
+ default:
+ if (local_got_refcounts[r_symndx] > 0)
+ local_got_refcounts[r_symndx]--;
+ break;
+ }
}
break;

@@ -2672,6 +2688,21 @@ tpoff (struct bfd_link_info *info, bfd_vma address)
return address - htab->tls_size - htab->tls_sec->vma;
}

+/* Return the relocation value for a %gdop relocation. */
+
+static bfd_vma
+gdopoff (struct bfd_link_info *info, bfd_vma address)
+{
+ struct elf_link_hash_table *htab = elf_hash_table (info);
+ bfd_vma got_base;
+
+ got_base = (htab->hgot->root.u.def.value
+ + htab->hgot->root.u.def.section->output_offset
+ + htab->hgot->root.u.def.section->output_section->vma);
+
+ return address - got_base;
+}
+
/* Relocate a SPARC ELF section. */

bfd_boolean
@@ -2821,10 +2852,17 @@ _bfd_sparc_elf_relocate_section (bfd *output_bfd,

switch (r_type)
{
- case R_SPARC_GOTDATA_HIX22:
- case R_SPARC_GOTDATA_LOX10:
+ case R_SPARC_GOTDATA_OP:
+ continue;
+
case R_SPARC_GOTDATA_OP_HIX22:
case R_SPARC_GOTDATA_OP_LOX10:
+ r_type = (r_type == R_SPARC_GOTDATA_OP_HIX22
+ ? R_SPARC_GOT22
+ : R_SPARC_GOT10);
+ howto = _bfd_sparc_elf_howto_table + r_type;
+ /* Fall through. */
+
case R_SPARC_GOT10:
case R_SPARC_GOT13:
case R_SPARC_GOT22:
@@ -2911,19 +2949,37 @@ _bfd_sparc_elf_relocate_section (bfd *output_bfd,

switch (r_type)
{
- case R_SPARC_GOTDATA_HIX22:
- case R_SPARC_GOTDATA_LOX10:
case R_SPARC_GOTDATA_OP_HIX22:
case R_SPARC_GOTDATA_OP_LOX10:
- /* We don't support these code transformation optimizations
- yet, so just leave the sequence alone and treat as
- GOT22/GOT10. */
- if (r_type == R_SPARC_GOTDATA_HIX22
- || r_type == R_SPARC_GOTDATA_OP_HIX22)
- r_type = R_SPARC_GOT22;
+ if (SYMBOL_REFERENCES_LOCAL (info, h))
+ r_type = (r_type == R_SPARC_GOTDATA_OP_HIX22
+ ? R_SPARC_GOTDATA_HIX22
+ : R_SPARC_GOTDATA_LOX10);
else
- r_type = R_SPARC_GOT10;
- /* Fall through. */
+ r_type = (r_type == R_SPARC_GOTDATA_OP_HIX22
+ ? R_SPARC_GOT22
+ : R_SPARC_GOT10);
+ howto = _bfd_sparc_elf_howto_table + r_type;
+ break;
+
+ case R_SPARC_GOTDATA_OP:
+ if (SYMBOL_REFERENCES_LOCAL (info, h))
+ {
+ bfd_vma insn = bfd_get_32 (input_bfd, contents + rel->r_offset);
+
+ /* {ld,ldx} [%rs1 + %rs2], %rd --> add %rs1, %rs2, %rd */
+ relocation = 0x80000000 | (insn & 0x3e07c01f);
+ bfd_put_32 (output_bfd, relocation, contents + rel->r_offset);
+ }
+ continue;
+ }
+
+ switch (r_type)
+ {
+ case R_SPARC_GOTDATA_HIX22:
+ case R_SPARC_GOTDATA_LOX10:
+ relocation = gdopoff (info, relocation);
+ break;

case R_SPARC_GOT10:
case R_SPARC_GOT13:
@@ -3576,11 +3632,6 @@ _bfd_sparc_elf_relocate_section (bfd *output_bfd,
}
continue;

- case R_SPARC_GOTDATA_OP:
- /* We don't support gotdata code transformation optimizations
- yet, so simply leave the sequence as-is. */
- continue;
-
case R_SPARC_TLS_IE_LD:
case R_SPARC_TLS_IE_LDX:
if (! info->shared && (h == NULL || h->dynindx == -1))
@@ -3704,12 +3755,15 @@ _bfd_sparc_elf_relocate_section (bfd *output_bfd,

r = bfd_reloc_ok;
}
- else if (r_type == R_SPARC_HIX22)
+ else if (r_type == R_SPARC_HIX22
+ || r_type == R_SPARC_GOTDATA_HIX22)
{
bfd_vma x;

relocation += rel->r_addend;
- relocation = relocation ^ MINUS_ONE;
+ if (r_type == R_SPARC_HIX22
+ || (bfd_signed_vma) relocation < 0)
+ relocation = relocation ^ MINUS_ONE;

x = bfd_get_32 (input_bfd, contents + rel->r_offset);
x = (x & ~(bfd_vma) 0x3fffff) | ((relocation >> 10) & 0x3fffff);
@@ -3720,12 +3774,17 @@ _bfd_sparc_elf_relocate_section (bfd *output_bfd,
bfd_arch_bits_per_address (input_bfd),
relocation);
}
- else if (r_type == R_SPARC_LOX10)
+ else if (r_type == R_SPARC_LOX10
+ || r_type == R_SPARC_GOTDATA_LOX10)
{
bfd_vma x;

relocation += rel->r_addend;
- relocation = (relocation & 0x3ff) | 0x1c00;
+ if (r_type == R_SPARC_LOX10
+ || (bfd_signed_vma) relocation < 0)
+ relocation = (relocation & 0x3ff) | 0x1c00;
+ else
+ relocation = (relocation & 0x3ff);

x = bfd_get_32 (input_bfd, contents + rel->r_offset);
x = (x & ~(bfd_vma) 0x1fff) | relocation;
diff --git a/ld/testsuite/ld-sparc/gotop32.dd b/ld/testsuite/ld-sparc/gotop32.dd
index 9f6b1f6..a599930 100644
--- a/ld/testsuite/ld-sparc/gotop32.dd
+++ b/ld/testsuite/ld-sparc/gotop32.dd
@@ -17,12 +17,18 @@ Disassembly of section .text:
+1010: 7f ff ff fc call 1000 <_.*>
+1014: ae 05 e0 60 add %l7, 0x60, %l7 ! 11060 <.*>
+1018: 01 00 00 00 nop *
- +101c: 23 00 00 04 sethi %hi\(0x1000\), %l1
+ +101c: 23 00 00 00 sethi %hi\(0\), %l1
+1020: 01 00 00 00 nop *
+1024: a2 1c 60 04 xor %l1, 4, %l1
+1028: 01 00 00 00 nop *
+102c: f0 05 c0 11 ld \[ %l7 \+ %l1 \], %i0
+1030: 01 00 00 00 nop *
- +1034: 81 c7 e0 08 ret
- +1038: 81 e8 00 00 restore
+ +1034: 23 00 00 03 sethi %hi\(0xc00\), %l1
+ +1038: 01 00 00 00 nop
+ +103c: a2 1c 63 94 xor %l1, 0x394, %l1
+ +1040: 01 00 00 00 nop
+ +1044: b0 05 c0 11 add %l7, %l1, %i0
+ +1048: 01 00 00 00 nop
+ +104c: 81 c7 e0 08 ret
+ +1050: 81 e8 00 00 restore
#pass
diff --git a/ld/testsuite/ld-sparc/gotop32.rd b/ld/testsuite/ld-sparc/gotop32.rd
index 566066c..1cecad8 100644
--- a/ld/testsuite/ld-sparc/gotop32.rd
+++ b/ld/testsuite/ld-sparc/gotop32.rd
@@ -59,6 +59,7 @@ Symbol table '\.symtab' contains [0-9]+ entries:
.* SECTION +LOCAL +DEFAULT +6 *
.* SECTION +LOCAL +DEFAULT +7 *
.* SECTION +LOCAL +DEFAULT +8 *
+.* NOTYPE +LOCAL +DEFAULT +8 local_sym
.* OBJECT +LOCAL +DEFAULT +ABS _DYNAMIC
.* OBJECT +LOCAL +DEFAULT +ABS _PROCEDURE_LINKAGE_TABLE_
.* OBJECT +LOCAL +DEFAULT +ABS _GLOBAL_OFFSET_TABLE_
diff --git a/ld/testsuite/ld-sparc/gotop32.s b/ld/testsuite/ld-sparc/gotop32.s
index ac01d6f..604694c 100644
--- a/ld/testsuite/ld-sparc/gotop32.s
+++ b/ld/testsuite/ld-sparc/gotop32.s
@@ -3,6 +3,9 @@
.globl sym
sym: .word 0x12345678

+local_sym:
+ .word 0xdeadbeef
+
.text
.align 4096
.LLGETPC0:
@@ -24,5 +27,11 @@ foo:
nop
ld [%l7 + %l1], %i0, %gdop(sym)
nop
+ sethi %gdop_hix22(local_sym), %l1
+ nop
+ xor %l1, %gdop_lox10(local_sym), %l1
+ nop
+ ld [%l7 + %l1], %i0, %gdop(local_sym)
+ nop
ret
restore
diff --git a/ld/testsuite/ld-sparc/gotop32.td b/ld/testsuite/ld-sparc/gotop32.td
index e73482d..520788b 100644
--- a/ld/testsuite/ld-sparc/gotop32.td
+++ b/ld/testsuite/ld-sparc/gotop32.td
@@ -7,6 +7,6 @@
.*: +file format elf32-sparc

Contents of section .data:
- 13000 12345678 00000000 00000000 00000000 .*
+ 13000 12345678 deadbeef 00000000 00000000 .*
13010 00000000 00000000 00000000 00000000 .*
#pass
diff --git a/ld/testsuite/ld-sparc/gotop64.dd b/ld/testsuite/ld-sparc/gotop64.dd
index a78f55a..d73fb18 100644
--- a/ld/testsuite/ld-sparc/gotop64.dd
+++ b/ld/testsuite/ld-sparc/gotop64.dd
@@ -17,12 +17,18 @@ Disassembly of section .text:
+1010: 7f ff ff fc call 1000 <_.*>
+1014: ae 05 e0 d0 add %l7, 0xd0, %l7 ! 1010d0 <.*>
+1018: 01 00 00 00 nop *
- +101c: 23 00 00 08 sethi %hi\(0x2000\), %l1
+ +101c: 23 00 00 00 sethi %hi\(0\), %l1
+1020: 01 00 00 00 nop *
+1024: a2 1c 60 08 xor %l1, 8, %l1
+1028: 01 00 00 00 nop *
+102c: f0 5d c0 11 ldx \[ %l7 \+ %l1 \], %i0
+1030: 01 00 00 00 nop *
- +1034: 81 c7 e0 08 ret
- +1038: 81 e8 00 00 restore
+ +1034: 23 00 00 03 sethi %hi\(0xc00\), %l1
+ +1038: 01 00 00 00 nop *
+ +103c: a2 1c 63 24 xor %l1, 0x324, %l1
+ +1040: 01 00 00 00 nop *
+ +1044: b0 05 c0 11 add %l7, %l1, %i0
+ +1048: 01 00 00 00 nop *
+ +104c: 81 c7 e0 08 ret
+ +1050: 81 e8 00 00 restore
#pass
diff --git a/ld/testsuite/ld-sparc/gotop64.rd b/ld/testsuite/ld-sparc/gotop64.rd
index 4d3e519..509c8f8 100644
--- a/ld/testsuite/ld-sparc/gotop64.rd
+++ b/ld/testsuite/ld-sparc/gotop64.rd
@@ -59,6 +59,7 @@ Symbol table '\.symtab' contains [0-9]+ entries:
.* SECTION +LOCAL +DEFAULT +6 *
.* SECTION +LOCAL +DEFAULT +7 *
.* SECTION +LOCAL +DEFAULT +8 *
+.* NOTYPE +LOCAL +DEFAULT +8 local_sym
.* OBJECT +LOCAL +DEFAULT +ABS _DYNAMIC
.* OBJECT +LOCAL +DEFAULT +ABS _PROCEDURE_LINKAGE_TABLE_
.* OBJECT +LOCAL +DEFAULT +ABS _GLOBAL_OFFSET_TABLE_
diff --git a/ld/testsuite/ld-sparc/gotop64.s b/ld/testsuite/ld-sparc/gotop64.s
index 8a8ff82..9910813 100644
--- a/ld/testsuite/ld-sparc/gotop64.s
+++ b/ld/testsuite/ld-sparc/gotop64.s
@@ -3,6 +3,9 @@
.globl sym
sym: .word 0x12345678

+local_sym:
+ .word 0xdeadbeef
+
.text
.align 4096
.LLGETPC0:
@@ -24,5 +27,11 @@ foo:
nop
ldx [%l7 + %l1], %i0, %gdop(sym)
nop
+ sethi %gdop_hix22(local_sym), %l1
+ nop
+ xor %l1, %gdop_lox10(local_sym), %l1
+ nop
+ ldx [%l7 + %l1], %i0, %gdop(local_sym)
+ nop
ret
restore
diff --git a/ld/testsuite/ld-sparc/gotop64.td b/ld/testsuite/ld-sparc/gotop64.td
index f16cf50..28d40ed 100644
--- a/ld/testsuite/ld-sparc/gotop64.td
+++ b/ld/testsuite/ld-sparc/gotop64.td
@@ -7,6 +7,6 @@
.*: +file format elf64-sparc

Contents of section .data:
- 103000 12345678 00000000 00000000 00000000 .*
+ 103000 12345678 deadbeef 00000000 00000000 .*
103010 00000000 00000000 00000000 00000000 .*
#pass
--
1.6.6.1
Nick Clifton
2010-02-11 12:32:46 UTC
Permalink
Hi David,
Post by David Miller
bfd/
* elfxx-sparc.c (_bfd_sparc_elf_check_relocs): For R_SPARC_GOTDATA_OP_HIX22
and R_SPARC_GOTDATA_OP_LOX10, only bump the GOT refcount for global
symbols.
(_bfd_sparc_elf_gc_sweep_hook): Likewise only decrement the GOT count for
these relocs on global symbols.
(gdopoff): New.
(_bfd_sparc_elf_relocate_section): Perform GOTDATA optimizations on
local symbol references which are not STT_GNU_IFUNC. Handle
relocation of them like R_SPARC_HIX22 and R_SPARC_LOX10 respectively,
and deal with negative vs. non-negative values properly.
ld/testsuite
* ld-sparc/gotop32.s: Add local symbol case.
* ld-sparc/gotop64.s: Likewise.
* ld-sparc/gotop32.rd: Adjust expected results.
* ld-sparc/gotop32.td: Likewise.
* ld-sparc/gotop64.dd: Likewise.
* ld-sparc/gotop64.rd: Likewise.
* ld-sparc/gotop64.td: Likewise.
Approved - please apply.

Note - there appear to be a few, very minor, formatting issues with the
patch, where a whitespace character and an operator symbol appear to
Post by David Miller
+ if (local_got_refcounts[r_symndx]> 0)
+ if (local_got_refcounts[r_symndx] > 0)
+ || (bfd_signed_vma) relocation< 0)
+ || (bfd_signed_vma) relocation< 0)
+ relocation = (relocation& 0x3ff) | 0x1c00;
+ relocation = (relocation& 0x3ff);
Cheers
Nick
David Miller
2010-02-11 19:31:05 UTC
Permalink
From: Nick Clifton <***@redhat.com>
Date: Thu, 11 Feb 2010 12:32:46 +0000
Post by Nick Clifton
Approved - please apply.
Thanks for reviewing.
Post by Nick Clifton
Note - there appear to be a few, very minor, formatting issues with
the patch, where a whitespace character and an operator symbol appear
I'll fix these up, thanks for noticing.
David Miller
2010-02-11 19:34:53 UTC
Permalink
From: Nick Clifton <***@redhat.com>
Date: Thu, 11 Feb 2010 12:32:46 +0000
Post by Nick Clifton
Note - there appear to be a few, very minor, formatting issues with
the patch, where a whitespace character and an operator symbol appear
Post by David Miller
+ if (local_got_refcounts[r_symndx]> 0)
+ if (local_got_refcounts[r_symndx] > 0)
Nick, I took a second look and I do not see this in my copy
of the patch posting.

Perhaps your email client or similar did something with the
formatting?

Even in the archives:

http://sourceware.org/ml/binutils/2010-02/msg00176.html

The spacing is there on either side of the ">"

Similarly for the other spacing issues you mentioned.

Loading...