Re: [PATCH] [RISC-V] Optimize GP-relative addressing for linker.

[Kito Cheng <kito.cheng@sifive.com>]

Testcase in your git commit has a base offset, but the testcase in the
code only contains a base with no offset?

I am a little concerned about the case with an offset like "lui
a5,%hi(global_array+256) addi    a5,a5,%lo(global_array+256)" is still
right for this optimization?

On Tue, Jun 27, 2023 at 8:47 PM Die Li <lidie@eswincomputing.com> wrote:
>
> Consider the following test:
>
> //test.c file
>
> struct Person {
>     char name[20];
>     int age;
>     int nation;
>     int hobby[100];
> };
>
> int global_var;
> int global_array[920];
> struct Person person;
>
> int main() {
>     global_var = 3;
>     global_array[365] = 16;
>     sprintf(person.name, "Lee");
>     person.age = 27;
>     person.nation = 77;
>     return 0;
> }
>
> Cflags:
> -Xlinker --relax
>
> Link relaxation can be turned on by the above option, and in fact
> is turned on by default. After compiling, linking, and disassembling
> the test files, there are the following results:
>
> Before this patch:
> Disassembly of section \.text:
> 0+[0-9a-f]+ <main>:
> .*:[    ]+[0-9a-f]+[    ]+sw[   ]+[0-9a-f]+,gp,\-[0-9]+ # [0-9a-f]+ <global_var>
> .*:[    ]+[0-9a-f]+[    ]+c\.lui[       ]+.*
> .*:[    ]+[0-9a-f]+[    ]+addi[         ]+[0-9a-f]+,[0-9a-f]+,[0-9]+ # [0-9a-f]+ <global_array>
> .*:[    ]+[0-9a-f]+[    ]+c\.lui[       ]+.*
> .*:[    ]+[0-9a-f]+[    ]+addi[         ]+[0-9a-f]+,[0-9a-f]+,[0-9]+ # [0-9a-f]+ <person>
>
> After this patch:
> Disassembly of section \.text:
> 0+[0-9a-f]+ <main>:
> .*:[    ]+[0-9a-f]+[    ]+sw[   ]+[0-9a-f]+,gp,\-[0-9]+ # [0-9a-f]+ <global_var>
> .*:[    ]+[0-9a-f]+[    ]+addi[         ]+[0-9a-f]+,gp,\-[0-9]+ # [0-9a-f]+ <global_array>
> .*:[    ]+[0-9a-f]+[    ]+addi[         ]+[0-9a-f]+,gp,\-[0-9]+ # [0-9a-f]+ <person>
>
> After applying this patch, both array element and structure member
> accesses have been optimized. In fact, for both array elements and
> structure members, access is based on the data and the base address
> of the structure, which is the address of the first member of the
> array or structure. However, current linker takes into account the
> size of arrays and structures when performing GP-relative addressing.
> As a result, some array and structure bases within the GP-relative
> addressing range cannot benefit from relaxation optimization. This
> patch resolves this issue.
>
> Signed-off-by: Die Li <lidie@eswincomputing.com>
>
> ChangeLog:
>
>         * bfd/elfnn-riscv.c (_bfd_riscv_relax_section): Update reserve_size.
>         * ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp: New test entry.
>         * ld/testsuite/ld-riscv-elf/gp-relax.d: New test.
>         * ld/testsuite/ld-riscv-elf/gp-relax.s: New test.
> ---
>  bfd/elfnn-riscv.c                          |  7 +++++
>  ld/testsuite/ld-riscv-elf/gp-relax.d       | 12 +++++++++
>  ld/testsuite/ld-riscv-elf/gp-relax.s       | 31 ++++++++++++++++++++++
>  ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp |  1 +
>  4 files changed, 51 insertions(+)
>  create mode 100644 ld/testsuite/ld-riscv-elf/gp-relax.d
>  create mode 100644 ld/testsuite/ld-riscv-elf/gp-relax.s
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 09aa7be225e..29fc5484e0f 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -5169,6 +5169,13 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec,
>           if (h->type != STT_FUNC)
>             reserve_size =
>               (h->size - rel->r_addend) > h->size ? 0 : h->size - rel->r_addend;
> +
> +          /* For global pointer relative addressing, it is sufficient to ensure
> +             that the symbol's base address falls within the range of global
> +             pointer relative addressing.  */
> +          if (h->type == STT_OBJECT)
> +            reserve_size = 0;
> +
>           symtype = h->type;
>         }
>
> diff --git a/ld/testsuite/ld-riscv-elf/gp-relax.d b/ld/testsuite/ld-riscv-elf/gp-relax.d
> new file mode 100644
> index 00000000000..ec2f59b1b19
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/gp-relax.d
> @@ -0,0 +1,12 @@
> +#source: gp-relax.s
> +#ld: --relax
> +#objdump: -d -Mno-aliases
> +
> +.*:[   ]+file format .*
> +
> +
> +Disassembly of section \.text:
> +
> +0+[0-9a-f]+ <_start>:
> +.*:[   ]+[0-9a-f]+[    ]+addi[         ]+a5,gp,\-[0-9]+ # [0-9a-f]+ <global_array>
> +.*:[   ]+[0-9a-f]+[    ]+addi[         ]+a4,gp,[0-9]+ # [0-9a-f]+ <person>
> diff --git a/ld/testsuite/ld-riscv-elf/gp-relax.s b/ld/testsuite/ld-riscv-elf/gp-relax.s
> new file mode 100644
> index 00000000000..05548888ebf
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/gp-relax.s
> @@ -0,0 +1,31 @@
> +       .text
> +       .globl  global_var
> +       .section        .sbss,"aw",@nobits
> +       .align  2
> +       .type   global_var, @object
> +       .size   global_var, 4
> +global_var:
> +       .zero   4
> +       .globl  global_array
> +       .bss
> +       .align  3
> +       .type   global_array, @object
> +       .size   global_array, 3680
> +global_array:
> +       .zero   3680
> +       .globl  person
> +       .align  3
> +       .type   person, @object
> +       .size   person, 428
> +person:
> +       .zero   428
> +       .text
> +       .align  1
> +       .globl  _start
> +       .type   _start, @function
> +_start:
> +       lui     a5,%hi(global_array)
> +       addi    a5,a5,%lo(global_array)
> +
> +       lui     a4,%hi(person)
> +       addi    a4,a4,%lo(person)
> diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> index 947a266ba72..6a04955b23b 100644
> --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> @@ -124,6 +124,7 @@ if [istarget "riscv*-*-*"] {
>      run_dump_test "pcgp-relax-01"
>      run_dump_test "pcgp-relax-01-norelaxgp"
>      run_dump_test "pcgp-relax-02"
> +    run_dump_test "gp-relax"
>      run_dump_test "c-lui"
>      run_dump_test "c-lui-2"
>      run_dump_test "disas-jalr"
> --
> 2.17.1
>