[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Thanks, I'm happy with this, though please wait a day or two for comments from the other reviewers.

We should also add some documentation covering the rules we're using for the floating-point environment in C++, particularly around initializers for variables with static storage duration, given that that's not governed by any language standard.



================
Comment at: clang/test/CodeGen/fp-floatcontrol-pragma.cpp:159
+  //CHECK: store float 3.0{{.*}}retval{{.*}}
+  static double v = 1.0 / 3.0;
+  //CHECK-FENV: llvm.experimental.constrained.fptrunc.f32.f64{{.*}}
----------------
mibintc wrote:
> @rsmith - the division is constant folded.  is that right? 
Yes, that's in line with our new expectations for C++. Specifically: for static / thread-local variables, first try evaluating the initializer in a constant context, including in the constant floating point environment (just like in C), and then, if that fails, fall back to emitting runtime code to perform the initialization (which might in general be in a different FP environment). Given that `1.0 / 3.0` is a constant initializer, it should get folded using the constant rounding mode. But, for example:

```
double f() {
  int n = 0;
  static double v = 1.0 / 3.0 + 0 * n;
  return v;
}
```

... should emit a constrained `fdiv` at runtime, because the read of `n` causes the initializer to not be a constant initializer. And similarly:

```
double f() {
  double v = 1.0 / 3.0 + 0 * n;
  return v;
}
```

... should emit a constrained `fdiv`, because `v` is not a static storage duration variable, so there is formally no "first try evaluating the initializer as a constant" step.


================
Comment at: clang/test/Parser/pragma-fenv_access.c:28
+#if defined(CPP) & defined(STRICT)
+//not-expected-error at +3 {{constexpr variable 'frac' must be initialized by a constant expression}}
+//not-expected-note at +2 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
----------------
mibintc wrote:
> @rsmith no diagnostic, is this OK?
Under the new approach from D89360, I would expect no diagnostic with `CONST` defined as `constexpr`, because the initializer is then manifestly constant-evaluated, so should be evaluated in the constant rounding mode.

With `CONST` defined as merely `const`, I'd expect that we emit a constrained floating-point operation using the runtime rounding mode here. You can test that we don't constant-evaluate the value of a (non-`constexpr`) `const float` variable by using this hack (with `CONST` defined as `const`):

```
enum {
  e1 = (int)one, e3 = (int)three, e4 = (int)four, e_four_quarters = (int)(frac_ok * 4)
};
static_assert(e1 == 1  && e3 == 3 && e4 == 4 && e_four_quarters == 1, "");
enum {
  e_three_thirds = (int)(frac * 3) // expected-error {{not an integral constant expression}}
};
```

(The hack here is that we permit constant folding in the values of enumerators, and we allow constant folding to look at the evaluated values of `const float` variables. This should not be allowed for `frac`, because attempting to evaluate its initializer should fail.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528