Page MenuHomePhabricator

Enable '#pragma STDC FENV_ACCESS' in frontend
ClosedPublic

Authored by mibintc on Sep 11 2020, 10:09 AM.

Details

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

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
rsmith added inline comments.Sep 28 2020, 9:27 AM
clang/lib/AST/ExprConstant.cpp
2895

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

3042

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

4387

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

12556

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.

13547–13548

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

13631
13689
13726

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

13750

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

13751

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

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!

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

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

mibintc updated this revision to Diff 295880.Oct 2 2020, 11:30 AM

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 added inline comments.Oct 5 2020, 12:25 AM
clang/include/clang/Basic/LangOptions.h
415–416

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.

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
2899

This should be FFDiag?

12567

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 added inline comments.Oct 5 2020, 8:37 AM
clang/include/clang/Basic/LangOptions.h
415–416

Thanks @sepavloff I'll make the change you recommend

mibintc updated this revision to Diff 296221.Oct 5 2020, 10:34 AM

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.

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.

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 updated this revision to Diff 296520.Oct 6 2020, 12:36 PM

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 updated this revision to Diff 297260.Oct 9 2020, 9:42 AM

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?

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?

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.

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 added inline comments.Oct 16 2020, 3:41 PM
clang/lib/AST/ExprConstant.cpp
2607

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.

2659–2666

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

12565

Skip this check if Info.InConstantContext.

mibintc updated this revision to Diff 299395.Oct 20 2020, 9:56 AM

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 marked 2 inline comments as done.Oct 20 2020, 10:20 AM

@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
2659–2666

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 updated this revision to Diff 299426.Oct 20 2020, 11:22 AM

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 accepted this revision.Oct 20 2020, 3:57 PM

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

This revision is now accepted and ready to land.Oct 20 2020, 3:57 PM
sepavloff accepted this revision.Oct 20 2020, 11:28 PM

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

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

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
14291–14292

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 added inline comments.Oct 22 2020, 12:27 PM
clang/lib/Sema/SemaAttr.cpp
1020–1023

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 added inline comments.Oct 22 2020, 2:43 PM
clang/test/Parser/pragma-fenv_access.c
29

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

rsmith added inline comments.Oct 22 2020, 8:05 PM
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 added inline comments.Oct 23 2020, 3:46 AM
clang/lib/Sema/SemaAttr.cpp
1020–1023

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 updated this revision to Diff 300494.Oct 24 2020, 10:54 AM
mibintc retitled this revision from Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress to Enable '#pragma STDC FENV_ACCESS' in frontend.

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!

This revision was landed with ongoing or failed builds.Oct 25 2020, 7:10 AM
This revision was automatically updated to reflect the committed changes.
dmajor added a subscriber: dmajor.Oct 27 2020, 8:57 AM

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.

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

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.

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?

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.

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?

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