From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x2f.google.com (mail-oa1-x2f.google.com [IPv6:2001:4860:4864:20::2f]) by sourceware.org (Postfix) with ESMTPS id 970C43858421 for ; Fri, 21 Apr 2023 07:50:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 970C43858421 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com Received: by mail-oa1-x2f.google.com with SMTP id 586e51a60fabf-1878f1ebf46so1122207fac.1 for ; Fri, 21 Apr 2023 00:50:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20221208.gappssmtp.com; s=20221208; t=1682063416; x=1684655416; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=npq8zjLu3UCYRGWhOBdUV+h8LJt6TKSBPYUSgxlClxM=; b=oeau8jn37m9UsoLJd+EcnGnQafcrkOzOGUCf6jugYGCzn1wdnosMJis1vOtimzLgZc jl+LZxhsG93GwzuInzMtMp/Vif5OlC/IyGac0K9YcECS3XhTdarDqWbHHVpPqivXFPQY /MZVftRmiOYXzkpZ6drS8xqkXSDlUvJkAbIaae528u5DtcFTtsoCPCn23tP44UX8eLhl 5Ol0j39/9AHAvM6tQLmjtsfAan84vBOkSHk9eF8LEFMzOKWEE8u3DvZUv0oNGPUldd/P JEfoKdjetSi8TP0Ap1QKmAJtljKMsYRruEQxao+bsN3NcqoJtqn70vBZCtFAAf/RzLap cWyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682063416; x=1684655416; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=npq8zjLu3UCYRGWhOBdUV+h8LJt6TKSBPYUSgxlClxM=; b=D1VPKZy9u64gGOfdCOiUAv2VchbGM1rpQIeXtd1VvBih6AD8iCe6Pq2tF30zGWjB7j qtdwz8m5QlytsDXq1mYolEVMQuQvXnl1MmkD811Q3QXfFGu/IhP3GRaiI8N/3cI46T/A i8oX/KyjH5cIa0nmZhGBRvK/B2l/Kvz5XHI3qgPmo64/iFCj+H+C1xPFsyU26/EhsOyr D5RlkFHRkl0QXrnh6SYYB7pknTxNbXG0VnOdGRDKyBCRfN+1rt59/Io/vRCDbigq3ayc vixNWsx+Qse0pHvxeVZPUZW0oRpRar7SPMV8HPGNAM1lTsbtz6HfRc76D85+QVS10wdn vExA== X-Gm-Message-State: AAQBX9fGZsQGZaIhYj7YDwJbdxsPCJDc7HyMaq2cBO49sdOxvtjHgUHK DiLOh403YpH41W5Wwo9uNrkvu2rEYLeeTfpeYcmhyw5CNEvYn3+0SYQ= X-Google-Smtp-Source: AKy350bTwG3BNtU+T0NIRl9ewtO9lF5cIWR1gbtZgAjFqdGX1nKF270T89X8ZVeVzr7M8peDx8Dsabh+fF3SvnAqlRA= X-Received: by 2002:a05:6808:48f:b0:38c:68db:2da4 with SMTP id z15-20020a056808048f00b0038c68db2da4mr2339211oid.50.1682063416623; Fri, 21 Apr 2023 00:50:16 -0700 (PDT) MIME-Version: 1.0 References: <20230417095443.73581-1-nelson@rivosinc.com> In-Reply-To: <20230417095443.73581-1-nelson@rivosinc.com> From: Nelson Chu Date: Fri, 21 Apr 2023 15:50:05 +0800 Message-ID: Subject: Re: [PATCH] RISC-V: Optimize relaxation of gp with max_alignment. To: binutils@sourceware.org, jim.wilson.gcc@gmail.com, palmer@dabbelt.com, lifang_xia@linux.alibaba.com Content-Type: multipart/alternative; boundary="0000000000007018be05f9d3e819" X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,GIT_PATCH_0,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: --0000000000007018be05f9d3e819 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Committed, thanks. Nelson On Mon, Apr 17, 2023 at 5:54=E2=80=AFPM Nelson Chu wr= ote: > 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 id= ea > 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 a= nd > 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+2= K) > 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 th= at > 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 =3D (bfd_vma) -1; > + ret->max_alignment_for_gp =3D (bfd_vma) -1; > > /* Create hash table for local ifunc. */ > ret->loc_hash_table =3D 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 =3D 0; > asection *o; > > for (o =3D sec->output_section->owner->sections; o !=3D NULL; o =3D o-= >next) > { > - if (o->alignment_power > max_alignment_power) > + bool valid =3D true; > + if (gp > + && !(VALID_ITYPE_IMM (sec_addr (o) - gp) > + || VALID_ITYPE_IMM (sec_addr (o) + o->size - gp))) > + valid =3D false; > + > + if (valid && o->alignment_power > max_alignment_power) > max_alignment_power =3D 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 =3D riscv_elf_hash_table > (link_info); > bfd_byte *contents =3D elf_section_data (sec)->this_hdr.contents; > - bfd_vma gp =3D riscv_elf_hash_table (link_info)->params->relax_gp > - ? riscv_global_pointer_value (link_info) > - : 0; > + bfd_vma gp =3D htab->params->relax_gp > + ? riscv_global_pointer_value (link_info) > + : 0; > int use_rvc =3D elf_elfheader (abfd)->e_flags & EF_RISCV_RVC; > > BFD_ASSERT (rel->r_offset + 4 <=3D 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 =3D=3D sym_sec->output_section > && sym_sec->output_section !=3D bfd_abs_section_ptr) > max_alignment =3D (bfd_vma) 1 << > sym_sec->output_section->alignment_power; > + else > + { > + /* Consider output section alignments which are in [gp-2K, > gp+2K). */ > + max_alignment =3D htab->max_alignment_for_gp; > + if (max_alignment =3D=3D (bfd_vma) -1) > + { > + max_alignment =3D _bfd_riscv_get_max_alignment (sec, gp); > + htab->max_alignment_for_gp =3D 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 >=3D 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 >=3D gp > + && VALID_ITYPE_IMM (symval - gp + max_alignment + reserve_size)) > + || (symval < gp > + && VALID_ITYPE_IMM (symval - gp - max_alignment - reserve_size)= )) > { > unsigned sym =3D 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 =3D riscv_elf_hash_table > (link_info); > bfd_byte *contents =3D elf_section_data (sec)->this_hdr.contents; > bfd_vma gp =3D 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 =3D=3D sym_sec->output_section > && sym_sec->output_section !=3D bfd_abs_section_ptr) > max_alignment =3D (bfd_vma) 1 << > sym_sec->output_section->alignment_power; > + else > + { > + /* Consider output section alignments which are in [gp-2K, > gp+2K). */ > + max_alignment =3D htab->max_alignment_for_gp; > + if (max_alignment =3D=3D (bfd_vma) -1) > + { > + max_alignment =3D _bfd_riscv_get_max_alignment (sec, gp); > + htab->max_alignment_for_gp =3D 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 >=3D 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 >=3D gp > + && VALID_ITYPE_IMM (symval - gp + max_alignment + reserve_size)) > + || (symval < gp > + && VALID_ITYPE_IMM (symval - gp - max_alignment - reserve_size)= )) > { > unsigned sym =3D 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 =3D 0; > riscv_pcgp_relocs pcgp_relocs; > + static asection *first_section =3D NULL; > > *again =3D false; > > @@ -4892,6 +4933,13 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec, > || *(htab->data_segment_phase) =3D=3D 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 =3D=3D NULL) > + first_section =3D sec; > + else if (first_section =3D=3D sec) > + htab->max_alignment_for_gp =3D -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 *se= c, > info->keep_memory))) > goto fail; > > - if (htab) > + /* Estimate the maximum alignment for all output sections once time > + should be enough. */ > + max_alignment =3D htab->max_alignment; > + if (max_alignment =3D=3D (bfd_vma) -1) > { > - max_alignment =3D htab->max_alignment; > - if (max_alignment =3D=3D (bfd_vma) -1) > - { > - max_alignment =3D _bfd_riscv_get_max_alignment (sec); > - htab->max_alignment =3D max_alignment; > - } > + max_alignment =3D _bfd_riscv_get_max_alignment (sec, 0/* gp */); > + htab->max_alignment =3D max_alignment; > } > - else > - max_alignment =3D _bfd_riscv_get_max_alignment (sec); > > /* Examine and consider relaxing each reloc. */ > for (i =3D 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=3Drv32i -mabi=3Dilp32" {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) > > --0000000000007018be05f9d3e819--