* [PATCH] LoongArch: Use tab to indent assembly in TLSDESC test suite
2023-12-28 14:58 [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE Tatsuyuki Ishi
@ 2023-12-28 14:58 ` Tatsuyuki Ishi
2024-01-22 9:47 ` mengqinggang
2023-12-28 14:58 ` [PATCH] LoongArch: Update comment about bottom bit usage in TLS GOT construction Tatsuyuki Ishi
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Tatsuyuki Ishi @ 2023-12-28 14:58 UTC (permalink / raw)
To: binutils
Cc: cailulu, chenglulu, hejinyang, i.swmail, liuzhensong, luweining,
maskray, mengqinggang, nickc, wanglei, xry111, xuchenghua,
Tatsuyuki Ishi
The usual convention is to use tabs. Not all test are following this,
but at least when using tabs, let's use it consistently throughout the
file.
---
gas/testsuite/gas/loongarch/tlsdesc_32.s | 2 +-
gas/testsuite/gas/loongarch/tlsdesc_64.s | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/gas/testsuite/gas/loongarch/tlsdesc_32.s b/gas/testsuite/gas/loongarch/tlsdesc_32.s
index ef6aee94fbb..2a139c041b1 100644
--- a/gas/testsuite/gas/loongarch/tlsdesc_32.s
+++ b/gas/testsuite/gas/loongarch/tlsdesc_32.s
@@ -4,7 +4,7 @@
# R_LARCH_TLS_DESC_PC_LO12 var
addi.w $a0,$a0,%desc_pc_lo12(var)
# R_LARCH_TLS_DESC_LD var
- ld.w $ra,$a0,%desc_ld(var)
+ ld.w $ra,$a0,%desc_ld(var)
# R_LARCH_TLS_DESC_CALL var
jirl $ra,$ra,%desc_call(var)
diff --git a/gas/testsuite/gas/loongarch/tlsdesc_64.s b/gas/testsuite/gas/loongarch/tlsdesc_64.s
index 9d0ccb170ad..9850940ef93 100644
--- a/gas/testsuite/gas/loongarch/tlsdesc_64.s
+++ b/gas/testsuite/gas/loongarch/tlsdesc_64.s
@@ -4,7 +4,7 @@
# R_LARCH_TLS_DESC_PC_LO12 var
addi.d $a0,$a0,%desc_pc_lo12(var)
# R_LARCH_TLS_DESC_LD var
- ld.d $ra,$a0,%desc_ld(var)
+ ld.d $ra,$a0,%desc_ld(var)
# R_LARCH_TLS_DESC_CALL var
jirl $ra,$ra,%desc_call(var)
--
2.40.1
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] LoongArch: Use tab to indent assembly in TLSDESC test suite
2023-12-28 14:58 ` [PATCH] LoongArch: Use tab to indent assembly in TLSDESC test suite Tatsuyuki Ishi
@ 2024-01-22 9:47 ` mengqinggang
0 siblings, 0 replies; 13+ messages in thread
From: mengqinggang @ 2024-01-22 9:47 UTC (permalink / raw)
To: Tatsuyuki Ishi, binutils
Cc: cailulu, chenglulu, hejinyang, i.swmail, liuzhensong, luweining,
maskray, nickc, wanglei, xry111, xuchenghua
Thank you very much, it has been applied.
在 2023/12/28 下午10:58, Tatsuyuki Ishi 写道:
> The usual convention is to use tabs. Not all test are following this,
> but at least when using tabs, let's use it consistently throughout the
> file.
> ---
> gas/testsuite/gas/loongarch/tlsdesc_32.s | 2 +-
> gas/testsuite/gas/loongarch/tlsdesc_64.s | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gas/testsuite/gas/loongarch/tlsdesc_32.s b/gas/testsuite/gas/loongarch/tlsdesc_32.s
> index ef6aee94fbb..2a139c041b1 100644
> --- a/gas/testsuite/gas/loongarch/tlsdesc_32.s
> +++ b/gas/testsuite/gas/loongarch/tlsdesc_32.s
> @@ -4,7 +4,7 @@
> # R_LARCH_TLS_DESC_PC_LO12 var
> addi.w $a0,$a0,%desc_pc_lo12(var)
> # R_LARCH_TLS_DESC_LD var
> - ld.w $ra,$a0,%desc_ld(var)
> + ld.w $ra,$a0,%desc_ld(var)
> # R_LARCH_TLS_DESC_CALL var
> jirl $ra,$ra,%desc_call(var)
>
> diff --git a/gas/testsuite/gas/loongarch/tlsdesc_64.s b/gas/testsuite/gas/loongarch/tlsdesc_64.s
> index 9d0ccb170ad..9850940ef93 100644
> --- a/gas/testsuite/gas/loongarch/tlsdesc_64.s
> +++ b/gas/testsuite/gas/loongarch/tlsdesc_64.s
> @@ -4,7 +4,7 @@
> # R_LARCH_TLS_DESC_PC_LO12 var
> addi.d $a0,$a0,%desc_pc_lo12(var)
> # R_LARCH_TLS_DESC_LD var
> - ld.d $ra,$a0,%desc_ld(var)
> + ld.d $ra,$a0,%desc_ld(var)
> # R_LARCH_TLS_DESC_CALL var
> jirl $ra,$ra,%desc_call(var)
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] LoongArch: Update comment about bottom bit usage in TLS GOT construction
2023-12-28 14:58 [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE Tatsuyuki Ishi
2023-12-28 14:58 ` [PATCH] LoongArch: Use tab to indent assembly in TLSDESC test suite Tatsuyuki Ishi
@ 2023-12-28 14:58 ` Tatsuyuki Ishi
2023-12-29 8:50 ` mengqinggang
2024-01-22 5:45 ` [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE Tatsuyuki Ishi
2024-01-22 9:46 ` mengqinggang
3 siblings, 1 reply; 13+ messages in thread
From: Tatsuyuki Ishi @ 2023-12-28 14:58 UTC (permalink / raw)
To: binutils
Cc: cailulu, chenglulu, hejinyang, i.swmail, liuzhensong, luweining,
maskray, mengqinggang, nickc, wanglei, xry111, xuchenghua,
Tatsuyuki Ishi
The GOT assignment logic, likely copied from another backend, does not in
fact require multiple flags to construct multiple type of slots. Instead,
all slots are initialized on the first relocation encountered. Update
comment to avoid confusion.
---
bfd/elfnn-loongarch.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index 64c34e99261..d66dcee1100 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -3656,16 +3656,13 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
relocation -= elf_hash_table (info)->tls_sec->vma;
break;
- /* TLS IE LD/GD process separately is troublesome.
- When a symbol is both ie and LD/GD, h->got.off |= 1
- make only one type be relocated. We must use
- h->got.offset |= 1 and h->got.offset |= 2
- diff IE and LD/GD. And all (got_off & (~(bfd_vma)1))
- (IE LD/GD and reusable GOT reloc) must change to
- (got_off & (~(bfd_vma)3)), beause we use lowest 2 bits
- as a tag.
- Now, LD and GD is both GOT_TLS_GD type, LD seems to
- can be omitted. */
+ /* The bottom bit of h->got.offset is (ab)used as a flag.
+ Upon encountering the first TLS relocation, we initialize all GOT
+ slots for the corresponding symbol and set the bottom bit to 1.
+
+ The second and subsequent relocations will check the flag, see that
+ the slot is already initialized, and move on to just relocating the
+ entry. */
case R_LARCH_TLS_IE_PC_HI20:
case R_LARCH_TLS_IE_HI20:
case R_LARCH_TLS_LD_PC_HI20:
--
2.40.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] LoongArch: Update comment about bottom bit usage in TLS GOT construction
2023-12-28 14:58 ` [PATCH] LoongArch: Update comment about bottom bit usage in TLS GOT construction Tatsuyuki Ishi
@ 2023-12-29 8:50 ` mengqinggang
2023-12-29 9:11 ` Tatsuyuki Ishi
0 siblings, 1 reply; 13+ messages in thread
From: mengqinggang @ 2023-12-29 8:50 UTC (permalink / raw)
To: Tatsuyuki Ishi, binutils
Cc: cailulu, chenglulu, hejinyang, i.swmail, liuzhensong, luweining,
maskray, nickc, wanglei, xry111, xuchenghua
At the beginning of implementation, we try to IE and GD only generate
their own GOT entry,
so we need two bits to do the flags.
在 2023/12/28 下午10:58, Tatsuyuki Ishi 写道:
> The GOT assignment logic, likely copied from another backend, does not in
> fact require multiple flags to construct multiple type of slots. Instead,
> all slots are initialized on the first relocation encountered. Update
> comment to avoid confusion.
> ---
> bfd/elfnn-loongarch.c | 17 +++++++----------
> 1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index 64c34e99261..d66dcee1100 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -3656,16 +3656,13 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
> relocation -= elf_hash_table (info)->tls_sec->vma;
> break;
>
> - /* TLS IE LD/GD process separately is troublesome.
> - When a symbol is both ie and LD/GD, h->got.off |= 1
> - make only one type be relocated. We must use
> - h->got.offset |= 1 and h->got.offset |= 2
> - diff IE and LD/GD. And all (got_off & (~(bfd_vma)1))
> - (IE LD/GD and reusable GOT reloc) must change to
> - (got_off & (~(bfd_vma)3)), beause we use lowest 2 bits
> - as a tag.
> - Now, LD and GD is both GOT_TLS_GD type, LD seems to
> - can be omitted. */
> + /* The bottom bit of h->got.offset is (ab)used as a flag.
> + Upon encountering the first TLS relocation, we initialize all GOT
> + slots for the corresponding symbol and set the bottom bit to 1.
> +
> + The second and subsequent relocations will check the flag, see that
> + the slot is already initialized, and move on to just relocating the
> + entry. */
> case R_LARCH_TLS_IE_PC_HI20:
> case R_LARCH_TLS_IE_HI20:
> case R_LARCH_TLS_LD_PC_HI20:
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] LoongArch: Update comment about bottom bit usage in TLS GOT construction
2023-12-29 8:50 ` mengqinggang
@ 2023-12-29 9:11 ` Tatsuyuki Ishi
2023-12-29 9:59 ` mengqinggang
0 siblings, 1 reply; 13+ messages in thread
From: Tatsuyuki Ishi @ 2023-12-29 9:11 UTC (permalink / raw)
To: mengqinggang
Cc: binutils, Lulu Cai, chenglulu, hejinyang, i.swmail, liuzhensong,
luweining, Fangrui Song, nickc, wanglei, xry111, xuchenghua
> On Dec 29, 2023, at 17:50, mengqinggang <mengqinggang@loongson.cn> wrote:
>
> At the beginning of implementation, we try to IE and GD only generate their own GOT entry,
> so we need two bits to do the flags.
I checked the tree back at the date the comment was introduced (6d13722a: "bfd: Add supported for LoongArch new relocations.”). Even at that time, there was no |= 2 accesses, so if my understanding is correct, this comment was wrong from the beginning.
Could you clarify what you mean here, in particular what “At the beginning of implementation” refer to?
> 在 2023/12/28 下午10:58, Tatsuyuki Ishi 写道:
>> The GOT assignment logic, likely copied from another backend, does not in
>> fact require multiple flags to construct multiple type of slots. Instead,
>> all slots are initialized on the first relocation encountered. Update
>> comment to avoid confusion.
>> ---
>> bfd/elfnn-loongarch.c | 17 +++++++----------
>> 1 file changed, 7 insertions(+), 10 deletions(-)
>>
>> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
>> index 64c34e99261..d66dcee1100 100644
>> --- a/bfd/elfnn-loongarch.c
>> +++ b/bfd/elfnn-loongarch.c
>> @@ -3656,16 +3656,13 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
>> relocation -= elf_hash_table (info)->tls_sec->vma;
>> break;
>> - /* TLS IE LD/GD process separately is troublesome.
>> - When a symbol is both ie and LD/GD, h->got.off |= 1
>> - make only one type be relocated. We must use
>> - h->got.offset |= 1 and h->got.offset |= 2
>> - diff IE and LD/GD. And all (got_off & (~(bfd_vma)1))
>> - (IE LD/GD and reusable GOT reloc) must change to
>> - (got_off & (~(bfd_vma)3)), beause we use lowest 2 bits
>> - as a tag.
>> - Now, LD and GD is both GOT_TLS_GD type, LD seems to
>> - can be omitted. */
>> + /* The bottom bit of h->got.offset is (ab)used as a flag.
>> + Upon encountering the first TLS relocation, we initialize all GOT
>> + slots for the corresponding symbol and set the bottom bit to 1.
>> +
>> + The second and subsequent relocations will check the flag, see that
>> + the slot is already initialized, and move on to just relocating the
>> + entry. */
>> case R_LARCH_TLS_IE_PC_HI20:
>> case R_LARCH_TLS_IE_HI20:
>> case R_LARCH_TLS_LD_PC_HI20:
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] LoongArch: Update comment about bottom bit usage in TLS GOT construction
2023-12-29 9:11 ` Tatsuyuki Ishi
@ 2023-12-29 9:59 ` mengqinggang
0 siblings, 0 replies; 13+ messages in thread
From: mengqinggang @ 2023-12-29 9:59 UTC (permalink / raw)
To: Tatsuyuki Ishi
Cc: binutils, Lulu Cai, chenglulu, hejinyang, i.swmail, liuzhensong,
luweining, Fangrui Song, nickc, wanglei, xry111, xuchenghua
These comments mainly explain the issue of another implementation method.
If the code like below, TLS IE and GD processed separately, we need two
bits as flags.
case R_LARCH_TLS_IE_PC_HI20:
if ((got_off & 1) == 0)
{
...
}
case R_LARCH_TLS_GD_PC_HI20:
if ((got_off & 2) == 0)
{
...
}
在 2023/12/29 下午5:11, Tatsuyuki Ishi 写道:
>> On Dec 29, 2023, at 17:50, mengqinggang <mengqinggang@loongson.cn> wrote:
>>
>> At the beginning of implementation, we try to IE and GD only generate their own GOT entry,
>> so we need two bits to do the flags.
> I checked the tree back at the date the comment was introduced (6d13722a: "bfd: Add supported for LoongArch new relocations.”). Even at that time, there was no |= 2 accesses, so if my understanding is correct, this comment was wrong from the beginning.
>
> Could you clarify what you mean here, in particular what “At the beginning of implementation” refer to?
>
>> 在 2023/12/28 下午10:58, Tatsuyuki Ishi 写道:
>>> The GOT assignment logic, likely copied from another backend, does not in
>>> fact require multiple flags to construct multiple type of slots. Instead,
>>> all slots are initialized on the first relocation encountered. Update
>>> comment to avoid confusion.
>>> ---
>>> bfd/elfnn-loongarch.c | 17 +++++++----------
>>> 1 file changed, 7 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
>>> index 64c34e99261..d66dcee1100 100644
>>> --- a/bfd/elfnn-loongarch.c
>>> +++ b/bfd/elfnn-loongarch.c
>>> @@ -3656,16 +3656,13 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
>>> relocation -= elf_hash_table (info)->tls_sec->vma;
>>> break;
>>> - /* TLS IE LD/GD process separately is troublesome.
>>> - When a symbol is both ie and LD/GD, h->got.off |= 1
>>> - make only one type be relocated. We must use
>>> - h->got.offset |= 1 and h->got.offset |= 2
>>> - diff IE and LD/GD. And all (got_off & (~(bfd_vma)1))
>>> - (IE LD/GD and reusable GOT reloc) must change to
>>> - (got_off & (~(bfd_vma)3)), beause we use lowest 2 bits
>>> - as a tag.
>>> - Now, LD and GD is both GOT_TLS_GD type, LD seems to
>>> - can be omitted. */
>>> + /* The bottom bit of h->got.offset is (ab)used as a flag.
>>> + Upon encountering the first TLS relocation, we initialize all GOT
>>> + slots for the corresponding symbol and set the bottom bit to 1.
>>> +
>>> + The second and subsequent relocations will check the flag, see that
>>> + the slot is already initialized, and move on to just relocating the
>>> + entry. */
>>> case R_LARCH_TLS_IE_PC_HI20:
>>> case R_LARCH_TLS_IE_HI20:
>>> case R_LARCH_TLS_LD_PC_HI20:
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE
2023-12-28 14:58 [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE Tatsuyuki Ishi
2023-12-28 14:58 ` [PATCH] LoongArch: Use tab to indent assembly in TLSDESC test suite Tatsuyuki Ishi
2023-12-28 14:58 ` [PATCH] LoongArch: Update comment about bottom bit usage in TLS GOT construction Tatsuyuki Ishi
@ 2024-01-22 5:45 ` Tatsuyuki Ishi
2024-01-22 5:48 ` Fangrui Song
2024-01-22 8:27 ` mengqinggang
2024-01-22 9:46 ` mengqinggang
3 siblings, 2 replies; 13+ messages in thread
From: Tatsuyuki Ishi @ 2024-01-22 5:45 UTC (permalink / raw)
To: Tatsuyuki Ishi
Cc: binutils, Lulu Cai, chenglulu, hejinyang, i.swmail, liuzhensong,
luweining, Fangrui Song, mengqinggang, nickc, wanglei, xry111,
xuchenghua
> On Dec 28, 2023, at 23:58, Tatsuyuki Ishi <ishitatsuyuki@gmail.com> wrote:
>
> TLS LE is exclusively for executables, while DF_STATIC_TLS is for DLLs.
> DF_STATIC_TLS should only be set for TLS IE (and when it's DLL), not LE.
> ---
> bfd/elfnn-loongarch.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index bd448cda453..64c34e99261 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -862,8 +862,6 @@ loongarch_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
> if (!bfd_link_executable (info))
> return false;
>
> - info->flags |= DF_STATIC_TLS;
> -
> if (!loongarch_elf_record_tls_and_got_reference (abfd, info, h,
> r_symndx,
> GOT_TLS_LE))
> --
> 2.40.1
>
>
Any interest in reviewing / merging this and the other two patches sent together?
The DF_STATIC_TLS change is pretty short, the formatting patch is trivial.
As for the last patch introducing a comment change, I’m not sure what Mengqing’s stance is, but my intention for the comment change is to provide a better context for the reader rather than comparing to a solution that is not currently implemented in the codebase.
Tatsuyuki.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE
2024-01-22 5:45 ` [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE Tatsuyuki Ishi
@ 2024-01-22 5:48 ` Fangrui Song
2024-01-22 6:51 ` Xi Ruoyao
2024-01-22 8:27 ` mengqinggang
1 sibling, 1 reply; 13+ messages in thread
From: Fangrui Song @ 2024-01-22 5:48 UTC (permalink / raw)
To: Tatsuyuki Ishi
Cc: binutils, Lulu Cai, chenglulu, hejinyang, i.swmail, liuzhensong,
luweining, mengqinggang, nickc, wanglei, xry111, xuchenghua
On Sun, Jan 21, 2024 at 9:45 PM Tatsuyuki Ishi <ishitatsuyuki@gmail.com> wrote:
>
> > On Dec 28, 2023, at 23:58, Tatsuyuki Ishi <ishitatsuyuki@gmail.com> wrote:
> >
> > TLS LE is exclusively for executables, while DF_STATIC_TLS is for DLLs.
> > DF_STATIC_TLS should only be set for TLS IE (and when it's DLL), not LE.
> > ---
> > bfd/elfnn-loongarch.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> > index bd448cda453..64c34e99261 100644
> > --- a/bfd/elfnn-loongarch.c
> > +++ b/bfd/elfnn-loongarch.c
> > @@ -862,8 +862,6 @@ loongarch_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
> > if (!bfd_link_executable (info))
> > return false;
> >
> > - info->flags |= DF_STATIC_TLS;
> > -
> > if (!loongarch_elf_record_tls_and_got_reference (abfd, info, h,
> > r_symndx,
> > GOT_TLS_LE))
> > --
> > 2.40.1
> >
> >
>
> Any interest in reviewing / merging this and the other two patches sent together?
> The DF_STATIC_TLS change is pretty short, the formatting patch is trivial.
> As for the last patch introducing a comment change, I’m not sure what Mengqing’s stance is, but my intention for the comment change is to provide a better context for the reader rather than comparing to a solution that is not currently implemented in the codebase.
>
> Tatsuyuki.
I think this is correct and should be applied.
--
宋方睿
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE
2024-01-22 5:48 ` Fangrui Song
@ 2024-01-22 6:51 ` Xi Ruoyao
2024-01-22 7:11 ` Fangrui Song
0 siblings, 1 reply; 13+ messages in thread
From: Xi Ruoyao @ 2024-01-22 6:51 UTC (permalink / raw)
To: Fangrui Song, Tatsuyuki Ishi
Cc: binutils, Lulu Cai, chenglulu, hejinyang, i.swmail, liuzhensong,
luweining, mengqinggang, nickc, wanglei, xuchenghua
On Sun, 2024-01-21 at 21:48 -0800, Fangrui Song wrote:
> On Sun, Jan 21, 2024 at 9:45 PM Tatsuyuki Ishi <ishitatsuyuki@gmail.com> wrote:
> >
> > > On Dec 28, 2023, at 23:58, Tatsuyuki Ishi <ishitatsuyuki@gmail.com> wrote:
> > >
> > > TLS LE is exclusively for executables, while DF_STATIC_TLS is for DLLs.
> > > DF_STATIC_TLS should only be set for TLS IE (and when it's DLL), not LE.
> > > ---
> > > bfd/elfnn-loongarch.c | 2 --
> > > 1 file changed, 2 deletions(-)
> > >
> > > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> > > index bd448cda453..64c34e99261 100644
> > > --- a/bfd/elfnn-loongarch.c
> > > +++ b/bfd/elfnn-loongarch.c
> > > @@ -862,8 +862,6 @@ loongarch_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
> > > if (!bfd_link_executable (info))
> > > return false;
> > >
> > > - info->flags |= DF_STATIC_TLS;
> > > -
> > > if (!loongarch_elf_record_tls_and_got_reference (abfd, info, h,
> > > r_symndx,
> > > GOT_TLS_LE))
> > > --
> > > 2.40.1
> > >
> > >
> >
> > Any interest in reviewing / merging this and the other two patches sent together?
> > The DF_STATIC_TLS change is pretty short, the formatting patch is trivial.
> > As for the last patch introducing a comment change, I’m not sure what Mengqing’s stance is, but my intention for the comment change is to provide a better context for the reader rather than comparing to a solution that is not currently implemented in the codebase.
> >
> > Tatsuyuki.
>
> I think this is correct and should be applied.
Will this cause a breakage in practice? If so we need to apply it for 2.42 branch too.
--
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE
2024-01-22 6:51 ` Xi Ruoyao
@ 2024-01-22 7:11 ` Fangrui Song
0 siblings, 0 replies; 13+ messages in thread
From: Fangrui Song @ 2024-01-22 7:11 UTC (permalink / raw)
To: Xi Ruoyao
Cc: Tatsuyuki Ishi, binutils, Lulu Cai, chenglulu, hejinyang,
i.swmail, liuzhensong, luweining, mengqinggang, nickc, wanglei,
xuchenghua
On Sun, Jan 21, 2024 at 10:51 PM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Sun, 2024-01-21 at 21:48 -0800, Fangrui Song wrote:
> > On Sun, Jan 21, 2024 at 9:45 PM Tatsuyuki Ishi <ishitatsuyuki@gmail.com> wrote:
> > >
> > > > On Dec 28, 2023, at 23:58, Tatsuyuki Ishi <ishitatsuyuki@gmail.com> wrote:
> > > >
> > > > TLS LE is exclusively for executables, while DF_STATIC_TLS is for DLLs.
> > > > DF_STATIC_TLS should only be set for TLS IE (and when it's DLL), not LE.
> > > > ---
> > > > bfd/elfnn-loongarch.c | 2 --
> > > > 1 file changed, 2 deletions(-)
> > > >
> > > > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> > > > index bd448cda453..64c34e99261 100644
> > > > --- a/bfd/elfnn-loongarch.c
> > > > +++ b/bfd/elfnn-loongarch.c
> > > > @@ -862,8 +862,6 @@ loongarch_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
> > > > if (!bfd_link_executable (info))
> > > > return false;
> > > >
> > > > - info->flags |= DF_STATIC_TLS;
> > > > -
> > > > if (!loongarch_elf_record_tls_and_got_reference (abfd, info, h,
> > > > r_symndx,
> > > > GOT_TLS_LE))
> > > > --
> > > > 2.40.1
> > > >
> > > >
> > >
> > > Any interest in reviewing / merging this and the other two patches sent together?
> > > The DF_STATIC_TLS change is pretty short, the formatting patch is trivial.
> > > As for the last patch introducing a comment change, I’m not sure what Mengqing’s stance is, but my intention for the comment change is to provide a better context for the reader rather than comparing to a solution that is not currently implemented in the codebase.
> > >
> > > Tatsuyuki.
> >
> > I think this is correct and should be applied.
>
> Will this cause a breakage in practice? If so we need to apply it for 2.42 branch too.
No. DF_STATIC_TLS is a not-so-useful dynamic tag. It helps a `readelf
-d` reader to know whether the DSO uses initial-exec.
For lld, I made a similar change in Nov 2021
(https://github.com/llvm/llvm-project/commit/6ca8fde226e907db13bc538e721af8724f0e92d0).
lld before Dec 2022 did not set DF_STATIC_TLS for non-x86
architectures (AArch64 and PPC).
I changed it to set DF_STATIC_TLS.
For both changes nobody has noticed anything.
--
宋方睿
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE
2024-01-22 5:45 ` [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE Tatsuyuki Ishi
2024-01-22 5:48 ` Fangrui Song
@ 2024-01-22 8:27 ` mengqinggang
1 sibling, 0 replies; 13+ messages in thread
From: mengqinggang @ 2024-01-22 8:27 UTC (permalink / raw)
To: Tatsuyuki Ishi
Cc: binutils, Lulu Cai, chenglulu, hejinyang, i.swmail, liuzhensong,
luweining, Fangrui Song, nickc, wanglei, xry111, xuchenghua
Thank you very much, I will apply this patch soon.
For the last patch, I think we can add new comments while retaining the
original ones.
The original comments describe another implementation method and can be
compared with the existing ones.
If I wants to allocate GOT entry separately for IE/GD in the future, I
can refer to these comments.
在 2024/1/22 下午1:45, Tatsuyuki Ishi 写道:
>> On Dec 28, 2023, at 23:58, Tatsuyuki Ishi <ishitatsuyuki@gmail.com> wrote:
>>
>> TLS LE is exclusively for executables, while DF_STATIC_TLS is for DLLs.
>> DF_STATIC_TLS should only be set for TLS IE (and when it's DLL), not LE.
>> ---
>> bfd/elfnn-loongarch.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
>> index bd448cda453..64c34e99261 100644
>> --- a/bfd/elfnn-loongarch.c
>> +++ b/bfd/elfnn-loongarch.c
>> @@ -862,8 +862,6 @@ loongarch_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
>> if (!bfd_link_executable (info))
>> return false;
>>
>> - info->flags |= DF_STATIC_TLS;
>> -
>> if (!loongarch_elf_record_tls_and_got_reference (abfd, info, h,
>> r_symndx,
>> GOT_TLS_LE))
>> --
>> 2.40.1
>>
>>
> Any interest in reviewing / merging this and the other two patches sent together?
> The DF_STATIC_TLS change is pretty short, the formatting patch is trivial.
> As for the last patch introducing a comment change, I’m not sure what Mengqing’s stance is, but my intention for the comment change is to provide a better context for the reader rather than comparing to a solution that is not currently implemented in the codebase.
>
> Tatsuyuki.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE
2023-12-28 14:58 [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE Tatsuyuki Ishi
` (2 preceding siblings ...)
2024-01-22 5:45 ` [PATCH] LoongArch: Do not add DF_STATIC_TLS for TLS LE Tatsuyuki Ishi
@ 2024-01-22 9:46 ` mengqinggang
3 siblings, 0 replies; 13+ messages in thread
From: mengqinggang @ 2024-01-22 9:46 UTC (permalink / raw)
To: Tatsuyuki Ishi, binutils
Cc: cailulu, chenglulu, hejinyang, i.swmail, liuzhensong, luweining,
maskray, nickc, wanglei, xry111, xuchenghua
Thank you very much, it has been applied.
在 2023/12/28 下午10:58, Tatsuyuki Ishi 写道:
> TLS LE is exclusively for executables, while DF_STATIC_TLS is for DLLs.
> DF_STATIC_TLS should only be set for TLS IE (and when it's DLL), not LE.
> ---
> bfd/elfnn-loongarch.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index bd448cda453..64c34e99261 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -862,8 +862,6 @@ loongarch_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
> if (!bfd_link_executable (info))
> return false;
>
> - info->flags |= DF_STATIC_TLS;
> -
> if (!loongarch_elf_record_tls_and_got_reference (abfd, info, h,
> r_symndx,
> GOT_TLS_LE))
^ permalink raw reply [flat|nested] 13+ messages in thread