RISC-V: PR32014, .option directives shuoldn't affect elf attribute.
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_binutils_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-arm |
success
|
Test passed
|
Commit Message
Since .option arch/rvc/norvc/push/pop are function-level architecture setting,
they should affect the mapping symbols only rather than file-level elf
architecture attribute. Otherwise, the elf architecture attribute will appear
to missing some extensions when -flto merges files with different .option
architecture settings.
bfd/
* elfxx-riscv.h (riscv_subset_list_t): Renamed arch_str to
func_arch_str, to represent it's function-level.
* elfxx-riscv.c (riscv_release_subset_list): Updated since arch_str
renamed to func_arch_str.
(riscv_copy_subset_list): Likewise.
gas/
* config/tc-riscv.c (file_arch_str): New const char *, rather than the
func_arch_str in the riscv_rps_as.subset_list, it's file-level so only
be affected by .attribute arch directive.
(riscv_reset_subsets_list_arch_str): Renamed to riscv_set_arch_str,
and also can handle both file_arch_str and func_arch_str, just give
the pointer address as the input.
(riscv_set_arch): Called by -march and .attribute arch, so set both
file_arch_str and func_arch_str.
(riscv_mapping_state): Updated since arch_str renamed to func_arch_str.
(s_riscv_option): Updated .option arch/rvc/norvc/push/pop that only
set the func_arch_str.
(riscv_write_out_attrs): Output elf architecture attribute according to
file_arch_str. Freed file_arch_str.
* testsuite/gas/riscv/option-arch.s: From option-arch-01/02/03 merged.
* testsuite/gas/riscv/option-arch-dis.d: Likewise, for dis-assembler.
* testsuite/gas/riscv/option-arch-attr.d: Likewise, to check readelf -A.
---
bfd/elfxx-riscv.c | 8 ++---
bfd/elfxx-riscv.h | 2 +-
gas/config/tc-riscv.c | 34 +++++++++++--------
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} | 3 +-
gas/testsuite/gas/riscv/option-arch-dis.d | 26 ++++++++++++++
gas/testsuite/gas/riscv/option-arch.s | 11 ++++++
12 files changed, 62 insertions(+), 73 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} (77%)
create mode 100644 gas/testsuite/gas/riscv/option-arch-dis.d
create mode 100644 gas/testsuite/gas/riscv/option-arch.s
Comments
Just one minor comment from me:
maybe scoped_arch_str rather than func_arch_str since `.option arch`
not really tied to function and it may also apply on a small scope
only (like some usage of rvc/norvc)
Otherwise it's LGTM :)
On Thu, Aug 1, 2024 at 8:54 AM Nelson Chu <nelson@rivosinc.com> wrote:
>
> Since .option arch/rvc/norvc/push/pop are function-level architecture setting,
> they should affect the mapping symbols only rather than file-level elf
> architecture attribute. Otherwise, the elf architecture attribute will appear
> to missing some extensions when -flto merges files with different .option
> architecture settings.
>
> bfd/
> * elfxx-riscv.h (riscv_subset_list_t): Renamed arch_str to
> func_arch_str, to represent it's function-level.
> * elfxx-riscv.c (riscv_release_subset_list): Updated since arch_str
> renamed to func_arch_str.
> (riscv_copy_subset_list): Likewise.
> gas/
> * config/tc-riscv.c (file_arch_str): New const char *, rather than the
> func_arch_str in the riscv_rps_as.subset_list, it's file-level so only
> be affected by .attribute arch directive.
> (riscv_reset_subsets_list_arch_str): Renamed to riscv_set_arch_str,
> and also can handle both file_arch_str and func_arch_str, just give
> the pointer address as the input.
> (riscv_set_arch): Called by -march and .attribute arch, so set both
> file_arch_str and func_arch_str.
> (riscv_mapping_state): Updated since arch_str renamed to func_arch_str.
> (s_riscv_option): Updated .option arch/rvc/norvc/push/pop that only
> set the func_arch_str.
> (riscv_write_out_attrs): Output elf architecture attribute according to
> file_arch_str. Freed file_arch_str.
> * testsuite/gas/riscv/option-arch.s: From option-arch-01/02/03 merged.
> * testsuite/gas/riscv/option-arch-dis.d: Likewise, for dis-assembler.
> * testsuite/gas/riscv/option-arch-attr.d: Likewise, to check readelf -A.
> ---
> bfd/elfxx-riscv.c | 8 ++---
> bfd/elfxx-riscv.h | 2 +-
> gas/config/tc-riscv.c | 34 +++++++++++--------
> 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} | 3 +-
> gas/testsuite/gas/riscv/option-arch-dis.d | 26 ++++++++++++++
> gas/testsuite/gas/riscv/option-arch.s | 11 ++++++
> 12 files changed, 62 insertions(+), 73 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} (77%)
> create mode 100644 gas/testsuite/gas/riscv/option-arch-dis.d
> create mode 100644 gas/testsuite/gas/riscv/option-arch.s
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index b8f314d1c01..741b8148725 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -1826,10 +1826,10 @@ riscv_release_subset_list (riscv_subset_list_t *subset_list)
>
> subset_list->tail = NULL;
>
> - if (subset_list->arch_str != NULL)
> + if (subset_list->func_arch_str != NULL)
> {
> - free ((void*) subset_list->arch_str);
> - subset_list->arch_str = NULL;
> + free ((void*) subset_list->func_arch_str);
> + subset_list->func_arch_str = NULL;
> }
> }
>
> @@ -2341,7 +2341,7 @@ riscv_copy_subset_list (riscv_subset_list_t *subset_list)
> {
> riscv_subset_list_t *new = xmalloc (sizeof *new);
> new->head = riscv_copy_subset (new, subset_list->head);
> - new->arch_str = strdup (subset_list->arch_str);
> + new->func_arch_str = strdup (subset_list->func_arch_str);
> return new;
> }
>
> diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> index 49be71746b9..402a50cd836 100644
> --- a/bfd/elfxx-riscv.h
> +++ b/bfd/elfxx-riscv.h
> @@ -64,7 +64,7 @@ typedef struct
> {
> riscv_subset_t *head;
> riscv_subset_t *tail;
> - const char *arch_str;
> + const char *func_arch_str;
> } riscv_subset_list_t;
>
> extern void
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index c09bd429bab..201e7bd0ee8 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -162,6 +162,7 @@ struct riscv_ip_error
>
> static const char default_arch[] = DEFAULT_ARCH;
> static const char *default_arch_with_ext = DEFAULT_RISCV_ARCH_WITH_EXT;
> +static const char *file_arch_str = NULL;
> static enum riscv_spec_class default_isa_spec = ISA_SPEC_CLASS_NONE;
> static enum riscv_spec_class default_priv_spec = PRIV_SPEC_CLASS_NONE;
>
> @@ -305,15 +306,17 @@ static riscv_parse_subset_t riscv_rps_as =
> true, /* check_unknown_prefixed_ext. */
> };
>
> -/* Update the architecture string in the subset_list. */
> +/* Update file/function-level architecture string according to the
> + subset_list. */
>
> static void
> -riscv_reset_subsets_list_arch_str (void)
> +riscv_set_arch_str (const char **arch_str_p)
> {
> 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);
> + const char *arch_str = *arch_str_p;
> + if (arch_str != NULL)
> + free ((void *) arch_str);
> + *arch_str_p = riscv_arch_str (xlen, subsets);
> }
>
> /* This structure is used to hold a stack of .option values. */
> @@ -343,11 +346,12 @@ riscv_set_arch (const char *s)
> riscv_rps_as.subset_list = XNEW (riscv_subset_list_t);
> riscv_rps_as.subset_list->head = NULL;
> riscv_rps_as.subset_list->tail = NULL;
> - riscv_rps_as.subset_list->arch_str = NULL;
> + riscv_rps_as.subset_list->func_arch_str = NULL;
> }
> riscv_release_subset_list (riscv_rps_as.subset_list);
> riscv_parse_subset (&riscv_rps_as, s);
> - riscv_reset_subsets_list_arch_str ();
> + riscv_set_arch_str (&file_arch_str);
> + riscv_set_arch_str (&riscv_rps_as.subset_list->func_arch_str);
>
> riscv_set_rvc (false);
> if (riscv_subset_supports (&riscv_rps_as, "c")
> @@ -612,7 +616,7 @@ riscv_mapping_state (enum riscv_seg_mstate to_state,
> else if (seg_arch_symbol != 0
> && to_state == MAP_INSN
> && !fr_align_code
> - && strcmp (riscv_rps_as.subset_list->arch_str,
> + && strcmp (riscv_rps_as.subset_list->func_arch_str,
> S_GET_NAME (seg_arch_symbol) + 2) != 0)
> {
> reset_seg_arch_str = true;
> @@ -623,7 +627,7 @@ riscv_mapping_state (enum riscv_seg_mstate to_state,
> valueT value = (valueT) (frag_now_fix () - max_chars);
> seg_info (now_seg)->tc_segment_info_data.map_state = to_state;
> const char *arch_str = reset_seg_arch_str
> - ? riscv_rps_as.subset_list->arch_str : NULL;
> + ? riscv_rps_as.subset_list->func_arch_str : NULL;
> make_mapping_symbol (to_state, value, frag_now, arch_str,
> false/* odd_data_padding */);
> }
> @@ -4811,13 +4815,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_set_arch_str (&riscv_rps_as.subset_list->func_arch_str);
> riscv_set_rvc (true);
> }
> else if (strcmp (name, "norvc") == 0)
> {
> riscv_update_subset (&riscv_rps_as, "-c");
> - riscv_reset_subsets_list_arch_str ();
> + riscv_set_arch_str (&riscv_rps_as.subset_list->func_arch_str);
> riscv_set_rvc (false);
> }
> else if (strcmp (name, "pic") == 0)
> @@ -4838,7 +4842,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_set_arch_str (&riscv_rps_as.subset_list->func_arch_str);
>
> riscv_set_rvc (false);
> if (riscv_subset_supports (&riscv_rps_as, "c")
> @@ -5363,7 +5367,7 @@ s_riscv_insn (int x ATTRIBUTE_UNUSED)
> static void
> riscv_write_out_attrs (void)
> {
> - const char *arch_str, *priv_str, *p;
> + const char *priv_str, *p;
> /* versions[0]: major version.
> versions[1]: minor version.
> versions[2]: revision version. */
> @@ -5371,10 +5375,10 @@ riscv_write_out_attrs (void)
> unsigned int i;
>
> /* Re-write architecture elf attribute. */
> - arch_str = riscv_rps_as.subset_list->arch_str;
> - if (!bfd_elf_add_proc_attr_string (stdoutput, Tag_RISCV_arch, arch_str))
> + if (!bfd_elf_add_proc_attr_string (stdoutput, Tag_RISCV_arch, file_arch_str))
> as_fatal (_("error adding attribute: %s"),
> bfd_errmsg (bfd_get_error ()));
> + free ((void *) file_arch_str);
>
> /* For the file without any instruction, we don't set the default_priv_spec
> according to the privileged elf attributes since the md_assemble isn't
> 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 77%
> rename from gas/testsuite/gas/riscv/option-arch-01b.d
> rename to gas/testsuite/gas/riscv/option-arch-attr.d
> index 8f4284d5f15..8c1f6658796 100644
> --- a/gas/testsuite/gas/riscv/option-arch-01b.d
> +++ b/gas/testsuite/gas/riscv/option-arch-attr.d
> @@ -1,8 +1,7 @@
> #as: -misa-spec=2.2
> +#source: option-arch.s
> #readelf: -A
> -#source: option-arch-01.s
>
> Attribute Section: riscv
> File Attributes
> Tag_RISCV_arch: "rv64i2p0_c2p0"
> -#...
> 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..6768fe72806
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/option-arch-dis.d
> @@ -0,0 +1,26 @@
> +#as: -misa-spec=2.2
> +#source: option-arch.s
> +#objdump: -d --syms --special-syms
> +
> +.*:[ ]+file format .*
> +
> +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 \$xrv64i2p0_c2p0
> +0+02 l .text 0+00 \$xrv64i2p0_f2p0_d2p0_xvendor1p0
> +0+0a l .text 0+00 \$xrv64i2p0_m3p0_f2p0_d3p0_c2p0_zmmul1p0_xvendor32x3p0
> +0+0c l .text 0+00 \$xrv32i2p1_c2p0
> +0+00 l d .riscv.attributes 0+00 .riscv.attributes
> +
> +
> +Disassembly of section .text:
> +
> +0+000 <.text>:
> +[ ]+0:[ ]+952e[ ]+add[ ]+a0,a0,a1
> +[ ]+2:[ ]+00b50533[ ]+add[ ]+a0,a0,a1
> +[ ]+6:[ ]+00302573[ ]+frcsr[ ]+a0
> +[ ]+a:[ ]+952e[ ]+add[ ]+a0,a0,a1
> +[ ]+c:[ ]+c8002573[ ]+.insn[ ]+4, 0xc8002573
> +#...
> diff --git a/gas/testsuite/gas/riscv/option-arch.s b/gas/testsuite/gas/riscv/option-arch.s
> new file mode 100644
> index 00000000000..4d2d261684a
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/option-arch.s
> @@ -0,0 +1,11 @@
> +.attribute arch, "rv64ic" # file-level, rv64ic
> +add a0, a0, a1
> +.option push
> +.option arch, +d2p0, -c, +xvendor1p0
> +add a0, a0, a1 # func-level, rv64i_d2p0_xvendor1p0
> +frcsr a0
> +.option pop
> +.option arch, +m3p0, +d3p0, +xvendor32x3p0
> +add a0, a0, a1 # func-level, rv64i_m3p0_d3p0_c_xvendor32x3p0
> +.option arch, rv32i2p1c2p0 # FIXME: maybe we should adjust xlen in dis-assembler according to mappin symbols?
> +rdcycleh a0 # func-level, rv32i2p1_c2p0
> --
> 2.39.3 (Apple Git-146)
>
On 01.08.2024 02:51, Nelson Chu wrote:
> Since .option arch/rvc/norvc/push/pop are function-level architecture setting,
> they should affect the mapping symbols only rather than file-level elf
> architecture attribute. Otherwise, the elf architecture attribute will appear
> to missing some extensions when -flto merges files with different .option
> architecture settings.
>
> bfd/
> * elfxx-riscv.h (riscv_subset_list_t): Renamed arch_str to
> func_arch_str, to represent it's function-level.
> * elfxx-riscv.c (riscv_release_subset_list): Updated since arch_str
> renamed to func_arch_str.
> (riscv_copy_subset_list): Likewise.
> gas/
> * config/tc-riscv.c (file_arch_str): New const char *, rather than the
> func_arch_str in the riscv_rps_as.subset_list, it's file-level so only
> be affected by .attribute arch directive.
> (riscv_reset_subsets_list_arch_str): Renamed to riscv_set_arch_str,
> and also can handle both file_arch_str and func_arch_str, just give
> the pointer address as the input.
> (riscv_set_arch): Called by -march and .attribute arch, so set both
> file_arch_str and func_arch_str.
> (riscv_mapping_state): Updated since arch_str renamed to func_arch_str.
> (s_riscv_option): Updated .option arch/rvc/norvc/push/pop that only
> set the func_arch_str.
> (riscv_write_out_attrs): Output elf architecture attribute according to
> file_arch_str. Freed file_arch_str.
> * testsuite/gas/riscv/option-arch.s: From option-arch-01/02/03 merged.
> * testsuite/gas/riscv/option-arch-dis.d: Likewise, for dis-assembler.
> * testsuite/gas/riscv/option-arch-attr.d: Likewise, to check readelf -A.
> ---
> bfd/elfxx-riscv.c | 8 ++---
> bfd/elfxx-riscv.h | 2 +-
> gas/config/tc-riscv.c | 34 +++++++++++--------
> 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} | 3 +-
> gas/testsuite/gas/riscv/option-arch-dis.d | 26 ++++++++++++++
> gas/testsuite/gas/riscv/option-arch.s | 11 ++++++
> 12 files changed, 62 insertions(+), 73 deletions(-)
Any chance the (intended) behavior could also be added to gas/docs/c-riscv.texi?
Even if only to (explicitly) say that .option doesn't affect the attributes?
Jan
>
> Any chance the (intended) behavior could also be added to
> gas/docs/c-riscv.texi?
> Even if only to (explicitly) say that .option doesn't affect the
> attributes?
>
Oops, I forgot it... Yeah it's good to mention this in the gas/doc at
least, so everyone who sees the document should be noticed. I tried to
add some descriptions there in v2 patch, but since I'm not a native English
speaker, just feel free to correct me if there's something wrong or not
good enough. Thanks :-)
On Thu, Aug 1, 2024 at 9:32 AM Kito Cheng <kito.cheng@gmail.com> wrote:
> Just one minor comment from me:
>
> maybe scoped_arch_str rather than func_arch_str since `.option arch`
> not really tied to function and it may also apply on a small scope
> only (like some usage of rvc/norvc)
>
Make sense, updated to the v2 patch. Thanks :-)
Nelson
@@ -1826,10 +1826,10 @@ riscv_release_subset_list (riscv_subset_list_t *subset_list)
subset_list->tail = NULL;
- if (subset_list->arch_str != NULL)
+ if (subset_list->func_arch_str != NULL)
{
- free ((void*) subset_list->arch_str);
- subset_list->arch_str = NULL;
+ free ((void*) subset_list->func_arch_str);
+ subset_list->func_arch_str = NULL;
}
}
@@ -2341,7 +2341,7 @@ riscv_copy_subset_list (riscv_subset_list_t *subset_list)
{
riscv_subset_list_t *new = xmalloc (sizeof *new);
new->head = riscv_copy_subset (new, subset_list->head);
- new->arch_str = strdup (subset_list->arch_str);
+ new->func_arch_str = strdup (subset_list->func_arch_str);
return new;
}
@@ -64,7 +64,7 @@ typedef struct
{
riscv_subset_t *head;
riscv_subset_t *tail;
- const char *arch_str;
+ const char *func_arch_str;
} riscv_subset_list_t;
extern void
@@ -162,6 +162,7 @@ struct riscv_ip_error
static const char default_arch[] = DEFAULT_ARCH;
static const char *default_arch_with_ext = DEFAULT_RISCV_ARCH_WITH_EXT;
+static const char *file_arch_str = NULL;
static enum riscv_spec_class default_isa_spec = ISA_SPEC_CLASS_NONE;
static enum riscv_spec_class default_priv_spec = PRIV_SPEC_CLASS_NONE;
@@ -305,15 +306,17 @@ static riscv_parse_subset_t riscv_rps_as =
true, /* check_unknown_prefixed_ext. */
};
-/* Update the architecture string in the subset_list. */
+/* Update file/function-level architecture string according to the
+ subset_list. */
static void
-riscv_reset_subsets_list_arch_str (void)
+riscv_set_arch_str (const char **arch_str_p)
{
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);
+ const char *arch_str = *arch_str_p;
+ if (arch_str != NULL)
+ free ((void *) arch_str);
+ *arch_str_p = riscv_arch_str (xlen, subsets);
}
/* This structure is used to hold a stack of .option values. */
@@ -343,11 +346,12 @@ riscv_set_arch (const char *s)
riscv_rps_as.subset_list = XNEW (riscv_subset_list_t);
riscv_rps_as.subset_list->head = NULL;
riscv_rps_as.subset_list->tail = NULL;
- riscv_rps_as.subset_list->arch_str = NULL;
+ riscv_rps_as.subset_list->func_arch_str = NULL;
}
riscv_release_subset_list (riscv_rps_as.subset_list);
riscv_parse_subset (&riscv_rps_as, s);
- riscv_reset_subsets_list_arch_str ();
+ riscv_set_arch_str (&file_arch_str);
+ riscv_set_arch_str (&riscv_rps_as.subset_list->func_arch_str);
riscv_set_rvc (false);
if (riscv_subset_supports (&riscv_rps_as, "c")
@@ -612,7 +616,7 @@ riscv_mapping_state (enum riscv_seg_mstate to_state,
else if (seg_arch_symbol != 0
&& to_state == MAP_INSN
&& !fr_align_code
- && strcmp (riscv_rps_as.subset_list->arch_str,
+ && strcmp (riscv_rps_as.subset_list->func_arch_str,
S_GET_NAME (seg_arch_symbol) + 2) != 0)
{
reset_seg_arch_str = true;
@@ -623,7 +627,7 @@ riscv_mapping_state (enum riscv_seg_mstate to_state,
valueT value = (valueT) (frag_now_fix () - max_chars);
seg_info (now_seg)->tc_segment_info_data.map_state = to_state;
const char *arch_str = reset_seg_arch_str
- ? riscv_rps_as.subset_list->arch_str : NULL;
+ ? riscv_rps_as.subset_list->func_arch_str : NULL;
make_mapping_symbol (to_state, value, frag_now, arch_str,
false/* odd_data_padding */);
}
@@ -4811,13 +4815,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_set_arch_str (&riscv_rps_as.subset_list->func_arch_str);
riscv_set_rvc (true);
}
else if (strcmp (name, "norvc") == 0)
{
riscv_update_subset (&riscv_rps_as, "-c");
- riscv_reset_subsets_list_arch_str ();
+ riscv_set_arch_str (&riscv_rps_as.subset_list->func_arch_str);
riscv_set_rvc (false);
}
else if (strcmp (name, "pic") == 0)
@@ -4838,7 +4842,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_set_arch_str (&riscv_rps_as.subset_list->func_arch_str);
riscv_set_rvc (false);
if (riscv_subset_supports (&riscv_rps_as, "c")
@@ -5363,7 +5367,7 @@ s_riscv_insn (int x ATTRIBUTE_UNUSED)
static void
riscv_write_out_attrs (void)
{
- const char *arch_str, *priv_str, *p;
+ const char *priv_str, *p;
/* versions[0]: major version.
versions[1]: minor version.
versions[2]: revision version. */
@@ -5371,10 +5375,10 @@ riscv_write_out_attrs (void)
unsigned int i;
/* Re-write architecture elf attribute. */
- arch_str = riscv_rps_as.subset_list->arch_str;
- if (!bfd_elf_add_proc_attr_string (stdoutput, Tag_RISCV_arch, arch_str))
+ if (!bfd_elf_add_proc_attr_string (stdoutput, Tag_RISCV_arch, file_arch_str))
as_fatal (_("error adding attribute: %s"),
bfd_errmsg (bfd_get_error ()));
+ free ((void *) file_arch_str);
/* For the file without any instruction, we don't set the default_priv_spec
according to the privileged elf attributes since the md_assemble isn't
deleted file mode 100644
@@ -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
deleted file mode 100644
@@ -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
-#...
deleted file mode 100644
@@ -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"
-#...
deleted file mode 100644
@@ -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
deleted file mode 100644
@@ -1,8 +0,0 @@
-#as:
-#readelf: -A
-#source: option-arch-03.s
-
-Attribute Section: riscv
-File Attributes
- Tag_RISCV_arch: "rv32i2p1_c2p0"
-#...
deleted file mode 100644
@@ -1,3 +0,0 @@
-.attribute arch, "rv64ic"
-.option arch, +d2p0, -c
-.option arch, rv32i2p1c2p0
similarity index 77%
rename from gas/testsuite/gas/riscv/option-arch-01b.d
rename to gas/testsuite/gas/riscv/option-arch-attr.d
@@ -1,8 +1,7 @@
#as: -misa-spec=2.2
+#source: option-arch.s
#readelf: -A
-#source: option-arch-01.s
Attribute Section: riscv
File Attributes
Tag_RISCV_arch: "rv64i2p0_c2p0"
-#...
new file mode 100644
@@ -0,0 +1,26 @@
+#as: -misa-spec=2.2
+#source: option-arch.s
+#objdump: -d --syms --special-syms
+
+.*:[ ]+file format .*
+
+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 \$xrv64i2p0_c2p0
+0+02 l .text 0+00 \$xrv64i2p0_f2p0_d2p0_xvendor1p0
+0+0a l .text 0+00 \$xrv64i2p0_m3p0_f2p0_d3p0_c2p0_zmmul1p0_xvendor32x3p0
+0+0c l .text 0+00 \$xrv32i2p1_c2p0
+0+00 l d .riscv.attributes 0+00 .riscv.attributes
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ ]+0:[ ]+952e[ ]+add[ ]+a0,a0,a1
+[ ]+2:[ ]+00b50533[ ]+add[ ]+a0,a0,a1
+[ ]+6:[ ]+00302573[ ]+frcsr[ ]+a0
+[ ]+a:[ ]+952e[ ]+add[ ]+a0,a0,a1
+[ ]+c:[ ]+c8002573[ ]+.insn[ ]+4, 0xc8002573
+#...
new file mode 100644
@@ -0,0 +1,11 @@
+.attribute arch, "rv64ic" # file-level, rv64ic
+add a0, a0, a1
+.option push
+.option arch, +d2p0, -c, +xvendor1p0
+add a0, a0, a1 # func-level, rv64i_d2p0_xvendor1p0
+frcsr a0
+.option pop
+.option arch, +m3p0, +d3p0, +xvendor32x3p0
+add a0, a0, a1 # func-level, rv64i_m3p0_d3p0_c_xvendor32x3p0
+.option arch, rv32i2p1c2p0 # FIXME: maybe we should adjust xlen in dis-assembler according to mappin symbols?
+rdcycleh a0 # func-level, rv32i2p1_c2p0