* [PATCH 1/2] RISC-V: File-level architecture shouldn't be affected by section-level ones.
@ 2022-11-04 11:50 Nelson Chu
2022-11-04 11:50 ` [PATCH 2/2] RISC-V: Clarify the suppress rule of mapping symbol with architecture string Nelson Chu
0 siblings, 1 reply; 4+ messages in thread
From: Nelson Chu @ 2022-11-04 11:50 UTC (permalink / raw)
To: binutils, research_trasio, kito.cheng; +Cc: nelson
There are three ways to set architecture in assembler - elf arch attribute
(.attribute arch, "rv32i"), -march option (-march=rv32i) and .option directive
(.option arch, rv32i/+c/-c). This patch clarifies that the first two are
belonged to the file-level architecture setting, and the third .option is
belonged to the section-level setting. However, the file-level architecture
should not be affected by the section-level ones since they have different
purpose. The former is recorded in elf architecture attribue, which used
to decide if the objects can be linked or not; The later are recorded in
mapping symbols, which generally used for ifunc to enable extensions for
specific functions or regions.
gas/
* config/tc-riscv.c (riscv_file_arch): Static const char pointer, used to
record the file-level architecture string.
(riscv_reset_subsets_list_arch_str): Also set riscv_file_arch if needed.
(riscv_set_arch): Updated since riscv_reset_subsets_list_arch_str changed.
(s_riscv_option): Likewise.
(riscv_elf_final_processing): Free riscv_file_arch.
(riscv_write_out_attrs): Output file-level architecture to elf attributes
rather than the section-level one.
* testsuite/gas/riscv/option-arch.s: Collect all related testcases here.
* gas/testsuite/gas/riscv/option-arch-dis.d: Likewise.
* testsuite/gas/riscv/option-arch-symbols.d: Likewise.
* testsuite/gas/riscv/option-arch-01.s: Removed and moved to option-arch.
* testsuite/gas/riscv/option-arch-01a.d: Likewise.
* testsuite/gas/riscv/option-arch-01b.d: Likewise.
* testsuite/gas/riscv/option-arch-02.s: Likewise.
* testsuite/gas/riscv/option-arch-02.d: Likewise.
* testsuite/gas/riscv/option-arch-03.s: Likewise.
* testsuite/gas/riscv/option-arch-03.d: Likewise.
---
gas/config/tc-riscv.c | 22 ++++++++++++++-----
gas/testsuite/gas/riscv/option-arch-01.s | 10 ---------
gas/testsuite/gas/riscv/option-arch-01a.d | 14 ------------
gas/testsuite/gas/riscv/option-arch-02.d | 8 -------
gas/testsuite/gas/riscv/option-arch-02.s | 8 -------
gas/testsuite/gas/riscv/option-arch-03.d | 8 -------
gas/testsuite/gas/riscv/option-arch-03.s | 3 ---
.../{option-arch-01b.d => option-arch-attr.d} | 2 +-
gas/testsuite/gas/riscv/option-arch-dis.d | 15 +++++++++++++
gas/testsuite/gas/riscv/option-arch-symbols.d | 13 +++++++++++
gas/testsuite/gas/riscv/option-arch.s | 13 +++++++++++
11 files changed, 58 insertions(+), 58 deletions(-)
delete mode 100644 gas/testsuite/gas/riscv/option-arch-01.s
delete mode 100644 gas/testsuite/gas/riscv/option-arch-01a.d
delete mode 100644 gas/testsuite/gas/riscv/option-arch-02.d
delete mode 100644 gas/testsuite/gas/riscv/option-arch-02.s
delete mode 100644 gas/testsuite/gas/riscv/option-arch-03.d
delete mode 100644 gas/testsuite/gas/riscv/option-arch-03.s
rename gas/testsuite/gas/riscv/{option-arch-01b.d => option-arch-attr.d} (81%)
create mode 100644 gas/testsuite/gas/riscv/option-arch-dis.d
create mode 100644 gas/testsuite/gas/riscv/option-arch-symbols.d
create mode 100644 gas/testsuite/gas/riscv/option-arch.s
diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 2dc92ecd3c3..dcb70f9d7ad 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -279,15 +279,24 @@ static riscv_parse_subset_t riscv_rps_as =
true, /* check_unknown_prefixed_ext. */
};
+static const char *riscv_file_arch = NULL;
+
/* Update the architecture string in the subset_list. */
static void
-riscv_reset_subsets_list_arch_str (void)
+riscv_reset_subsets_list_arch_str (bool file_level)
{
riscv_subset_list_t *subsets = riscv_rps_as.subset_list;
if (subsets->arch_str != NULL)
free ((void *) subsets->arch_str);
subsets->arch_str = riscv_arch_str (xlen, subsets);
+
+ if (file_level)
+ {
+ if (riscv_file_arch != NULL)
+ free ((void *) riscv_file_arch);
+ riscv_file_arch = strdup (subsets->arch_str);
+ }
}
/* This structure is used to hold a stack of .option values. */
@@ -321,7 +330,7 @@ riscv_set_arch (const char *s)
}
riscv_release_subset_list (riscv_rps_as.subset_list);
riscv_parse_subset (&riscv_rps_as, s);
- riscv_reset_subsets_list_arch_str ();
+ riscv_reset_subsets_list_arch_str (true/* file_level */);
riscv_set_rvc (false);
if (riscv_subset_supports (&riscv_rps_as, "c"))
@@ -4045,13 +4054,13 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
if (strcmp (name, "rvc") == 0)
{
riscv_update_subset (&riscv_rps_as, "+c");
- riscv_reset_subsets_list_arch_str ();
+ riscv_reset_subsets_list_arch_str (false/* file_level */);
riscv_set_rvc (true);
}
else if (strcmp (name, "norvc") == 0)
{
riscv_update_subset (&riscv_rps_as, "-c");
- riscv_reset_subsets_list_arch_str ();
+ riscv_reset_subsets_list_arch_str (false/* file_level */);
riscv_set_rvc (false);
}
else if (strcmp (name, "pic") == 0)
@@ -4072,7 +4081,7 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
if (ISSPACE (*name) && *name != '\0')
name++;
riscv_update_subset (&riscv_rps_as, name);
- riscv_reset_subsets_list_arch_str ();
+ riscv_reset_subsets_list_arch_str (false/* file_level */);
riscv_set_rvc (false);
if (riscv_subset_supports (&riscv_rps_as, "c"))
@@ -4526,6 +4535,7 @@ riscv_elf_final_processing (void)
{
riscv_set_abi_by_arch ();
riscv_release_subset_list (riscv_rps_as.subset_list);
+ free ((void *) riscv_file_arch);
elf_elfheader (stdoutput)->e_flags |= elf_flags;
}
@@ -4610,7 +4620,7 @@ riscv_write_out_attrs (void)
unsigned int i;
/* Re-write architecture elf attribute. */
- arch_str = riscv_rps_as.subset_list->arch_str;
+ arch_str = riscv_file_arch;
bfd_elf_add_proc_attr_string (stdoutput, Tag_RISCV_arch, arch_str);
/* For the file without any instruction, we don't set the default_priv_spec
diff --git a/gas/testsuite/gas/riscv/option-arch-01.s b/gas/testsuite/gas/riscv/option-arch-01.s
deleted file mode 100644
index 50285fc8c73..00000000000
--- a/gas/testsuite/gas/riscv/option-arch-01.s
+++ /dev/null
@@ -1,10 +0,0 @@
-.attribute arch, "rv64ic"
-add a0, a0, a1
-.option push
-.option arch, +d2p0, -c, +xvendor1p0
-add a0, a0, a1
-frcsr a0 # Should add mapping symbol with ISA here, and then dump it to frcsr.
-.option push
-.option arch, +m3p0, +d3p0
-.option pop
-.option pop
diff --git a/gas/testsuite/gas/riscv/option-arch-01a.d b/gas/testsuite/gas/riscv/option-arch-01a.d
deleted file mode 100644
index 1d14c604dec..00000000000
--- a/gas/testsuite/gas/riscv/option-arch-01a.d
+++ /dev/null
@@ -1,14 +0,0 @@
-#as: -misa-spec=2.2
-#source: option-arch-01.s
-#objdump: -d
-
-.*:[ ]+file format .*
-
-
-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[ ]+frcsr[ ]+a0
-#...
diff --git a/gas/testsuite/gas/riscv/option-arch-02.d b/gas/testsuite/gas/riscv/option-arch-02.d
deleted file mode 100644
index 3c27419f9d3..00000000000
--- a/gas/testsuite/gas/riscv/option-arch-02.d
+++ /dev/null
@@ -1,8 +0,0 @@
-#as: -misa-spec=2.2
-#readelf: -A
-#source: option-arch-02.s
-
-Attribute Section: riscv
-File Attributes
- Tag_RISCV_arch: "rv64i2p0_m3p0_f2p0_d3p0_c2p0_zmmul1p0_xvendor32x3p0"
-#...
diff --git a/gas/testsuite/gas/riscv/option-arch-02.s b/gas/testsuite/gas/riscv/option-arch-02.s
deleted file mode 100644
index e0f5de321d6..00000000000
--- a/gas/testsuite/gas/riscv/option-arch-02.s
+++ /dev/null
@@ -1,8 +0,0 @@
-.attribute arch, "rv64ic"
-add a0, a0, a1
-.option push
-.option arch, +d2p0, -c, +xvendor1p0
-add a0, a0, a1
-frcsr a0
-.option pop
-.option arch, +m3p0, +d3p0, +xvendor32x3p0
diff --git a/gas/testsuite/gas/riscv/option-arch-03.d b/gas/testsuite/gas/riscv/option-arch-03.d
deleted file mode 100644
index 62d7f7d5ed2..00000000000
--- a/gas/testsuite/gas/riscv/option-arch-03.d
+++ /dev/null
@@ -1,8 +0,0 @@
-#as:
-#readelf: -A
-#source: option-arch-03.s
-
-Attribute Section: riscv
-File Attributes
- Tag_RISCV_arch: "rv32i2p1_c2p0"
-#...
diff --git a/gas/testsuite/gas/riscv/option-arch-03.s b/gas/testsuite/gas/riscv/option-arch-03.s
deleted file mode 100644
index ccdb1c354b0..00000000000
--- a/gas/testsuite/gas/riscv/option-arch-03.s
+++ /dev/null
@@ -1,3 +0,0 @@
-.attribute arch, "rv64ic"
-.option arch, +d2p0, -c
-.option arch, rv32i2p1c2p0
diff --git a/gas/testsuite/gas/riscv/option-arch-01b.d b/gas/testsuite/gas/riscv/option-arch-attr.d
similarity index 81%
rename from gas/testsuite/gas/riscv/option-arch-01b.d
rename to gas/testsuite/gas/riscv/option-arch-attr.d
index 8f4284d5f15..efe83c7e117 100644
--- a/gas/testsuite/gas/riscv/option-arch-01b.d
+++ b/gas/testsuite/gas/riscv/option-arch-attr.d
@@ -1,6 +1,6 @@
#as: -misa-spec=2.2
+#source: option-arch.s
#readelf: -A
-#source: option-arch-01.s
Attribute Section: riscv
File Attributes
diff --git a/gas/testsuite/gas/riscv/option-arch-dis.d b/gas/testsuite/gas/riscv/option-arch-dis.d
new file mode 100644
index 00000000000..9fa19f7f792
--- /dev/null
+++ b/gas/testsuite/gas/riscv/option-arch-dis.d
@@ -0,0 +1,15 @@
+#as: -misa-spec=2.2
+#source: option-arch.s
+#objdump: -d
+
+.*:[ ]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ ]+[0-9a-f]+:[ ]+0001[ ]+nop
+[ ]+[0-9a-f]+:[ ]+00000013[ ]+nop
+[ ]+[0-9a-f]+:[ ]+00302573[ ]+frcsr[ ]+a0
+[ ]+[0-9a-f]+:[ ]+00000013[ ]+nop
+[ ]+[0-9a-f]+:[ ]+0001[ ]+nop
diff --git a/gas/testsuite/gas/riscv/option-arch-symbols.d b/gas/testsuite/gas/riscv/option-arch-symbols.d
new file mode 100644
index 00000000000..9e2eef6f055
--- /dev/null
+++ b/gas/testsuite/gas/riscv/option-arch-symbols.d
@@ -0,0 +1,13 @@
+#as: -misa-spec=2.2
+#source: option-arch.s
+#objdump: --syms --special-syms
+
+.*file format.*riscv.*
+
+SYMBOL TABLE:
+#...
+0+00 l .text 0000000000000000 \$xrv64i2p0_c2p0
+0+02 l .text 0000000000000000 \$xrv64i2p0_f2p0_d2p0_xvendor1p0
+0+0a l .text 0000000000000000 \$xrv64i2p0_m3p0_f2p0_d2p0_zmmul1p0_xvendor1p0_xvendor32x3p0
+0+0e l .text 0000000000000000 \$xrv32i2p1_c2p0
+#...
diff --git a/gas/testsuite/gas/riscv/option-arch.s b/gas/testsuite/gas/riscv/option-arch.s
new file mode 100644
index 00000000000..1aa36be88e3
--- /dev/null
+++ b/gas/testsuite/gas/riscv/option-arch.s
@@ -0,0 +1,13 @@
+.attribute arch, "rv64ic"
+nop # $xrv64i2p0_c2p0
+.option push
+.option arch, +d2p0, -c, +xvendor1p0
+nop # $xrv64i2p0_f2p0_d2p0_xvendor1p0
+frcsr a0
+.option push
+.option arch, +m3p0, +d3p0, +xvendor32x3p0
+nop # $xrv64i2p0_m3p0_f2p0_d2p0_zmmul1p0_xvendor1p0_xvendor32x3p0
+.option pop
+.option pop
+.option arch, rv32i2p1c2p0
+nop # $xrv32i2p1_c2p0
--
2.37.0 (Apple Git-136)
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] RISC-V: Clarify the suppress rule of mapping symbol with architecture string.
2022-11-04 11:50 [PATCH 1/2] RISC-V: File-level architecture shouldn't be affected by section-level ones Nelson Chu
@ 2022-11-04 11:50 ` Nelson Chu
2022-11-05 10:47 ` Tsukasa OI
0 siblings, 1 reply; 4+ messages in thread
From: Nelson Chu @ 2022-11-04 11:50 UTC (permalink / raw)
To: binutils, research_trasio, kito.cheng; +Cc: nelson
From: Tsukasa OI <research_trasio@irq.a4lg.com>
Clarify the suppress rule - if the section (segment) only has a mapping symbol
with architecture marked at the first instruction, and the architecture is also
same as the file-level setting, then we can suppress the mapping symbol name to
$x. However, we also need to distinguish the meanings of the suppressed $x in
dis-assembler, since sometimes it means the architecture is same as the previous
one, or the same as the file-level one.
gas/
* config/tc-riscv.h (struct riscv_segment_info_type): Added new boolean
`arch_changed', to record if the segment has more than one mapping symbol
with architecture. All segments should have at least one at their first
instruction.
* config/tc-riscv.c (riscv_mapping_state): Set `arch_changed' to true if
the segment has more than one mapping symbol with architecture.
(riscv_check_mapping_symbols): Suppress the mapping symbol with architecture
to $x if the section/segment only has this setting and the architecture is
same as the file-level one.
* testsuite/gas/riscv/mapping.s: Updated, and added new testcases,
.text.suppress, .text.suppress.push.pop and .text.no.suppress.
* testsuite/gas/riscv/mapping-dis.d: Updated.
* testsuite/gas/riscv/mapping-symbols.d: Likewise.
* testsuite/gas/riscv/mapping-attr.d: New testcase to see if the file-level
architecure is polluted by other section-level .option settings.
opcodes/
* riscv-dis.c (riscv_file_subsets): Added to record the file-level subsets.
(riscv_rps_dis): Delay to set subset_list in riscv_get_disassembler or
riscv_get_map_state.
(from_last_map_symbol): Moved from riscv_search_mapping_symbol and make it
global. Set to false means we are dumping a new section.
(riscv_get_map_state): Switch riscv_rps_dis.subset_list to riscv_file_subsets
if the mapping symbol is $x, and we are dumping a new section. Otherwise,
set it to riscv_subsets, and then parsing the architecture from $x+arch.
Besides, add new boolean parameter `update', since we don't need to update
the architecture when called from riscv_data_length.
(riscv_search_mapping_symbol): Updated.
(riscv_get_disassembler): Set the riscv_rps_dis.subset_list to riscv_file_subsets.
Co-developed-by: Nelson Chu <nelson@rivosinc.com>
---
gas/config/tc-riscv.c | 13 ++++++++
gas/config/tc-riscv.h | 1 +
gas/testsuite/gas/riscv/mapping-attr.d | 8 +++++
gas/testsuite/gas/riscv/mapping-dis.d | 23 +++++++++++--
gas/testsuite/gas/riscv/mapping-symbols.d | 11 ++++++-
gas/testsuite/gas/riscv/mapping.s | 35 +++++++++++++++++---
opcodes/riscv-dis.c | 39 +++++++++++++++--------
7 files changed, 110 insertions(+), 20 deletions(-)
create mode 100644 gas/testsuite/gas/riscv/mapping-attr.d
diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index dcb70f9d7ad..56ece8b2dc3 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -598,6 +598,7 @@ riscv_mapping_state (enum riscv_seg_mstate to_state,
S_GET_NAME (seg_arch_symbol) + 2) != 0)
{
reset_seg_arch_str = true;
+ seg_info (now_seg)->tc_segment_info_data.arch_changed = true;
}
else if (from_state == to_state)
return;
@@ -639,6 +640,18 @@ riscv_check_mapping_symbols (bfd *abfd ATTRIBUTE_UNUSED,
if (seginfo == NULL || seginfo->frchainP == NULL)
return;
+ /* If the section only has a mapping symbol at the first instruction,
+ and it is the same as the file-level architecture, then no need to
+ add $x+arch for it, just add $x is enough.
+
+ Please see gas/testsuite/gas/riscv/mapping.s: <your testcase
+ section name>. */
+ symbolS *arch_map_symbol = seginfo->tc_segment_info_data.arch_map_symbol;
+ if (!seginfo->tc_segment_info_data.arch_changed
+ && arch_map_symbol != 0
+ && strcmp (riscv_file_arch, S_GET_NAME (arch_map_symbol) + 2) == 0)
+ S_SET_NAME (arch_map_symbol, "$x");
+
for (fragp = seginfo->frchainP->frch_root;
fragp != NULL;
fragp = fragp->fr_next)
diff --git a/gas/config/tc-riscv.h b/gas/config/tc-riscv.h
index 19c45ba2d12..0dc567e1665 100644
--- a/gas/config/tc-riscv.h
+++ b/gas/config/tc-riscv.h
@@ -140,6 +140,7 @@ struct riscv_segment_info_type
enum riscv_seg_mstate map_state;
/* The current mapping symbol with architecture string. */
symbolS *arch_map_symbol;
+ bool arch_changed;
};
/* Define target fragment type. */
diff --git a/gas/testsuite/gas/riscv/mapping-attr.d b/gas/testsuite/gas/riscv/mapping-attr.d
new file mode 100644
index 00000000000..71bcebf17d4
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-attr.d
@@ -0,0 +1,8 @@
+#as: -misa-spec=20191213
+#source: mapping.s
+#readelf: -A
+
+Attribute Section: riscv
+File Attributes
+ Tag_RISCV_arch: "rv32i2p1_f2p2_c2p0_zicsr2p0"
+#...
diff --git a/gas/testsuite/gas/riscv/mapping-dis.d b/gas/testsuite/gas/riscv/mapping-dis.d
index b1a26fbd151..fe581996e3c 100644
--- a/gas/testsuite/gas/riscv/mapping-dis.d
+++ b/gas/testsuite/gas/riscv/mapping-dis.d
@@ -8,8 +8,8 @@
Disassembly of section .text.cross.section.A:
0+000 <funcA>:
-[ ]+[0-9a-f]+:[ ]+4505[ ]+li[ ]+a0,1
-[ ]+[0-9a-f]+:[ ]+bffd[ ]+j[ ]+0 <funcA>
+[ ]+[0-9a-f]+:[ ]+00100513[ ]+li[ ]+a0,1
+[ ]+[0-9a-f]+:[ ]+bff5[ ]+j[ ]+0 <funcA>
Disassembly of section .text.corss.section.B:
@@ -91,3 +91,22 @@ Disassembly of section .text.relax.align:
[ ]+[0-9a-f]+:[ ]+00000013[ ]+nop
[ ]+[0-9a-f]+:[ ]+00200513[ ]+li[ ]+a0,2
[ ]+[0-9a-f]+:[ ]+00000013[ ]+nop
+
+Disassembly of section .text.suppress:
+
+0+000 <.text.suppress>:
+[ ]+[0-9a-f]+:[ ]+003022f3[ ]+frcsr[ ]+t0
+[ ]+[0-9a-f]+:[ ]+4505[ ]+li[ ]+a0,1
+
+Disassembly of section .text.suppress.push.pop:
+
+0+000 <.text.suppress.push.pop>:
+[ ]+[0-9a-f]+:[ ]+003022f3[ ]+frcsr[ ]+t0
+[ ]+[0-9a-f]+:[ ]+4505[ ]+li[ ]+a0,1
+
+Disassembly of section .text.no.suppress:
+
+0+000 <.text.no.suppress>:
+[ ]+[0-9a-f]+:[ ]+003022f3[ ]+frcsr[ ]+t0
+[ ]+[0-9a-f]+:[ ]+003022f3[ ]+csrr[ ]+t0,fcsr
+[ ]+[0-9a-f]+:[ ]+003022f3[ ]+frcsr[ ]+t0
diff --git a/gas/testsuite/gas/riscv/mapping-symbols.d b/gas/testsuite/gas/riscv/mapping-symbols.d
index 40df3409736..db69c05bfa4 100644
--- a/gas/testsuite/gas/riscv/mapping-symbols.d
+++ b/gas/testsuite/gas/riscv/mapping-symbols.d
@@ -9,7 +9,8 @@ SYMBOL TABLE:
0+00 l d .data 0+00 .data
0+00 l d .bss 0+00 .bss
0+00 l d .text.cross.section.A 0+00 .text.cross.section.A
-0+00 l .text.cross.section.A 0+00 \$xrv32i2p1_c2p0
+0+00 l .text.cross.section.A 0+00 \$xrv32i2p1
+0+04 l .text.cross.section.A 0+00 \$xrv32i2p1_c2p0
0+00 l d .text.corss.section.B 0+00 .text.corss.section.B
0+00 l .text.corss.section.B 0+00 \$xrv32i2p1_c2p0
0+02 l .text.corss.section.B 0+00 \$xrv32i2p1
@@ -42,6 +43,14 @@ SYMBOL TABLE:
0+00 l d .text.relax.align 0+00 .text.relax.align
0+00 l .text.relax.align 0+00 \$xrv32i2p1_c2p0
0+08 l .text.relax.align 0+00 \$xrv32i2p1
+0+00 l d .text.suppress 0+00 .text.suppress
+0+00 l .text.suppress 0+00 \$x
+0+00 l d .text.suppress.push.pop 0+00 .text.suppress.push.pop
+0+00 l .text.suppress.push.pop 0+00 \$x
+0+00 l d .text.no.suppress 0+00 .text.no.suppress
+0+00 l .text.no.suppress 0+00 \$xrv32i2p1_f2p2_c2p0_zicsr2p0
+0+04 l .text.no.suppress 0+00 \$xrv32i2p1_c2p0_zicsr2p0
+0+08 l .text.no.suppress 0+00 \$xrv32i2p1_f2p2_c2p0_zicsr2p0
0+0a l .text.section.padding 0+00 \$x
0+03 l .text.odd.align.start.insn 0+00 \$d
0+04 l .text.odd.align.start.insn 0+00 \$x
diff --git a/gas/testsuite/gas/riscv/mapping.s b/gas/testsuite/gas/riscv/mapping.s
index 3014a69e792..5041d4d4cd4 100644
--- a/gas/testsuite/gas/riscv/mapping.s
+++ b/gas/testsuite/gas/riscv/mapping.s
@@ -1,10 +1,11 @@
-.attribute arch, "rv32ic"
+.attribute arch, "rv32ifc"
.option norelax # FIXME: assembler fill the paddings after parsing everything,
# so we probably won't fill anything for the norelax region when
# the riscv_opts.relax is enabled at somewhere.
.section .text.cross.section.A, "ax"
.option push
+.option arch, rv32i
.global funcA
funcA:
addi a0, zero, 1 # rv32i
@@ -20,6 +21,7 @@ j funcB # rv32i
.section .text.data, "ax"
.option push
+.option arch, rv32ic
.word 0 # $d
.long 1
addi a0, zero, 1 # rv32ic
@@ -35,7 +37,7 @@ addi a0, zero, 2 # $x, but same as previous addi, so removed
.section .text.odd.align.start.insn, "ax"
.option push
.option norelax
-.option arch, +c
+.option arch, rv32ic
addi a0, zero, 1 # $xrv32ic
.byte 1 # $d
.option arch, -c
@@ -46,7 +48,7 @@ addi a0, zero, 2 # $xrv32i
.section .text.odd.align.start.data, "ax"
.option push
.option norelax
-.option arch, +c
+.option arch, rv32ic
.byte 1 # $d
.align 2 # odd alignment, $xrv32ic replaced by $d + $xrv32ic
addi a0, zero, 1
@@ -55,6 +57,7 @@ addi a0, zero, 1
.section .text.zero.fill.first, "ax"
.option push
.option norelax
+.option arch, rv32ic
.fill 1, 0, 0 # $d with zero size, removed in make_mapping_symbol
addi a0, zero, 1 # $xrv32ic
.option pop
@@ -62,6 +65,7 @@ addi a0, zero, 1 # $xrv32ic
.section .text.zero.fill.last, "ax"
.option push
.option norelax
+.option arch, rv32ic
addi a0, zero, 1 # $xrv32ic
.fill 1, 0, 0 # $d with zero size, removed in make_mapping_symbol
addi a0, zero, 2 # $x, FIXME: need find a way to remove?
@@ -71,6 +75,7 @@ addi a0, zero, 2 # $x, FIXME: need find a way to remove?
.section .text.zero.fill.align.A, "ax"
.option push
.option norelax
+.option arch, rv32ic
.align 2 # $xrv32ic, .align and .fill are in the different frag, so neither be removed
.fill 1, 0, 0 # $d with zero size, removed in make_mapping_symbol when adding $xrv32ic
addi a0, zero, 1 # $x, should be removed in riscv_check_mapping_symbols
@@ -81,6 +86,7 @@ addi a0, zero, 2
.section .text.zero.fill.align.B, "ax"
.option push
.option norelax
+.option arch, rv32ic
.align 2 # $xrv32ic, .align and .fill are in the different frag, so neither be removed,
# but will be removed in riscv_check_mapping_symbols
.fill 1, 0, 0 # $d with zero size, removed in make_mapping_symbol when adding $xrv32ic
@@ -92,7 +98,7 @@ addi a0, zero, 2
.section .text.last.section, "ax"
.option push
.option norelax
-.option arch, -c
+.option arch, rv32i
addi a0, zero, 1 # $xrv32i
.word 1 # $d
.align 2 # zero section padding, $x at the end of section, removed in riscv_check_mapping_symbols
@@ -101,6 +107,7 @@ addi a0, zero, 1 # $xrv32i
.section .text.section.padding, "ax"
.option push
.option norelax
+.option arch, rv32ic
.align 2
addi a0, zero, 1 # $rv32ic
.option arch, +a
@@ -119,3 +126,23 @@ addi a0, zero, 1 # $x, won't added
.align 3 # $x, won't added
addi a0, zero, 2 # $xrv32i
.option pop
+
+.section .text.suppress, "ax"
+csrrs t0, fcsr, zero # suppress $xrv32ifc to $x
+addi a0, zero, 1
+
+.section .text.suppress.push.pop, "ax"
+.option push
+.option arch, rv32i
+.option arch, +f, +c
+csrrs t0, fcsr, zero # suppress $xrv32ifc to $x
+addi a0, zero, 1
+.option pop
+
+.section .text.no.suppress, "ax"
+csrrs t0, fcsr, zero # $xrv32ifc, since not only one $x+arch, so don't suppress
+.option push
+.option arch, -f
+csrrs t0, fcsr, zero # $xrv32ic
+.option pop
+csrrs t0, fcsr, zero # $xrv32ifc, likewise
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 3a31647a2f8..f935490c64f 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -42,10 +42,11 @@ static enum riscv_spec_class default_isa_spec = ISA_SPEC_CLASS_DRAFT - 1;
(as specified by the ELF attributes or the `priv-spec' option). */
static enum riscv_spec_class default_priv_spec = PRIV_SPEC_CLASS_NONE;
+static riscv_subset_list_t riscv_file_subsets;
static riscv_subset_list_t riscv_subsets;
static riscv_parse_subset_t riscv_rps_dis =
{
- &riscv_subsets, /* subset_list. */
+ NULL, /* subset_list. */
opcodes_error_handler,/* error_handler. */
&xlen, /* xlen. */
&default_isa_spec, /* isa_spec. */
@@ -64,6 +65,7 @@ struct riscv_private_data
/* Used for mapping symbols. */
static int last_map_symbol = -1;
static bfd_vma last_stop_offset = 0;
+static bool from_last_map_symbol = false;
/* Register names as used by the disassembler. */
static const char * const *riscv_gpr_names;
@@ -821,7 +823,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
static bool
riscv_get_map_state (int n,
enum riscv_seg_mstate *state,
- struct disassemble_info *info)
+ struct disassemble_info *info,
+ bool update)
{
const char *name;
@@ -831,15 +834,25 @@ riscv_get_map_state (int n,
return false;
name = bfd_asymbol_name(info->symtab[n]);
- if (strcmp (name, "$x") == 0)
- *state = MAP_INSN;
- else if (strcmp (name, "$d") == 0)
+ if (strcmp (name, "$d") == 0)
*state = MAP_DATA;
+ else if (strcmp (name, "$x") == 0)
+ {
+ *state = MAP_INSN;
+ /* Switch back to file-level architecture when starting from
+ a new section. */
+ if (update && !from_last_map_symbol)
+ riscv_rps_dis.subset_list = &riscv_file_subsets;
+ }
else if (strncmp (name, "$xrv", 4) == 0)
{
*state = MAP_INSN;
- riscv_release_subset_list (&riscv_subsets);
- riscv_parse_subset (&riscv_rps_dis, name + 2);
+ if (update)
+ {
+ riscv_rps_dis.subset_list = &riscv_subsets;
+ riscv_release_subset_list (riscv_rps_dis.subset_list);
+ riscv_parse_subset (&riscv_rps_dis, name + 2);
+ }
}
else
return false;
@@ -855,7 +868,6 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
struct disassemble_info *info)
{
enum riscv_seg_mstate mstate;
- bool from_last_map_symbol;
bool found = false;
int symbol = -1;
int n;
@@ -872,7 +884,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
|| bfd_asymbol_flavour (*info->symtab) != bfd_target_elf_flavour)
return mstate;
- /* Reset the last_map_symbol if we start to dump a new section. */
+ /* Reset the last_map_symbol when pc <= 0. */
if (memaddr <= 0)
last_map_symbol = -1;
@@ -895,7 +907,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
/* We have searched all possible symbols in the range. */
if (addr > memaddr)
break;
- if (riscv_get_map_state (n, &mstate, info))
+ if (riscv_get_map_state (n, &mstate, info, true/* update */))
{
symbol = n;
found = true;
@@ -922,7 +934,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
if (addr < (info->section ? info->section->vma : 0))
break;
/* Stop searching once we find the closed mapping symbol. */
- if (riscv_get_map_state (n, &mstate, info))
+ if (riscv_get_map_state (n, &mstate, info, true/* update */))
{
symbol = n;
found = true;
@@ -958,7 +970,7 @@ riscv_data_length (bfd_vma memaddr,
{
bfd_vma addr = bfd_asymbol_value (info->symtab[n]);
if (addr > memaddr
- && riscv_get_map_state (n, &m, info))
+ && riscv_get_map_state (n, &m, info, false/* update */))
{
if (addr - memaddr < length)
length = addr - memaddr;
@@ -1106,7 +1118,8 @@ riscv_get_disassembler (bfd *abfd)
}
}
- riscv_release_subset_list (&riscv_subsets);
+ riscv_rps_dis.subset_list = &riscv_file_subsets;
+ riscv_release_subset_list (riscv_rps_dis.subset_list);
riscv_parse_subset (&riscv_rps_dis, default_arch);
return print_insn_riscv;
}
--
2.37.0 (Apple Git-136)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] RISC-V: Clarify the suppress rule of mapping symbol with architecture string.
2022-11-04 11:50 ` [PATCH 2/2] RISC-V: Clarify the suppress rule of mapping symbol with architecture string Nelson Chu
@ 2022-11-05 10:47 ` Tsukasa OI
2022-11-07 2:37 ` Nelson Chu
0 siblings, 1 reply; 4+ messages in thread
From: Tsukasa OI @ 2022-11-05 10:47 UTC (permalink / raw)
To: Nelson Chu, binutils, Kito Cheng
On 2022/11/04 20:50, Nelson Chu wrote:
> From: Tsukasa OI <research_trasio@irq.a4lg.com>
>
> Clarify the suppress rule - if the section (segment) only has a mapping symbol
> with architecture marked at the first instruction, and the architecture is also
> same as the file-level setting, then we can suppress the mapping symbol name to
> $x. However, we also need to distinguish the meanings of the suppressed $x in
> dis-assembler, since sometimes it means the architecture is same as the previous
> one, or the same as the file-level one.
For PATCH 1/2, LGTM. I truly appreciate that.
For PATCH 2/2, as far as I can tell, it looks not bad to me but with
some comments and to-dos (two of which related to Binutils itself).
1. Disassembler part
Your implementation itself looks good.
I was making multiple part 4/4 disassembler improvement patchsets
depending on the possible interpretation of "$x" and this interpretation
was my "likely winner". So, I might be forced to remove your
disassembler contribution in the process if yours is merged first.
I will need to apologize that first.
2. Relationship with Linker
The rule: 'the first "$x" symbol in the section means instruction with
ELF attributes' makes sense to me but may force the linker to do
something more than the current implementation. This is because, the
first "$x" symbol in an section *of an object file* might not be the
first "$x" symbol in an section of the final (linked) object file.
Both my RFC PATCH and your patchset (PATCH 2/2) might complicate the
linker because of this property.
They replaced the first "$x+arch" with "$x" in the section when the
architecture matches ELF attributes in the assembler. However, not
doing this like your commit 0ce50fc900a5 ("RISC-V: Always generate
mapping symbols at the start of the sections.") might be an option (NOT
as a workaround).
If this option is acceptable to you, my new proposal on linker is either:
(a) Do nothing (just like the old linker)
It looks like a joke but I want to say that object files generated
from your commit 0ce50fc900a5 is compatible with the old linker,
assuming new "$x" interpretation rule. Also, multiple long
"$x+arch" symbols do not increase the final file size that much
(because the same symbol names are reused on disk).
The description above is not strictly true when we consider about
multi-toolchain object linking support but still applies on
many cases.
(b) Replace "$x+arch" with "$x" if the implied architecture
depending on the context is either:
(1) The same as the final ELF attributes or
(2) compatible with that.
and an optional handling:
(c) (Virtually/actually) replace first "$x" in the section with
"$x+arch" with input ELF file's attribute (not output) when
preprocessing input object files.
Following combinations are possible:
1. None (a)
2. (c)
3. (b.1)
4. (b.1) + (c)
5. (b.2)
6. (b.2) + (c)
I'm sorry to say I haven't read GNU ld code much and I'm not sure
whether the options (b.{1,2}) are feasible.
Option (c) is for much complex situation: multi-toolchain object linking
support (e.g. a static library is built with an old toolchain without
"$x+arch" but with "$x", application code is built with a new toolchain
with "$x+arch" and we link them together with a new [or even newer] one
supporting "$x+arch").
3. Outside Binutils
Current mapping symbol proposal:
<https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/196>
requires some updates because, in that proposal, all "$x" means "revert
to default (as in ELF attributes)". So we need to talk to Kito about
changing this proposal.
Thanks,
Tsukasa
>
> gas/
> * config/tc-riscv.h (struct riscv_segment_info_type): Added new boolean
> `arch_changed', to record if the segment has more than one mapping symbol
> with architecture. All segments should have at least one at their first
> instruction.
> * config/tc-riscv.c (riscv_mapping_state): Set `arch_changed' to true if
> the segment has more than one mapping symbol with architecture.
> (riscv_check_mapping_symbols): Suppress the mapping symbol with architecture
> to $x if the section/segment only has this setting and the architecture is
> same as the file-level one.
> * testsuite/gas/riscv/mapping.s: Updated, and added new testcases,
> .text.suppress, .text.suppress.push.pop and .text.no.suppress.
> * testsuite/gas/riscv/mapping-dis.d: Updated.
> * testsuite/gas/riscv/mapping-symbols.d: Likewise.
> * testsuite/gas/riscv/mapping-attr.d: New testcase to see if the file-level
> architecure is polluted by other section-level .option settings.
> opcodes/
> * riscv-dis.c (riscv_file_subsets): Added to record the file-level subsets.
> (riscv_rps_dis): Delay to set subset_list in riscv_get_disassembler or
> riscv_get_map_state.
> (from_last_map_symbol): Moved from riscv_search_mapping_symbol and make it
> global. Set to false means we are dumping a new section.
> (riscv_get_map_state): Switch riscv_rps_dis.subset_list to riscv_file_subsets
> if the mapping symbol is $x, and we are dumping a new section. Otherwise,
> set it to riscv_subsets, and then parsing the architecture from $x+arch.
> Besides, add new boolean parameter `update', since we don't need to update
> the architecture when called from riscv_data_length.
> (riscv_search_mapping_symbol): Updated.
> (riscv_get_disassembler): Set the riscv_rps_dis.subset_list to riscv_file_subsets.
>
> Co-developed-by: Nelson Chu <nelson@rivosinc.com>
> ---
> gas/config/tc-riscv.c | 13 ++++++++
> gas/config/tc-riscv.h | 1 +
> gas/testsuite/gas/riscv/mapping-attr.d | 8 +++++
> gas/testsuite/gas/riscv/mapping-dis.d | 23 +++++++++++--
> gas/testsuite/gas/riscv/mapping-symbols.d | 11 ++++++-
> gas/testsuite/gas/riscv/mapping.s | 35 +++++++++++++++++---
> opcodes/riscv-dis.c | 39 +++++++++++++++--------
> 7 files changed, 110 insertions(+), 20 deletions(-)
> create mode 100644 gas/testsuite/gas/riscv/mapping-attr.d
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index dcb70f9d7ad..56ece8b2dc3 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -598,6 +598,7 @@ riscv_mapping_state (enum riscv_seg_mstate to_state,
> S_GET_NAME (seg_arch_symbol) + 2) != 0)
> {
> reset_seg_arch_str = true;
> + seg_info (now_seg)->tc_segment_info_data.arch_changed = true;
> }
> else if (from_state == to_state)
> return;
> @@ -639,6 +640,18 @@ riscv_check_mapping_symbols (bfd *abfd ATTRIBUTE_UNUSED,
> if (seginfo == NULL || seginfo->frchainP == NULL)
> return;
>
> + /* If the section only has a mapping symbol at the first instruction,
> + and it is the same as the file-level architecture, then no need to
> + add $x+arch for it, just add $x is enough.
> +
> + Please see gas/testsuite/gas/riscv/mapping.s: <your testcase
> + section name>. */
> + symbolS *arch_map_symbol = seginfo->tc_segment_info_data.arch_map_symbol;
> + if (!seginfo->tc_segment_info_data.arch_changed
> + && arch_map_symbol != 0
> + && strcmp (riscv_file_arch, S_GET_NAME (arch_map_symbol) + 2) == 0)
> + S_SET_NAME (arch_map_symbol, "$x");
> +
> for (fragp = seginfo->frchainP->frch_root;
> fragp != NULL;
> fragp = fragp->fr_next)
> diff --git a/gas/config/tc-riscv.h b/gas/config/tc-riscv.h
> index 19c45ba2d12..0dc567e1665 100644
> --- a/gas/config/tc-riscv.h
> +++ b/gas/config/tc-riscv.h
> @@ -140,6 +140,7 @@ struct riscv_segment_info_type
> enum riscv_seg_mstate map_state;
> /* The current mapping symbol with architecture string. */
> symbolS *arch_map_symbol;
> + bool arch_changed;
> };
>
> /* Define target fragment type. */
> diff --git a/gas/testsuite/gas/riscv/mapping-attr.d b/gas/testsuite/gas/riscv/mapping-attr.d
> new file mode 100644
> index 00000000000..71bcebf17d4
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/mapping-attr.d
> @@ -0,0 +1,8 @@
> +#as: -misa-spec=20191213
> +#source: mapping.s
> +#readelf: -A
> +
> +Attribute Section: riscv
> +File Attributes
> + Tag_RISCV_arch: "rv32i2p1_f2p2_c2p0_zicsr2p0"
> +#...
> diff --git a/gas/testsuite/gas/riscv/mapping-dis.d b/gas/testsuite/gas/riscv/mapping-dis.d
> index b1a26fbd151..fe581996e3c 100644
> --- a/gas/testsuite/gas/riscv/mapping-dis.d
> +++ b/gas/testsuite/gas/riscv/mapping-dis.d
> @@ -8,8 +8,8 @@
> Disassembly of section .text.cross.section.A:
>
> 0+000 <funcA>:
> -[ ]+[0-9a-f]+:[ ]+4505[ ]+li[ ]+a0,1
> -[ ]+[0-9a-f]+:[ ]+bffd[ ]+j[ ]+0 <funcA>
> +[ ]+[0-9a-f]+:[ ]+00100513[ ]+li[ ]+a0,1
> +[ ]+[0-9a-f]+:[ ]+bff5[ ]+j[ ]+0 <funcA>
>
> Disassembly of section .text.corss.section.B:
>
> @@ -91,3 +91,22 @@ Disassembly of section .text.relax.align:
> [ ]+[0-9a-f]+:[ ]+00000013[ ]+nop
> [ ]+[0-9a-f]+:[ ]+00200513[ ]+li[ ]+a0,2
> [ ]+[0-9a-f]+:[ ]+00000013[ ]+nop
> +
> +Disassembly of section .text.suppress:
> +
> +0+000 <.text.suppress>:
> +[ ]+[0-9a-f]+:[ ]+003022f3[ ]+frcsr[ ]+t0
> +[ ]+[0-9a-f]+:[ ]+4505[ ]+li[ ]+a0,1
> +
> +Disassembly of section .text.suppress.push.pop:
> +
> +0+000 <.text.suppress.push.pop>:
> +[ ]+[0-9a-f]+:[ ]+003022f3[ ]+frcsr[ ]+t0
> +[ ]+[0-9a-f]+:[ ]+4505[ ]+li[ ]+a0,1
> +
> +Disassembly of section .text.no.suppress:
> +
> +0+000 <.text.no.suppress>:
> +[ ]+[0-9a-f]+:[ ]+003022f3[ ]+frcsr[ ]+t0
> +[ ]+[0-9a-f]+:[ ]+003022f3[ ]+csrr[ ]+t0,fcsr
> +[ ]+[0-9a-f]+:[ ]+003022f3[ ]+frcsr[ ]+t0
> diff --git a/gas/testsuite/gas/riscv/mapping-symbols.d b/gas/testsuite/gas/riscv/mapping-symbols.d
> index 40df3409736..db69c05bfa4 100644
> --- a/gas/testsuite/gas/riscv/mapping-symbols.d
> +++ b/gas/testsuite/gas/riscv/mapping-symbols.d
> @@ -9,7 +9,8 @@ SYMBOL TABLE:
> 0+00 l d .data 0+00 .data
> 0+00 l d .bss 0+00 .bss
> 0+00 l d .text.cross.section.A 0+00 .text.cross.section.A
> -0+00 l .text.cross.section.A 0+00 \$xrv32i2p1_c2p0
> +0+00 l .text.cross.section.A 0+00 \$xrv32i2p1
> +0+04 l .text.cross.section.A 0+00 \$xrv32i2p1_c2p0
> 0+00 l d .text.corss.section.B 0+00 .text.corss.section.B
> 0+00 l .text.corss.section.B 0+00 \$xrv32i2p1_c2p0
> 0+02 l .text.corss.section.B 0+00 \$xrv32i2p1
> @@ -42,6 +43,14 @@ SYMBOL TABLE:
> 0+00 l d .text.relax.align 0+00 .text.relax.align
> 0+00 l .text.relax.align 0+00 \$xrv32i2p1_c2p0
> 0+08 l .text.relax.align 0+00 \$xrv32i2p1
> +0+00 l d .text.suppress 0+00 .text.suppress
> +0+00 l .text.suppress 0+00 \$x
> +0+00 l d .text.suppress.push.pop 0+00 .text.suppress.push.pop
> +0+00 l .text.suppress.push.pop 0+00 \$x
> +0+00 l d .text.no.suppress 0+00 .text.no.suppress
> +0+00 l .text.no.suppress 0+00 \$xrv32i2p1_f2p2_c2p0_zicsr2p0
> +0+04 l .text.no.suppress 0+00 \$xrv32i2p1_c2p0_zicsr2p0
> +0+08 l .text.no.suppress 0+00 \$xrv32i2p1_f2p2_c2p0_zicsr2p0
> 0+0a l .text.section.padding 0+00 \$x
> 0+03 l .text.odd.align.start.insn 0+00 \$d
> 0+04 l .text.odd.align.start.insn 0+00 \$x
> diff --git a/gas/testsuite/gas/riscv/mapping.s b/gas/testsuite/gas/riscv/mapping.s
> index 3014a69e792..5041d4d4cd4 100644
> --- a/gas/testsuite/gas/riscv/mapping.s
> +++ b/gas/testsuite/gas/riscv/mapping.s
> @@ -1,10 +1,11 @@
> -.attribute arch, "rv32ic"
> +.attribute arch, "rv32ifc"
> .option norelax # FIXME: assembler fill the paddings after parsing everything,
> # so we probably won't fill anything for the norelax region when
> # the riscv_opts.relax is enabled at somewhere.
>
> .section .text.cross.section.A, "ax"
> .option push
> +.option arch, rv32i
> .global funcA
> funcA:
> addi a0, zero, 1 # rv32i
> @@ -20,6 +21,7 @@ j funcB # rv32i
>
> .section .text.data, "ax"
> .option push
> +.option arch, rv32ic
> .word 0 # $d
> .long 1
> addi a0, zero, 1 # rv32ic
> @@ -35,7 +37,7 @@ addi a0, zero, 2 # $x, but same as previous addi, so removed
> .section .text.odd.align.start.insn, "ax"
> .option push
> .option norelax
> -.option arch, +c
> +.option arch, rv32ic
> addi a0, zero, 1 # $xrv32ic
> .byte 1 # $d
> .option arch, -c
> @@ -46,7 +48,7 @@ addi a0, zero, 2 # $xrv32i
> .section .text.odd.align.start.data, "ax"
> .option push
> .option norelax
> -.option arch, +c
> +.option arch, rv32ic
> .byte 1 # $d
> .align 2 # odd alignment, $xrv32ic replaced by $d + $xrv32ic
> addi a0, zero, 1
> @@ -55,6 +57,7 @@ addi a0, zero, 1
> .section .text.zero.fill.first, "ax"
> .option push
> .option norelax
> +.option arch, rv32ic
> .fill 1, 0, 0 # $d with zero size, removed in make_mapping_symbol
> addi a0, zero, 1 # $xrv32ic
> .option pop
> @@ -62,6 +65,7 @@ addi a0, zero, 1 # $xrv32ic
> .section .text.zero.fill.last, "ax"
> .option push
> .option norelax
> +.option arch, rv32ic
> addi a0, zero, 1 # $xrv32ic
> .fill 1, 0, 0 # $d with zero size, removed in make_mapping_symbol
> addi a0, zero, 2 # $x, FIXME: need find a way to remove?
> @@ -71,6 +75,7 @@ addi a0, zero, 2 # $x, FIXME: need find a way to remove?
> .section .text.zero.fill.align.A, "ax"
> .option push
> .option norelax
> +.option arch, rv32ic
> .align 2 # $xrv32ic, .align and .fill are in the different frag, so neither be removed
> .fill 1, 0, 0 # $d with zero size, removed in make_mapping_symbol when adding $xrv32ic
> addi a0, zero, 1 # $x, should be removed in riscv_check_mapping_symbols
> @@ -81,6 +86,7 @@ addi a0, zero, 2
> .section .text.zero.fill.align.B, "ax"
> .option push
> .option norelax
> +.option arch, rv32ic
> .align 2 # $xrv32ic, .align and .fill are in the different frag, so neither be removed,
> # but will be removed in riscv_check_mapping_symbols
> .fill 1, 0, 0 # $d with zero size, removed in make_mapping_symbol when adding $xrv32ic
> @@ -92,7 +98,7 @@ addi a0, zero, 2
> .section .text.last.section, "ax"
> .option push
> .option norelax
> -.option arch, -c
> +.option arch, rv32i
> addi a0, zero, 1 # $xrv32i
> .word 1 # $d
> .align 2 # zero section padding, $x at the end of section, removed in riscv_check_mapping_symbols
> @@ -101,6 +107,7 @@ addi a0, zero, 1 # $xrv32i
> .section .text.section.padding, "ax"
> .option push
> .option norelax
> +.option arch, rv32ic
> .align 2
> addi a0, zero, 1 # $rv32ic
> .option arch, +a
> @@ -119,3 +126,23 @@ addi a0, zero, 1 # $x, won't added
> .align 3 # $x, won't added
> addi a0, zero, 2 # $xrv32i
> .option pop
> +
> +.section .text.suppress, "ax"
> +csrrs t0, fcsr, zero # suppress $xrv32ifc to $x
> +addi a0, zero, 1
> +
> +.section .text.suppress.push.pop, "ax"
> +.option push
> +.option arch, rv32i
> +.option arch, +f, +c
> +csrrs t0, fcsr, zero # suppress $xrv32ifc to $x
> +addi a0, zero, 1
> +.option pop
> +
> +.section .text.no.suppress, "ax"
> +csrrs t0, fcsr, zero # $xrv32ifc, since not only one $x+arch, so don't suppress
> +.option push
> +.option arch, -f
> +csrrs t0, fcsr, zero # $xrv32ic
> +.option pop
> +csrrs t0, fcsr, zero # $xrv32ifc, likewise
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 3a31647a2f8..f935490c64f 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -42,10 +42,11 @@ static enum riscv_spec_class default_isa_spec = ISA_SPEC_CLASS_DRAFT - 1;
> (as specified by the ELF attributes or the `priv-spec' option). */
> static enum riscv_spec_class default_priv_spec = PRIV_SPEC_CLASS_NONE;
>
> +static riscv_subset_list_t riscv_file_subsets;
> static riscv_subset_list_t riscv_subsets;
> static riscv_parse_subset_t riscv_rps_dis =
> {
> - &riscv_subsets, /* subset_list. */
> + NULL, /* subset_list. */
> opcodes_error_handler,/* error_handler. */
> &xlen, /* xlen. */
> &default_isa_spec, /* isa_spec. */
> @@ -64,6 +65,7 @@ struct riscv_private_data
> /* Used for mapping symbols. */
> static int last_map_symbol = -1;
> static bfd_vma last_stop_offset = 0;
> +static bool from_last_map_symbol = false;
>
> /* Register names as used by the disassembler. */
> static const char * const *riscv_gpr_names;
> @@ -821,7 +823,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
> static bool
> riscv_get_map_state (int n,
> enum riscv_seg_mstate *state,
> - struct disassemble_info *info)
> + struct disassemble_info *info,
> + bool update)
> {
> const char *name;
>
> @@ -831,15 +834,25 @@ riscv_get_map_state (int n,
> return false;
>
> name = bfd_asymbol_name(info->symtab[n]);
> - if (strcmp (name, "$x") == 0)
> - *state = MAP_INSN;
> - else if (strcmp (name, "$d") == 0)
> + if (strcmp (name, "$d") == 0)
> *state = MAP_DATA;
> + else if (strcmp (name, "$x") == 0)
> + {
> + *state = MAP_INSN;
> + /* Switch back to file-level architecture when starting from
> + a new section. */
> + if (update && !from_last_map_symbol)
> + riscv_rps_dis.subset_list = &riscv_file_subsets;
> + }
> else if (strncmp (name, "$xrv", 4) == 0)
> {
> *state = MAP_INSN;
> - riscv_release_subset_list (&riscv_subsets);
> - riscv_parse_subset (&riscv_rps_dis, name + 2);
> + if (update)
> + {
> + riscv_rps_dis.subset_list = &riscv_subsets;
> + riscv_release_subset_list (riscv_rps_dis.subset_list);
> + riscv_parse_subset (&riscv_rps_dis, name + 2);
> + }
> }
> else
> return false;
> @@ -855,7 +868,6 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
> struct disassemble_info *info)
> {
> enum riscv_seg_mstate mstate;
> - bool from_last_map_symbol;
> bool found = false;
> int symbol = -1;
> int n;
> @@ -872,7 +884,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
> || bfd_asymbol_flavour (*info->symtab) != bfd_target_elf_flavour)
> return mstate;
>
> - /* Reset the last_map_symbol if we start to dump a new section. */
> + /* Reset the last_map_symbol when pc <= 0. */
> if (memaddr <= 0)
> last_map_symbol = -1;
>
> @@ -895,7 +907,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
> /* We have searched all possible symbols in the range. */
> if (addr > memaddr)
> break;
> - if (riscv_get_map_state (n, &mstate, info))
> + if (riscv_get_map_state (n, &mstate, info, true/* update */))
> {
> symbol = n;
> found = true;
> @@ -922,7 +934,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
> if (addr < (info->section ? info->section->vma : 0))
> break;
> /* Stop searching once we find the closed mapping symbol. */
> - if (riscv_get_map_state (n, &mstate, info))
> + if (riscv_get_map_state (n, &mstate, info, true/* update */))
> {
> symbol = n;
> found = true;
> @@ -958,7 +970,7 @@ riscv_data_length (bfd_vma memaddr,
> {
> bfd_vma addr = bfd_asymbol_value (info->symtab[n]);
> if (addr > memaddr
> - && riscv_get_map_state (n, &m, info))
> + && riscv_get_map_state (n, &m, info, false/* update */))
> {
> if (addr - memaddr < length)
> length = addr - memaddr;
> @@ -1106,7 +1118,8 @@ riscv_get_disassembler (bfd *abfd)
> }
> }
>
> - riscv_release_subset_list (&riscv_subsets);
> + riscv_rps_dis.subset_list = &riscv_file_subsets;
> + riscv_release_subset_list (riscv_rps_dis.subset_list);
> riscv_parse_subset (&riscv_rps_dis, default_arch);
> return print_insn_riscv;
> }
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] RISC-V: Clarify the suppress rule of mapping symbol with architecture string.
2022-11-05 10:47 ` Tsukasa OI
@ 2022-11-07 2:37 ` Nelson Chu
0 siblings, 0 replies; 4+ messages in thread
From: Nelson Chu @ 2022-11-07 2:37 UTC (permalink / raw)
To: Tsukasa OI; +Cc: binutils, Kito Cheng
On Sat, Nov 5, 2022 at 6:47 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> On 2022/11/04 20:50, Nelson Chu wrote:
> > From: Tsukasa OI <research_trasio@irq.a4lg.com>
> >
> > Clarify the suppress rule - if the section (segment) only has a mapping symbol
> > with architecture marked at the first instruction, and the architecture is also
> > same as the file-level setting, then we can suppress the mapping symbol name to
> > $x. However, we also need to distinguish the meanings of the suppressed $x in
> > dis-assembler, since sometimes it means the architecture is same as the previous
> > one, or the same as the file-level one.
>
> For PATCH 1/2, LGTM. I truly appreciate that.
> For PATCH 2/2, as far as I can tell, it looks not bad to me but with
> some comments and to-dos (two of which related to Binutils itself).
>
>
> 1. Disassembler part
>
> Your implementation itself looks good.
>
> I was making multiple part 4/4 disassembler improvement patchsets
> depending on the possible interpretation of "$x" and this interpretation
> was my "likely winner". So, I might be forced to remove your
> disassembler contribution in the process if yours is merged first.
> I will need to apologize that first.
Yeah that's fine, always happens. I ported this from arm and aarch64,
and tried to get it working without taking too much time. So If your
idea works as expected and better, and worth to do, then no problem,
no need to apologize though. I suggest try to keep the porting
similar to others as possible, so that other maintainers or people may
easier to help and also fix our port in the future, when they are
fixing their ports.
> 2. Relationship with Linker
>
> The rule: 'the first "$x" symbol in the section means instruction with
> ELF attributes' makes sense to me but may force the linker to do
> something more than the current implementation. This is because, the
> first "$x" symbol in an section *of an object file* might not be the
> first "$x" symbol in an section of the final (linked) object file.
>
> Both my RFC PATCH and your patchset (PATCH 2/2) might complicate the
> linker because of this property.
>
> They replaced the first "$x+arch" with "$x" in the section when the
> architecture matches ELF attributes in the assembler. However, not
> doing this like your commit 0ce50fc900a5 ("RISC-V: Always generate
> mapping symbols at the start of the sections.") might be an option (NOT
> as a workaround).
>
> If this option is acceptable to you, my new proposal on linker is either:
>
> (a) Do nothing (just like the old linker)
> It looks like a joke but I want to say that object files generated
> from your commit 0ce50fc900a5 is compatible with the old linker,
> assuming new "$x" interpretation rule. Also, multiple long
> "$x+arch" symbols do not increase the final file size that much
> (because the same symbol names are reused on disk).
> The description above is not strictly true when we consider about
> multi-toolchain object linking support but still applies on
> many cases.
Not a joke, this should be the best solution. Consider that there are
two objects, A and B, with rv32ic and rv32if, linker will merge their
file-level architectures (elf arch attributes) to rv32ifc as the
output file attribute. Therefore, the $x from A enables f, that's
work fine; But the $x from B enables c, and that conflicts with this
pr (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/116).
Personally I don't have plan to change too much in linker about this,
so (a) should be the easier way to do.
> (b) Replace "$x+arch" with "$x" if the implied architecture
> depending on the context is either:
> (1) The same as the final ELF attributes or
> (2) compatible with that.
>
> and an optional handling:
>
> (c) (Virtually/actually) replace first "$x" in the section with
> "$x+arch" with input ELF file's attribute (not output) when
> preprocessing input object files.
This will beed to store every input elf attributes, and
> Following combinations are possible:
>
> 1. None (a)
> 2. (c)
> 3. (b.1)
> 4. (b.1) + (c)
> 5. (b.2)
> 6. (b.2) + (c)
>
> I'm sorry to say I haven't read GNU ld code much and I'm not sure
> whether the options (b.{1,2}) are feasible.
>
> Option (c) is for much complex situation: multi-toolchain object linking
> support (e.g. a static library is built with an old toolchain without
> "$x+arch" but with "$x", application code is built with a new toolchain
> with "$x+arch" and we link them together with a new [or even newer] one
> supporting "$x+arch").
The compatible of toolchain is bound to conflict on this issue. The
old toolchains don't really have the concepts of mapping symbol with
architecture, so rebuilt the stuff to make things work make sense to
me.
>
> 3. Outside Binutils
>
> Current mapping symbol proposal:
> <https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/196>
> requires some updates because, in that proposal, all "$x" means "revert
> to default (as in ELF attributes)". So we need to talk to Kito about
> changing this proposal.
Agree, I will talk to him when he come back :)
Thanks
Nelson
>
> Thanks,
> Tsukasa
>
>
> >
> > gas/
> > * config/tc-riscv.h (struct riscv_segment_info_type): Added new boolean
> > `arch_changed', to record if the segment has more than one mapping symbol
> > with architecture. All segments should have at least one at their first
> > instruction.
> > * config/tc-riscv.c (riscv_mapping_state): Set `arch_changed' to true if
> > the segment has more than one mapping symbol with architecture.
> > (riscv_check_mapping_symbols): Suppress the mapping symbol with architecture
> > to $x if the section/segment only has this setting and the architecture is
> > same as the file-level one.
> > * testsuite/gas/riscv/mapping.s: Updated, and added new testcases,
> > .text.suppress, .text.suppress.push.pop and .text.no.suppress.
> > * testsuite/gas/riscv/mapping-dis.d: Updated.
> > * testsuite/gas/riscv/mapping-symbols.d: Likewise.
> > * testsuite/gas/riscv/mapping-attr.d: New testcase to see if the file-level
> > architecure is polluted by other section-level .option settings.
> > opcodes/
> > * riscv-dis.c (riscv_file_subsets): Added to record the file-level subsets.
> > (riscv_rps_dis): Delay to set subset_list in riscv_get_disassembler or
> > riscv_get_map_state.
> > (from_last_map_symbol): Moved from riscv_search_mapping_symbol and make it
> > global. Set to false means we are dumping a new section.
> > (riscv_get_map_state): Switch riscv_rps_dis.subset_list to riscv_file_subsets
> > if the mapping symbol is $x, and we are dumping a new section. Otherwise,
> > set it to riscv_subsets, and then parsing the architecture from $x+arch.
> > Besides, add new boolean parameter `update', since we don't need to update
> > the architecture when called from riscv_data_length.
> > (riscv_search_mapping_symbol): Updated.
> > (riscv_get_disassembler): Set the riscv_rps_dis.subset_list to riscv_file_subsets.
> >
> > Co-developed-by: Nelson Chu <nelson@rivosinc.com>
> > ---
> > gas/config/tc-riscv.c | 13 ++++++++
> > gas/config/tc-riscv.h | 1 +
> > gas/testsuite/gas/riscv/mapping-attr.d | 8 +++++
> > gas/testsuite/gas/riscv/mapping-dis.d | 23 +++++++++++--
> > gas/testsuite/gas/riscv/mapping-symbols.d | 11 ++++++-
> > gas/testsuite/gas/riscv/mapping.s | 35 +++++++++++++++++---
> > opcodes/riscv-dis.c | 39 +++++++++++++++--------
> > 7 files changed, 110 insertions(+), 20 deletions(-)
> > create mode 100644 gas/testsuite/gas/riscv/mapping-attr.d
> >
> > diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> > index dcb70f9d7ad..56ece8b2dc3 100644
> > --- a/gas/config/tc-riscv.c
> > +++ b/gas/config/tc-riscv.c
> > @@ -598,6 +598,7 @@ riscv_mapping_state (enum riscv_seg_mstate to_state,
> > S_GET_NAME (seg_arch_symbol) + 2) != 0)
> > {
> > reset_seg_arch_str = true;
> > + seg_info (now_seg)->tc_segment_info_data.arch_changed = true;
> > }
> > else if (from_state == to_state)
> > return;
> > @@ -639,6 +640,18 @@ riscv_check_mapping_symbols (bfd *abfd ATTRIBUTE_UNUSED,
> > if (seginfo == NULL || seginfo->frchainP == NULL)
> > return;
> >
> > + /* If the section only has a mapping symbol at the first instruction,
> > + and it is the same as the file-level architecture, then no need to
> > + add $x+arch for it, just add $x is enough.
> > +
> > + Please see gas/testsuite/gas/riscv/mapping.s: <your testcase
> > + section name>. */
> > + symbolS *arch_map_symbol = seginfo->tc_segment_info_data.arch_map_symbol;
> > + if (!seginfo->tc_segment_info_data.arch_changed
> > + && arch_map_symbol != 0
> > + && strcmp (riscv_file_arch, S_GET_NAME (arch_map_symbol) + 2) == 0)
> > + S_SET_NAME (arch_map_symbol, "$x");
> > +
> > for (fragp = seginfo->frchainP->frch_root;
> > fragp != NULL;
> > fragp = fragp->fr_next)
> > diff --git a/gas/config/tc-riscv.h b/gas/config/tc-riscv.h
> > index 19c45ba2d12..0dc567e1665 100644
> > --- a/gas/config/tc-riscv.h
> > +++ b/gas/config/tc-riscv.h
> > @@ -140,6 +140,7 @@ struct riscv_segment_info_type
> > enum riscv_seg_mstate map_state;
> > /* The current mapping symbol with architecture string. */
> > symbolS *arch_map_symbol;
> > + bool arch_changed;
> > };
> >
> > /* Define target fragment type. */
> > diff --git a/gas/testsuite/gas/riscv/mapping-attr.d b/gas/testsuite/gas/riscv/mapping-attr.d
> > new file mode 100644
> > index 00000000000..71bcebf17d4
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/mapping-attr.d
> > @@ -0,0 +1,8 @@
> > +#as: -misa-spec=20191213
> > +#source: mapping.s
> > +#readelf: -A
> > +
> > +Attribute Section: riscv
> > +File Attributes
> > + Tag_RISCV_arch: "rv32i2p1_f2p2_c2p0_zicsr2p0"
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/mapping-dis.d b/gas/testsuite/gas/riscv/mapping-dis.d
> > index b1a26fbd151..fe581996e3c 100644
> > --- a/gas/testsuite/gas/riscv/mapping-dis.d
> > +++ b/gas/testsuite/gas/riscv/mapping-dis.d
> > @@ -8,8 +8,8 @@
> > Disassembly of section .text.cross.section.A:
> >
> > 0+000 <funcA>:
> > -[ ]+[0-9a-f]+:[ ]+4505[ ]+li[ ]+a0,1
> > -[ ]+[0-9a-f]+:[ ]+bffd[ ]+j[ ]+0 <funcA>
> > +[ ]+[0-9a-f]+:[ ]+00100513[ ]+li[ ]+a0,1
> > +[ ]+[0-9a-f]+:[ ]+bff5[ ]+j[ ]+0 <funcA>
> >
> > Disassembly of section .text.corss.section.B:
> >
> > @@ -91,3 +91,22 @@ Disassembly of section .text.relax.align:
> > [ ]+[0-9a-f]+:[ ]+00000013[ ]+nop
> > [ ]+[0-9a-f]+:[ ]+00200513[ ]+li[ ]+a0,2
> > [ ]+[0-9a-f]+:[ ]+00000013[ ]+nop
> > +
> > +Disassembly of section .text.suppress:
> > +
> > +0+000 <.text.suppress>:
> > +[ ]+[0-9a-f]+:[ ]+003022f3[ ]+frcsr[ ]+t0
> > +[ ]+[0-9a-f]+:[ ]+4505[ ]+li[ ]+a0,1
> > +
> > +Disassembly of section .text.suppress.push.pop:
> > +
> > +0+000 <.text.suppress.push.pop>:
> > +[ ]+[0-9a-f]+:[ ]+003022f3[ ]+frcsr[ ]+t0
> > +[ ]+[0-9a-f]+:[ ]+4505[ ]+li[ ]+a0,1
> > +
> > +Disassembly of section .text.no.suppress:
> > +
> > +0+000 <.text.no.suppress>:
> > +[ ]+[0-9a-f]+:[ ]+003022f3[ ]+frcsr[ ]+t0
> > +[ ]+[0-9a-f]+:[ ]+003022f3[ ]+csrr[ ]+t0,fcsr
> > +[ ]+[0-9a-f]+:[ ]+003022f3[ ]+frcsr[ ]+t0
> > diff --git a/gas/testsuite/gas/riscv/mapping-symbols.d b/gas/testsuite/gas/riscv/mapping-symbols.d
> > index 40df3409736..db69c05bfa4 100644
> > --- a/gas/testsuite/gas/riscv/mapping-symbols.d
> > +++ b/gas/testsuite/gas/riscv/mapping-symbols.d
> > @@ -9,7 +9,8 @@ SYMBOL TABLE:
> > 0+00 l d .data 0+00 .data
> > 0+00 l d .bss 0+00 .bss
> > 0+00 l d .text.cross.section.A 0+00 .text.cross.section.A
> > -0+00 l .text.cross.section.A 0+00 \$xrv32i2p1_c2p0
> > +0+00 l .text.cross.section.A 0+00 \$xrv32i2p1
> > +0+04 l .text.cross.section.A 0+00 \$xrv32i2p1_c2p0
> > 0+00 l d .text.corss.section.B 0+00 .text.corss.section.B
> > 0+00 l .text.corss.section.B 0+00 \$xrv32i2p1_c2p0
> > 0+02 l .text.corss.section.B 0+00 \$xrv32i2p1
> > @@ -42,6 +43,14 @@ SYMBOL TABLE:
> > 0+00 l d .text.relax.align 0+00 .text.relax.align
> > 0+00 l .text.relax.align 0+00 \$xrv32i2p1_c2p0
> > 0+08 l .text.relax.align 0+00 \$xrv32i2p1
> > +0+00 l d .text.suppress 0+00 .text.suppress
> > +0+00 l .text.suppress 0+00 \$x
> > +0+00 l d .text.suppress.push.pop 0+00 .text.suppress.push.pop
> > +0+00 l .text.suppress.push.pop 0+00 \$x
> > +0+00 l d .text.no.suppress 0+00 .text.no.suppress
> > +0+00 l .text.no.suppress 0+00 \$xrv32i2p1_f2p2_c2p0_zicsr2p0
> > +0+04 l .text.no.suppress 0+00 \$xrv32i2p1_c2p0_zicsr2p0
> > +0+08 l .text.no.suppress 0+00 \$xrv32i2p1_f2p2_c2p0_zicsr2p0
> > 0+0a l .text.section.padding 0+00 \$x
> > 0+03 l .text.odd.align.start.insn 0+00 \$d
> > 0+04 l .text.odd.align.start.insn 0+00 \$x
> > diff --git a/gas/testsuite/gas/riscv/mapping.s b/gas/testsuite/gas/riscv/mapping.s
> > index 3014a69e792..5041d4d4cd4 100644
> > --- a/gas/testsuite/gas/riscv/mapping.s
> > +++ b/gas/testsuite/gas/riscv/mapping.s
> > @@ -1,10 +1,11 @@
> > -.attribute arch, "rv32ic"
> > +.attribute arch, "rv32ifc"
> > .option norelax # FIXME: assembler fill the paddings after parsing everything,
> > # so we probably won't fill anything for the norelax region when
> > # the riscv_opts.relax is enabled at somewhere.
> >
> > .section .text.cross.section.A, "ax"
> > .option push
> > +.option arch, rv32i
> > .global funcA
> > funcA:
> > addi a0, zero, 1 # rv32i
> > @@ -20,6 +21,7 @@ j funcB # rv32i
> >
> > .section .text.data, "ax"
> > .option push
> > +.option arch, rv32ic
> > .word 0 # $d
> > .long 1
> > addi a0, zero, 1 # rv32ic
> > @@ -35,7 +37,7 @@ addi a0, zero, 2 # $x, but same as previous addi, so removed
> > .section .text.odd.align.start.insn, "ax"
> > .option push
> > .option norelax
> > -.option arch, +c
> > +.option arch, rv32ic
> > addi a0, zero, 1 # $xrv32ic
> > .byte 1 # $d
> > .option arch, -c
> > @@ -46,7 +48,7 @@ addi a0, zero, 2 # $xrv32i
> > .section .text.odd.align.start.data, "ax"
> > .option push
> > .option norelax
> > -.option arch, +c
> > +.option arch, rv32ic
> > .byte 1 # $d
> > .align 2 # odd alignment, $xrv32ic replaced by $d + $xrv32ic
> > addi a0, zero, 1
> > @@ -55,6 +57,7 @@ addi a0, zero, 1
> > .section .text.zero.fill.first, "ax"
> > .option push
> > .option norelax
> > +.option arch, rv32ic
> > .fill 1, 0, 0 # $d with zero size, removed in make_mapping_symbol
> > addi a0, zero, 1 # $xrv32ic
> > .option pop
> > @@ -62,6 +65,7 @@ addi a0, zero, 1 # $xrv32ic
> > .section .text.zero.fill.last, "ax"
> > .option push
> > .option norelax
> > +.option arch, rv32ic
> > addi a0, zero, 1 # $xrv32ic
> > .fill 1, 0, 0 # $d with zero size, removed in make_mapping_symbol
> > addi a0, zero, 2 # $x, FIXME: need find a way to remove?
> > @@ -71,6 +75,7 @@ addi a0, zero, 2 # $x, FIXME: need find a way to remove?
> > .section .text.zero.fill.align.A, "ax"
> > .option push
> > .option norelax
> > +.option arch, rv32ic
> > .align 2 # $xrv32ic, .align and .fill are in the different frag, so neither be removed
> > .fill 1, 0, 0 # $d with zero size, removed in make_mapping_symbol when adding $xrv32ic
> > addi a0, zero, 1 # $x, should be removed in riscv_check_mapping_symbols
> > @@ -81,6 +86,7 @@ addi a0, zero, 2
> > .section .text.zero.fill.align.B, "ax"
> > .option push
> > .option norelax
> > +.option arch, rv32ic
> > .align 2 # $xrv32ic, .align and .fill are in the different frag, so neither be removed,
> > # but will be removed in riscv_check_mapping_symbols
> > .fill 1, 0, 0 # $d with zero size, removed in make_mapping_symbol when adding $xrv32ic
> > @@ -92,7 +98,7 @@ addi a0, zero, 2
> > .section .text.last.section, "ax"
> > .option push
> > .option norelax
> > -.option arch, -c
> > +.option arch, rv32i
> > addi a0, zero, 1 # $xrv32i
> > .word 1 # $d
> > .align 2 # zero section padding, $x at the end of section, removed in riscv_check_mapping_symbols
> > @@ -101,6 +107,7 @@ addi a0, zero, 1 # $xrv32i
> > .section .text.section.padding, "ax"
> > .option push
> > .option norelax
> > +.option arch, rv32ic
> > .align 2
> > addi a0, zero, 1 # $rv32ic
> > .option arch, +a
> > @@ -119,3 +126,23 @@ addi a0, zero, 1 # $x, won't added
> > .align 3 # $x, won't added
> > addi a0, zero, 2 # $xrv32i
> > .option pop
> > +
> > +.section .text.suppress, "ax"
> > +csrrs t0, fcsr, zero # suppress $xrv32ifc to $x
> > +addi a0, zero, 1
> > +
> > +.section .text.suppress.push.pop, "ax"
> > +.option push
> > +.option arch, rv32i
> > +.option arch, +f, +c
> > +csrrs t0, fcsr, zero # suppress $xrv32ifc to $x
> > +addi a0, zero, 1
> > +.option pop
> > +
> > +.section .text.no.suppress, "ax"
> > +csrrs t0, fcsr, zero # $xrv32ifc, since not only one $x+arch, so don't suppress
> > +.option push
> > +.option arch, -f
> > +csrrs t0, fcsr, zero # $xrv32ic
> > +.option pop
> > +csrrs t0, fcsr, zero # $xrv32ifc, likewise
> > diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> > index 3a31647a2f8..f935490c64f 100644
> > --- a/opcodes/riscv-dis.c
> > +++ b/opcodes/riscv-dis.c
> > @@ -42,10 +42,11 @@ static enum riscv_spec_class default_isa_spec = ISA_SPEC_CLASS_DRAFT - 1;
> > (as specified by the ELF attributes or the `priv-spec' option). */
> > static enum riscv_spec_class default_priv_spec = PRIV_SPEC_CLASS_NONE;
> >
> > +static riscv_subset_list_t riscv_file_subsets;
> > static riscv_subset_list_t riscv_subsets;
> > static riscv_parse_subset_t riscv_rps_dis =
> > {
> > - &riscv_subsets, /* subset_list. */
> > + NULL, /* subset_list. */
> > opcodes_error_handler,/* error_handler. */
> > &xlen, /* xlen. */
> > &default_isa_spec, /* isa_spec. */
> > @@ -64,6 +65,7 @@ struct riscv_private_data
> > /* Used for mapping symbols. */
> > static int last_map_symbol = -1;
> > static bfd_vma last_stop_offset = 0;
> > +static bool from_last_map_symbol = false;
> >
> > /* Register names as used by the disassembler. */
> > static const char * const *riscv_gpr_names;
> > @@ -821,7 +823,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
> > static bool
> > riscv_get_map_state (int n,
> > enum riscv_seg_mstate *state,
> > - struct disassemble_info *info)
> > + struct disassemble_info *info,
> > + bool update)
> > {
> > const char *name;
> >
> > @@ -831,15 +834,25 @@ riscv_get_map_state (int n,
> > return false;
> >
> > name = bfd_asymbol_name(info->symtab[n]);
> > - if (strcmp (name, "$x") == 0)
> > - *state = MAP_INSN;
> > - else if (strcmp (name, "$d") == 0)
> > + if (strcmp (name, "$d") == 0)
> > *state = MAP_DATA;
> > + else if (strcmp (name, "$x") == 0)
> > + {
> > + *state = MAP_INSN;
> > + /* Switch back to file-level architecture when starting from
> > + a new section. */
> > + if (update && !from_last_map_symbol)
> > + riscv_rps_dis.subset_list = &riscv_file_subsets;
> > + }
> > else if (strncmp (name, "$xrv", 4) == 0)
> > {
> > *state = MAP_INSN;
> > - riscv_release_subset_list (&riscv_subsets);
> > - riscv_parse_subset (&riscv_rps_dis, name + 2);
> > + if (update)
> > + {
> > + riscv_rps_dis.subset_list = &riscv_subsets;
> > + riscv_release_subset_list (riscv_rps_dis.subset_list);
> > + riscv_parse_subset (&riscv_rps_dis, name + 2);
> > + }
> > }
> > else
> > return false;
> > @@ -855,7 +868,6 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
> > struct disassemble_info *info)
> > {
> > enum riscv_seg_mstate mstate;
> > - bool from_last_map_symbol;
> > bool found = false;
> > int symbol = -1;
> > int n;
> > @@ -872,7 +884,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
> > || bfd_asymbol_flavour (*info->symtab) != bfd_target_elf_flavour)
> > return mstate;
> >
> > - /* Reset the last_map_symbol if we start to dump a new section. */
> > + /* Reset the last_map_symbol when pc <= 0. */
> > if (memaddr <= 0)
> > last_map_symbol = -1;
> >
> > @@ -895,7 +907,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
> > /* We have searched all possible symbols in the range. */
> > if (addr > memaddr)
> > break;
> > - if (riscv_get_map_state (n, &mstate, info))
> > + if (riscv_get_map_state (n, &mstate, info, true/* update */))
> > {
> > symbol = n;
> > found = true;
> > @@ -922,7 +934,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
> > if (addr < (info->section ? info->section->vma : 0))
> > break;
> > /* Stop searching once we find the closed mapping symbol. */
> > - if (riscv_get_map_state (n, &mstate, info))
> > + if (riscv_get_map_state (n, &mstate, info, true/* update */))
> > {
> > symbol = n;
> > found = true;
> > @@ -958,7 +970,7 @@ riscv_data_length (bfd_vma memaddr,
> > {
> > bfd_vma addr = bfd_asymbol_value (info->symtab[n]);
> > if (addr > memaddr
> > - && riscv_get_map_state (n, &m, info))
> > + && riscv_get_map_state (n, &m, info, false/* update */))
> > {
> > if (addr - memaddr < length)
> > length = addr - memaddr;
> > @@ -1106,7 +1118,8 @@ riscv_get_disassembler (bfd *abfd)
> > }
> > }
> >
> > - riscv_release_subset_list (&riscv_subsets);
> > + riscv_rps_dis.subset_list = &riscv_file_subsets;
> > + riscv_release_subset_list (riscv_rps_dis.subset_list);
> > riscv_parse_subset (&riscv_rps_dis, default_arch);
> > return print_insn_riscv;
> > }
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-11-07 2:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-04 11:50 [PATCH 1/2] RISC-V: File-level architecture shouldn't be affected by section-level ones Nelson Chu
2022-11-04 11:50 ` [PATCH 2/2] RISC-V: Clarify the suppress rule of mapping symbol with architecture string Nelson Chu
2022-11-05 10:47 ` Tsukasa OI
2022-11-07 2:37 ` Nelson Chu
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).