* [PATCH v1 1/6] LoongArch: Fix ld --no-relax bug
2023-11-16 6:23 [PATCH v1 0/6] Fix some bugs of relaxation mengqinggang
@ 2023-11-16 6:23 ` mengqinggang
2023-11-16 7:34 ` WANG Xuerui
2023-11-16 6:23 ` [PATCH v1 2/6] LoongArch: Directly delete relaxed instuctions in first relaxation pass mengqinggang
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: mengqinggang @ 2023-11-16 6:23 UTC (permalink / raw)
To: binutils
Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray,
mengqinggang
When ld --no-relax, pcalau12i + ld.d still can be relaxed.
---
bfd/elfnn-loongarch.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index 18ad3cc91ca..1162cb35cd6 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -3996,29 +3996,22 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
loongarch_relax_align (abfd, sec, sym_sec, info, rel, symval);
break;
case R_LARCH_DELETE:
- if (info->relax_pass == 1)
+ if (1 == info->relax_pass)
{
loongarch_relax_delete_bytes (abfd, sec, rel->r_offset, 4, info);
rel->r_info = ELFNN_R_INFO (0, R_LARCH_NONE);
}
break;
case R_LARCH_PCALA_HI20:
- if (info->relax_pass == 0)
- {
- if (i + 4 > sec->reloc_count)
- break;
- loongarch_relax_pcala_addi (abfd, sec, rel, symval);
- }
+ if (0 == info->relax_pass && (i + 4) <= sec->reloc_count)
+ loongarch_relax_pcala_addi (abfd, sec, rel, symval);
break;
case R_LARCH_GOT_PC_HI20:
- if (local_got)
+ if (local_got && 0 == info->relax_pass
+ && (i + 4) <= sec->reloc_count)
{
- if (i + 4 > sec->reloc_count)
- break;
if (loongarch_relax_pcala_ld (abfd, sec, rel))
- {
- loongarch_relax_pcala_addi (abfd, sec, rel, symval);
- }
+ loongarch_relax_pcala_addi (abfd, sec, rel, symval);
}
break;
default:
--
2.36.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v1 1/6] LoongArch: Fix ld --no-relax bug
2023-11-16 6:23 ` [PATCH v1 1/6] LoongArch: Fix ld --no-relax bug mengqinggang
@ 2023-11-16 7:34 ` WANG Xuerui
0 siblings, 0 replies; 11+ messages in thread
From: WANG Xuerui @ 2023-11-16 7:34 UTC (permalink / raw)
To: mengqinggang, binutils
Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray
On 11/16/23 14:23, mengqinggang wrote:
> When ld --no-relax, pcalau12i + ld.d still can be relaxed.
"When calling ld with --no-relax, ..."
Apart from the grammatical fix, is the "pcalau12i + ld.d pair still gets
relaxed" behavior actually intended, or is it the broken behavior that's
being fixed? Looking at the code it's not so evident so it's better to
explain a bit.
> ---
> bfd/elfnn-loongarch.c | 19 ++++++-------------
> 1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index 18ad3cc91ca..1162cb35cd6 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -3996,29 +3996,22 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
> loongarch_relax_align (abfd, sec, sym_sec, info, rel, symval);
> break;
> case R_LARCH_DELETE:
> - if (info->relax_pass == 1)
> + if (1 == info->relax_pass)
> {
> loongarch_relax_delete_bytes (abfd, sec, rel->r_offset, 4, info);
> rel->r_info = ELFNN_R_INFO (0, R_LARCH_NONE);
> }
> break;
> case R_LARCH_PCALA_HI20:
> - if (info->relax_pass == 0)
> - {
> - if (i + 4 > sec->reloc_count)
> - break;
> - loongarch_relax_pcala_addi (abfd, sec, rel, symval);
> - }
> + if (0 == info->relax_pass && (i + 4) <= sec->reloc_count)
> + loongarch_relax_pcala_addi (abfd, sec, rel, symval);
> break;
> case R_LARCH_GOT_PC_HI20:
> - if (local_got)
> + if (local_got && 0 == info->relax_pass
> + && (i + 4) <= sec->reloc_count)
> {
> - if (i + 4 > sec->reloc_count)
> - break;
> if (loongarch_relax_pcala_ld (abfd, sec, rel))
> - {
> - loongarch_relax_pcala_addi (abfd, sec, rel, symval);
> - }
> + loongarch_relax_pcala_addi (abfd, sec, rel, symval);
> }
> break;
> default:
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1 2/6] LoongArch: Directly delete relaxed instuctions in first relaxation pass
2023-11-16 6:23 [PATCH v1 0/6] Fix some bugs of relaxation mengqinggang
2023-11-16 6:23 ` [PATCH v1 1/6] LoongArch: Fix ld --no-relax bug mengqinggang
@ 2023-11-16 6:23 ` mengqinggang
2023-11-16 6:23 ` [PATCH v1 3/6] LoongArch: Multiple relax_trip in one relax_pass mengqinggang
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: mengqinggang @ 2023-11-16 6:23 UTC (permalink / raw)
To: binutils
Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray,
mengqinggang
Directly delete relaxed instuctions in first relaxation pass, not use
R_LARCH_DELETE relocation. If not, the PC-relative offset may increase.
---
bfd/elfnn-loongarch.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index 1162cb35cd6..9d4ea7e4ae7 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -3739,7 +3739,8 @@ loongarch_relax_delete_bytes (bfd *abfd,
/* Relax pcalau12i,addi.d => pcaddi. */
static bool
loongarch_relax_pcala_addi (bfd *abfd, asection *sec,
- Elf_Internal_Rela *rel_hi, bfd_vma symval)
+ Elf_Internal_Rela *rel_hi, bfd_vma symval,
+ struct bfd_link_info *info)
{
bfd_byte *contents = elf_section_data (sec)->this_hdr.contents;
Elf_Internal_Rela *rel_lo = rel_hi + 2;
@@ -3771,8 +3772,9 @@ loongarch_relax_pcala_addi (bfd *abfd, asection *sec,
/* Adjust relocations. */
rel_hi->r_info = ELFNN_R_INFO (ELFNN_R_SYM (rel_hi->r_info),
R_LARCH_PCREL20_S2);
- rel_lo->r_info = ELFNN_R_INFO (ELFNN_R_SYM (rel_hi->r_info),
- R_LARCH_DELETE);
+ rel_lo->r_info = ELFNN_R_INFO (0, R_LARCH_NONE);
+
+ loongarch_relax_delete_bytes (abfd, sec, rel_lo->r_offset, 4, info);
return true;
}
@@ -4004,14 +4006,14 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
break;
case R_LARCH_PCALA_HI20:
if (0 == info->relax_pass && (i + 4) <= sec->reloc_count)
- loongarch_relax_pcala_addi (abfd, sec, rel, symval);
+ loongarch_relax_pcala_addi (abfd, sec, rel, symval, info);
break;
case R_LARCH_GOT_PC_HI20:
if (local_got && 0 == info->relax_pass
&& (i + 4) <= sec->reloc_count)
{
if (loongarch_relax_pcala_ld (abfd, sec, rel))
- loongarch_relax_pcala_addi (abfd, sec, rel, symval);
+ loongarch_relax_pcala_addi (abfd, sec, rel, symval, info);
}
break;
default:
--
2.36.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v1 3/6] LoongArch: Multiple relax_trip in one relax_pass
2023-11-16 6:23 [PATCH v1 0/6] Fix some bugs of relaxation mengqinggang
2023-11-16 6:23 ` [PATCH v1 1/6] LoongArch: Fix ld --no-relax bug mengqinggang
2023-11-16 6:23 ` [PATCH v1 2/6] LoongArch: Directly delete relaxed instuctions in first relaxation pass mengqinggang
@ 2023-11-16 6:23 ` mengqinggang
2023-11-16 6:23 ` [PATCH v1 4/6] LoongArch: Remove "elf_seg_map (info->output_bfd) == NULL" relaxation condition mengqinggang
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: mengqinggang @ 2023-11-16 6:23 UTC (permalink / raw)
To: binutils
Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray,
mengqinggang
If deleting instructions in one relax_trip, set again to true to start the
next relax_trip.
---
bfd/elfnn-loongarch.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index 9d4ea7e4ae7..7436a14441f 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -3740,7 +3740,7 @@ loongarch_relax_delete_bytes (bfd *abfd,
static bool
loongarch_relax_pcala_addi (bfd *abfd, asection *sec,
Elf_Internal_Rela *rel_hi, bfd_vma symval,
- struct bfd_link_info *info)
+ struct bfd_link_info *info, bool *again)
{
bfd_byte *contents = elf_section_data (sec)->this_hdr.contents;
Elf_Internal_Rela *rel_lo = rel_hi + 2;
@@ -3766,6 +3766,9 @@ loongarch_relax_pcala_addi (bfd *abfd, asection *sec,
|| ((bfd_signed_vma)(symval - pc) > (bfd_signed_vma)(int32_t)0x1ffffc))
return false;
+ /* Continue next relax trip. */
+ *again = true;
+
pca = pcaddi | rd;
bfd_put (32, abfd, pca, contents + rel_hi->r_offset);
@@ -4006,14 +4009,15 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
break;
case R_LARCH_PCALA_HI20:
if (0 == info->relax_pass && (i + 4) <= sec->reloc_count)
- loongarch_relax_pcala_addi (abfd, sec, rel, symval, info);
+ loongarch_relax_pcala_addi (abfd, sec, rel, symval, info, again);
break;
case R_LARCH_GOT_PC_HI20:
if (local_got && 0 == info->relax_pass
&& (i + 4) <= sec->reloc_count)
{
if (loongarch_relax_pcala_ld (abfd, sec, rel))
- loongarch_relax_pcala_addi (abfd, sec, rel, symval, info);
+ loongarch_relax_pcala_addi (abfd, sec, rel, symval,
+ info, again);
}
break;
default:
--
2.36.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v1 4/6] LoongArch: Remove "elf_seg_map (info->output_bfd) == NULL" relaxation condition
2023-11-16 6:23 [PATCH v1 0/6] Fix some bugs of relaxation mengqinggang
` (2 preceding siblings ...)
2023-11-16 6:23 ` [PATCH v1 3/6] LoongArch: Multiple relax_trip in one relax_pass mengqinggang
@ 2023-11-16 6:23 ` mengqinggang
2023-11-16 7:43 ` WANG Xuerui
2023-11-16 6:23 ` [PATCH v1 5/6] LoongArch: Modify link_info.relax_pass from 3 to 2 mengqinggang
2023-11-16 6:23 ` [PATCH v1 6/6] LoongArch: Add more relaxation testcases mengqinggang
5 siblings, 1 reply; 11+ messages in thread
From: mengqinggang @ 2023-11-16 6:23 UTC (permalink / raw)
To: binutils
Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray,
mengqinggang
This condition cause .so(shared object) can't be relaxed.
Without this condition, we need to update program header size and .eh_frame_hdr
size before relaxation.
---
bfd/elfnn-loongarch.c | 24 ++++++++++++++++++++----
ld/emultempl/loongarchelf.em | 18 ++++++++++++++++++
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index 7436a14441f..d331eefb8ac 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -3738,7 +3738,7 @@ loongarch_relax_delete_bytes (bfd *abfd,
/* Relax pcalau12i,addi.d => pcaddi. */
static bool
-loongarch_relax_pcala_addi (bfd *abfd, asection *sec,
+loongarch_relax_pcala_addi (bfd *abfd, asection *sec, asection *sym_sec,
Elf_Internal_Rela *rel_hi, bfd_vma symval,
struct bfd_link_info *info, bool *again)
{
@@ -3747,7 +3747,23 @@ loongarch_relax_pcala_addi (bfd *abfd, asection *sec,
uint32_t pca = bfd_get (32, abfd, contents + rel_hi->r_offset);
uint32_t add = bfd_get (32, abfd, contents + rel_lo->r_offset);
uint32_t rd = pca & 0x1f;
+
+ /* Because previous sections' relax, output_offset may increase and need to
+ be updated before relax. But it update after relax in
+ size_input_section defaultly, so we manually updating here. */
+ sec->output_offset = sec->output_section->size;
bfd_vma pc = sec_addr (sec) + rel_hi->r_offset;
+
+ /* If pc and symbol not in the same segment, add/sub segment alignment.
+ Fixme: if there are multiple readonly segments? */
+ if (!(sym_sec->flags & SEC_READONLY))
+ {
+ if (symval > pc)
+ pc -= info->maxpagesize;
+ else if (symval < pc)
+ pc += info->maxpagesize;
+ }
+
const uint32_t addi_d = 0x02c00000;
const uint32_t pcaddi = 0x18000000;
@@ -3889,7 +3905,6 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
|| sec->sec_flg0
|| (sec->flags & SEC_RELOC) == 0
|| sec->reloc_count == 0
- || elf_seg_map (info->output_bfd) == NULL
|| (info->disable_target_specific_optimizations
&& info->relax_pass == 0)
/* The exp_seg_relro_adjust is enum phase_enum (0x4),
@@ -4009,14 +4024,15 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
break;
case R_LARCH_PCALA_HI20:
if (0 == info->relax_pass && (i + 4) <= sec->reloc_count)
- loongarch_relax_pcala_addi (abfd, sec, rel, symval, info, again);
+ loongarch_relax_pcala_addi (abfd, sec, sym_sec, rel, symval,
+ info, again);
break;
case R_LARCH_GOT_PC_HI20:
if (local_got && 0 == info->relax_pass
&& (i + 4) <= sec->reloc_count)
{
if (loongarch_relax_pcala_ld (abfd, sec, rel))
- loongarch_relax_pcala_addi (abfd, sec, rel, symval,
+ loongarch_relax_pcala_addi (abfd, sec, sym_sec, rel, symval,
info, again);
}
break;
diff --git a/ld/emultempl/loongarchelf.em b/ld/emultempl/loongarchelf.em
index 4850feb8767..d81c99da48b 100644
--- a/ld/emultempl/loongarchelf.em
+++ b/ld/emultempl/loongarchelf.em
@@ -62,6 +62,24 @@ gld${EMULATION_NAME}_after_allocation (void)
}
}
+ /* The program header size of executable file may increase. */
+ if (bfd_get_flavour (link_info.output_bfd) == bfd_target_elf_flavour
+ && !bfd_link_relocatable (&link_info))
+ {
+ if (lang_phdr_list == NULL)
+ elf_seg_map (link_info.output_bfd) = NULL;
+ if (!_bfd_elf_map_sections_to_segments (link_info.output_bfd,
+ &link_info,
+ NULL))
+ einfo (_("%F%P: map sections to segments failed: %E\n"));
+ }
+
+ /* Adjust program header size and .eh_frame_hdr size before
+ lang_relax_sections. Without it, the vma of data segment may increase. */
+ lang_do_assignments (lang_allocating_phase_enum);
+ lang_reset_memory_regions ();
+ lang_size_sections (NULL, true);
+
enum phase_enum *phase = &(expld.dataseg.phase);
bfd_elf${ELFSIZE}_loongarch_set_data_segment_info (&link_info, (int *) phase);
/* gld${EMULATION_NAME}_map_segments (need_layout); */
--
2.36.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v1 4/6] LoongArch: Remove "elf_seg_map (info->output_bfd) == NULL" relaxation condition
2023-11-16 6:23 ` [PATCH v1 4/6] LoongArch: Remove "elf_seg_map (info->output_bfd) == NULL" relaxation condition mengqinggang
@ 2023-11-16 7:43 ` WANG Xuerui
2023-11-16 9:38 ` mengqinggang
0 siblings, 1 reply; 11+ messages in thread
From: WANG Xuerui @ 2023-11-16 7:43 UTC (permalink / raw)
To: mengqinggang, binutils
Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray
On 11/16/23 14:23, mengqinggang wrote:
> This condition cause .so(shared object) can't be relaxed.
"Previously the condition prevented shared objects from being relaxed."
> Without this condition, we need to update program header size and .eh_frame_hdr
> size before relaxation.
"To remove the limitation, we need to ..."
> ---
> bfd/elfnn-loongarch.c | 24 ++++++++++++++++++++----
> ld/emultempl/loongarchelf.em | 18 ++++++++++++++++++
> 2 files changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index 7436a14441f..d331eefb8ac 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -3738,7 +3738,7 @@ loongarch_relax_delete_bytes (bfd *abfd,
>
> /* Relax pcalau12i,addi.d => pcaddi. */
> static bool
> -loongarch_relax_pcala_addi (bfd *abfd, asection *sec,
> +loongarch_relax_pcala_addi (bfd *abfd, asection *sec, asection *sym_sec,
> Elf_Internal_Rela *rel_hi, bfd_vma symval,
> struct bfd_link_info *info, bool *again)
> {
> @@ -3747,7 +3747,23 @@ loongarch_relax_pcala_addi (bfd *abfd, asection *sec,
> uint32_t pca = bfd_get (32, abfd, contents + rel_hi->r_offset);
> uint32_t add = bfd_get (32, abfd, contents + rel_lo->r_offset);
> uint32_t rd = pca & 0x1f;
> +
> + /* Because previous sections' relax, output_offset may increase and need to
> + be updated before relax. But it update after relax in
> + size_input_section defaultly, so we manually updating here. */
This is rather hard to understand...
"This section's output_offset may change after previous section(s) have
been relaxed, so it needs to be updated beforehand. size_input_section
already took care of updating it after relaxation, so we additionally
update once here."
Does this sound okay?
> + sec->output_offset = sec->output_section->size;
> bfd_vma pc = sec_addr (sec) + rel_hi->r_offset;
> +
> + /* If pc and symbol not in the same segment, add/sub segment alignment.
> + Fixme: if there are multiple readonly segments? */
Usually FIXME notes are spelled with all-caps to make it easier for the
eyes and tools.
> + if (!(sym_sec->flags & SEC_READONLY))
> + {
> + if (symval > pc)
> + pc -= info->maxpagesize;
> + else if (symval < pc)
> + pc += info->maxpagesize;
> + }
> +
> const uint32_t addi_d = 0x02c00000;
> const uint32_t pcaddi = 0x18000000;
>
> @@ -3889,7 +3905,6 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
> || sec->sec_flg0
> || (sec->flags & SEC_RELOC) == 0
> || sec->reloc_count == 0
> - || elf_seg_map (info->output_bfd) == NULL
> || (info->disable_target_specific_optimizations
> && info->relax_pass == 0)
> /* The exp_seg_relro_adjust is enum phase_enum (0x4),
> @@ -4009,14 +4024,15 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
> break;
> case R_LARCH_PCALA_HI20:
> if (0 == info->relax_pass && (i + 4) <= sec->reloc_count)
> - loongarch_relax_pcala_addi (abfd, sec, rel, symval, info, again);
> + loongarch_relax_pcala_addi (abfd, sec, sym_sec, rel, symval,
> + info, again);
> break;
> case R_LARCH_GOT_PC_HI20:
> if (local_got && 0 == info->relax_pass
> && (i + 4) <= sec->reloc_count)
> {
> if (loongarch_relax_pcala_ld (abfd, sec, rel))
> - loongarch_relax_pcala_addi (abfd, sec, rel, symval,
> + loongarch_relax_pcala_addi (abfd, sec, sym_sec, rel, symval,
> info, again);
> }
> break;
> diff --git a/ld/emultempl/loongarchelf.em b/ld/emultempl/loongarchelf.em
> index 4850feb8767..d81c99da48b 100644
> --- a/ld/emultempl/loongarchelf.em
> +++ b/ld/emultempl/loongarchelf.em
> @@ -62,6 +62,24 @@ gld${EMULATION_NAME}_after_allocation (void)
> }
> }
>
> + /* The program header size of executable file may increase. */
> + if (bfd_get_flavour (link_info.output_bfd) == bfd_target_elf_flavour
> + && !bfd_link_relocatable (&link_info))
> + {
> + if (lang_phdr_list == NULL)
> + elf_seg_map (link_info.output_bfd) = NULL;
> + if (!_bfd_elf_map_sections_to_segments (link_info.output_bfd,
> + &link_info,
> + NULL))
> + einfo (_("%F%P: map sections to segments failed: %E\n"));
> + }
> +
> + /* Adjust program header size and .eh_frame_hdr size before
> + lang_relax_sections. Without it, the vma of data segment may increase. */
Out of curiosity (and unfamiliarity), is it only "increases" and no
decreases?
> + lang_do_assignments (lang_allocating_phase_enum);
> + lang_reset_memory_regions ();
> + lang_size_sections (NULL, true);
> +
> enum phase_enum *phase = &(expld.dataseg.phase);
> bfd_elf${ELFSIZE}_loongarch_set_data_segment_info (&link_info, (int *) phase);
> /* gld${EMULATION_NAME}_map_segments (need_layout); */
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v1 4/6] LoongArch: Remove "elf_seg_map (info->output_bfd) == NULL" relaxation condition
2023-11-16 7:43 ` WANG Xuerui
@ 2023-11-16 9:38 ` mengqinggang
0 siblings, 0 replies; 11+ messages in thread
From: mengqinggang @ 2023-11-16 9:38 UTC (permalink / raw)
To: WANG Xuerui, binutils; +Cc: xuchenghua, chenglulu, liuzhensong, xry111, maskray
在 2023/11/16 下午3:43, WANG Xuerui 写道:
> On 11/16/23 14:23, mengqinggang wrote:
>> This condition cause .so(shared object) can't be relaxed.
> "Previously the condition prevented shared objects from being relaxed."
>> Without this condition, we need to update program header size and
>> .eh_frame_hdr
>> size before relaxation.
> "To remove the limitation, we need to ..."
>> ---
>> bfd/elfnn-loongarch.c | 24 ++++++++++++++++++++----
>> ld/emultempl/loongarchelf.em | 18 ++++++++++++++++++
>> 2 files changed, 38 insertions(+), 4 deletions(-)
>>
>> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
>> index 7436a14441f..d331eefb8ac 100644
>> --- a/bfd/elfnn-loongarch.c
>> +++ b/bfd/elfnn-loongarch.c
>> @@ -3738,7 +3738,7 @@ loongarch_relax_delete_bytes (bfd *abfd,
>> /* Relax pcalau12i,addi.d => pcaddi. */
>> static bool
>> -loongarch_relax_pcala_addi (bfd *abfd, asection *sec,
>> +loongarch_relax_pcala_addi (bfd *abfd, asection *sec, asection
>> *sym_sec,
>> Elf_Internal_Rela *rel_hi, bfd_vma symval,
>> struct bfd_link_info *info, bool *again)
>> {
>> @@ -3747,7 +3747,23 @@ loongarch_relax_pcala_addi (bfd *abfd,
>> asection *sec,
>> uint32_t pca = bfd_get (32, abfd, contents + rel_hi->r_offset);
>> uint32_t add = bfd_get (32, abfd, contents + rel_lo->r_offset);
>> uint32_t rd = pca & 0x1f;
>> +
>> + /* Because previous sections' relax, output_offset may increase
>> and need to
>> + be updated before relax. But it update after relax in
>> + size_input_section defaultly, so we manually updating here. */
>
> This is rather hard to understand...
>
> "This section's output_offset may change after previous section(s)
> have been relaxed, so it needs to be updated beforehand.
> size_input_section already took care of updating it after relaxation,
> so we additionally update once here."
>
> Does this sound okay?
>
>> + sec->output_offset = sec->output_section->size;
>> bfd_vma pc = sec_addr (sec) + rel_hi->r_offset;
>> +
>> + /* If pc and symbol not in the same segment, add/sub segment
>> alignment.
>> + Fixme: if there are multiple readonly segments? */
> Usually FIXME notes are spelled with all-caps to make it easier for
> the eyes and tools.
>> + if (!(sym_sec->flags & SEC_READONLY))
>> + {
>> + if (symval > pc)
>> + pc -= info->maxpagesize;
>> + else if (symval < pc)
>> + pc += info->maxpagesize;
>> + }
>> +
>> const uint32_t addi_d = 0x02c00000;
>> const uint32_t pcaddi = 0x18000000;
>> @@ -3889,7 +3905,6 @@ loongarch_elf_relax_section (bfd *abfd,
>> asection *sec,
>> || sec->sec_flg0
>> || (sec->flags & SEC_RELOC) == 0
>> || sec->reloc_count == 0
>> - || elf_seg_map (info->output_bfd) == NULL
>> || (info->disable_target_specific_optimizations
>> && info->relax_pass == 0)
>> /* The exp_seg_relro_adjust is enum phase_enum (0x4),
>> @@ -4009,14 +4024,15 @@ loongarch_elf_relax_section (bfd *abfd,
>> asection *sec,
>> break;
>> case R_LARCH_PCALA_HI20:
>> if (0 == info->relax_pass && (i + 4) <= sec->reloc_count)
>> - loongarch_relax_pcala_addi (abfd, sec, rel, symval, info,
>> again);
>> + loongarch_relax_pcala_addi (abfd, sec, sym_sec, rel, symval,
>> + info, again);
>> break;
>> case R_LARCH_GOT_PC_HI20:
>> if (local_got && 0 == info->relax_pass
>> && (i + 4) <= sec->reloc_count)
>> {
>> if (loongarch_relax_pcala_ld (abfd, sec, rel))
>> - loongarch_relax_pcala_addi (abfd, sec, rel, symval,
>> + loongarch_relax_pcala_addi (abfd, sec, sym_sec, rel, symval,
>> info, again);
>> }
>> break;
>> diff --git a/ld/emultempl/loongarchelf.em b/ld/emultempl/loongarchelf.em
>> index 4850feb8767..d81c99da48b 100644
>> --- a/ld/emultempl/loongarchelf.em
>> +++ b/ld/emultempl/loongarchelf.em
>> @@ -62,6 +62,24 @@ gld${EMULATION_NAME}_after_allocation (void)
>> }
>> }
>> + /* The program header size of executable file may increase. */
>> + if (bfd_get_flavour (link_info.output_bfd) == bfd_target_elf_flavour
>> + && !bfd_link_relocatable (&link_info))
>> + {
>> + if (lang_phdr_list == NULL)
>> + elf_seg_map (link_info.output_bfd) = NULL;
>> + if (!_bfd_elf_map_sections_to_segments (link_info.output_bfd,
>> + &link_info,
>> + NULL))
>> + einfo (_("%F%P: map sections to segments failed: %E\n"));
>> + }
>> +
>> + /* Adjust program header size and .eh_frame_hdr size before
>> + lang_relax_sections. Without it, the vma of data segment may
>> increase. */
> Out of curiosity (and unfamiliarity), is it only "increases" and no
> decreases?
After relaxation, the vma of data segment may decreases. But before
relaxation,
the vma of data segment should only increase due to the increase in the
number of
segments and the determination of the .eh_frame_hdr section size.
>> + lang_do_assignments (lang_allocating_phase_enum);
>> + lang_reset_memory_regions ();
>> + lang_size_sections (NULL, true);
>> +
>> enum phase_enum *phase = &(expld.dataseg.phase);
>> bfd_elf${ELFSIZE}_loongarch_set_data_segment_info (&link_info,
>> (int *) phase);
>> /* gld${EMULATION_NAME}_map_segments (need_layout); */
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1 5/6] LoongArch: Modify link_info.relax_pass from 3 to 2
2023-11-16 6:23 [PATCH v1 0/6] Fix some bugs of relaxation mengqinggang
` (3 preceding siblings ...)
2023-11-16 6:23 ` [PATCH v1 4/6] LoongArch: Remove "elf_seg_map (info->output_bfd) == NULL" relaxation condition mengqinggang
@ 2023-11-16 6:23 ` mengqinggang
2023-11-16 6:23 ` [PATCH v1 6/6] LoongArch: Add more relaxation testcases mengqinggang
5 siblings, 0 replies; 11+ messages in thread
From: mengqinggang @ 2023-11-16 6:23 UTC (permalink / raw)
To: binutils
Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray,
mengqinggang
The first pass handles R_LARCH_RELAX relocations, the second pass
handles R_LARCH_ALIGN relocations.
---
bfd/elfnn-loongarch.c | 2 +-
ld/emultempl/loongarchelf.em | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index d331eefb8ac..9d0cfab5f36 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -4012,7 +4012,7 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
switch (ELFNN_R_TYPE (rel->r_info))
{
case R_LARCH_ALIGN:
- if (2 == info->relax_pass)
+ if (1 == info->relax_pass)
loongarch_relax_align (abfd, sec, sym_sec, info, rel, symval);
break;
case R_LARCH_DELETE:
diff --git a/ld/emultempl/loongarchelf.em b/ld/emultempl/loongarchelf.em
index d81c99da48b..9974989489c 100644
--- a/ld/emultempl/loongarchelf.em
+++ b/ld/emultempl/loongarchelf.em
@@ -42,7 +42,7 @@ larch_elf_before_allocation (void)
ENABLE_RELAXATION;
}
- link_info.relax_pass = 3;
+ link_info.relax_pass = 2;
}
static void
--
2.36.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v1 6/6] LoongArch: Add more relaxation testcases
2023-11-16 6:23 [PATCH v1 0/6] Fix some bugs of relaxation mengqinggang
` (4 preceding siblings ...)
2023-11-16 6:23 ` [PATCH v1 5/6] LoongArch: Modify link_info.relax_pass from 3 to 2 mengqinggang
@ 2023-11-16 6:23 ` mengqinggang
2023-11-16 6:39 ` Fangrui Song
5 siblings, 1 reply; 11+ messages in thread
From: mengqinggang @ 2023-11-16 6:23 UTC (permalink / raw)
To: binutils
Cc: xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray,
mengqinggang
1. .so relaxation testcase
2. ld --no-relax testcase
3. segment alignment testcase
---
.../ld-loongarch-elf/relax-segment-max.s | 13 +++
.../ld-loongarch-elf/relax-segment-min.s | 13 +++
ld/testsuite/ld-loongarch-elf/relax-so.s | 4 +
ld/testsuite/ld-loongarch-elf/relax.exp | 80 +++++++++++++++++--
4 files changed, 105 insertions(+), 5 deletions(-)
create mode 100644 ld/testsuite/ld-loongarch-elf/relax-segment-max.s
create mode 100644 ld/testsuite/ld-loongarch-elf/relax-segment-min.s
create mode 100644 ld/testsuite/ld-loongarch-elf/relax-so.s
diff --git a/ld/testsuite/ld-loongarch-elf/relax-segment-max.s b/ld/testsuite/ld-loongarch-elf/relax-segment-max.s
new file mode 100644
index 00000000000..417260fb880
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/relax-segment-max.s
@@ -0,0 +1,13 @@
+# The .align may cause overflow because deleting nops.
+ .text # 0x120000000
+ .fill 0x4000
+ .align 3 # 0x120004000
+ la.local $r12, .L1
+
+# .fill 0x1f7ffc # max fill without overflow, .data address is 0x120200000
+# .fill 0x1f8000 # min fill with overflow, .data address is 0x120204000
+ .fill 0x1fbff4 # max fill with overflow, .data address is 0x120204000
+
+ .data
+.L1:
+ .byte 2
diff --git a/ld/testsuite/ld-loongarch-elf/relax-segment-min.s b/ld/testsuite/ld-loongarch-elf/relax-segment-min.s
new file mode 100644
index 00000000000..14a541b97c8
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/relax-segment-min.s
@@ -0,0 +1,13 @@
+# The .align may cause overflow because deleting nops.
+ .text # 0x120000000
+ .fill 0x4000
+ .align 3 # 0x120004000
+ la.local $r12, .L1
+
+# .fill 0x1f7ffc # max fill without overflow, .data address is 0x120200000
+ .fill 0x1f8000 # min fill with overflow, .data address is 0x120204000
+# .fill 0x1fbff4 # max fill with overflow, .data address is 0x120204000
+
+ .data
+.L1:
+ .byte 2
diff --git a/ld/testsuite/ld-loongarch-elf/relax-so.s b/ld/testsuite/ld-loongarch-elf/relax-so.s
new file mode 100644
index 00000000000..01a404a8b47
--- /dev/null
+++ b/ld/testsuite/ld-loongarch-elf/relax-so.s
@@ -0,0 +1,4 @@
+.text
+.align 2
+.L1:
+ la.local $r12, .L1
diff --git a/ld/testsuite/ld-loongarch-elf/relax.exp b/ld/testsuite/ld-loongarch-elf/relax.exp
index 7ff876d7914..426b72dac5d 100644
--- a/ld/testsuite/ld-loongarch-elf/relax.exp
+++ b/ld/testsuite/ld-loongarch-elf/relax.exp
@@ -22,7 +22,7 @@
if [istarget loongarch64-*-*] {
if [isbuild loongarch64-*-*] {
- set testname "loongarch relax build"
+ set testname "loongarch relax .exe build"
set pre_builds [list \
[list \
"$testname" \
@@ -39,17 +39,87 @@ if [istarget loongarch64-*-*] {
if [file exist "tmpdir/relax"] {
set objdump_output [run_host_cmd "objdump" "-d tmpdir/relax"]
if { [ regexp ".*pcaddi.*pcaddi.*" $objdump_output] } {
- pass "loongarch relax"
+ pass "loongarch relax .exe"
} {
- fail "loongarch relax"
+ fail "loongarch relax .exe"
}
}
+
+ set testname "loongarch ld --no-relax build"
+ set pre_builds [list \
+ [list \
+ "$testname" \
+ "-Wl,--no-relax" \
+ "" \
+ {relax.s} \
+ {} \
+ "norelax" \
+ ] \
+ ]
+
+ run_cc_link_tests $pre_builds
+
+ if [file exist "tmpdir/norelax"] {
+ set objdump_output [run_host_cmd "objdump" "-d tmpdir/norelax"]
+ if { [ regexp ".*pcaddi.*" $objdump_output] } {
+ fail "loongarch ld --no-relax"
+ } {
+ pass "loongarch ld --no-relax"
+ }
+ }
+
+ run_ld_link_tests \
+ [list \
+ [list \
+ "loongarch relax .so build" \
+ "-shared -e 0x0" "" \
+ "" \
+ {relax-so.s} \
+ {} \
+ "relax-so" \
+ ] \
+ ]
+
+ if [file exist "tmpdir/relax-so"] {
+ set objdump_output [run_host_cmd "objdump" "-d tmpdir/relax-so"]
+ if { [ regexp ".*pcaddi.*" $objdump_output] } {
+ pass "loongarch relax .so"
+ } {
+ fail "loongarch relax .so"
+ }
+ }
+
+ # If symbol in data segment, offset need to sub segment align to prevent
+ # overflow.
+ run_ld_link_tests \
+ [list \
+ [list \
+ "loongarch relax segment alignment min" \
+ "-e0 -Ttext 0x120000000 -pie" "" \
+ "" \
+ {relax-segment-min.s} \
+ {} \
+ "relax-segment-min" \
+ ] \
+ ]
+
+ run_ld_link_tests \
+ [list \
+ [list \
+ "loongarch relax segment alignment max" \
+ "-e0 -Ttext 0x120000000 -pie" "" \
+ "" \
+ {relax-segment-max.s} \
+ {} \
+ "relax-segment-max" \
+ ] \
+ ]
}
run_ld_link_tests \
[list \
[list \
- "relax-align" \
+ "loongarch relax-align" \
"-e 0x0 -z relro" "" \
"" \
{relax-align.s} \
@@ -64,7 +134,7 @@ if [istarget loongarch64-*-*] {
run_ld_link_tests \
[list \
[list \
- "uleb128" \
+ "loongarch uleb128" \
"-e 0x0" "" \
"" \
{uleb128.s} \
--
2.36.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v1 6/6] LoongArch: Add more relaxation testcases
2023-11-16 6:23 ` [PATCH v1 6/6] LoongArch: Add more relaxation testcases mengqinggang
@ 2023-11-16 6:39 ` Fangrui Song
0 siblings, 0 replies; 11+ messages in thread
From: Fangrui Song @ 2023-11-16 6:39 UTC (permalink / raw)
To: mengqinggang
Cc: binutils, xuchenghua, chenglulu, liuzhensong, xry111, i.swmail, maskray
On Wed, Nov 15, 2023 at 10:23 PM mengqinggang <mengqinggang@loongson.cn> wrote:
>
> 1. .so relaxation testcase
> 2. ld --no-relax testcase
> 3. segment alignment testcase
> ---
> .../ld-loongarch-elf/relax-segment-max.s | 13 +++
> .../ld-loongarch-elf/relax-segment-min.s | 13 +++
> ld/testsuite/ld-loongarch-elf/relax-so.s | 4 +
> ld/testsuite/ld-loongarch-elf/relax.exp | 80 +++++++++++++++++--
> 4 files changed, 105 insertions(+), 5 deletions(-)
> create mode 100644 ld/testsuite/ld-loongarch-elf/relax-segment-max.s
> create mode 100644 ld/testsuite/ld-loongarch-elf/relax-segment-min.s
> create mode 100644 ld/testsuite/ld-loongarch-elf/relax-so.s
>
> diff --git a/ld/testsuite/ld-loongarch-elf/relax-segment-max.s b/ld/testsuite/ld-loongarch-elf/relax-segment-max.s
> new file mode 100644
> index 00000000000..417260fb880
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/relax-segment-max.s
> @@ -0,0 +1,13 @@
> +# The .align may cause overflow because deleting nops.
> + .text # 0x120000000
> + .fill 0x4000
> + .align 3 # 0x120004000
> + la.local $r12, .L1
> +
> +# .fill 0x1f7ffc # max fill without overflow, .data address is 0x120200000
> +# .fill 0x1f8000 # min fill with overflow, .data address is 0x120204000
> + .fill 0x1fbff4 # max fill with overflow, .data address is 0x120204000
> +
> + .data
> +.L1:
> + .byte 2
> diff --git a/ld/testsuite/ld-loongarch-elf/relax-segment-min.s b/ld/testsuite/ld-loongarch-elf/relax-segment-min.s
> new file mode 100644
> index 00000000000..14a541b97c8
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/relax-segment-min.s
> @@ -0,0 +1,13 @@
> +# The .align may cause overflow because deleting nops.
> + .text # 0x120000000
> + .fill 0x4000
> + .align 3 # 0x120004000
> + la.local $r12, .L1
> +
> +# .fill 0x1f7ffc # max fill without overflow, .data address is 0x120200000
> + .fill 0x1f8000 # min fill with overflow, .data address is 0x120204000
> +# .fill 0x1fbff4 # max fill with overflow, .data address is 0x120204000
> +
> + .data
> +.L1:
> + .byte 2
You may want something more complex.
When I implemented linker relaxation for lld, I put a lot of thoughts
into organizing the tests.
I have come up with
https://github.com/llvm/llvm-project/blob/main/lld/test/ELF/riscv-relax-call.s
https://github.com/llvm/llvm-project/blob/main/lld/test/ELF/riscv-relax-call2.s
https://github.com/llvm/llvm-project/blob/main/lld/test/ELF/riscv-relax-align.s
.fill 0x4000 creates a lot of zeroes. I prefer output section addresses.
> diff --git a/ld/testsuite/ld-loongarch-elf/relax-so.s b/ld/testsuite/ld-loongarch-elf/relax-so.s
> new file mode 100644
> index 00000000000..01a404a8b47
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/relax-so.s
> @@ -0,0 +1,4 @@
> +.text
> +.align 2
> +.L1:
> + la.local $r12, .L1
> diff --git a/ld/testsuite/ld-loongarch-elf/relax.exp b/ld/testsuite/ld-loongarch-elf/relax.exp
> index 7ff876d7914..426b72dac5d 100644
> --- a/ld/testsuite/ld-loongarch-elf/relax.exp
> +++ b/ld/testsuite/ld-loongarch-elf/relax.exp
> @@ -22,7 +22,7 @@
> if [istarget loongarch64-*-*] {
>
> if [isbuild loongarch64-*-*] {
> - set testname "loongarch relax build"
> + set testname "loongarch relax .exe build"
> set pre_builds [list \
> [list \
> "$testname" \
> @@ -39,17 +39,87 @@ if [istarget loongarch64-*-*] {
> if [file exist "tmpdir/relax"] {
> set objdump_output [run_host_cmd "objdump" "-d tmpdir/relax"]
> if { [ regexp ".*pcaddi.*pcaddi.*" $objdump_output] } {
> - pass "loongarch relax"
> + pass "loongarch relax .exe"
> } {
> - fail "loongarch relax"
> + fail "loongarch relax .exe"
> }
> }
> +
> + set testname "loongarch ld --no-relax build"
> + set pre_builds [list \
> + [list \
> + "$testname" \
> + "-Wl,--no-relax" \
> + "" \
> + {relax.s} \
> + {} \
> + "norelax" \
> + ] \
> + ]
> +
> + run_cc_link_tests $pre_builds
> +
> + if [file exist "tmpdir/norelax"] {
> + set objdump_output [run_host_cmd "objdump" "-d tmpdir/norelax"]
> + if { [ regexp ".*pcaddi.*" $objdump_output] } {
> + fail "loongarch ld --no-relax"
> + } {
> + pass "loongarch ld --no-relax"
> + }
> + }
> +
> + run_ld_link_tests \
> + [list \
> + [list \
> + "loongarch relax .so build" \
> + "-shared -e 0x0" "" \
> + "" \
> + {relax-so.s} \
> + {} \
> + "relax-so" \
> + ] \
> + ]
> +
> + if [file exist "tmpdir/relax-so"] {
> + set objdump_output [run_host_cmd "objdump" "-d tmpdir/relax-so"]
> + if { [ regexp ".*pcaddi.*" $objdump_output] } {
> + pass "loongarch relax .so"
> + } {
> + fail "loongarch relax .so"
> + }
> + }
> +
> + # If symbol in data segment, offset need to sub segment align to prevent
> + # overflow.
> + run_ld_link_tests \
> + [list \
> + [list \
> + "loongarch relax segment alignment min" \
> + "-e0 -Ttext 0x120000000 -pie" "" \
> + "" \
> + {relax-segment-min.s} \
> + {} \
> + "relax-segment-min" \
> + ] \
> + ]
> +
> + run_ld_link_tests \
> + [list \
> + [list \
> + "loongarch relax segment alignment max" \
> + "-e0 -Ttext 0x120000000 -pie" "" \
> + "" \
> + {relax-segment-max.s} \
> + {} \
> + "relax-segment-max" \
> + ] \
> + ]
> }
>
> run_ld_link_tests \
> [list \
> [list \
> - "relax-align" \
> + "loongarch relax-align" \
> "-e 0x0 -z relro" "" \
> "" \
> {relax-align.s} \
> @@ -64,7 +134,7 @@ if [istarget loongarch64-*-*] {
> run_ld_link_tests \
> [list \
> [list \
> - "uleb128" \
> + "loongarch uleb128" \
> "-e 0x0" "" \
> "" \
> {uleb128.s} \
> --
> 2.36.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread