public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Tsukasa OI <research_trasio@irq.a4lg.com>
To: Tsukasa OI <research_trasio@irq.a4lg.com>,
	Nelson Chu <nelson.chu@sifive.com>,
	Kito Cheng <kito.cheng@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>
Cc: binutils@sourceware.org
Subject: [RFC PATCH 3/5] RISC-V: Mapping symbols with ISA string on assembler
Date: Thu, 11 Aug 2022 16:00:51 +0900	[thread overview]
Message-ID: <21270977a91ae08292727829cbee7ef230896d2f.1660201178.git.research_trasio@irq.a4lg.com> (raw)
In-Reply-To: <cover.1660201178.git.research_trasio@irq.a4lg.com>

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


  parent reply	other threads:[~2022-08-11  7:01 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-05  9:36 [PATCH] RISC-V: Output mapping symbols with ISA string once .option arch is used Nelson Chu
2022-08-05 10:31 ` Tsukasa OI
2022-08-11  7:00 ` [RFC PATCH 0/5] RISC-V: Support mapping symbols with ISA string Tsukasa OI
2022-08-11  7:00   ` [RFC PATCH 1/5] RISC-V: Use bool on riscv_set_options members Tsukasa OI
2022-08-11  7:00   ` [RFC PATCH 2/5] gas: Copyediting on tc-riscv.c Tsukasa OI
2022-08-11  7:00   ` Tsukasa OI [this message]
2022-08-11  7:31     ` [RFC PATCH 3/5] RISC-V: Mapping symbols with ISA string on assembler Jan Beulich
2022-08-11 11:43       ` Tsukasa OI
2022-08-11 12:12         ` Jan Beulich
2022-08-11  7:00   ` [RFC PATCH 4/5] RISC-V: Mapping symbols with ISA string on disassembler Tsukasa OI
2022-08-11  7:00   ` [RFC PATCH 5/5] RISC-V: New testcases for mapping symbols w/ ISA Tsukasa OI

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=21270977a91ae08292727829cbee7ef230896d2f.1660201178.git.research_trasio@irq.a4lg.com \
    --to=research_trasio@irq.a4lg.com \
    --cc=binutils@sourceware.org \
    --cc=kito.cheng@sifive.com \
    --cc=nelson.chu@sifive.com \
    --cc=palmer@dabbelt.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).