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

[Palmer Dabbelt]

On Thu, 16 Jan 2025 15:21:33 PST (-0800), Andrew Waterman wrote:
> 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 !

Ya, that seems like a reasonable answer to me.  I didn't know it's UB to 
call into glibc with FE_TONEARESTFROMZERO (aka RMM), though, so maybe we 
just screwed something up?

Poking around it seems we've got explicit RMM sets in both the round() 
and lround() autovec expansions, but my simple test cases for both 
restore the dynamic rounding mode before calling/returning (under -Ofast 
-march=rv64gcv).

    void foo(void *);
    
    void vec_lround(long *out, const double *in)
    { 
      for (long i = 0; i < 1024; ++i)
        out[i] = __builtin_lround(in[i]);
      
      foo(out);
      
      for (long i = 1024; i < 2048; ++i)
        out[i] = __builtin_lround(in[i]);
    }
    
    void vec_round(double *out, const double *in)
    {
      for (long i = 0; i < 1024; ++i)
        out[i] = __builtin_round(in[i]);
    
      foo(out);
    
      for (long i = 1024; i < 2048; ++i)
        out[i] = __builtin_round(in[i]);
    } 

So I guess no luck there...

>> @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