[PATCH] D125625: Implementation of '#pragma STDC FENV_ROUND'

Mon Jul 11 05:54:58 PDT 2022

sepavloff added inline comments.

================
Comment at: clang/lib/CodeGen/CGStmt.cpp:488
+  }
+  if (NewRM != llvm::RoundingMode::Dynamic)
+    Builder.CreateIntrinsic(
----------------
aaron.ballman wrote:
> I was imagining that if the new rounding mode is dynamic, we still need to reset the rounding mode back to what it was when the thread was created or to the previous rounding mode set by a library call, but now I'm not certain. I commented on one of the test cases about what I was thinking.
Yes, it is required by the standard draft (7.6.2p2):
```
... at the end of a compound statement the static rounding
mode is restored to its condition just before the compound statement.
```
Rounding mode is not set if FE_DYNAMIC is in effect, but it is restored upon leaving such region, see test func_4 in pragma-fenv_round.c.

================
Comment at: clang/test/CodeGen/pragma-fenv_round.c:54-55
+// CHECK:  call float @llvm.experimental.constrained.fmul.f32({{.*}}, metadata !"round.towardzero", metadata !"fpexcept.ignore")
+// CHECK:  call void @llvm.set.rounding(i32 1)
+// CHECK:  call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+// CHECK:  call void @llvm.set.rounding(i32 0)
----------------
aaron.ballman wrote:
> I was trying to figure out whether this was correct or not when I was looking at the codegen logic. C2x 7.6.2p3 says "... If no FENV_ROUND pragma is in effect, or the specified constant rounding mode is FE_DYNAMIC, rounding is according to the mode specified by the dynamic floating-point environment, which is the dynamic rounding mode that was established either at thread creation time or by a call to fesetround, fesetmode, fesetenv, or feupdateenv. ..." p2 says: "... When outside external declarations, the pragma takes effect from its occurrence until another FENV_ROUND pragma is encountered, or until the end of the translation unit. When inside a compound statement, the pragma takes effect from its first occurrence until another FENV_ROUND pragma is encountered (including within a nested compound statement), or until the end of the compound statement; ...". Finally, p4 says: "... Within the scope of an FENV_ROUND pragma establishing a mode other than FE_DYNAMIC, ... shall be evaluated according to the specified constant rounding mode (as though no constant mode was specified and the corresponding dynamic rounding mode had been established by a call to fesetround)."
> 
> I *think* this means that the rounding mode for `result += z;` should actually be `FE_TOWARDZERO`, but I'm not 100% sure. We enter the compound statement for the function body, then we enter a constant rounding mode of FE_TOWARDZERO. Then we get into a new nested compound statement, and we enter the rounding mode of FE_DYNAMIC, which says it should behave in the manner of the last call to fesetround, which was (as-if) called by the earlier pragma.
> 
> Thoughts?
> 
> I would be good to have a test for the behavior at file scope as well as at compound scope, e.g.,
> ```
> float default_mode = 1.0f / 3.0f;
> 
> #pragma STDC FENV_ROUND FE_UPWARD  
> float up = 1.0f / 3.0f;
> 
> #pragma STDC FENV_ROUND FE_DOWNWARD
> float down = 1.0f / 3.0f;
> 
> #pragma STDC FENV_ROUND FE_DYNAMIC
> float dynamic = 1.0f / 3.0f;
> ```
> I'd expect the `up` and `down` cases to have different values, and I'd expect the `dynamic` case to either be the same as `down` or the same as `default_mode`.
It is easier to rationalize effect of FENV_ROUND if target supports static rounding mode, like RISCV. On such target the rounding mode may be specified in every instruction that depends on it. Such instruction does not depend on FP environment, the rounding mode is known at compile-time, and it perfectly fits notion of constant rounding mode provided in the standard. There is also a special value of rounding mode that does not designates concrete rounding but instruct the processor to take rounding mode from a special register. This is dynamic rounding mode, In general it is unknown at compile-time and may be set, for example, by a call to `fesetround`. On such target FE_DYNAMIC means that instructions are created with dynamic rounding mode, while any other mode selected by FENV_ROUND results in concrete static rounding mode.

As for this example (func_3), the first pragma sets FE_TOWARDZERO mode. This is a static rounding mode, it is known in compile-time. In the nested compound statement pragma FENV_ROUND FE_DYNAMIC requires that FP operations use dynamic rounding mode. It means that actual rounding mode is known in runtime and the actual rounding mode must be set somewhere by a call to `fesetround` or some other manner. In this test no 'FENV_ACCESS` is specified, to compiler may assume the dynamic rounding mode is FE_TONEAREST. In the test `func_4` there is `pragma FENV_ACCESS ON` and rounding mode specified in the FP operation is `dynamic`.

> I would be good to have a test for the behavior at file scope as well as at compound scope, e.g.,

Added. FE_DYNAMIC in this case is identical to FE_TONEAREST, because initial dynamic rounding mode is such.

================
Comment at: clang/test/CodeGen/pragma-fenv_round.c:71
+
+
+#pragma STDC FENV_ROUND FE_DOWNWARD
----------------
aaron.ballman wrote:
> I think it'd be nice to add a test case here like:
> ```
> float func_21(float x, float y) {
>   return x + y;
> }
> ```
> to demonstrate that FE_DOWNWARD is still in effect within this function too.
Added.

================
Comment at: clang/test/CodeGen/pragma-fenv_round.c:92
+}
+
+// CHECK-LABEL: @func_22
----------------
aaron.ballman wrote:
> Another fun test case to add would be a C++ case:
> ```
> #pragma STDC FENV_ROUND FE_DOWNWARD
> float func_cxx(float x, float y, float z = 1.0f/3.0f) {
>   return x + y + z;
> }
> 
> int main() {
> #pragma STDC FENV_ROUND FE_UPWARD
>   func_cxx(1.0f, 2.0f);
> }
> ```
> is `z` evaluated as upward (because default arguments are evaluated at the call site IIRC), or downward (because of the rounding mode when defining the function)?
> 
> I suppose another interesting case is:
> ```
> inline float func(float x, float y) {
> #pragma STDC FENV_ROUND FE_DOWNWARD
>   return x + y;
> }
> 
> int main(void) {
>   func(1.0f, 2.0f);
>   float f = 1.0f / 3.0f;
> }
> ```
> Even if the call to `func` is truly inlined, I still wouldn't expect `f` to be calculated in downward (unless that's the default rounding mode). Is that how you read the standard as well?
As FENV_ROUND is defined in C standard, its interaction with C++ is undefined. In this case it is reasonable to have "most obvious" behavior, although it is not well-defined without standard. In this case default argument initializer is specified in the region where `#pragma STDC FENV_ROUND FE_DOWNWARD` is in effect, so it seems natural to have the initialized calculated with this mode. Of course, this behavior may be changed. Added some relevant tests in pragma-fenv_round.cpp.

Inlining is made at llvm part, there are tests in https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/Inline/inline-strictfp.ll.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125625