RISC-V: Improve multiple relax passes problem.

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

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index d3c1d403529b26a84850c205f508aeb09f7da476..c184a4e72fcda5e574fd1fbf525e8126ed8777f1 100644 (file)
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,19 @@
+2021-03-11  Nelson Chu  <nelson.chu@sifive.com>
+
+       * 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.
+
 2021-03-10  Jan Beulich  <jbeulich@suse.com>
 
        * cofflink.c (_bfd_coff_write_global_sym): Range-check symbol

diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 76d29b733321e130fd85bcc3162ab9eabfc68309..364d67b6749bf7d929668a740ae73f174f82426e 100644 (file)
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -131,6 +131,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;
 };
 
 /* Instruction access functions. */
@@ -399,6 +402,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,
@@ -4365,7 +4369,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,
@@ -4388,9 +4393,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)
     {
@@ -4586,7 +4588,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)
 {
@@ -4594,15 +4596,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;
 }
 
+/* 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 code sequences for LUI/CALL/TPREL relocs.
    Pass 1: Shortens code sequences for PCREL relocs.
-   Pass 2: Deletes the bytes that pass 1 made obselete.
-   Pass 3: Which cannot be disabled, handles code alignment directives.  */
+   Pass 2: Deletes the bytes that pass 1 made obsolete.
+   Pass 3: Which cannot be disabled, handles code alignment directives.
+
+   The `again` is used to determine whether the relax pass itself 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 relaxations 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,
@@ -4621,11 +4647,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);
@@ -4864,6 +4891,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 a7d348ca020fdd17d855ecdacedc05b989d1808d..a676d5cf757aff3b0d28ecedc6f47f5edf07adf4 100644 (file)
--- a/bfd/elfxx-riscv.h
+++ b/bfd/elfxx-riscv.h
@@ -105,3 +105,9 @@ riscv_get_prefix_class (const char *);
 
 extern int
 riscv_compare_subsets (const char *, const char *);
+
+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/ChangeLog b/ld/ChangeLog
index ec7854163fa72c6a9f7b1246e7076162dab203f8..b5298b9848edddab14e6c0600d82cb4936c4057a 100644 (file)
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,13 @@
+2021-03-11  Nelson Chu  <nelson.chu@sifive.com>
+
+       * 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.
+
 2021-03-10  Jan Beulich  <jbeulich@suse.com>
 
        * testsuite/ld-scripts/pr22267.t: Avoid symbol value with more

diff --git a/ld/emultempl/riscvelf.em b/ld/emultempl/riscvelf.em
index 735eae0a52baeef57fc137890d0b9f452852fc7c..5fa7c773fc88f94f3bc6efa5c7bbf88ebe2201ab 100644 (file)
--- 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 7081af1e8e3622abf76e4c2c8f1e77e66aff7e07..f3ff95cf96ec79c24fedee5823805e32cc6f62ec 100644 (file)
--- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
+++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
@@ -86,6 +86,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 (file)
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 (file)
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