[PATCH] D57143: [builtins] Rounding mode support for addxf3/subxf3

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

The mix of soft-floating point and hard-floating point (with soft-float interface) can happen on embedded systems code as well. The solution Arm's proprietary toolchain had to this problem is for the linker to select libraries based on build-attibutes. It would detect the rounding mode build attribute and select the appropriate library variant, but that isn't available to us here. A colleague suggested an easily available customisation point that someone that had a local need for a different rounding mode could implement.



================
Comment at: compiler-rt/lib/builtins/arm/fp_mode.c:25
+  uint32_t fpscr;
+  __asm__ __volatile__("vmrs  %0, fpscr" : "=r" (fpscr));
+  fpscr = fpscr >> ARM_RMODE_SHIFT & ARM_RMODE_MASK;
----------------
efriedma wrote:
> kongyi wrote:
> > MaskRay wrote:
> > > MaskRay wrote:
> > > > efriedma wrote:
> > > > > This needs to be guarded; vmrs is only valid on targets which support VFP.  Not sure if we need to try to check that at runtime...
> > > > On ARM, perhaps we can assume only `FE_TONEAREST` is available.
> > > > 
> > > > https://github.com/gcc-mirror/gcc/blob/master/libgcc/config/arm/ieee754-sf.S
> > > > 
> > > > > Only the default rounding mode is intended for best performances.
> > > For aarch64, implementing `__fegetround` in assembly also looks good to me. In musl http://git.musl-libc.org/cgit/musl/tree/src/fenv/aarch64/fenv.s
> > > 
> > > ```
> > > .global fegetround
> > > .type fegetround,%function
> > > fegetround:
> > > 	mrs x0, fpcr
> > > 	and w0, w0, #0xc00000
> > > 	ret
> > > ```
> > Added check for `__ARM_FP`. Fallback to default rounding if VFP is not available.
> I'm a little concerned this will do something unexpected on soft-float ARM Linux (which is usually built for a target without VFP support, but often runs on a chip that actually supports it).  I guess always disabling other rounding modes is more intuitive than enabling them conditionally, though; should be fine.
In this situation I'm not sure we can do much better in generic compiler-rt. If we thought it would be necessary to allow change from FE_TONEAREST one possibility would be to replace the return FE_TONEAREST in
```
#else
  return FE_TONEAREST;
#endif
```
With a call to a weak definition of another function returning FE_TONEAREST . A user that cared sufficiently about the rounding mode could then reimplement this function with the rounding mode of their choice. This would be a bit easier than adding definitions of both __fe_raise_inexact and __fe_getround but not much.




Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D57143