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

Message ID 20230920083124.2072273-1-ruiu@bluewhale.systems
State Unresolved
Headers
Series [v2] RISC-V: emit R_RISCV_RELAX for the la pseudo instruction |

Checks

Context Check Description
snail/binutils-gdb-check warning Git am fail log

Commit Message

Rui Ueyama Sept. 20, 2023, 8:31 a.m. UTC
  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(+)
  

Comments

Nelson Chu Sept. 21, 2023, 12:18 a.m. UTC | #1
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
>
>
  
Rui Ueyama Sept. 21, 2023, 5:32 a.m. UTC | #2
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
>>
  
Nelson Chu Sept. 21, 2023, 7:37 a.m. UTC | #3
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
  
Rui Ueyama Dec. 12, 2023, 6:40 a.m. UTC | #4
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
  
Andreas Schwab Dec. 12, 2023, 9:25 a.m. UTC | #5
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.
  
Nelson Chu Dec. 12, 2023, 9:38 a.m. UTC | #6
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
  

Patch

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\*