Enable '#pragma STDC FENV_ACCESS' in frontend

[mibintc]

Summary

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; }

[rjmccall]

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.

[sepavloff]

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

[rsmith]

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

[rjmccall]

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

[rsmith]

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 compatible with all directions the C++ committee might reasonably go in, and it gives the same result as C for all cases it accepts. It's also the most predictable option: all floating-point operations within FENV_ACCESS ON regions always support non-default FP environments. It would be much less friendly towards people trying to mix constant expressions and non-default floating-point environments, though; how important is that?

Providing a middle ground between these two options would likely have somewhat higher implementation complexity, but I think we could do that too if there's some middle-ground we're happier with (eg, manifestly constant evaluated minus initializers of automatic storage duration variables). My inclination would be to go for the more-restrictive rule, at least initially -- treat all constrained FP operations as non-constant in all contexts in C++ -- and consider switching to a different rule if we get user feedback that the restrictions are too unpleasant.

[rjmccall]

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

[mibintc]

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

[rsmith]

clang/lib/AST/ExprConstant.cpp

775–778 ↗

(On Diff #291736)

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.

13197–13198 ↗

(On Diff #291736)

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.

[mibintc]

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

[mibintc]

clang/lib/AST/ExprConstant.cpp

13197–13198 ↗

(On Diff #291736)

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?

[rsmith]

clang/lib/AST/ExprConstant.cpp

2683 ↗

(On Diff #292343)

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

2685 ↗

(On Diff #292343)

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.

13283–13286 ↗

(On Diff #292343)

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

13372–13376 ↗

(On Diff #292343)

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?).

clang/test/CodeGen/pragma-fenv_access.c

2

Is this RUN line intentionally disabled?

[mibintc]

Responded to @rsmith's review

[mibintc]

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

[mibintc]

clang/lib/AST/ExprConstant.cpp

2683 ↗

(On Diff #292343)

OK I changed it to use BinaryOperator

2685 ↗

(On Diff #292343)

OK I made this change

13283–13286 ↗

(On Diff #292343)

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

13372–13376 ↗

(On Diff #292343)

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?

clang/test/CodeGen/pragma-fenv_access.c

2

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

[sepavloff]

clang/test/CodeGen/fp-floatcontrol-pragma.cpp

154

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.000001p0F 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.

clang/test/CodeGen/pragma-fenv_access.c

10

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.

[mibintc]

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"

[mibintc]

clang/test/CodeGen/fp-floatcontrol-pragma.cpp

154

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.

[sepavloff]

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.

[mibintc]

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.

[rsmith]

clang/lib/AST/ExprConstant.cpp

2446 ↗

(On Diff #294074)

FFDiag

2677 ↗

(On Diff #294074)

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?)

2827 ↗

(On Diff #294074)

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

4158 ↗

(On Diff #294074)

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

[rsmith]

clang/lib/AST/ExprConstant.cpp

12242 ↗

(On Diff #294074)

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.

13290 ↗

(On Diff #294074)

13347 ↗

(On Diff #294074)

13383 ↗

(On Diff #294074)

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

13406 ↗

(On Diff #294074)

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

13407 ↗

(On Diff #294074)

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

13197–13198 ↗

(On Diff #291736)

Yes, we should check for anything that could behave differently in a different FP environment.

[mibintc]

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!

[sepavloff]

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

[rjmccall]

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

[mibintc]

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

[sepavloff]

clang/include/clang/Basic/LangOptions.h

410–411 ↗

(On Diff #295880)

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.

[mibintc]

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.000000e+00

^

<stdin>:1:1: note: scanning from here
; ModuleID = '/export/iusers/mblower/sandbox/llorg/llvm-project/clang/test/AST/const-fpfeatures.cpp'
^
<stdin>:24:1: note: possible intended match here
@V1 = global float 0.000000e+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?

clang/lib/AST/ExprConstant.cpp

2734 ↗

(On Diff #295880)

This should be FFDiag?

12302 ↗

(On Diff #295880)

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.

clang/test/CodeGen/pragma-fenv_access.c

10

Yes thanks @sepavloff ! i made this change

[mibintc]

clang/include/clang/Basic/LangOptions.h

410–411 ↗

(On Diff #295880)

Thanks @sepavloff I'll make the change you recommend

[mibintc]

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.

[mibintc]

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.

[sepavloff]

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?

[mibintc]

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.

[mibintc]

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?

[mibintc]

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?

[rjmccall]

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.

[rsmith]

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.

[rsmith]

clang/lib/AST/ExprConstant.cpp

2445 ↗

(On Diff #297260)

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.

2496–2498 ↗

(On Diff #297260)

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

12302 ↗

(On Diff #297260)

Skip this check if Info.InConstantContext.

[mibintc]

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

[mibintc]

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

clang/lib/AST/ExprConstant.cpp

2496–2498 ↗

(On Diff #297260)

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.

clang/test/CodeGen/fp-floatcontrol-pragma.cpp

159

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

clang/test/Parser/pragma-fenv_access.c

29

@rsmith no diagnostic, is this OK?

[mibintc]

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

[rsmith]

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.

clang/test/CodeGen/fp-floatcontrol-pragma.cpp

159

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.

clang/test/Parser/pragma-fenv_access.c

29

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

[sepavloff]

LGTM

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

Looking forward to results of SPEC runs!

clang/docs/UsersManual.rst

1389 ↗

(On Diff #299426)

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.

clang/lib/Sema/SemaAttr.cpp

1020–1023 ↗

(On Diff #299426)

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?

clang/lib/Sema/SemaDecl.cpp

14281–14282 ↗

(On Diff #299426)

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

[mibintc]

clang/lib/Sema/SemaAttr.cpp

1020–1023 ↗

(On Diff #299426)

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

[mibintc]

clang/test/Parser/pragma-fenv_access.c

29

I'm not seeing the expected-error for e_three_thirds = (int)(frac * 3)

[rsmith]

clang/test/Parser/pragma-fenv_access.c

29

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.

[sepavloff]

clang/lib/Sema/SemaAttr.cpp

1020–1023 ↗

(On Diff #299426)

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 test case should work:

// RUN: %clang_cc1 -S -emit-llvm %s -o - | FileCheck %s

float func_01(float x, float y, float z) {
  #pragma STDC FENV_ACCESS ON
  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.strict")
// 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.strict")

Again, the pragma in the inner block claims there is no access to FP environment, but outer block can have set non-default rounding mode.

[mibintc]

I updated the test cases, added documentation about constant folding to the UserManual. I'll push the patch tomorrow if I don't hear to the contrary thanks everybody!

[dmajor]

After this commit our bots have a failure on the wasm target, could you please take a look?

test.c:

double a, b;
c() {
#pragma STDC FENV_ACCESS ON
  b == a;
}

Run clang -cc1 -triple wasm32-unknown-wasi -emit-obj test.c (clang needs to be build with -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=WebAssembly)

Result:

clang: /home/vm/src/llvm-project/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:3764: bool (anonymous namespace)::SelectionDAGLegalize::ExpandNode(llvm::SDNode *): Assertion `!IsStrict && "Don't know how to expand for strict nodes."' failed.

[mibintc]

Thank you. I don't know the solution but I can remove the assert and create a bug report to have it resolved.

[mibintc]

Actually kludging it by just removing the assert isn't going to work. I'll ping Pengfei to see about developing a patch for this problem.

[craig.topper]

In D87528#2357043, @mibintc wrote:

Actually kludging it by just removing the assert isn't going to work. I'll ping Pengfei to see about developing a patch for this problem.

This is likely a WebAssembly backend problem. Each backend needs to do work to support constrained intrinsics. D80952 added a way to know if a target supports constrained intrinsics. Probably need to apply that to the changes here?

[pengfei]

I agreed with Craig. Emitting constrained intrinsics on an unsupported target may result in problems. It's better to check if it is supported and prevent from the front end.

[uabelho]

We got similar problems for our downstream target. We worked around that by bailing out early in PragmaSTDC_FENV_ACCESSHandler::HandlePragma for our target but perhaps one could check hasStrictFP() instead and ignore the pragma if that is false?

[mibintc]

In D87528#2357970, @pengfei wrote:

I agreed with Craig. Emitting constrained intrinsics on an unsupported target may result in problems. It's better to check if it is supported and prevent from the front end.

Here's a patch for review, check if target supports Strict FP, otherwise ignore the pragma. See https://reviews.llvm.org/D90316. Thank you

clang/include/clang/Basic/DiagnosticParseKinds.td

def warn_pragma_init_seg_unsupported_target : Warning<
   InGroup<IgnoredPragmas>;
 // - #pragma restricted to file scope or start of compound statement
 def err_pragma_file_or_compound_scope : Error<
   "'#pragma %0' can only appear at file scope or at the start of a "
   "compound statement">;
 // - #pragma stdc unknown
 def ext_stdc_pragma_ignored : ExtWarn<"unknown pragma in STDC namespace">,
    InGroup<UnknownPragmas>;
-def warn_stdc_fenv_access_not_supported :
-   Warning<"pragma STDC FENV_ACCESS ON is not supported, ignoring pragma">,
-   InGroup<UnknownPragmas>;
+def err_pragma_stdc_fenv_access_scope : Error<
+  "'#pragma STDC FENV_ACCESS' can only appear at file scope or at the start of a "
+  "compound statement">;
 def warn_stdc_fenv_round_not_supported :
    Warning<"pragma STDC FENV_ROUND is not supported">,
    InGroup<UnknownPragmas>;
 def warn_stdc_unknown_rounding_mode : Warning<
   "invalid or unsupported rounding mode in '#pragma STDC FENV_ROUND' - ignored">,
   InGroup<IgnoredPragmas>;
 // - #pragma comment
 def err_pragma_comment_malformed : Error<

clang/include/clang/Sema/Sema.h

public:
 
   StmtResult ActOnNullStmt(SourceLocation SemiLoc,
                            bool HasLeadingEmptyMacro = false);
 
   void ActOnStartOfCompoundStmt(bool IsStmtExpr);
   void ActOnFinishOfCompoundStmt();
   StmtResult ActOnCompoundStmt(SourceLocation L, SourceLocation R,
                                ArrayRef<Stmt *> Elts, bool isStmtExpr);
+  void ActOnAfterCompoundStatementLeadingPragmas();
 
   /// A RAII object to enter scope of a compound statement.
   class CompoundScopeRAII {
   public:
     CompoundScopeRAII(Sema &S, bool IsStmtExpr = false) : S(S) {
       S.ActOnStartOfCompoundStmt(IsStmtExpr);
     }
 

clang/lib/CodeGen/CodeGenFunction.cpp

if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D)) {
     if ((getLangOpts().CPlusPlus && FD->isMain()) || getLangOpts().OpenCL ||
         getLangOpts().SYCLIsDevice ||
         (getLangOpts().CUDA && FD->hasAttr<CUDAGlobalAttr>()))
       Fn->addFnAttr(llvm::Attribute::NoRecurse);
   }
 
   if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D)) {
     Builder.setIsFPConstrained(FD->usesFPIntrin());
-    if (FD->usesFPIntrin())
+    if (FD->usesFPIntrin()) {
       Fn->addFnAttr(llvm::Attribute::StrictFP);
+      Builder.setIsFPConstrained(true);
+    }
   }
 
   // If a custom alignment is used, force realigning to this alignment on
   // any main function which certainly will need it.
   if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D))
     if ((FD->isMain() || FD->isMSVCRTEntryPoint()) &&
         CGM.getCodeGenOpts().StackAlignment)
       Fn->addFnAttr("stackrealign");

clang/lib/CodeGen/CodeGenModule.h

public:
   /// Set the LLVM function attributes (sext, zext, etc).
   void SetLLVMFunctionAttributes(GlobalDecl GD, const CGFunctionInfo &Info,
                                  llvm::Function *F);
 
   /// Set the LLVM function attributes which only apply to a function
   /// definition.
   void SetLLVMFunctionAttributesForDefinition(const Decl *D, llvm::Function *F);
 
+  /// Set the LLVM function attributes that represent floating point
+  /// environment.
+  void setLLVMFunctionFEnvAttributes(const FunctionDecl *D, llvm::Function *F);
+
   /// Return true iff the given type uses 'sret' when used as a return type.
   bool ReturnTypeUsesSRet(const CGFunctionInfo &FI);
 
   /// Return true iff the given type uses an argument slot when 'sret' is used
   /// as a return type.
   bool ReturnSlotInterferesWithArgs(const CGFunctionInfo &FI);
 
   /// Return true iff the given type uses 'fpret' when used as a return type.

clang/lib/CodeGen/CodeGenModule.cpp

for (const CXXRecordDecl *Base : getMostBaseClasses(MD->getParent())) {
       llvm::Metadata *Id =
           CreateMetadataIdentifierForType(Context.getMemberPointerType(
               MD->getType(), Context.getRecordType(Base).getTypePtr()));
       F->addTypeMetadata(0, Id);
     }
   }
 }
 
+void CodeGenModule::setLLVMFunctionFEnvAttributes(const FunctionDecl *D,
+                                                  llvm::Function *F) {
+  if (D->usesFPIntrin()) {
+    llvm::AttrBuilder FuncAttrs;
+    FuncAttrs.addAttribute("strictfp");
+    F->addAttributes(llvm::AttributeList::FunctionIndex, FuncAttrs);
+  }
+}
+
 void CodeGenModule::SetCommonAttributes(GlobalDecl GD, llvm::GlobalValue *GV) {
   const Decl *D = GD.getDecl();
   if (dyn_cast_or_null<NamedDecl>(D))
     setGVProperties(GV, GD);
   else
     GV->setVisibility(llvm::GlobalValue::DefaultVisibility);
 
   if (D && D->hasAttr<UsedAttr>())
void CodeGenModule::EmitGlobalFunctionDefinition(GlobalDecl GD,
   auto *Fn = cast<llvm::Function>(GV);
   setFunctionLinkage(GD, Fn);
 
   // FIXME: this is redundant with part of setFunctionDefinitionAttributes
   setGVProperties(Fn, GD);
 
   MaybeHandleStaticInExternC(D, Fn);
 
-
   maybeSetTrivialComdat(*D, *Fn);
 
+  // Set CodeGen attributes that represent floating point environment.
+  setLLVMFunctionFEnvAttributes(D, Fn);
+
   CodeGenFunction(*this).GenerateCode(GD, Fn, FI);
 
   setNonAliasAttributes(GD, Fn);
   SetLLVMFunctionAttributesForDefinition(D, Fn);
 
   if (const ConstructorAttr *CA = D->getAttr<ConstructorAttr>())
     AddGlobalCtor(Fn, CA->getPriority());
   if (const DestructorAttr *DA = D->getAttr<DestructorAttr>())

clang/lib/Parse/ParsePragma.cpp

 struct PragmaSTDC_FENV_ACCESSHandler : public PragmaHandler {
   PragmaSTDC_FENV_ACCESSHandler() : PragmaHandler("FENV_ACCESS") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
                     Token &Tok) override {
     tok::OnOffSwitch OOS;
     if (PP.LexOnOffSwitch(OOS))
      return;
-    if (OOS == tok::OOS_ON) {
-      PP.Diag(Tok, diag::warn_stdc_fenv_access_not_supported);
-      return;
-    }
 
     MutableArrayRef<Token> Toks(PP.getPreprocessorAllocator().Allocate<Token>(1),
                                 1);
     Toks[0].startToken();
     Toks[0].setKind(tok::annot_pragma_fenv_access);
     Toks[0].setLocation(Tok.getLocation());
     Toks[0].setAnnotationEndLoc(Tok.getLocation());
     Toks[0].setAnnotationValue(reinterpret_cast<void*>(

clang/lib/Parse/ParseStmt.cpp

Retry:
   case tok::annot_pragma_fp:
     ProhibitAttributes(Attrs);
     Diag(Tok, diag::err_pragma_file_or_compound_scope) << "clang fp";
     ConsumeAnnotationToken();
     return StmtError();
 
   case tok::annot_pragma_fenv_access:
     ProhibitAttributes(Attrs);
-    HandlePragmaFEnvAccess();
+    Diag(Tok, diag::err_pragma_stdc_fenv_access_scope);
+    ConsumeAnnotationToken();
     return StmtEmpty();
 
   case tok::annot_pragma_fenv_round:
     ProhibitAttributes(Attrs);
     Diag(Tok, diag::err_pragma_file_or_compound_scope) << "STDC FENV_ROUND";
     ConsumeAnnotationToken();
     return StmtError();
 

 /// ActOnCompoundStmt action.  This expects the '{' to be the current token, and
 /// consume the '}' at the end of the block.  It does not manipulate the scope
 /// stack.
 StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
   PrettyStackTraceLoc CrashInfo(PP.getSourceManager(),
                                 Tok.getLocation(),
                                 "in compound statement ('{}')");
 
-  // Record the state of the FPFeatures, restore on leaving the
+  // Record the current FPFeatures, restore on leaving the
   // compound statement.
-  Sema::FPFeaturesStateRAII SaveFPContractState(Actions);
+  Sema::FPFeaturesStateRAII SaveFPFeatures(Actions);
 
   InMessageExpressionRAIIObject InMessage(*this, false);
   BalancedDelimiterTracker T(*this, tok::l_brace);
   if (T.consumeOpen())
     return StmtError();
 
   Sema::CompoundScopeRAII CompoundScope(Actions, isStmtExpr);
 
   // Parse any pragmas at the beginning of the compound statement.
   ParseCompoundStatementLeadingPragmas();
+  Actions.ActOnAfterCompoundStatementLeadingPragmas();
 
   StmtVector Stmts;
 
   // "__label__ X, Y, Z;" is the GNU "Local Label" extension.  These are
   // only allowed at the start of a compound stmt regardless of the language.
   while (Tok.is(tok::kw___label__)) {
     SourceLocation LabelLoc = ConsumeToken();
 

clang/lib/Sema/SemaStmt.cpp

 void Sema::ActOnStartOfCompoundStmt(bool IsStmtExpr) {
   PushCompoundScope(IsStmtExpr);
 }
 
 void Sema::ActOnFinishOfCompoundStmt() {
   PopCompoundScope();
 }
 
+void Sema::ActOnAfterCompoundStatementLeadingPragmas() {
+  if (getCurFPFeatures().getAllowFEnvAccess()) {
+    FunctionDecl *F = getCurFunctionDecl();
+    F->setUsesFPIntrin(true);
+  }
+}
+
 sema::CompoundScopeInfo &Sema::getCurCompoundScope() const {
   return getCurFunction()->CompoundScopes.back();
 }
 
 StmtResult Sema::ActOnCompoundStmt(SourceLocation L, SourceLocation R,
                                    ArrayRef<Stmt *> Elts, bool isStmtExpr) {
   const unsigned NumElts = Elts.size();
 

clang/test/CodeGen/fp-floatcontrol-pragma.cpp

 // RUN: %clang_cc1 -DEXCEPT=1 -fcxx-exceptions -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-NS %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -verify -DFENV_ON=1 -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -DFENV_ON=1 -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-FENV %s
 // RUN: %clang_cc1 -triple %itanium_abi_triple -O3 -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-O3 %s
 
 // Verify float_control(precise, off) enables fast math flags on fp operations.
 float fp_precise_1(float a, float b, float c) {
 // CHECK-O3: _Z12fp_precise_1fff
 // CHECK-O3: %[[M:.+]] = fmul fast float{{.*}}
 // CHECK-O3: fadd fast float %[[M]], %c
 #pragma float_control(precise, off)

 
 float test_OperatorCall() {
   return add(1.0f, 2.0f);
   //CHECK: llvm.experimental.constrained.fadd{{.*}}fpexcept.strict
 }
 // CHECK-LABEL define float  {{.*}}test_OperatorCall{{.*}}
 
 #if FENV_ON
-// expected-warning@+1{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
 #pragma STDC FENV_ACCESS ON
 #endif
 // CHECK-LABEL: define {{.*}}callt{{.*}}
 
 void callt() {
   volatile float z;
   z = z * z;
-//CHECK: = fmul float
+  //CHECK-FENV: llvm.experimental.constrained.fmul{{.*}}
+}
+
+// CHECK-LABEL: define {{.*}}myAdd{{.*}}
+float myAdd() {
+  return 1.0 + 2.0;
+  // Check that floating point constant folding doesn't occur if
+  // #pragma STC FENV_ACCESS is enabled.
+  //CHECK-FENV: llvm.experimental.constrained.fadd{{.*}}double 1.0{{.*}}double 2.0{{.*}}
+  //CHECK: ret float 3.0{{.*}}
 }
 
 #if EXCEPT
 namespace ns {
 // Check that pragma float_control can appear in namespace.
 #pragma float_control(except, on, push)
 float exc_on(double x, float zero) {
 // CHECK-NS: define {{.*}}exc_on{{.*}}
   {} try {
     x = 1.0 / zero; /* division by zero, the result unused */

clang/test/CodeGen/pragma-fenv_access.c

+// RUN: %clang_cc1 -ffp-exception-behavior=strict -frounding-math -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.dynamic", metadata !"fpexcept.strict")
+
+
+float func_02(float x, float y) {
+  #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.dynamic", metadata !"fpexcept.strict")
+
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(float x, float y) {
+  return x + y;
+}
+// CHECK-LABEL: @func_04
+// CHECK: fadd float {{.*}}
+
+
+float func_05(float x, float y) {
+  #pragma STDC FENV_ACCESS ON
+  return x + y;
+}
+// CHECK-LABEL: @func_05
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+
+
+float func_06(float x, float y) {
+  return x + y;
+}
+// CHECK-LABEL: @func_06
+// CHECK: fadd float {{.*}}
+
+
+float func_07(float x, float y) {
+  x -= y;
+  if (x) {
+    #pragma STDC FENV_ACCESS ON
+    y *= 2;
+  }
+  return y + 4;
+}
+// CHECK-LABEL: @func_07
+// CHECK: call float @llvm.experimental.constrained.fsub.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// CHECK: call float @llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")
+// CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.dynamic", metadata !"fpexcept.strict")

clang/test/Parser/fp-floatcontrol-syntax.cpp

 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fdenormal-fp-math=preserve-sign,preserve-sign -ftrapping-math -fsyntax-only %s -DDEFAULT -verify
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only %s -ffp-contract=fast -DPRECISE -verify
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only %s -ftrapping-math -ffp-contract=off -frounding-math -ffp-exception-behavior=strict -DSTRICT -verify
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -menable-no-infs -menable-no-nans -menable-unsafe-fp-math -fno-signed-zeros -mreassociate -freciprocal-math -ffp-contract=fast -ffast-math -ffinite-math-only -fsyntax-only %s -DFAST -verify
 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)
 #pragma float_control(except, on) // OK
 #ifndef STRICT
 #pragma float_control(except, on)
 #pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when except is enabled}}
 #endif

clang/test/Parser/pragma-fenv_access.c

+// 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;
+}

clang/test/Preprocessor/pragma_unknown.c

 #pragma STDC FP_CONTRACT OFF
 #pragma STDC FP_CONTRACT DEFAULT
 #pragma STDC FP_CONTRACT IN_BETWEEN  // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT ON{{$}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT OFF{{$}}
 // 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 
 #pragma STDC CX_LIMITED_RANGE IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
 // CHECK: {{^}}#pragma STDC CX_LIMITED_RANGE ON{{$}}
 // CHECK: {{^}}#pragma STDC CX_LIMITED_RANGE OFF{{$}}
 // CHECK: {{^}}#pragma STDC CX_LIMITED_RANGE DEFAULT{{$}}
 // CHECK: {{^}}#pragma STDC CX_LIMITED_RANGE IN_BETWEEN{{$}}