Hi Robin,
Thanks for reviewing, sorry for my silly mistakes in the original patch, CIL.
> Here you are referring to 10.1 in the spec I assume. Could we add this as a
> comment in the code?
Sure, from the spec rvv 1.0, aka "All standard vector floating-point arithmetic
operations follow the IEEE-754/2008 standard. All vector floating-point
operations use the dynamic rounding mode in the frm register".
> What does that mean or rather how is that reflected in the code?
This part from spec 20191213 F part, aka "111 DYN In instruction’s rm fi eld,
selects dynamic rounding mode; In Rounding Mode register, Invalid.".
> This still has yesterday's bug right? I.e. get_attr_frm_mode returns 5 for
> dyn instead of 7 because the enums don't match (leading to SIGILL).
Should be no, FRM_MODE_DYN is generated by vector.md, which present the mode
for switching and different from the frm value defined in riscv-v.cc. However
and more generally, we should have a function convert from frm_mode to frm as I
understand.
> I would prefer to have an execution test here as well. Even
> though we likely FAIL in other tests when the rounding mode
> is off, it would be good to have a specific one. Maybe it
> doesn't exactly fit into this patch but in general.
Sure, I can file another PATCH for execution, like vfadd with RMM mode but the
frm value is changed by this intrinsic, the underlying dynamic round mode can
leverage this case I bet.
Pan
-----Original Message-----
From: Robin Dapp <rdapp....@gmail.com>
Sent: Tuesday, July 4, 2023 8:52 PM
To: Li, Pan2 <pan2...@intel.com>; gcc-patches@gcc.gnu.org
Cc: rdapp....@gmail.com; juzhe.zh...@rivai.ai; jeffreya...@gmail.com; Wang,
Yanzhang <yanzhang.w...@intel.com>; kito.ch...@gmail.com
Subject: Re: [PATCH v2] RISC-V: Fix one bug for floating-point static frm
Hi Pan,
I only just now got back to my mails and I'm a bit confused about
the several patches related to rounding mode.
> 1. By default, the RVV floating-point will take dyn mode.
Here you are referring to 10.1 in the spec I assume. Could we
add this as a comment in the code?
> 2. DYN is invalid in FRM register for RVV floating-point.
What does that mean or rather how is that reflected in the code?
> - return code >= 0 ? get_attr_frm_mode (insn) : FRM_MODE_NONE;
> + return code >= 0 ? get_attr_frm_mode (insn) : FRM_MODE_DYN;
This still has yesterday's bug right? I.e. get_attr_frm_mode returns
5 for dyn instead of 7 because the enums don't match (leading to
SIGILL).
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/float-point-frm-insert-6.c
> @@ -0,0 +1,31 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64 -O3 -Wno-psabi" } */
I would prefer to have an execution test here as well. Even
though we likely FAIL in other tests when the rounding mode
is off, it would be good to have a specific one. Maybe it
doesn't exactly fit into this patch but in general.
Regards
Robin
