diff mbox series

[v4] RISC-V: Fixbug for fsflags instruction error using immediate.

Message ID 20230726054104.403-1-jinma@linux.alibaba.com
State Unresolved
Headers
Series [v4] RISC-V: Fixbug for fsflags instruction error using immediate. |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jin Ma July 26, 2023, 5:41 a.m. UTC 
  The pattern mistakenly believes that fsflags can use immediate numbers,
but in fact it does not support it. Immediate numbers should use fsflagsi.

For example:
__builtin_riscv_fsflags(4);

The following error occurred.
/tmp/ccoWdWqT.s: Assembler messages:
/tmp/ccoWdWqT.s:14: Error: illegal operands `fsflags 4'

gcc/ChangeLog:

	* config/riscv/riscv.md: Likewise.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/fsflags.c: New test.
---
 gcc/config/riscv/riscv.md                |  4 ++--
 gcc/testsuite/gcc.target/riscv/fsflags.c | 16 ++++++++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/fsflags.c

Comments

juzhe.zhong@rivai.ai July 26, 2023, 6:04 a.m. UTC | #1 
LGTM. 
Thanks for fixing it.


juzhe.zhong@rivai.ai
 
From: Jin Ma
Date: 2023-07-26 13:41
To: gcc-patches
CC: jeffreyalaw; palmer; richard.sandiford; kito.cheng; philipp.tomsich; christoph.muellner; rdapp.gcc; juzhe.zhong; jinma.contrib; Jin Ma
Subject: [PATCH v4] RISC-V: Fixbug for fsflags instruction error using immediate.
The pattern mistakenly believes that fsflags can use immediate numbers,
but in fact it does not support it. Immediate numbers should use fsflagsi.
 
For example:
__builtin_riscv_fsflags(4);
 
The following error occurred.
/tmp/ccoWdWqT.s: Assembler messages:
/tmp/ccoWdWqT.s:14: Error: illegal operands `fsflags 4'
 
gcc/ChangeLog:
 
* config/riscv/riscv.md: Likewise.
 
gcc/testsuite/ChangeLog:
 
* gcc.target/riscv/fsflags.c: New test.
---
gcc/config/riscv/riscv.md                |  4 ++--
gcc/testsuite/gcc.target/riscv/fsflags.c | 16 ++++++++++++++++
2 files changed, 18 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/fsflags.c
 
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 4615e811947..24515bcf706 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -3074,7 +3074,7 @@ (define_insn "riscv_frcsr"
   "frcsr\t%0")
(define_insn "riscv_fscsr"
-  [(unspec_volatile [(match_operand:SI 0 "csr_operand" "rK")] UNSPECV_FSCSR)]
+  [(unspec_volatile [(match_operand:SI 0 "register_operand" "r")] UNSPECV_FSCSR)]
   "TARGET_HARD_FLOAT || TARGET_ZFINX"
   "fscsr\t%0")
@@ -3087,7 +3087,7 @@ (define_insn "riscv_frflags"
(define_insn "riscv_fsflags"
   [(unspec_volatile [(match_operand:SI 0 "csr_operand" "rK")] UNSPECV_FSFLAGS)]
   "TARGET_HARD_FLOAT || TARGET_ZFINX"
-  "fsflags\t%0")
+  "fsflags%i0\t%0")
(define_insn "*riscv_fsnvsnan<mode>2"
   [(unspec_volatile [(match_operand:ANYF 0 "register_operand" "f")
diff --git a/gcc/testsuite/gcc.target/riscv/fsflags.c b/gcc/testsuite/gcc.target/riscv/fsflags.c
new file mode 100644
index 00000000000..74a97b8a7c7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/fsflags.c
@@ -0,0 +1,16 @@
+/* Verify that fsflags is using the correct register or immediate.  */
+/* { dg-do compile } */
+/* { dg-require-effective-target hard_float } */
+/* { dg-options "-O" } */
+
+void foo1 (int a)
+{
+   __builtin_riscv_fsflags(a);
+}
+void foo2 ()
+{
+   __builtin_riscv_fsflags(4);
+}
+
+/* { dg-final { scan-assembler-times "fsflags\t" 1 } } */
+/* { dg-final { scan-assembler-times "fsflagsi\t" 1 } } */
-- 
2.17.1
Kito Cheng July 26, 2023, 7:07 a.m. UTC | #2 
On Wed, Jul 26, 2023 at 1:41 PM Jin Ma via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> The pattern mistakenly believes that fsflags can use immediate numbers,
> but in fact it does not support it. Immediate numbers should use fsflagsi.
>
> For example:
> __builtin_riscv_fsflags(4);
>
> The following error occurred.
> /tmp/ccoWdWqT.s: Assembler messages:
> /tmp/ccoWdWqT.s:14: Error: illegal operands `fsflags 4'
>
> gcc/ChangeLog:
>
>         * config/riscv/riscv.md: Likewise.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/fsflags.c: New test.
> ---
>  gcc/config/riscv/riscv.md                |  4 ++--
>  gcc/testsuite/gcc.target/riscv/fsflags.c | 16 ++++++++++++++++
>  2 files changed, 18 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/fsflags.c
>
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 4615e811947..24515bcf706 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -3074,7 +3074,7 @@ (define_insn "riscv_frcsr"
>    "frcsr\t%0")
>
>  (define_insn "riscv_fscsr"
> -  [(unspec_volatile [(match_operand:SI 0 "csr_operand" "rK")] UNSPECV_FSCSR)]
> +  [(unspec_volatile [(match_operand:SI 0 "register_operand" "r")] UNSPECV_FSCSR)]
>    "TARGET_HARD_FLOAT || TARGET_ZFINX"
>    "fscsr\t%0")

Wait, this patch still drops K?
juzhe.zhong@rivai.ai July 26, 2023, 7:19 a.m. UTC | #3 
I just checked SPEC:

fscsr rd, rs csrrw rd, fcsr, rs 
Swap FP control/status register 
fscsr rs csrrw x0, fcsr, rs 
Write FP control/status register 

It seems that fscsr doesn't have immediate form? I am not sure.


juzhe.zhong@rivai.ai
 
From: Kito Cheng
Date: 2023-07-26 15:07
To: Jin Ma
CC: gcc-patches; jeffreyalaw; palmer; richard.sandiford; philipp.tomsich; christoph.muellner; rdapp.gcc; juzhe.zhong; jinma.contrib
Subject: Re: [PATCH v4] RISC-V: Fixbug for fsflags instruction error using immediate.
On Wed, Jul 26, 2023 at 1:41 PM Jin Ma via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> The pattern mistakenly believes that fsflags can use immediate numbers,
> but in fact it does not support it. Immediate numbers should use fsflagsi.
>
> For example:
> __builtin_riscv_fsflags(4);
>
> The following error occurred.
> /tmp/ccoWdWqT.s: Assembler messages:
> /tmp/ccoWdWqT.s:14: Error: illegal operands `fsflags 4'
>
> gcc/ChangeLog:
>
>         * config/riscv/riscv.md: Likewise.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/fsflags.c: New test.
> ---
>  gcc/config/riscv/riscv.md                |  4 ++--
>  gcc/testsuite/gcc.target/riscv/fsflags.c | 16 ++++++++++++++++
>  2 files changed, 18 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/fsflags.c
>
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 4615e811947..24515bcf706 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -3074,7 +3074,7 @@ (define_insn "riscv_frcsr"
>    "frcsr\t%0")
>
>  (define_insn "riscv_fscsr"
> -  [(unspec_volatile [(match_operand:SI 0 "csr_operand" "rK")] UNSPECV_FSCSR)]
> +  [(unspec_volatile [(match_operand:SI 0 "register_operand" "r")] UNSPECV_FSCSR)]
>    "TARGET_HARD_FLOAT || TARGET_ZFINX"
>    "fscsr\t%0")
 
Wait, this patch still drops K?
Kito Cheng July 26, 2023, 7:39 a.m. UTC | #4 
Oh, yeah, my bad, there is no fscsri, gonna test and push :)

On Wed, Jul 26, 2023 at 3:20 PM juzhe.zhong@rivai.ai
<juzhe.zhong@rivai.ai> wrote:
>
> I just checked SPEC:
>
> fscsr rd, rs csrrw rd, fcsr, rs
> Swap FP control/status register
> fscsr rs csrrw x0, fcsr, rs
> Write FP control/status register
>
> It seems that fscsr doesn't have immediate form? I am not sure.
>
>
> juzhe.zhong@rivai.ai
>
> From: Kito Cheng
> Date: 2023-07-26 15:07
> To: Jin Ma
> CC: gcc-patches; jeffreyalaw; palmer; richard.sandiford; philipp.tomsich; christoph.muellner; rdapp.gcc; juzhe.zhong; jinma.contrib
> Subject: Re: [PATCH v4] RISC-V: Fixbug for fsflags instruction error using immediate.
> On Wed, Jul 26, 2023 at 1:41 PM Jin Ma via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > The pattern mistakenly believes that fsflags can use immediate numbers,
> > but in fact it does not support it. Immediate numbers should use fsflagsi.
> >
> > For example:
> > __builtin_riscv_fsflags(4);
> >
> > The following error occurred.
> > /tmp/ccoWdWqT.s: Assembler messages:
> > /tmp/ccoWdWqT.s:14: Error: illegal operands `fsflags 4'
> >
> > gcc/ChangeLog:
> >
> >         * config/riscv/riscv.md: Likewise.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.target/riscv/fsflags.c: New test.
> > ---
> >  gcc/config/riscv/riscv.md                |  4 ++--
> >  gcc/testsuite/gcc.target/riscv/fsflags.c | 16 ++++++++++++++++
> >  2 files changed, 18 insertions(+), 2 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/fsflags.c
> >
> > diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> > index 4615e811947..24515bcf706 100644
> > --- a/gcc/config/riscv/riscv.md
> > +++ b/gcc/config/riscv/riscv.md
> > @@ -3074,7 +3074,7 @@ (define_insn "riscv_frcsr"
> >    "frcsr\t%0")
> >
> >  (define_insn "riscv_fscsr"
> > -  [(unspec_volatile [(match_operand:SI 0 "csr_operand" "rK")] UNSPECV_FSCSR)]
> > +  [(unspec_volatile [(match_operand:SI 0 "register_operand" "r")] UNSPECV_FSCSR)]
> >    "TARGET_HARD_FLOAT || TARGET_ZFINX"
> >    "fscsr\t%0")
>
> Wait, this patch still drops K?
>
Kito Cheng July 26, 2023, 8:22 a.m. UTC | #5 
commit, thanks :)

On Wed, Jul 26, 2023 at 3:39 PM Kito Cheng <kito.cheng@gmail.com> wrote:
>
> Oh, yeah, my bad, there is no fscsri, gonna test and push :)
>
> On Wed, Jul 26, 2023 at 3:20 PM juzhe.zhong@rivai.ai
> <juzhe.zhong@rivai.ai> wrote:
> >
> > I just checked SPEC:
> >
> > fscsr rd, rs csrrw rd, fcsr, rs
> > Swap FP control/status register
> > fscsr rs csrrw x0, fcsr, rs
> > Write FP control/status register
> >
> > It seems that fscsr doesn't have immediate form? I am not sure.
> >
> >
> > juzhe.zhong@rivai.ai
> >
> > From: Kito Cheng
> > Date: 2023-07-26 15:07
> > To: Jin Ma
> > CC: gcc-patches; jeffreyalaw; palmer; richard.sandiford; philipp.tomsich; christoph.muellner; rdapp.gcc; juzhe.zhong; jinma.contrib
> > Subject: Re: [PATCH v4] RISC-V: Fixbug for fsflags instruction error using immediate.
> > On Wed, Jul 26, 2023 at 1:41 PM Jin Ma via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> > >
> > > The pattern mistakenly believes that fsflags can use immediate numbers,
> > > but in fact it does not support it. Immediate numbers should use fsflagsi.
> > >
> > > For example:
> > > __builtin_riscv_fsflags(4);
> > >
> > > The following error occurred.
> > > /tmp/ccoWdWqT.s: Assembler messages:
> > > /tmp/ccoWdWqT.s:14: Error: illegal operands `fsflags 4'
> > >
> > > gcc/ChangeLog:
> > >
> > >         * config/riscv/riscv.md: Likewise.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >         * gcc.target/riscv/fsflags.c: New test.
> > > ---
> > >  gcc/config/riscv/riscv.md                |  4 ++--
> > >  gcc/testsuite/gcc.target/riscv/fsflags.c | 16 ++++++++++++++++
> > >  2 files changed, 18 insertions(+), 2 deletions(-)
> > >  create mode 100644 gcc/testsuite/gcc.target/riscv/fsflags.c
> > >
> > > diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> > > index 4615e811947..24515bcf706 100644
> > > --- a/gcc/config/riscv/riscv.md
> > > +++ b/gcc/config/riscv/riscv.md
> > > @@ -3074,7 +3074,7 @@ (define_insn "riscv_frcsr"
> > >    "frcsr\t%0")
> > >
> > >  (define_insn "riscv_fscsr"
> > > -  [(unspec_volatile [(match_operand:SI 0 "csr_operand" "rK")] UNSPECV_FSCSR)]
> > > +  [(unspec_volatile [(match_operand:SI 0 "register_operand" "r")] UNSPECV_FSCSR)]
> > >    "TARGET_HARD_FLOAT || TARGET_ZFINX"
> > >    "fscsr\t%0")
> >
> > Wait, this patch still drops K?
> >
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 4615e811947..24515bcf706 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -3074,7 +3074,7 @@  (define_insn "riscv_frcsr"
   "frcsr\t%0")
 
 (define_insn "riscv_fscsr"
-  [(unspec_volatile [(match_operand:SI 0 "csr_operand" "rK")] UNSPECV_FSCSR)]
+  [(unspec_volatile [(match_operand:SI 0 "register_operand" "r")] UNSPECV_FSCSR)]
   "TARGET_HARD_FLOAT || TARGET_ZFINX"
   "fscsr\t%0")
 
@@ -3087,7 +3087,7 @@  (define_insn "riscv_frflags"
 (define_insn "riscv_fsflags"
   [(unspec_volatile [(match_operand:SI 0 "csr_operand" "rK")] UNSPECV_FSFLAGS)]
   "TARGET_HARD_FLOAT || TARGET_ZFINX"
-  "fsflags\t%0")
+  "fsflags%i0\t%0")
 
 (define_insn "*riscv_fsnvsnan<mode>2"
   [(unspec_volatile [(match_operand:ANYF 0 "register_operand" "f")
diff --git a/gcc/testsuite/gcc.target/riscv/fsflags.c b/gcc/testsuite/gcc.target/riscv/fsflags.c
new file mode 100644
index 00000000000..74a97b8a7c7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/fsflags.c
@@ -0,0 +1,16 @@ 
+/* Verify that fsflags is using the correct register or immediate.  */
+/* { dg-do compile } */
+/* { dg-require-effective-target hard_float } */
+/* { dg-options "-O" } */
+
+void foo1 (int a)
+{
+   __builtin_riscv_fsflags(a);
+}
+void foo2 ()
+{
+   __builtin_riscv_fsflags(4);
+}
+
+/* { dg-final { scan-assembler-times "fsflags\t" 1 } } */
+/* { dg-final { scan-assembler-times "fsflagsi\t" 1 } } */