[PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[Xing GUO]

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

This patch fixes -march option parsing when `p` extension exists,
e.g., -march=rv64imafdcp should produce

.attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p"

rather than

.attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c_p"

---
gcc/ChangeLog:

        * common/config/riscv/riscv-common.c
(riscv_subset_list::parsing_subset_version):
        Fix -march option parsing when `p` extension exists.

gcc/testsuite/ChangeLog:

        * gcc.target/riscv/attribute-18.c: New test.
-- 

Cheers,
Xing

[-- Attachment #2: fix-riscv-march-parsing.patch --]
[-- Type: text/x-patch, Size: 985 bytes --]

diff --git a/gcc/common/config/riscv/riscv-common.c b/gcc/common/config/riscv/riscv-common.c
index b3f5c07c819..a034e218b75 100644
--- a/gcc/common/config/riscv/riscv-common.c
+++ b/gcc/common/config/riscv/riscv-common.c
@@ -527,8 +527,7 @@ riscv_subset_list::parsing_subset_version (const char *ext,
 	      /* Might be beginning of `p` extension.  */
 	      if (std_ext_p)
 		{
-		  *major_version = version;
-		  *minor_version = 0;
+		  get_default_version (ext, major_version, minor_version);
 		  *explicit_version_p = true;
 		  return p;
 		}
diff --git a/gcc/testsuite/gcc.target/riscv/attribute-18.c b/gcc/testsuite/gcc.target/riscv/attribute-18.c
new file mode 100644
index 00000000000..30a12543ed2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/attribute-18.c
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64imafdcp -mabi=lp64d" } */
+int foo() {}
+/* { dg-final { scan-assembler ".attribute arch, \"rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p\"" } } */

Re: [PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[Kito Cheng]

Hi Xing:

Thanks for your patch, but I would like to know did you have the
copyright assignment for FSF? or your employee/company has signed
that?

On Thu, Jan 21, 2021 at 8:48 PM Xing GUO via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This patch fixes -march option parsing when `p` extension exists,
> e.g., -march=rv64imafdcp should produce
>
> .attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p"
>
> rather than
>
> .attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c_p"
>
> ---
> gcc/ChangeLog:
>
>         * common/config/riscv/riscv-common.c
> (riscv_subset_list::parsing_subset_version):
>         Fix -march option parsing when `p` extension exists.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/attribute-18.c: New test.
> --
>
> Cheers,
> Xing

Re: [PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[Xing GUO]

Hi Kito,

I’ve sent my assignments and my school’s disclaimer to assign@gnu.org <mailto:assign@gnu.org> ([gnu.org #1673033] Xing GUO), but I haven’t got response so far. Not sure if this patch can be accepted as  a small bugfix patch. If not, I’m happy to wait until FSF prove it.

Best Regards,
Xing

> On Jan 22, 2021, at 2:26 PM, Kito Cheng <kito.cheng@gmail.com> wrote:
> 
> Hi Xing:
> 
> Thanks for your patch, but I would like to know did you have the
> copyright assignment for FSF? or your employee/company has signed
> that?
> 
> On Thu, Jan 21, 2021 at 8:48 PM Xing GUO via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> 
>> This patch fixes -march option parsing when `p` extension exists,
>> e.g., -march=rv64imafdcp should produce
>> 
>> .attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p"
>> 
>> rather than
>> 
>> .attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c_p"
>> 
>> ---
>> gcc/ChangeLog:
>> 
>>        * common/config/riscv/riscv-common.c
>> (riscv_subset_list::parsing_subset_version):
>>        Fix -march option parsing when `p` extension exists.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>>        * gcc.target/riscv/attribute-18.c: New test.
>> --
>> 
>> Cheers,
>> Xing

Re: [PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[Kito Cheng]

Hi Jim:

I think this patch is small enough to accept without FSF copyright
assignment, and he is also on the way of the process, what do you
think?

On Fri, Jan 22, 2021 at 2:44 PM Xing GUO <higuoxing@gmail.com> wrote:
>
> Hi Kito,
>
> I’ve sent my assignments and my school’s disclaimer to assign@gnu.org ([gnu.org #1673033] Xing GUO), but I haven’t got response so far. Not sure if this patch can be accepted as  a small bugfix patch. If not, I’m happy to wait until FSF prove it.
>
> Best Regards,
> Xing
>
> On Jan 22, 2021, at 2:26 PM, Kito Cheng <kito.cheng@gmail.com> wrote:
>
> Hi Xing:
>
> Thanks for your patch, but I would like to know did you have the
> copyright assignment for FSF? or your employee/company has signed
> that?
>
> On Thu, Jan 21, 2021 at 8:48 PM Xing GUO via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>
>
> This patch fixes -march option parsing when `p` extension exists,
> e.g., -march=rv64imafdcp should produce
>
> .attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p"
>
> rather than
>
> .attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c_p"
>
> ---
> gcc/ChangeLog:
>
>        * common/config/riscv/riscv-common.c
> (riscv_subset_list::parsing_subset_version):
>        Fix -march option parsing when `p` extension exists.
>
> gcc/testsuite/ChangeLog:
>
>        * gcc.target/riscv/attribute-18.c: New test.
> --
>
> Cheers,
> Xing
>
>

Re: [PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[Xing GUO]

[-- Attachment #1: Type: text/plain, Size: 1700 bytes --]

On 1/22/21, Kito Cheng <kito.cheng@sifive.com> wrote:
> Hi Jim:
>
> I think this patch is small enough to accept without FSF copyright
> assignment, and he is also on the way of the process, what do you
> think?
>
> On Fri, Jan 22, 2021 at 2:44 PM Xing GUO <higuoxing@gmail.com> wrote:
>>
>> Hi Kito,
>>
>> I’ve sent my assignments and my school’s disclaimer to assign@gnu.org
>> ([gnu.org #1673033] Xing GUO), but I haven’t got response so far. Not sure
>> if this patch can be accepted as  a small bugfix patch. If not, I’m happy
>> to wait until FSF prove it.
>>
>> Best Regards,
>> Xing
>>
>> On Jan 22, 2021, at 2:26 PM, Kito Cheng <kito.cheng@gmail.com> wrote:
>>
>> Hi Xing:
>>
>> Thanks for your patch, but I would like to know did you have the
>> copyright assignment for FSF? or your employee/company has signed
>> that?
>>
>> On Thu, Jan 21, 2021 at 8:48 PM Xing GUO via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>
>>
>> This patch fixes -march option parsing when `p` extension exists,
>> e.g., -march=rv64imafdcp should produce
>>
>> .attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p"
>>
>> rather than
>>
>> .attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c_p"
>>
>> ---
>> gcc/ChangeLog:
>>
>>        * common/config/riscv/riscv-common.c
>> (riscv_subset_list::parsing_subset_version):
>>        Fix -march option parsing when `p` extension exists.
>>
>> gcc/testsuite/ChangeLog:
>>
>>        * gcc.target/riscv/attribute-18.c: New test.
>> --
>>
>> Cheers,
>> Xing
>>
>>
>

Hi Kito and Jim

I've received my copyright assignment from FSF. Could you please merge
this patch? Thanks!

-- 
Cheers,
Xing

[-- Attachment #2: Guo.1673033.GCC.pdf --]
[-- Type: application/pdf, Size: 910157 bytes --]

Re: [PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[Xing GUO]

[-- Attachment #1: Type: text/plain, Size: 393 bytes --]

Sorry, I forgot to remove the line '*explicit_version_p = true;' in my
previous patch.

This is an updated patch.

Thanks!

---
gcc/ChangeLog:

        * common/config/riscv/riscv-common.c
(riscv_subset_list::parsing_subset_version):
        Fix -march option parsing when `p` extension exists.

gcc/testsuite/ChangeLog:

        * gcc.target/riscv/attribute-18.c: New test.

-- 
Cheers,
Xing

[-- Attachment #2: fix-riscv-march-parsing-v2.patch --]
[-- Type: text/x-patch, Size: 998 bytes --]

diff --git a/gcc/common/config/riscv/riscv-common.c b/gcc/common/config/riscv/riscv-common.c
index b3f5c07c819..6bbe25dba89 100644
--- a/gcc/common/config/riscv/riscv-common.c
+++ b/gcc/common/config/riscv/riscv-common.c
@@ -527,9 +527,7 @@ riscv_subset_list::parsing_subset_version (const char *ext,
 	      /* Might be beginning of `p` extension.  */
 	      if (std_ext_p)
 		{
-		  *major_version = version;
-		  *minor_version = 0;
-		  *explicit_version_p = true;
+		  get_default_version (ext, major_version, minor_version);
 		  return p;
 		}
 	      else
diff --git a/gcc/testsuite/gcc.target/riscv/attribute-18.c b/gcc/testsuite/gcc.target/riscv/attribute-18.c
new file mode 100644
index 00000000000..30a12543ed2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/attribute-18.c
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64imafdcp -mabi=lp64d" } */
+int foo() {}
+/* { dg-final { scan-assembler ".attribute arch, \"rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p\"" } } */

Re: [PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[Kito Cheng]

Thanks! committed to master :)

On Wed, Jan 27, 2021 at 1:58 PM Xing GUO via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Sorry, I forgot to remove the line '*explicit_version_p = true;' in my
> previous patch.
>
> This is an updated patch.
>
> Thanks!
>
> ---
> gcc/ChangeLog:
>
>         * common/config/riscv/riscv-common.c
> (riscv_subset_list::parsing_subset_version):
>         Fix -march option parsing when `p` extension exists.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/attribute-18.c: New test.
>
> --
> Cheers,
> Xing

Re: [PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[Andreas Schwab]

FAIL: gcc.target/riscv/attribute-18.c scan-assembler .attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p"

$ grep -c 'attribute arch' attribute-18.s 
0

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[Xing GUO]

Hi Andreas and Kito,

I haven't reproduced this failure, but it looks that I forget to
append `-mriscv-attribute` to dg-options in attribute-18.c. I'll reply
to this thread ASAP.

Thanks,
Xing

On 2/1/21, Andreas Schwab <schwab@linux-m68k.org> wrote:
> FAIL: gcc.target/riscv/attribute-18.c scan-assembler .attribute arch,
> "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p"
>
> $ grep -c 'attribute arch' attribute-18.s
> 0
>
> Andreas.
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> "And now for something completely different."
>


-- 
Cheers,
Xing

Re: [PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[Xing GUO]

Hi,

I've reproduced the failure. It's because my gcc is configured as a
bare-metal toolchain and built with binutils that supports RISC-V
attribute. That is to say, my gcc emits RISC-V attributes by default.
Below is the patch that should fix the failure. Sorry for the
inconvenience.

diff --git a/gcc/testsuite/gcc.target/riscv/attribute-18.c
b/gcc/testsuite/gcc.target/riscv/attribute-18.c
index 1fd80fed51b..492360cf7c1 100644
--- a/gcc/testsuite/gcc.target/riscv/attribute-18.c
+++ b/gcc/testsuite/gcc.target/riscv/attribute-18.c
@@ -1,4 +1,4 @@
 /* { dg-do compile } */
-/* { dg-options "-march=rv64imafdcp -mabi=lp64d -misa-spec=2.2" } */
+/* { dg-options "-mriscv-attribute -march=rv64imafdcp -mabi=lp64d
-misa-spec=2.2" } */
 int foo() {}
 /* { dg-final { scan-assembler ".attribute arch,
\"rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p\"" } } */

On 2/1/21, Xing GUO <higuoxing@gmail.com> wrote:
> Hi Andreas and Kito,
>
> I haven't reproduced this failure, but it looks that I forget to
> append `-mriscv-attribute` to dg-options in attribute-18.c. I'll reply
> to this thread ASAP.
>
> Thanks,
> Xing
>
> On 2/1/21, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> FAIL: gcc.target/riscv/attribute-18.c scan-assembler .attribute arch,
>> "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p"
>>
>> $ grep -c 'attribute arch' attribute-18.s
>> 0
>>
>> Andreas.
>>
>> --
>> Andreas Schwab, schwab@linux-m68k.org
>> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
>> "And now for something completely different."
>>
>
>
> --
> Cheers,
> Xing
>


-- 
Cheers,
Xing

Re: [PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[Kito Cheng]

Pushed, thanks :)

On Mon, Feb 1, 2021 at 4:58 PM Xing GUO <higuoxing@gmail.com> wrote:
>
> Hi,
>
> I've reproduced the failure. It's because my gcc is configured as a
> bare-metal toolchain and built with binutils that supports RISC-V
> attribute. That is to say, my gcc emits RISC-V attributes by default.
> Below is the patch that should fix the failure. Sorry for the
> inconvenience.
>
> diff --git a/gcc/testsuite/gcc.target/riscv/attribute-18.c
> b/gcc/testsuite/gcc.target/riscv/attribute-18.c
> index 1fd80fed51b..492360cf7c1 100644
> --- a/gcc/testsuite/gcc.target/riscv/attribute-18.c
> +++ b/gcc/testsuite/gcc.target/riscv/attribute-18.c
> @@ -1,4 +1,4 @@
>  /* { dg-do compile } */
> -/* { dg-options "-march=rv64imafdcp -mabi=lp64d -misa-spec=2.2" } */
> +/* { dg-options "-mriscv-attribute -march=rv64imafdcp -mabi=lp64d
> -misa-spec=2.2" } */
>  int foo() {}
>  /* { dg-final { scan-assembler ".attribute arch,
> \"rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p\"" } } */
>
> On 2/1/21, Xing GUO <higuoxing@gmail.com> wrote:
> > Hi Andreas and Kito,
> >
> > I haven't reproduced this failure, but it looks that I forget to
> > append `-mriscv-attribute` to dg-options in attribute-18.c. I'll reply
> > to this thread ASAP.
> >
> > Thanks,
> > Xing
> >
> > On 2/1/21, Andreas Schwab <schwab@linux-m68k.org> wrote:
> >> FAIL: gcc.target/riscv/attribute-18.c scan-assembler .attribute arch,
> >> "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0_p"
> >>
> >> $ grep -c 'attribute arch' attribute-18.s
> >> 0
> >>
> >> Andreas.
> >>
> >> --
> >> Andreas Schwab, schwab@linux-m68k.org
> >> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> >> "And now for something completely different."
> >>
> >
> >
> > --
> > Cheers,
> > Xing
> >
>
>
> --
> Cheers,
> Xing

Re: [PATCH] RISC-V: Fix -march option parsing when `p` extension exists.

[Jim Wilson]

On Thu, Jan 21, 2021 at 10:49 PM Kito Cheng <kito.cheng@sifive.com> wrote:

> I think this patch is small enough to accept without FSF copyright
> assignment, and he is also on the way of the process, what do you
> think?
>

I see the copyright assignments on file at the FSF when I checked today.

Jim