[PATCH] RISC-V: Output mapping symbols with ISA string once .option arch is used.

[PATCH] RISC-V: Output mapping symbols with ISA string once .option arch is used.

[Nelson Chu]

[1] Proposal: Extend .option directive for control enabled extensions on
specific code region,
https://github.com/riscv-non-isa/riscv-asm-manual/pull/67

[2] Proposal: Add mapping symbol,
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/196

Originally, the extended .option directive [1] was used to resolve the known
ifunc issue, but it also allows users to enable and disable extensions of a
certain code region at will.  Once the extensions have overlapped encodings,
it is difficult to dump the instructions correctly for now.  Therefore, the
mapping symbols with ISA string [2] for different code regions was proposed
to resolve the dump issue, and this patch is for that purpose.

The riscv-gnu-toolchain regressions passed for this patch.

gas/
    * config/tc-riscv.c (updated_riscv_subsets): New boolean.  Output the
    instruction mapping symbols with ISA string if it is true, otherwise
    set it to false.
    (remove_mapping_symbol): New function.  Remove the previous mapping
    symbol by checking the current one.
    (make_mapping_symbol): Added the current ISA string to $x when
    updated_riscv_subsets is true.  Besides, call remove_mapping_symbol
    rather than jusr remove the previous symbol.
    (riscv_mapping_state): Handle new mapping type MAP_INSN_ARCH, and also
    call remove_mapping_symbol before removing it.
    (s_riscv_option): Set updated_riscv_subsets to true once we meet .option
    rvc/norvc/arch/pop directives.
    * testsuite/gas/riscv/mapping-01a.d: Updated.
    * testsuite/gas/riscv/mapping-02a.d: Likewise.
    * testsuite/gas/riscv/mapping-03a.d: Likewise.
    * testsuite/gas/riscv/mapping-04a.d: Likewise.
    * testsuite/gas/riscv/mapping-norelax-03a.d: Likewise.
    * testsuite/gas/riscv/mapping-norelax-04a.d: Likewise.
    * testsuite/gas/riscv/option-arch-01a.d: Dump frcsr since f should be
    enabled in that code region.
include/
    * opcode/riscv.h (enum riscv_seg_mstat): Added MAP_INSN_ARCH, means
    instructions with specific extensions.
opcodes/
    * riscv-dis.c (riscv_disassemble_insn): Set riscv_fpr_names back to
    riscv_fpr_names_abi or riscv_fpr_names_numeric when zfinx is disabled
    for some specfic code region.
    (riscv_get_map_state): Recognized mapping symbols with ISA, and then
    reset the architecture string once the ISA is different.
---
 gas/config/tc-riscv.c                         | 63 ++++++++++++++++++++++-----
 gas/testsuite/gas/riscv/mapping-01a.d         |  2 +-
 gas/testsuite/gas/riscv/mapping-02a.d         |  2 +-
 gas/testsuite/gas/riscv/mapping-03a.d         |  2 +-
 gas/testsuite/gas/riscv/mapping-04a.d         |  2 +-
 gas/testsuite/gas/riscv/mapping-norelax-03a.d |  2 +-
 gas/testsuite/gas/riscv/mapping-norelax-04a.d |  2 +-
 gas/testsuite/gas/riscv/option-arch-01a.d     |  2 +-
 include/opcode/riscv.h                        |  1 +
 opcodes/riscv-dis.c                           | 12 ++++-
 10 files changed, 71 insertions(+), 19 deletions(-)

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 34ce68e..f61dd86 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -282,6 +282,9 @@ struct riscv_option_stack
 
 static struct riscv_option_stack *riscv_opts_stack = NULL;
 
+/* Output the instruction mapping symbols with ISA string if it is true.  */
+static bool updated_riscv_subsets = false;
+
 /* Set which ISA and extensions are available.  */
 
 static void
@@ -445,6 +448,22 @@ static char *expr_end;
 #define OPCODE_MATCHES(OPCODE, OP) \
   (((OPCODE) & MASK_##OP) == MATCH_##OP)
 
+/* Remove PREVIOUS mapping symbol by checking the CURRENT.  */
+
+static void
+remove_mapping_symbol (symbolS *previous, symbolS *current)
+{
+  /* If the mapping symbol we add isn't MAP_INSN_ARCH, but
+     the removed previous mapping symbol is MAP_INSN_ARCH,
+     then the next MAP_INSN should be converted to MAP_INSN_ARCH.  */
+  if (current != NULL
+      && !strncmp (S_GET_NAME (current), "$xrv", 4) == 0
+      && strncmp (S_GET_NAME (previous), "$xrv", 4) == 0)
+    updated_riscv_subsets = true;
+
+  symbol_remove (previous, &symbol_rootP, &symbol_lastP);
+}
+
 /* Create a new mapping symbol for the transition to STATE.  */
 
 static void
@@ -452,14 +471,28 @@ make_mapping_symbol (enum riscv_seg_mstate state,
 		     valueT value,
 		     fragS *frag)
 {
-  const char *name;
+  const char *name = NULL;
+  char *buff = NULL;
+
   switch (state)
     {
     case MAP_DATA:
       name = "$d";
       break;
     case MAP_INSN:
-      name = "$x";
+    case MAP_INSN_ARCH:
+      if (updated_riscv_subsets)
+	{
+	  char *isa = riscv_arch_str (xlen, riscv_subsets);
+	  size_t size = strlen (isa) + 3; /* "rv" + '\0'  */
+	  buff = xmalloc (size);
+	  snprintf (buff, size, "$x%s", isa);
+	  xfree ((void *) isa);
+	  name = buff;
+	  updated_riscv_subsets = false;
+	}
+      else
+	name = "$x";
       break;
     default:
       abort ();
@@ -467,19 +500,19 @@ make_mapping_symbol (enum riscv_seg_mstate state,
 
   symbolS *symbol = symbol_new (name, now_seg, frag, value);
   symbol_get_bfdsym (symbol)->flags |= (BSF_NO_FLAGS | BSF_LOCAL);
+  xfree ((void *) buff);
 
   /* If .fill or other data filling directive generates zero sized data,
      or we are adding odd alignemnts, then the mapping symbol for the
      following code will have the same value.  */
   if (value == 0)
     {
-       if (frag->tc_frag_data.first_map_symbol != NULL)
+      if (frag->tc_frag_data.first_map_symbol != NULL)
 	{
 	  know (S_GET_VALUE (frag->tc_frag_data.first_map_symbol)
 		== S_GET_VALUE (symbol));
 	  /* Remove the old one.  */
-	  symbol_remove (frag->tc_frag_data.first_map_symbol,
-			 &symbol_rootP, &symbol_lastP);
+	  remove_mapping_symbol (frag->tc_frag_data.first_map_symbol, symbol);
 	}
       frag->tc_frag_data.first_map_symbol = symbol;
     }
@@ -491,8 +524,7 @@ make_mapping_symbol (enum riscv_seg_mstate state,
       /* Remove the old one.  */
       if (S_GET_VALUE (frag->tc_frag_data.last_map_symbol)
 	  == S_GET_VALUE (symbol))
-	symbol_remove (frag->tc_frag_data.last_map_symbol,
-		       &symbol_rootP, &symbol_lastP);
+	remove_mapping_symbol (frag->tc_frag_data.last_map_symbol, symbol);
     }
   frag->tc_frag_data.last_map_symbol = symbol;
 }
@@ -514,9 +546,14 @@ riscv_mapping_state (enum riscv_seg_mstate to_state,
       || !subseg_text_p (now_seg))
     return;
 
+  if (to_state == MAP_INSN && updated_riscv_subsets)
+    to_state = MAP_INSN_ARCH;
+
   /* The mapping symbol should be emitted if not in the right
-     mapping state  */
-  if (from_state == to_state)
+     mapping state.  */
+  if ((from_state == MAP_DATA && to_state == MAP_DATA)
+      || (from_state == MAP_INSN && to_state == MAP_INSN)
+      || (from_state == MAP_INSN_ARCH && to_state == MAP_INSN))
     return;
 
   valueT value = (valueT) (frag_now_fix () - max_chars);
@@ -573,7 +610,7 @@ riscv_check_mapping_symbols (bfd *abfd ATTRIBUTE_UNUSED,
 	    {
 	      /* The last mapping symbol overlaps with another one
 		 which at the start of the next frag.  */
-	      symbol_remove (last, &symbol_rootP, &symbol_lastP);
+	      remove_mapping_symbol (last, NULL);
 	      break;
 	    }
 
@@ -581,7 +618,7 @@ riscv_check_mapping_symbols (bfd *abfd ATTRIBUTE_UNUSED,
 	    {
 	      /* The last mapping symbol is at the end of the section.  */
 	      know (next->fr_fix == 0 && next->fr_var == 0);
-	      symbol_remove (last, &symbol_rootP, &symbol_lastP);
+	      remove_mapping_symbol (last, NULL);
 	      break;
 	    }
 
@@ -3855,11 +3892,13 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
   if (strcmp (name, "rvc") == 0)
     {
       riscv_update_subset (&riscv_rps_as, "+c");
+      updated_riscv_subsets = true;
       riscv_set_rvc (true);
     }
   else if (strcmp (name, "norvc") == 0)
     {
       riscv_update_subset (&riscv_rps_as, "-c");
+      updated_riscv_subsets = true;
       riscv_set_rvc (false);
     }
   else if (strcmp (name, "pic") == 0)
@@ -3880,6 +3919,7 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
       if (ISSPACE (*name) && *name != '\0')
 	name++;
       riscv_update_subset (&riscv_rps_as, name);
+      updated_riscv_subsets = true;
 
       riscv_set_rvc (false);
       if (riscv_subset_supports (&riscv_rps_as, "c"))
@@ -3911,6 +3951,7 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
 	  riscv_opts = s->options;
 	  riscv_subsets = s->subset_list;
 	  riscv_rps_as.subset_list = riscv_subsets;
+	  updated_riscv_subsets = true;
 	  riscv_release_subset_list (release_subsets);
 	  free (s);
 	}
diff --git a/gas/testsuite/gas/riscv/mapping-01a.d b/gas/testsuite/gas/riscv/mapping-01a.d
index 32e0027..2aefe5a 100644
--- a/gas/testsuite/gas/riscv/mapping-01a.d
+++ b/gas/testsuite/gas/riscv/mapping-01a.d
@@ -8,7 +8,7 @@ SYMBOL TABLE:
 0+00 l    d  .text	0+00 .text
 0+00 l    d  .data	0+00 .data
 0+00 l    d  .bss	0+00 .bss
-0+00 l       .text	0+00 \$x
+0+00 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+00 l    d  .foo	0+00 .foo
 0+00 l       .foo	0+00 foo
 0+00 l       .foo	0+00 \$x
diff --git a/gas/testsuite/gas/riscv/mapping-02a.d b/gas/testsuite/gas/riscv/mapping-02a.d
index 333f12c..155ae02 100644
--- a/gas/testsuite/gas/riscv/mapping-02a.d
+++ b/gas/testsuite/gas/riscv/mapping-02a.d
@@ -9,7 +9,7 @@ SYMBOL TABLE:
 0+00 l    d  .data	0+00 .data
 0+00 l    d  .bss	0+00 .bss
 0+00 l       .text	0+00 \$d
-0+04 l       .text	0+00 \$x
+0+04 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+0c l       .text	0+00 \$d
 0+0e l       .text	0+00 \$x
 0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
diff --git a/gas/testsuite/gas/riscv/mapping-03a.d b/gas/testsuite/gas/riscv/mapping-03a.d
index d3663b6..36646bf 100644
--- a/gas/testsuite/gas/riscv/mapping-03a.d
+++ b/gas/testsuite/gas/riscv/mapping-03a.d
@@ -8,7 +8,7 @@ SYMBOL TABLE:
 0+00 l    d  .text	0+00 .text
 0+00 l    d  .data	0+00 .data
 0+00 l    d  .bss	0+00 .bss
-0+00 l       .text	0+00 \$x
+0+00 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+04 l       .text	0+00 \$d
 0+08 l       .text	0+00 \$x
 0+14 l       .text	0+00 \$d
diff --git a/gas/testsuite/gas/riscv/mapping-04a.d b/gas/testsuite/gas/riscv/mapping-04a.d
index 1ae9653..13d17bc 100644
--- a/gas/testsuite/gas/riscv/mapping-04a.d
+++ b/gas/testsuite/gas/riscv/mapping-04a.d
@@ -9,7 +9,7 @@ SYMBOL TABLE:
 0+00 l    d  .data	0+00 .data
 0+00 l    d  .bss	0+00 .bss
 0+00 l       .text	0+00 \$d
-0+0d l       .text	0+00 \$x
+0+0d l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+15 l       .text	0+00 \$d
 0+1f l       .text	0+00 \$x
 0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03a.d b/gas/testsuite/gas/riscv/mapping-norelax-03a.d
index 916f732..dd92854 100644
--- a/gas/testsuite/gas/riscv/mapping-norelax-03a.d
+++ b/gas/testsuite/gas/riscv/mapping-norelax-03a.d
@@ -8,7 +8,7 @@ SYMBOL TABLE:
 0+00 l    d  .text	0+00 .text
 0+00 l    d  .data	0+00 .data
 0+00 l    d  .bss	0+00 .bss
-0+00 l       .text	0+00 \$x
+0+00 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+04 l       .text	0+00 \$d
 0+08 l       .text	0+00 \$x
 0+10 l       .text	0+00 \$d
diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04a.d b/gas/testsuite/gas/riscv/mapping-norelax-04a.d
index d552a7f..a22da2c 100644
--- a/gas/testsuite/gas/riscv/mapping-norelax-04a.d
+++ b/gas/testsuite/gas/riscv/mapping-norelax-04a.d
@@ -12,5 +12,5 @@ SYMBOL TABLE:
 0+14 l       .text	0+00 \$d
 0+1e l       .text	0+00 \$x
 0+0d l       .text	0+00 \$d
-0+0e l       .text	0+00 \$x
+0+0e l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
diff --git a/gas/testsuite/gas/riscv/option-arch-01a.d b/gas/testsuite/gas/riscv/option-arch-01a.d
index aed4ca8..1d14c60 100644
--- a/gas/testsuite/gas/riscv/option-arch-01a.d
+++ b/gas/testsuite/gas/riscv/option-arch-01a.d
@@ -10,5 +10,5 @@ Disassembly of section .text:
 0+000 <.text>:
 [ 	]+[0-9a-f]+:[  	]+952e[    	]+add[        	]+a0,a0,a1
 [ 	]+[0-9a-f]+:[  	]+00b50533[    	]+add[        	]+a0,a0,a1
-[ 	]+[0-9a-f]+:[  	]+00302573[    	]+csrr[        	]+a0,fcsr
+[ 	]+[0-9a-f]+:[  	]+00302573[    	]+frcsr[        	]+a0
 #...
diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
index b115e33..3721131 100644
--- a/include/opcode/riscv.h
+++ b/include/opcode/riscv.h
@@ -515,6 +515,7 @@ enum riscv_seg_mstate
   MAP_NONE = 0,		/* Must be zero, for seginfo in new sections.  */
   MAP_DATA,		/* Data.  */
   MAP_INSN,		/* Instructions.  */
+  MAP_INSN_ARCH,	/* Instructions with specific extensions.  */
 };
 
 extern const char * const riscv_gpr_names_numeric[NGPR];
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 164fd20..e7eb641 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -640,6 +640,9 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
       /* If arch has ZFINX flags, use gpr for disassemble.  */
       if(riscv_subset_supports (&riscv_rps_dis, "zfinx"))
 	riscv_fpr_names = riscv_gpr_names;
+      else
+	riscv_fpr_names = riscv_gpr_names == riscv_gpr_names_abi ?
+			  riscv_fpr_names_abi : riscv_fpr_names_numeric;
 
       for (; op->name; op++)
 	{
@@ -750,7 +753,14 @@ riscv_get_map_state (int n,
     return false;
 
   name = bfd_asymbol_name(info->symtab[n]);
-  if (strcmp (name, "$x") == 0)
+  if (strncmp (name, "$xrv", 4) == 0)
+    {
+      *state = MAP_INSN_ARCH;
+
+      riscv_release_subset_list (&riscv_subsets);
+      riscv_parse_subset (&riscv_rps_dis, name + 2);
+    }
+  else if (strcmp (name, "$x") == 0)
     *state = MAP_INSN;
   else if (strcmp (name, "$d") == 0)
     *state = MAP_DATA;
-- 
2.7.4

Re: [PATCH] RISC-V: Output mapping symbols with ISA string once .option arch is used.

[Tsukasa OI]

LGTM.

Also, it's very interesting because it enables the disassembler to
process multi-arch ELF files (e.g. with optimized code paths).  I have a
plan to make the disassembler much faster, flexible and maintainable and
I think my changes can be also built on your changes.

Part of my ongoing (not submitted) works:
<https://github.com/a4lg/binutils-gdb/wiki/riscv_dis_opts_batch_1>
<https://github.com/a4lg/binutils-gdb/wiki/riscv_dis_arch_priv_spec>
<https://github.com/a4lg/binutils-gdb/wiki/riscv_dis_opts_batch_2>
(I plan to submit them after Zfinx and some disassembler changes are
merged upstream.)

Two comments follow:

On 2022/08/05 18:36, Nelson Chu wrote:
> [1] Proposal: Extend .option directive for control enabled extensions on
> specific code region,
> https://github.com/riscv-non-isa/riscv-asm-manual/pull/67
> 
> [2] Proposal: Add mapping symbol,
> https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/196
> 
> Originally, the extended .option directive [1] was used to resolve the known
> ifunc issue, but it also allows users to enable and disable extensions of a
> certain code region at will.  Once the extensions have overlapped encodings,
> it is difficult to dump the instructions correctly for now.  Therefore, the
> mapping symbols with ISA string [2] for different code regions was proposed
> to resolve the dump issue, and this patch is for that purpose.
> 
> The riscv-gnu-toolchain regressions passed for this patch.
> 
> gas/
>     * config/tc-riscv.c (updated_riscv_subsets): New boolean.  Output the
>     instruction mapping symbols with ISA string if it is true, otherwise
>     set it to false.
>     (remove_mapping_symbol): New function.  Remove the previous mapping
>     symbol by checking the current one.
>     (make_mapping_symbol): Added the current ISA string to $x when
>     updated_riscv_subsets is true.  Besides, call remove_mapping_symbol
>     rather than jusr remove the previous symbol.

Typo: just?

>     (riscv_mapping_state): Handle new mapping type MAP_INSN_ARCH, and also
>     call remove_mapping_symbol before removing it.
>     (s_riscv_option): Set updated_riscv_subsets to true once we meet .option
>     rvc/norvc/arch/pop directives.
>     * testsuite/gas/riscv/mapping-01a.d: Updated.
>     * testsuite/gas/riscv/mapping-02a.d: Likewise.
>     * testsuite/gas/riscv/mapping-03a.d: Likewise.
>     * testsuite/gas/riscv/mapping-04a.d: Likewise.
>     * testsuite/gas/riscv/mapping-norelax-03a.d: Likewise.
>     * testsuite/gas/riscv/mapping-norelax-04a.d: Likewise.
>     * testsuite/gas/riscv/option-arch-01a.d: Dump frcsr since f should be
>     enabled in that code region.
> include/
>     * opcode/riscv.h (enum riscv_seg_mstat): Added MAP_INSN_ARCH, means
>     instructions with specific extensions.
> opcodes/
>     * riscv-dis.c (riscv_disassemble_insn): Set riscv_fpr_names back to
>     riscv_fpr_names_abi or riscv_fpr_names_numeric when zfinx is disabled
>     for some specfic code region.
>     (riscv_get_map_state): Recognized mapping symbols with ISA, and then
>     reset the architecture string once the ISA is different.
> ---
>  gas/config/tc-riscv.c                         | 63 ++++++++++++++++++++++-----
>  gas/testsuite/gas/riscv/mapping-01a.d         |  2 +-
>  gas/testsuite/gas/riscv/mapping-02a.d         |  2 +-
>  gas/testsuite/gas/riscv/mapping-03a.d         |  2 +-
>  gas/testsuite/gas/riscv/mapping-04a.d         |  2 +-
>  gas/testsuite/gas/riscv/mapping-norelax-03a.d |  2 +-
>  gas/testsuite/gas/riscv/mapping-norelax-04a.d |  2 +-
>  gas/testsuite/gas/riscv/option-arch-01a.d     |  2 +-
>  include/opcode/riscv.h                        |  1 +
>  opcodes/riscv-dis.c                           | 12 ++++-
>  10 files changed, 71 insertions(+), 19 deletions(-)
> 
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 34ce68e..f61dd86 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -282,6 +282,9 @@ struct riscv_option_stack
>  
>  static struct riscv_option_stack *riscv_opts_stack = NULL;
>  
> +/* Output the instruction mapping symbols with ISA string if it is true.  */
> +static bool updated_riscv_subsets = false;
> +
>  /* Set which ISA and extensions are available.  */
>  
>  static void
> @@ -445,6 +448,22 @@ static char *expr_end;
>  #define OPCODE_MATCHES(OPCODE, OP) \
>    (((OPCODE) & MASK_##OP) == MATCH_##OP)
>  
> +/* Remove PREVIOUS mapping symbol by checking the CURRENT.  */
> +
> +static void
> +remove_mapping_symbol (symbolS *previous, symbolS *current)
> +{
> +  /* If the mapping symbol we add isn't MAP_INSN_ARCH, but
> +     the removed previous mapping symbol is MAP_INSN_ARCH,
> +     then the next MAP_INSN should be converted to MAP_INSN_ARCH.  */
> +  if (current != NULL
> +      && !strncmp (S_GET_NAME (current), "$xrv", 4) == 0
> +      && strncmp (S_GET_NAME (previous), "$xrv", 4) == 0)
> +    updated_riscv_subsets = true;
> +
> +  symbol_remove (previous, &symbol_rootP, &symbol_lastP);
> +}
> +
>  /* Create a new mapping symbol for the transition to STATE.  */
>  
>  static void
> @@ -452,14 +471,28 @@ make_mapping_symbol (enum riscv_seg_mstate state,
>  		     valueT value,
>  		     fragS *frag)
>  {
> -  const char *name;
> +  const char *name = NULL;
> +  char *buff = NULL;
> +
>    switch (state)
>      {
>      case MAP_DATA:
>        name = "$d";
>        break;
>      case MAP_INSN:
> -      name = "$x";
> +    case MAP_INSN_ARCH:
> +      if (updated_riscv_subsets)
> +	{
> +	  char *isa = riscv_arch_str (xlen, riscv_subsets);
> +	  size_t size = strlen (isa) + 3; /* "rv" + '\0'  */
> +	  buff = xmalloc (size);
> +	  snprintf (buff, size, "$x%s", isa);
> +	  xfree ((void *) isa);
> +	  name = buff;
> +	  updated_riscv_subsets = false;
> +	}
> +      else
> +	name = "$x";
>        break;
>      default:
>        abort ();
> @@ -467,19 +500,19 @@ make_mapping_symbol (enum riscv_seg_mstate state,
>  
>    symbolS *symbol = symbol_new (name, now_seg, frag, value);
>    symbol_get_bfdsym (symbol)->flags |= (BSF_NO_FLAGS | BSF_LOCAL);
> +  xfree ((void *) buff);
>  
>    /* If .fill or other data filling directive generates zero sized data,
>       or we are adding odd alignemnts, then the mapping symbol for the
>       following code will have the same value.  */
>    if (value == 0)
>      {
> -       if (frag->tc_frag_data.first_map_symbol != NULL)
> +      if (frag->tc_frag_data.first_map_symbol != NULL)
>  	{
>  	  know (S_GET_VALUE (frag->tc_frag_data.first_map_symbol)
>  		== S_GET_VALUE (symbol));
>  	  /* Remove the old one.  */
> -	  symbol_remove (frag->tc_frag_data.first_map_symbol,
> -			 &symbol_rootP, &symbol_lastP);
> +	  remove_mapping_symbol (frag->tc_frag_data.first_map_symbol, symbol);
>  	}
>        frag->tc_frag_data.first_map_symbol = symbol;
>      }
> @@ -491,8 +524,7 @@ make_mapping_symbol (enum riscv_seg_mstate state,
>        /* Remove the old one.  */
>        if (S_GET_VALUE (frag->tc_frag_data.last_map_symbol)
>  	  == S_GET_VALUE (symbol))
> -	symbol_remove (frag->tc_frag_data.last_map_symbol,
> -		       &symbol_rootP, &symbol_lastP);
> +	remove_mapping_symbol (frag->tc_frag_data.last_map_symbol, symbol);
>      }
>    frag->tc_frag_data.last_map_symbol = symbol;
>  }
> @@ -514,9 +546,14 @@ riscv_mapping_state (enum riscv_seg_mstate to_state,
>        || !subseg_text_p (now_seg))
>      return;
>  
> +  if (to_state == MAP_INSN && updated_riscv_subsets)
> +    to_state = MAP_INSN_ARCH;
> +
>    /* The mapping symbol should be emitted if not in the right
> -     mapping state  */
> -  if (from_state == to_state)
> +     mapping state.  */
> +  if ((from_state == MAP_DATA && to_state == MAP_DATA)
> +      || (from_state == MAP_INSN && to_state == MAP_INSN)
> +      || (from_state == MAP_INSN_ARCH && to_state == MAP_INSN))
>      return;
>  
>    valueT value = (valueT) (frag_now_fix () - max_chars);
> @@ -573,7 +610,7 @@ riscv_check_mapping_symbols (bfd *abfd ATTRIBUTE_UNUSED,
>  	    {
>  	      /* The last mapping symbol overlaps with another one
>  		 which at the start of the next frag.  */
> -	      symbol_remove (last, &symbol_rootP, &symbol_lastP);
> +	      remove_mapping_symbol (last, NULL);
>  	      break;
>  	    }
>  
> @@ -581,7 +618,7 @@ riscv_check_mapping_symbols (bfd *abfd ATTRIBUTE_UNUSED,
>  	    {
>  	      /* The last mapping symbol is at the end of the section.  */
>  	      know (next->fr_fix == 0 && next->fr_var == 0);
> -	      symbol_remove (last, &symbol_rootP, &symbol_lastP);
> +	      remove_mapping_symbol (last, NULL);
>  	      break;
>  	    }
>  
> @@ -3855,11 +3892,13 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>    if (strcmp (name, "rvc") == 0)
>      {
>        riscv_update_subset (&riscv_rps_as, "+c");
> +      updated_riscv_subsets = true;
>        riscv_set_rvc (true);
>      }
>    else if (strcmp (name, "norvc") == 0)
>      {
>        riscv_update_subset (&riscv_rps_as, "-c");
> +      updated_riscv_subsets = true;
>        riscv_set_rvc (false);
>      }
>    else if (strcmp (name, "pic") == 0)
> @@ -3880,6 +3919,7 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>        if (ISSPACE (*name) && *name != '\0')
>  	name++;
>        riscv_update_subset (&riscv_rps_as, name);
> +      updated_riscv_subsets = true;
>  
>        riscv_set_rvc (false);
>        if (riscv_subset_supports (&riscv_rps_as, "c"))
> @@ -3911,6 +3951,7 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>  	  riscv_opts = s->options;
>  	  riscv_subsets = s->subset_list;
>  	  riscv_rps_as.subset_list = riscv_subsets;
> +	  updated_riscv_subsets = true;
>  	  riscv_release_subset_list (release_subsets);
>  	  free (s);
>  	}
> diff --git a/gas/testsuite/gas/riscv/mapping-01a.d b/gas/testsuite/gas/riscv/mapping-01a.d
> index 32e0027..2aefe5a 100644
> --- a/gas/testsuite/gas/riscv/mapping-01a.d
> +++ b/gas/testsuite/gas/riscv/mapping-01a.d
> @@ -8,7 +8,7 @@ SYMBOL TABLE:
>  0+00 l    d  .text	0+00 .text
>  0+00 l    d  .data	0+00 .data
>  0+00 l    d  .bss	0+00 .bss
> -0+00 l       .text	0+00 \$x
> +0+00 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
>  0+00 l    d  .foo	0+00 .foo
>  0+00 l       .foo	0+00 foo
>  0+00 l       .foo	0+00 \$x
> diff --git a/gas/testsuite/gas/riscv/mapping-02a.d b/gas/testsuite/gas/riscv/mapping-02a.d
> index 333f12c..155ae02 100644
> --- a/gas/testsuite/gas/riscv/mapping-02a.d
> +++ b/gas/testsuite/gas/riscv/mapping-02a.d
> @@ -9,7 +9,7 @@ SYMBOL TABLE:
>  0+00 l    d  .data	0+00 .data
>  0+00 l    d  .bss	0+00 .bss
>  0+00 l       .text	0+00 \$d
> -0+04 l       .text	0+00 \$x
> +0+04 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
>  0+0c l       .text	0+00 \$d
>  0+0e l       .text	0+00 \$x
>  0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
> diff --git a/gas/testsuite/gas/riscv/mapping-03a.d b/gas/testsuite/gas/riscv/mapping-03a.d
> index d3663b6..36646bf 100644
> --- a/gas/testsuite/gas/riscv/mapping-03a.d
> +++ b/gas/testsuite/gas/riscv/mapping-03a.d
> @@ -8,7 +8,7 @@ SYMBOL TABLE:
>  0+00 l    d  .text	0+00 .text
>  0+00 l    d  .data	0+00 .data
>  0+00 l    d  .bss	0+00 .bss
> -0+00 l       .text	0+00 \$x
> +0+00 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
>  0+04 l       .text	0+00 \$d
>  0+08 l       .text	0+00 \$x
>  0+14 l       .text	0+00 \$d
> diff --git a/gas/testsuite/gas/riscv/mapping-04a.d b/gas/testsuite/gas/riscv/mapping-04a.d
> index 1ae9653..13d17bc 100644
> --- a/gas/testsuite/gas/riscv/mapping-04a.d
> +++ b/gas/testsuite/gas/riscv/mapping-04a.d
> @@ -9,7 +9,7 @@ SYMBOL TABLE:
>  0+00 l    d  .data	0+00 .data
>  0+00 l    d  .bss	0+00 .bss
>  0+00 l       .text	0+00 \$d
> -0+0d l       .text	0+00 \$x
> +0+0d l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
>  0+15 l       .text	0+00 \$d
>  0+1f l       .text	0+00 \$x
>  0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
> diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03a.d b/gas/testsuite/gas/riscv/mapping-norelax-03a.d
> index 916f732..dd92854 100644
> --- a/gas/testsuite/gas/riscv/mapping-norelax-03a.d
> +++ b/gas/testsuite/gas/riscv/mapping-norelax-03a.d
> @@ -8,7 +8,7 @@ SYMBOL TABLE:
>  0+00 l    d  .text	0+00 .text
>  0+00 l    d  .data	0+00 .data
>  0+00 l    d  .bss	0+00 .bss
> -0+00 l       .text	0+00 \$x
> +0+00 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
>  0+04 l       .text	0+00 \$d
>  0+08 l       .text	0+00 \$x
>  0+10 l       .text	0+00 \$d
> diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04a.d b/gas/testsuite/gas/riscv/mapping-norelax-04a.d
> index d552a7f..a22da2c 100644
> --- a/gas/testsuite/gas/riscv/mapping-norelax-04a.d
> +++ b/gas/testsuite/gas/riscv/mapping-norelax-04a.d
> @@ -12,5 +12,5 @@ SYMBOL TABLE:
>  0+14 l       .text	0+00 \$d
>  0+1e l       .text	0+00 \$x
>  0+0d l       .text	0+00 \$d
> -0+0e l       .text	0+00 \$x
> +0+0e l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
>  0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
> diff --git a/gas/testsuite/gas/riscv/option-arch-01a.d b/gas/testsuite/gas/riscv/option-arch-01a.d
> index aed4ca8..1d14c60 100644
> --- a/gas/testsuite/gas/riscv/option-arch-01a.d
> +++ b/gas/testsuite/gas/riscv/option-arch-01a.d
> @@ -10,5 +10,5 @@ Disassembly of section .text:
>  0+000 <.text>:
>  [ 	]+[0-9a-f]+:[  	]+952e[    	]+add[        	]+a0,a0,a1
>  [ 	]+[0-9a-f]+:[  	]+00b50533[    	]+add[        	]+a0,a0,a1
> -[ 	]+[0-9a-f]+:[  	]+00302573[    	]+csrr[        	]+a0,fcsr
> +[ 	]+[0-9a-f]+:[  	]+00302573[    	]+frcsr[        	]+a0
>  #...
> diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> index b115e33..3721131 100644
> --- a/include/opcode/riscv.h
> +++ b/include/opcode/riscv.h
> @@ -515,6 +515,7 @@ enum riscv_seg_mstate
>    MAP_NONE = 0,		/* Must be zero, for seginfo in new sections.  */
>    MAP_DATA,		/* Data.  */
>    MAP_INSN,		/* Instructions.  */
> +  MAP_INSN_ARCH,	/* Instructions with specific extensions.  */
>  };
>  
>  extern const char * const riscv_gpr_names_numeric[NGPR];
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 164fd20..e7eb641 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -640,6 +640,9 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
>        /* If arch has ZFINX flags, use gpr for disassemble.  */
>        if(riscv_subset_supports (&riscv_rps_dis, "zfinx"))
>  	riscv_fpr_names = riscv_gpr_names;
> +      else
> +	riscv_fpr_names = riscv_gpr_names == riscv_gpr_names_abi ?
> +			  riscv_fpr_names_abi : riscv_fpr_names_numeric;
>  
>        for (; op->name; op++)
>  	{
> @@ -750,7 +753,14 @@ riscv_get_map_state (int n,
>      return false;
>  
>    name = bfd_asymbol_name(info->symtab[n]);
> -  if (strcmp (name, "$x") == 0)
> +  if (strncmp (name, "$xrv", 4) == 0)
> +    {
> +      *state = MAP_INSN_ARCH;
> +
> +      riscv_release_subset_list (&riscv_subsets);
> +      riscv_parse_subset (&riscv_rps_dis, name + 2);
> +    }
> +  else if (strcmp (name, "$x") == 0)
>      *state = MAP_INSN;

It seems, if we meet "$xrv...." and then "$x" (with no ISA), it does not
change the ISA (specified by "$xrv...") but it's against the current
proposal.

> an instruction mapping symobl
> without ISA string means using ISA configuration from ELF attribute.

I think we need to save the ISA string from the ELF attributes.
In my upcoming changes, it does save the ISA string from the ELF
attributes ("RISC-V: Minimize arch-related initialization") for a
different reason:

https://github.com/a4lg/binutils-gdb/pull/28/commits/3d553885ab46570c5fc4c810c6dc6c90b9bf3fc4


>    else if (strcmp (name, "$d") == 0)
>      *state = MAP_DATA;

Thanks,
Tsukasa

[RFC PATCH 0/5] RISC-V: Support mapping symbols with ISA string

[Tsukasa OI]

Hello,

After a detailed analysis of Nelson's patch, I made my own version based
on it.  It clearly lacks proper attribution (Signed-Off-By lines) and
should be considered as RFC.  Nelson, I would like to hear your thoughts.

Nelson's original patch:
<https://sourceware.org/pipermail/binutils/2022-August/122220.html>
Tracker on GitHub:
<https://github.com/a4lg/binutils-gdb/wiki/riscv_mapping_sym_with_arch>

    Also see:

    [1] Proposal: Extend .option directive for control enabled extensions
    on specific code region,
    <https://github.com/riscv-non-isa/riscv-asm-manual/pull/67>

    [2] Proposal: Add mapping symbol,
    <https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/196>



I found two issues on his patch:

(1) "$x" is the start of instructions with the default architecture.
Nelson's patch effectively handled it as a change to code but with no ISA
changes and this is clearly against Kito's proposal:

<https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/196/files#diff-361ae9d87b1e297413d1551ec0743e2e893217198eac78e95568a3718be26fb1R805>

> an instruction mapping symobl
> without ISA string means using ISA configuration from ELF attribute.

In my version, "$x" is emitted if no architecture string is changed by
.option directives and such.  Even if the arch is changed to revert the
original, it's not the same:

    # Example
    # (we have default architecture here such as RV32I)
    .option arch, +c
    .option arch, -c
    # (not default, even if the resulting ISA string is the same)

The only exception is when we use ".option push" before the architecture is
changed and that state is restored by ".option pop".

    # Example
    # (we have default architecture here)
    .option push
    # (now we can make changes to the architecture)
    .option arch, +c
    # (... then restore it)
    .option pop
    # (now we have the default architecture, again)

This also applies to the disassembler.  If we meet the "$x" mapping symbol,
we need to revert to the default architecture.  Since we need to save the
default architecture, the disassembler part is a bit more complex than
Nelson's implementation.


(2) Also, the mapping symbols are handled per section but "current"
architecture as managed by the assembler is not.  It did work previously
because we didn't have to keep track of the architecture (ISA string) but
we have to explicitly handle that now.

So, the first instruction after the section change requires emitting the
mapping symbol if necessary.  Still, to do that, looking up the last
mapping symbol is sufficient (saving RISC-V preset list is not required).


My changes made MAP_INSN_ARCH not necessary so I removed it.


I also added four test cases (mapping-{05..08}) whether we are handling
complex situations correctly.  Note that mapping-{05,06,08} will fail on
Nelson's implementation.



p.s.

Nelson, are you going to upstream this feature now or wait until the
proposal by Kito is approved?  If your answer is the latter, I will submit
my disassembler changes first (to make the disassembler support of this
feature cleaner).



Thanks,
Tsukasa




Tsukasa OI (5):
  RISC-V: Use bool on riscv_set_options members
  gas: Copyediting on tc-riscv.c
  RISC-V: Mapping symbols with ISA string on assembler
  RISC-V: Mapping symbols with ISA string on disassembler
  RISC-V: New testcases for mapping symbols w/ ISA

 gas/config/tc-riscv.c                         | 67 +++++++++++++++----
 gas/config/tc-riscv.h                         |  5 ++
 gas/testsuite/gas/riscv/mapping-01a.d         |  4 +-
 gas/testsuite/gas/riscv/mapping-02a.d         |  4 +-
 gas/testsuite/gas/riscv/mapping-03a.d         | 10 +--
 gas/testsuite/gas/riscv/mapping-04a.d         |  4 +-
 gas/testsuite/gas/riscv/mapping-05.s          | 11 +++
 gas/testsuite/gas/riscv/mapping-05a.d         | 14 ++++
 gas/testsuite/gas/riscv/mapping-05b.d         | 14 ++++
 gas/testsuite/gas/riscv/mapping-06.s          | 11 +++
 gas/testsuite/gas/riscv/mapping-06a.d         | 14 ++++
 gas/testsuite/gas/riscv/mapping-06b.d         | 14 ++++
 gas/testsuite/gas/riscv/mapping-07.s          | 12 ++++
 gas/testsuite/gas/riscv/mapping-07a.d         | 14 ++++
 gas/testsuite/gas/riscv/mapping-07b.d         | 21 ++++++
 gas/testsuite/gas/riscv/mapping-08.s          | 34 ++++++++++
 gas/testsuite/gas/riscv/mapping-08a.d         | 18 +++++
 gas/testsuite/gas/riscv/mapping-08b.d         | 23 +++++++
 gas/testsuite/gas/riscv/mapping-norelax-03a.d | 10 +--
 gas/testsuite/gas/riscv/mapping-norelax-04a.d |  4 +-
 gas/testsuite/gas/riscv/option-arch-01a.d     |  2 +-
 opcodes/riscv-dis.c                           | 44 ++++++++++--
 22 files changed, 317 insertions(+), 37 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/mapping-05.s
 create mode 100644 gas/testsuite/gas/riscv/mapping-05a.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-05b.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-06.s
 create mode 100644 gas/testsuite/gas/riscv/mapping-06a.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-06b.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-07.s
 create mode 100644 gas/testsuite/gas/riscv/mapping-07a.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-07b.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-08.s
 create mode 100644 gas/testsuite/gas/riscv/mapping-08a.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-08b.d


base-commit: 453595283c323e106a60b229999756b45ae6b2d8
-- 
2.34.1

[RFC PATCH 1/5] RISC-V: Use bool on riscv_set_options members

[Tsukasa OI]

All uses of riscv_set_options members are safe to be bool.  If we change
them from `int' to `bool', we can shrink the `riscv_set_options' and the
intent of the members are clearer.

gas/ChangeLog:

	* config/tc-riscv.c (struct riscv_set_options): Make all members
	from `int' to `bool'.
---
 gas/config/tc-riscv.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 34ce68e8252..92eabf0cbc7 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -229,20 +229,20 @@ riscv_set_default_priv_spec (const char *s)
 /* This is the set of options which the .option pseudo-op may modify.  */
 struct riscv_set_options
 {
-  int pic; /* Generate position-independent code.  */
-  int rvc; /* Generate RVC code.  */
-  int relax; /* Emit relocs the linker is allowed to relax.  */
-  int arch_attr; /* Emit architecture and privileged elf attributes.  */
-  int csr_check; /* Enable the CSR checking.  */
+  bool pic; /* Generate position-independent code.  */
+  bool rvc; /* Generate RVC code.  */
+  bool relax; /* Emit relocs the linker is allowed to relax.  */
+  bool arch_attr; /* Emit architecture and privileged elf attributes.  */
+  bool csr_check; /* Enable the CSR checking.  */
 };
 
 static struct riscv_set_options riscv_opts =
 {
-  0, /* pic */
-  0, /* rvc */
-  1, /* relax */
-  DEFAULT_RISCV_ATTR, /* arch_attr */
-  0, /* csr_check */
+  false, /* pic */
+  false, /* rvc */
+  true,  /* relax */
+  (bool) DEFAULT_RISCV_ATTR, /* arch_attr */
+  false, /* csr_check */
 };
 
 /* Enable or disable the rvc flags for riscv_opts.  Turn on the rvc flag
-- 
2.34.1

[RFC PATCH 2/5] gas: Copyediting on tc-riscv.c

[Tsukasa OI]

Before applying "RISC-V: Mapping symbols with ISA string on assembler", this
commit takes care of some non-functional issues.

gas/ChangeLog:

	* config/tc-riscv.c (make_mapping_symbol): Make indentation
	consistent.  (riscv_mapping_state): Add "period" to a comment.
---
 gas/config/tc-riscv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 92eabf0cbc7..a09403fbd11 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -473,7 +473,7 @@ make_mapping_symbol (enum riscv_seg_mstate state,
      following code will have the same value.  */
   if (value == 0)
     {
-       if (frag->tc_frag_data.first_map_symbol != NULL)
+      if (frag->tc_frag_data.first_map_symbol != NULL)
 	{
 	  know (S_GET_VALUE (frag->tc_frag_data.first_map_symbol)
 		== S_GET_VALUE (symbol));
@@ -515,7 +515,7 @@ riscv_mapping_state (enum riscv_seg_mstate to_state,
     return;
 
   /* The mapping symbol should be emitted if not in the right
-     mapping state  */
+     mapping state.  */
   if (from_state == to_state)
     return;
 
-- 
2.34.1

[RFC PATCH 3/5] RISC-V: Mapping symbols with ISA string on assembler

[Tsukasa OI]

The mapping symbols with ISA string is proposed to deal with so called
"ifunc issue".  It enables disassembling a certain range of the code with
a different architecture than the rest, even if conflicting.  This is useful
when there's "optimized" implementation is available but dynamically
switched only if a certain extension is available.

This commit implements the assembler support to emit mapping symbols with
ISA string (and partial disassembler support only to pass tests).

[1] Proposal: Extend .option directive for control enabled extensions on
specific code region,
https://github.com/riscv-non-isa/riscv-asm-manual/pull/67

[2] Proposal: Add mapping symbol,
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/196

This commit is based on Nelson Chu's proposal "RISC-V: Output mapping
symbols with ISA string once .option arch is used." but heavily modified to
reflect the intent of Kito's original proposal.  It is also made smarter so
that it no longer requires MAP_INSN_ARCH.

gas/ChangeLog:

	* config/tc-riscv.c (struct riscv_set_options): Add new field
	`arch_is_default' to keep track whether the architecture is still
	holds its default.
	(updated_riscv_subsets) New variable to keep track of whether the
	architecture is possibly changed and inspected to emit proper
	mapping symbols.
	(make_mapping_symbol): Make mapping symbols with ISA string if
	necessary.  Don't emit the mapping symbol if the previous one in the
	same section has the same name.
	(riscv_elf_section_change_hook): New.  Try to emit a new mapping
	symbol if the section is changed.
	(riscv_mapping_state): Don't skip if the architecture is possibly
	changed and the new state is "code".
	(s_riscv_option): Keep track of `updated_riscv_subsets' and
	`riscv_opts.arch_is_default'.
	* config/tc-riscv.h (md_elf_section_change_hook): Define as
	`riscv_elf_section_change_hook'.
	(riscv_elf_section_change_hook): Declare.
	* testsuite/gas/riscv/mapping-01a.d: Reflect mapping symbols with
	ISA string.
	* testsuite/gas/riscv/mapping-02a.d: Likewise.
	* testsuite/gas/riscv/mapping-03a.d: Likewise.
	* testsuite/gas/riscv/mapping-04a.d: Likewise.
	* testsuite/gas/riscv/mapping-norelax-03a.d: Likewise.
	* testsuite/gas/riscv/mapping-norelax-04a.d: Likewise.

opcodes/ChangeLog:

	* riscv-dis.c (riscv_get_map_state): Minimum support of mapping
	symbols with ISA string without actually parsing the ISA string.
	The only purpose of this change is to pass the tests.
---
 gas/config/tc-riscv.c                         | 43 ++++++++++++++++++-
 gas/config/tc-riscv.h                         |  5 +++
 gas/testsuite/gas/riscv/mapping-01a.d         |  4 +-
 gas/testsuite/gas/riscv/mapping-02a.d         |  4 +-
 gas/testsuite/gas/riscv/mapping-03a.d         | 10 ++---
 gas/testsuite/gas/riscv/mapping-04a.d         |  4 +-
 gas/testsuite/gas/riscv/mapping-norelax-03a.d | 10 ++---
 gas/testsuite/gas/riscv/mapping-norelax-04a.d |  4 +-
 opcodes/riscv-dis.c                           |  2 +-
 9 files changed, 65 insertions(+), 21 deletions(-)

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index a09403fbd11..416ec99c8e4 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -234,6 +234,7 @@ struct riscv_set_options
   bool relax; /* Emit relocs the linker is allowed to relax.  */
   bool arch_attr; /* Emit architecture and privileged elf attributes.  */
   bool csr_check; /* Enable the CSR checking.  */
+  bool arch_is_default; /* Architecture string is not modified.  */
 };
 
 static struct riscv_set_options riscv_opts =
@@ -243,6 +244,7 @@ static struct riscv_set_options riscv_opts =
   true,  /* relax */
   (bool) DEFAULT_RISCV_ATTR, /* arch_attr */
   false, /* csr_check */
+  true,  /* arch_is_default */
 };
 
 /* Enable or disable the rvc flags for riscv_opts.  Turn on the rvc flag
@@ -282,6 +284,9 @@ struct riscv_option_stack
 
 static struct riscv_option_stack *riscv_opts_stack = NULL;
 
+/* Output the instruction mapping symbols with ISA string if it is true.  */
+static bool updated_riscv_subsets = false;
+
 /* Set which ISA and extensions are available.  */
 
 static void
@@ -453,6 +458,7 @@ make_mapping_symbol (enum riscv_seg_mstate state,
 		     fragS *frag)
 {
   const char *name;
+  char *buff = NULL;
   switch (state)
     {
     case MAP_DATA:
@@ -460,6 +466,15 @@ make_mapping_symbol (enum riscv_seg_mstate state,
       break;
     case MAP_INSN:
       name = "$x";
+      if (!riscv_opts.arch_is_default)
+	{
+	  char *isa = riscv_arch_str (xlen, riscv_subsets);
+	  size_t size = strlen (isa) + 3; /* "$x" + '\0'  */
+	  name = buff = xmalloc (size);
+	  snprintf (buff, size, "$x%s", isa);
+	  xfree ((void *) isa);
+	}
+      updated_riscv_subsets = false;
       break;
     default:
       abort ();
@@ -467,6 +482,7 @@ make_mapping_symbol (enum riscv_seg_mstate state,
 
   symbolS *symbol = symbol_new (name, now_seg, frag, value);
   symbol_get_bfdsym (symbol)->flags |= (BSF_NO_FLAGS | BSF_LOCAL);
+  xfree ((void *) buff);
 
   /* If .fill or other data filling directive generates zero sized data,
      or we are adding odd alignemnts, then the mapping symbol for the
@@ -488,7 +504,14 @@ make_mapping_symbol (enum riscv_seg_mstate state,
       /* The mapping symbols should be added in offset order.  */
       know (S_GET_VALUE (frag->tc_frag_data.last_map_symbol)
 			 <= S_GET_VALUE (symbol));
-      /* Remove the old one.  */
+      /* Remove the new one if the name matches.  */
+      if (strcmp (S_GET_NAME (frag->tc_frag_data.last_map_symbol),
+		  S_GET_NAME (symbol)) == 0)
+	{
+	  symbol_remove (symbol, &symbol_rootP, &symbol_lastP);
+	  return;
+	}
+      /* Remove the old one if the value matches.  */
       if (S_GET_VALUE (frag->tc_frag_data.last_map_symbol)
 	  == S_GET_VALUE (symbol))
 	symbol_remove (frag->tc_frag_data.last_map_symbol,
@@ -497,6 +520,14 @@ make_mapping_symbol (enum riscv_seg_mstate state,
   frag->tc_frag_data.last_map_symbol = symbol;
 }
 
+/* Called when the section is change.  Try to emit a new mapping symbol.  */
+
+void
+riscv_elf_section_change_hook (void)
+{
+  updated_riscv_subsets = true;
+}
+
 /* Set the mapping state for frag_now.  */
 
 void
@@ -516,7 +547,8 @@ riscv_mapping_state (enum riscv_seg_mstate to_state,
 
   /* The mapping symbol should be emitted if not in the right
      mapping state.  */
-  if (from_state == to_state)
+  if (from_state == to_state
+      && (to_state != MAP_INSN || !updated_riscv_subsets))
     return;
 
   valueT value = (valueT) (frag_now_fix () - max_chars);
@@ -3855,11 +3887,15 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
   if (strcmp (name, "rvc") == 0)
     {
       riscv_update_subset (&riscv_rps_as, "+c");
+      updated_riscv_subsets = true;
+      riscv_opts.arch_is_default = false;
       riscv_set_rvc (true);
     }
   else if (strcmp (name, "norvc") == 0)
     {
       riscv_update_subset (&riscv_rps_as, "-c");
+      updated_riscv_subsets = true;
+      riscv_opts.arch_is_default = false;
       riscv_set_rvc (false);
     }
   else if (strcmp (name, "pic") == 0)
@@ -3880,6 +3916,8 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
       if (ISSPACE (*name) && *name != '\0')
 	name++;
       riscv_update_subset (&riscv_rps_as, name);
+      updated_riscv_subsets = true;
+      riscv_opts.arch_is_default = false;
 
       riscv_set_rvc (false);
       if (riscv_subset_supports (&riscv_rps_as, "c"))
@@ -3911,6 +3949,7 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
 	  riscv_opts = s->options;
 	  riscv_subsets = s->subset_list;
 	  riscv_rps_as.subset_list = riscv_subsets;
+	  updated_riscv_subsets = true;
 	  riscv_release_subset_list (release_subsets);
 	  free (s);
 	}
diff --git a/gas/config/tc-riscv.h b/gas/config/tc-riscv.h
index 802e7afe670..f46308bddb2 100644
--- a/gas/config/tc-riscv.h
+++ b/gas/config/tc-riscv.h
@@ -140,6 +140,11 @@ struct riscv_segment_info_type
   enum riscv_seg_mstate map_state;
 };
 
+#ifdef OBJ_ELF
+#define md_elf_section_change_hook() riscv_elf_section_change_hook ()
+extern void riscv_elf_section_change_hook (void);
+#endif
+
 /* Define target fragment type.  */
 #define TC_FRAG_TYPE struct riscv_frag_type
 struct riscv_frag_type
diff --git a/gas/testsuite/gas/riscv/mapping-01a.d b/gas/testsuite/gas/riscv/mapping-01a.d
index 32e0027a13d..386127c7855 100644
--- a/gas/testsuite/gas/riscv/mapping-01a.d
+++ b/gas/testsuite/gas/riscv/mapping-01a.d
@@ -8,10 +8,10 @@ SYMBOL TABLE:
 0+00 l    d  .text	0+00 .text
 0+00 l    d  .data	0+00 .data
 0+00 l    d  .bss	0+00 .bss
-0+00 l       .text	0+00 \$x
+0+00 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+00 l    d  .foo	0+00 .foo
 0+00 l       .foo	0+00 foo
-0+00 l       .foo	0+00 \$x
+0+00 l       .foo	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
 0+00 g       .text	0+00 funcA
 0+08 g       .text	0+00 funcB
diff --git a/gas/testsuite/gas/riscv/mapping-02a.d b/gas/testsuite/gas/riscv/mapping-02a.d
index 333f12cd343..d59f7805645 100644
--- a/gas/testsuite/gas/riscv/mapping-02a.d
+++ b/gas/testsuite/gas/riscv/mapping-02a.d
@@ -9,7 +9,7 @@ SYMBOL TABLE:
 0+00 l    d  .data	0+00 .data
 0+00 l    d  .bss	0+00 .bss
 0+00 l       .text	0+00 \$d
-0+04 l       .text	0+00 \$x
+0+04 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+0c l       .text	0+00 \$d
-0+0e l       .text	0+00 \$x
+0+0e l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
diff --git a/gas/testsuite/gas/riscv/mapping-03a.d b/gas/testsuite/gas/riscv/mapping-03a.d
index d3663b663aa..d2cd9376800 100644
--- a/gas/testsuite/gas/riscv/mapping-03a.d
+++ b/gas/testsuite/gas/riscv/mapping-03a.d
@@ -8,13 +8,13 @@ SYMBOL TABLE:
 0+00 l    d  .text	0+00 .text
 0+00 l    d  .data	0+00 .data
 0+00 l    d  .bss	0+00 .bss
-0+00 l       .text	0+00 \$x
+0+00 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+04 l       .text	0+00 \$d
-0+08 l       .text	0+00 \$x
+0+08 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+14 l       .text	0+00 \$d
-0+18 l       .text	0+00 \$x
+0+18 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+1c l       .text	0+00 \$d
-0+21 l       .text	0+00 \$x
+0+21 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+2d l       .text	0+00 \$d
-0+31 l       .text	0+00 \$x
+0+31 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
diff --git a/gas/testsuite/gas/riscv/mapping-04a.d b/gas/testsuite/gas/riscv/mapping-04a.d
index 1ae9653212b..817bace9690 100644
--- a/gas/testsuite/gas/riscv/mapping-04a.d
+++ b/gas/testsuite/gas/riscv/mapping-04a.d
@@ -9,7 +9,7 @@ SYMBOL TABLE:
 0+00 l    d  .data	0+00 .data
 0+00 l    d  .bss	0+00 .bss
 0+00 l       .text	0+00 \$d
-0+0d l       .text	0+00 \$x
+0+0d l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+15 l       .text	0+00 \$d
-0+1f l       .text	0+00 \$x
+0+1f l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03a.d b/gas/testsuite/gas/riscv/mapping-norelax-03a.d
index 916f732b7f7..2dc2f399c7b 100644
--- a/gas/testsuite/gas/riscv/mapping-norelax-03a.d
+++ b/gas/testsuite/gas/riscv/mapping-norelax-03a.d
@@ -8,14 +8,14 @@ SYMBOL TABLE:
 0+00 l    d  .text	0+00 .text
 0+00 l    d  .data	0+00 .data
 0+00 l    d  .bss	0+00 .bss
-0+00 l       .text	0+00 \$x
+0+00 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+04 l       .text	0+00 \$d
-0+08 l       .text	0+00 \$x
+0+08 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+10 l       .text	0+00 \$d
-0+14 l       .text	0+00 \$x
+0+14 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+18 l       .text	0+00 \$d
 0+20 l       .text	0+00 \$d
-0+24 l       .text	0+00 \$x
+0+24 l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+1d l       .text	0+00 \$d
-0+1e l       .text	0+00 \$x
+0+1e l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04a.d b/gas/testsuite/gas/riscv/mapping-norelax-04a.d
index d552a7f632a..9050df5a11a 100644
--- a/gas/testsuite/gas/riscv/mapping-norelax-04a.d
+++ b/gas/testsuite/gas/riscv/mapping-norelax-04a.d
@@ -10,7 +10,7 @@ SYMBOL TABLE:
 0+00 l    d  .bss	0+00 .bss
 0+00 l       .text	0+00 \$d
 0+14 l       .text	0+00 \$d
-0+1e l       .text	0+00 \$x
+0+1e l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+0d l       .text	0+00 \$d
-0+0e l       .text	0+00 \$x
+0+0e l       .text	0+00 \$xrv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0
 0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 164fd209dbd..4b7b17235ee 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -750,7 +750,7 @@ riscv_get_map_state (int n,
     return false;
 
   name = bfd_asymbol_name(info->symtab[n]);
-  if (strcmp (name, "$x") == 0)
+  if (strncmp (name, "$x", 2) == 0)
     *state = MAP_INSN;
   else if (strcmp (name, "$d") == 0)
     *state = MAP_DATA;
-- 
2.34.1

[RFC PATCH 4/5] RISC-V: Mapping symbols with ISA string on disassembler

[Tsukasa OI]

The mapping symbols with ISA string is proposed to deal with so called
"ifunc issue".  It enables disassembling a certain range of the code with
a different architecture than the rest, even if conflicting.  This is useful
when there's "optimized" implementation is available but dynamically
switched only if a certain extension is available.

This commit implements the disassembler support to parse mapping symbols
with ISA string.

[1] Proposal: Extend .option directive for control enabled extensions on
specific code region,
https://github.com/riscv-non-isa/riscv-asm-manual/pull/67

[2] Proposal: Add mapping symbol,
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/196

This commit is based on Nelson Chu's proposal "RISC-V: Output mapping
symbols with ISA string once .option arch is used." but heavily modified to
reflect the intent of Kito's original proposal.  It is also made smarter so
that it no longer requires MAP_INSN_ARCH.

gas/ChangeLog:

	* testsuite/gas/riscv/option-arch-01a.d: Reflect the disassembler
	support of mapping symbols with ISA string.

opcodes/ChangeLog:

	* riscv-dis.c (initial_default_arch) Default architecture string if
	no ELF attributes are available.
	(default_arch): A copy of the default architecture string.
	(is_arch_mapping): New variable to keep track of whether the current
	architecture is deviced from a mapping symbol.
	(riscv_disassemble_insn): Update FPR names when a mapping symbol
	with ISA string is encountered.
	(riscv_get_map_state): Support mapping symbols with ISA string.
	Use `is_arch_mapping' to stop repeatedly parsing the default
	architecture.
	(riscv_get_disassembler): Safer architecture string handling.
	Copy the string to switch to the default while disassembling.
---
 gas/testsuite/gas/riscv/option-arch-01a.d |  2 +-
 opcodes/riscv-dis.c                       | 44 ++++++++++++++++++++---
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/gas/testsuite/gas/riscv/option-arch-01a.d b/gas/testsuite/gas/riscv/option-arch-01a.d
index aed4ca8e4d9..b9c260c9949 100644
--- a/gas/testsuite/gas/riscv/option-arch-01a.d
+++ b/gas/testsuite/gas/riscv/option-arch-01a.d
@@ -10,5 +10,5 @@ Disassembly of section .text:
 0+000 <.text>:
 [ 	]+[0-9a-f]+:[  	]+952e[    	]+add[        	]+a0,a0,a1
 [ 	]+[0-9a-f]+:[  	]+00b50533[    	]+add[        	]+a0,a0,a1
-[ 	]+[0-9a-f]+:[  	]+00302573[    	]+csrr[        	]+a0,fcsr
+[ 	]+[0-9a-f]+:[  	]+00302573[    	]+frcsr[       	]+a0
 #...
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 4b7b17235ee..c029c65827c 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -32,6 +32,16 @@
 #include <stdint.h>
 #include <ctype.h>
 
+/* Default architecture string (if not available).  */
+static const char *initial_default_arch = "rv64gc";
+
+/* Default architecture string
+   (as specified by the ELF attributes or `initial_default_arch').  */
+static const char *default_arch = NULL;
+
+/* True if the architecture is set from a mapping symbol.  */
+static bool is_arch_mapping = false;
+
 static enum riscv_spec_class default_isa_spec = ISA_SPEC_CLASS_DRAFT - 1;
 static enum riscv_spec_class default_priv_spec = PRIV_SPEC_CLASS_NONE;
 
@@ -638,8 +648,12 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
 	}
 
       /* If arch has ZFINX flags, use gpr for disassemble.  */
-      if(riscv_subset_supports (&riscv_rps_dis, "zfinx"))
+      if (riscv_subset_supports (&riscv_rps_dis, "zfinx"))
 	riscv_fpr_names = riscv_gpr_names;
+      else
+	riscv_fpr_names = riscv_gpr_names == riscv_gpr_names_abi
+			      ? riscv_fpr_names_abi
+			      : riscv_fpr_names_numeric;
 
       for (; op->name; op++)
 	{
@@ -750,8 +764,23 @@ riscv_get_map_state (int n,
     return false;
 
   name = bfd_asymbol_name(info->symtab[n]);
-  if (strncmp (name, "$x", 2) == 0)
+  if (strncmp (name, "$xrv", 4) == 0)
+  {
     *state = MAP_INSN;
+    riscv_release_subset_list (&riscv_subsets);
+    riscv_parse_subset (&riscv_rps_dis, name + 2);
+    is_arch_mapping = true;
+  }
+  else if (strcmp (name, "$x") == 0)
+  {
+    *state = MAP_INSN;
+    if (is_arch_mapping)
+    {
+      riscv_release_subset_list (&riscv_subsets);
+      riscv_parse_subset (&riscv_rps_dis, default_arch);
+      is_arch_mapping = false;
+    }
+  }
   else if (strcmp (name, "$d") == 0)
     *state = MAP_DATA;
   else
@@ -1000,7 +1029,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
 disassembler_ftype
 riscv_get_disassembler (bfd *abfd)
 {
-  const char *default_arch = "rv64gc";
+  const char *default_arch_next = initial_default_arch;
 
   if (abfd && bfd_get_flavour (abfd) == bfd_target_elf_flavour)
     {
@@ -1015,12 +1044,19 @@ riscv_get_disassembler (bfd *abfd)
 						  attr[Tag_b].i,
 						  attr[Tag_c].i,
 						  &default_priv_spec);
-	  default_arch = attr[Tag_RISCV_arch].s;
+	  default_arch_next = attr[Tag_RISCV_arch].s;
+	  /* For ELF files with (somehow) no architecture string
+	     in the attributes, use the default value.  */
+	  if (!default_arch_next)
+	    default_arch_next = initial_default_arch;
 	}
     }
+  free ((void *) default_arch);
+  default_arch = xstrdup (default_arch_next);
 
   riscv_release_subset_list (&riscv_subsets);
   riscv_parse_subset (&riscv_rps_dis, default_arch);
+  is_arch_mapping = false;
   return print_insn_riscv;
 }
 
-- 
2.34.1

[RFC PATCH 5/5] RISC-V: New testcases for mapping symbols w/ ISA

[Tsukasa OI]

Mapping symbols are handled per section but the "current" architecture
handled by the assembler has a file scope (regardless of the section).
We have to make sure that we can handle such text section changes while
changing the architecture.

Also, mapping symbol "$x" means that it follows the code with the default
architecture (possibly guessed by the ELF attributes).  So, if the assembler
has the default architecture, we have to make sure that "$x" is emitted.

These four new tests are meant to test such complex conditions.

gas/ChangeLog:

	* testsuite/gas/riscv/mapping-05.s:  New test for FPR switching.
	* testsuite/gas/riscv/mapping-05a.d: Likewise.
	* testsuite/gas/riscv/mapping-05b.d: Likewise.
	* testsuite/gas/riscv/mapping-06.s:  New to test FPR switching and
	make sure the last mapping symbol is the default without ISA string.
	* testsuite/gas/riscv/mapping-06a.d: Likewise.
	* testsuite/gas/riscv/mapping-06b.d: Likewise.
	* testsuite/gas/riscv/mapping-07.s:  New to test no excess mapping
	symbols are generated by switching between text sections.
	* testsuite/gas/riscv/mapping-07a.d: Likewise.
	* testsuite/gas/riscv/mapping-07b.d: Likewise.
	* testsuite/gas/riscv/mapping-08.s:  New to test whether the
	assembler correctly keeps track of architecture changes even if
	text sections are changed in the process.
	* testsuite/gas/riscv/mapping-08a.d: Likewise.
	* testsuite/gas/riscv/mapping-08b.d: Likewise.
---
 gas/testsuite/gas/riscv/mapping-05.s  | 11 +++++++++
 gas/testsuite/gas/riscv/mapping-05a.d | 14 +++++++++++
 gas/testsuite/gas/riscv/mapping-05b.d | 14 +++++++++++
 gas/testsuite/gas/riscv/mapping-06.s  | 11 +++++++++
 gas/testsuite/gas/riscv/mapping-06a.d | 14 +++++++++++
 gas/testsuite/gas/riscv/mapping-06b.d | 14 +++++++++++
 gas/testsuite/gas/riscv/mapping-07.s  | 12 ++++++++++
 gas/testsuite/gas/riscv/mapping-07a.d | 14 +++++++++++
 gas/testsuite/gas/riscv/mapping-07b.d | 21 +++++++++++++++++
 gas/testsuite/gas/riscv/mapping-08.s  | 34 +++++++++++++++++++++++++++
 gas/testsuite/gas/riscv/mapping-08a.d | 18 ++++++++++++++
 gas/testsuite/gas/riscv/mapping-08b.d | 23 ++++++++++++++++++
 12 files changed, 200 insertions(+)
 create mode 100644 gas/testsuite/gas/riscv/mapping-05.s
 create mode 100644 gas/testsuite/gas/riscv/mapping-05a.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-05b.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-06.s
 create mode 100644 gas/testsuite/gas/riscv/mapping-06a.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-06b.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-07.s
 create mode 100644 gas/testsuite/gas/riscv/mapping-07a.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-07b.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-08.s
 create mode 100644 gas/testsuite/gas/riscv/mapping-08a.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-08b.d

diff --git a/gas/testsuite/gas/riscv/mapping-05.s b/gas/testsuite/gas/riscv/mapping-05.s
new file mode 100644
index 00000000000..e3a3da70868
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-05.s
@@ -0,0 +1,11 @@
+	.text
+	# F
+	fadd.s	f10, f11, f12
+	.option	arch, -f
+	.option	arch, +zfinx
+	# Zfinx
+	fadd.s	x10, x11, x12
+	.option	arch, -zfinx
+	.option	arch, +f
+	# F (RV32IF -F +Zfinx -Zfinx +F)
+	fadd.s	f10, f11, f12
diff --git a/gas/testsuite/gas/riscv/mapping-05a.d b/gas/testsuite/gas/riscv/mapping-05a.d
new file mode 100644
index 00000000000..4a775707d02
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-05a.d
@@ -0,0 +1,14 @@
+#as: -march=rv32if
+#source: mapping-05.s
+#objdump: --syms --special-syms
+
+.*file format.*riscv.*
+
+SYMBOL TABLE:
+0+00 l    d  .text	0+00 .text
+0+00 l    d  .data	0+00 .data
+0+00 l    d  .bss	0+00 .bss
+0+00 l       .text	0+00 \$x
+0+04 l       .text	0+00 \$xrv32i2p1_zicsr2p0_zfinx1p0
+0+08 l       .text	0+00 \$xrv32i2p1_f2p2_zicsr2p0
+0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
diff --git a/gas/testsuite/gas/riscv/mapping-05b.d b/gas/testsuite/gas/riscv/mapping-05b.d
new file mode 100644
index 00000000000..04115b5062b
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-05b.d
@@ -0,0 +1,14 @@
+#as: -march=rv32if
+#source: mapping-05.s
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ 	]+0:[ 	]+00c5f553[ 	]+fadd\.s[ 	]+fa0,fa1,fa2
+[ 	]+4:[ 	]+00c5f553[ 	]+fadd\.s[ 	]+a0,a1,a2
+[ 	]+8:[ 	]+00c5f553[ 	]+fadd\.s[ 	]+fa0,fa1,fa2
+#...
diff --git a/gas/testsuite/gas/riscv/mapping-06.s b/gas/testsuite/gas/riscv/mapping-06.s
new file mode 100644
index 00000000000..c2a1d1689ba
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-06.s
@@ -0,0 +1,11 @@
+	.text
+	# F
+	fadd.s	f10, f11, f12
+	.option	push
+	.option	arch, -f
+	.option	arch, +zfinx
+	# Zfinx
+	fadd.s	x10, x11, x12
+	.option	pop
+	# F (reverted to default)
+	fadd.s	f10, f11, f12
diff --git a/gas/testsuite/gas/riscv/mapping-06a.d b/gas/testsuite/gas/riscv/mapping-06a.d
new file mode 100644
index 00000000000..b39d46449c9
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-06a.d
@@ -0,0 +1,14 @@
+#as: -march=rv32if
+#source: mapping-06.s
+#objdump: --syms --special-syms
+
+.*file format.*riscv.*
+
+SYMBOL TABLE:
+0+00 l    d  .text	0+00 .text
+0+00 l    d  .data	0+00 .data
+0+00 l    d  .bss	0+00 .bss
+0+00 l       .text	0+00 \$x
+0+04 l       .text	0+00 \$xrv32i2p1_zicsr2p0_zfinx1p0
+0+08 l       .text	0+00 \$x
+0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
diff --git a/gas/testsuite/gas/riscv/mapping-06b.d b/gas/testsuite/gas/riscv/mapping-06b.d
new file mode 100644
index 00000000000..bf4af0d4499
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-06b.d
@@ -0,0 +1,14 @@
+#as: -march=rv32if
+#source: mapping-06.s
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ 	]+0:[ 	]+00c5f553[ 	]+fadd\.s[ 	]+fa0,fa1,fa2
+[ 	]+4:[ 	]+00c5f553[ 	]+fadd\.s[ 	]+a0,a1,a2
+[ 	]+8:[ 	]+00c5f553[ 	]+fadd\.s[ 	]+fa0,fa1,fa2
+#...
diff --git a/gas/testsuite/gas/riscv/mapping-07.s b/gas/testsuite/gas/riscv/mapping-07.s
new file mode 100644
index 00000000000..04a0dd55a35
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-07.s
@@ -0,0 +1,12 @@
+	.section	.text,  "ax", @progbits
+	add	a0, a1, a2
+	.section	.text2, "ax", @progbits
+	sub	a0, a1, a2
+	.section	.text,  "ax", @progbits
+	add	a0, a1, a2
+	.section	.text2, "ax", @progbits
+	sub	a0, a1, a2
+	.section	.text,  "ax", @progbits
+	add	a0, a1, a2
+	.section	.text2, "ax", @progbits
+	sub	a0, a1, a2
diff --git a/gas/testsuite/gas/riscv/mapping-07a.d b/gas/testsuite/gas/riscv/mapping-07a.d
new file mode 100644
index 00000000000..75af5d91e71
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-07a.d
@@ -0,0 +1,14 @@
+#as:
+#source: mapping-07.s
+#objdump: --syms --special-syms
+
+.*file format.*riscv.*
+
+SYMBOL TABLE:
+0+00 l    d  .text	0+00 .text
+0+00 l    d  .data	0+00 .data
+0+00 l    d  .bss	0+00 .bss
+0+00 l       .text	0+00 \$x
+0+00 l    d  .text2	0+00 .text2
+0+00 l       .text2	0+00 \$x
+0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
diff --git a/gas/testsuite/gas/riscv/mapping-07b.d b/gas/testsuite/gas/riscv/mapping-07b.d
new file mode 100644
index 00000000000..b18d9eea1b6
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-07b.d
@@ -0,0 +1,21 @@
+#as:
+#source: mapping-07.s
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ 	]+0:[ 	]+00c58533[ 	]+add[ 	]+a0,a1,a2
+[ 	]+4:[ 	]+00c58533[ 	]+add[ 	]+a0,a1,a2
+[ 	]+8:[ 	]+00c58533[ 	]+add[ 	]+a0,a1,a2
+#...
+Disassembly of section .text2:
+
+0+000 <.text2>:
+[ 	]+0:[ 	]+40c58533[ 	]+sub[ 	]+a0,a1,a2
+[ 	]+4:[ 	]+40c58533[ 	]+sub[ 	]+a0,a1,a2
+[ 	]+8:[ 	]+40c58533[ 	]+sub[ 	]+a0,a1,a2
+#...
diff --git a/gas/testsuite/gas/riscv/mapping-08.s b/gas/testsuite/gas/riscv/mapping-08.s
new file mode 100644
index 00000000000..b992d8e47fc
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-08.s
@@ -0,0 +1,34 @@
+	.section	.text,  "ax", @progbits
+	# (text:0x0) Arch: default
+	add	a0, a0, a1
+	.option	push
+	.option	arch, +c
+
+	.section	.text2, "ax", @progbits
+	# (text2:0x0) Arch: +c
+	sub	a0, a0, a1
+	.option	pop
+
+	.section	.text,  "ax", @progbits
+	# (text:0x4) Arch: default (no change)
+	add	a0, a0, a1
+	.option	push
+	.option	arch, +c
+	# (text:0x8) Arch: +c
+	add	a0, a0, a1
+
+	.section	.text2, "ax", @progbits
+	# (text2:0x2) Arch: +c (no change)
+	sub	a0, a0, a1
+	.option	arch, -c
+	# (text2:0x4) Arch: +c-c (not default)
+	sub	a0, a0, a1
+
+	.section	.text,  "ax", @progbits
+	# (text:0xa) Arch: +c-c (not default, changed from +c)
+	add	a0, a0, a1
+	.option	pop
+
+	.section	.text2, "ax", @progbits
+	# (text2:0x8) Arch: Default (changed from +c-c)
+	sub	a0, a0, a1
diff --git a/gas/testsuite/gas/riscv/mapping-08a.d b/gas/testsuite/gas/riscv/mapping-08a.d
new file mode 100644
index 00000000000..7c983785471
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-08a.d
@@ -0,0 +1,18 @@
+#as: -march=rv32i
+#source: mapping-08.s
+#objdump: --syms --special-syms
+
+.*file format.*riscv.*
+
+SYMBOL TABLE:
+0+00 l    d  .text	0+00 .text
+0+00 l    d  .data	0+00 .data
+0+00 l    d  .bss	0+00 .bss
+0+00 l       .text	0+00 \$x
+0+00 l    d  .text2	0+00 .text2
+0+00 l       .text2	0+00 \$xrv32i2p1_c2p0
+0+08 l       .text	0+00 \$xrv32i2p1_c2p0
+0+04 l       .text2	0+00 \$xrv32i2p1
+0+0a l       .text	0+00 \$xrv32i2p1
+0+08 l       .text2	0+00 \$x
+0+00 l    d  .riscv.attributes	0+00 .riscv.attributes
diff --git a/gas/testsuite/gas/riscv/mapping-08b.d b/gas/testsuite/gas/riscv/mapping-08b.d
new file mode 100644
index 00000000000..3d9db75e544
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-08b.d
@@ -0,0 +1,23 @@
+#as: -march=rv32i
+#source: mapping-08.s
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ 	]+0:[ 	]+00b50533[ 	]+add[ 	]+a0,a0,a1
+[ 	]+4:[ 	]+00b50533[ 	]+add[ 	]+a0,a0,a1
+[ 	]+8:[ 	]+952e[ 	]+add[ 	]+a0,a0,a1
+[ 	]+a:[ 	]+00b50533[ 	]+add[ 	]+a0,a0,a1
+#...
+Disassembly of section .text2:
+
+0+000 <.text2>:
+[ 	]+0:[ 	]+8d0d[ 	]+sub[ 	]+a0,a0,a1
+[ 	]+2:[ 	]+8d0d[ 	]+sub[ 	]+a0,a0,a1
+[ 	]+4:[ 	]+40b50533[ 	]+sub[ 	]+a0,a0,a1
+[ 	]+8:[ 	]+40b50533[ 	]+sub[ 	]+a0,a0,a1
+#...
-- 
2.34.1

Re: [RFC PATCH 3/5] RISC-V: Mapping symbols with ISA string on assembler

[Jan Beulich]

On 11.08.2022 09:00, Tsukasa OI via Binutils wrote:
> @@ -3855,11 +3887,15 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>    if (strcmp (name, "rvc") == 0)
>      {
>        riscv_update_subset (&riscv_rps_as, "+c");
> +      updated_riscv_subsets = true;
> +      riscv_opts.arch_is_default = false;
>        riscv_set_rvc (true);
>      }
>    else if (strcmp (name, "norvc") == 0)
>      {
>        riscv_update_subset (&riscv_rps_as, "-c");
> +      updated_riscv_subsets = true;
> +      riscv_opts.arch_is_default = false;
>        riscv_set_rvc (false);
>      }
>    else if (strcmp (name, "pic") == 0)
> @@ -3880,6 +3916,8 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>        if (ISSPACE (*name) && *name != '\0')
>  	name++;
>        riscv_update_subset (&riscv_rps_as, name);
> +      updated_riscv_subsets = true;
> +      riscv_opts.arch_is_default = false;

Seeing that all three call sites of riscv_update_subset() gain the
same extra code - wouldn't these assignments better move into that
function? (The function living in bfd may make this difficult, but
it being used by gas only suggests it might better be moved over. Or
otherwise maybe add a local helper function doing all three things?)

As to the resulting "suffix" to $x - is this then intended to be in
strictly canonical form?

> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -750,7 +750,7 @@ riscv_get_map_state (int n,
>      return false;
>  
>    name = bfd_asymbol_name(info->symtab[n]);
> -  if (strcmp (name, "$x") == 0)
> +  if (strncmp (name, "$x", 2) == 0)

Nit: The assembler was switched to use startswith() in similar
cases.

Jan

Re: [RFC PATCH 3/5] RISC-V: Mapping symbols with ISA string on assembler

[Tsukasa OI]

On 2022/08/11 16:31, Jan Beulich wrote:
> On 11.08.2022 09:00, Tsukasa OI via Binutils wrote:
>> @@ -3855,11 +3887,15 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>>    if (strcmp (name, "rvc") == 0)
>>      {
>>        riscv_update_subset (&riscv_rps_as, "+c");
>> +      updated_riscv_subsets = true;
>> +      riscv_opts.arch_is_default = false;
>>        riscv_set_rvc (true);
>>      }
>>    else if (strcmp (name, "norvc") == 0)
>>      {
>>        riscv_update_subset (&riscv_rps_as, "-c");
>> +      updated_riscv_subsets = true;
>> +      riscv_opts.arch_is_default = false;
>>        riscv_set_rvc (false);
>>      }
>>    else if (strcmp (name, "pic") == 0)
>> @@ -3880,6 +3916,8 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>>        if (ISSPACE (*name) && *name != '\0')
>>  	name++;
>>        riscv_update_subset (&riscv_rps_as, name);
>> +      updated_riscv_subsets = true;
>> +      riscv_opts.arch_is_default = false;
> 
> Seeing that all three call sites of riscv_update_subset() gain the
> same extra code - wouldn't these assignments better move into that
> function? (The function living in bfd may make this difficult, but
> it being used by gas only suggests it might better be moved over. Or
> otherwise maybe add a local helper function doing all three things?)

Adding a local helper function would be the best.

> 
> As to the resulting "suffix" to $x - is this then intended to be in
> strictly canonical form?

I'm not sure but both mine and Nelson's generate "$x${CANONICAL}".

> 
>> --- a/opcodes/riscv-dis.c
>> +++ b/opcodes/riscv-dis.c
>> @@ -750,7 +750,7 @@ riscv_get_map_state (int n,
>>      return false;
>>  
>>    name = bfd_asymbol_name(info->symtab[n]);
>> -  if (strcmp (name, "$x") == 0)
>> +  if (strncmp (name, "$x", 2) == 0)
> 
> Nit: The assembler was switched to use startswith() in similar
> cases.

I'll fix this in the next version of the patch (I'll also fix PATCH 4/5).

Thanks,
Tsukasa

> 
> Jan
> 

Re: [RFC PATCH 3/5] RISC-V: Mapping symbols with ISA string on assembler

[Jan Beulich]

On 11.08.2022 13:43, Tsukasa OI wrote:
> On 2022/08/11 16:31, Jan Beulich wrote:
>> On 11.08.2022 09:00, Tsukasa OI via Binutils wrote:
>>> @@ -3855,11 +3887,15 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>>>    if (strcmp (name, "rvc") == 0)
>>>      {
>>>        riscv_update_subset (&riscv_rps_as, "+c");
>>> +      updated_riscv_subsets = true;
>>> +      riscv_opts.arch_is_default = false;
>>>        riscv_set_rvc (true);
>>>      }
>>>    else if (strcmp (name, "norvc") == 0)
>>>      {
>>>        riscv_update_subset (&riscv_rps_as, "-c");
>>> +      updated_riscv_subsets = true;
>>> +      riscv_opts.arch_is_default = false;
>>>        riscv_set_rvc (false);
>>>      }
>>>    else if (strcmp (name, "pic") == 0)
>>> @@ -3880,6 +3916,8 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
>>>        if (ISSPACE (*name) && *name != '\0')
>>>  	name++;
>>>        riscv_update_subset (&riscv_rps_as, name);
>>> +      updated_riscv_subsets = true;
>>> +      riscv_opts.arch_is_default = false;
>>
>> Seeing that all three call sites of riscv_update_subset() gain the
>> same extra code - wouldn't these assignments better move into that
>> function? (The function living in bfd may make this difficult, but
>> it being used by gas only suggests it might better be moved over. Or
>> otherwise maybe add a local helper function doing all three things?)
> 
> Adding a local helper function would be the best.

Just out of curiosity, could you explain to me why the function is
to remain in bfd when gas is its only user?

Jan