[PATCH] D128900: [LoongArch] Add codegen support for converting between unsigned integer and floating-point

[Xi Ruoyao via Phabricator via llvm-commits]

xry111 added inline comments.


================
Comment at: llvm/test/CodeGen/LoongArch/ir-instruction/double-convert.ll:138-153
+; LA64-NEXT:    pcalau12i $a0, .LCPI7_0
+; LA64-NEXT:    addi.d $a0, $a0, .LCPI7_0
+; LA64-NEXT:    fld.d $fa1, $a0, 0
+; LA64-NEXT:    fsub.d $fa2, $fa0, $fa1
+; LA64-NEXT:    ftintrz.w.d $fa2, $fa2
+; LA64-NEXT:    movfr2gr.s $a0, $fa2
+; LA64-NEXT:    lu12i.w $a1, -524288
----------------
xry111 wrote:
> xen0n wrote:
> > SixWeining wrote:
> > > xen0n wrote:
> > > > For `f64 -> u32` conversion on LA64, isn't `ftintrz.l.d` followed by `movfr2gr.s` enough for all values within `u32`'s domain? Overflow is UB both in C and LLVM IR so we can technically ignore the (very) inconsistent behavior when input overflows.
> > > > 
> > > > (BTW I noticed AArch64 has native support for the `fptoui` semantics by means of the `fcvtzu` insn. Hope LoongArch will gain similar niceties in a future revision...)
> > > Thanks.
> > > 
> > > Actually we have considered the approach you mentioned. But similar to the `divide-by-zero` case, we choose the same approach as gcc what did.
> > > 
> > > And I'd like to discuss with you that is there anything missed. For example,  the floating-point rounding and exception handling.  `ftintrz.l.d` may not raise exception when the value is range [UINT32_MAX + 1, INT64_MAX - 1]. Not sure whehter we need to manually raise an exception.
> > > 
> > > @xry111 Hope to see inputs from you, too.
> > Ah that's nice to know. Thanks for sharing the thoughts behind the current implementation decision.
> > 
> > As for artificial "consistency" with gcc, I think the obvious way forward is to pursue the best practice where possible, no matter which project you're working on, then make the others adopt the verified best practice. So it doesn't matter if we're working on LLVM or gcc, we simply decide on a better way then make the other project do the same.
> > 
> > As for the exception handling and such, my opinion is: since UB means "anything is possible", either raising exceptions or not is okay, so we may well choose the faster one, that is, simply don't care. Of course this is different from the current behavior which is saturating for out-of-range inputs, which is arguably more useful for users, but it's UB after all so I think there's definitely room for further discussion. (Ideally/academically, one would try to gather some statistics over a wide range of software corpus, then base one's argument on that; however this doesn't change the fact that such usage is invoking UB so we may not be in a position to "accommodate" for their mistakes.)
> In C standard, annex F specifies C language support for the IEC 60559 (aka IEEE-754) floating-point standard.  F.4 para 1 says:
> 
> > If the integer type is _Bool, 6.3.1.2 applies and no floating-point exceptions are raised
> > (even for NaN). Otherwise, if the floating value is infinite or NaN or if the integral part
> > of the floating value exceeds the range of the integer type, then the ‘‘invalid’’ floating-
> > point exception is raised and the resulting value is unspecified.
> 
> So for this we need to raise an exception, unless (1) the user explicitly tells the compiler that he/she doesn't care about the exception (using `-fno-trapping-math` in GCC for example), or (2) LLVM explicitly claims it won't strictly follow IEEE-754 exception rules in the documentation.
BUT, it seems LLVM defines fptoui w/o any exception specified.  Clang's default is `-fno-trapping-math` (while GCC defaults to `-ftrapping-math` instead) and if you use `-ftrapping-math` explicitly, it will generate a different IR with `llvm.experimental.constrained.fptoui.i32.f64`.  So yes: for a plain `fptoui` we don't need to raise exception.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128900/new/

https://reviews.llvm.org/D128900