Include math-errno with fast-math

[zahiraam]

Summary

Take math-errno into account with '#pragma float_control(precise,on)' and 'attribute__((optnone)).

https://github.com/llvm/llvm-project/issues/62098

[andrew.w.kaylor]

clang/include/clang/Basic/FPOptions.def

29

Does this mean MathErrno is tracked in both LangOpts and FPOptions?

clang/lib/CodeGen/CGBuiltin.cpp

2232

You should add a comment here explaining what this is doing. I don't think it's obvious apart from the description of this patch.

clang/lib/CodeGen/CGCall.cpp

2384

I don't understand what this is doing.

clang/lib/CodeGen/CodeGenModule.h

611

Why is this a module level flag, and why does it default to true?

clang/test/CodeGen/math-errno.c

3

Isn't math-errno true by default when fast-math isn't used?

50

I think the 'afn' flag here is a problem. The backend has no concept of errno, so 'afn' will be treated as allowing the function to be replaced.

[andrew.w.kaylor]

clang/test/CodeGen/math-errno.c

34

Can you add a runline with -O0. That should prevent all instances of the intrinsics, right?

[zahiraam]

clang/include/clang/Basic/FPOptions.def

29

Yes. So that we can check that if it's overridden by a pragma or an attribute. I don't see another way.

clang/test/CodeGen/math-errno.c

34

Yes.

50

Added all the commands that are triggered by fast-math in the RUN line command.

[andrew.w.kaylor]

clang/lib/CodeGen/CGBuiltin.cpp

2240

Since we handle "#pragma float_control" for all targets, we should be consistent. I don't think the reference to MSVC here is useful.

I'm not sure why MSVC thinks it needs to call the sqrtf() function at O1, but I don't necessarily think we should follow that lead. If we generate the intrinsic here, the optimizer will make its own decisions about O1 versus O2, etc. There's no reason that errno should be preserved at O1 if it's been disabled with -fno-math-errno or we're enabled fast-math.

2252

Based on the comment above, if the function call is marked as const, then it will never set errno, so I don't think we need to check EmitIntrinsic in that case.

2254

I'm confused by this condition. If MathErrnoOverride is true, don't we need to set errno regardless of the optimization level or fast-math setting?

2257

Why do we need to check FastMath if the function is marked as optnone?

clang/test/CodeGen/math-errno.c

29

Shouldn't the precise pragma remove the nofpcalss(nan inf) attributes? The definition of setFPPreciseEnabled looks like it's trying to do that. Maybe we need another patch to handle that?

41

Again, this may be beyond the scope of your change, but it seems like '#pragma float_control(precise, off) should lead to fast-math flags being set on this call.

44

I expected the intrinsic to be called in this case. Do you know why it isn't? I believe it is without your change.

47

I didn't expect to see the intrinsic in this case. If we use the intrinsic, the x86-backend will generate sqrtss instead of the function call, and I think that is wrong.

57

Again, I didn't expect the intrinsic with optnone in any of these cases.

[zahiraam]

clang/test/CodeGen/math-errno.c

3

Yes, that's correct. This RUN line simulates compilation at -O2. The following cc1 options are triggered: -fmath-errno -ffp-contract=on
-fno-rounding-math -O2. That's what I am using here.

29

Shouldn't the precise pragma remove the nofpcalss(nan inf) attributes? The definition of setFPPreciseEnabled looks like it's trying to do that. Maybe we need another patch to handle that?

I will make a note of it and work on it in a subsequent patch.

41

Again, this may be beyond the scope of your change, but it seems like '#pragma float_control(precise, off) should lead to fast-math flags being set on this call.

Yes. Making a note of it.

[zahiraam]

Review anyone please? Thanks.

[aaron.ballman]

clang/include/clang/Basic/FPOptions.def

28–30

Shouldn't this one be Float16ExcessPrecision? (Are we missing test coverage that would have caught that?)

clang/include/clang/Basic/LangOptions.h

853

Everything else does !Value; is it intentional that you're using Value instead?

clang/lib/CodeGen/CGBuiltin.cpp

2256–2263

I think this has gotten sufficiently complex that it might be worth splitting the logic out a bit more:

bool Optimize = FD->hasAttr<ConstAttr>() && !ErrnoOverriden && !OptNone;
if (!Optimize) Optimize = FD->hasAttr<ConstAttr>() && !ErrnoOverriden && !OptNone;
... and so on ...

WDYT?

[zahiraam]

clang/include/clang/Basic/FPOptions.def

28–30

oops! the test would be for the BFloat16ExcessPrecision, right?

clang/include/clang/Basic/LangOptions.h

853

Yes. I want to set the value of math-errno to the value of the pragma in the source.

[aaron.ballman]

clang/include/clang/Basic/FPOptions.def

28–30

Yup -- that can be handled in a separate patch as it doesn't really have much to do with this one -- just something I noticed as a drive-by.

clang/include/clang/Basic/LangOptions.h

853

Hmm, okay. Making sure I understand the logic, please excuse me if this is a dumb question. :-) So the idea here is that if fp_precise is on, we have to honor math errno, and if fp_precise is off, we can ignore math errno?

clang/lib/CodeGen/CGBuiltin.cpp

2251

Is the part about -O2 accurate? We just check not -O0 in the code.

[zahiraam]

clang/include/clang/Basic/LangOptions.h

853

Correct.

[zahiraam]

clang/lib/CodeGen/CGBuiltin.cpp

2256–2263

I understand I need to split the logic, but I don't get your proposal here. Do you mean?

bool Optimize = FD->hasAttr<ConstAttr>() && !ErrnoOverriden && !OptNone;
if (!Optimize) Optimize = !getLangOpts().MathErrno && !ErrnoOverriden && !OptNone;
... and so on ...

[aaron.ballman]

clang/lib/CodeGen/CGBuiltin.cpp

2256–2263

Oops, yeah, that a was copy pasta mistake on my part. You've got the right idea -- mostly just split up the logic and add comments explaining why the predicates exist.

[zahiraam]

clang/lib/CodeGen/CGBuiltin.cpp

2256–2263

Changed the name of the variable you proposed. Is that OK?

[aaron.ballman]

clang/lib/CodeGen/CGBuiltin.cpp

2256–2263

Totally fine by me, but there's some more logic that you can strip out of the if statement, and it'd help me out if there were some comments explaining why each of these blocks might disable generating intrinsics.

[aaron.ballman]

clang/lib/CodeGen/CGBuiltin.cpp

2253–2255

[aaron.ballman]

clang/docs/LanguageExtensions.rst

4698 ↗

(On Diff #545666)

4702 ↗

(On Diff #545666)

clang/docs/UsersManual.rst

1727 ↗

(On Diff #545666)

Can you re-flow this to 80 columns?

[zahiraam]

clang/docs/LanguageExtensions.rst

4702 ↗

(On Diff #545666)

At least I was consistent :-)

clang/docs/UsersManual.rst

1727 ↗

(On Diff #545666)

Did the same for the rest of the text too. Is that OK?

[aaron.ballman]

clang/docs/UsersManual.rst

1727 ↗

(On Diff #545666)

Yeah, that's not the end of the world (if it was touching the whole file, that should be done as an NFC change outside of this patch, but these changes are related enough I think it's fine to do in this patch).

[aaron.ballman]

LGTM, but please add a release note describing the changes when you land them.

[zahiraam]

In D151834#4639536, @aaron.ballman wrote:

LGTM, but please add a release note describing the changes when you land them.

@aaron.ballman thanks for the reviews. Added some note in the RN, let me know if that is enough.

[aaron.ballman]

In D151834#4639600, @zahiraam wrote:

In D151834#4639536, @aaron.ballman wrote:

LGTM, but please add a release note describing the changes when you land them.

@aaron.ballman thanks for the reviews. Added some note in the RN, let me know if that is enough.

Release note looks great, thank you!

[zahiraam]

@andrew.w.kaylor Would you mind taking a last look at this before I can push this, please? I got the thumbs up from Aaron.

[andrew.w.kaylor]

lgtm

[thakis]

This breaks check-clang on macOS: http://45.33.8.238/macm1/68762/step_7.txt

Please take a look and revert for now if it takes a while to fix.

[zahiraam]

In D151834#4642195, @thakis wrote:

This breaks check-clang on macOS: http://45.33.8.238/macm1/68762/step_7.txt

Please take a look and revert for now if it takes a while to fix.

I have a fix for it here: https://reviews.llvm.org/D159486

[uabelho]

Hi @zahiraam ,

I have a couple of downstream testcases that fail with this patch.
Before

> clang bbi-86364.c -lm -O3
> ./a.out

passed but with the patch the assert in the program fails:

a.out: bbi-86364.c:9: int main(): Assertion `(*__errno_location ()) == 33' failed.

Is this as expected?

bbi-86364.c165 BDownload

[aaron.ballman]

In D151834#4643925, @uabelho wrote:

Hi @zahiraam ,

I have a couple of downstream testcases that fail with this patch.
Before

> clang bbi-86364.c -lm -O3
> ./a.out

passed but with the patch the assert in the program fails:

a.out: bbi-86364.c:9: int main(): Assertion `(*__errno_location ()) == 33' failed.

Is this as expected?

bbi-86364.c165 BDownload

This seems unexpected to me and it seems to relate to whether you include errno.h or not: https://godbolt.org/z/EPWzazx9r -- @zahiraam do you have ideas as to what's going on?

[zahiraam]

In D151834#4644373, @aaron.ballman wrote:

In D151834#4643925, @uabelho wrote:

Hi @zahiraam ,

I have a couple of downstream testcases that fail with this patch.
Before

> clang bbi-86364.c -lm -O3
> ./a.out

passed but with the patch the assert in the program fails:

a.out: bbi-86364.c:9: int main(): Assertion `(*__errno_location ()) == 33' failed.

Is this as expected?

bbi-86364.c165 BDownload

This seems unexpected to me and it seems to relate to whether you include errno.h or not: https://godbolt.org/z/EPWzazx9r -- @zahiraam do you have ideas as to what's going on?

I haven't looked at it as I saw that the comment has been deleted. Let me look into it.

[aaron.ballman]

In D151834#4644375, @zahiraam wrote:

In D151834#4644373, @aaron.ballman wrote:

In D151834#4643925, @uabelho wrote:

Hi @zahiraam ,

I have a couple of downstream testcases that fail with this patch.
Before

> clang bbi-86364.c -lm -O3
> ./a.out

passed but with the patch the assert in the program fails:

a.out: bbi-86364.c:9: int main(): Assertion `(*__errno_location ()) == 33' failed.

Is this as expected?

bbi-86364.c165 BDownload

This seems unexpected to me and it seems to relate to whether you include errno.h or not: https://godbolt.org/z/EPWzazx9r -- @zahiraam do you have ideas as to what's going on?

I haven't looked at it as I saw that the comment has been deleted. Let me look into it.

Oh, it's not the inclusion of errno.h that matters, it's the declaration of __errno_location: https://godbolt.org/z/zo4PaPEME -- it seems that inclusion of __attribute__ ((__const__)) is what makes the distinction: https://godbolt.org/z/1bePhvaG4

[zahiraam]

In D151834#4644378, @aaron.ballman wrote:

In D151834#4644375, @zahiraam wrote:

In D151834#4644373, @aaron.ballman wrote:

In D151834#4643925, @uabelho wrote:

Hi @zahiraam ,

I have a couple of downstream testcases that fail with this patch.
Before

> clang bbi-86364.c -lm -O3
> ./a.out

passed but with the patch the assert in the program fails:

a.out: bbi-86364.c:9: int main(): Assertion `(*__errno_location ()) == 33' failed.

Is this as expected?

bbi-86364.c165 BDownload

This seems unexpected to me and it seems to relate to whether you include errno.h or not: https://godbolt.org/z/EPWzazx9r -- @zahiraam do you have ideas as to what's going on?

I haven't looked at it as I saw that the comment has been deleted. Let me look into it.

Oh, it's not the inclusion of errno.h that matters, it's the declaration of __errno_location: https://godbolt.org/z/zo4PaPEME -- it seems that inclusion of __attribute__ ((__const__)) is what makes the distinction: https://godbolt.org/z/1bePhvaG4

Yes confirming this. In the errno.h header the function is declared: extern int *errno_location (void) attribute__ ((nothrow )) attribute ((const));
Since we can't really change the header, this needs to be changed in the compiler?

[aaron.ballman]

In D151834#4644391, @zahiraam wrote:

In D151834#4644378, @aaron.ballman wrote:

In D151834#4644375, @zahiraam wrote:

In D151834#4644373, @aaron.ballman wrote:

In D151834#4643925, @uabelho wrote:

Hi @zahiraam ,

I have a couple of downstream testcases that fail with this patch.
Before

> clang bbi-86364.c -lm -O3
> ./a.out

passed but with the patch the assert in the program fails:

a.out: bbi-86364.c:9: int main(): Assertion `(*__errno_location ()) == 33' failed.

Is this as expected?

bbi-86364.c165 BDownload

This seems unexpected to me and it seems to relate to whether you include errno.h or not: https://godbolt.org/z/EPWzazx9r -- @zahiraam do you have ideas as to what's going on?

I haven't looked at it as I saw that the comment has been deleted. Let me look into it.

Oh, it's not the inclusion of errno.h that matters, it's the declaration of __errno_location: https://godbolt.org/z/zo4PaPEME -- it seems that inclusion of __attribute__ ((__const__)) is what makes the distinction: https://godbolt.org/z/1bePhvaG4

Yes confirming this. In the errno.h header the function is declared: extern int *errno_location (void) attribute__ ((nothrow )) attribute ((const));
Since we can't really change the header, this needs to be changed in the compiler?

Yeah, I think this is a bug with your patch, but I'm not certain where. I suspect the changes to CGBuiltin.cpp are what are causing this issue -- CC @efriedma and @rjmccall as they might have ideas.

[zahiraam]

In D151834#4644394, @aaron.ballman wrote:

In D151834#4644391, @zahiraam wrote:

In D151834#4644378, @aaron.ballman wrote:

In D151834#4644375, @zahiraam wrote:

In D151834#4644373, @aaron.ballman wrote:

In D151834#4643925, @uabelho wrote:

Hi @zahiraam ,

I have a couple of downstream testcases that fail with this patch.
Before

> clang bbi-86364.c -lm -O3
> ./a.out

passed but with the patch the assert in the program fails:

a.out: bbi-86364.c:9: int main(): Assertion `(*__errno_location ()) == 33' failed.

Is this as expected?

bbi-86364.c165 BDownload

This seems unexpected to me and it seems to relate to whether you include errno.h or not: https://godbolt.org/z/EPWzazx9r -- @zahiraam do you have ideas as to what's going on?

I haven't looked at it as I saw that the comment has been deleted. Let me look into it.

Oh, it's not the inclusion of errno.h that matters, it's the declaration of __errno_location: https://godbolt.org/z/zo4PaPEME -- it seems that inclusion of __attribute__ ((__const__)) is what makes the distinction: https://godbolt.org/z/1bePhvaG4

Yes confirming this. In the errno.h header the function is declared: extern int *errno_location (void) attribute__ ((nothrow )) attribute ((const));
Since we can't really change the header, this needs to be changed in the compiler?

Yeah, I think this is a bug with your patch, but I'm not certain where. I suspect the changes to CGBuiltin.cpp are what are causing this issue -- CC @efriedma and @rjmccall as they might have ideas.

I think the issue comes from here: https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGBuiltin.cpp#L2365

[zahiraam]

In D151834#4644422, @zahiraam wrote:

In D151834#4644394, @aaron.ballman wrote:

In D151834#4644391, @zahiraam wrote:

In D151834#4644378, @aaron.ballman wrote:

In D151834#4644375, @zahiraam wrote:

In D151834#4644373, @aaron.ballman wrote:

In D151834#4643925, @uabelho wrote:

Hi @zahiraam ,

I have a couple of downstream testcases that fail with this patch.
Before

> clang bbi-86364.c -lm -O3
> ./a.out

passed but with the patch the assert in the program fails:

a.out: bbi-86364.c:9: int main(): Assertion `(*__errno_location ()) == 33' failed.

Is this as expected?

bbi-86364.c165 BDownload

This seems unexpected to me and it seems to relate to whether you include errno.h or not: https://godbolt.org/z/EPWzazx9r -- @zahiraam do you have ideas as to what's going on?

I haven't looked at it as I saw that the comment has been deleted. Let me look into it.

Oh, it's not the inclusion of errno.h that matters, it's the declaration of __errno_location: https://godbolt.org/z/zo4PaPEME -- it seems that inclusion of __attribute__ ((__const__)) is what makes the distinction: https://godbolt.org/z/1bePhvaG4

Yes confirming this. In the errno.h header the function is declared: extern int *errno_location (void) attribute__ ((nothrow )) attribute ((const));
Since we can't really change the header, this needs to be changed in the compiler?

Yeah, I think this is a bug with your patch, but I'm not certain where. I suspect the changes to CGBuiltin.cpp are what are causing this issue -- CC @efriedma and @rjmccall as they might have ideas.

I think the issue comes from here: https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGBuiltin.cpp#L2365

This looks like it's happening only for -Oi (i>=1) and on Linux only?

[uabelho]

In D151834#4644692, @zahiraam wrote:

[...]

I think the issue comes from here: https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGBuiltin.cpp#L2365

This looks like it's happening only for -Oi (i>=1) and on Linux only?

Yes I haven't seen it with -O0. I also haven't seen it for my out-of-tree target, only for linux/X86.

[uabelho]

What's the way forward here? Revert if it's unclear what to do?

[zahiraam]

In D151834#4645866, @uabelho wrote:

What's the way forward here? Revert if it's unclear what to do?

We have a fix for it. By eod a PR will be created. Thanks.

[uabelho]

In D151834#4645910, @zahiraam wrote:

In D151834#4645866, @uabelho wrote:

What's the way forward here? Revert if it's unclear what to do?

We have a fix for it. By eod a PR will be created. Thanks.

Ok, nice. Thanks!

clang/include/clang/Basic/FPOptions.def

 OPTION(AllowFEnvAccess, bool, 1, SpecifiedExceptionMode)
 OPTION(AllowFPReassociate, bool, 1, AllowFEnvAccess)
 OPTION(NoHonorNaNs, bool, 1, AllowFPReassociate)
 OPTION(NoHonorInfs, bool, 1, NoHonorNaNs)
 OPTION(NoSignedZero, bool, 1, NoHonorInfs)
 OPTION(AllowReciprocal, bool, 1, NoSignedZero)
 OPTION(AllowApproxFunc, bool, 1, AllowReciprocal)
 OPTION(FPEvalMethod, LangOptions::FPEvalMethodKind, 2, AllowApproxFunc)
 OPTION(Float16ExcessPrecision, LangOptions::ExcessPrecisionKind, 2, FPEvalMethod)
+OPTION(MathErrno, bool, 1, Float16ExcessPrecision)
 #undef OPTION

clang/include/clang/Basic/LangOptions.h

public:
 
   void setFPPreciseEnabled(bool Value) {
     setAllowFPReassociateOverride(!Value);
     setNoHonorNaNsOverride(!Value);
     setNoHonorInfsOverride(!Value);
     setNoSignedZeroOverride(!Value);
     setAllowReciprocalOverride(!Value);
     setAllowApproxFuncOverride(!Value);
+    setMathErrnoOverride(Value);
     if (Value)
       /* Precise mode implies fp_contract=on and disables ffast-math */
       setAllowFPContractWithinStatement();
     else
       /* Precise mode disabled sets fp_contract=fast and enables ffast-math */
       setAllowFPContractAcrossStatement();
   }
 

clang/lib/CodeGen/CGBuiltin.cpp

if (getTarget().getTriple().isPPC64() &&
     BuiltinID = mutateLongDoubleBuiltin(BuiltinID);
 
   // If the builtin has been declared explicitly with an assembler label,
   // disable the specialized emitting below. Ideally we should communicate the
   // rename in IR, or at least avoid generating the intrinsic calls that are
   // likely to get lowered to the renamed library functions.
   const unsigned BuiltinIDIfNoAsmLabel =
       FD->hasAttr<AsmLabelAttr>() ? 0 : BuiltinID;
+  bool MathErrnoOverrride = false;
+  if (E->hasStoredFPFeatures()) {
+    FPOptionsOverride OP = E->getFPFeatures();
+    if (OP.hasMathErrnoOverride())
+      MathErrnoOverrride = OP.getMathErrnoOverride();
+  }
+  bool EmitIntrinsic = !MathErrnoOverrride && CGM.OptNone;
 
   // There are LLVM math intrinsics/instructions corresponding to math library
   // functions except the LLVM op will never set errno while the math library
   // might. Also, math builtins have the same semantics as their math library
   // twins. Thus, we can transform math library and builtin calls to their
   // LLVM counterparts if the call is marked 'const' (known to never set errno).
   // In case FP exceptions are enabled, the experimental versions of the
   // intrinsics model those.
   bool ConstWithoutErrnoAndExceptions =
       getContext().BuiltinInfo.isConstWithoutErrnoAndExceptions(BuiltinID);
   bool ConstWithoutExceptions =
       getContext().BuiltinInfo.isConstWithoutExceptions(BuiltinID);
-  if (FD->hasAttr<ConstAttr>() ||
+
+  if ((FD->hasAttr<ConstAttr>() && EmitIntrinsic) ||
+      (EmitIntrinsic ||
        ((ConstWithoutErrnoAndExceptions || ConstWithoutExceptions) &&
-       (!ConstWithoutErrnoAndExceptions || (!getLangOpts().MathErrno)))) {
+         (!ConstWithoutErrnoAndExceptions ||
+         (!getLangOpts().MathErrno && EmitIntrinsic))))) {
     switch (BuiltinIDIfNoAsmLabel) {
     case Builtin::BIceil:
     case Builtin::BIceilf:
     case Builtin::BIceill:
     case Builtin::BI__builtin_ceil:
     case Builtin::BI__builtin_ceilf:
     case Builtin::BI__builtin_ceilf16:
     case Builtin::BI__builtin_ceill:
     case Builtin::BI__builtin_ceilf128:
       return RValue::get(emitUnaryMaybeConstrainedFPBuiltin(*this, E,
                                    Intrinsic::ceil,
                                    Intrinsic::experimental_constrained_ceil));
 
     case Builtin::BIcopysign:
     case Builtin::BIcopysignf:

clang/lib/CodeGen/CGCall.cpp

void CodeGenModule::ConstructAttributeList(StringRef Name,
   // The attributes can come from:
   // * LangOpts: -ffreestanding, -fno-builtin, -fno-builtin-<name>
   // * FunctionDecl attributes: __attribute__((no_builtin(...)))
   addNoBuiltinAttributes(FuncAttrs, getLangOpts(), NBA);
 
   // Collect function IR attributes based on global settiings.
   getDefaultFunctionAttributes(Name, HasOptnone, AttrOnCallSite, FuncAttrs);
 
+  if (HasOptnone && !getLangOpts().MathErrno)
+    OptNone = false;
+
   // Override some default IR attributes based on declaration-specific
   // information.
   if (TargetDecl) {
     if (TargetDecl->hasAttr<NoSpeculativeLoadHardeningAttr>())
       FuncAttrs.removeAttribute(llvm::Attribute::SpeculativeLoadHardening);
     if (TargetDecl->hasAttr<SpeculativeLoadHardeningAttr>())
       FuncAttrs.addAttribute(llvm::Attribute::SpeculativeLoadHardening);
     if (TargetDecl->hasAttr<NoSplitStackAttr>())

clang/lib/CodeGen/CodeGenModule.h

CodeGenModule(ASTContext &C, IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
                 const PreprocessorOptions &ppopts,
                 const CodeGenOptions &CodeGenOpts, llvm::Module &M,
                 DiagnosticsEngine &Diags,
                 CoverageSourceInfo *CoverageInfo = nullptr);
 
   ~CodeGenModule();
 
   void clear();
+  bool OptNone = true;
 
   /// Finalize LLVM code generation.
   void Release();
 
   /// Return true if we should emit location information for expressions.
   bool getExpressionLocationsEnabled() const;
 
   /// Return a reference to the configured Objective-C runtime.

clang/test/CodeGen/math-errno.c

+// Tests that at -O2 math-errno is taken into account. Same than MSVC.
+// RUN: %clang_cc1 -Wno-implicit-function-declaration -fmath-errno \
+// RUN: -O2 -emit-llvm -o - %s \
+// RUN: | FileCheck %s
+
+// Tests that with -ffast-math math-errno is taken into account with
+// float_control(precise,on) and optnone.
+// RUN: %clang_cc1 -Wno-implicit-function-declaration  \
+// RUN: -ffast-math -emit-llvm -o - %s \
+// RUN: | FileCheck %s -check-prefix=FAST
+
+float f1(float x) { 
+#pragma float_control(precise,on)
+return sqrtf(x);
+}
+
+float f2(float x) { 
+#pragma float_control(precise,on)
+  {
+    float y = sqrtf(x);
+    return y;
+  }
+}
+
+float f3(float x) { 
+#pragma float_control(precise,off)
+  {
+    float y = sqrtf(x);
+  return y;
+  }
+}
+
+__attribute__((optnone))
+float f4(float x) { 
+  x = sqrtf(x); 
+  return x;
+}
+
+// CHECK-LABEL: define dso_local float @f2(float noundef {{.*}})
+// CHECK:  tail call float @sqrtf(float noundef {{.*}})
+
+// CHECK-LABEL: define dso_local float @f3(float noundef {{.*}})
+// CHECK: call float @llvm.sqrt.f32(float {{.*}})
+
+// FAST-LABEL: define dso_local nofpclass(nan inf) float @f1(float noundef nofpclass(nan inf) {{.*}})
+// FAST: call reassoc nnan ninf nsz arcp afn nofpclass(nan inf) float @sqrtf(float noundef nofpclass(nan inf) {{.*}}) #[[ATTR0:[0-9]+]]
+
+// FAST-LABEL: define dso_local nofpclass(nan inf) float @f4(float noundef nofpclass(nan inf) {{.*}})
+// FAST: call reassoc nnan ninf nsz arcp afn nofpclass(nan inf) float @sqrtf(float noundef nofpclass(nan inf) %0) #[[ATTR0:[0-9]+]]
+
+// FAST: attributes #[[ATTR0]] = { {{.*}} memory(none) }
+