public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Lifang Xia <lifang_xia@linux.alibaba.com>
To: Nelson Chu <nelson@rivosinc.com>
Cc: binutils@sourceware.org, Palmer Dabbelt <palmer@rivosinc.com>,
	Greg McGary <gkm@rivosinc.com>
Subject: Re: [PATCH] RISC-V: Optimize relax of GP with max_alignment.
Date: Fri, 23 Sep 2022 17:09:48 +0800	[thread overview]
Message-ID: <82abef39-0f8b-4ec0-6145-39c083ffb24c@linux.alibaba.com> (raw)
In-Reply-To: <CAPpQWtDemjf+qTJtn1P=stAsPnbSRmdD-1Dq3wyJRpSPfX4YBw@mail.gmail.com>


On 2022/9/23 16:05, Nelson Chu wrote:
> On Fri, Sep 23, 2022 at 2:48 PM <lifang_xia@linux.alibaba.com> wrote:
>> From: Lifang Xia <lifang_xia@linux.alibaba.com>
>>
>> The max_alignment defined out of [gp-2K, gp+2k), the max_alignment
>> shouldn't affect the relax of gp.
>> If the symbol is in [gp-2K, gp+2k), the max_alignment would be
>> replaced with the max_alignment of the section in [gp-2k, gp+2k).
> This idea should works!  But I need to pass the riscv-gnu-toolchain
> regression at least, though the regression shouldn't be broken by the
> change.
> Btw, also cc Palmer and Greg that they may have interested in and have
> some though on this.
Thanks for this. And thanks for the review, I'll update the patch v2 later.
> Thanks
> Nelson
>
>> bfd/
>>          * elfnn-riscv.c (_bfd_riscv_get_max_alignment_in_gp): New.
>>          (_bfd_riscv_relax_lui): The max_alignment of sections is from
>>          [gp-2K, gp+2K).
>>          (_bfd_riscv_relax_pc): Likewise.
>> ld/
>>          * ld/testsuite/ld-riscv-elf/relax-max-align.*: New tests.
>> ---
>>   bfd/elfnn-riscv.c                           | 36 ++++++++++++++++
>>   ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp  |  1 +
>>   ld/testsuite/ld-riscv-elf/relax-max-align.d | 46 +++++++++++++++++++++
>>   ld/testsuite/ld-riscv-elf/relax-max-align.s | 29 +++++++++++++
>>   4 files changed, 112 insertions(+)
>>   create mode 100644 ld/testsuite/ld-riscv-elf/relax-max-align.d
>>   create mode 100644 ld/testsuite/ld-riscv-elf/relax-max-align.s
>>
>> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
>> index 3d2ddf4e651..76f888d805c 100644
>> --- a/bfd/elfnn-riscv.c
>> +++ b/bfd/elfnn-riscv.c
>> @@ -4262,6 +4262,28 @@ _bfd_riscv_get_max_alignment (asection *sec)
>>     return (bfd_vma) 1 << max_alignment_power;
>>   }
>>
>> +/* Traverse all output sections in [gp-2K, gp+2K) and return the max alignment.  */
>> +
>> +static bfd_vma
>> +_bfd_riscv_get_max_alignment_in_gp (asection *sec, bfd_vma gp)
>> +{
>> +  unsigned int max_alignment_power = 0;
>> +  asection *o;
>> +
>> +  if (sec == NULL)
>> +    return 0;
>> +
>> +  for (o = sec->output_section->owner->sections; o != NULL; o = o->next)
>> +    {
>> +      if (VALID_ITYPE_IMM (sec_addr(o) - gp)
>> +         || VALID_ITYPE_IMM (sec_addr(o) + o->size - gp))
>> +       if (o->alignment_power > max_alignment_power)
>> +         max_alignment_power = o->alignment_power;
>> +    }
> probably can use only one if rather than two.
>
>> +  return (bfd_vma) 1 << max_alignment_power;
>> +}
>> +
>>   /* Relax non-PIC global variable references to GP-relative references.  */
>>
>>   static bool
>> @@ -4293,6 +4315,13 @@ _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;
>> +
>> +      /* Otherwise, consider the alignment of sections in [gp-2K,gp+2K). */
>> +      else if (!undefined_weak){
>> +         bfd_vma new_max_alignment = _bfd_riscv_get_max_alignment_in_gp (sym_sec, gp);
>> +         if (new_max_alignment)
>> +           max_alignment = new_max_alignment;
>> +      }
>>       }
> if (...)
>    max_alignment = ...
> else if (!undefined_weak)
> {
>    /* Otherwise, ...  */
>    ...
> }
>
>>     /* Is the reference in range of x0 or gp?
>> @@ -4559,6 +4588,13 @@ _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;
>> +
>> +      /* Otherwise, consider the alignment of sections in [gp-2K,gp+2K). */
>> +      else if (!undefined_weak){
>> +         bfd_vma new_max_alignment = _bfd_riscv_get_max_alignment_in_gp (sym_sec, gp);
>> +         if (new_max_alignment)
>> +           max_alignment = new_max_alignment;
>> +      }
>>       }
> Likewise, it is minor though.
>
>>     /* Is the reference in range of x0 or gp?
>> diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
>> index df89e0ee68b..e53f16facfa 100644
>> --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
>> +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
>> @@ -169,6 +169,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"
>>       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.d b/ld/testsuite/ld-riscv-elf/relax-max-align.d
>> new file mode 100644
>> index 00000000000..4ba920da970
>> --- /dev/null
>> +++ b/ld/testsuite/ld-riscv-elf/relax-max-align.d
>> @@ -0,0 +1,46 @@
>> +#source: relax-max-align.s
>> +#ld:
>> +#objdump: -d
>> +
>> +.*:[   ]+file format .*
>> +
>> +
>> +Disassembly of section .text:
>> +
>> +0+[0-9a-f]+ <_start>:
>> +.*:[   ]+[0-9a-f]+[    ]+addi[         ]+.*<gdata>
>> +.*:[   ]+[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]+ <func>:
>> +.*:[   ]+[0-9a-f]+[    ]+ret
>> +[      ]+...
>> diff --git a/ld/testsuite/ld-riscv-elf/relax-max-align.s b/ld/testsuite/ld-riscv-elf/relax-max-align.s
>> new file mode 100644
>> index 00000000000..e8abe0f34a1
>> --- /dev/null
>> +++ b/ld/testsuite/ld-riscv-elf/relax-max-align.s
>> @@ -0,0 +1,29 @@
>> +
>> +.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.32.1 (Apple Git-133)
>>

  reply	other threads:[~2022-09-23  9:09 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-23  6:47 lifang_xia
2022-09-23  8:05 ` Nelson Chu
2022-09-23  9:09   ` Lifang Xia [this message]
2022-09-23  9:46 ` lifang_xia
2022-09-23 10:44   ` Nelson Chu
2022-09-24  1:54     ` Lifang Xia
2022-09-27  8:51       ` Nelson Chu
2022-10-27  3:19       ` [PATCH v2] RISC-V: Optimize relax of GP/call " lifang_xia
2022-11-09  2:52         ` Lifang Xia
2022-11-22 10:19         ` Nelson Chu
2022-11-22 16:19           ` Palmer Dabbelt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=82abef39-0f8b-4ec0-6145-39c083ffb24c@linux.alibaba.com \
    --to=lifang_xia@linux.alibaba.com \
    --cc=binutils@sourceware.org \
    --cc=gkm@rivosinc.com \
    --cc=nelson@rivosinc.com \
    --cc=palmer@rivosinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).