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

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff marked 2 inline comments as done.
sepavloff added a comment.

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.

> Add to the IR verifier checks that the input is a constant that matches the documentation. The type and input values all need to be checked.

The type of input value is already checked by generic code. As for values, there are two notes.

1. Input values may be a variable, not constant.
2. A target may support non-standard rounding modes.

However constant values are limited by 3 bits, of which one (Dynamic) cannot be used as argument. So adding check to IR verifier makes sense.



================
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. 
----------------
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.


================
Comment at: llvm/docs/LangRef.rst:18146-18150
+    0  - toward zero
+    1  - to nearest, ties to even
+    2  - toward positive infinity
+    3  - toward negative infinity
+    4  - to nearest, ties away from zero
----------------
RKSimon wrote:
> arsenm wrote:
> > I'm wondering if this should be more opaque, and broader for the entire FP environment (not just the rounding mode). For AMDGPU we have a number of additional bits in the FP environment. We also have the denormal mode, enabling FP exceptions, and a few more exotic target specific FP mode bits.
> @arsenm Would these extra bits be exclusive modes or would you need this to support target specific mode combos?
> I'm wondering if this should be more opaque, and broader for the entire FP environment (not just the rounding mode). 

C library defined function `fesetmode`, which sets all control modes, not just rounding. It make sense to introduce intrinsic for it, which would serve these purposes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74729