[PATCH] RISC-V: emit R_RISCV_RELAX for the la pseudo instruction

[PATCH] RISC-V: emit R_RISCV_RELAX for the la pseudo instruction

[Rui Ueyama]

Some psABIs define a relaxation to turn a GOT load into a PC-relative
address materialization. For example, the AArch64's psABI allows
adrp+ldr to be rewritten to nop+adr to eliminate the memory load.
This patch is part of the effort to make such optimization possible
for RISC-V.

For RISC-V, we use the la assembly pseudo instruction to load a symbol
address from the GOT. The pseudo instruction is expanded to auipc+ld.
If the address loaded by the instruction pair is actually a
PC-relative link-time constant, we want the linker to rewrite the
instruction pair with auipc+addi.

We can't rewrite all existing auipc+ld pairs with auipc+addi in the
linker because there might be code that jumps to the middle of the
instruction pair. That should be extremely rare, if ever exists, but
you can at least in theory write a program in assembly that jumps to
the ld instruction of the instruction pair. We need a marker to
identify that an auipc+ld can be safely relaxed (i.e. they are emitted
for la).

This patch is to annotate R_RISCV_GOT_HI20 with R_RISCV_RELAX only
when the relocation is emitted for the la pseudo instruction. The
linker will use it as a signal that the instruction pair can be safely
relaxed.

Proposal to the RISC-V psABI:
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/397
---
 bfd/bfd-in2.h                         | 1 +
 bfd/elfxx-riscv.c                     | 1 +
 gas/config/tc-riscv.c                 | 3 ++-
 gas/testsuite/gas/riscv/la-variants.d | 3 +++
 4 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 1c4f75ae244..e15641a1d00 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -5413,6 +5413,7 @@ number for the SBIC, SBIS, SBI and CBI instructions  */
   BFD_RELOC_RISCV_SUB32,
   BFD_RELOC_RISCV_SUB64,
   BFD_RELOC_RISCV_GOT_HI20,
+  BFD_RELOC_RISCV_GOT_HI20_RELAX,
   BFD_RELOC_RISCV_TLS_GOT_HI20,
   BFD_RELOC_RISCV_TLS_GD_HI20,
   BFD_RELOC_RISCV_JMP,
diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index 6ed657171f0..71e63e7b789 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -913,6 +913,7 @@ static const struct elf_reloc_map riscv_reloc_map[] =
   { BFD_RELOC_RISCV_PCREL_HI20, R_RISCV_PCREL_HI20 },
   { BFD_RELOC_RISCV_JMP, R_RISCV_JAL },
   { BFD_RELOC_RISCV_GOT_HI20, R_RISCV_GOT_HI20 },
+  { BFD_RELOC_RISCV_GOT_HI20_RELAX, R_RISCV_GOT_HI20 },
   { BFD_RELOC_RISCV_TLS_DTPMOD32, R_RISCV_TLS_DTPMOD32 },
   { BFD_RELOC_RISCV_TLS_DTPREL32, R_RISCV_TLS_DTPREL32 },
   { BFD_RELOC_RISCV_TLS_DTPMOD64, R_RISCV_TLS_DTPMOD64 },
diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 3b520ad208b..303ae18436c 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -2039,7 +2039,7 @@ macro (struct riscv_cl_insn *ip, expressionS *imm_expr,
       else if ((riscv_opts.pic && mask == M_LA)
 	       || mask == M_LGA)
 	pcrel_load (rd, rd, imm_expr, LOAD_ADDRESS_INSN,
-		    BFD_RELOC_RISCV_GOT_HI20, BFD_RELOC_RISCV_PCREL_LO12_I);
+		    BFD_RELOC_RISCV_GOT_HI20_RELAX, BFD_RELOC_RISCV_PCREL_LO12_I);
       /* Local PIC symbol, or any non-PIC symbol.  */
       else
 	pcrel_load (rd, rd, imm_expr, "addi",
@@ -4244,6 +4244,7 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED)
       relaxable = true;
       break;
 
+    case BFD_RELOC_RISCV_GOT_HI20_RELAX:
     case BFD_RELOC_RISCV_PCREL_HI20:
     case BFD_RELOC_RISCV_PCREL_LO12_S:
     case BFD_RELOC_RISCV_PCREL_LO12_I:
diff --git a/gas/testsuite/gas/riscv/la-variants.d b/gas/testsuite/gas/riscv/la-variants.d
index b1d316983b7..e8ac09c2af2 100644
--- a/gas/testsuite/gas/riscv/la-variants.d
+++ b/gas/testsuite/gas/riscv/la-variants.d
@@ -21,11 +21,13 @@ Disassembly of section .text:
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
 [ 	]+[0-9a-f]+:[ 	]+00000617[ 	]+auipc[ 	]+a2,0x0
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_GOT_HI20[ 	]+a
+[ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
 [ 	]+[0-9a-f]+:[ 	]+(00062603|00063603)[ 	]+(lw|ld)[ 	]+a2,0\(a2\).*
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_PCREL_LO12_I[ 	]+\.L0[ ]+
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
 [ 	]+[0-9a-f]+:[ 	]+00000697[ 	]+auipc[ 	]+a3,0x0
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_GOT_HI20[ 	]+a
+[ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
 [ 	]+[0-9a-f]+:[ 	]+(0006a683|0006b683)[ 	]+(lw|ld)[ 	]+a3,0\(a3\).*
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_PCREL_LO12_I[ 	]+\.L0[ ]+
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
@@ -37,6 +39,7 @@ Disassembly of section .text:
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
 [ 	]+[0-9a-f]+:[ 	]+00000797[ 	]+auipc[ 	]+a5,0x0
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_GOT_HI20[ 	]+a
+[ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
 [ 	]+[0-9a-f]+:[ 	]+(0007a783|0007b783)[ 	]+(lw|ld)[ 	]+a5,0\(a5\).*
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_PCREL_LO12_I[ 	]+\.L0[ ]+
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
-- 
2.34.1

Re: [PATCH] RISC-V: emit R_RISCV_RELAX for the la pseudo instruction

[Nelson Chu]

[-- Attachment #1: Type: text/plain, Size: 6670 bytes --]

On Tue, Sep 19, 2023 at 3:03 PM Rui Ueyama via Binutils <
binutils@sourceware.org> wrote:

> Some psABIs define a relaxation to turn a GOT load into a PC-relative
> address materialization. For example, the AArch64's psABI allows
> adrp+ldr to be rewritten to nop+adr to eliminate the memory load.
> This patch is part of the effort to make such optimization possible
> for RISC-V.
>
> For RISC-V, we use the la assembly pseudo instruction to load a symbol
> address from the GOT. The pseudo instruction is expanded to auipc+ld.
> If the address loaded by the instruction pair is actually a
> PC-relative link-time constant, we want the linker to rewrite the
> instruction pair with auipc+addi.
>
> We can't rewrite all existing auipc+ld pairs with auipc+addi in the
> linker because there might be code that jumps to the middle of the
> instruction pair. That should be extremely rare, if ever exists, but
> you can at least in theory write a program in assembly that jumps to
> the ld instruction of the instruction pair. We need a marker to
> identify that an auipc+ld can be safely relaxed (i.e. they are emitted
> for la).
>

Make sense.


> This patch is to annotate R_RISCV_GOT_HI20 with R_RISCV_RELAX only
> when the relocation is emitted for the la pseudo instruction. The
> linker will use it as a signal that the instruction pair can be safely
> relaxed.
>

Currently GNU ld won't do any relaxations for GOT patterns, so we will also
need the related updates in the future.  Besides, not sure if the new GOT
relaxations will break this feature for undefined weak symbol,
https://github.com/bminor/binutils-gdb/commit/9d1da81b261a20050ef2ad01a5b4c8cf78404222.
But fortunately if we just mark the R_RISCV_RELAX for GOT_HI20 in
assembler, that shouldn't cause a problem since currently ld will ignore it
for now.


> Proposal to the RISC-V psABI:
> https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/397
> ---
>  bfd/bfd-in2.h                         | 1 +
>  bfd/elfxx-riscv.c                     | 1 +
>  gas/config/tc-riscv.c                 | 3 ++-
>  gas/testsuite/gas/riscv/la-variants.d | 3 +++
>  4 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
> index 1c4f75ae244..e15641a1d00 100644
> --- a/bfd/bfd-in2.h
> +++ b/bfd/bfd-in2.h
> @@ -5413,6 +5413,7 @@ number for the SBIC, SBIS, SBI and CBI instructions
> */
>    BFD_RELOC_RISCV_SUB32,
>    BFD_RELOC_RISCV_SUB64,
>    BFD_RELOC_RISCV_GOT_HI20,
> +  BFD_RELOC_RISCV_GOT_HI20_RELAX,
>    BFD_RELOC_RISCV_TLS_GOT_HI20,
>    BFD_RELOC_RISCV_TLS_GD_HI20,
>    BFD_RELOC_RISCV_JMP,
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index 6ed657171f0..71e63e7b789 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -913,6 +913,7 @@ static const struct elf_reloc_map riscv_reloc_map[] =
>    { BFD_RELOC_RISCV_PCREL_HI20, R_RISCV_PCREL_HI20 },
>    { BFD_RELOC_RISCV_JMP, R_RISCV_JAL },
>    { BFD_RELOC_RISCV_GOT_HI20, R_RISCV_GOT_HI20 },
> +  { BFD_RELOC_RISCV_GOT_HI20_RELAX, R_RISCV_GOT_HI20 },
>    { BFD_RELOC_RISCV_TLS_DTPMOD32, R_RISCV_TLS_DTPMOD32 },
>    { BFD_RELOC_RISCV_TLS_DTPREL32, R_RISCV_TLS_DTPREL32 },
>    { BFD_RELOC_RISCV_TLS_DTPMOD64, R_RISCV_TLS_DTPMOD64 },
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 3b520ad208b..303ae18436c 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -2039,7 +2039,7 @@ macro (struct riscv_cl_insn *ip, expressionS
> *imm_expr,
>        else if ((riscv_opts.pic && mask == M_LA)
>                || mask == M_LGA)
>         pcrel_load (rd, rd, imm_expr, LOAD_ADDRESS_INSN,
> -                   BFD_RELOC_RISCV_GOT_HI20,
> BFD_RELOC_RISCV_PCREL_LO12_I);
> +                   BFD_RELOC_RISCV_GOT_HI20_RELAX,
> BFD_RELOC_RISCV_PCREL_LO12_I);
>        /* Local PIC symbol, or any non-PIC symbol.  */
>        else
>         pcrel_load (rd, rd, imm_expr, "addi",
> @@ -4244,6 +4244,7 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg
> ATTRIBUTE_UNUSED)
>        relaxable = true;
>        break;
>
> +    case BFD_RELOC_RISCV_GOT_HI20_RELAX:

     case BFD_RELOC_RISCV_PCREL_HI20:
>      case BFD_RELOC_RISCV_PCREL_LO12_S:
>      case BFD_RELOC_RISCV_PCREL_LO12_I:
>

In the gas/write.h, there is a special hook called tc_fix_data, which
allows backends to attach their own data to the fixup structure.  The fixup
structure is used to represent the relocation in assembler.  Therefore, I
suggest that we should add a new flag in the tc_fix_data to represent the
relocations which are expanded from the macro, setup the flag in the
macro_build function, and then enable to attach the R_RISCV_RELAX for
GOT_HI20 if it is expanded from the macro LA/LGA by checking that fag.  The
advantage of doing this is that we don't need to add a new special bfd
relocation for the pseudo LA, and the similar usage in the future also will
not need it.

Thanks
Nelson


> diff --git a/gas/testsuite/gas/riscv/la-variants.d
> b/gas/testsuite/gas/riscv/la-variants.d
> index b1d316983b7..e8ac09c2af2 100644
> --- a/gas/testsuite/gas/riscv/la-variants.d
> +++ b/gas/testsuite/gas/riscv/la-variants.d
> @@ -21,11 +21,13 @@ Disassembly of section .text:
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>  [      ]+[0-9a-f]+:[   ]+00000617[     ]+auipc[        ]+a2,0x0
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_GOT_HI20[     ]+a
> +[      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>  [      ]+[0-9a-f]+:[   ]+(00062603|00063603)[  ]+(lw|ld)[
> ]+a2,0\(a2\).*
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_PCREL_LO12_I[         ]+\.L0[ ]+
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>  [      ]+[0-9a-f]+:[   ]+00000697[     ]+auipc[        ]+a3,0x0
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_GOT_HI20[     ]+a
> +[      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>  [      ]+[0-9a-f]+:[   ]+(0006a683|0006b683)[  ]+(lw|ld)[
> ]+a3,0\(a3\).*
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_PCREL_LO12_I[         ]+\.L0[ ]+
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
> @@ -37,6 +39,7 @@ Disassembly of section .text:
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>  [      ]+[0-9a-f]+:[   ]+00000797[     ]+auipc[        ]+a5,0x0
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_GOT_HI20[     ]+a
> +[      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>  [      ]+[0-9a-f]+:[   ]+(0007a783|0007b783)[  ]+(lw|ld)[
> ]+a5,0\(a5\).*
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_PCREL_LO12_I[         ]+\.L0[ ]+
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
> --
> 2.34.1
>
>

[PATCH v2] RISC-V: emit R_RISCV_RELAX for the la pseudo instruction

[Rui Ueyama]

Now the macro identifier is stored to tc_fix_data if a relocation
is created as a result of assembler macro expansion.


---
 gas/config/tc-riscv.c                 | 15 +++++++++++++++
 gas/config/tc-riscv.h                 |  8 ++++++++
 gas/testsuite/gas/riscv/la-variants.d |  3 +++
 3 files changed, 26 insertions(+)

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 3b520ad208b..c761b793afc 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -59,6 +59,9 @@ struct riscv_cl_insn
   fixS *fixp;
 };
 
+/* The identifier of the assembler macro we are expanding, if any. */
+static int source_macro = -1;
+
 /* All RISC-V CSR belong to one of these classes.  */
 enum riscv_csr_class
 {
@@ -1659,6 +1662,7 @@ append_insn (struct riscv_cl_insn *ip, expressionS *address_expr,
 				  address_expr, false, reloc_type);
 
 	  ip->fixp->fx_tcbit = riscv_opts.relax;
+	  ip->fixp->tc_fix_data.source_macro = source_macro;
 	}
     }
 
@@ -2020,6 +2024,8 @@ macro (struct riscv_cl_insn *ip, expressionS *imm_expr,
   int rs2 = (ip->insn_opcode >> OP_SH_RS2) & OP_MASK_RS2;
   int mask = ip->insn_mo->mask;
 
+  source_macro = mask;
+
   switch (mask)
     {
     case M_LI:
@@ -2168,6 +2174,8 @@ macro (struct riscv_cl_insn *ip, expressionS *imm_expr,
       as_bad (_("internal: macro %s not implemented"), ip->insn_mo->name);
       break;
     }
+
+  source_macro = -1;
 }
 
 static const struct percent_op_match percent_op_utype[] =
@@ -4049,6 +4057,13 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED)
       break;
 
     case BFD_RELOC_RISCV_GOT_HI20:
+      /* R_RISCV_GOT_HI20 and the following R_RISCV_LO12_I are relaxable
+	 only if it is created as a result of la or lga assembler macros. */
+      if (fixP->tc_fix_data.source_macro == M_LA ||
+	  fixP->tc_fix_data.source_macro == M_LGA)
+	relaxable = true;
+      break;
+
     case BFD_RELOC_RISCV_ADD8:
     case BFD_RELOC_RISCV_ADD16:
     case BFD_RELOC_RISCV_ADD32:
diff --git a/gas/config/tc-riscv.h b/gas/config/tc-riscv.h
index 0c70c7d4739..4fba3a07829 100644
--- a/gas/config/tc-riscv.h
+++ b/gas/config/tc-riscv.h
@@ -101,6 +101,14 @@ extern void riscv_pre_output_hook (void);
 #define TC_FORCE_RELOCATION_LOCAL(FIX) 1
 #define DIFF_EXPR_OK 1
 
+struct riscv_fix
+{
+  int source_macro;
+};
+
+#define TC_FIX_TYPE struct riscv_fix
+#define TC_INIT_FIX_DATA(FIX) (FIX)->tc_fix_data.source_macro = -1
+
 extern void riscv_pop_insert (void);
 #define md_pop_insert()		riscv_pop_insert ()
 
diff --git a/gas/testsuite/gas/riscv/la-variants.d b/gas/testsuite/gas/riscv/la-variants.d
index b1d316983b7..e8ac09c2af2 100644
--- a/gas/testsuite/gas/riscv/la-variants.d
+++ b/gas/testsuite/gas/riscv/la-variants.d
@@ -21,11 +21,13 @@ Disassembly of section .text:
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
 [ 	]+[0-9a-f]+:[ 	]+00000617[ 	]+auipc[ 	]+a2,0x0
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_GOT_HI20[ 	]+a
+[ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
 [ 	]+[0-9a-f]+:[ 	]+(00062603|00063603)[ 	]+(lw|ld)[ 	]+a2,0\(a2\).*
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_PCREL_LO12_I[ 	]+\.L0[ ]+
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
 [ 	]+[0-9a-f]+:[ 	]+00000697[ 	]+auipc[ 	]+a3,0x0
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_GOT_HI20[ 	]+a
+[ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
 [ 	]+[0-9a-f]+:[ 	]+(0006a683|0006b683)[ 	]+(lw|ld)[ 	]+a3,0\(a3\).*
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_PCREL_LO12_I[ 	]+\.L0[ ]+
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
@@ -37,6 +39,7 @@ Disassembly of section .text:
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
 [ 	]+[0-9a-f]+:[ 	]+00000797[ 	]+auipc[ 	]+a5,0x0
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_GOT_HI20[ 	]+a
+[ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
 [ 	]+[0-9a-f]+:[ 	]+(0007a783|0007b783)[ 	]+(lw|ld)[ 	]+a5,0\(a5\).*
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_PCREL_LO12_I[ 	]+\.L0[ ]+
 [ 	]+[0-9a-f]+:[ 	]+R_RISCV_RELAX[ 	]+\*ABS\*
-- 
2.34.1

Re: [PATCH v2] RISC-V: emit R_RISCV_RELAX for the la pseudo instruction

[Nelson Chu]

[-- Attachment #1: Type: text/plain, Size: 4601 bytes --]

Looks good, thanks!

Nelson

On Wed, Sep 20, 2023 at 4:34 PM Rui Ueyama <rui314@gmail.com> wrote:

> Now the macro identifier is stored to tc_fix_data if a relocation
> is created as a result of assembler macro expansion.
>
>
> ---
>  gas/config/tc-riscv.c                 | 15 +++++++++++++++
>  gas/config/tc-riscv.h                 |  8 ++++++++
>  gas/testsuite/gas/riscv/la-variants.d |  3 +++
>  3 files changed, 26 insertions(+)
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 3b520ad208b..c761b793afc 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -59,6 +59,9 @@ struct riscv_cl_insn
>    fixS *fixp;
>  };
>
> +/* The identifier of the assembler macro we are expanding, if any. */
> +static int source_macro = -1;
> +
>  /* All RISC-V CSR belong to one of these classes.  */
>  enum riscv_csr_class
>  {
> @@ -1659,6 +1662,7 @@ append_insn (struct riscv_cl_insn *ip, expressionS
> *address_expr,
>                                   address_expr, false, reloc_type);
>
>           ip->fixp->fx_tcbit = riscv_opts.relax;
> +         ip->fixp->tc_fix_data.source_macro = source_macro;
>         }
>      }
>
> @@ -2020,6 +2024,8 @@ macro (struct riscv_cl_insn *ip, expressionS
> *imm_expr,
>    int rs2 = (ip->insn_opcode >> OP_SH_RS2) & OP_MASK_RS2;
>    int mask = ip->insn_mo->mask;
>
> +  source_macro = mask;
> +
>    switch (mask)
>      {
>      case M_LI:
> @@ -2168,6 +2174,8 @@ macro (struct riscv_cl_insn *ip, expressionS
> *imm_expr,
>        as_bad (_("internal: macro %s not implemented"), ip->insn_mo->name);
>        break;
>      }
> +
> +  source_macro = -1;
>  }
>
>  static const struct percent_op_match percent_op_utype[] =
> @@ -4049,6 +4057,13 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg
> ATTRIBUTE_UNUSED)
>        break;
>
>      case BFD_RELOC_RISCV_GOT_HI20:
> +      /* R_RISCV_GOT_HI20 and the following R_RISCV_LO12_I are relaxable
> +        only if it is created as a result of la or lga assembler macros.
> */
> +      if (fixP->tc_fix_data.source_macro == M_LA ||
> +         fixP->tc_fix_data.source_macro == M_LGA)
> +       relaxable = true;
> +      break;
> +
>      case BFD_RELOC_RISCV_ADD8:
>      case BFD_RELOC_RISCV_ADD16:
>      case BFD_RELOC_RISCV_ADD32:
> diff --git a/gas/config/tc-riscv.h b/gas/config/tc-riscv.h
> index 0c70c7d4739..4fba3a07829 100644
> --- a/gas/config/tc-riscv.h
> +++ b/gas/config/tc-riscv.h
> @@ -101,6 +101,14 @@ extern void riscv_pre_output_hook (void);
>  #define TC_FORCE_RELOCATION_LOCAL(FIX) 1
>  #define DIFF_EXPR_OK 1
>
> +struct riscv_fix
> +{
> +  int source_macro;
> +};
> +
> +#define TC_FIX_TYPE struct riscv_fix
> +#define TC_INIT_FIX_DATA(FIX) (FIX)->tc_fix_data.source_macro = -1
> +
>  extern void riscv_pop_insert (void);
>  #define md_pop_insert()                riscv_pop_insert ()
>
> diff --git a/gas/testsuite/gas/riscv/la-variants.d
> b/gas/testsuite/gas/riscv/la-variants.d
> index b1d316983b7..e8ac09c2af2 100644
> --- a/gas/testsuite/gas/riscv/la-variants.d
> +++ b/gas/testsuite/gas/riscv/la-variants.d
> @@ -21,11 +21,13 @@ Disassembly of section .text:
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>  [      ]+[0-9a-f]+:[   ]+00000617[     ]+auipc[        ]+a2,0x0
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_GOT_HI20[     ]+a
> +[      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>  [      ]+[0-9a-f]+:[   ]+(00062603|00063603)[  ]+(lw|ld)[
> ]+a2,0\(a2\).*
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_PCREL_LO12_I[         ]+\.L0[ ]+
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>  [      ]+[0-9a-f]+:[   ]+00000697[     ]+auipc[        ]+a3,0x0
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_GOT_HI20[     ]+a
> +[      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>  [      ]+[0-9a-f]+:[   ]+(0006a683|0006b683)[  ]+(lw|ld)[
> ]+a3,0\(a3\).*
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_PCREL_LO12_I[         ]+\.L0[ ]+
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
> @@ -37,6 +39,7 @@ Disassembly of section .text:
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>  [      ]+[0-9a-f]+:[   ]+00000797[     ]+auipc[        ]+a5,0x0
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_GOT_HI20[     ]+a
> +[      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>  [      ]+[0-9a-f]+:[   ]+(0007a783|0007b783)[  ]+(lw|ld)[
> ]+a5,0\(a5\).*
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_PCREL_LO12_I[         ]+\.L0[ ]+
>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
> --
> 2.34.1
>
>

Re: [PATCH v2] RISC-V: emit R_RISCV_RELAX for the la pseudo instruction

[Rui Ueyama]

Thank you for reviewing! I'll let you know when you can merge this
patch (i.e. when my proposal is ratified.)

Meanwhile, do I need to fill in the copyright assignment form?


On Thu, Sep 21, 2023 at 9:19 AM Nelson Chu <nelson@rivosinc.com> wrote:
>
> Looks good, thanks!
>
> Nelson
>
> On Wed, Sep 20, 2023 at 4:34 PM Rui Ueyama <rui314@gmail.com> wrote:
>>
>> Now the macro identifier is stored to tc_fix_data if a relocation
>> is created as a result of assembler macro expansion.
>>
>>
>> ---
>>  gas/config/tc-riscv.c                 | 15 +++++++++++++++
>>  gas/config/tc-riscv.h                 |  8 ++++++++
>>  gas/testsuite/gas/riscv/la-variants.d |  3 +++
>>  3 files changed, 26 insertions(+)
>>
>> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
>> index 3b520ad208b..c761b793afc 100644
>> --- a/gas/config/tc-riscv.c
>> +++ b/gas/config/tc-riscv.c
>> @@ -59,6 +59,9 @@ struct riscv_cl_insn
>>    fixS *fixp;
>>  };
>>
>> +/* The identifier of the assembler macro we are expanding, if any. */
>> +static int source_macro = -1;
>> +
>>  /* All RISC-V CSR belong to one of these classes.  */
>>  enum riscv_csr_class
>>  {
>> @@ -1659,6 +1662,7 @@ append_insn (struct riscv_cl_insn *ip, expressionS *address_expr,
>>                                   address_expr, false, reloc_type);
>>
>>           ip->fixp->fx_tcbit = riscv_opts.relax;
>> +         ip->fixp->tc_fix_data.source_macro = source_macro;
>>         }
>>      }
>>
>> @@ -2020,6 +2024,8 @@ macro (struct riscv_cl_insn *ip, expressionS *imm_expr,
>>    int rs2 = (ip->insn_opcode >> OP_SH_RS2) & OP_MASK_RS2;
>>    int mask = ip->insn_mo->mask;
>>
>> +  source_macro = mask;
>> +
>>    switch (mask)
>>      {
>>      case M_LI:
>> @@ -2168,6 +2174,8 @@ macro (struct riscv_cl_insn *ip, expressionS *imm_expr,
>>        as_bad (_("internal: macro %s not implemented"), ip->insn_mo->name);
>>        break;
>>      }
>> +
>> +  source_macro = -1;
>>  }
>>
>>  static const struct percent_op_match percent_op_utype[] =
>> @@ -4049,6 +4057,13 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED)
>>        break;
>>
>>      case BFD_RELOC_RISCV_GOT_HI20:
>> +      /* R_RISCV_GOT_HI20 and the following R_RISCV_LO12_I are relaxable
>> +        only if it is created as a result of la or lga assembler macros. */
>> +      if (fixP->tc_fix_data.source_macro == M_LA ||
>> +         fixP->tc_fix_data.source_macro == M_LGA)
>> +       relaxable = true;
>> +      break;
>> +
>>      case BFD_RELOC_RISCV_ADD8:
>>      case BFD_RELOC_RISCV_ADD16:
>>      case BFD_RELOC_RISCV_ADD32:
>> diff --git a/gas/config/tc-riscv.h b/gas/config/tc-riscv.h
>> index 0c70c7d4739..4fba3a07829 100644
>> --- a/gas/config/tc-riscv.h
>> +++ b/gas/config/tc-riscv.h
>> @@ -101,6 +101,14 @@ extern void riscv_pre_output_hook (void);
>>  #define TC_FORCE_RELOCATION_LOCAL(FIX) 1
>>  #define DIFF_EXPR_OK 1
>>
>> +struct riscv_fix
>> +{
>> +  int source_macro;
>> +};
>> +
>> +#define TC_FIX_TYPE struct riscv_fix
>> +#define TC_INIT_FIX_DATA(FIX) (FIX)->tc_fix_data.source_macro = -1
>> +
>>  extern void riscv_pop_insert (void);
>>  #define md_pop_insert()                riscv_pop_insert ()
>>
>> diff --git a/gas/testsuite/gas/riscv/la-variants.d b/gas/testsuite/gas/riscv/la-variants.d
>> index b1d316983b7..e8ac09c2af2 100644
>> --- a/gas/testsuite/gas/riscv/la-variants.d
>> +++ b/gas/testsuite/gas/riscv/la-variants.d
>> @@ -21,11 +21,13 @@ Disassembly of section .text:
>>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>>  [      ]+[0-9a-f]+:[   ]+00000617[     ]+auipc[        ]+a2,0x0
>>  [      ]+[0-9a-f]+:[   ]+R_RISCV_GOT_HI20[     ]+a
>> +[      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>>  [      ]+[0-9a-f]+:[   ]+(00062603|00063603)[  ]+(lw|ld)[      ]+a2,0\(a2\).*
>>  [      ]+[0-9a-f]+:[   ]+R_RISCV_PCREL_LO12_I[         ]+\.L0[ ]+
>>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>>  [      ]+[0-9a-f]+:[   ]+00000697[     ]+auipc[        ]+a3,0x0
>>  [      ]+[0-9a-f]+:[   ]+R_RISCV_GOT_HI20[     ]+a
>> +[      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>>  [      ]+[0-9a-f]+:[   ]+(0006a683|0006b683)[  ]+(lw|ld)[      ]+a3,0\(a3\).*
>>  [      ]+[0-9a-f]+:[   ]+R_RISCV_PCREL_LO12_I[         ]+\.L0[ ]+
>>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>> @@ -37,6 +39,7 @@ Disassembly of section .text:
>>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>>  [      ]+[0-9a-f]+:[   ]+00000797[     ]+auipc[        ]+a5,0x0
>>  [      ]+[0-9a-f]+:[   ]+R_RISCV_GOT_HI20[     ]+a
>> +[      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>>  [      ]+[0-9a-f]+:[   ]+(0007a783|0007b783)[  ]+(lw|ld)[      ]+a5,0\(a5\).*
>>  [      ]+[0-9a-f]+:[   ]+R_RISCV_PCREL_LO12_I[         ]+\.L0[ ]+
>>  [      ]+[0-9a-f]+:[   ]+R_RISCV_RELAX[        ]+\*ABS\*
>> --
>> 2.34.1
>>

Re: [PATCH v2] RISC-V: emit R_RISCV_RELAX for the la pseudo instruction

[Nelson Chu]

[-- Attachment #1: Type: text/plain, Size: 624 bytes --]

On Thu, Sep 21, 2023 at 1:32 PM Rui Ueyama <ruiu@bluewhale.systems> wrote:

> Thank you for reviewing! I'll let you know when you can merge this
> patch (i.e. when my proposal is ratified.)
>

Thanks :-)


> Meanwhile, do I need to fill in the copyright assignment form?
>

Oops, I almost forgot this...  Yes, it would be great and convenient if you
have the assignment, I remember the new form was here,
https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=tree;f=doc/Copyright
You can fill one of them (probably, seem like all three look the same ),
and then send it to assign@gnu.org.

Thanks
Nelson

Re: [PATCH v2] RISC-V: emit R_RISCV_RELAX for the la pseudo instruction

[Rui Ueyama]

https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/397 has been merged.
I also completed the copyright assignment form.

Could you merge this patch?

On Thu, Sep 21, 2023 at 4:37 PM Nelson Chu <nelson@rivosinc.com> wrote:
>
> On Thu, Sep 21, 2023 at 1:32 PM Rui Ueyama <ruiu@bluewhale.systems> wrote:
>>
>> Thank you for reviewing! I'll let you know when you can merge this
>> patch (i.e. when my proposal is ratified.)
>
>
> Thanks :-)
>
>>
>> Meanwhile, do I need to fill in the copyright assignment form?
>
>
> Oops, I almost forgot this...  Yes, it would be great and convenient if you have the assignment, I remember the new form was here,
> https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=tree;f=doc/Copyright
> You can fill one of them (probably, seem like all three look the same ), and then send it to assign@gnu.org.
>
> Thanks
> Nelson

Re: [PATCH v2] RISC-V: emit R_RISCV_RELAX for the la pseudo instruction

[Andreas Schwab]

On Sep 20 2023, Rui Ueyama via Binutils wrote:

> @@ -4049,6 +4057,13 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED)
>        break;
>  
>      case BFD_RELOC_RISCV_GOT_HI20:
> +      /* R_RISCV_GOT_HI20 and the following R_RISCV_LO12_I are relaxable
> +	 only if it is created as a result of la or lga assembler macros. */
> +      if (fixP->tc_fix_data.source_macro == M_LA ||
> +	  fixP->tc_fix_data.source_macro == M_LGA)

Line break before the operator, not after.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH v2] RISC-V: emit R_RISCV_RELAX for the la pseudo instruction

[Nelson Chu]

[-- Attachment #1: Type: text/plain, Size: 1004 bytes --]

On Tue, Dec 12, 2023 at 2:40 PM Rui Ueyama <rui314@gmail.com> wrote:

> https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/397 has been
> merged.
> I also completed the copyright assignment form.
>
> Could you merge this patch?
>

Sure, Thanks for helping to push this through.

On Tue, Dec 12, 2023 at 5:25 PM Andreas Schwab <schwab@suse.de> wrote:

> On Sep 20 2023, Rui Ueyama via Binutils wrote:
>
> > @@ -4049,6 +4057,13 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg
> ATTRIBUTE_UNUSED)
> >        break;
> >
> >      case BFD_RELOC_RISCV_GOT_HI20:
> > +      /* R_RISCV_GOT_HI20 and the following R_RISCV_LO12_I are relaxable
> > +      only if it is created as a result of la or lga assembler macros.
> */
> > +      if (fixP->tc_fix_data.source_macro == M_LA ||
> > +       fixP->tc_fix_data.source_macro == M_LGA)
>
> Line break before the operator, not after.


Thanks, committed with updated ChangeLog/messages and this line break fix.

Nelson