RE: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition
Yes, thanks Robin, we can move to another mail thread for this, avoiding misleading. Pan -Original Message- From: Robin Dapp Sent: Tuesday, July 4, 2023 9:15 PM To: Li, Pan2 ; juzhe.zh...@rivai.ai; gcc-patches Cc: rdapp@gmail.com; jeffreyalaw ; Wang, Yanzhang ; kito.cheng 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
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
RE: [PATCH v1] RISC-V: Fix one typo of FRM dynamic definition
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 ; juzhe.zh...@rivai.ai; gcc-patches Cc: jeffreyalaw ; Wang, Yanzhang ; kito.cheng 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 Sent: Monday, July 3, 2023 10:57 PM To: Li, Pan2 ; juzhe.zh...@rivai.ai; gcc-patches Cc: rdapp@gmail.com; jeffreyalaw ; Wang, Yanzhang ; kito.cheng 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
Sure, every change need test and will pay attention for this in future. Pan -Original Message- From: Robin Dapp Sent: Monday, July 3, 2023 10:57 PM To: Li, Pan2 ; juzhe.zh...@rivai.ai; gcc-patches Cc: rdapp@gmail.com; jeffreyalaw ; Wang, Yanzhang ; kito.cheng 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
> 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
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 Sent: Monday, July 3, 2023 10:49 PM To: Li, Pan2 ; juzhe.zh...@rivai.ai; gcc-patches Cc: rdapp@gmail.com; jeffreyalaw ; Wang, Yanzhang ; kito.cheng 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
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
Committed, thanks Robin and Juzhe. Pan -Original Message- From: Robin Dapp Sent: Monday, July 3, 2023 4:15 PM To: juzhe.zh...@rivai.ai; Li, Pan2 ; gcc-patches Cc: rdapp@gmail.com; jeffreyalaw ; Wang, Yanzhang ; kito.cheng 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
> 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
Thanks for fixing it. LGTM. I think you can merge it when Robin is ok since this is a simple typo fix. Thanks. juzhe.zh...@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 This patch would like to fix one typo that take rdn instead of dyn by mistake. Signed-off-by: Pan Li 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
[PATCH v1] RISC-V: Fix one typo of FRM dynamic definition
From: Pan Li This patch would like to fix one typo that take rdn instead of dyn by mistake. Signed-off-by: Pan Li 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
