[v1] RISC-V: Fix one typo of FRM dynamic definition
Checks
Commit Message
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(-)
Comments
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
> 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
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
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
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
> 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
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
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
> 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
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
@@ -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")
)