[PATCH] RISC-V: Improve multiple relax passes problem.
Nelson Chu
nelson.chu@sifive.com
Fri Dec 18 04:07:02 GMT 2020
Hi Guys,
Forgot to mention one thing - lang_relax_sections do the relaxations
only for link_info.relax_pass times. Therefore, I have to find
another way to re-run the whole relaxation. In the target emulate
after_allocation, we call ldelf_map_segments to calculate all the
section offsets, and it also calls lang_relax_sections to do the
relaxations, so I think calling the ldelf_map_segments here many times
should be fine.
Thanks
Nelson
On Fri, Dec 18, 2020 at 11:55 AM Nelson Chu <nelson.chu@sifive.com> wrote:
>
> According to the commit abd20cb637008da9d32018b4b03973e119388a0a, an
> intersting thing is that - the more relax passes, the more chances of
> relaxations are reduced [1]. Originally, we set the boolean `again`
> to TRUE once the code is actually deleted, and then we run the relxations
> repeatedly if `again` is still TRUE. But `again` only works for the
> relax pass itself, and won't affect others. That is - we can not use
> `again` to re-run the relax pass when we already enter into the following
> passes (can not run the relax passes backwards). Besides, we must seperate
> the PCREL relaxations into two relax passes for some reasons [2], it make
> us lose some relax opportunities.
>
> This patch try to fix the problem, and the basic idea was come from Jim
> Wilson - we use a new boolean, restart_relax, to determine if we need to
> run the whole relax passes again from 0 to 2. Once we have deleted the
> code between relax pass 0 to 2, the restart_relax will be set to TRUE,
> we should run the whole relxations again to give them more chances to
> shorten the code. We will only enter into the relax pass 3 when the
> restart_relax is FALSE, since we can't relax anything else once we start
> to handle the alignments.
>
> I have passed the gcc/binutils regressions by riscv-gnu-toolchain, and
> looks fine for now.
>
> [1] https://sourceware.org/pipermail/binutils/2020-November/114223.html
> [2] https://sourceware.org/pipermail/binutils/2020-November/114235.html
>
> bfd/
> * elfnn-riscv.c (riscv_elf_link_hash_table): New boolean restart_relax,
> used to check if we need to run the whole relaxations from relax pass 0
> to 2 again.
> (riscv_elf_link_hash_table_create): Init restart_relax to FALSE.
> (_bfd_riscv_relax_align): Remove obsolete sec_flg0 set.
> (_bfd_riscv_relax_delete): Set again to TRUE if we do delete the code.
> (bfd_elfNN_riscv_restart_relax_sections): New function. Called by
> after_allocation to check if we need to run the whole relaxations again.
> (_bfd_riscv_relax_section): We will only enter into the relax pass 3 when
> the restart_relax is FALSE; At last set restart_relax to TRUE if again is
> TRUE, too.
> * elfxx-riscv.h (bfd_elf32_riscv_restart_relax_sections): Declaration.
> (bfd_elf64_riscv_restart_relax_sections): Likewise.
> ld/
> * emultempl/riscvelf.em (after_allocation): Run ldelf_map_segments many
> times if riscv_restart_relax_sections returns TRUE.
> * testsuite/ld-riscv-elf/restart-relax.d: New testcase. Before applying
> this patch, the call won't be relaxed to jal; But now we have more chances
> to do relaxations.
> * testsuite/ld-riscv-elf/restart-relax.s: Likewise.
> * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated.
> ---
> bfd/elfnn-riscv.c | 53 ++++++++++++++++++++++++------
> bfd/elfxx-riscv.h | 6 ++++
> ld/emultempl/riscvelf.em | 6 +++-
> ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 1 +
> ld/testsuite/ld-riscv-elf/restart-relax.d | 14 ++++++++
> ld/testsuite/ld-riscv-elf/restart-relax.s | 17 ++++++++++
> 6 files changed, 86 insertions(+), 11 deletions(-)
> create mode 100644 ld/testsuite/ld-riscv-elf/restart-relax.d
> create mode 100644 ld/testsuite/ld-riscv-elf/restart-relax.s
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 20944c8..c326a94 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -123,6 +123,9 @@ struct riscv_elf_link_hash_table
>
> /* The index of the last unused .rel.iplt slot. */
> bfd_vma last_iplt_index;
> +
> + /* Re-run the relaxations from relax pass 0 if TRUE. */
> + bfd_boolean restart_relax;
> };
>
>
> @@ -370,6 +373,7 @@ riscv_elf_link_hash_table_create (bfd *abfd)
> }
>
> ret->max_alignment = (bfd_vma) -1;
> + ret->restart_relax = FALSE;
>
> /* Create hash table for local ifunc. */
> ret->loc_hash_table = htab_try_create (1024,
> @@ -4353,7 +4357,8 @@ _bfd_riscv_relax_tls_le (bfd *abfd,
> }
> }
>
> -/* Implement R_RISCV_ALIGN by deleting excess alignment NOPs. */
> +/* Implement R_RISCV_ALIGN by deleting excess alignment NOPs.
> + Once we've handled an R_RISCV_ALIGN, we can't relax anything else. */
>
> static bfd_boolean
> _bfd_riscv_relax_align (bfd *abfd, asection *sec,
> @@ -4376,9 +4381,6 @@ _bfd_riscv_relax_align (bfd *abfd, asection *sec,
> bfd_vma aligned_addr = ((symval - 1) & ~(alignment - 1)) + alignment;
> bfd_vma nop_bytes = aligned_addr - symval;
>
> - /* Once we've handled an R_RISCV_ALIGN, we can't relax anything else. */
> - sec->sec_flg0 = TRUE;
> -
> /* Make sure there are enough NOPs to actually achieve the alignment. */
> if (rel->r_addend < nop_bytes)
> {
> @@ -4574,7 +4576,7 @@ _bfd_riscv_relax_delete (bfd *abfd,
> bfd_vma symval ATTRIBUTE_UNUSED,
> bfd_vma max_alignment ATTRIBUTE_UNUSED,
> bfd_vma reserve_size ATTRIBUTE_UNUSED,
> - bfd_boolean *again ATTRIBUTE_UNUSED,
> + bfd_boolean *again,
> riscv_pcgp_relocs *pcgp_relocs ATTRIBUTE_UNUSED,
> bfd_boolean undefined_weak ATTRIBUTE_UNUSED)
> {
> @@ -4582,12 +4584,39 @@ _bfd_riscv_relax_delete (bfd *abfd,
> link_info))
> return FALSE;
> rel->r_info = ELFNN_R_INFO(0, R_RISCV_NONE);
> + *again = TRUE;
> return TRUE;
> }
>
> -/* Relax a section. Pass 0 shortens code sequences unless disabled. Pass 1
> - deletes the bytes that pass 0 made obselete. Pass 2, which cannot be
> - disabled, handles code alignment directives. */
> +/* Called by after_allocation to check if we need to run the whole
> + relaxations again. */
> +
> +bfd_boolean
> +bfd_elfNN_riscv_restart_relax_sections (struct bfd_link_info *info)
> +{
> + struct riscv_elf_link_hash_table *htab = riscv_elf_hash_table (info);
> + bfd_boolean restart = htab->restart_relax;
> + /* Reset the flag. */
> + htab->restart_relax = FALSE;
> + return restart;
> +}
> +
> +/* Relax a section.
> +
> + Pass 0: shortens absolute/call/tprel code sequences unless disabled.
> + Pass 1: shortens pcrel code sequences unless disabled.
> + Pass 2: deletes the bytes that pass 1 made obselete.
> + Pass 3: handles code alignment directives, which can not be disabled
> +
> + The `again` is used to determine whether the relax pass itslef needs to
> + run again. And the `restart_relax` is used to determine if we need to
> + run the whole relax passes again from 0 to 2. Once we have deleted the
> + code between relax pass 0 to 2, the restart_relax will be set to TRUE,
> + and we should run the whole relxations again to give them more chances
> + to shorten the code.
> +
> + Since we can't relax anything else once we start to handle the alignments,
> + we will only enter into the relax pass 3 when the restart_relax is FALSE. */
>
> static bfd_boolean
> _bfd_riscv_relax_section (bfd *abfd, asection *sec,
> @@ -4606,11 +4635,12 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec,
> *again = FALSE;
>
> if (bfd_link_relocatable (info)
> - || sec->sec_flg0
> || (sec->flags & SEC_RELOC) == 0
> || sec->reloc_count == 0
> || (info->disable_target_specific_optimizations
> - && info->relax_pass < 2))
> + && info->relax_pass < 2)
> + || (htab->restart_relax
> + && info->relax_pass == 3))
> return TRUE;
>
> riscv_init_pcgp_relocs (&pcgp_relocs);
> @@ -4849,6 +4879,9 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec,
> free (relocs);
> riscv_free_pcgp_relocs(&pcgp_relocs, abfd, sec);
>
> + if (*again)
> + htab->restart_relax = TRUE;
> +
> return ret;
> }
>
> diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> index 4e03ce1..5a34bee 100644
> --- a/bfd/elfxx-riscv.h
> +++ b/bfd/elfxx-riscv.h
> @@ -116,3 +116,9 @@ riscv_get_priv_spec_class_from_numbers (unsigned int,
>
> extern const char *
> riscv_get_priv_spec_name (enum riscv_priv_spec_class);
> +
> +extern bfd_boolean
> +bfd_elf32_riscv_restart_relax_sections (struct bfd_link_info *);
> +
> +extern bfd_boolean
> +bfd_elf64_riscv_restart_relax_sections (struct bfd_link_info *);
> diff --git a/ld/emultempl/riscvelf.em b/ld/emultempl/riscvelf.em
> index 077f976..cdfa1b5 100644
> --- a/ld/emultempl/riscvelf.em
> +++ b/ld/emultempl/riscvelf.em
> @@ -62,7 +62,11 @@ gld${EMULATION_NAME}_after_allocation (void)
> }
> }
>
> - ldelf_map_segments (need_layout);
> + do
> + {
> + ldelf_map_segments (need_layout);
> + }
> + while (bfd_elf${ELFSIZE}_riscv_restart_relax_sections (&link_info));
> }
>
> /* This is a convenient point to tell BFD about target specific flags.
> diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> index 86910e6..9460575 100644
> --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> @@ -68,6 +68,7 @@ if [istarget "riscv*-*-*"] {
> run_dump_test "disas-jalr"
> run_dump_test "pcrel-lo-addend"
> run_dump_test "pcrel-lo-addend-2"
> + run_dump_test "restart-relax"
> run_dump_test "attr-merge-arch-01"
> run_dump_test "attr-merge-arch-02"
> run_dump_test "attr-merge-arch-03"
> diff --git a/ld/testsuite/ld-riscv-elf/restart-relax.d b/ld/testsuite/ld-riscv-elf/restart-relax.d
> new file mode 100644
> index 0000000..57b62eb
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/restart-relax.d
> @@ -0,0 +1,14 @@
> +#source: restart-relax.s
> +#as:
> +#ld:
> +#objdump: -d
> +
> +#...
> +Disassembly of section .text:
> +
> +0+[0-9a-f]+ <_start>:
> +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+.*
> +#...
> +.*:[ ]+[0-9a-f]+[ ]+jal[ ]+ra,[0-9a-f]+ <_start>
> +.*:[ ]+[0-9a-f]+[ ]+add[ ]+a0,a1,a2
> +#pass
> diff --git a/ld/testsuite/ld-riscv-elf/restart-relax.s b/ld/testsuite/ld-riscv-elf/restart-relax.s
> new file mode 100644
> index 0000000..efc881d
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/restart-relax.s
> @@ -0,0 +1,17 @@
> + .text
> + .global _start
> +_start:
> + lla a0, data_g
> +.rept 0x3fffe
> + nop
> +.endr
> + call _start
> + .option rvc
> + .align 2
> + add a0, a1, a2
> +
> + .data
> + .global data_g
> + .dword 0x0
> +data_g:
> + .word 0x1000
> --
> 2.7.4
>
More information about the Binutils
mailing list