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

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/lib/Sema/SemaAttr.cpp:1020-1023
+// Resume the default rounding and exception modes.
+NewFPFeatures.setRoundingModeOverride(
+llvm::RoundingMode::NearestTiesToEven);
+NewFPFeatures.setFPExceptionModeOverride(LangOptions::FPE_Ignore);

mibintc wrote:
> sepavloff wrote:
> > The previous values of rounding mode and exception behavior may differ from 
> > the default values. For example, `#pragma STDC FENV_ROUND` may set constant 
> > rounding direction different from FE_TONEAREST`. Similarly, command line 
> > options may set exception behavior different from `ignore`.
> > Can pragma stack be used for this?
> > The previous values of rounding mode and exception behavior may differ from 
> > the default values. For example, `#pragma STDC FENV_ROUND` may set constant 
> > rounding direction different from FE_TONEAREST`. Similarly, command line 
> > options may set exception behavior different from `ignore`.
> > Can pragma stack be used for this?
> 
> I guess I could just NewFPFeatures.setAllowFEnvAccessOverride(false); and 
> leave the other 2 unset, does that sound right? The values of the FPFeatures 
> are being preserved around compound statements with FPFeaturesStateRAII
> I guess I could just NewFPFeatures.setAllowFEnvAccessOverride(false); and 
> leave the other 2 unset, does that sound right?

It should fix the most important case:
```
// RUN: %clang_cc1 -S -emit-llvm %s -o - | FileCheck %s

float func_01(float x, float y, float z) {
  #pragma STDC FENV_ROUND FE_DOWNWARD
  float res = x + y;
  {
#pragma STDC FENV_ACCESS OFF
res *= z;
  }
  return res - x;
}

// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, 
float {{.*}}, metadata !"round.downward", metadata !"fpexcept.ignore")
// CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, 
float {{.*}}, metadata !"round.downward", metadata !"fpexcept.ignore")
// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, 
float {{.*}}, metadata !"round.downward", metadata !"fpexcept.ignore")
```
Another case, which now fails is:
```
// RUN: %clang_cc1 -S -emit-llvm -frounding-math %s -o - | FileCheck %s

float func_01(float x, float y, float z) {
  float res = x + y;
  {
#pragma STDC FENV_ACCESS OFF
res *= z;
  }
  return res - x;
}

// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, 
float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
// CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, 
float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, 
float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.ignore")
```
The function may be called from a context, where rounding mode is not default, 
command-line options allow that. So rounding mode inside the inner block is 
dynamic, although FP state is not accessed in it.

Now consider exception behavior. For example, the test case, which now passes:
```
// RUN: %clang_cc1 -S -emit-llvm -ffp-exception-behavior=strict %s -o - | 
FileCheck %s

float func_01(float x, float y, float z) {
  float res = x + y;
  {
#pragma STDC FENV_ACCESS OFF
res *= z;
  }
  return res - x;
}

// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, 
float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
// CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, 
float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, 
float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
```
... is correct IMHO. Setting FENV_ACCESS to OFF means the code in the affected 
region does not check FP exceptions, so they can be ignored. This is a way to 
have default FP state even if the command-line options set FENV_ACCESS. As for 
`maytrap` I am not sure, but I think the following test should pass:  
```
// RUN: %clang_cc1 -S -emit-llvm -ffp-exception-behavior=maytrap %s -o - | 
FileCheck %s

float func_01(float x, float y, float z) {
  float res = x + y;
  {
#pragma STDC FENV_ACCESS OFF
res *= z;
  }
  return res - x;
}

// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, 
float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
// CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, 
float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, 
float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
```
The outer code may set hardware for trapping and although in the inner block 
there is no access to FP state, traps are possible and compiler must be aware 
of them.

And finally the following 

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

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

rsmith added inline comments.



Comment at: clang/test/Parser/pragma-fenv_access.c:28
+#if defined(CPP) & defined(STRICT)
+//not-expected-error@+3 {{constexpr variable 'frac' must be initialized by a 
constant expression}}
+//not-expected-note@+2 {{compile time floating point arithmetic suppressed in 
strict evaluation modes}}

mibintc wrote:
> rsmith wrote:
> > 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.)
> I'm not seeing the expected-error for e_three_thirds = (int)(frac * 3)
Are you sync'd past commit rG08c8d5bc51c512e605840b8003fcf38c86d0fc96? We had a 
bug that would incorrectly treat the initializer of `frac` as being in a 
constant context until pretty recently.

This testcase:

```
int main() {
  const float f = 1.0 / 3.0;
  enum { e = (int)(f * 3) };
}
```

... produces an error under `-frounding-math` but works without 
`-frounding-math` with recent trunk.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added inline comments.



Comment at: clang/test/Parser/pragma-fenv_access.c:28
+#if defined(CPP) & defined(STRICT)
+//not-expected-error@+3 {{constexpr variable 'frac' must be initialized by a 
constant expression}}
+//not-expected-note@+2 {{compile time floating point arithmetic suppressed in 
strict evaluation modes}}

rsmith wrote:
> 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.)
I'm not seeing the expected-error for e_three_thirds = (int)(frac * 3)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added inline comments.



Comment at: clang/lib/Sema/SemaAttr.cpp:1020-1023
+// Resume the default rounding and exception modes.
+NewFPFeatures.setRoundingModeOverride(
+llvm::RoundingMode::NearestTiesToEven);
+NewFPFeatures.setFPExceptionModeOverride(LangOptions::FPE_Ignore);

sepavloff wrote:
> The previous values of rounding mode and exception behavior may differ from 
> the default values. For example, `#pragma STDC FENV_ROUND` may set constant 
> rounding direction different from FE_TONEAREST`. Similarly, command line 
> options may set exception behavior different from `ignore`.
> Can pragma stack be used for this?
> The previous values of rounding mode and exception behavior may differ from 
> the default values. For example, `#pragma STDC FENV_ROUND` may set constant 
> rounding direction different from FE_TONEAREST`. Similarly, command line 
> options may set exception behavior different from `ignore`.
> Can pragma stack be used for this?

I guess I could just NewFPFeatures.setAllowFEnvAccessOverride(false); and leave 
the other 2 unset, does that sound right? The values of the FPFeatures are 
being preserved around compound statements with FPFeaturesStateRAII


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff accepted this revision.
sepavloff added a comment.

LGTM

There is concern with restoring FP options but it could be solved separately.

Looking forward to results of SPEC runs!




Comment at: clang/docs/UsersManual.rst:1389
* ``precise`` Disables optimizations that are not value-safe on 
floating-point data, although FP contraction (FMA) is enabled 
(``-ffp-contract=fast``).  This is the default behavior.
-   * ``strict`` Enables ``-frounding-math`` and 
``-ffp-exception-behavior=strict``, and disables contractions (FMA).  All of 
the ``-ffast-math`` enablements are disabled.
+   * ``strict`` Enables ``-frounding-math`` and 
``-ffp-exception-behavior=strict``, and disables contractions (FMA).  All of 
the ``-ffast-math`` enablements are disabled. Enables ``STDC FENV_ACCESS``.
* ``fast`` Behaves identically to specifying both ``-ffast-math`` and 
``ffp-contract=fast``

For a user manual `Enables STDC FENV_ACCESS` looks too vague. Probably we need 
to reword it to make more clear for compiler users. Something like `Assumes 
implicit #pragma STDC FENV_ACCESS ON at the beginning of source files`. Or, 
maybe this statement could be dropped.



Comment at: clang/lib/Sema/SemaAttr.cpp:1020-1023
+// Resume the default rounding and exception modes.
+NewFPFeatures.setRoundingModeOverride(
+llvm::RoundingMode::NearestTiesToEven);
+NewFPFeatures.setFPExceptionModeOverride(LangOptions::FPE_Ignore);

The previous values of rounding mode and exception behavior may differ from the 
default values. For example, `#pragma STDC FENV_ROUND` may set constant 
rounding direction different from FE_TONEAREST`. Similarly, command line 
options may set exception behavior different from `ignore`.
Can pragma stack be used for this?



Comment at: clang/lib/Sema/SemaDecl.cpp:14281-14282
 
+  if (FSI->UsesFPIntrin)
+assert(FD);
+  if (FSI->UsesFPIntrin && !FD->hasAttr())

It is better to put `assert` into compound statement to avoid compiler 
confusion in release mode. Or to use something like: `assert(!FSI->UsesFPIntrin 
|| FD)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

[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@+3 {{constexpr variable 'frac' must be initialized by a 
constant expression}}
+//not-expected-note@+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

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 299426.
mibintc added a comment.

I made the change requested by @rsmith to HandleIntToFloatCast, avoiding the 
check if Info.InConstantContext, then I corrected 2 tests so that -verify 
matches.

  clang/test/CXX/expr/expr.const/p2-0x.cpp
  clang/test/Parser/pragma-fenv_access.c

(I had 30 lit tests failing in my sandbox but I fixed the logic, so please 
ignore that remark from my previous revision)
check-clang is running clean


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/ScopeInfo.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CXX/expr/expr.const/p2-0x.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/pragma-fenv_access.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  clang/test/Parser/pragma-fenv_access.c
  clang/test/Preprocessor/pragma_unknown.c

Index: clang/test/Preprocessor/pragma_unknown.c
===
--- clang/test/Preprocessor/pragma_unknown.c
+++ clang/test/Preprocessor/pragma_unknown.c
@@ -16,15 +16,6 @@
 // CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT IN_BETWEEN{{$}}
 
-#pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS OFF
-#pragma STDC FENV_ACCESS DEFAULT
-#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS ON{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS OFF{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS DEFAULT{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS IN_BETWEEN{{$}}
-
 #pragma STDC CX_LIMITED_RANGE ON
 #pragma STDC CX_LIMITED_RANGE OFF
 #pragma STDC CX_LIMITED_RANGE DEFAULT 
Index: clang/test/Parser/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/Parser/pragma-fenv_access.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -DSTRICT -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DCPP -DSTRICT -ffp-exception-behavior=strict -fsyntax-only -verify %s
+#ifdef CPP
+#define CONST constexpr
+#else
+#define CONST const
+#endif
+
+#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(int x, float y) {
+  if (x)
+return y + 2;
+  #pragma STDC FENV_ACCESS ON // expected-error{{'#pragma STDC FENV_ACCESS' can only appear at file scope or at the start of a compound statement}}
+  return x + y;
+}
+
+#pragma STDC FENV_ACCESS ON
+int main() {
+  CONST float one = 1.0F ;
+  CONST float three = 3.0F ;
+  CONST float four = 4.0F ;
+  CONST float frac_ok = one/four;
+#if defined(CPP) & defined(STRICT)
+//not-expected-error@+3 {{constexpr variable 'frac' must be initialized by a constant expression}}
+//not-expected-note@+2 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+  CONST float frac = one/three; // rounding
+  CONST double d = one;
+  CONST int not_too_big = 255;
+  CONST float fnot_too_big = not_too_big;
+  CONST int too_big = 0x7ff0;
+#if defined(CPP) & defined(STRICT)
+//not-expected-error@+6 {{constexpr variable 'fbig' must be initialized by a constant expression}}
+//not-expected-note@+5 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+#if defined(CPP)
+//expected-warning@+2{{implicit conversion}}
+#endif
+  CONST float fbig = too_big; // inexact
+  if (one <= four)  return 0;
+  return -1;
+}
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -26,19 +26,13 @@
 double a = 0.0;
 double b = 1.0;
 
-//FIXME At some point this warning will be removed, until then
-//  document the warning
-#ifdef FAST
-// expected-warning@+1{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}

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

[Melanie Blower via Phabricator via cfe-commits]

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

In D87528#2270541 , @rsmith wrote:

>> @rsmith asked "I don't see changes to the constant evaluator". The semantic 
>> rules for enabling this pragma require that strict or precise semantics be 
>> in effect., see SemaAttr.cpp And the floating point constant evaluation 
>> doesn't occur when strict or precise
>
> What do you mean "the floating point constant evaluation doesn't occur when 
> strict or precise"? I don't see any code to do that in `ExprConstant.cpp`, 
> neither in trunk nor in this patch. The constant evaluator certainly is still 
> invoked when we're in strict or precise mode. I would expect the evaluator 
> would need to check for FP strictness whenever it encounters a floating-point 
> operator (eg, here: 
> https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/ExprConstant.cpp#L13340),
>  and treat such cases as non-constant.

1.0/3.0 is now static folded.  @rsmith Did you change your mind about this?  Is 
static folding what you want here?

> I'd like to see a C++ test for something like this:
>
>   float myAdd() {
>   #pragma STDC FENV_ACCESS ON
>   static double v = 1.0 / 3.0;
>   return v;
>   }
>
> If that generates a runtime guarded initialization for `v`, then I'd be a bit 
> more convinced we're getting this right.

Some inline comments, still owe you a list of test that fails.  check-clang 
runs clean with this patch




Comment at: clang/lib/AST/ExprConstant.cpp:2496-2498
+  if (llvm::APFloatBase::opOK != Result.convertFromAPInt(Value,
+   Value.isSigned(),
+   APFloat::rmNearestTiesToEven) &&

rsmith wrote:
> Following D89360, we should skip this check if `Info.InConstantContext`.
I tried making this change but I got about 30 lit fails, maybe my patch was 
incorrect. I'll rerun with the patch and post the list of fails. 



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{{.*}}

@rsmith - the division is constant folded.  is that right? 



Comment at: clang/test/Parser/pragma-fenv_access.c:28
+#if defined(CPP) & defined(STRICT)
+//not-expected-error@+3 {{constexpr variable 'frac' must be initialized by a 
constant expression}}
+//not-expected-note@+2 {{compile time floating point arithmetic suppressed in 
strict evaluation modes}}

@rsmith no diagnostic, is this OK?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 299395.
mibintc added a comment.
Herald added a reviewer: aaron.ballman.
Herald added a subscriber: dexonsmith.

I added StrictFPAttr clang function attribute accroding to @rjmccall 's 
suggestion instead of using a boolean on the function decl, rebased, and 
responded to other code review comments. will add replies inline


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/ScopeInfo.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CXX/expr/expr.const/p2-0x.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/pragma-fenv_access.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  clang/test/Parser/pragma-fenv_access.c
  clang/test/Preprocessor/pragma_unknown.c

Index: clang/test/Preprocessor/pragma_unknown.c
===
--- clang/test/Preprocessor/pragma_unknown.c
+++ clang/test/Preprocessor/pragma_unknown.c
@@ -16,15 +16,6 @@
 // CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT IN_BETWEEN{{$}}
 
-#pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS OFF
-#pragma STDC FENV_ACCESS DEFAULT
-#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS ON{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS OFF{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS DEFAULT{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS IN_BETWEEN{{$}}
-
 #pragma STDC CX_LIMITED_RANGE ON
 #pragma STDC CX_LIMITED_RANGE OFF
 #pragma STDC CX_LIMITED_RANGE DEFAULT 
Index: clang/test/Parser/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/Parser/pragma-fenv_access.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -DSTRICT -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DCPP -DSTRICT -ffp-exception-behavior=strict -fsyntax-only -verify %s
+#ifdef CPP
+#define CONST constexpr
+#else
+#define CONST const
+#endif
+
+#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(int x, float y) {
+  if (x)
+return y + 2;
+  #pragma STDC FENV_ACCESS ON // expected-error{{'#pragma STDC FENV_ACCESS' can only appear at file scope or at the start of a compound statement}}
+  return x + y;
+}
+
+#pragma STDC FENV_ACCESS ON
+int main() {
+  CONST float one = 1.0F ;
+  CONST float three = 3.0F ;
+  CONST float four = 4.0F ;
+  CONST float frac_ok = one/four;
+#if defined(CPP) & defined(STRICT)
+//not-expected-error@+3 {{constexpr variable 'frac' must be initialized by a constant expression}}
+//not-expected-note@+2 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+  CONST float frac = one/three; // rounding
+  CONST double d = one;
+  CONST int not_too_big = 255;
+  CONST float fnot_too_big = not_too_big;
+  CONST int too_big = 0x7ff0;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+6 {{constexpr variable 'fbig' must be initialized by a constant expression}}
+//expected-note@+5 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+#if defined(CPP)
+//expected-warning@+2{{implicit conversion}}
+#endif
+  CONST float fbig = too_big; // inexact
+  if (one <= four)  return 0;
+  return -1;
+}
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -26,19 +26,13 @@
 double a = 0.0;
 double b = 1.0;
 
-//FIXME At some point this warning will be removed, until then
-//  document the warning
-#ifdef FAST
-// expected-warning@+1{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS ON
-#else
-#pragma STDC FENV_ACCESS ON // expected-warning{{pragma STDC FENV_ACCESS ON is 

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

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

rsmith added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:2445
+   FPO.getFPExceptionMode() != LangOptions::FPE_Ignore ||
+   FPO.getAllowFEnvAccess()) && Info.Ctx.getLangOpts().CPlusPlus) {
+Info.FFDiag(E, diag::note_constexpr_float_arithmetic_strict);

I don't think we need the `CPlusPlus` check here; we bail out of this function 
in constant contexts in all language modes, and in non-constant contexts we 
want to abort evaluation under this condition in C too.



Comment at: clang/lib/AST/ExprConstant.cpp:2496-2498
+  if (llvm::APFloatBase::opOK != Result.convertFromAPInt(Value,
+   Value.isSigned(),
+   APFloat::rmNearestTiesToEven) &&

Following D89360, we should skip this check if `Info.InConstantContext`.



Comment at: clang/lib/AST/ExprConstant.cpp:12302
+llvm::APFloatBase::cmpResult CmpResult = LHS.compare(RHS);
+if (CmpResult == APFloat::cmpUnordered &&
+E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {

Skip this check if `Info.InConstantContext`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

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

rsmith added a comment.

FYI: I posted a patch to implement the "manifestly constant evaluated" rule as 
https://reviews.llvm.org/D89360. It looks like the stricter rule doesn't work 
in practice; far too much C++ code uses `-frounding-math` and expects to be 
able to still include things like standard library headers that define 
`constexpr` functions and variables that involve floating-point rounding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

The GCC docs seem pretty clear that `-frounding-math` is meant to allow dynamic 
access to the FP environment.  There might be optimizations that are perfectly 
legal with a constant but non-default rounding mode, and there might be some 
flag that only implies that weaker constraint, but `-frounding-math` needs to 
turn `FENV_ACCESS` on by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D88498  @rsmith wrote,
s far from clear to me that this is correct in C++. In principle, for a dynamic 
initializer, the rounding mode could have been set by an earlier initializer.

Perhaps we can make an argument that, due to the permission to interleave 
initializers from different TUs, every dynamic initializer must leave the 
program in the default rounding mode, but I don't think even that makes this 
approach correct, because an initializer could do this:

double d;
double e = (fesetround(...), d = some calculation, fesetround(...default...), 
d);
I think we can only do this in C and will need something different for C++.

(I think this also has problems in C++ due to constexpr functions: it's not the 
case that all floating point operations that will be evaluated as part of the 
initializer lexically appear within it.)

For the initialization expression to be correct, the RoundingMode at the 
declaration of e needs to be set to Dynamic, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 297260.
mibintc added a comment.

I rebased the patch
I added documentation to UserManual showing that -ffp-model=strict enables 
FENV_ACCESS
I removed ActOnAfterCompoundStatementLeadingPragmas since it was redundant with 
work being done in ActOnCompoundStmt

This patch is based on D69272 , perhaps i 
should merge both patches and upload them into this single review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/AST/const-fpfeatures-diag.c
  clang/test/CXX/expr/expr.const/p2-0x.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/pragma-fenv_access.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  clang/test/Parser/pragma-fenv_access.c
  clang/test/Preprocessor/pragma_unknown.c

Index: clang/test/Preprocessor/pragma_unknown.c
===
--- clang/test/Preprocessor/pragma_unknown.c
+++ clang/test/Preprocessor/pragma_unknown.c
@@ -16,15 +16,6 @@
 // CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT IN_BETWEEN{{$}}
 
-#pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS OFF
-#pragma STDC FENV_ACCESS DEFAULT
-#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS ON{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS OFF{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS DEFAULT{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS IN_BETWEEN{{$}}
-
 #pragma STDC CX_LIMITED_RANGE ON
 #pragma STDC CX_LIMITED_RANGE OFF
 #pragma STDC CX_LIMITED_RANGE DEFAULT 
Index: clang/test/Parser/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/Parser/pragma-fenv_access.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -DSTRICT -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DCPP -DSTRICT -ffp-exception-behavior=strict -fsyntax-only -verify %s
+#ifdef CPP
+#define CONST constexpr
+#else
+#define CONST const
+#endif
+
+#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(int x, float y) {
+  if (x)
+return y + 2;
+  #pragma STDC FENV_ACCESS ON // expected-error{{'#pragma STDC FENV_ACCESS' can only appear at file scope or at the start of a compound statement}}
+  return x + y;
+}
+
+#pragma STDC FENV_ACCESS ON
+int main() {
+  CONST float one = 1.0F ;
+  CONST float three = 3.0F ;
+  CONST float four = 4.0F ;
+  CONST float frac_ok = one/four;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+3 {{constexpr variable 'frac' must be initialized by a constant expression}}
+//expected-note@+2 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+  CONST float frac = one/three; // rounding
+  CONST double d = one;
+  CONST int not_too_big = 255;
+  CONST float fnot_too_big = not_too_big;
+  CONST int too_big = 0x7ff0;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+6 {{constexpr variable 'fbig' must be initialized by a constant expression}}
+//expected-note@+5 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+#if defined(CPP)
+//expected-warning@+2{{implicit conversion}}
+#endif
+  CONST float fbig = too_big; // inexact
+  if (one <= four)  return 0;
+  return -1;
+}
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -26,19 +26,13 @@
 double a = 0.0;
 double b = 1.0;
 
-//FIXME At some point this warning will be removed, until then
-//  document the warning
-#ifdef FAST
-// expected-warning@+1{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS ON
-#else
-#pragma STDC FENV_ACCESS ON // expected-warning{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#endif
 #ifdef STRICT
 #pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when except is enabled}}
 #else
-// Currently FENV_ACCESS cannot be enabled by pragma, skip error check

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 296520.
mibintc added a comment.

I added a sentence in the clang UserManual pointing out that ffp-model=strict 
automatically enables FENV_ACCESS. I changed checkFloatingPointResult the way 
@sepavloff suggested to solve the LIT failure in 
clang/test/AST/const-fpfeatures.cpp, as long as the ExceptionBehavior is 
Ignore, the FENV_ACCESS is Off and RoundingMode isn't Dynamic, it's OK to fold 
even if the Status isn't OK. Now check-clang is running clean.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/AST/const-fpfeatures-diag.c
  clang/test/CXX/expr/expr.const/p2-0x.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/pragma-fenv_access.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  clang/test/Parser/pragma-fenv_access.c
  clang/test/Preprocessor/pragma_unknown.c

Index: clang/test/Preprocessor/pragma_unknown.c
===
--- clang/test/Preprocessor/pragma_unknown.c
+++ clang/test/Preprocessor/pragma_unknown.c
@@ -16,15 +16,6 @@
 // CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT IN_BETWEEN{{$}}
 
-#pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS OFF
-#pragma STDC FENV_ACCESS DEFAULT
-#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS ON{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS OFF{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS DEFAULT{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS IN_BETWEEN{{$}}
-
 #pragma STDC CX_LIMITED_RANGE ON
 #pragma STDC CX_LIMITED_RANGE OFF
 #pragma STDC CX_LIMITED_RANGE DEFAULT 
Index: clang/test/Parser/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/Parser/pragma-fenv_access.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -DSTRICT -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DCPP -DSTRICT -ffp-exception-behavior=strict -fsyntax-only -verify %s
+#ifdef CPP
+#define CONST constexpr
+#else
+#define CONST const
+#endif
+
+#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(int x, float y) {
+  if (x)
+return y + 2;
+  #pragma STDC FENV_ACCESS ON // expected-error{{'#pragma STDC FENV_ACCESS' can only appear at file scope or at the start of a compound statement}}
+  return x + y;
+}
+
+#pragma STDC FENV_ACCESS ON
+int main() {
+  CONST float one = 1.0F ;
+  CONST float three = 3.0F ;
+  CONST float four = 4.0F ;
+  CONST float frac_ok = one/four;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+3 {{constexpr variable 'frac' must be initialized by a constant expression}}
+//expected-note@+2 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+  CONST float frac = one/three; // rounding
+  CONST double d = one;
+  CONST int not_too_big = 255;
+  CONST float fnot_too_big = not_too_big;
+  CONST int too_big = 0x7ff0;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+6 {{constexpr variable 'fbig' must be initialized by a constant expression}}
+//expected-note@+5 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+#if defined(CPP)
+//expected-warning@+2{{implicit conversion}}
+#endif
+  CONST float fbig = too_big; // inexact
+  if (one <= four)  return 0;
+  return -1;
+}
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -26,19 +26,13 @@
 double a = 0.0;
 double b = 1.0;
 
-//FIXME At some point this warning will be removed, until then
-//  document the warning
-#ifdef FAST
-// expected-warning@+1{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS ON
-#else
-#pragma STDC FENV_ACCESS ON // expected-warning{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#endif
 #ifdef STRICT
 #pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when except is enabled}}
 #else
-// Currently 

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

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D87528#2312230 , @mibintc wrote:

> For the LIT test clang/test/AST/const-fpfeatures.cpp, currently failing in 
> this patch, the variables V1 and others are initialized via call to "global 
> var init" which performs the rounding at execution time, I think that's 
> correct not certain.

This file does not contain `pragma STDC FENV_ACCESS`. As for using `pragma STDC 
FENV_ROUND` in constexpr functions, I don't see any reason to prohibit it.

Could it be a result of setting rounding mode to `dynamic`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

For the LIT test clang/test/AST/const-fpfeatures.cpp, currently failing in this 
patch, the variables V1 and others are initialized via call to "global var 
init" which performs the rounding at execution time, I think that's correct not 
certain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 296221.
mibintc added a comment.

In the previous version of this patch, I enabled FENV_ACCESS if frounding-math, 
but @sepavloff commented that this deduction is not correct.  I changed the 
logic to check for the "ffp-model=strict" settings, and if those settings are 
in effect from the command line, then I also enable fenv_access.  To repeat 
what I've said elsewhere, this is the documented behavior of the "/fp:strict" 
Microsoft command line option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/AST/const-fpfeatures-diag.c
  clang/test/CXX/expr/expr.const/p2-0x.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/pragma-fenv_access.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  clang/test/Parser/pragma-fenv_access.c
  clang/test/Preprocessor/pragma_unknown.c

Index: clang/test/Preprocessor/pragma_unknown.c
===
--- clang/test/Preprocessor/pragma_unknown.c
+++ clang/test/Preprocessor/pragma_unknown.c
@@ -16,15 +16,6 @@
 // CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT IN_BETWEEN{{$}}
 
-#pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS OFF
-#pragma STDC FENV_ACCESS DEFAULT
-#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS ON{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS OFF{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS DEFAULT{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS IN_BETWEEN{{$}}
-
 #pragma STDC CX_LIMITED_RANGE ON
 #pragma STDC CX_LIMITED_RANGE OFF
 #pragma STDC CX_LIMITED_RANGE DEFAULT 
Index: clang/test/Parser/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/Parser/pragma-fenv_access.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -DSTRICT -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DCPP -DSTRICT -ffp-exception-behavior=strict -fsyntax-only -verify %s
+#ifdef CPP
+#define CONST constexpr
+#else
+#define CONST const
+#endif
+
+#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(int x, float y) {
+  if (x)
+return y + 2;
+  #pragma STDC FENV_ACCESS ON // expected-error{{'#pragma STDC FENV_ACCESS' can only appear at file scope or at the start of a compound statement}}
+  return x + y;
+}
+
+#pragma STDC FENV_ACCESS ON
+int main() {
+  CONST float one = 1.0F ;
+  CONST float three = 3.0F ;
+  CONST float four = 4.0F ;
+  CONST float frac_ok = one/four;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+3 {{constexpr variable 'frac' must be initialized by a constant expression}}
+//expected-note@+2 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+  CONST float frac = one/three; // rounding
+  CONST double d = one;
+  CONST int not_too_big = 255;
+  CONST float fnot_too_big = not_too_big;
+  CONST int too_big = 0x7ff0;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+6 {{constexpr variable 'fbig' must be initialized by a constant expression}}
+//expected-note@+5 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+#if defined(CPP)
+//expected-warning@+2{{implicit conversion}}
+#endif
+  CONST float fbig = too_big; // inexact
+  if (one <= four)  return 0;
+  return -1;
+}
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -26,19 +26,13 @@
 double a = 0.0;
 double b = 1.0;
 
-//FIXME At some point this warning will be removed, until then
-//  document the warning
-#ifdef FAST
-// expected-warning@+1{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS ON
-#else
-#pragma STDC FENV_ACCESS ON // expected-warning{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#endif
 #ifdef STRICT
 #pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when except is enabled}}
 #else
-// Currently FENV_ACCESS cannot be enabled by 

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:410-411
 setRoundingMode(LO.getFPRoundingMode());
+// FENV access is true if rounding math is enabled.
+setAllowFEnvAccess((getRoundingMode() == llvm::RoundingMode::Dynamic));
 setFPExceptionMode(LO.getFPExceptionMode());

sepavloff wrote:
> 
> I don't think this deduction is correct.
> 
> `AllowFEnvAccess` implies that user can access FP environment in a way 
> unknown to the compiler, for example by calling external functions or by 
> using inline assembler. So if `AllowFEnvAccess` is set, the compiler must 
> assume that rounding mode is dynamic, as user can set it to any value unknown 
> to the compiler. Similarly, `FPExceptionMode` must be set to strict as user 
> may read/write exception bits in any way or may associate them with traps. 
> These deductions are valid and compiler must apply them.
> 
> The reverse is not valid. `AllowFEnvAccess` is a very coarse instrument to 
> manipulate FP environment, it prevents from many optimization techniques. In 
> contrast, setting `RoundingMode` to dynamic means only that rounding mode may 
> be different from the default value. If a user changes rounding mode by calls 
> to external functions like `fesetmode` or by using some intrinsic, the 
> compiler still can, for example, reorder FP operations, if `FPExceptionMode` 
> is `ignore`. Impact of performance in this case can be minimized.
> 
> 
Thanks @sepavloff I'll make the change you recommend


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

I neglected to hit the "submit" button last week, these are the inline 
questions I have.

Also in addition to the comments above, check-clang is showing 1 LIT test 
failing AST/const-fpfeatures.cpp
I believe the clang constant folder is returning False for the expression, and 
the constant folding is being done by the LLVM constant folder
llvm-project/clang/test/AST/const-fpfeatures.cpp:21:11: error: CHECK: expected 
string not found in input
// CHECK: @V1 = {{.*}} float 1.00e+00

  ^

:1:1: note: scanning from here
; ModuleID = 
'/export/iusers/mblower/sandbox/llorg/llvm-project/clang/test/AST/const-fpfeatures.cpp'
^
:24:1: note: possible intended match here
@V1 = global float 0.00e+00, align 4

The reason i think clang constant folder/my patch/ isn't to blame for faulty 
logic is because if I change the test to make V1 constexpr, there is an error 
message "not constant". So i'm not sure if the bug is in clang or the test case 
needs to be fixed?




Comment at: clang/lib/AST/ExprConstant.cpp:2734
   if (LHS.isNaN()) {
-Info.CCEDiag(E, diag::note_constexpr_float_arithmetic) << LHS.isNaN();
+Info.FFDiag(E, diag::note_constexpr_float_arithmetic) << LHS.isNaN();
 return Info.noteUndefinedBehavior();

This should be FFDiag?



Comment at: clang/lib/AST/ExprConstant.cpp:12302
+E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {
+  // Note: Compares may raise invalid in some cases involving NaN or sNaN.
+  Info.FFDiag(E, diag::note_constexpr_float_arithmetic_strict);

My scribbled notes say "ISO 10967 LIA-1 
Equality returns invalid if either operand is signaling NaN
Other comparisons < <= >= > return invalid if either operand is NaN".  I'm not 
putting my hands on the exact reference. 



Comment at: clang/test/CodeGen/pragma-fenv_access.c:9
+// CHECK-LABEL: @func_01
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, 
float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+

sepavloff wrote:
> Shall the rounding mode be `dynamic` rather than `tonearest`? If `#pragma 
> STDC FENV_ACCESS ON` is specified, it means FP environment may be changed in 
> arbitrary way and we cannot expect any particular rounding mode. Probably the 
> environment was changed in the caller of `func_01`.
Yes thanks @sepavloff !  i made this change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:410-411
 setRoundingMode(LO.getFPRoundingMode());
+// FENV access is true if rounding math is enabled.
+setAllowFEnvAccess((getRoundingMode() == llvm::RoundingMode::Dynamic));
 setFPExceptionMode(LO.getFPExceptionMode());


I don't think this deduction is correct.

`AllowFEnvAccess` implies that user can access FP environment in a way unknown 
to the compiler, for example by calling external functions or by using inline 
assembler. So if `AllowFEnvAccess` is set, the compiler must assume that 
rounding mode is dynamic, as user can set it to any value unknown to the 
compiler. Similarly, `FPExceptionMode` must be set to strict as user may 
read/write exception bits in any way or may associate them with traps. These 
deductions are valid and compiler must apply them.

The reverse is not valid. `AllowFEnvAccess` is a very coarse instrument to 
manipulate FP environment, it prevents from many optimization techniques. In 
contrast, setting `RoundingMode` to dynamic means only that rounding mode may 
be different from the default value. If a user changes rounding mode by calls 
to external functions like `fesetmode` or by using some intrinsic, the compiler 
still can, for example, reorder FP operations, if `FPExceptionMode` is 
`ignore`. Impact of performance in this case can be minimized.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 295880.
mibintc added a comment.

I patched my workspace with D88498  from Sept 
29. I fixed some nits and also enabled FENV_ACCESS when the command line option 
specifies frounding-math. This corresponds to Microsoft's documentation of 
their option /fp:strict 
https://docs.microsoft.com/en-us/cpp/preprocessor/fenv-access?view=vs-2019 "The 
[/fp:strict] command-line option automatically enables fenv_access."  Also if 
#pragma to enable fenv_access is seen, RoundingMode is set to Dynamic
I have some questions I'll add them as inline comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/expr/expr.const/p2-0x.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/pragma-fenv_access.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  clang/test/Parser/pragma-fenv_access.c
  clang/test/Preprocessor/pragma_unknown.c

Index: clang/test/Preprocessor/pragma_unknown.c
===
--- clang/test/Preprocessor/pragma_unknown.c
+++ clang/test/Preprocessor/pragma_unknown.c
@@ -16,15 +16,6 @@
 // CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT IN_BETWEEN{{$}}
 
-#pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS OFF
-#pragma STDC FENV_ACCESS DEFAULT
-#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS ON{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS OFF{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS DEFAULT{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS IN_BETWEEN{{$}}
-
 #pragma STDC CX_LIMITED_RANGE ON
 #pragma STDC CX_LIMITED_RANGE OFF
 #pragma STDC CX_LIMITED_RANGE DEFAULT 
Index: clang/test/Parser/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/Parser/pragma-fenv_access.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -DSTRICT -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DCPP -DSTRICT -ffp-exception-behavior=strict -fsyntax-only -verify %s
+#ifdef CPP
+#define CONST constexpr
+#else
+#define CONST const
+#endif
+
+#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(int x, float y) {
+  if (x)
+return y + 2;
+  #pragma STDC FENV_ACCESS ON // expected-error{{'#pragma STDC FENV_ACCESS' can only appear at file scope or at the start of a compound statement}}
+  return x + y;
+}
+
+int main() {
+  CONST float one = 1.0F ;
+  CONST float three = 3.0F ;
+  CONST float four = 4.0F ;
+  CONST float frac_ok = one/four;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+3 {{constexpr variable 'frac' must be initialized by a constant expression}}
+//expected-note@+2 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+  CONST float frac = one/three; // rounding
+  CONST double d = one;
+  CONST int not_too_big = 255;
+  CONST float fnot_too_big = not_too_big;
+  CONST int too_big = 0x7ff0;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+6 {{constexpr variable 'fbig' must be initialized by a constant expression}}
+//expected-note@+5 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+#if defined(CPP)
+//expected-warning@+2{{implicit conversion}}
+#endif
+  CONST float fbig = too_big; // inexact
+  if (one <= four)  return 0;
+  return -1;
+}
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -26,19 +26,14 @@
 double a = 0.0;
 double b = 1.0;
 
-//FIXME At some point this warning will be removed, until then
-//  document the warning
-#ifdef FAST
-// expected-warning@+1{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS ON
-#else
-#pragma STDC FENV_ACCESS ON // expected-warning{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#endif
 #ifdef STRICT
 #pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, 

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Richard and I had a long conversation about this further up-thread, if you 
missed it.


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

https://reviews.llvm.org/D87528

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

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

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D87528#2299497 , @mibintc wrote:

> In D87528#2297647 , @sepavloff wrote:
>
>> In D87528#2295015 , @mibintc wrote:
>>
>>> I tried using the 0924 version of the patch on an internal workload SPEC 
>>> "cpu2017" and found that a few files failed to compile because of an error 
>>> message on static initializer, like this: struct s { float f; }; static 
>>> struct s x = {0.63};   Compiled with ffp-model=strict "initializer..is not 
>>> a compile-time constant"
>>
>> Thank you for trying this.
>>
>> The error happens because static variable initializer gets rounding mode 
>> calculated from command-line options (`dynamic`), but this is wrong because 
>> such initializers must be calculated using constant rounding mode 
>> (`tonearest` in this case). Either we must force default FP mode in such 
>> initializers, or we are wrong when using 
>> `FPOptions::defaultWithoutTrailingStorage`. Need to analyze this problem 
>> more.
>
> @sepavloff Please provide precise reference to support the claim "but this is 
> wrong because such initializers must be calculated using constant rounding 
> mode..."  many thanks!

From http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2478.pdf:

**6.7.9 Initialization**
…
4 All the expressions in an initializer for an object that has static or thread 
storage duration shall be
constant expressions or string literals.

**6.6 Constant expressions**
…
2 A constant expression can be evaluated during translation rather than 
runtime, and accordingly
may be used in any place that a constant may be.

**F.8.2 Translation**
1 During translation, constant rounding direction modes (7.6.2) are in effect 
where specified. Elsewhere,
during translation the IEC 60559 default modes are in effect:

- The rounding direction mode is rounding to nearest.

…

**7.6.2 The FENV_ROUND pragma**
…
2 The FENV_ROUND pragma provides a means to specify a constant rounding 
direction for floating point
operations for standard floating types within a translation unit or compound 
statement. …


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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D87528#2297647 , @sepavloff wrote:

> In D87528#2295015 , @mibintc wrote:
>
>> I tried using the 0924 version of the patch on an internal workload SPEC 
>> "cpu2017" and found that a few files failed to compile because of an error 
>> message on static initializer, like this: struct s { float f; }; static 
>> struct s x = {0.63};   Compiled with ffp-model=strict "initializer..is not a 
>> compile-time constant"
>
> Thank you for trying this.
>
> The error happens because static variable initializer gets rounding mode 
> calculated from command-line options (`dynamic`), but this is wrong because 
> such initializers must be calculated using constant rounding mode 
> (`tonearest` in this case). Either we must force default FP mode in such 
> initializers, or we are wrong when using 
> `FPOptions::defaultWithoutTrailingStorage`. Need to analyze this problem more.

@sepavloff Please provide precise reference to support the claim "but this is 
wrong because such initializers must be calculated using constant rounding 
mode..."  many thanks!


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

https://reviews.llvm.org/D87528

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

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

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

rsmith added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:2446
+  fpOptions.isFPConstrained()) {
+Info.CCEDiag(E, diag::note_constexpr_float_arithmetic_strict);
+return false;

FFDiag



Comment at: clang/lib/AST/ExprConstant.cpp:2677
+  if (E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained() &&
+  llvm::APFloatBase::opOK != status) {
+Info.FFDiag(E, diag::note_constexpr_float_arithmetic_strict);

Nit: do these checks in the opposite order; we don't need to compute 
`FPOptions` if the calculation was exact. (I assume we don't get an OK status 
if the result was rounded?)



Comment at: clang/lib/AST/ExprConstant.cpp:2827
 
-  if (!handleFloatFloatBinOp(Info, E, LHSFloat, Opcode,
- RHSElt.getFloat())) {
+  if (!handleFloatFloatBinOp(Info, dyn_cast(E), LHSFloat,
+ Opcode, RHSElt.getFloat())) {

Change this function to take a `BinaryOperator` too rather than using a cast 
here.



Comment at: clang/lib/AST/ExprConstant.cpp:4158
 } else if (RHS.isFloat()) {
+  const BinaryOperator *BE = dyn_cast(E);
+  const FPOptions fpOptions = BE->getFPFeaturesInEffect(

Change the `E` member to be a `BinaryOperator*` rather than using a cast here.



Comment at: clang/lib/AST/ExprConstant.cpp:12242
+// but this doesn't affect constant folding since NaN are
+// not constant expressions.
+

We can form NaNs in constant evaluation using `__builtin_nan()`, and that's 
exposed to the user via `std::numeric_limits`, so I think we do need to check 
for this.



Comment at: clang/lib/AST/ExprConstant.cpp:13290
   case Builtin::BI__builtin_fabsf128:
+// The c lang ref says that "fabs raises no floating-point exceptions,
+// even if x is a signaling NaN. The returned value is independent of





Comment at: clang/lib/AST/ExprConstant.cpp:13347
 bool FloatExprEvaluator::VisitUnaryOperator(const UnaryOperator *E) {
+  // In C lang ref, the unary - raises no floating point exceptions,
+  // even if the operand is signalling.





Comment at: clang/lib/AST/ExprConstant.cpp:13383
+  return ST && DT && DT->isRealFloatingType() && ST->isRealFloatingType() &&
+ ST->getKind() <= DT->getKind();
+}

I don't think this works in general. There is no "wideness" order between PPC 
double-double and IEEE float128.



Comment at: clang/lib/AST/ExprConstant.cpp:13406
+if (E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained() &&
+!isWideningFPConversion(E->getType(), SubExpr->getType())) {
+  // In C lang ref, footnote, cast may raise inexact exception.

Can we check whether the value is representable, not only the type? Eg, it 
would seem preferable to accept `float x = 1.0;`



Comment at: clang/lib/AST/ExprConstant.cpp:13407
+!isWideningFPConversion(E->getType(), SubExpr->getType())) {
+  // In C lang ref, footnote, cast may raise inexact exception.
+  // Cast may also raise invalid.

It would be useful to say which C standard and which footnote, eg "C11 footnote 
123"



Comment at: clang/lib/AST/ExprConstant.cpp:13197-13198
   assert(E->isRValue() && E->getType()->isRealFloatingType());
+  if (Info.isStrictFP)
+return false;
   return FloatExprEvaluator(Info, Result).Visit(E);

mibintc wrote:
> rsmith wrote:
> > I think we should be able to evaluate (for example) `constexpr float f = 
> > 1.0f;` even in a strict FP context. I think only floating point operations 
> > that depend on the rounding mode should be disabled, not all floating-point 
> > evaluation. Perhaps we should propagate the `FPOptions` into 
> > `handleFloatFloatBinOp` and perform the check there, along with any other 
> > places that care (I think we probably have some builtins that we can 
> > constant-evaluate that care about rounding modes.)
> > 
> > You also need to produce a diagnostic when you treat an expression as 
> > non-constant -- please read the comment at the top of the file for details.
> > I think we should be able to evaluate (for example) `constexpr float f = 
> > 1.0f;` even in a strict FP context. I think only floating point operations 
> > that depend on the rounding mode should be disabled, not all floating-point 
> > evaluation. Perhaps we should propagate the `FPOptions` into 
> > `handleFloatFloatBinOp` and perform the check there, along with any other 
> > places that care (I think we probably have some builtins that we can 
> > constant-evaluate that care about rounding modes.)
> > 
> > You also need to produce a diagnostic when you treat an expression as 
> > non-constant -- please read the comment 

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D87528#2297647 , @sepavloff wrote:

> In D87528#2295015 , @mibintc wrote:
>
>> I tried using the 0924 version of the patch on an internal workload SPEC 
>> "cpu2017" and found that a few files failed to compile because of an error 
>> message on static initializer, like this: struct s { float f; }; static 
>> struct s x = {0.63};   Compiled with ffp-model=strict "initializer..is not a 
>> compile-time constant"
>
> Thank you for trying this.
>
> The error happens because static variable initializer gets rounding mode 
> calculated from command-line options (`dynamic`), but this is wrong because 
> such initializers must be calculated using constant rounding mode 
> (`tonearest` in this case). Either we must force default FP mode in such 
> initializers, or we are wrong when using 
> `FPOptions::defaultWithoutTrailingStorage`. Need to analyze this problem more.

If the literal was typed, there was no diagnostic, "x = { 0.63F }".  Thanks for 
the insight re: rounding mode.


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

https://reviews.llvm.org/D87528

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

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

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D87528#2295015 , @mibintc wrote:

> I tried using the 0924 version of the patch on an internal workload SPEC 
> "cpu2017" and found that a few files failed to compile because of an error 
> message on static initializer, like this: struct s { float f; }; static 
> struct s x = {0.63};   Compiled with ffp-model=strict "initializer..is not a 
> compile-time constant"

Thank you for trying this.

The error happens because static variable initializer gets rounding mode 
calculated from command-line options (`dynamic`), but this is wrong because 
such initializers must be calculated using constant rounding mode (`tonearest` 
in this case). Either we must force default FP mode in such initializers, or we 
are wrong when using `FPOptions::defaultWithoutTrailingStorage`. Need to 
analyze this problem more.


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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added inline comments.



Comment at: clang/test/CodeGen/fp-floatcontrol-pragma.cpp:154
+  if (i<0)
+  return 1.0 + 2.0;
+  // Check that floating point constant folding doesn't occur if

sepavloff wrote:
> In this particular case we know for sure that the result does not depend on 
> rounding mode and no FP exceptions occurs, so it is safe to constfold the 
> expression.
> 
> Expressions like `1.0 / 0.0` or `1.0F + 0x0.01p0F` indeed may require 
> execution in runtime, depending on the required exception handling. I would 
> propose to connect their const-evaluability with `FPExceptionMode` and set 
> the latter to `strict` whenever `AllowFEnvAccess` is set to `true`. 
Thank you, I will study these remarks.  BTW I had inserted checks to verify 
semantic rules about the use of pragma float_control and fenv_access, to follow 
what Microsoft describes on this page, 
https://docs.microsoft.com/en-us/cpp/preprocessor/fenv-access?view=vs-2019 
Quoting:
There are restrictions on the ways you can use the fenv_access pragma in 
combination with other floating-point settings:

You can't enable fenv_access unless precise semantics are enabled. Precise 
semantics can be enabled either by the float_control pragma, or by using the 
/fp:precise or /fp:strict compiler options. The compiler defaults to 
/fp:precise if no other floating-point command-line option is specified.

You can't use float_control to disable precise semantics when fenv_access(on) 
is set.


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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D87528#2270502 , @sepavloff wrote:

>> @sepavloff Is it OK if I continue work on this item? Not sure about the 
>> protocol when continuing someone else's patch.
>
> It is OK for me. There is also an action in Phabricator "Commandeer Revision" 
> to transfer ownership on a revision item.
>
> I don't think however that the implementation in frontend is the main 
> obstacle for enabling the pragma. It is the part of the standard and is user 
> visible, so clang must provide satisfactory support so that users could try 
> this feature in real applications. This support mainly depends on the support 
> of constrained intrinsics in IR and codegen.
>
> One of the probable ways to confirm the support is to build some pretty large 
> project that uses floating point operations extensively, build it with option 
> `-fp-model=strict` and check if it works. A good choice could be SPEC 
> benchmarks. It would provide us with not only evidence of support but also 
> with number how strict operations slow down execution. Maybe other projects 
> may be used for this purpose, but I don't know such.

I tried using the 0924 version of the patch on an internal workload SPEC 
"cpu2017" and found that a few files failed to compile because of an error 
message on static initializer, like this: struct s { float f; }; static struct 
s x = {0.63};   Compiled with ffp-model=strict "initializer..is not a 
compile-time constant"


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

https://reviews.llvm.org/D87528

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

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

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/test/CodeGen/fp-floatcontrol-pragma.cpp:154
+  if (i<0)
+  return 1.0 + 2.0;
+  // Check that floating point constant folding doesn't occur if

In this particular case we know for sure that the result does not depend on 
rounding mode and no FP exceptions occurs, so it is safe to constfold the 
expression.

Expressions like `1.0 / 0.0` or `1.0F + 0x0.01p0F` indeed may require 
execution in runtime, depending on the required exception handling. I would 
propose to connect their const-evaluability with `FPExceptionMode` and set the 
latter to `strict` whenever `AllowFEnvAccess` is set to `true`. 



Comment at: clang/test/CodeGen/pragma-fenv_access.c:9
+// CHECK-LABEL: @func_01
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, 
float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+

Shall the rounding mode be `dynamic` rather than `tonearest`? If `#pragma STDC 
FENV_ACCESS ON` is specified, it means FP environment may be changed in 
arbitrary way and we cannot expect any particular rounding mode. Probably the 
environment was changed in the caller of `func_01`.


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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked 2 inline comments as done.
mibintc added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:2683
   }
+  if (const BinaryOperator *BE = dyn_cast(E)) {
+if (BE->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {

rsmith wrote:
> `E` here is only intended for use in determining diagnostic locations, and 
> isn't intended to necessarily be the expression being evaluated. Instead of 
> assuming that this is the right expression to inspect, please either query 
> the FP features in the caller and pass them into here or change this function 
> to take a `const BinaryOperator*`.
OK I changed it to use BinaryOperator



Comment at: clang/lib/AST/ExprConstant.cpp:2685
+if (BE->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {
+  Info.CCEDiag(E, diag::note_constexpr_float_arithmetic_strict);
+  return false;

rsmith wrote:
> This should be an `FFDiag` not a `CCEDiag` because we want to suppress all 
> constant folding, not only treating the value as a core constant expression. 
> Also we should check this before checking for a NaN result, because if both 
> happen at once, this is the diagnostic we want to produce.
OK I made this change



Comment at: clang/lib/AST/ExprConstant.cpp:13283-13286
+if (E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {
+  Info.CCEDiag(E, diag::note_constexpr_float_arithmetic_strict);
+  return false;
+}

rsmith wrote:
> Hm, does `fabs` depend on the floating-point environment? Is the concern the 
> interaction with signaling NaNs?
You're right, "fabs(x) raises no floating-point exceptions, even if x is a 
signaling NaN. The returned value is independent of the current rounding 
direction mode.".  Thanks a lot for the careful reading, I really appreciate it



Comment at: clang/lib/AST/ExprConstant.cpp:13372-13376
+  if (E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {
+// In C lang ref, footnote, cast may raise inexact exception.
+Info.CCEDiag(E, diag::note_constexpr_float_arithmetic_strict);
+return false;
+  }

rsmith wrote:
> Is it feasible to only reject cases that would actually be inexact, rather 
> than disallowing all conversions to floating-point types? It would seem 
> preferable to allow at least widening FP conversions and complex -> real, 
> since they never depend on the FP environment (right?).
I changed it like you suggested, do the binary APFloat calculation and check 
the operation status to see if the result is inexact. I also added logic to 
check "is widening".  Is that OK (builtin kind <) or maybe I should rather 
check the precision bitwidth? 



Comment at: clang/test/CodeGen/pragma-fenv_access.c:1
+// xxx: %clang_cc1 -ffp-exception-behavior=strict -frounding-math -triple 
%itanium_abi_triple -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -triple %itanium_abi_triple 
-emit-llvm %s -o - | FileCheck %s

rsmith wrote:
> Is this RUN line intentionally disabled?
Oops thank you. I was working with an old revision of the patch and the test 
cause no longer worked because rounding setting was different. i just got rid 
of the run line that includes frounding-math


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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

Note, @sepavloff is working on overlapping concerns here, D87822 
: [FPEnv] Evaluate constant expressions under 
non-default rounding modes


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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 294074.
mibintc added a comment.

Responded to @rsmith's review


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

https://reviews.llvm.org/D87528

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/expr/expr.const/p2-0x.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/pragma-fenv_access.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  clang/test/Parser/pragma-fenv_access.c
  clang/test/Preprocessor/pragma_unknown.c

Index: clang/test/Preprocessor/pragma_unknown.c
===
--- clang/test/Preprocessor/pragma_unknown.c
+++ clang/test/Preprocessor/pragma_unknown.c
@@ -16,15 +16,6 @@
 // CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT IN_BETWEEN{{$}}
 
-#pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS OFF
-#pragma STDC FENV_ACCESS DEFAULT
-#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS ON{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS OFF{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS DEFAULT{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS IN_BETWEEN{{$}}
-
 #pragma STDC CX_LIMITED_RANGE ON
 #pragma STDC CX_LIMITED_RANGE OFF
 #pragma STDC CX_LIMITED_RANGE DEFAULT 
Index: clang/test/Parser/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/Parser/pragma-fenv_access.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -DSTRICT -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DCPP -DSTRICT -ffp-exception-behavior=strict -fsyntax-only -verify %s
+#ifdef CPP
+#define CONST constexpr
+#else
+#define CONST const
+#endif
+
+#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(int x, float y) {
+  if (x)
+return y + 2;
+  #pragma STDC FENV_ACCESS ON // expected-error{{'#pragma STDC FENV_ACCESS' can only appear at file scope or at the start of a compound statement}}
+  return x + y;
+}
+
+int main() {
+  CONST float one = 1.0F ;
+  CONST float three = 3.0F ;
+  CONST float four = 4.0F ;
+  CONST float frac_ok = one/four;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+3 {{constexpr variable 'frac' must be initialized by a constant expression}}
+//expected-note@+2 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+  CONST float frac = one/three; // rounding
+  CONST double d = one;
+  CONST int not_too_big = 255;
+  CONST float fnot_too_big = not_too_big;
+  CONST int too_big = 0x7ff0;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+6 {{constexpr variable 'fbig' must be initialized by a constant expression}}
+//expected-note@+5 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+#if defined(CPP)
+//expected-warning@+2{{implicit conversion}}
+#endif
+  CONST float fbig = too_big; // inexact
+  if (one <= four)  return 0;
+  return -1;
+}
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -26,19 +26,14 @@
 double a = 0.0;
 double b = 1.0;
 
-//FIXME At some point this warning will be removed, until then
-//  document the warning
-#ifdef FAST
-// expected-warning@+1{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS ON
-#else
-#pragma STDC FENV_ACCESS ON // expected-warning{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#endif
 #ifdef STRICT
 #pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when except is enabled}}
 #else
+#ifndef FAST
 // Currently FENV_ACCESS cannot be enabled by pragma, skip error check
-#pragma float_control(precise, off) // not-expected-error {{'#pragma float_control(precise, off)' is illegal when fenv_access is enabled}}
+#pragma STDC FENV_ACCESS ON
+#pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when fenv_access is enabled}}
+#endif
 #endif
 
 #pragma float_control(precise, on)
Index: clang/test/CodeGen/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/CodeGen/pragma-fenv_access.c
@@ 

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

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

rsmith added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:2683
   }
+  if (const BinaryOperator *BE = dyn_cast(E)) {
+if (BE->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {

`E` here is only intended for use in determining diagnostic locations, and 
isn't intended to necessarily be the expression being evaluated. Instead of 
assuming that this is the right expression to inspect, please either query the 
FP features in the caller and pass them into here or change this function to 
take a `const BinaryOperator*`.



Comment at: clang/lib/AST/ExprConstant.cpp:2685
+if (BE->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {
+  Info.CCEDiag(E, diag::note_constexpr_float_arithmetic_strict);
+  return false;

This should be an `FFDiag` not a `CCEDiag` because we want to suppress all 
constant folding, not only treating the value as a core constant expression. 
Also we should check this before checking for a NaN result, because if both 
happen at once, this is the diagnostic we want to produce.



Comment at: clang/lib/AST/ExprConstant.cpp:13283-13286
+if (E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {
+  Info.CCEDiag(E, diag::note_constexpr_float_arithmetic_strict);
+  return false;
+}

Hm, does `fabs` depend on the floating-point environment? Is the concern the 
interaction with signaling NaNs?



Comment at: clang/lib/AST/ExprConstant.cpp:13372-13376
+  if (E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {
+// In C lang ref, footnote, cast may raise inexact exception.
+Info.CCEDiag(E, diag::note_constexpr_float_arithmetic_strict);
+return false;
+  }

Is it feasible to only reject cases that would actually be inexact, rather than 
disallowing all conversions to floating-point types? It would seem preferable 
to allow at least widening FP conversions and complex -> real, since they never 
depend on the FP environment (right?).



Comment at: clang/test/CodeGen/pragma-fenv_access.c:1
+// xxx: %clang_cc1 -ffp-exception-behavior=strict -frounding-math -triple 
%itanium_abi_triple -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -triple %itanium_abi_triple 
-emit-llvm %s -o - | FileCheck %s

Is this RUN line intentionally disabled?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:13197-13198
   assert(E->isRValue() && E->getType()->isRealFloatingType());
+  if (Info.isStrictFP)
+return false;
   return FloatExprEvaluator(Info, Result).Visit(E);

rsmith wrote:
> I think we should be able to evaluate (for example) `constexpr float f = 
> 1.0f;` even in a strict FP context. I think only floating point operations 
> that depend on the rounding mode should be disabled, not all floating-point 
> evaluation. Perhaps we should propagate the `FPOptions` into 
> `handleFloatFloatBinOp` and perform the check there, along with any other 
> places that care (I think we probably have some builtins that we can 
> constant-evaluate that care about rounding modes.)
> 
> You also need to produce a diagnostic when you treat an expression as 
> non-constant -- please read the comment at the top of the file for details.
> I think we should be able to evaluate (for example) `constexpr float f = 
> 1.0f;` even in a strict FP context. I think only floating point operations 
> that depend on the rounding mode should be disabled, not all floating-point 
> evaluation. Perhaps we should propagate the `FPOptions` into 
> `handleFloatFloatBinOp` and perform the check there, along with any other 
> places that care (I think we probably have some builtins that we can 
> constant-evaluate that care about rounding modes.)
> 
> You also need to produce a diagnostic when you treat an expression as 
> non-constant -- please read the comment at the top of the file for details.

It's not just rounding mode, right? If fp exceptions are unmasked then we 
should inhibit the folding in particular cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 292343.
mibintc added a comment.

I pulled out the isStrictFP boolean from EvalInfo and used the FPOptions when 
visiting floating point BinaryOperator, CastExpr and builtin CallExpr. I 
created the diagnostic note explaining why constant evaluation failed.  Not 
certain about the language rules C vs C++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/expr/expr.const/p2-0x.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/pragma-fenv_access.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  clang/test/Parser/pragma-fenv_access.c
  clang/test/Preprocessor/pragma_unknown.c

Index: clang/test/Preprocessor/pragma_unknown.c
===
--- clang/test/Preprocessor/pragma_unknown.c
+++ clang/test/Preprocessor/pragma_unknown.c
@@ -16,15 +16,6 @@
 // CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT IN_BETWEEN{{$}}
 
-#pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS OFF
-#pragma STDC FENV_ACCESS DEFAULT
-#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS ON{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS OFF{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS DEFAULT{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS IN_BETWEEN{{$}}
-
 #pragma STDC CX_LIMITED_RANGE ON
 #pragma STDC CX_LIMITED_RANGE OFF
 #pragma STDC CX_LIMITED_RANGE DEFAULT 
Index: clang/test/Parser/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/Parser/pragma-fenv_access.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(int x, float y) {
+  if (x)
+return y + 2;
+  #pragma STDC FENV_ACCESS ON // expected-error{{'#pragma STDC FENV_ACCESS' can only appear at file scope or at the start of a compound statement}}
+  return x + y;
+}
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -26,19 +26,14 @@
 double a = 0.0;
 double b = 1.0;
 
-//FIXME At some point this warning will be removed, until then
-//  document the warning
-#ifdef FAST
-// expected-warning@+1{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS ON
-#else
-#pragma STDC FENV_ACCESS ON // expected-warning{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#endif
 #ifdef STRICT
 #pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when except is enabled}}
 #else
+#ifndef FAST
 // Currently FENV_ACCESS cannot be enabled by pragma, skip error check
-#pragma float_control(precise, off) // not-expected-error {{'#pragma float_control(precise, off)' is illegal when fenv_access is enabled}}
+#pragma STDC FENV_ACCESS ON
+#pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when fenv_access is enabled}}
+#endif
 #endif
 
 #pragma float_control(precise, on)
Index: clang/test/CodeGen/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/CodeGen/pragma-fenv_access.c
@@ -0,0 +1,66 @@
+// xxx: %clang_cc1 -ffp-exception-behavior=strict -frounding-math -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s
+
+#pragma STDC FENV_ACCESS ON
+
+float func_01(float x, float y) {
+  return x + y;
+}
+// CHECK-LABEL: @func_01
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+
+
+float func_02(float x, float y) {
+  #pragma float_control(except, off)
+  #pragma STDC FENV_ACCESS OFF
+  return x + y;
+}
+// CHECK-LABEL: @func_02
+// CHECK: fadd float {{.*}}
+
+
+float func_03(float x, float y) {
+  return x + y;
+}
+// CHECK-LABEL: @func_03
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata 

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

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

rsmith requested changes to this revision.
rsmith added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/AST/ExprConstant.cpp:775-778
+/// Strict floating point is enabled, this inhibits
+/// floating ponit constant folding.
+bool isStrictFP;
+

This is a property of the operation being constant-evaluated, not of the 
`EvalInfo`. The way you compute it below will only pick up the right value if 
the top-level expression in the evaluation happens to be a constrained 
floating-point evaluation.



Comment at: clang/lib/AST/ExprConstant.cpp:13197-13198
   assert(E->isRValue() && E->getType()->isRealFloatingType());
+  if (Info.isStrictFP)
+return false;
   return FloatExprEvaluator(Info, Result).Visit(E);

I think we should be able to evaluate (for example) `constexpr float f = 1.0f;` 
even in a strict FP context. I think only floating point operations that depend 
on the rounding mode should be disabled, not all floating-point evaluation. 
Perhaps we should propagate the `FPOptions` into `handleFloatFloatBinOp` and 
perform the check there, along with any other places that care (I think we 
probably have some builtins that we can constant-evaluate that care about 
rounding modes.)

You also need to produce a diagnostic when you treat an expression as 
non-constant -- please read the comment at the top of the file for details.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 291736.
mibintc added a comment.

This update uses context information from Expr->getFPFeaturesInEffect() to 
disable fp constant folding in ExprConstant.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/pragma-fenv_access.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  clang/test/Parser/pragma-fenv_access.c
  clang/test/Preprocessor/pragma_unknown.c

Index: clang/test/Preprocessor/pragma_unknown.c
===
--- clang/test/Preprocessor/pragma_unknown.c
+++ clang/test/Preprocessor/pragma_unknown.c
@@ -16,15 +16,6 @@
 // CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT IN_BETWEEN{{$}}
 
-#pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS OFF
-#pragma STDC FENV_ACCESS DEFAULT
-#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS ON{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS OFF{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS DEFAULT{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS IN_BETWEEN{{$}}
-
 #pragma STDC CX_LIMITED_RANGE ON
 #pragma STDC CX_LIMITED_RANGE OFF
 #pragma STDC CX_LIMITED_RANGE DEFAULT 
Index: clang/test/Parser/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/Parser/pragma-fenv_access.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(int x, float y) {
+  if (x)
+return y + 2;
+  #pragma STDC FENV_ACCESS ON // expected-error{{'#pragma STDC FENV_ACCESS' can only appear at file scope or at the start of a compound statement}}
+  return x + y;
+}
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -26,19 +26,14 @@
 double a = 0.0;
 double b = 1.0;
 
-//FIXME At some point this warning will be removed, until then
-//  document the warning
-#ifdef FAST
-// expected-warning@+1{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS ON
-#else
-#pragma STDC FENV_ACCESS ON // expected-warning{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#endif
 #ifdef STRICT
 #pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when except is enabled}}
 #else
+#ifndef FAST
 // Currently FENV_ACCESS cannot be enabled by pragma, skip error check
-#pragma float_control(precise, off) // not-expected-error {{'#pragma float_control(precise, off)' is illegal when fenv_access is enabled}}
+#pragma STDC FENV_ACCESS ON
+#pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when fenv_access is enabled}}
+#endif
 #endif
 
 #pragma float_control(precise, on)
Index: clang/test/CodeGen/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/CodeGen/pragma-fenv_access.c
@@ -0,0 +1,66 @@
+// xxx: %clang_cc1 -ffp-exception-behavior=strict -frounding-math -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -triple %itanium_abi_triple -emit-llvm %s -o - | FileCheck %s
+
+#pragma STDC FENV_ACCESS ON
+
+float func_01(float x, float y) {
+  return x + y;
+}
+// CHECK-LABEL: @func_01
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+
+
+float func_02(float x, float y) {
+  #pragma float_control(except, off)
+  #pragma STDC FENV_ACCESS OFF
+  return x + y;
+}
+// CHECK-LABEL: @func_02
+// CHECK: fadd float {{.*}}
+
+
+float func_03(float x, float y) {
+  return x + y;
+}
+// CHECK-LABEL: @func_03
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(float x, float y) {
+  #pragma float_control(except, off)
+  return x + y;
+}
+// CHECK-LABEL: @func_04
+// CHECK: fadd float {{.*}}
+
+
+float 

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a subscriber: scanon.
rjmccall added a comment.

That's a really useful concept, and given that it exists, I agree that we 
shouldn't invent something else.  The fact that it covers local variables with 
constant initializers that happen to be `const` seems really unfortunate, 
though.  @scanon, any opinions here about how important it is to give 
consistent semantics to a floating-point expression in a static local 
initializer in C and C++ under `#pragma STDC FENV_ACCESS`?

Note that we'll need to implement the C semantics in any case, which I'm pretty 
sure means we need to track whether an expression is in a constant initializer 
or not — we need to constant-evaluate `1.0 / 3.0` as the initializer of a 
static local but refuse to do so as the initializer of an auto local — so I'm 
not sure the implementation gets all that much simpler based on our decision 
for C++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

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

rsmith added a comment.

In D87528#2270561 , @rjmccall wrote:

> Richard, what do you think the right rule is for when to use the special 
> constant-evaluation rules for this feature in C++?  The C standard says that 
> constant expressions should use the default rules rather than considering the 
> dynamic environment, but C can get away with that because constant 
> expressions are restricted to a narrow set of syntactic situations.

Right, C has a lot of leeway here because floating-point operations aren't 
allowed in integer constant expressions in C; the only place FP operations are 
guaranteed to be evaluated during translation is in the initializers of 
variables of static storage duration, which are all notionally initialized 
before the program begins executing (and thus before a non-default FP 
environment can be entered) anyway. Needing to deal with (for example) 
floating-point operations in array bounds is a novel C++ problem.

I think the broadest set of places we could consider assuming the default FP 
environment is manifestly constant evaluated 
 contexts; that's probably the most 
natural category in C++ for this kind of thing. That is the same condition 
under which `std::is_constant_evaluated()` returns `true`. (The "manifestly 
constant evaluated" property is already tracked within the constant evaluator 
by the `InConstantContext` flag.) And your list:

> - initializers for objects of static storage duration if they would be 
> constant initializers if they weren't covered by the pragma; this should 
> roughly align C and C++ semantics for things like `static const` within a 
> function
> - any `constexpr` / `constinit` context, and it should be ill-formed to use 
> the pragma in such a context; that is, it's ignored from outside and 
> prohibited (directly) inside

... roughly matches "manifestly constant evaluated". (Aside: I don't think we 
need to, or should, prohibit the pragma inside `constexpr` functions. We'll try 
to reject such functions anyway if they can't produce constant expressions, so 
a special case rule seems redundant, and such functions can legitimately 
contain code only intended for use in non-constant contexts. Disallowing it in 
`consteval` functions might be OK, but we don't disallow calls to 
non-`constexpr` functions from `consteval` functions, for which the same 
concerns would apply.)

"Manifestly constant evaluated" describes the set of expressions that an 
implementation is required to reduce to a constant, and covers two sets of 
cases:

- expressions for which the program is ill-formed if they're not constant: 
template arguments, array bounds, enumerators, case labels, `consteval` 
function calls, `constexpr` and `constinit` variables, concepts, constexpr if
- expressions whose semantics are potentially affected by constantness, and for 
which an implementation is required to guarantee to commit to the constant 
semantics whenever it can: when checking to see if a variable with 
static/thread storage duration has static initialization, or whether an 
automatic variable of reference or const-qualified integral type is usable in 
constant expressions

If we go that way, there'd be at least one class of surprising cases. Here's an 
example of it:

  void f() {
  #pragma STDC FENV_ACCESS ON
fesetround(FE_DOWNWARD);
CONST bool b = (1.0 / 3.0) * 3.0 < 1.0;
if (b) g();
fesetround(FE_TONEAREST);
  }

Under the "manifestly constant evaluated" rule, with `-DCONST=`, this would 
call `g()`, but with `-DCONST=const`, it would *not* call `g()`, because the 
initializer of `b` would be manifestly constant evaluated. That's both 
surprising and different from the behavior in C. (The surprise isn't novel; 
C++'s `std::is_constant_evaluated()` has the same surprising semantics. It's 
not completely unreasonable to claim that C++ says that reference and `const` 
integral variables are implicitly `constexpr` whenever they can be, though 
that's not how the rule is expressed.) Basing this off "manifestly constant 
evaluated" would also be C-incompatible in at least one other way: we'd treat 
FP operations in an array bound as being in the default FP environment in C++ 
if that makes the overall evaluation constant, but in C they always result in a 
VLA.

So I suppose the question is, are we comfortable with all that, or do we want 
to use a different rule?

There's another uninventive rule at the opposite end of the spectrum. Prior 
discussion in the C++ committee, before we had "manifestly constant evaluated" 
and associated machinery, seemed to be converging on this model:

1. constrained FP operations are always treated as non-constant in all 
contexts, and
2. in a C++ translation unit, the initial state for `FENV_ACCESS` is always 
`OFF`.

That approach has the advantage of being both simple and compatible with all 
code we currently support, as well as probably 

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Richard, what do you think the right rule is for when to use the special 
constant-evaluation rules for this feature in C++?  The C standard says that 
constant expressions should use the default rules rather than considering the 
dynamic environment, but C can get away with that because constant expressions 
are restricted to a narrow set of syntactic situations.  In C++, [cfenv.syn]p1 
says:

  If the pragma is used to enable control over the floating-point environment, 
this document does not specify the effect on floating-point evaluation in 
constant expressions.

i.e. the standard punts to the implementation.  So I think we need to actually 
formulate a policy for when something counts as a "constant expression" for the 
purposes of ignoring this pragma.  Allow me to suggest:

- initializers for objects of static storage duration if they would be constant 
initializers if they weren't covered by the pragma; this should roughly align C 
and C++ semantics for things like `static const` within a function
- any `constexpr` / `constinit` context, and it should be ill-formed to use the 
pragma in such a context; that is, it's ignored from outside and prohibited 
(directly) inside


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

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

rsmith added a comment.

> @rsmith asked "I don't see changes to the constant evaluator". The semantic 
> rules for enabling this pragma require that strict or precise semantics be in 
> effect., see SemaAttr.cpp And the floating point constant evaluation doesn't 
> occur when strict or precise

What do you mean "the floating point constant evaluation doesn't occur when 
strict or precise"? I don't see any code to do that in `ExprConstant.cpp`, 
neither in trunk nor in this patch. The constant evaluator certainly is still 
invoked when we're in strict or precise mode. I would expect the evaluator 
would need to check for FP strictness whenever it encounters a floating-point 
operator (eg, here: 
https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/ExprConstant.cpp#L13340),
 and treat such cases as non-constant.

I'd like to see a C++ test for something like this:

  float myAdd() {
  #pragma STDC FENV_ACCESS ON
  static double v = 1.0 / 3.0;
  return v;
  }

If that generates a runtime guarded initialization for `v`, then I'd be a bit 
more convinced we're getting this right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

> @sepavloff Is it OK if I continue work on this item? Not sure about the 
> protocol when continuing someone else's patch.

It is OK for me. There is also an action in Phabricator "Commandeer Revision" 
to transfer ownership on a revision item.

I don't think however that the implementation in frontend is the main obstacle 
for enabling the pragma. It is the part of the standard and is user visible, so 
clang must provide satisfactory support so that users could try this feature in 
real applications. This support mainly depends on the support of constrained 
intrinsics in IR and codegen.

One of the probable ways to confirm the support is to build some pretty large 
project that uses floating point operations extensively, build it with option 
`-fp-model=strict` and check if it works. A good choice could be SPEC 
benchmarks. It would provide us with not only evidence of support but also with 
number how strict operations slow down execution. Maybe other projects may be 
used for this purpose, but I don't know such.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

IRGen generally doesn't query the AST constant evaluator for arbitrary 
expressions; we do it for certain calls and loads because otherwise we might 
emit an illegal ODR-use, but otherwise we just expect LLVM's constant-folding 
to do a decent job.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc created this revision.
mibintc added reviewers: sepavloff, rjmccall.
Herald added a project: clang.
mibintc requested review of this revision.

I rebased https://reviews.llvm.org/D69272 to work on the blocking comment from 
@rsmith 
@sepavloff Is it OK if I continue work on this item?  Not sure about the 
protocol when continuing someone else's patch.

@rsmith asked "I don't see changes to the constant evaluator".  The semantic 
rules for enabling this pragma require that strict or precise semantics be in 
effect., see SemaAttr.cpp  And the floating point constant evaluation doesn't 
occur when strict or precise

One of the test cases in this patch shows 1+2 isn't folded in strict mode.

However, I don't see where is the decision about floating point constant 
folding, can someone tell me where that occurs? . I thought the constant 
folding was in AST/ExprConstant.cpp and indeed constant folding does occur 
there. But sometimes, if the floating point semantics are set to 'strict', even 
tho' folding has occurred successfully in ExprConstant.cpp, when i look at 
emit-llvm, there is arithmetic emitted for the floating point expression;  For 
example if you use the command line option -ffp-exception-behavior=strict and 
you compile this function, it will emit the add instruction; but without the 
option you will see the folded expression 3.0.  Either way (i.e. regardless of 
option) if you put a breakpoint inside ExprConstant.cpp the calculation of the 
floating sum does occur.  The function is float myAdd(void) { return 1.0 + 2.0; 
}


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87528

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/pragma-fenv_access.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  clang/test/Parser/pragma-fenv_access.c
  clang/test/Preprocessor/pragma_unknown.c

Index: clang/test/Preprocessor/pragma_unknown.c
===
--- clang/test/Preprocessor/pragma_unknown.c
+++ clang/test/Preprocessor/pragma_unknown.c
@@ -16,15 +16,6 @@
 // CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT IN_BETWEEN{{$}}
 
-#pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS OFF
-#pragma STDC FENV_ACCESS DEFAULT
-#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS ON{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS OFF{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS DEFAULT{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS IN_BETWEEN{{$}}
-
 #pragma STDC CX_LIMITED_RANGE ON
 #pragma STDC CX_LIMITED_RANGE OFF
 #pragma STDC CX_LIMITED_RANGE DEFAULT 
Index: clang/test/Parser/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/Parser/pragma-fenv_access.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(int x, float y) {
+  if (x)
+return y + 2;
+  #pragma STDC FENV_ACCESS ON // expected-error{{'#pragma STDC FENV_ACCESS' can only appear at file scope or at the start of a compound statement}}
+  return x + y;
+}
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -26,19 +26,14 @@
 double a = 0.0;
 double b = 1.0;
 
-//FIXME At some point this warning will be removed, until then
-//  document the warning
-#ifdef FAST
-// expected-warning@+1{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS ON
-#else
-#pragma STDC FENV_ACCESS ON // expected-warning{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#endif
 #ifdef STRICT
 #pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when except is enabled}}
 #else
+#ifndef FAST
 // Currently FENV_ACCESS cannot be enabled by pragma, skip error check
-#pragma float_control(precise, off) // not-expected-error {{'#pragma float_control(precise, off)' is illegal when fenv_access is enabled}}
+#pragma STDC FENV_ACCESS ON
+#pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when fenv_access is enabled}}
+#endif
 #endif
 
 #pragma float_control(precise, on)
Index: