diff mbox series

[v2,1/7] RISC-V: minor effort reduction in relocation specifier parsing

Message ID f1892080-b6fe-a164-73bb-91cd88302e0f@suse.com
State Accepted
Headers
Series RISC-V/gas: insn operand parsing |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Jan Beulich March 10, 2023, 9:25 a.m. UTC 
  The sole caller of parse_relocation() has already checked for the %
prefix, so there's no need to check for it again in the strncasecmp()
and there's also no reason to make the involved string literals longer
than necessary.
---
v2: New.

Comments

Jan Beulich March 10, 2023, 11:24 a.m. UTC | #1 
On 10.03.2023 10:25, Jan Beulich via Binutils wrote:
> The sole caller of parse_relocation() has already checked for the %
> prefix, so there's no need to check for it again in the strncasecmp()
> and there's also no reason to make the involved string literals longer
> than necessary.

Taking this together with the .altmacro observation in
https://sourceware.org/pipermail/binutils/2023-March/126601.html
it may be worthwhile to consider allowing a 2nd prefix character
for relocation specifiers (e.g. '@'), such that they could be
passed as macro arguments without conflicting with the % operator.
The change here is effectively laying the grounds for very easily
doing so (might then be as simple as a 1-line change).

Jan

> ---
> v2: New.
> 
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -2135,34 +2135,34 @@ macro (struct riscv_cl_insn *ip, express
>  
>  static const struct percent_op_match percent_op_utype[] =
>  {
> -  {"%tprel_hi", BFD_RELOC_RISCV_TPREL_HI20},
> -  {"%pcrel_hi", BFD_RELOC_RISCV_PCREL_HI20},
> -  {"%got_pcrel_hi", BFD_RELOC_RISCV_GOT_HI20},
> -  {"%tls_ie_pcrel_hi", BFD_RELOC_RISCV_TLS_GOT_HI20},
> -  {"%tls_gd_pcrel_hi", BFD_RELOC_RISCV_TLS_GD_HI20},
> -  {"%hi", BFD_RELOC_RISCV_HI20},
> +  {"tprel_hi", BFD_RELOC_RISCV_TPREL_HI20},
> +  {"pcrel_hi", BFD_RELOC_RISCV_PCREL_HI20},
> +  {"got_pcrel_hi", BFD_RELOC_RISCV_GOT_HI20},
> +  {"tls_ie_pcrel_hi", BFD_RELOC_RISCV_TLS_GOT_HI20},
> +  {"tls_gd_pcrel_hi", BFD_RELOC_RISCV_TLS_GD_HI20},
> +  {"hi", BFD_RELOC_RISCV_HI20},
>    {0, 0}
>  };
>  
>  static const struct percent_op_match percent_op_itype[] =
>  {
> -  {"%lo", BFD_RELOC_RISCV_LO12_I},
> -  {"%tprel_lo", BFD_RELOC_RISCV_TPREL_LO12_I},
> -  {"%pcrel_lo", BFD_RELOC_RISCV_PCREL_LO12_I},
> +  {"lo", BFD_RELOC_RISCV_LO12_I},
> +  {"tprel_lo", BFD_RELOC_RISCV_TPREL_LO12_I},
> +  {"pcrel_lo", BFD_RELOC_RISCV_PCREL_LO12_I},
>    {0, 0}
>  };
>  
>  static const struct percent_op_match percent_op_stype[] =
>  {
> -  {"%lo", BFD_RELOC_RISCV_LO12_S},
> -  {"%tprel_lo", BFD_RELOC_RISCV_TPREL_LO12_S},
> -  {"%pcrel_lo", BFD_RELOC_RISCV_PCREL_LO12_S},
> +  {"lo", BFD_RELOC_RISCV_LO12_S},
> +  {"tprel_lo", BFD_RELOC_RISCV_TPREL_LO12_S},
> +  {"pcrel_lo", BFD_RELOC_RISCV_PCREL_LO12_S},
>    {0, 0}
>  };
>  
>  static const struct percent_op_match percent_op_rtype[] =
>  {
> -  {"%tprel_add", BFD_RELOC_RISCV_TPREL_ADD},
> +  {"tprel_add", BFD_RELOC_RISCV_TPREL_ADD},
>    {0, 0}
>  };
>  
> @@ -2180,14 +2180,14 @@ parse_relocation (char **str, bfd_reloc_
>  		  const struct percent_op_match *percent_op)
>  {
>    for ( ; percent_op->str; percent_op++)
> -    if (strncasecmp (*str, percent_op->str, strlen (percent_op->str)) == 0)
> +    if (strncasecmp (*str + 1, percent_op->str, strlen (percent_op->str)) == 0)
>        {
> -	int len = strlen (percent_op->str);
> +	size_t len = 1 + strlen (percent_op->str);
>  
>  	if (!ISSPACE ((*str)[len]) && (*str)[len] != '(')
>  	  continue;
>  
> -	*str += strlen (percent_op->str);
> +	*str += len;
>  	*reloc = percent_op->reloc;
>  
>  	/* Check whether the output BFD supports this relocation.
>
diff mbox series

Patch

--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -2135,34 +2135,34 @@  macro (struct riscv_cl_insn *ip, express
 
 static const struct percent_op_match percent_op_utype[] =
 {
-  {"%tprel_hi", BFD_RELOC_RISCV_TPREL_HI20},
-  {"%pcrel_hi", BFD_RELOC_RISCV_PCREL_HI20},
-  {"%got_pcrel_hi", BFD_RELOC_RISCV_GOT_HI20},
-  {"%tls_ie_pcrel_hi", BFD_RELOC_RISCV_TLS_GOT_HI20},
-  {"%tls_gd_pcrel_hi", BFD_RELOC_RISCV_TLS_GD_HI20},
-  {"%hi", BFD_RELOC_RISCV_HI20},
+  {"tprel_hi", BFD_RELOC_RISCV_TPREL_HI20},
+  {"pcrel_hi", BFD_RELOC_RISCV_PCREL_HI20},
+  {"got_pcrel_hi", BFD_RELOC_RISCV_GOT_HI20},
+  {"tls_ie_pcrel_hi", BFD_RELOC_RISCV_TLS_GOT_HI20},
+  {"tls_gd_pcrel_hi", BFD_RELOC_RISCV_TLS_GD_HI20},
+  {"hi", BFD_RELOC_RISCV_HI20},
   {0, 0}
 };
 
 static const struct percent_op_match percent_op_itype[] =
 {
-  {"%lo", BFD_RELOC_RISCV_LO12_I},
-  {"%tprel_lo", BFD_RELOC_RISCV_TPREL_LO12_I},
-  {"%pcrel_lo", BFD_RELOC_RISCV_PCREL_LO12_I},
+  {"lo", BFD_RELOC_RISCV_LO12_I},
+  {"tprel_lo", BFD_RELOC_RISCV_TPREL_LO12_I},
+  {"pcrel_lo", BFD_RELOC_RISCV_PCREL_LO12_I},
   {0, 0}
 };
 
 static const struct percent_op_match percent_op_stype[] =
 {
-  {"%lo", BFD_RELOC_RISCV_LO12_S},
-  {"%tprel_lo", BFD_RELOC_RISCV_TPREL_LO12_S},
-  {"%pcrel_lo", BFD_RELOC_RISCV_PCREL_LO12_S},
+  {"lo", BFD_RELOC_RISCV_LO12_S},
+  {"tprel_lo", BFD_RELOC_RISCV_TPREL_LO12_S},
+  {"pcrel_lo", BFD_RELOC_RISCV_PCREL_LO12_S},
   {0, 0}
 };
 
 static const struct percent_op_match percent_op_rtype[] =
 {
-  {"%tprel_add", BFD_RELOC_RISCV_TPREL_ADD},
+  {"tprel_add", BFD_RELOC_RISCV_TPREL_ADD},
   {0, 0}
 };
 
@@ -2180,14 +2180,14 @@  parse_relocation (char **str, bfd_reloc_
 		  const struct percent_op_match *percent_op)
 {
   for ( ; percent_op->str; percent_op++)
-    if (strncasecmp (*str, percent_op->str, strlen (percent_op->str)) == 0)
+    if (strncasecmp (*str + 1, percent_op->str, strlen (percent_op->str)) == 0)
       {
-	int len = strlen (percent_op->str);
+	size_t len = 1 + strlen (percent_op->str);
 
 	if (!ISSPACE ((*str)[len]) && (*str)[len] != '(')
 	  continue;
 
-	*str += strlen (percent_op->str);
+	*str += len;
 	*reloc = percent_op->reloc;
 
 	/* Check whether the output BFD supports this relocation.