* [PATCH] RISC-V: Add --[no-]relax-gp to ld
@ 2022-07-13 8:26 Fangrui Song
2022-07-13 9:10 ` Andrew Waterman
2023-02-18 7:30 ` Fangrui Song
0 siblings, 2 replies; 6+ messages in thread
From: Fangrui Song @ 2022-07-13 8:26 UTC (permalink / raw)
To: binutils
From: Fangrui Song <i@maskray.me>
--relax enables all relaxations. --no-relax-gp disables GP relaxation to
allow measuring its effect.
Link: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/298
bfd/
* elfnn-riscv.c (struct riscv_elf_link_hash_table): Add params.
(riscv_elfNN_set_options): New.
(riscv_info_to_howto_rela): Check relax_gp.
(_bfd_riscv_relax_section): Likewise.
* elfxx-riscv.h (struct riscv_elf_params): New.
(riscv_elf32_set_options): New.
(riscv_elf64_set_options): New.
ld/
* emultempl/riscvelf.em: Add option parsing.
* testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d: New.
* testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d: New.
* testsuite/ld-riscv-elf/pcgp-relax-02.d: Test --relax --relax-gp can be
used together.
---
bfd/elfnn-riscv.c | 22 ++++++++++---
bfd/elfxx-riscv.h | 11 +++++++
ld/emultempl/riscvelf.em | 31 +++++++++++++++++++
.../code-model-relax-medlow-01-norelaxgp.d | 4 +++
ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 2 ++
.../ld-riscv-elf/pcgp-relax-01-norelaxgp.d | 18 +++++++++++
ld/testsuite/ld-riscv-elf/pcgp-relax-02.d | 2 +-
7 files changed, 84 insertions(+), 6 deletions(-)
create mode 100644 ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
create mode 100644 ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 8f9f0d8a86a..527c3e74e97 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -118,6 +118,9 @@ struct riscv_elf_link_hash_table
{
struct elf_link_hash_table elf;
+ /* Various options and other info passed from the linker. */
+ struct riscv_elf_params *params;
+
/* Short-cuts to get to dynamic linker sections. */
asection *sdyntdata;
@@ -157,6 +160,13 @@ struct riscv_elf_link_hash_table
&& elf_hash_table_id (elf_hash_table (p)) == RISCV_ELF_DATA) \
? (struct riscv_elf_link_hash_table *) (p)->hash : NULL)
+void
+riscv_elfNN_set_options (struct bfd_link_info *link_info,
+ struct riscv_elf_params *params)
+{
+ riscv_elf_hash_table (link_info)->params = params;
+}
+
static bool
riscv_info_to_howto_rela (bfd *abfd,
arelent *cache_ptr,
@@ -4289,7 +4299,9 @@ _bfd_riscv_relax_lui (bfd *abfd,
bool undefined_weak)
{
bfd_byte *contents = elf_section_data (sec)->this_hdr.contents;
- bfd_vma gp = riscv_global_pointer_value (link_info);
+ bfd_vma gp = riscv_elf_hash_table (link_info)->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);
@@ -4754,10 +4766,10 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec,
|| type == R_RISCV_TPREL_LO12_I
|| type == R_RISCV_TPREL_LO12_S)
relax_func = _bfd_riscv_relax_tls_le;
- else if (!bfd_link_pic (info)
- && (type == R_RISCV_PCREL_HI20
- || type == R_RISCV_PCREL_LO12_I
- || type == R_RISCV_PCREL_LO12_S))
+ else if ((type == R_RISCV_PCREL_HI20 || type == R_RISCV_PCREL_LO12_I
+ || type == R_RISCV_PCREL_LO12_S)
+ && !bfd_link_pic (info)
+ && riscv_elf_hash_table (info)->params->relax_gp)
relax_func = _bfd_riscv_relax_pc;
else
continue;
diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
index ea7126bdb4d..f9bd0ad882c 100644
--- a/bfd/elfxx-riscv.h
+++ b/bfd/elfxx-riscv.h
@@ -27,6 +27,17 @@
#define RISCV_UNKNOWN_VERSION -1
+struct riscv_elf_params
+{
+ /* Whether to relax code sequences to GP-relative addressing. */
+ bool relax_gp;
+};
+
+extern void riscv_elf32_set_options (struct bfd_link_info *,
+ struct riscv_elf_params *);
+extern void riscv_elf64_set_options (struct bfd_link_info *,
+ struct riscv_elf_params *);
+
extern reloc_howto_type *
riscv_reloc_name_lookup (bfd *, const char *);
diff --git a/ld/emultempl/riscvelf.em b/ld/emultempl/riscvelf.em
index 645a807f239..37a0fd08e96 100644
--- a/ld/emultempl/riscvelf.em
+++ b/ld/emultempl/riscvelf.em
@@ -25,6 +25,35 @@ fragment <<EOF
#include "elf/riscv.h"
#include "elfxx-riscv.h"
+static struct riscv_elf_params params = { .relax_gp = 1 };
+EOF
+
+# Define some shell vars to insert bits of code into the standard elf
+# parse_args and list_options functions. */
+PARSE_AND_LIST_PROLOGUE=${PARSE_AND_LIST_PROLOGUE}'
+enum risccv_opt
+{
+ OPTION_RELAX_GP = 321,
+ OPTION_NO_RELAX_GP,
+};
+'
+
+PARSE_AND_LIST_LONGOPTS=${PARSE_AND_LIST_LONGOPTS}'
+ { "relax-gp", no_argument, NULL, OPTION_RELAX_GP },
+ { "no-relax-gp", no_argument, NULL, OPTION_NO_RELAX_GP },
+'
+
+PARSE_AND_LIST_ARGS_CASES=${PARSE_AND_LIST_ARGS_CASES}'
+ case OPTION_RELAX_GP:
+ params.relax_gp = 1;
+ break;
+
+ case OPTION_NO_RELAX_GP:
+ params.relax_gp = 0;
+ break;
+'
+
+fragment <<EOF
static void
riscv_elf_before_allocation (void)
{
@@ -96,6 +125,8 @@ riscv_create_output_section_statements (void)
" whilst linking %s binaries\n"), "RISC-V");
return;
}
+
+ riscv_elf${ELFSIZE}_set_options (&link_info, ¶ms);
}
EOF
diff --git a/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
new file mode 100644
index 00000000000..8e40cc5f32d
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
@@ -0,0 +1,4 @@
+#source: code-model.s
+#as: -march=rv64i -mabi=lp64 --defsym __medlow__=1
+#ld: -Tcode-model-01.ld -melf64lriscv --no-relax-gp --relax
+#error: .*relocation truncated to fit: R_RISCV_HI20 against `symbolL'
diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
index 272424b33e3..1711d12ba23 100644
--- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
+++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
@@ -122,6 +122,7 @@ if [istarget "riscv*-*-*"] {
run_dump_test "align-small-region"
run_dump_test "call-relax"
run_dump_test "pcgp-relax-01"
+ run_dump_test "pcgp-relax-01-norelaxgp"
run_dump_test "pcgp-relax-02"
run_dump_test "c-lui"
run_dump_test "c-lui-2"
@@ -141,6 +142,7 @@ if [istarget "riscv*-*-*"] {
run_dump_test "code-model-medany-weakref-01"
run_dump_test "code-model-medany-weakref-02"
run_dump_test "code-model-relax-medlow-01"
+ run_dump_test "code-model-relax-medlow-01-norelaxgp"
run_dump_test "code-model-relax-medlow-02"
run_dump_test "code-model-relax-medlow-weakref-01"
run_dump_test "code-model-relax-medlow-weakref-02"
diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
new file mode 100644
index 00000000000..d1344576ff3
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
@@ -0,0 +1,18 @@
+#source: pcgp-relax-01.s
+#ld: --no-relax-gp --relax
+#objdump: -d -Mno-aliases
+
+.*:[ ]+file format .*
+
+
+Disassembly of section \.text:
+
+0+[0-9a-f]+ <_start>:
+.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a0,a0,[0-9]+
+.*:[ ]+[0-9a-f]+[ ]+jal[ ]+ra,[0-9a-f]+ <_start>
+.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a1,0x[0-9a-f]+
+.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a1,a1,[0-9]+ # [0-9a-f]+ <data_g>
+.*:[ ]+[0-9a-f]+[ ]+lui[ ]+a2,0x[0-9a-f]+
+.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a2,a2,[0-9]+ # [0-9a-f]+ <data_g>
+.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a3,tp,0 # 0 <data_t>
+.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a0,0x[0-9a-f]+
diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
index c6c73c54265..984d457aed9 100644
--- a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
+++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
@@ -1,6 +1,6 @@
#source: pcgp-relax-02.s
#as:
-#ld: --relax
+#ld: --relax --relax-gp
#objdump: -d
.*:[ ]+file format .*
--
2.37.0.144.g8ac04bfd2-goog
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: Add --[no-]relax-gp to ld
2022-07-13 8:26 [PATCH] RISC-V: Add --[no-]relax-gp to ld Fangrui Song
@ 2022-07-13 9:10 ` Andrew Waterman
2023-02-18 7:30 ` Fangrui Song
1 sibling, 0 replies; 6+ messages in thread
From: Andrew Waterman @ 2022-07-13 9:10 UTC (permalink / raw)
To: Fangrui Song; +Cc: Binutils
Wowza.
On Wed, Jul 13, 2022 at 1:26 AM Fangrui Song via Binutils
<binutils@sourceware.org> wrote:
>
> From: Fangrui Song <i@maskray.me>
>
> --relax enables all relaxations. --no-relax-gp disables GP relaxation to
> allow measuring its effect.
>
> Link: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/298
>
> bfd/
> * elfnn-riscv.c (struct riscv_elf_link_hash_table): Add params.
> (riscv_elfNN_set_options): New.
> (riscv_info_to_howto_rela): Check relax_gp.
> (_bfd_riscv_relax_section): Likewise.
> * elfxx-riscv.h (struct riscv_elf_params): New.
> (riscv_elf32_set_options): New.
> (riscv_elf64_set_options): New.
> ld/
> * emultempl/riscvelf.em: Add option parsing.
> * testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d: New.
> * testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d: New.
> * testsuite/ld-riscv-elf/pcgp-relax-02.d: Test --relax --relax-gp can be
> used together.
> ---
> bfd/elfnn-riscv.c | 22 ++++++++++---
> bfd/elfxx-riscv.h | 11 +++++++
> ld/emultempl/riscvelf.em | 31 +++++++++++++++++++
> .../code-model-relax-medlow-01-norelaxgp.d | 4 +++
> ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 2 ++
> .../ld-riscv-elf/pcgp-relax-01-norelaxgp.d | 18 +++++++++++
> ld/testsuite/ld-riscv-elf/pcgp-relax-02.d | 2 +-
> 7 files changed, 84 insertions(+), 6 deletions(-)
> create mode 100644 ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
> create mode 100644 ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 8f9f0d8a86a..527c3e74e97 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -118,6 +118,9 @@ struct riscv_elf_link_hash_table
> {
> struct elf_link_hash_table elf;
>
> + /* Various options and other info passed from the linker. */
> + struct riscv_elf_params *params;
> +
> /* Short-cuts to get to dynamic linker sections. */
> asection *sdyntdata;
>
> @@ -157,6 +160,13 @@ struct riscv_elf_link_hash_table
> && elf_hash_table_id (elf_hash_table (p)) == RISCV_ELF_DATA) \
> ? (struct riscv_elf_link_hash_table *) (p)->hash : NULL)
>
> +void
> +riscv_elfNN_set_options (struct bfd_link_info *link_info,
> + struct riscv_elf_params *params)
> +{
> + riscv_elf_hash_table (link_info)->params = params;
> +}
> +
> static bool
> riscv_info_to_howto_rela (bfd *abfd,
> arelent *cache_ptr,
> @@ -4289,7 +4299,9 @@ _bfd_riscv_relax_lui (bfd *abfd,
> bool undefined_weak)
> {
> bfd_byte *contents = elf_section_data (sec)->this_hdr.contents;
> - bfd_vma gp = riscv_global_pointer_value (link_info);
> + bfd_vma gp = riscv_elf_hash_table (link_info)->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);
> @@ -4754,10 +4766,10 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec,
> || type == R_RISCV_TPREL_LO12_I
> || type == R_RISCV_TPREL_LO12_S)
> relax_func = _bfd_riscv_relax_tls_le;
> - else if (!bfd_link_pic (info)
> - && (type == R_RISCV_PCREL_HI20
> - || type == R_RISCV_PCREL_LO12_I
> - || type == R_RISCV_PCREL_LO12_S))
> + else if ((type == R_RISCV_PCREL_HI20 || type == R_RISCV_PCREL_LO12_I
> + || type == R_RISCV_PCREL_LO12_S)
> + && !bfd_link_pic (info)
> + && riscv_elf_hash_table (info)->params->relax_gp)
> relax_func = _bfd_riscv_relax_pc;
> else
> continue;
> diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> index ea7126bdb4d..f9bd0ad882c 100644
> --- a/bfd/elfxx-riscv.h
> +++ b/bfd/elfxx-riscv.h
> @@ -27,6 +27,17 @@
>
> #define RISCV_UNKNOWN_VERSION -1
>
> +struct riscv_elf_params
> +{
> + /* Whether to relax code sequences to GP-relative addressing. */
> + bool relax_gp;
> +};
> +
> +extern void riscv_elf32_set_options (struct bfd_link_info *,
> + struct riscv_elf_params *);
> +extern void riscv_elf64_set_options (struct bfd_link_info *,
> + struct riscv_elf_params *);
> +
> extern reloc_howto_type *
> riscv_reloc_name_lookup (bfd *, const char *);
>
> diff --git a/ld/emultempl/riscvelf.em b/ld/emultempl/riscvelf.em
> index 645a807f239..37a0fd08e96 100644
> --- a/ld/emultempl/riscvelf.em
> +++ b/ld/emultempl/riscvelf.em
> @@ -25,6 +25,35 @@ fragment <<EOF
> #include "elf/riscv.h"
> #include "elfxx-riscv.h"
>
> +static struct riscv_elf_params params = { .relax_gp = 1 };
> +EOF
> +
> +# Define some shell vars to insert bits of code into the standard elf
> +# parse_args and list_options functions. */
> +PARSE_AND_LIST_PROLOGUE=${PARSE_AND_LIST_PROLOGUE}'
> +enum risccv_opt
> +{
> + OPTION_RELAX_GP = 321,
> + OPTION_NO_RELAX_GP,
> +};
> +'
> +
> +PARSE_AND_LIST_LONGOPTS=${PARSE_AND_LIST_LONGOPTS}'
> + { "relax-gp", no_argument, NULL, OPTION_RELAX_GP },
> + { "no-relax-gp", no_argument, NULL, OPTION_NO_RELAX_GP },
> +'
> +
> +PARSE_AND_LIST_ARGS_CASES=${PARSE_AND_LIST_ARGS_CASES}'
> + case OPTION_RELAX_GP:
> + params.relax_gp = 1;
> + break;
> +
> + case OPTION_NO_RELAX_GP:
> + params.relax_gp = 0;
> + break;
> +'
> +
> +fragment <<EOF
> static void
> riscv_elf_before_allocation (void)
> {
> @@ -96,6 +125,8 @@ riscv_create_output_section_statements (void)
> " whilst linking %s binaries\n"), "RISC-V");
> return;
> }
> +
> + riscv_elf${ELFSIZE}_set_options (&link_info, ¶ms);
> }
>
> EOF
> diff --git a/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
> new file mode 100644
> index 00000000000..8e40cc5f32d
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
> @@ -0,0 +1,4 @@
> +#source: code-model.s
> +#as: -march=rv64i -mabi=lp64 --defsym __medlow__=1
> +#ld: -Tcode-model-01.ld -melf64lriscv --no-relax-gp --relax
> +#error: .*relocation truncated to fit: R_RISCV_HI20 against `symbolL'
> diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> index 272424b33e3..1711d12ba23 100644
> --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> @@ -122,6 +122,7 @@ if [istarget "riscv*-*-*"] {
> run_dump_test "align-small-region"
> run_dump_test "call-relax"
> run_dump_test "pcgp-relax-01"
> + run_dump_test "pcgp-relax-01-norelaxgp"
> run_dump_test "pcgp-relax-02"
> run_dump_test "c-lui"
> run_dump_test "c-lui-2"
> @@ -141,6 +142,7 @@ if [istarget "riscv*-*-*"] {
> run_dump_test "code-model-medany-weakref-01"
> run_dump_test "code-model-medany-weakref-02"
> run_dump_test "code-model-relax-medlow-01"
> + run_dump_test "code-model-relax-medlow-01-norelaxgp"
> run_dump_test "code-model-relax-medlow-02"
> run_dump_test "code-model-relax-medlow-weakref-01"
> run_dump_test "code-model-relax-medlow-weakref-02"
> diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
> new file mode 100644
> index 00000000000..d1344576ff3
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
> @@ -0,0 +1,18 @@
> +#source: pcgp-relax-01.s
> +#ld: --no-relax-gp --relax
> +#objdump: -d -Mno-aliases
> +
> +.*:[ ]+file format .*
> +
> +
> +Disassembly of section \.text:
> +
> +0+[0-9a-f]+ <_start>:
> +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a0,a0,[0-9]+
> +.*:[ ]+[0-9a-f]+[ ]+jal[ ]+ra,[0-9a-f]+ <_start>
> +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a1,0x[0-9a-f]+
> +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a1,a1,[0-9]+ # [0-9a-f]+ <data_g>
> +.*:[ ]+[0-9a-f]+[ ]+lui[ ]+a2,0x[0-9a-f]+
> +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a2,a2,[0-9]+ # [0-9a-f]+ <data_g>
> +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a3,tp,0 # 0 <data_t>
> +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a0,0x[0-9a-f]+
> diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
> index c6c73c54265..984d457aed9 100644
> --- a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
> +++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
> @@ -1,6 +1,6 @@
> #source: pcgp-relax-02.s
> #as:
> -#ld: --relax
> +#ld: --relax --relax-gp
> #objdump: -d
>
> .*:[ ]+file format .*
> --
> 2.37.0.144.g8ac04bfd2-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: Add --[no-]relax-gp to ld
2022-07-13 8:26 [PATCH] RISC-V: Add --[no-]relax-gp to ld Fangrui Song
2022-07-13 9:10 ` Andrew Waterman
@ 2023-02-18 7:30 ` Fangrui Song
2023-02-20 1:55 ` Nelson Chu
1 sibling, 1 reply; 6+ messages in thread
From: Fangrui Song @ 2023-02-18 7:30 UTC (permalink / raw)
To: Fangrui Song; +Cc: binutils
On Wed, Jul 13, 2022 at 1:26 AM Fangrui Song <maskray@google.com> wrote:
>
> From: Fangrui Song <i@maskray.me>
>
> --relax enables all relaxations. --no-relax-gp disables GP relaxation to
> allow measuring its effect.
>
> Link: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/298
>
> bfd/
> * elfnn-riscv.c (struct riscv_elf_link_hash_table): Add params.
> (riscv_elfNN_set_options): New.
> (riscv_info_to_howto_rela): Check relax_gp.
> (_bfd_riscv_relax_section): Likewise.
> * elfxx-riscv.h (struct riscv_elf_params): New.
> (riscv_elf32_set_options): New.
> (riscv_elf64_set_options): New.
> ld/
> * emultempl/riscvelf.em: Add option parsing.
> * testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d: New.
> * testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d: New.
> * testsuite/ld-riscv-elf/pcgp-relax-02.d: Test --relax --relax-gp can be
> used together.
> ---
> bfd/elfnn-riscv.c | 22 ++++++++++---
> bfd/elfxx-riscv.h | 11 +++++++
> ld/emultempl/riscvelf.em | 31 +++++++++++++++++++
> .../code-model-relax-medlow-01-norelaxgp.d | 4 +++
> ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 2 ++
> .../ld-riscv-elf/pcgp-relax-01-norelaxgp.d | 18 +++++++++++
> ld/testsuite/ld-riscv-elf/pcgp-relax-02.d | 2 +-
> 7 files changed, 84 insertions(+), 6 deletions(-)
> create mode 100644 ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
> create mode 100644 ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 8f9f0d8a86a..527c3e74e97 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -118,6 +118,9 @@ struct riscv_elf_link_hash_table
> {
> struct elf_link_hash_table elf;
>
> + /* Various options and other info passed from the linker. */
> + struct riscv_elf_params *params;
> +
> /* Short-cuts to get to dynamic linker sections. */
> asection *sdyntdata;
>
> @@ -157,6 +160,13 @@ struct riscv_elf_link_hash_table
> && elf_hash_table_id (elf_hash_table (p)) == RISCV_ELF_DATA) \
> ? (struct riscv_elf_link_hash_table *) (p)->hash : NULL)
>
> +void
> +riscv_elfNN_set_options (struct bfd_link_info *link_info,
> + struct riscv_elf_params *params)
> +{
> + riscv_elf_hash_table (link_info)->params = params;
> +}
> +
> static bool
> riscv_info_to_howto_rela (bfd *abfd,
> arelent *cache_ptr,
> @@ -4289,7 +4299,9 @@ _bfd_riscv_relax_lui (bfd *abfd,
> bool undefined_weak)
> {
> bfd_byte *contents = elf_section_data (sec)->this_hdr.contents;
> - bfd_vma gp = riscv_global_pointer_value (link_info);
> + bfd_vma gp = riscv_elf_hash_table (link_info)->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);
> @@ -4754,10 +4766,10 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec,
> || type == R_RISCV_TPREL_LO12_I
> || type == R_RISCV_TPREL_LO12_S)
> relax_func = _bfd_riscv_relax_tls_le;
> - else if (!bfd_link_pic (info)
> - && (type == R_RISCV_PCREL_HI20
> - || type == R_RISCV_PCREL_LO12_I
> - || type == R_RISCV_PCREL_LO12_S))
> + else if ((type == R_RISCV_PCREL_HI20 || type == R_RISCV_PCREL_LO12_I
> + || type == R_RISCV_PCREL_LO12_S)
> + && !bfd_link_pic (info)
> + && riscv_elf_hash_table (info)->params->relax_gp)
> relax_func = _bfd_riscv_relax_pc;
> else
> continue;
> diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> index ea7126bdb4d..f9bd0ad882c 100644
> --- a/bfd/elfxx-riscv.h
> +++ b/bfd/elfxx-riscv.h
> @@ -27,6 +27,17 @@
>
> #define RISCV_UNKNOWN_VERSION -1
>
> +struct riscv_elf_params
> +{
> + /* Whether to relax code sequences to GP-relative addressing. */
> + bool relax_gp;
> +};
> +
> +extern void riscv_elf32_set_options (struct bfd_link_info *,
> + struct riscv_elf_params *);
> +extern void riscv_elf64_set_options (struct bfd_link_info *,
> + struct riscv_elf_params *);
> +
> extern reloc_howto_type *
> riscv_reloc_name_lookup (bfd *, const char *);
>
> diff --git a/ld/emultempl/riscvelf.em b/ld/emultempl/riscvelf.em
> index 645a807f239..37a0fd08e96 100644
> --- a/ld/emultempl/riscvelf.em
> +++ b/ld/emultempl/riscvelf.em
> @@ -25,6 +25,35 @@ fragment <<EOF
> #include "elf/riscv.h"
> #include "elfxx-riscv.h"
>
> +static struct riscv_elf_params params = { .relax_gp = 1 };
> +EOF
> +
> +# Define some shell vars to insert bits of code into the standard elf
> +# parse_args and list_options functions. */
> +PARSE_AND_LIST_PROLOGUE=${PARSE_AND_LIST_PROLOGUE}'
> +enum risccv_opt
> +{
> + OPTION_RELAX_GP = 321,
> + OPTION_NO_RELAX_GP,
> +};
> +'
> +
> +PARSE_AND_LIST_LONGOPTS=${PARSE_AND_LIST_LONGOPTS}'
> + { "relax-gp", no_argument, NULL, OPTION_RELAX_GP },
> + { "no-relax-gp", no_argument, NULL, OPTION_NO_RELAX_GP },
> +'
> +
> +PARSE_AND_LIST_ARGS_CASES=${PARSE_AND_LIST_ARGS_CASES}'
> + case OPTION_RELAX_GP:
> + params.relax_gp = 1;
> + break;
> +
> + case OPTION_NO_RELAX_GP:
> + params.relax_gp = 0;
> + break;
> +'
> +
> +fragment <<EOF
> static void
> riscv_elf_before_allocation (void)
> {
> @@ -96,6 +125,8 @@ riscv_create_output_section_statements (void)
> " whilst linking %s binaries\n"), "RISC-V");
> return;
> }
> +
> + riscv_elf${ELFSIZE}_set_options (&link_info, ¶ms);
> }
>
> EOF
> diff --git a/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
> new file mode 100644
> index 00000000000..8e40cc5f32d
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
> @@ -0,0 +1,4 @@
> +#source: code-model.s
> +#as: -march=rv64i -mabi=lp64 --defsym __medlow__=1
> +#ld: -Tcode-model-01.ld -melf64lriscv --no-relax-gp --relax
> +#error: .*relocation truncated to fit: R_RISCV_HI20 against `symbolL'
> diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> index 272424b33e3..1711d12ba23 100644
> --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> @@ -122,6 +122,7 @@ if [istarget "riscv*-*-*"] {
> run_dump_test "align-small-region"
> run_dump_test "call-relax"
> run_dump_test "pcgp-relax-01"
> + run_dump_test "pcgp-relax-01-norelaxgp"
> run_dump_test "pcgp-relax-02"
> run_dump_test "c-lui"
> run_dump_test "c-lui-2"
> @@ -141,6 +142,7 @@ if [istarget "riscv*-*-*"] {
> run_dump_test "code-model-medany-weakref-01"
> run_dump_test "code-model-medany-weakref-02"
> run_dump_test "code-model-relax-medlow-01"
> + run_dump_test "code-model-relax-medlow-01-norelaxgp"
> run_dump_test "code-model-relax-medlow-02"
> run_dump_test "code-model-relax-medlow-weakref-01"
> run_dump_test "code-model-relax-medlow-weakref-02"
> diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
> new file mode 100644
> index 00000000000..d1344576ff3
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
> @@ -0,0 +1,18 @@
> +#source: pcgp-relax-01.s
> +#ld: --no-relax-gp --relax
> +#objdump: -d -Mno-aliases
> +
> +.*:[ ]+file format .*
> +
> +
> +Disassembly of section \.text:
> +
> +0+[0-9a-f]+ <_start>:
> +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a0,a0,[0-9]+
> +.*:[ ]+[0-9a-f]+[ ]+jal[ ]+ra,[0-9a-f]+ <_start>
> +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a1,0x[0-9a-f]+
> +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a1,a1,[0-9]+ # [0-9a-f]+ <data_g>
> +.*:[ ]+[0-9a-f]+[ ]+lui[ ]+a2,0x[0-9a-f]+
> +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a2,a2,[0-9]+ # [0-9a-f]+ <data_g>
> +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a3,tp,0 # 0 <data_t>
> +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a0,0x[0-9a-f]+
> diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
> index c6c73c54265..984d457aed9 100644
> --- a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
> +++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
> @@ -1,6 +1,6 @@
> #source: pcgp-relax-02.s
> #as:
> -#ld: --relax
> +#ld: --relax --relax-gp
> #objdump: -d
>
> .*:[ ]+file format .*
> --
> 2.37.0.144.g8ac04bfd2-goog
>
Ping. I maintain lld/ELF and we are considering a new opt-in option
--relax-gp to perform GP relaxation.
It will be opt-in since many users don't find the relaxation useful
and some feel GP relaxation will conflict with using GP for other
purposes (some ABI variants).
I wish that the two linkers agree on the same options.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: Add --[no-]relax-gp to ld
2023-02-18 7:30 ` Fangrui Song
@ 2023-02-20 1:55 ` Nelson Chu
2023-02-20 3:46 ` Fangrui Song
0 siblings, 1 reply; 6+ messages in thread
From: Nelson Chu @ 2023-02-20 1:55 UTC (permalink / raw)
To: Fangrui Song; +Cc: Fangrui Song, binutils
This is just a target option, and it's not only for compatibility with
lld, sometimes this option is also convenient to catch whether the gp
relaxation has a bug without rebuilding the linker to disable the
specific relaxation. Personally I would say this is a good idea at
least for debugging, we just default enable the relax flags in the
riscv_elf_params, so that everything won't change. Perhaps from the
debug point of view, it may be the right direction to have other
developer target options to close the remaining specific relaxations.
Anyway, this target option works for me, it's not only good for
developer debugging, but also keeps it compatible with lld. However,
it would be better to just wait so as not to miss some of the other
suggestions, I think 1-2 weeks should be enough.
Thanks
Nelson
On Sat, Feb 18, 2023 at 3:31 PM Fangrui Song <i@maskray.me> wrote:
>
> On Wed, Jul 13, 2022 at 1:26 AM Fangrui Song <maskray@google.com> wrote:
> >
> > From: Fangrui Song <i@maskray.me>
> >
> > --relax enables all relaxations. --no-relax-gp disables GP relaxation to
> > allow measuring its effect.
> >
> > Link: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/298
> >
> > bfd/
> > * elfnn-riscv.c (struct riscv_elf_link_hash_table): Add params.
> > (riscv_elfNN_set_options): New.
> > (riscv_info_to_howto_rela): Check relax_gp.
> > (_bfd_riscv_relax_section): Likewise.
> > * elfxx-riscv.h (struct riscv_elf_params): New.
> > (riscv_elf32_set_options): New.
> > (riscv_elf64_set_options): New.
> > ld/
> > * emultempl/riscvelf.em: Add option parsing.
> > * testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d: New.
> > * testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d: New.
> > * testsuite/ld-riscv-elf/pcgp-relax-02.d: Test --relax --relax-gp can be
> > used together.
> > ---
> > bfd/elfnn-riscv.c | 22 ++++++++++---
> > bfd/elfxx-riscv.h | 11 +++++++
> > ld/emultempl/riscvelf.em | 31 +++++++++++++++++++
> > .../code-model-relax-medlow-01-norelaxgp.d | 4 +++
> > ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 2 ++
> > .../ld-riscv-elf/pcgp-relax-01-norelaxgp.d | 18 +++++++++++
> > ld/testsuite/ld-riscv-elf/pcgp-relax-02.d | 2 +-
> > 7 files changed, 84 insertions(+), 6 deletions(-)
> > create mode 100644 ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
> > create mode 100644 ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
> >
> > diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> > index 8f9f0d8a86a..527c3e74e97 100644
> > --- a/bfd/elfnn-riscv.c
> > +++ b/bfd/elfnn-riscv.c
> > @@ -118,6 +118,9 @@ struct riscv_elf_link_hash_table
> > {
> > struct elf_link_hash_table elf;
> >
> > + /* Various options and other info passed from the linker. */
> > + struct riscv_elf_params *params;
> > +
> > /* Short-cuts to get to dynamic linker sections. */
> > asection *sdyntdata;
> >
> > @@ -157,6 +160,13 @@ struct riscv_elf_link_hash_table
> > && elf_hash_table_id (elf_hash_table (p)) == RISCV_ELF_DATA) \
> > ? (struct riscv_elf_link_hash_table *) (p)->hash : NULL)
> >
> > +void
> > +riscv_elfNN_set_options (struct bfd_link_info *link_info,
> > + struct riscv_elf_params *params)
> > +{
> > + riscv_elf_hash_table (link_info)->params = params;
> > +}
> > +
> > static bool
> > riscv_info_to_howto_rela (bfd *abfd,
> > arelent *cache_ptr,
> > @@ -4289,7 +4299,9 @@ _bfd_riscv_relax_lui (bfd *abfd,
> > bool undefined_weak)
> > {
> > bfd_byte *contents = elf_section_data (sec)->this_hdr.contents;
> > - bfd_vma gp = riscv_global_pointer_value (link_info);
> > + bfd_vma gp = riscv_elf_hash_table (link_info)->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);
> > @@ -4754,10 +4766,10 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec,
> > || type == R_RISCV_TPREL_LO12_I
> > || type == R_RISCV_TPREL_LO12_S)
> > relax_func = _bfd_riscv_relax_tls_le;
> > - else if (!bfd_link_pic (info)
Probably just add "riscv_elf_hash_table (info)->params->relax_gp"
here, to make the coding style same as above.
> > - && (type == R_RISCV_PCREL_HI20
> > - || type == R_RISCV_PCREL_LO12_I
> > - || type == R_RISCV_PCREL_LO12_S))
> > + else if ((type == R_RISCV_PCREL_HI20 || type == R_RISCV_PCREL_LO12_I
> > + || type == R_RISCV_PCREL_LO12_S)
> > + && !bfd_link_pic (info)
> > + && riscv_elf_hash_table (info)->params->relax_gp)
> > relax_func = _bfd_riscv_relax_pc;
> > else
> > continue;
> > diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> > index ea7126bdb4d..f9bd0ad882c 100644
> > --- a/bfd/elfxx-riscv.h
> > +++ b/bfd/elfxx-riscv.h
> > @@ -27,6 +27,17 @@
> >
> > #define RISCV_UNKNOWN_VERSION -1
> >
> > +struct riscv_elf_params
> > +{
> > + /* Whether to relax code sequences to GP-relative addressing. */
> > + bool relax_gp;
> > +};
> > +
> > +extern void riscv_elf32_set_options (struct bfd_link_info *,
> > + struct riscv_elf_params *);
> > +extern void riscv_elf64_set_options (struct bfd_link_info *,
> > + struct riscv_elf_params *);
> > +
> > extern reloc_howto_type *
> > riscv_reloc_name_lookup (bfd *, const char *);
> >
> > diff --git a/ld/emultempl/riscvelf.em b/ld/emultempl/riscvelf.em
> > index 645a807f239..37a0fd08e96 100644
> > --- a/ld/emultempl/riscvelf.em
> > +++ b/ld/emultempl/riscvelf.em
> > @@ -25,6 +25,35 @@ fragment <<EOF
> > #include "elf/riscv.h"
> > #include "elfxx-riscv.h"
> >
> > +static struct riscv_elf_params params = { .relax_gp = 1 };
> > +EOF
> > +
> > +# Define some shell vars to insert bits of code into the standard elf
> > +# parse_args and list_options functions. */
> > +PARSE_AND_LIST_PROLOGUE=${PARSE_AND_LIST_PROLOGUE}'
> > +enum risccv_opt
> > +{
> > + OPTION_RELAX_GP = 321,
> > + OPTION_NO_RELAX_GP,
> > +};
> > +'
Not really sure if the value of base enum is flexible or has some
meaning. I see ppc starts with 321, mips and nds32 start with 301,
and aarch64 starts with 309. If the start value doesn't mean
anything, then it's fine here.
> > +
> > +PARSE_AND_LIST_LONGOPTS=${PARSE_AND_LIST_LONGOPTS}'
> > + { "relax-gp", no_argument, NULL, OPTION_RELAX_GP },
> > + { "no-relax-gp", no_argument, NULL, OPTION_NO_RELAX_GP },
> > +'
> > +
It's good to also define PARSE_AND_LIST_OPTIONS, so that --target-help
should work for the new options.
> > +PARSE_AND_LIST_ARGS_CASES=${PARSE_AND_LIST_ARGS_CASES}'
> > + case OPTION_RELAX_GP:
> > + params.relax_gp = 1;
> > + break;
> > +
> > + case OPTION_NO_RELAX_GP:
> > + params.relax_gp = 0;
> > + break;
> > +'
> > +
> > +fragment <<EOF
> > static void
> > riscv_elf_before_allocation (void)
> > {
> > @@ -96,6 +125,8 @@ riscv_create_output_section_statements (void)
> > " whilst linking %s binaries\n"), "RISC-V");
> > return;
> > }
> > +
> > + riscv_elf${ELFSIZE}_set_options (&link_info, ¶ms);
> > }
> >
> > EOF
> > diff --git a/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
> > new file mode 100644
> > index 00000000000..8e40cc5f32d
> > --- /dev/null
> > +++ b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
> > @@ -0,0 +1,4 @@
> > +#source: code-model.s
> > +#as: -march=rv64i -mabi=lp64 --defsym __medlow__=1
> > +#ld: -Tcode-model-01.ld -melf64lriscv --no-relax-gp --relax
> > +#error: .*relocation truncated to fit: R_RISCV_HI20 against `symbolL'
> > diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> > index 272424b33e3..1711d12ba23 100644
> > --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> > +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> > @@ -122,6 +122,7 @@ if [istarget "riscv*-*-*"] {
> > run_dump_test "align-small-region"
> > run_dump_test "call-relax"
> > run_dump_test "pcgp-relax-01"
> > + run_dump_test "pcgp-relax-01-norelaxgp"
> > run_dump_test "pcgp-relax-02"
> > run_dump_test "c-lui"
> > run_dump_test "c-lui-2"
> > @@ -141,6 +142,7 @@ if [istarget "riscv*-*-*"] {
> > run_dump_test "code-model-medany-weakref-01"
> > run_dump_test "code-model-medany-weakref-02"
> > run_dump_test "code-model-relax-medlow-01"
> > + run_dump_test "code-model-relax-medlow-01-norelaxgp"
> > run_dump_test "code-model-relax-medlow-02"
> > run_dump_test "code-model-relax-medlow-weakref-01"
> > run_dump_test "code-model-relax-medlow-weakref-02"
> > diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
> > new file mode 100644
> > index 00000000000..d1344576ff3
> > --- /dev/null
> > +++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
> > @@ -0,0 +1,18 @@
> > +#source: pcgp-relax-01.s
> > +#ld: --no-relax-gp --relax
> > +#objdump: -d -Mno-aliases
> > +
> > +.*:[ ]+file format .*
> > +
> > +
> > +Disassembly of section \.text:
> > +
> > +0+[0-9a-f]+ <_start>:
> > +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a0,a0,[0-9]+
> > +.*:[ ]+[0-9a-f]+[ ]+jal[ ]+ra,[0-9a-f]+ <_start>
> > +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a1,0x[0-9a-f]+
> > +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a1,a1,[0-9]+ # [0-9a-f]+ <data_g>
> > +.*:[ ]+[0-9a-f]+[ ]+lui[ ]+a2,0x[0-9a-f]+
> > +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a2,a2,[0-9]+ # [0-9a-f]+ <data_g>
> > +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a3,tp,0 # 0 <data_t>
> > +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a0,0x[0-9a-f]+
> > diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
> > index c6c73c54265..984d457aed9 100644
> > --- a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
> > +++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
> > @@ -1,6 +1,6 @@
> > #source: pcgp-relax-02.s
> > #as:
> > -#ld: --relax
> > +#ld: --relax --relax-gp
> > #objdump: -d
> >
> > .*:[ ]+file format .*
> > --
> > 2.37.0.144.g8ac04bfd2-goog
> >
>
> Ping. I maintain lld/ELF and we are considering a new opt-in option
> --relax-gp to perform GP relaxation.
> It will be opt-in since many users don't find the relaxation useful
> and some feel GP relaxation will conflict with using GP for other
> purposes (some ABI variants).
> I wish that the two linkers agree on the same options.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: Add --[no-]relax-gp to ld
2023-02-20 1:55 ` Nelson Chu
@ 2023-02-20 3:46 ` Fangrui Song
2023-02-20 10:02 ` Nelson Chu
0 siblings, 1 reply; 6+ messages in thread
From: Fangrui Song @ 2023-02-20 3:46 UTC (permalink / raw)
To: Nelson Chu; +Cc: Fangrui Song, binutils
[-- Attachment #1: Type: text/plain, Size: 11698 bytes --]
On 2023-02-20, Nelson Chu wrote:
>This is just a target option, and it's not only for compatibility with
>lld, sometimes this option is also convenient to catch whether the gp
>relaxation has a bug without rebuilding the linker to disable the
>specific relaxation. Personally I would say this is a good idea at
>least for debugging, we just default enable the relax flags in the
>riscv_elf_params, so that everything won't change. Perhaps from the
>debug point of view, it may be the right direction to have other
>developer target options to close the remaining specific relaxations.
>Anyway, this target option works for me, it's not only good for
>developer debugging, but also keeps it compatible with lld. However,
>it would be better to just wait so as not to miss some of the other
>suggestions, I think 1-2 weeks should be enough.
>
>Thanks
>Nelson
Thank you for the feedback!
>On Sat, Feb 18, 2023 at 3:31 PM Fangrui Song <i@maskray.me> wrote:
>>
>> On Wed, Jul 13, 2022 at 1:26 AM Fangrui Song <maskray@google.com> wrote:
>> >
>> > From: Fangrui Song <i@maskray.me>
>> >
>> > --relax enables all relaxations. --no-relax-gp disables GP relaxation to
>> > allow measuring its effect.
>> >
>> > Link: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/298
>> >
>> > bfd/
>> > * elfnn-riscv.c (struct riscv_elf_link_hash_table): Add params.
>> > (riscv_elfNN_set_options): New.
>> > (riscv_info_to_howto_rela): Check relax_gp.
>> > (_bfd_riscv_relax_section): Likewise.
>> > * elfxx-riscv.h (struct riscv_elf_params): New.
>> > (riscv_elf32_set_options): New.
>> > (riscv_elf64_set_options): New.
>> > ld/
>> > * emultempl/riscvelf.em: Add option parsing.
>> > * testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d: New.
>> > * testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d: New.
>> > * testsuite/ld-riscv-elf/pcgp-relax-02.d: Test --relax --relax-gp can be
>> > used together.
>> > ---
>> > bfd/elfnn-riscv.c | 22 ++++++++++---
>> > bfd/elfxx-riscv.h | 11 +++++++
>> > ld/emultempl/riscvelf.em | 31 +++++++++++++++++++
>> > .../code-model-relax-medlow-01-norelaxgp.d | 4 +++
>> > ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 2 ++
>> > .../ld-riscv-elf/pcgp-relax-01-norelaxgp.d | 18 +++++++++++
>> > ld/testsuite/ld-riscv-elf/pcgp-relax-02.d | 2 +-
>> > 7 files changed, 84 insertions(+), 6 deletions(-)
>> > create mode 100644 ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
>> > create mode 100644 ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
>> >
>> > diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
>> > index 8f9f0d8a86a..527c3e74e97 100644
>> > --- a/bfd/elfnn-riscv.c
>> > +++ b/bfd/elfnn-riscv.c
>> > @@ -118,6 +118,9 @@ struct riscv_elf_link_hash_table
>> > {
>> > struct elf_link_hash_table elf;
>> >
>> > + /* Various options and other info passed from the linker. */
>> > + struct riscv_elf_params *params;
>> > +
>> > /* Short-cuts to get to dynamic linker sections. */
>> > asection *sdyntdata;
>> >
>> > @@ -157,6 +160,13 @@ struct riscv_elf_link_hash_table
>> > && elf_hash_table_id (elf_hash_table (p)) == RISCV_ELF_DATA) \
>> > ? (struct riscv_elf_link_hash_table *) (p)->hash : NULL)
>> >
>> > +void
>> > +riscv_elfNN_set_options (struct bfd_link_info *link_info,
>> > + struct riscv_elf_params *params)
>> > +{
>> > + riscv_elf_hash_table (link_info)->params = params;
>> > +}
>> > +
>> > static bool
>> > riscv_info_to_howto_rela (bfd *abfd,
>> > arelent *cache_ptr,
>> > @@ -4289,7 +4299,9 @@ _bfd_riscv_relax_lui (bfd *abfd,
>> > bool undefined_weak)
>> > {
>> > bfd_byte *contents = elf_section_data (sec)->this_hdr.contents;
>> > - bfd_vma gp = riscv_global_pointer_value (link_info);
>> > + bfd_vma gp = riscv_elf_hash_table (link_info)->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);
>> > @@ -4754,10 +4766,10 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec,
>> > || type == R_RISCV_TPREL_LO12_I
>> > || type == R_RISCV_TPREL_LO12_S)
>> > relax_func = _bfd_riscv_relax_tls_le;
>> > - else if (!bfd_link_pic (info)
>
>Probably just add "riscv_elf_hash_table (info)->params->relax_gp"
>here, to make the coding style same as above.
I'm going to use:
else if (!bfd_link_pic (info) && htab->params->relax_gp
&& (type == R_RISCV_PCREL_HI20
...
>> > - && (type == R_RISCV_PCREL_HI20
>> > - || type == R_RISCV_PCREL_LO12_I
>> > - || type == R_RISCV_PCREL_LO12_S))
>> > + else if ((type == R_RISCV_PCREL_HI20 || type == R_RISCV_PCREL_LO12_I
>> > + || type == R_RISCV_PCREL_LO12_S)
>> > + && !bfd_link_pic (info)
>> > + && riscv_elf_hash_table (info)->params->relax_gp)
>> > relax_func = _bfd_riscv_relax_pc;
>> > else
>> > continue;
>> > diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
>> > index ea7126bdb4d..f9bd0ad882c 100644
>> > --- a/bfd/elfxx-riscv.h
>> > +++ b/bfd/elfxx-riscv.h
>> > @@ -27,6 +27,17 @@
>> >
>> > #define RISCV_UNKNOWN_VERSION -1
>> >
>> > +struct riscv_elf_params
>> > +{
>> > + /* Whether to relax code sequences to GP-relative addressing. */
>> > + bool relax_gp;
>> > +};
>> > +
>> > +extern void riscv_elf32_set_options (struct bfd_link_info *,
>> > + struct riscv_elf_params *);
>> > +extern void riscv_elf64_set_options (struct bfd_link_info *,
>> > + struct riscv_elf_params *);
>> > +
>> > extern reloc_howto_type *
>> > riscv_reloc_name_lookup (bfd *, const char *);
>> >
>> > diff --git a/ld/emultempl/riscvelf.em b/ld/emultempl/riscvelf.em
>> > index 645a807f239..37a0fd08e96 100644
>> > --- a/ld/emultempl/riscvelf.em
>> > +++ b/ld/emultempl/riscvelf.em
>> > @@ -25,6 +25,35 @@ fragment <<EOF
>> > #include "elf/riscv.h"
>> > #include "elfxx-riscv.h"
>> >
>> > +static struct riscv_elf_params params = { .relax_gp = 1 };
>> > +EOF
>> > +
>> > +# Define some shell vars to insert bits of code into the standard elf
>> > +# parse_args and list_options functions. */
>> > +PARSE_AND_LIST_PROLOGUE=${PARSE_AND_LIST_PROLOGUE}'
>> > +enum risccv_opt
>> > +{
>> > + OPTION_RELAX_GP = 321,
>> > + OPTION_NO_RELAX_GP,
>> > +};
>> > +'
>
>Not really sure if the value of base enum is flexible or has some
>meaning. I see ppc starts with 321, mips and nds32 start with 301,
>and aarch64 starts with 309. If the start value doesn't mean
>anything, then it's fine here.
I think the start value is arbitrary. I stick with the ppc style which is
well maintained by Alan :)
>> > +
>> > +PARSE_AND_LIST_LONGOPTS=${PARSE_AND_LIST_LONGOPTS}'
>> > + { "relax-gp", no_argument, NULL, OPTION_RELAX_GP },
>> > + { "no-relax-gp", no_argument, NULL, OPTION_NO_RELAX_GP },
>> > +'
>> > +
>
>It's good to also define PARSE_AND_LIST_OPTIONS, so that --target-help
>should work for the new options.
Ack. I did not know PARSE_AND_LIST_OPTIONS.
PARSE_AND_LIST_OPTIONS=${PARSE_AND_LIST_OPTIONS}'
fprintf (file, _(" --relax-gp Perform GP relaxation\n"));
fprintf (file, _(" --no-relax-gp Don'\''t perform GP relaxation\n"));
'
>> > +PARSE_AND_LIST_ARGS_CASES=${PARSE_AND_LIST_ARGS_CASES}'
>> > + case OPTION_RELAX_GP:
>> > + params.relax_gp = 1;
>> > + break;
>> > +
>> > + case OPTION_NO_RELAX_GP:
>> > + params.relax_gp = 0;
>> > + break;
>> > +'
>> > +
>> > +fragment <<EOF
>> > static void
>> > riscv_elf_before_allocation (void)
>> > {
>> > @@ -96,6 +125,8 @@ riscv_create_output_section_statements (void)
>> > " whilst linking %s binaries\n"), "RISC-V");
>> > return;
>> > }
>> > +
>> > + riscv_elf${ELFSIZE}_set_options (&link_info, ¶ms);
>> > }
>> >
>> > EOF
>> > diff --git a/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
>> > new file mode 100644
>> > index 00000000000..8e40cc5f32d
>> > --- /dev/null
>> > +++ b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
>> > @@ -0,0 +1,4 @@
>> > +#source: code-model.s
>> > +#as: -march=rv64i -mabi=lp64 --defsym __medlow__=1
>> > +#ld: -Tcode-model-01.ld -melf64lriscv --no-relax-gp --relax
>> > +#error: .*relocation truncated to fit: R_RISCV_HI20 against `symbolL'
>> > diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
>> > index 272424b33e3..1711d12ba23 100644
>> > --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
>> > +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
>> > @@ -122,6 +122,7 @@ if [istarget "riscv*-*-*"] {
>> > run_dump_test "align-small-region"
>> > run_dump_test "call-relax"
>> > run_dump_test "pcgp-relax-01"
>> > + run_dump_test "pcgp-relax-01-norelaxgp"
>> > run_dump_test "pcgp-relax-02"
>> > run_dump_test "c-lui"
>> > run_dump_test "c-lui-2"
>> > @@ -141,6 +142,7 @@ if [istarget "riscv*-*-*"] {
>> > run_dump_test "code-model-medany-weakref-01"
>> > run_dump_test "code-model-medany-weakref-02"
>> > run_dump_test "code-model-relax-medlow-01"
>> > + run_dump_test "code-model-relax-medlow-01-norelaxgp"
>> > run_dump_test "code-model-relax-medlow-02"
>> > run_dump_test "code-model-relax-medlow-weakref-01"
>> > run_dump_test "code-model-relax-medlow-weakref-02"
>> > diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
>> > new file mode 100644
>> > index 00000000000..d1344576ff3
>> > --- /dev/null
>> > +++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
>> > @@ -0,0 +1,18 @@
>> > +#source: pcgp-relax-01.s
>> > +#ld: --no-relax-gp --relax
>> > +#objdump: -d -Mno-aliases
>> > +
>> > +.*:[ ]+file format .*
>> > +
>> > +
>> > +Disassembly of section \.text:
>> > +
>> > +0+[0-9a-f]+ <_start>:
>> > +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a0,a0,[0-9]+
>> > +.*:[ ]+[0-9a-f]+[ ]+jal[ ]+ra,[0-9a-f]+ <_start>
>> > +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a1,0x[0-9a-f]+
>> > +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a1,a1,[0-9]+ # [0-9a-f]+ <data_g>
>> > +.*:[ ]+[0-9a-f]+[ ]+lui[ ]+a2,0x[0-9a-f]+
>> > +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a2,a2,[0-9]+ # [0-9a-f]+ <data_g>
>> > +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a3,tp,0 # 0 <data_t>
>> > +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a0,0x[0-9a-f]+
>> > diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
>> > index c6c73c54265..984d457aed9 100644
>> > --- a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
>> > +++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
>> > @@ -1,6 +1,6 @@
>> > #source: pcgp-relax-02.s
>> > #as:
>> > -#ld: --relax
>> > +#ld: --relax --relax-gp
>> > #objdump: -d
>> >
>> > .*:[ ]+file format .*
>> > --
>> > 2.37.0.144.g8ac04bfd2-goog
>> >
>>
>> Ping. I maintain lld/ELF and we are considering a new opt-in option
>> --relax-gp to perform GP relaxation.
>> It will be opt-in since many users don't find the relaxation useful
>> and some feel GP relaxation will conflict with using GP for other
>> purposes (some ABI variants).
>> I wish that the two linkers agree on the same options.
[-- Attachment #2: 0001-RISC-V-Add-no-relax-gp-to-ld.patch --]
[-- Type: text/x-diff, Size: 8071 bytes --]
From c80f89bdcf65ca1350c423b6fab66b3029e1aaa7 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i@maskray.me>
Date: Sun, 19 Feb 2023 19:45:51 -0800
Subject: [PATCH] RISC-V: Add --[no-]relax-gp to ld
--relax enables all relaxations. --no-relax-gp disables GP relaxation to
allow measuring its effect.
Link: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/298
bfd/
* elfnn-riscv.c (struct riscv_elf_link_hash_table): Add params.
(riscv_elfNN_set_options): New.
(riscv_info_to_howto_rela): Check relax_gp.
(_bfd_riscv_relax_section): Likewise.
* elfxx-riscv.h (struct riscv_elf_params): New.
(riscv_elf32_set_options): New.
(riscv_elf64_set_options): New.
ld/
* emultempl/riscvelf.em: Add option parsing.
* testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d: New.
* testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d: New.
* testsuite/ld-riscv-elf/pcgp-relax-02.d: Test --relax --relax-gp can be
used together.
---
bfd/elfnn-riscv.c | 16 +++++++--
bfd/elfxx-riscv.h | 11 ++++++
ld/emultempl/riscvelf.em | 36 +++++++++++++++++++
.../code-model-relax-medlow-01-norelaxgp.d | 4 +++
ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 2 ++
.../ld-riscv-elf/pcgp-relax-01-norelaxgp.d | 18 ++++++++++
ld/testsuite/ld-riscv-elf/pcgp-relax-02.d | 2 +-
7 files changed, 86 insertions(+), 3 deletions(-)
create mode 100644 ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
create mode 100644 ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index c2604de0050..47e234c56e2 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -118,6 +118,9 @@ struct riscv_elf_link_hash_table
{
struct elf_link_hash_table elf;
+ /* Various options and other info passed from the linker. */
+ struct riscv_elf_params *params;
+
/* Short-cuts to get to dynamic linker sections. */
asection *sdyntdata;
@@ -157,6 +160,13 @@ struct riscv_elf_link_hash_table
&& elf_hash_table_id (elf_hash_table (p)) == RISCV_ELF_DATA) \
? (struct riscv_elf_link_hash_table *) (p)->hash : NULL)
+void
+riscv_elfNN_set_options (struct bfd_link_info *link_info,
+ struct riscv_elf_params *params)
+{
+ riscv_elf_hash_table (link_info)->params = params;
+}
+
static bool
riscv_info_to_howto_rela (bfd *abfd,
arelent *cache_ptr,
@@ -4389,7 +4399,9 @@ _bfd_riscv_relax_lui (bfd *abfd,
bool undefined_weak)
{
bfd_byte *contents = elf_section_data (sec)->this_hdr.contents;
- bfd_vma gp = riscv_global_pointer_value (link_info);
+ bfd_vma gp = riscv_elf_hash_table (link_info)->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);
@@ -4833,7 +4845,7 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec,
|| type == R_RISCV_TPREL_LO12_I
|| type == R_RISCV_TPREL_LO12_S)
relax_func = _bfd_riscv_relax_tls_le;
- else if (!bfd_link_pic (info)
+ else if (!bfd_link_pic (info) && htab->params->relax_gp
&& (type == R_RISCV_PCREL_HI20
|| type == R_RISCV_PCREL_LO12_I
|| type == R_RISCV_PCREL_LO12_S))
diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
index 5e1bdae9b2a..abcb409bd78 100644
--- a/bfd/elfxx-riscv.h
+++ b/bfd/elfxx-riscv.h
@@ -27,6 +27,17 @@
#define RISCV_UNKNOWN_VERSION -1
+struct riscv_elf_params
+{
+ /* Whether to relax code sequences to GP-relative addressing. */
+ bool relax_gp;
+};
+
+extern void riscv_elf32_set_options (struct bfd_link_info *,
+ struct riscv_elf_params *);
+extern void riscv_elf64_set_options (struct bfd_link_info *,
+ struct riscv_elf_params *);
+
extern reloc_howto_type *
riscv_reloc_name_lookup (bfd *, const char *);
diff --git a/ld/emultempl/riscvelf.em b/ld/emultempl/riscvelf.em
index b12d15065c4..bb6298d3e8d 100644
--- a/ld/emultempl/riscvelf.em
+++ b/ld/emultempl/riscvelf.em
@@ -25,6 +25,40 @@ fragment <<EOF
#include "elf/riscv.h"
#include "elfxx-riscv.h"
+static struct riscv_elf_params params = { .relax_gp = 1 };
+EOF
+
+# Define some shell vars to insert bits of code into the standard elf
+# parse_args and list_options functions. */
+PARSE_AND_LIST_PROLOGUE=${PARSE_AND_LIST_PROLOGUE}'
+enum risccv_opt
+{
+ OPTION_RELAX_GP = 321,
+ OPTION_NO_RELAX_GP,
+};
+'
+
+PARSE_AND_LIST_LONGOPTS=${PARSE_AND_LIST_LONGOPTS}'
+ { "relax-gp", no_argument, NULL, OPTION_RELAX_GP },
+ { "no-relax-gp", no_argument, NULL, OPTION_NO_RELAX_GP },
+'
+
+PARSE_AND_LIST_OPTIONS=${PARSE_AND_LIST_OPTIONS}'
+ fprintf (file, _(" --relax-gp Perform GP relaxation\n"));
+ fprintf (file, _(" --no-relax-gp Don'\''t perform GP relaxation\n"));
+'
+
+PARSE_AND_LIST_ARGS_CASES=${PARSE_AND_LIST_ARGS_CASES}'
+ case OPTION_RELAX_GP:
+ params.relax_gp = 1;
+ break;
+
+ case OPTION_NO_RELAX_GP:
+ params.relax_gp = 0;
+ break;
+'
+
+fragment <<EOF
static void
riscv_elf_before_allocation (void)
{
@@ -96,6 +130,8 @@ riscv_create_output_section_statements (void)
" whilst linking %s binaries\n"), "RISC-V");
return;
}
+
+ riscv_elf${ELFSIZE}_set_options (&link_info, ¶ms);
}
EOF
diff --git a/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
new file mode 100644
index 00000000000..8e40cc5f32d
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
@@ -0,0 +1,4 @@
+#source: code-model.s
+#as: -march=rv64i -mabi=lp64 --defsym __medlow__=1
+#ld: -Tcode-model-01.ld -melf64lriscv --no-relax-gp --relax
+#error: .*relocation truncated to fit: R_RISCV_HI20 against `symbolL'
diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
index 0f7ccd92ed5..5dd6144efd3 100644
--- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
+++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
@@ -122,6 +122,7 @@ if [istarget "riscv*-*-*"] {
run_dump_test "align-small-region"
run_dump_test "call-relax"
run_dump_test "pcgp-relax-01"
+ run_dump_test "pcgp-relax-01-norelaxgp"
run_dump_test "pcgp-relax-02"
run_dump_test "c-lui"
run_dump_test "c-lui-2"
@@ -141,6 +142,7 @@ if [istarget "riscv*-*-*"] {
run_dump_test "code-model-medany-weakref-01"
run_dump_test "code-model-medany-weakref-02"
run_dump_test "code-model-relax-medlow-01"
+ run_dump_test "code-model-relax-medlow-01-norelaxgp"
run_dump_test "code-model-relax-medlow-02"
run_dump_test "code-model-relax-medlow-weakref-01"
run_dump_test "code-model-relax-medlow-weakref-02"
diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
new file mode 100644
index 00000000000..d1344576ff3
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
@@ -0,0 +1,18 @@
+#source: pcgp-relax-01.s
+#ld: --no-relax-gp --relax
+#objdump: -d -Mno-aliases
+
+.*:[ ]+file format .*
+
+
+Disassembly of section \.text:
+
+0+[0-9a-f]+ <_start>:
+.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a0,a0,[0-9]+
+.*:[ ]+[0-9a-f]+[ ]+jal[ ]+ra,[0-9a-f]+ <_start>
+.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a1,0x[0-9a-f]+
+.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a1,a1,[0-9]+ # [0-9a-f]+ <data_g>
+.*:[ ]+[0-9a-f]+[ ]+lui[ ]+a2,0x[0-9a-f]+
+.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a2,a2,[0-9]+ # [0-9a-f]+ <data_g>
+.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a3,tp,0 # 0 <data_t>
+.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a0,0x[0-9a-f]+
diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
index 1248132ab42..055f03e57f2 100644
--- a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
+++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
@@ -1,6 +1,6 @@
#source: pcgp-relax-02.s
#as:
-#ld: --relax
+#ld: --relax --relax-gp
#objdump: -d
.*:[ ]+file format .*
--
2.39.2.637.g21b0678d19-goog
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: Add --[no-]relax-gp to ld
2023-02-20 3:46 ` Fangrui Song
@ 2023-02-20 10:02 ` Nelson Chu
0 siblings, 0 replies; 6+ messages in thread
From: Nelson Chu @ 2023-02-20 10:02 UTC (permalink / raw)
To: Fangrui Song; +Cc: Fangrui Song, binutils
Thanks! All LGTM :-)
Nelson
On Mon, Feb 20, 2023 at 11:46 AM Fangrui Song <maskray@google.com> wrote:
>
> On 2023-02-20, Nelson Chu wrote:
> >This is just a target option, and it's not only for compatibility with
> >lld, sometimes this option is also convenient to catch whether the gp
> >relaxation has a bug without rebuilding the linker to disable the
> >specific relaxation. Personally I would say this is a good idea at
> >least for debugging, we just default enable the relax flags in the
> >riscv_elf_params, so that everything won't change. Perhaps from the
> >debug point of view, it may be the right direction to have other
> >developer target options to close the remaining specific relaxations.
> >Anyway, this target option works for me, it's not only good for
> >developer debugging, but also keeps it compatible with lld. However,
> >it would be better to just wait so as not to miss some of the other
> >suggestions, I think 1-2 weeks should be enough.
> >
> >Thanks
> >Nelson
>
> Thank you for the feedback!
>
> >On Sat, Feb 18, 2023 at 3:31 PM Fangrui Song <i@maskray.me> wrote:
> >>
> >> On Wed, Jul 13, 2022 at 1:26 AM Fangrui Song <maskray@google.com> wrote:
> >> >
> >> > From: Fangrui Song <i@maskray.me>
> >> >
> >> > --relax enables all relaxations. --no-relax-gp disables GP relaxation to
> >> > allow measuring its effect.
> >> >
> >> > Link: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/298
> >> >
> >> > bfd/
> >> > * elfnn-riscv.c (struct riscv_elf_link_hash_table): Add params.
> >> > (riscv_elfNN_set_options): New.
> >> > (riscv_info_to_howto_rela): Check relax_gp.
> >> > (_bfd_riscv_relax_section): Likewise.
> >> > * elfxx-riscv.h (struct riscv_elf_params): New.
> >> > (riscv_elf32_set_options): New.
> >> > (riscv_elf64_set_options): New.
> >> > ld/
> >> > * emultempl/riscvelf.em: Add option parsing.
> >> > * testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d: New.
> >> > * testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d: New.
> >> > * testsuite/ld-riscv-elf/pcgp-relax-02.d: Test --relax --relax-gp can be
> >> > used together.
> >> > ---
> >> > bfd/elfnn-riscv.c | 22 ++++++++++---
> >> > bfd/elfxx-riscv.h | 11 +++++++
> >> > ld/emultempl/riscvelf.em | 31 +++++++++++++++++++
> >> > .../code-model-relax-medlow-01-norelaxgp.d | 4 +++
> >> > ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 2 ++
> >> > .../ld-riscv-elf/pcgp-relax-01-norelaxgp.d | 18 +++++++++++
> >> > ld/testsuite/ld-riscv-elf/pcgp-relax-02.d | 2 +-
> >> > 7 files changed, 84 insertions(+), 6 deletions(-)
> >> > create mode 100644 ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
> >> > create mode 100644 ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
> >> >
> >> > diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> >> > index 8f9f0d8a86a..527c3e74e97 100644
> >> > --- a/bfd/elfnn-riscv.c
> >> > +++ b/bfd/elfnn-riscv.c
> >> > @@ -118,6 +118,9 @@ struct riscv_elf_link_hash_table
> >> > {
> >> > struct elf_link_hash_table elf;
> >> >
> >> > + /* Various options and other info passed from the linker. */
> >> > + struct riscv_elf_params *params;
> >> > +
> >> > /* Short-cuts to get to dynamic linker sections. */
> >> > asection *sdyntdata;
> >> >
> >> > @@ -157,6 +160,13 @@ struct riscv_elf_link_hash_table
> >> > && elf_hash_table_id (elf_hash_table (p)) == RISCV_ELF_DATA) \
> >> > ? (struct riscv_elf_link_hash_table *) (p)->hash : NULL)
> >> >
> >> > +void
> >> > +riscv_elfNN_set_options (struct bfd_link_info *link_info,
> >> > + struct riscv_elf_params *params)
> >> > +{
> >> > + riscv_elf_hash_table (link_info)->params = params;
> >> > +}
> >> > +
> >> > static bool
> >> > riscv_info_to_howto_rela (bfd *abfd,
> >> > arelent *cache_ptr,
> >> > @@ -4289,7 +4299,9 @@ _bfd_riscv_relax_lui (bfd *abfd,
> >> > bool undefined_weak)
> >> > {
> >> > bfd_byte *contents = elf_section_data (sec)->this_hdr.contents;
> >> > - bfd_vma gp = riscv_global_pointer_value (link_info);
> >> > + bfd_vma gp = riscv_elf_hash_table (link_info)->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);
> >> > @@ -4754,10 +4766,10 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec,
> >> > || type == R_RISCV_TPREL_LO12_I
> >> > || type == R_RISCV_TPREL_LO12_S)
> >> > relax_func = _bfd_riscv_relax_tls_le;
> >> > - else if (!bfd_link_pic (info)
> >
> >Probably just add "riscv_elf_hash_table (info)->params->relax_gp"
> >here, to make the coding style same as above.
>
> I'm going to use:
>
> else if (!bfd_link_pic (info) && htab->params->relax_gp
> && (type == R_RISCV_PCREL_HI20
> ...
>
> >> > - && (type == R_RISCV_PCREL_HI20
> >> > - || type == R_RISCV_PCREL_LO12_I
> >> > - || type == R_RISCV_PCREL_LO12_S))
> >> > + else if ((type == R_RISCV_PCREL_HI20 || type == R_RISCV_PCREL_LO12_I
> >> > + || type == R_RISCV_PCREL_LO12_S)
> >> > + && !bfd_link_pic (info)
> >> > + && riscv_elf_hash_table (info)->params->relax_gp)
> >> > relax_func = _bfd_riscv_relax_pc;
> >> > else
> >> > continue;
> >> > diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> >> > index ea7126bdb4d..f9bd0ad882c 100644
> >> > --- a/bfd/elfxx-riscv.h
> >> > +++ b/bfd/elfxx-riscv.h
> >> > @@ -27,6 +27,17 @@
> >> >
> >> > #define RISCV_UNKNOWN_VERSION -1
> >> >
> >> > +struct riscv_elf_params
> >> > +{
> >> > + /* Whether to relax code sequences to GP-relative addressing. */
> >> > + bool relax_gp;
> >> > +};
> >> > +
> >> > +extern void riscv_elf32_set_options (struct bfd_link_info *,
> >> > + struct riscv_elf_params *);
> >> > +extern void riscv_elf64_set_options (struct bfd_link_info *,
> >> > + struct riscv_elf_params *);
> >> > +
> >> > extern reloc_howto_type *
> >> > riscv_reloc_name_lookup (bfd *, const char *);
> >> >
> >> > diff --git a/ld/emultempl/riscvelf.em b/ld/emultempl/riscvelf.em
> >> > index 645a807f239..37a0fd08e96 100644
> >> > --- a/ld/emultempl/riscvelf.em
> >> > +++ b/ld/emultempl/riscvelf.em
> >> > @@ -25,6 +25,35 @@ fragment <<EOF
> >> > #include "elf/riscv.h"
> >> > #include "elfxx-riscv.h"
> >> >
> >> > +static struct riscv_elf_params params = { .relax_gp = 1 };
> >> > +EOF
> >> > +
> >> > +# Define some shell vars to insert bits of code into the standard elf
> >> > +# parse_args and list_options functions. */
> >> > +PARSE_AND_LIST_PROLOGUE=${PARSE_AND_LIST_PROLOGUE}'
> >> > +enum risccv_opt
> >> > +{
> >> > + OPTION_RELAX_GP = 321,
> >> > + OPTION_NO_RELAX_GP,
> >> > +};
> >> > +'
> >
> >Not really sure if the value of base enum is flexible or has some
> >meaning. I see ppc starts with 321, mips and nds32 start with 301,
> >and aarch64 starts with 309. If the start value doesn't mean
> >anything, then it's fine here.
>
> I think the start value is arbitrary. I stick with the ppc style which is
> well maintained by Alan :)
>
> >> > +
> >> > +PARSE_AND_LIST_LONGOPTS=${PARSE_AND_LIST_LONGOPTS}'
> >> > + { "relax-gp", no_argument, NULL, OPTION_RELAX_GP },
> >> > + { "no-relax-gp", no_argument, NULL, OPTION_NO_RELAX_GP },
> >> > +'
> >> > +
> >
> >It's good to also define PARSE_AND_LIST_OPTIONS, so that --target-help
> >should work for the new options.
>
> Ack. I did not know PARSE_AND_LIST_OPTIONS.
>
> PARSE_AND_LIST_OPTIONS=${PARSE_AND_LIST_OPTIONS}'
> fprintf (file, _(" --relax-gp Perform GP relaxation\n"));
> fprintf (file, _(" --no-relax-gp Don'\''t perform GP relaxation\n"));
> '
>
> >> > +PARSE_AND_LIST_ARGS_CASES=${PARSE_AND_LIST_ARGS_CASES}'
> >> > + case OPTION_RELAX_GP:
> >> > + params.relax_gp = 1;
> >> > + break;
> >> > +
> >> > + case OPTION_NO_RELAX_GP:
> >> > + params.relax_gp = 0;
> >> > + break;
> >> > +'
> >> > +
> >> > +fragment <<EOF
> >> > static void
> >> > riscv_elf_before_allocation (void)
> >> > {
> >> > @@ -96,6 +125,8 @@ riscv_create_output_section_statements (void)
> >> > " whilst linking %s binaries\n"), "RISC-V");
> >> > return;
> >> > }
> >> > +
> >> > + riscv_elf${ELFSIZE}_set_options (&link_info, ¶ms);
> >> > }
> >> >
> >> > EOF
> >> > diff --git a/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
> >> > new file mode 100644
> >> > index 00000000000..8e40cc5f32d
> >> > --- /dev/null
> >> > +++ b/ld/testsuite/ld-riscv-elf/code-model-relax-medlow-01-norelaxgp.d
> >> > @@ -0,0 +1,4 @@
> >> > +#source: code-model.s
> >> > +#as: -march=rv64i -mabi=lp64 --defsym __medlow__=1
> >> > +#ld: -Tcode-model-01.ld -melf64lriscv --no-relax-gp --relax
> >> > +#error: .*relocation truncated to fit: R_RISCV_HI20 against `symbolL'
> >> > diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> >> > index 272424b33e3..1711d12ba23 100644
> >> > --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> >> > +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> >> > @@ -122,6 +122,7 @@ if [istarget "riscv*-*-*"] {
> >> > run_dump_test "align-small-region"
> >> > run_dump_test "call-relax"
> >> > run_dump_test "pcgp-relax-01"
> >> > + run_dump_test "pcgp-relax-01-norelaxgp"
> >> > run_dump_test "pcgp-relax-02"
> >> > run_dump_test "c-lui"
> >> > run_dump_test "c-lui-2"
> >> > @@ -141,6 +142,7 @@ if [istarget "riscv*-*-*"] {
> >> > run_dump_test "code-model-medany-weakref-01"
> >> > run_dump_test "code-model-medany-weakref-02"
> >> > run_dump_test "code-model-relax-medlow-01"
> >> > + run_dump_test "code-model-relax-medlow-01-norelaxgp"
> >> > run_dump_test "code-model-relax-medlow-02"
> >> > run_dump_test "code-model-relax-medlow-weakref-01"
> >> > run_dump_test "code-model-relax-medlow-weakref-02"
> >> > diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
> >> > new file mode 100644
> >> > index 00000000000..d1344576ff3
> >> > --- /dev/null
> >> > +++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-01-norelaxgp.d
> >> > @@ -0,0 +1,18 @@
> >> > +#source: pcgp-relax-01.s
> >> > +#ld: --no-relax-gp --relax
> >> > +#objdump: -d -Mno-aliases
> >> > +
> >> > +.*:[ ]+file format .*
> >> > +
> >> > +
> >> > +Disassembly of section \.text:
> >> > +
> >> > +0+[0-9a-f]+ <_start>:
> >> > +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a0,a0,[0-9]+
> >> > +.*:[ ]+[0-9a-f]+[ ]+jal[ ]+ra,[0-9a-f]+ <_start>
> >> > +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a1,0x[0-9a-f]+
> >> > +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a1,a1,[0-9]+ # [0-9a-f]+ <data_g>
> >> > +.*:[ ]+[0-9a-f]+[ ]+lui[ ]+a2,0x[0-9a-f]+
> >> > +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a2,a2,[0-9]+ # [0-9a-f]+ <data_g>
> >> > +.*:[ ]+[0-9a-f]+[ ]+addi[ ]+a3,tp,0 # 0 <data_t>
> >> > +.*:[ ]+[0-9a-f]+[ ]+auipc[ ]+a0,0x[0-9a-f]+
> >> > diff --git a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
> >> > index c6c73c54265..984d457aed9 100644
> >> > --- a/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
> >> > +++ b/ld/testsuite/ld-riscv-elf/pcgp-relax-02.d
> >> > @@ -1,6 +1,6 @@
> >> > #source: pcgp-relax-02.s
> >> > #as:
> >> > -#ld: --relax
> >> > +#ld: --relax --relax-gp
> >> > #objdump: -d
> >> >
> >> > .*:[ ]+file format .*
> >> > --
> >> > 2.37.0.144.g8ac04bfd2-goog
> >> >
> >>
> >> Ping. I maintain lld/ELF and we are considering a new opt-in option
> >> --relax-gp to perform GP relaxation.
> >> It will be opt-in since many users don't find the relaxation useful
> >> and some feel GP relaxation will conflict with using GP for other
> >> purposes (some ABI variants).
> >> I wish that the two linkers agree on the same options.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-02-20 11:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-13 8:26 [PATCH] RISC-V: Add --[no-]relax-gp to ld Fangrui Song
2022-07-13 9:10 ` Andrew Waterman
2023-02-18 7:30 ` Fangrui Song
2023-02-20 1:55 ` Nelson Chu
2023-02-20 3:46 ` Fangrui Song
2023-02-20 10:02 ` Nelson Chu
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).