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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to