[PATCH] D74729: [FPEnv] Intrinsic for setting rounding mode

Mon May 4 05:50:08 PDT 2020

sepavloff marked an inline comment as done.
sepavloff added a comment.

In D74729#2008437 <https://reviews.llvm.org/D74729#2008437>, @kpn wrote:

> In D74729#2008093 <https://reviews.llvm.org/D74729#2008093>, @sepavloff wrote:
>
> > In D74729#2005958 <https://reviews.llvm.org/D74729#2005958>, @kpn wrote:
> >
> > > I think we need to be clear in the documentation that this is _not_ a substitute for the constrained FP intrinsics. Can you please add that to your new documentation? Include a link to the "Floating-Point Environment" section in the language ref.
> >
> >
> > It cannot be such substitute. IIUC constrained intrinsics were introduced to represent variants of corresponding C functions that operate in non-default FP environment to distinguish them from "ordinary" variants, that are pure functions. Functions like `set_rounding` always access FP environment, it always have side effect and must be properly ordered.
>
>
> Sure, you know that, and I know that, but someone who hasn't been following along the past couple of years may not know that. A sentence or two tying things together won't hurt. Something along the lines of "Altering the rounding mode requires special care. See 'Floating-Point Environment'.", with a link to that section of the documentation.

Ah, got it! Added note to the documentation. Thank you!

================
Comment at: llvm/docs/LangRef.rst:18120
+
+These functions read or write floating point environment, such as rounding
+mode or state of floating point exceptions. 
----------------
sepavloff wrote:
> RKSimon wrote:
> > Remove "read or " ?
> This is a section for group of intrinsics. Now there is only one intrinsic in it, which indeed only writes FP environment. It makes sense to implement intrinsics fo standard C functions, like `fegetmode`, `fetestexcept` and others.
> 
> Actually the intrinsic `flt_rounds`  may be documented here. I will add documentation for it.
Added `flt_rounds` in D79322.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74729