public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] LoongArch: Allow la.got -> la.pcrel relaxation for shared object
@ 2023-12-05 19:05 Xi Ruoyao
  2023-12-15 12:22 ` Ping: " Xi Ruoyao
  0 siblings, 1 reply; 3+ messages in thread
From: Xi Ruoyao @ 2023-12-05 19:05 UTC (permalink / raw)
  To: binutils
  Cc: liuzhensong, mengqinggang, i.swmail, maskray, hejinyang, cailulu,
	Xi Ruoyao

Even in shared objects, la.got -> la.pcrel relaxation can still be
performed for symbols with hidden visibility. For example, if a.c is:

    extern int x;
    int f() { return x++; }

and b.c is:

    int x = 114514;

If compiling and linking with:

    gcc -shared -fPIC -O2 -fvisibility=hidden a.c b.c

Then the la.got in a.o should be relaxed to la.pcrel, and the resulted f
should be like:

    pcaddi  $t0, x
    ldptr.w $a0, $t0, 0
    addi.w  $t1, $a0, 1
    stptr.w $t1, $t0, 0
    ret

Remove bfd_link_executable from the condition of la.got -> la.pcrel
relaxation so this will really happen.  The SYMBOL_REFERENCES_LOCAL
check is enough not to wrongly relax preemptable symbols (for e.g.
when -fvisibility=hidden is not used).

Note that on x86_64 this is also relaxed and the produced code is like:

    lea x(%rip), %rdx
    mov (%rdx), %rax
    lea 1(%rax), %ecx
    mov %ecx, (%rdx)
    ret

Tested by running ld test suite, bootstrapping and regtesting GCC with
the patched ld, and building and testing Glibc with the patched ld.  No
regression is observed.

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
 bfd/elfnn-loongarch.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index 024c5d4cd96..80739b817ab 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -3986,8 +3986,7 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
 	  else
 	    continue;
 
-	  if (h && bfd_link_executable (info)
-	      && SYMBOL_REFERENCES_LOCAL (info, h))
+	  if (h && SYMBOL_REFERENCES_LOCAL (info, h))
 	    local_got = true;
 	  symtype = h->type;
 	}
-- 
2.43.0


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

* Ping: [PATCH] LoongArch: Allow la.got -> la.pcrel relaxation for shared object
  2023-12-05 19:05 [PATCH] LoongArch: Allow la.got -> la.pcrel relaxation for shared object Xi Ruoyao
@ 2023-12-15 12:22 ` Xi Ruoyao
  2023-12-19  7:48   ` mengqinggang
  0 siblings, 1 reply; 3+ messages in thread
From: Xi Ruoyao @ 2023-12-15 12:22 UTC (permalink / raw)
  To: binutils; +Cc: liuzhensong, mengqinggang, i.swmail, maskray, hejinyang, cailulu

Ping.

On Wed, 2023-12-06 at 03:05 +0800, Xi Ruoyao wrote:
> Even in shared objects, la.got -> la.pcrel relaxation can still be
> performed for symbols with hidden visibility. For example, if a.c is:
> 
>     extern int x;
>     int f() { return x++; }
> 
> and b.c is:
> 
>     int x = 114514;
> 
> If compiling and linking with:
> 
>     gcc -shared -fPIC -O2 -fvisibility=hidden a.c b.c
> 
> Then the la.got in a.o should be relaxed to la.pcrel, and the resulted f
> should be like:
> 
>     pcaddi  $t0, x
>     ldptr.w $a0, $t0, 0
>     addi.w  $t1, $a0, 1
>     stptr.w $t1, $t0, 0
>     ret
> 
> Remove bfd_link_executable from the condition of la.got -> la.pcrel
> relaxation so this will really happen.  The SYMBOL_REFERENCES_LOCAL
> check is enough not to wrongly relax preemptable symbols (for e.g.
> when -fvisibility=hidden is not used).
> 
> Note that on x86_64 this is also relaxed and the produced code is like:
> 
>     lea x(%rip), %rdx
>     mov (%rdx), %rax
>     lea 1(%rax), %ecx
>     mov %ecx, (%rdx)
>     ret
> 
> Tested by running ld test suite, bootstrapping and regtesting GCC with
> the patched ld, and building and testing Glibc with the patched ld.  No
> regression is observed.
> 
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
>  bfd/elfnn-loongarch.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index 024c5d4cd96..80739b817ab 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -3986,8 +3986,7 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
>  	  else
>  	    continue;
>  
> -	  if (h && bfd_link_executable (info)
> -	      && SYMBOL_REFERENCES_LOCAL (info, h))
> +	  if (h && SYMBOL_REFERENCES_LOCAL (info, h))
>  	    local_got = true;
>  	  symtype = h->type;
>  	}

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

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

* Re: Ping: [PATCH] LoongArch: Allow la.got -> la.pcrel relaxation for shared object
  2023-12-15 12:22 ` Ping: " Xi Ruoyao
@ 2023-12-19  7:48   ` mengqinggang
  0 siblings, 0 replies; 3+ messages in thread
From: mengqinggang @ 2023-12-19  7:48 UTC (permalink / raw)
  To: Xi Ruoyao, binutils; +Cc: liuzhensong, i.swmail, maskray, hejinyang, cailulu

This patch have been merged. Thank you very much.


在 2023/12/15 下午8:22, Xi Ruoyao 写道:
> Ping.
>
> On Wed, 2023-12-06 at 03:05 +0800, Xi Ruoyao wrote:
>> Even in shared objects, la.got -> la.pcrel relaxation can still be
>> performed for symbols with hidden visibility. For example, if a.c is:
>>
>>      extern int x;
>>      int f() { return x++; }
>>
>> and b.c is:
>>
>>      int x = 114514;
>>
>> If compiling and linking with:
>>
>>      gcc -shared -fPIC -O2 -fvisibility=hidden a.c b.c
>>
>> Then the la.got in a.o should be relaxed to la.pcrel, and the resulted f
>> should be like:
>>
>>      pcaddi  $t0, x
>>      ldptr.w $a0, $t0, 0
>>      addi.w  $t1, $a0, 1
>>      stptr.w $t1, $t0, 0
>>      ret
>>
>> Remove bfd_link_executable from the condition of la.got -> la.pcrel
>> relaxation so this will really happen.  The SYMBOL_REFERENCES_LOCAL
>> check is enough not to wrongly relax preemptable symbols (for e.g.
>> when -fvisibility=hidden is not used).
>>
>> Note that on x86_64 this is also relaxed and the produced code is like:
>>
>>      lea x(%rip), %rdx
>>      mov (%rdx), %rax
>>      lea 1(%rax), %ecx
>>      mov %ecx, (%rdx)
>>      ret
>>
>> Tested by running ld test suite, bootstrapping and regtesting GCC with
>> the patched ld, and building and testing Glibc with the patched ld.  No
>> regression is observed.
>>
>> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
>> ---
>>   bfd/elfnn-loongarch.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
>> index 024c5d4cd96..80739b817ab 100644
>> --- a/bfd/elfnn-loongarch.c
>> +++ b/bfd/elfnn-loongarch.c
>> @@ -3986,8 +3986,7 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
>>   	  else
>>   	    continue;
>>   
>> -	  if (h && bfd_link_executable (info)
>> -	      && SYMBOL_REFERENCES_LOCAL (info, h))
>> +	  if (h && SYMBOL_REFERENCES_LOCAL (info, h))
>>   	    local_got = true;
>>   	  symtype = h->type;
>>   	}


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

end of thread, other threads:[~2023-12-19  7:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-05 19:05 [PATCH] LoongArch: Allow la.got -> la.pcrel relaxation for shared object Xi Ruoyao
2023-12-15 12:22 ` Ping: " Xi Ruoyao
2023-12-19  7:48   ` mengqinggang

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).