public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] RISC-V: Allow merging 'H' extension
@ 2022-11-26  3:13 Tsukasa OI
  2022-11-28  1:35 ` Nelson Chu
  0 siblings, 1 reply; 3+ messages in thread
From: Tsukasa OI @ 2022-11-26  3:13 UTC (permalink / raw)
  To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils

From: Tsukasa OI <research_trasio@irq.a4lg.com>

Because riscv_merge_std_ext function did not merge the 'H' extension, linked
executables lacked 'H' extension when multiple objects are linked.

This issue is found while building OpenSBI with 'H' extension (resulting
ELF files did not contain "h1p0" in "Tag_RISCV_arch" even if *all* linked
object files contained it).

This commit adds 'h' to standard_exts variable to merge 'H' extension.

bfd/ChangeLog:

	* elfnn-riscv.c (riscv_merge_std_ext): Add 'H' extension merging.

ld/ChangeLog:

	* testsuite/ld-riscv-elf/attr-merge-arch-02.d: Test merging of 'H'
	and 'Zicsr' extensions.
	* testsuite/ld-riscv-elf/attr-merge-arch-02a.s: Likewise.
---
 bfd/elfnn-riscv.c                               | 2 +-
 ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d  | 2 +-
 ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index a2d85dbe9396..a83c8ad2695e 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -3427,7 +3427,7 @@ riscv_merge_std_ext (bfd *ibfd,
 		     struct riscv_subset_t **pin,
 		     struct riscv_subset_t **pout)
 {
-  const char *standard_exts = "mafdqlcbjtpvn";
+  const char *standard_exts = "mafdqlcbjtpvnh";
   const char *p;
   struct riscv_subset_t *in = *pin;
   struct riscv_subset_t *out = *pout;
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d
index 381ef850d974..d1ed8667303c 100644
--- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d
@@ -6,4 +6,4 @@
 
 Attribute Section: riscv
 File Attributes
-  Tag_RISCV_arch: "rv32i2p1_a2p0"
+  Tag_RISCV_arch: "rv32i2p1_a2p0_h1p0_zicsr2p0"
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s
index 4593241c0249..696e83975346 100644
--- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s
@@ -1 +1 @@
-	.attribute arch, "rv32i2p1_a2p0"
+	.attribute arch, "rv32i2p1_a2p0_h1p0_zicsr2p0"

base-commit: 66e7b0f4d9c88781c2b7589f86e31a7d153ed60c
-- 
2.38.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] RISC-V: Allow merging 'H' extension
  2022-11-26  3:13 [PATCH] RISC-V: Allow merging 'H' extension Tsukasa OI
@ 2022-11-28  1:35 ` Nelson Chu
  2022-11-28  2:26   ` Tsukasa OI
  0 siblings, 1 reply; 3+ messages in thread
From: Nelson Chu @ 2022-11-28  1:35 UTC (permalink / raw)
  To: Tsukasa OI; +Cc: Kito Cheng, Palmer Dabbelt, binutils

On Sat, Nov 26, 2022 at 11:14 AM Tsukasa OI
<research_trasio@irq.a4lg.com> wrote:
>
> From: Tsukasa OI <research_trasio@irq.a4lg.com>
>
> Because riscv_merge_std_ext function did not merge the 'H' extension, linked
> executables lacked 'H' extension when multiple objects are linked.
>
> This issue is found while building OpenSBI with 'H' extension (resulting
> ELF files did not contain "h1p0" in "Tag_RISCV_arch" even if *all* linked
> object files contained it).
>
> This commit adds 'h' to standard_exts variable to merge 'H' extension.
>
> bfd/ChangeLog:
>
>         * elfnn-riscv.c (riscv_merge_std_ext): Add 'H' extension merging.
>
> ld/ChangeLog:
>
>         * testsuite/ld-riscv-elf/attr-merge-arch-02.d: Test merging of 'H'
>         and 'Zicsr' extensions.
>         * testsuite/ld-riscv-elf/attr-merge-arch-02a.s: Likewise.

Thanks, we missed this, so it looks good except that I think we don't
need to add/update test cases to test this trivial fix.

Nelson

> ---
>  bfd/elfnn-riscv.c                               | 2 +-
>  ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d  | 2 +-
>  ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index a2d85dbe9396..a83c8ad2695e 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -3427,7 +3427,7 @@ riscv_merge_std_ext (bfd *ibfd,
>                      struct riscv_subset_t **pin,
>                      struct riscv_subset_t **pout)
>  {
> -  const char *standard_exts = "mafdqlcbjtpvn";
> +  const char *standard_exts = "mafdqlcbjtpvnh";
>    const char *p;
>    struct riscv_subset_t *in = *pin;
>    struct riscv_subset_t *out = *pout;
> diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d
> index 381ef850d974..d1ed8667303c 100644
> --- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d
> +++ b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d
> @@ -6,4 +6,4 @@
>
>  Attribute Section: riscv
>  File Attributes
> -  Tag_RISCV_arch: "rv32i2p1_a2p0"
> +  Tag_RISCV_arch: "rv32i2p1_a2p0_h1p0_zicsr2p0"
> diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s
> index 4593241c0249..696e83975346 100644
> --- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s
> +++ b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s
> @@ -1 +1 @@
> -       .attribute arch, "rv32i2p1_a2p0"
> +       .attribute arch, "rv32i2p1_a2p0_h1p0_zicsr2p0"
>
> base-commit: 66e7b0f4d9c88781c2b7589f86e31a7d153ed60c
> --
> 2.38.1
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] RISC-V: Allow merging 'H' extension
  2022-11-28  1:35 ` Nelson Chu
@ 2022-11-28  2:26   ` Tsukasa OI
  0 siblings, 0 replies; 3+ messages in thread
From: Tsukasa OI @ 2022-11-28  2:26 UTC (permalink / raw)
  To: Nelson Chu; +Cc: binutils

On 2022/11/28 10:35, Nelson Chu wrote:
> On Sat, Nov 26, 2022 at 11:14 AM Tsukasa OI
> <research_trasio@irq.a4lg.com> wrote:
>>
>> From: Tsukasa OI <research_trasio@irq.a4lg.com>
>>
>> Because riscv_merge_std_ext function did not merge the 'H' extension, linked
>> executables lacked 'H' extension when multiple objects are linked.
>>
>> This issue is found while building OpenSBI with 'H' extension (resulting
>> ELF files did not contain "h1p0" in "Tag_RISCV_arch" even if *all* linked
>> object files contained it).
>>
>> This commit adds 'h' to standard_exts variable to merge 'H' extension.
>>
>> bfd/ChangeLog:
>>
>>         * elfnn-riscv.c (riscv_merge_std_ext): Add 'H' extension merging.
>>
>> ld/ChangeLog:
>>
>>         * testsuite/ld-riscv-elf/attr-merge-arch-02.d: Test merging of 'H'
>>         and 'Zicsr' extensions.
>>         * testsuite/ld-riscv-elf/attr-merge-arch-02a.s: Likewise.
> 
> Thanks, we missed this, so it looks good except that I think we don't
> need to add/update test cases to test this trivial fix.
> 
> Nelson

Okay, since we already test 'A' extension for riscv_merge_std_ext
behavior (in the same test) and the change I made to BFD is so clear,
it's possible that testing 'H' extension separately is not needed.

Committing without tests.

Thanks,
Tsukasa

> 
>> ---
>>  bfd/elfnn-riscv.c                               | 2 +-
>>  ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d  | 2 +-
>>  ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s | 2 +-
>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
>> index a2d85dbe9396..a83c8ad2695e 100644
>> --- a/bfd/elfnn-riscv.c
>> +++ b/bfd/elfnn-riscv.c
>> @@ -3427,7 +3427,7 @@ riscv_merge_std_ext (bfd *ibfd,
>>                      struct riscv_subset_t **pin,
>>                      struct riscv_subset_t **pout)
>>  {
>> -  const char *standard_exts = "mafdqlcbjtpvn";
>> +  const char *standard_exts = "mafdqlcbjtpvnh";
>>    const char *p;
>>    struct riscv_subset_t *in = *pin;
>>    struct riscv_subset_t *out = *pout;
>> diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d
>> index 381ef850d974..d1ed8667303c 100644
>> --- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d
>> +++ b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d
>> @@ -6,4 +6,4 @@
>>
>>  Attribute Section: riscv
>>  File Attributes
>> -  Tag_RISCV_arch: "rv32i2p1_a2p0"
>> +  Tag_RISCV_arch: "rv32i2p1_a2p0_h1p0_zicsr2p0"
>> diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s
>> index 4593241c0249..696e83975346 100644
>> --- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s
>> +++ b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02a.s
>> @@ -1 +1 @@
>> -       .attribute arch, "rv32i2p1_a2p0"
>> +       .attribute arch, "rv32i2p1_a2p0_h1p0_zicsr2p0"
>>
>> base-commit: 66e7b0f4d9c88781c2b7589f86e31a7d153ed60c
>> --
>> 2.38.1
>>
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-11-28  2:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-26  3:13 [PATCH] RISC-V: Allow merging 'H' extension Tsukasa OI
2022-11-28  1:35 ` Nelson Chu
2022-11-28  2:26   ` Tsukasa OI

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).