Re: [PATCH v2] RISC-V: Optimize relax of GP/call with max_alignment.

[Palmer Dabbelt <palmer@rivosinc.com>]

On Tue, 22 Nov 2022 02:19:00 PST (-0800), nelson@rivosinc.com wrote:
> On Thu, Oct 27, 2022 at 11:19 AM <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).
>>
>> Likewise, the max_alignment defined out of [call-1M, call+1M),
>> the max_alignment shouldn't affect the relax of call.
>> If the symbol is in [call-1M, call+1M), the max_alignment would be
>> replaced with the max_alignment of the section in [call-1M, call+1M).
>
> I'm thinking that - If it is safe enough to decide which output
> section alignments should be considered in the range of gp/pc, but the
> target symbols or the gp are ABS symbols.  If one of them is ABS, then
> that means the output sections in the range of pc/gp may be different
> in the different relax rounds.  So maybe we should,
> 1. make sure the target symbol and gp/pc are not ABS symbols, they
> should belong to a section.
> 2. consider the maximum output section alignment between the output
> sections of the target symbol and gp/pc.
> 3. Otherwise, consider all output section alignments as usual.

It's actually a bit screwier than that, we also have to contend with the 
psABI's mixing of "PC relative" and "position independent".  I've got a 
bug on this: https://sourceware.org/bugzilla/show_bug.cgi?id=28789

>
> Thanks
> Nelson
>
>> bfd/
>>         * elfnn-riscv.c (_bfd_riscv_get_max_alignment_in_jtype): New.
>>         (_bfd_riscv_relax_call): The max_alignment of sections is from
>>         [base-1M, base+1M).
>>         (_bfd_riscv_get_max_alignment_in_itype): 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-call.*: New tests.
>>         * ld/testsuite/ld-riscv-elf/relax-max-align-gp.*: New tests.
>> ---
>>  bfd/elfnn-riscv.c                             | 67 +++++++++++++++++++
>>  ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp    |  2 +
>>  .../ld-riscv-elf/relax-max-align-call.d       | 18 +++++
>>  .../ld-riscv-elf/relax-max-align-call.ld      | 22 ++++++
>>  .../ld-riscv-elf/relax-max-align-call.s       | 23 +++++++
>>  .../ld-riscv-elf/relax-max-align-gp.d         | 46 +++++++++++++
>>  .../ld-riscv-elf/relax-max-align-gp.s         | 29 ++++++++
>>  7 files changed, 207 insertions(+)
>>  create mode 100644 ld/testsuite/ld-riscv-elf/relax-max-align-call.d
>>  create mode 100644 ld/testsuite/ld-riscv-elf/relax-max-align-call.ld
>>  create mode 100644 ld/testsuite/ld-riscv-elf/relax-max-align-call.s
>>  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 cf852636c9c..ed72704bd80 100644
>> --- a/bfd/elfnn-riscv.c
>> +++ b/bfd/elfnn-riscv.c
>> @@ -4266,6 +4266,28 @@ typedef bool (*relax_func_t) (bfd *, asection *, asection *,
>>                               riscv_pcgp_relocs *,
>>                               bool undefined_weak);
>>
>> +/* Traverse all output sections in jtype, and return the max alignment.  */
>> +
>> +static bfd_vma
>> +_bfd_riscv_get_max_alignment_in_jtype (asection *sec, bfd_vma base)
>> +{
>> +  unsigned int max_alignment_power = 0;
>> +  asection *o;
>> +
>> +  if (sec == NULL)
>> +    return 0;
>> +
>> +  for (o = sec->owner->sections; o != NULL; o = o->next)
>> +    {
>> +      if (VALID_JTYPE_IMM (sec_addr(o) - base)
>> +         || VALID_ITYPE_IMM (sec_addr(o) + o->size - base))
>
> Probably should be JTYPE?
>
>
>> +       if (o->alignment_power > max_alignment_power)
>> +         max_alignment_power = o->alignment_power;
>> +    }
>> +
>> +  return (bfd_vma) 1 << max_alignment_power;
>> +}
>> +
>>  /* Relax AUIPC + JALR into JAL.  */
>>
>>  static bool
>> @@ -4294,6 +4316,15 @@ _bfd_riscv_relax_call (bfd *abfd, asection *sec, asection *sym_sec,
>>        if (sym_sec->output_section == sec->output_section
>>           && sym_sec->output_section != bfd_abs_section_ptr)
>>         max_alignment = (bfd_vma) 1 << sym_sec->output_section->alignment_power;
>> +      else
>> +       {
>> +         bfd_vma new_max_alignment =
>> +           _bfd_riscv_get_max_alignment_in_jtype (sec,
>> +                                                  (sec_addr (sec)
>> +                                                   + rel->r_offset));
>> +         if (new_max_alignment)
>> +           max_alignment = new_max_alignment;
>> +       }
>>        foff += ((bfd_signed_vma) foff < 0 ? -max_alignment : max_alignment);
>>      }
>>
>> @@ -4360,6 +4391,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_itype (asection *sec, bfd_vma gp)
>> +{
>> +  unsigned int max_alignment_power = 0;
>> +  asection *o;
>> +
>> +  if (sec == NULL)
>> +    return 0;
>> +
>> +  for (o = sec->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;
>> +    }
>> +
>> +  return (bfd_vma) 1 << max_alignment_power;
>> +}
>> +
>>
>>  /* Relax non-PIC global variable references to GP-relative references.  */
>>
>>  static bool
>> @@ -4391,6 +4444,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;
>> +      else
>> +       {
>> +         /* Otherwise, consider the alignment of sections in [gp-2K,gp+2K). */
>> +         bfd_vma new_max_alignment = _bfd_riscv_get_max_alignment_in_itype (sec, gp);
>> +         if (new_max_alignment)
>> +           max_alignment = new_max_alignment;
>> +       }
>>      }
>>
>>    /* Is the reference in range of x0 or gp?
>> @@ -4656,6 +4716,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;
>> +      else if (!undefined_weak)
>> +       {
>> +         /* Otherwise, consider the alignment of sections in [gp-2K,gp+2K). */
>> +         bfd_vma new_max_alignment = _bfd_riscv_get_max_alignment_in_itype (sec, gp);
>> +         if (new_max_alignment)
>> +           max_alignment = new_max_alignment;
>> +       }
>>      }
>>
>>    /* 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..f38807b991e 100644
>> --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
>> +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
>> @@ -169,6 +169,8 @@ 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_dump_test "relax-max-align-call"
>>      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-call.d b/ld/testsuite/ld-riscv-elf/relax-max-align-call.d
>> new file mode 100644
>> index 00000000000..490ad8c865e
>> --- /dev/null
>> +++ b/ld/testsuite/ld-riscv-elf/relax-max-align-call.d
>> @@ -0,0 +1,18 @@
>> +#source: relax-max-align-call.s
>> +#as: -march=rv64gc
>> +#ld: -Trelax-max-align-call.ld
>> +#objdump: -d
>> +
>> +.*:[   ]+file format .*
>> +
>> +
>> +Disassembly of section .text1:
>> +
>> +0000000000100000 <_start>:
>> +[      ]+100000:[      ]+79dff0ef[     ]+jal[  ]+1fff9c <foo>
>> +[      ]+100004:[      ]+a001[         ]+j[    ]+100004 <_start\+0x4>
>> +
>> +Disassembly of section .text2:
>> +
>> +00000000001fff9c <foo>:
>> +[      ]+1fff9c:[      ]+8082[         ]+ret
>> diff --git a/ld/testsuite/ld-riscv-elf/relax-max-align-call.ld b/ld/testsuite/ld-riscv-elf/relax-max-align-call.ld
>> new file mode 100644
>> index 00000000000..91991ed0f41
>> --- /dev/null
>> +++ b/ld/testsuite/ld-riscv-elf/relax-max-align-call.ld
>> @@ -0,0 +1,22 @@
>> +
>> +ENTRY(_start)
>> +
>> +SECTIONS
>> +{
>> +       . = 0x100000;
>> +
>> +       .text1 : {
>> +               *(.text1)
>> +       }
>> +
>> +       . = 0x200000 - 100;
>> +
>> +       .text2 : {
>> +               *(.text2)
>> +       }
>> +
>> +       . = 0x300000;
>> +       .data : {
>> +               *(.data)
>> +       }
>> +}
>> diff --git a/ld/testsuite/ld-riscv-elf/relax-max-align-call.s b/ld/testsuite/ld-riscv-elf/relax-max-align-call.s
>> new file mode 100644
>> index 00000000000..3335c0970e7
>> --- /dev/null
>> +++ b/ld/testsuite/ld-riscv-elf/relax-max-align-call.s
>> @@ -0,0 +1,23 @@
>> +
>> +.section .text1, "ax"
>> +.global _start
>> +.align 2
>> +_start:
>> +       call foo
>> +       j .
>> +       .size _start, . - _start
>> +
>> +.section .text2, "ax"
>> +.align 2
>> +.global foo
>> +.type foo, @function
>> +foo:
>> +       ret
>> +       .size foo, . - foo
>> +
>> +.data
>> +.global gdata
>> +.align 10
>> +gdata:
>> +       .long 0
>> +       .size gdata, . - gdata
>> 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[  ]+.*<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-gp.s b/ld/testsuite/ld-riscv-elf/relax-max-align-gp.s
>> new file mode 100644
>> index 00000000000..0d162ff4d93
>> --- /dev/null
>> +++ b/ld/testsuite/ld-riscv-elf/relax-max-align-gp.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.37.0 (Apple Git-136)
>>