Hi Pan,
yes, the problem is fixed for me. Still some comments ;) Sorry
it took a while.
> 1. By default, the RVV floating-point will take dyn mode.
> 2. DYN is invalid in FRM register for RVV floating-point.
>
> When mode switching the function entry and exit, it will take DYN as
> the frm mode.
We need to clarify this as it is misleading (even if it's just
a patch description, at least I was confused):
RVV floating-point instructions always (implicitly) use the dynamic
rounding mode. That's IMHO not a default but rather an unchangeable
fact. This implies that rounding is performed according to the
rounding mode set in the FRM register. The FRM register itself
only holds proper rounding modes and never the dynamic rounding mode.
> - if (mode != FRM_MODE_NONE && mode != prev_mode)
> + if (mode != FRM_MODE_DYN && mode != prev_mode)
> {
Adding a comment like "Switching to the dynamic rounding mode is not
necessary. When an instruction requests it, it effectively uses
the rounding mode already set in the FRM register. All other rounding
modes require us to switch the rounding mode via the FRM register."
> - return code >= 0 ? get_attr_frm_mode (insn) : FRM_MODE_NONE;
> + /* According to RVV 1.0 spec, all vector floating-point operations use
> + the dynamic rounding mode in the frm register. */
> + return code >= 0 ? get_attr_frm_mode (insn) : FRM_MODE_DYN;
As you reverted the previous patch get_attr_frm_mode is no longer
problematic because it returns FRM_MODE_NONE for instructions with
a dynamic rounding mode (instead of FRM_MODE_DYN). I still find
that a bit confusing or at least halfway inconsistent and somebody
reading it will suppose something is wrong. Could you either fix
the enum or add a TODO here that explains the situation?
The normal flow is that mode switching asks us if we need a mode
switch for an instruction and returning "NO MODE" means no. But
we return FRM_MODE_DYN by default and FRM_MODE_NONE for vector float
which appears odd.
In riscv_mode_after the default mode is again FRM_MODE_NONE. Wouldn't
we also want FRM_MODE_DYN here?
> @@ -7791,7 +7795,9 @@ riscv_mode_exit (int entity)
> case RISCV_VXRM:
> return VXRM_MODE_NONE;
> case RISCV_FRM:
> - return FRM_MODE_NONE;
> + /* According to RVV 1.0 spec, all vector floating-point operations use
> + the dynamic rounding mode in the frm register. */
> + return FRM_MODE_DYN;
I'd rather not have the comment duplicated all over the place. I
know I asked for it but I'd rather have it at a single spot explaining
what we need to do.
Regards
Robin
