Committed, thanks. Nelson On Mon, Apr 17, 2023 at 5:54 PM Nelson Chu wrote: > From: Lifang Xia > > This should be the first related issue, which posted in > riscv-gnu-toolchain, > https://github.com/riscv-collab/riscv-gnu-toolchain/issues/497 > > If the output sections are not between gp and the symbol, then their > alignments > shouldn't affect the gp relaxation. However, this patch improves this idea > even more, it limits the range to the gp+-2k, which means only the output > section which are in the [gp-2K, gp+2K) range need to be considered. > > Even if the output section candidates may be different for each relax > passes, > the symbol that can be relaxed ar this round will not be truncated at next > round. That is because this round you can do relax to indicate that the > section where the symbol is located is within the [gp-2K, gp+2K) range, so > all > the output section alignments between them should be considered. In other > words, if the alignments between them may cause truncated, then we should > already preserve the size and won't do the gp relaxation this time. > > This patch can resolve the github issue which mentioned above, and also > passed > all gcc/binutils regressions of riscv-gnu-toolchain, so should be worth and > safe enough to commit. > > Originally, this patch also do the same optimization for the call > relaxations, > https://sourceware.org/pipermail/binutils/2022-October/123918.html > But just in case there is something that has not been considered, we only > deal with the gp relaxation at this time. > > bfd/ > * elfnn-riscv.c (riscv_elf_link_hash_table): Added new bfd_vma, > max_alignment_for_gp. It is used to record the maximum alignment > of > the output sections, which are in the [gp-2K, gp+2k) range. > (riscv_elf_link_hash_table_create): Init max_alignment_for_gp to > -1. > (_bfd_riscv_get_max_alignment): Added new parameter, gp. If gp is > zero, then all the output section alignments are possible > candidates; > Otherwise, only the output sections which are in the [gp-2K, gp+2K) > range need to be considered. > (_bfd_riscv_relax_lui): Called _bfd_riscv_get_max_alignment with > the > non-zero gp if the max_alignment_for_gp is -1. > (_bfd_riscv_relax_pc): Likewise. > (_bfd_riscv_relax_section): Record the first input section, so that > we can reset the max_alignment_for_gp for each repeated relax > passes. > ld/ > * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated. > * testsuite/ld-riscv-elf/relax-max-align-gp.*: New testcase. It > fails > without this patch. > > Co-developed-by: Nelson Chu > --- > bfd/elfnn-riscv.c | 103 +++++++++++++----- > ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 1 + > .../ld-riscv-elf/relax-max-align-gp.d | 46 ++++++++ > .../ld-riscv-elf/relax-max-align-gp.s | 28 +++++ > 4 files changed, 149 insertions(+), 29 deletions(-) > create mode 100644 ld/testsuite/ld-riscv-elf/relax-max-align-gp.d > create mode 100644 ld/testsuite/ld-riscv-elf/relax-max-align-gp.s > > diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c > index 0dd9b27c8ae..e90e36b58bb 100644 > --- a/bfd/elfnn-riscv.c > +++ b/bfd/elfnn-riscv.c > @@ -200,6 +200,9 @@ struct riscv_elf_link_hash_table > /* The max alignment of output sections. */ > bfd_vma max_alignment; > > + /* The max alignment of output sections in [gp-2K, gp+2K) range. */ > + bfd_vma max_alignment_for_gp; > + > /* Used by local STT_GNU_IFUNC symbols. */ > htab_t loc_hash_table; > void * loc_hash_memory; > @@ -488,6 +491,7 @@ riscv_elf_link_hash_table_create (bfd *abfd) > } > > ret->max_alignment = (bfd_vma) -1; > + ret->max_alignment_for_gp = (bfd_vma) -1; > > /* Create hash table for local ifunc. */ > ret->loc_hash_table = htab_try_create (1024, > @@ -4460,17 +4464,27 @@ _bfd_riscv_relax_call (bfd *abfd, asection *sec, > asection *sym_sec, > link_info, pcgp_relocs, rel + 1); > } > > -/* Traverse all output sections and return the max alignment. */ > +/* Traverse all output sections and return the max alignment. > + > + If gp is zero, then all the output section alignments are > + possible candidates; Otherwise, only the output sections > + which are in the [gp-2K, gp+2K) range need to be considered. */ > > static bfd_vma > -_bfd_riscv_get_max_alignment (asection *sec) > +_bfd_riscv_get_max_alignment (asection *sec, bfd_vma gp) > { > unsigned int max_alignment_power = 0; > asection *o; > > for (o = sec->output_section->owner->sections; o != NULL; o = o->next) > { > - if (o->alignment_power > max_alignment_power) > + bool valid = true; > + if (gp > + && !(VALID_ITYPE_IMM (sec_addr (o) - gp) > + || VALID_ITYPE_IMM (sec_addr (o) + o->size - gp))) > + valid = false; > + > + if (valid && o->alignment_power > max_alignment_power) > max_alignment_power = o->alignment_power; > } > > @@ -4492,15 +4506,16 @@ _bfd_riscv_relax_lui (bfd *abfd, > riscv_pcgp_relocs *pcgp_relocs, > bool undefined_weak) > { > + struct riscv_elf_link_hash_table *htab = riscv_elf_hash_table > (link_info); > bfd_byte *contents = elf_section_data (sec)->this_hdr.contents; > - bfd_vma gp = riscv_elf_hash_table (link_info)->params->relax_gp > - ? riscv_global_pointer_value (link_info) > - : 0; > + bfd_vma gp = htab->params->relax_gp > + ? riscv_global_pointer_value (link_info) > + : 0; > int use_rvc = elf_elfheader (abfd)->e_flags & EF_RISCV_RVC; > > BFD_ASSERT (rel->r_offset + 4 <= sec->size); > > - if (gp) > + if (!undefined_weak && gp) > { > /* If gp and the symbol are in the same output section, which is > not the > abs section, then consider only that output section's alignment. > */ > @@ -4510,16 +4525,28 @@ _bfd_riscv_relax_lui (bfd *abfd, > if (h->u.def.section->output_section == sym_sec->output_section > && sym_sec->output_section != bfd_abs_section_ptr) > max_alignment = (bfd_vma) 1 << > sym_sec->output_section->alignment_power; > + else > + { > + /* Consider output section alignments which are in [gp-2K, > gp+2K). */ > + max_alignment = htab->max_alignment_for_gp; > + if (max_alignment == (bfd_vma) -1) > + { > + max_alignment = _bfd_riscv_get_max_alignment (sec, gp); > + htab->max_alignment_for_gp = max_alignment; > + } > + } > } > > /* Is the reference in range of x0 or gp? > - Valid gp range conservatively because of alignment issue. */ > + Valid gp range conservatively because of alignment issue. > + > + Should we also consider the alignment issue for x0 base? */ > if (undefined_weak > - || (VALID_ITYPE_IMM (symval) > - || (symval >= gp > - && VALID_ITYPE_IMM (symval - gp + max_alignment + > reserve_size)) > - || (symval < gp > - && VALID_ITYPE_IMM (symval - gp - max_alignment - > reserve_size)))) > + || VALID_ITYPE_IMM (symval) > + || (symval >= gp > + && VALID_ITYPE_IMM (symval - gp + max_alignment + reserve_size)) > + || (symval < gp > + && VALID_ITYPE_IMM (symval - gp - max_alignment - reserve_size))) > { > unsigned sym = ELFNN_R_SYM (rel->r_info); > switch (ELFNN_R_TYPE (rel->r_info)) > @@ -4709,6 +4736,7 @@ _bfd_riscv_relax_pc (bfd *abfd ATTRIBUTE_UNUSED, > riscv_pcgp_relocs *pcgp_relocs, > bool undefined_weak) > { > + struct riscv_elf_link_hash_table *htab = riscv_elf_hash_table > (link_info); > bfd_byte *contents = elf_section_data (sec)->this_hdr.contents; > bfd_vma gp = riscv_global_pointer_value (link_info); > > @@ -4765,7 +4793,7 @@ _bfd_riscv_relax_pc (bfd *abfd ATTRIBUTE_UNUSED, > abort (); > } > > - if (gp) > + if (!undefined_weak && gp) > { > /* If gp and the symbol are in the same output section, which is > not the > abs section, then consider only that output section's alignment. > */ > @@ -4775,16 +4803,28 @@ _bfd_riscv_relax_pc (bfd *abfd ATTRIBUTE_UNUSED, > if (h->u.def.section->output_section == sym_sec->output_section > && sym_sec->output_section != bfd_abs_section_ptr) > max_alignment = (bfd_vma) 1 << > sym_sec->output_section->alignment_power; > + else > + { > + /* Consider output section alignments which are in [gp-2K, > gp+2K). */ > + max_alignment = htab->max_alignment_for_gp; > + if (max_alignment == (bfd_vma) -1) > + { > + max_alignment = _bfd_riscv_get_max_alignment (sec, gp); > + htab->max_alignment_for_gp = max_alignment; > + } > + } > } > > /* Is the reference in range of x0 or gp? > - Valid gp range conservatively because of alignment issue. */ > + Valid gp range conservatively because of alignment issue. > + > + Should we also consider the alignment issue for x0 base? */ > if (undefined_weak > - || (VALID_ITYPE_IMM (symval) > - || (symval >= gp > - && VALID_ITYPE_IMM (symval - gp + max_alignment + > reserve_size)) > - || (symval < gp > - && VALID_ITYPE_IMM (symval - gp - max_alignment - > reserve_size)))) > + || VALID_ITYPE_IMM (symval) > + || (symval >= gp > + && VALID_ITYPE_IMM (symval - gp + max_alignment + reserve_size)) > + || (symval < gp > + && VALID_ITYPE_IMM (symval - gp - max_alignment - reserve_size))) > { > unsigned sym = hi_reloc.hi_sym; > switch (ELFNN_R_TYPE (rel->r_info)) > @@ -4877,6 +4917,7 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec, > unsigned int i; > bfd_vma max_alignment, reserve_size = 0; > riscv_pcgp_relocs pcgp_relocs; > + static asection *first_section = NULL; > > *again = false; > > @@ -4892,6 +4933,13 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec, > || *(htab->data_segment_phase) == 4) > return true; > > + /* Record the first relax section, so that we can reset the > + max_alignment_for_gp for the repeated relax passes. */ > + if (first_section == NULL) > + first_section = sec; > + else if (first_section == sec) > + htab->max_alignment_for_gp = -1; > + > riscv_init_pcgp_relocs (&pcgp_relocs); > > /* Read this BFD's relocs if we haven't done so already. */ > @@ -4901,17 +4949,14 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec, > info->keep_memory))) > goto fail; > > - if (htab) > + /* Estimate the maximum alignment for all output sections once time > + should be enough. */ > + max_alignment = htab->max_alignment; > + if (max_alignment == (bfd_vma) -1) > { > - max_alignment = htab->max_alignment; > - if (max_alignment == (bfd_vma) -1) > - { > - max_alignment = _bfd_riscv_get_max_alignment (sec); > - htab->max_alignment = max_alignment; > - } > + max_alignment = _bfd_riscv_get_max_alignment (sec, 0/* gp */); > + htab->max_alignment = max_alignment; > } > - else > - max_alignment = _bfd_riscv_get_max_alignment (sec); > > /* Examine and consider relaxing each reloc. */ > for (i = 0; i < sec->reloc_count; i++) > diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > index 43572c5286b..9e103b283f7 100644 > --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > @@ -171,6 +171,7 @@ if [istarget "riscv*-*-*"] { > run_dump_test "attr-merge-priv-spec-failed-05" > run_dump_test "attr-merge-priv-spec-failed-06" > run_dump_test "attr-phdr" > + run_dump_test "relax-max-align-gp" > run_ld_link_tests [list \ > [list "Weak reference 32" "-T weakref.ld > -m[riscv_choose_ilp32_emul]" "" \ > "-march=rv32i -mabi=ilp32" {weakref32.s} \ > diff --git a/ld/testsuite/ld-riscv-elf/relax-max-align-gp.d > b/ld/testsuite/ld-riscv-elf/relax-max-align-gp.d > new file mode 100644 > index 00000000000..637de426ee4 > --- /dev/null > +++ b/ld/testsuite/ld-riscv-elf/relax-max-align-gp.d > @@ -0,0 +1,46 @@ > +#source: relax-max-align-gp.s > +#ld: > +#objdump: -d > + > +.*:[ ]+file format .* > + > + > +Disassembly of section .text: > + > +0+[0-9a-f]+ <_start>: > +.*:[ ]+[0-9a-f]+[ ]+add[ ]+.* > +.*:[ ]+[0-9a-f]+[ ]+jal[ ]+.* > +.*:[ ]+[0-9a-f]+[ ]+j[ ]+.* > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > +.*:[ ]+[0-9a-f]+[ ]+nop > + > +0+[0-9a-f]+ : > +.*:[ ]+[0-9a-f]+[ ]+ret > +[ ]+... > diff --git a/ld/testsuite/ld-riscv-elf/relax-max-align-gp.s > b/ld/testsuite/ld-riscv-elf/relax-max-align-gp.s > new file mode 100644 > index 00000000000..ce3da21e7f4 > --- /dev/null > +++ b/ld/testsuite/ld-riscv-elf/relax-max-align-gp.s > @@ -0,0 +1,28 @@ > + > +.global _start > +_start: > + lui a0, %hi(gdata) > + addi a0, a0, %lo(gdata) > + call func > + j . > + .size _start, . - _start > + > +.global func > +.align 7 > +func: > + ret > + .size func, . - func > + > +.data > +padding: > + .long 0 > + .long 0 > + .long 0 > + .long 0 > + .size padding, . - padding > + > +.global gdata > +.type gdata, object > +gdata: > + .zero 4 > + .size gdata, . - gdata > -- > 2.39.2 (Apple Git-143) > >