RISC-V round_away () handling of non canonical rounding modes

[Andrew Waterman]

On Thu, Jan 16, 2025 at 3:07 PM Vineet Gupta <vineetg@rivosinc.com> wrote:
>
> +CC Juzhe, Robin, gcc patches mailing list
>
> On 1/16/25 14:49, Andrew Waterman wrote:
> > On Thu, Jan 16, 2025 at 11:43 AM Vineet Gupta <vineetg@rivosinc.com> wrote:
> >> On 1/16/25 11:14, Joseph Myers wrote:
> >>> The simple thing to do is to change sysdeps/riscv/rvf/get-rounding-mode.h
> >>> so it only returns a supported value (so making code using
> >>> get_rounding_mode treat FE_TONEARESTFROMZERO the same as FE_TONEAREST,
> >>> effectively).  That doesn't give you actual support for this rounding
> >>> mode, but should at least avoid aborts if it's set, in the absence of the
> >>> larger changes discussed above to implement full FE_TONEARESTFROMZERO
> >>> support.
> >> The simple approach feels simpler ;-)
> >> We can certainly fudge get_round_mode() to return FE_TONEARESTFROMZERO as
> >> FE_TONEAREST.
> >> But is that correct semantically as in the machine itself is in a different
> >> rounding mode than what glibc thinks it in and could compute values numerically
> >> differently than is expected.
> >>
> >> I wonder if gcc should even be generating insns with such rounding mode for the
> >> general (not explicit) cases.
> > I was wondering the same thing.  On the scalar side, the FP ops have
> > the static rounding mode field, so there isn't a reason to change the
> > dynamic rounding mode if the compiler wants to use directed rounding
> > for a specific scalar instruction.  The vector instructions mostly do
> > not have static rounding modes, so maybe this is the result of
> > autovectorization of e.g. a loop that invokes `lround`?
>
> Either autovec or just vec
>
> I notice the following pattern in generated code
>
>    90610:    00225073              fsrmi    zero,4
>    90614:    0d707057              vsetvli    zero,zero,e32,mf2,ta,ma
>    90618:    4a1890d7              vfncvt.x.f.w    v1,v1
>
> I have a feeling it is generated by following
>
> (define_insn "@pred_narrow_fcvt_x<v_su>_f<mode>"
>   [(set (match_operand:<VNCONVERT> 0 "register_operand"        "=vd, vd, vr,
> vr,  &vr,  &vr")
>     (if_then_else:<VNCONVERT>
>       (unspec:<VM>
>         [(match_operand:<VM> 1 "vector_mask_operand"       " vm,
> vm,Wc1,Wc1,vmWc1,vmWc1")
>          (match_operand 4 "vector_length_operand"          " rK, rK, rK, rK,
> rK,   rK")
>          (match_operand 5 "const_int_operand"              "  i,  i,  i,  i,
> i,    i")
>          (match_operand 6 "const_int_operand"              "  i,  i,  i,  i,
> i,    i")
>          (match_operand 7 "const_int_operand"              "  i,  i,  i,  i,
> i,    i")
>          (match_operand 8 "const_int_operand"              "  i,  i,  i,  i,
> i,    i")
>          (reg:SI VL_REGNUM)
>          (reg:SI VTYPE_REGNUM)
>          (reg:SI FRM_REGNUM)] UNSPEC_VPREDICATE)
>       (unspec:<VNCONVERT>
>          [(match_operand:V_VLSF 3 "register_operand"       "  0,  0,  0,  0,
> vr,   vr")] VFCVTS)
>       (match_operand:<VNCONVERT> 2 "vector_merge_operand"  " vu,  0, vu,  0,
> vu,    0")))]
>   "TARGET_VECTOR"
>   "vfncvt.x<v_su>.f.w\t%0,%3%p1"
>   [(set_attr "type" "vfncvtftoi")
>    (set_attr "mode" "<VNCONVERT>")
>    (set (attr "frm_mode")
>     (symbol_ref "riscv_vector::get_frm_mode (operands[8])"))
>    (set_attr "spec_restriction" "none,none,thv,thv,none,none")])
>
>
> Although I'm not sure how exactly this generates the FSRM (assuming I'm looking
> at right thing)
>
> FWIW all the testsuite tests for narrowing conversion from float2int seem to be
> checking rtz variant.
> Will have to reduce Fortran - oh well !
>
> > If so, I would have thought it was the compiler's responsibility to restore the
> > dynamic rounding mode before the next function call/return, even for
> > -ffast-math.
>
> Agree !
>
> @Andrew Side note since you are here, it seems binutils/objdump still seem to
> generate FSRMI pseudoinstruction, while this was removed from ISA spec a while
> back. Would you be willing to accept the patch to revert the removal.

The table I modified in that commit has since been moved to the ASM
manual, and today's ASM manual does include those pseudoinstructions:
https://github.com/riscv-non-isa/riscv-asm-manual/blob/0a043a0059b8bfd336e88660733e77dd0ca1ae97/src/asm-manual.adoc#L1174-L1180

So I think we're already in good shape.

>
> commit 8e3f8a50f2a31cc9c0a8ea901a1e6773358c6e38
> Author: Andrew Waterman <andrew@sifive.com>
> Date:   Wed Jun 14 14:46:06 2017 -0700
>
>     Remove FSFLAGSI, FSRMI pseudoinstructions
>
>
> Thx,
> -Vineet