[PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

[PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

[pan2.li]

From: Pan Li <pan2.li@intel.com>

This patch would like to fix one typo that take rdn instead of dyn by
mistake.

Signed-off-by: Pan Li <pan2.li@intel.com>

gcc/ChangeLog:

	* config/riscv/vector.md: Fix typo.
---
 gcc/config/riscv/vector.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/config/riscv/vector.md b/gcc/config/riscv/vector.md
index a6174f9483e..2864475b35a 100644
--- a/gcc/config/riscv/vector.md
+++ b/gcc/config/riscv/vector.md
@@ -491,8 +491,8 @@ (define_attr "frm_mode" "rne,rtz,rdn,rup,rmm,dyn,none"
 	  (match_test "INTVAL (operands[9]) == riscv_vector::FRM_RMM")
 	  (const_string "rmm")
 
-	  (match_test "INTVAL (operands[9]) == riscv_vector::FRM_RDN")
-	  (const_string "rdn")
+	  (match_test "INTVAL (operands[9]) == riscv_vector::FRM_DYN")
+	  (const_string "dyn")
 	]
 	(const_string "none")
       )
-- 
2.34.1

Re: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

[juzhe.zhong]

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

Thanks for fixing it. LGTM.
I think you can merge it when Robin is ok since this is a simple typo fix.

Thanks.


juzhe.zhong@rivai.ai
 
From: pan2.li
Date: 2023-07-03 16:08
To: gcc-patches
CC: juzhe.zhong; jeffreyalaw; pan2.li; yanzhang.wang; kito.cheng
Subject: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition
From: Pan Li <pan2.li@intel.com>
 
This patch would like to fix one typo that take rdn instead of dyn by
mistake.
 
Signed-off-by: Pan Li <pan2.li@intel.com>
 
gcc/ChangeLog:
 
* config/riscv/vector.md: Fix typo.
---
gcc/config/riscv/vector.md | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
 
diff --git a/gcc/config/riscv/vector.md b/gcc/config/riscv/vector.md
index a6174f9483e..2864475b35a 100644
--- a/gcc/config/riscv/vector.md
+++ b/gcc/config/riscv/vector.md
@@ -491,8 +491,8 @@ (define_attr "frm_mode" "rne,rtz,rdn,rup,rmm,dyn,none"
  (match_test "INTVAL (operands[9]) == riscv_vector::FRM_RMM")
  (const_string "rmm")
-   (match_test "INTVAL (operands[9]) == riscv_vector::FRM_RDN")
-   (const_string "rdn")
+   (match_test "INTVAL (operands[9]) == riscv_vector::FRM_DYN")
+   (const_string "dyn")
]
(const_string "none")
       )
-- 
2.34.1
 
 

Re: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

[Robin Dapp]

> Thanks for fixing it. LGTM.
> I think you can merge it when Robin is ok since this is a simple typo
> fix.

Yes, that's definitely simple enough :)

Regards
 Robin

RE: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

[Li, Pan2]

Committed, thanks Robin and Juzhe.

Pan

-----Original Message-----
From: Robin Dapp <rdapp.gcc@gmail.com> 
Sent: Monday, July 3, 2023 4:15 PM
To: juzhe.zhong@rivai.ai; Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: rdapp.gcc@gmail.com; jeffreyalaw <jeffreyalaw@gmail.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: Re: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition


> Thanks for fixing it. LGTM.
> I think you can merge it when Robin is ok since this is a simple typo
> fix.

Yes, that's definitely simple enough :)

Regards
 Robin

Re: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

[Robin Dapp]

Hmm, looks like it wasn't simple enough...

I'm seeing execution fails for various floating point test cases.
This is due to a mismatch between the FRM_DYN definition (0b111 == 7)
and the attribute value (== 5).  Therefore we set the rounding mode
to 5 instead of 7.

Regards
 Robin

RE: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

[Li, Pan2]

Sorry for inconvenient, still working on fix it. If urgent I can revert this change to unblock your work ASAP.

Pan

-----Original Message-----
From: Robin Dapp <rdapp.gcc@gmail.com> 
Sent: Monday, July 3, 2023 10:49 PM
To: Li, Pan2 <pan2.li@intel.com>; juzhe.zhong@rivai.ai; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: rdapp.gcc@gmail.com; jeffreyalaw <jeffreyalaw@gmail.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: Re: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

Hmm, looks like it wasn't simple enough...

I'm seeing execution fails for various floating point test cases.
This is due to a mismatch between the FRM_DYN definition (0b111 == 7)
and the attribute value (== 5).  Therefore we set the rounding mode
to 5 instead of 7.

Regards
 Robin

Re: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

[Robin Dapp]

> Sorry for inconvenient, still working on fix it. If urgent I can
> revert this change to unblock your work ASAP.

I'm not blocked by this, thanks, just wanted to document it here.
I was testing another patch and needed to dig for a while until
I realized the FAILs come from this one.  In general I would
assume that even obvious patches are tested before (I have
introduced bugs by obvious ones before so I make sure to).

Regards
 Robin

RE: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

[Li, Pan2]

Sure, every change need test and will pay attention for this in future.

Pan

-----Original Message-----
From: Robin Dapp <rdapp.gcc@gmail.com> 
Sent: Monday, July 3, 2023 10:57 PM
To: Li, Pan2 <pan2.li@intel.com>; juzhe.zhong@rivai.ai; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: rdapp.gcc@gmail.com; jeffreyalaw <jeffreyalaw@gmail.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: Re: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

> Sorry for inconvenient, still working on fix it. If urgent I can
> revert this change to unblock your work ASAP.

I'm not blocked by this, thanks, just wanted to document it here.
I was testing another patch and needed to dig for a while until
I realized the FAILs come from this one.  In general I would
assume that even obvious patches are tested before (I have
introduced bugs by obvious ones before so I make sure to).

Regards
 Robin

RE: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

[Li, Pan2]

Hi Robin,

Just revert this patch, it reports some weird illegal instr, I may need more time for this.

Pan

-----Original Message-----
From: Li, Pan2 
Sent: Monday, July 3, 2023 11:00 PM
To: Robin Dapp <rdapp.gcc@gmail.com>; juzhe.zhong@rivai.ai; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: jeffreyalaw <jeffreyalaw@gmail.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: RE: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

Sure, every change need test and will pay attention for this in future.

Pan

-----Original Message-----
From: Robin Dapp <rdapp.gcc@gmail.com> 
Sent: Monday, July 3, 2023 10:57 PM
To: Li, Pan2 <pan2.li@intel.com>; juzhe.zhong@rivai.ai; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: rdapp.gcc@gmail.com; jeffreyalaw <jeffreyalaw@gmail.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: Re: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

> Sorry for inconvenient, still working on fix it. If urgent I can
> revert this change to unblock your work ASAP.

I'm not blocked by this, thanks, just wanted to document it here.
I was testing another patch and needed to dig for a while until
I realized the FAILs come from this one.  In general I would
assume that even obvious patches are tested before (I have
introduced bugs by obvious ones before so I make sure to).

Regards
 Robin

Re: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

[Robin Dapp]

> Just revert this patch, it reports some weird illegal instr, I may
> need more time for this.

The illegal instruction is due to the wrong rounding mode.  We set
5 instead of 7 because the two enums don't match.  A simple but ugly
fix would be two dummy entries so that FRM_MODE_DYN is entry 7 in
enum attr_frm_mode but I'd prefer not to do that :) 

Regards
 Robin

RE: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

[Li, Pan2]

Yes, thanks Robin, we can move to another mail thread for this, avoiding misleading.

Pan

-----Original Message-----
From: Robin Dapp <rdapp.gcc@gmail.com> 
Sent: Tuesday, July 4, 2023 9:15 PM
To: Li, Pan2 <pan2.li@intel.com>; juzhe.zhong@rivai.ai; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: rdapp.gcc@gmail.com; jeffreyalaw <jeffreyalaw@gmail.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng <kito.cheng@gmail.com>
Subject: Re: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition

> Just revert this patch, it reports some weird illegal instr, I may
> need more time for this.

The illegal instruction is due to the wrong rounding mode.  We set
5 instead of 7 because the two enums don't match.  A simple but ugly
fix would be two dummy entries so that FRM_MODE_DYN is entry 7 in
enum attr_frm_mode but I'd prefer not to do that :) 

Regards
 Robin