Implement inlining of strictfp functions

[sepavloff]

Summary

According to the current design, if a floating point operation is
represented by constrained intrinsic somewhere in a function, all
floating point operations in the function must be represented by
constrained intrinsics. It imposes additional requirements to inlining
mechanism. If non-strictfp function is inlined into strictfp function,
all ordinary FP operations must be replaced with their constrained
counterparts. Such behavior is implemented by this change.

Inlining strictfp function into non-strictfp is not implemented as it
would require replacement of all FP operations in the host function,
which now is undesirable due to expected performance loss.

[kpn]

llvm/lib/Transforms/Utils/CloneFunction.cpp

331

Not all constrained intrinsics take both metadata arguments. See the LangRef or the IR verifier for the details.

333

I don't see the strictfp attribute being added to the call.

335

Is this the path that a call instruction goes through? Because it'll need the strictfp attribute added.

llvm/test/Transforms/Inline/inline_strictfp.ll

15

All function calls in a strictfp-marked function require the strictfp attribute. See D68233 for the current version of the verifier for this.

[andrew.w.kaylor]

llvm/include/llvm/IR/Instruction.h

50

It seems wasteful to take one of these bits for something that can be deduced from other information we already have.

llvm/lib/Transforms/Utils/CloneFunction.cpp

310

There is some ongoing discussion about how predicated vector instructions will handle constrained FP mode. I think Simon's intention is to have just a single intrinsic that is used regardless of whether strictfp semantics are needed.

Also, in addition to converting fp intrinsics to strict equivalents, I think we need to add the strictfp attribute to callsites. We're currently preventing libcall simplification for callsites marked strictfp. The plan has been for front ends to mark all calls as strictfp so that they don't need to identify math library calls. I could probably be convinced that this is not necessary. Arguably, simplifyLibCalls could look at the callee's function attributes instead. @kpn has been considering whether this behavior should be relaxed.

[simoll]

llvm/lib/Transforms/Utils/CloneFunction.cpp

310

Yes, the idea for LLVM-VP is to have only one set of fp intrinsics for both constrained and default-env fp ops (eg llvm.vp.fadd).
The intrinsic declarations are qualified as unconstrained by default (the function has the readnone attribute and does not have strictfp).
If the VP call does not have the strictfp attribute, the metadata params have to specify the default-fp env.

The VP fp intrinsics are constrained, only if the callsite has the strictfp attribute. In that case, the same rules as for llvm.experimental.constrained.* intrinsics apply.

[sepavloff]

Updated patch

• add attribute strictfp to calls of constrained intrinsics,
• do not use flag DependsOnFPEnvironment.

[andrew.w.kaylor]

llvm/lib/Transforms/Utils/CloneFunction.cpp

312

After further investigation, I think it is going to be necessary to attach the strictfp attribute to all callsites. In the call to cloneBlock() below, we're calling SimplifyInstruction after a call has been cloned but before it is inserted into a function. Consequently, if we don't attach the strictfp attribute to library calls, SimplifyInstruction may constant fold them away. That's not so bad for the case where we're inlining a no-strictfp function into a strictfp function, but for the case where both functions are strictfp it's a problem.

[sepavloff]

Updated patch

• Do not add rounding mode for some intrinsics,
• Add attribute strictfp to all calls in inlined function,
• Some cleanup of tests.

[sepavloff]

llvm/lib/Transforms/Utils/CloneFunction.cpp

310

Yes, the idea for LLVM-VP is to have only one set of fp intrinsics for both constrained and default-env fp ops (eg llvm.vp.fadd).

It means that these calls do not need to be transformed and they may be processes as any other intrinsic.

The VP fp intrinsics are constrained, only if the callsite has the strictfp attribute.

As @andrew.w.kaylor pointed out, all function calls need to have strictfp attribute, it is now attached to all calls in the inlined function.

[simoll]

llvm/lib/Transforms/Utils/CloneFunction.cpp

310

As @andrew.w.kaylor pointed out, all function calls need to have strictfp attribute, it is now attached to all calls in the inlined function.

Great! That should work seamlessly for VP intrinsics once we enable their fp-constrained usage.

[sepavloff]

When function that uses default FP environment is inlined into strictfp function, code of the former will be executed in the FP environment set in the strictfp function. To fix this behavior the FP environment should be saved upon entry to the inlined function, FP environment reset to default state, and the saved state must be restored upon leaving the inlined function.

[craig.topper]

In D69798#1792231, @sepavloff wrote:

When function that uses default FP environment is inlined into strictfp function, code of the former will be executed in the FP environment set in the strictfp function. To fix this behavior the FP environment should be saved upon entry to the inlined function, FP environment reset to default state, and the saved state must be restored upon leaving the inlined function.

So if the function is inlined it would use the reset state, but if it doesn't get inlined it would use the caller's state. Doesn't that mean whether or not the compiler inlines the function changes the behavior of the program?

[sepavloff]

In D69798#1792237, @craig.topper wrote:

In D69798#1792231, @sepavloff wrote:

When function that uses default FP environment is inlined into strictfp function, code of the former will be executed in the FP environment set in the strictfp function. To fix this behavior the FP environment should be saved upon entry to the inlined function, FP environment reset to default state, and the saved state must be restored upon leaving the inlined function.

So if the function is inlined it would use the reset state, but if it doesn't get inlined it would use the caller's state. Doesn't that mean whether or not the compiler inlines the function changes the behavior of the program?

It is so now. If a code in which #pargma STDC FENV_ACCESS ON acts calls an external function, it would be executed in caller's FP environment. This is wrong if the callee expects default one. We could put get_fenv, reset_fenv and set_fenv, introduced in D71742, around all calls, unless the called function has strictfp attribute, or we know that it does not use FP operation. It however could create unneeded code, which is bad for performance. We need to elaborate proper solution.

[kpn]

In D69798#1792906, @sepavloff wrote:

In D69798#1792237, @craig.topper wrote:

In D69798#1792231, @sepavloff wrote:

When function that uses default FP environment is inlined into strictfp function, code of the former will be executed in the FP environment set in the strictfp function. To fix this behavior the FP environment should be saved upon entry to the inlined function, FP environment reset to default state, and the saved state must be restored upon leaving the inlined function.

So if the function is inlined it would use the reset state, but if it doesn't get inlined it would use the caller's state. Doesn't that mean whether or not the compiler inlines the function changes the behavior of the program?

It is so now. If a code in which #pargma STDC FENV_ACCESS ON acts calls an external function, it would be executed in caller's FP environment. This is wrong if the callee expects default one. We could put get_fenv, reset_fenv and set_fenv, introduced in D71742, around all calls, unless the called function has strictfp attribute, or we know that it does not use FP operation. It however could create unneeded code, which is bad for performance. We need to elaborate proper solution.

If FENV_ACCESS is ON and a function is called that expects it to be OFF then isn't that just plain undefined behavior? Unless it was changed since C99 I don't see how this is the compiler's problem to solve. And the compiler really shouldn't be changing the FP environment implicitly just because an arbitrary function was called, or we fell out of an FENV_ACCESS=ON scope, or whatever.

[uweigand]

In D69798#1793237, @kpn wrote:

If FENV_ACCESS is ON and a function is called that expects it to be OFF then isn't that just plain undefined behavior? Unless it was changed since C99 I don't see how this is the compiler's problem to solve. And the compiler really shouldn't be changing the FP environment implicitly just because an arbitrary function was called, or we fell out of an FENV_ACCESS=ON scope, or whatever.

Yes, exactly. The purpose of FENV_ACCESS is to inform the compiler about FP environment changes that the program (explicitly) performs; under no circumstances is FENV_ACCESS intended to instruct the compiler to change the FP env on its own.

[sepavloff]

In D69798#1793237, @kpn wrote:

In D69798#1792906, @sepavloff wrote:

In D69798#1792237, @craig.topper wrote:

In D69798#1792231, @sepavloff wrote:

When function that uses default FP environment is inlined into strictfp function, code of the former will be executed in the FP environment set in the strictfp function. To fix this behavior the FP environment should be saved upon entry to the inlined function, FP environment reset to default state, and the saved state must be restored upon leaving the inlined function.

So if the function is inlined it would use the reset state, but if it doesn't get inlined it would use the caller's state. Doesn't that mean whether or not the compiler inlines the function changes the behavior of the program?

It is so now. If a code in which #pargma STDC FENV_ACCESS ON acts calls an external function, it would be executed in caller's FP environment. This is wrong if the callee expects default one. We could put get_fenv, reset_fenv and set_fenv, introduced in D71742, around all calls, unless the called function has strictfp attribute, or we know that it does not use FP operation. It however could create unneeded code, which is bad for performance. We need to elaborate proper solution.

If FENV_ACCESS is ON and a function is called that expects it to be OFF then isn't that just plain undefined behavior? Unless it was changed since C99 I don't see how this is the compiler's problem to solve. And the compiler really shouldn't be changing the FP environment implicitly just because an arbitrary function was called, or we fell out of an FENV_ACCESS=ON scope, or whatever.

This is a matter of convention. If it is a user responsibility to call only 'proper' functions, FP state switch is not required. It however could be a fragile solution, because in complex programs it is hard to guarantee that non of the call recursively does not imply default FP environment. The safer solution is to save/restore the environment in unclear cases, and let backend to optimize out unnecessary calls.

[kpn]

In D69798#1794639, @sepavloff wrote:

In D69798#1793237, @kpn wrote:

If FENV_ACCESS is ON and a function is called that expects it to be OFF then isn't that just plain undefined behavior? Unless it was changed since C99 I don't see how this is the compiler's problem to solve. And the compiler really shouldn't be changing the FP environment implicitly just because an arbitrary function was called, or we fell out of an FENV_ACCESS=ON scope, or whatever.

This is a matter of convention. If it is a user responsibility to call only 'proper' functions, FP state switch is not required. It however could be a fragile solution, because in complex programs it is hard to guarantee that non of the call recursively does not imply default FP environment. The safer solution is to save/restore the environment in unclear cases, and let backend to optimize out unnecessary calls.

It _is_ the responsibility of the user to not call problematic functions. That _is_ the current convention. Yes, it may be fragile, but that's still the user's problem to solve. And the compiler can't know when the calls aren't needed because that information about functions in other TU simply isn't available. Plus, there are cases where functions are compiled with the #pragma and are expecting to be called with a non-default FP environment, but they don't change the environment themselves. So having the compiler insert calls to change the environment before calling would be an error. It's a mistake to trade off a set of potential errors caused by the programmer in exchange for a set of problems caused by the compiler.

[sepavloff]

In D69798#1794921, @kpn wrote:

In D69798#1794639, @sepavloff wrote:

In D69798#1793237, @kpn wrote:

If FENV_ACCESS is ON and a function is called that expects it to be OFF then isn't that just plain undefined behavior? Unless it was changed since C99 I don't see how this is the compiler's problem to solve. And the compiler really shouldn't be changing the FP environment implicitly just because an arbitrary function was called, or we fell out of an FENV_ACCESS=ON scope, or whatever.

This is a matter of convention. If it is a user responsibility to call only 'proper' functions, FP state switch is not required. It however could be a fragile solution, because in complex programs it is hard to guarantee that non of the call recursively does not imply default FP environment. The safer solution is to save/restore the environment in unclear cases, and let backend to optimize out unnecessary calls.

It _is_ the responsibility of the user to not call problematic functions. That _is_ the current convention. Yes, it may be fragile, but that's still the user's problem to solve. And the compiler can't know when the calls aren't needed because that information about functions in other TU simply isn't available. Plus, there are cases where functions are compiled with the #pragma and are expecting to be called with a non-default FP environment, but they don't change the environment themselves. So having the compiler insert calls to change the environment before calling would be an error. It's a mistake to trade off a set of potential errors caused by the programmer in exchange for a set of problems caused by the compiler.

You are right. The case of strictfp function which does not set FP environment is especially nice. The idea to surround external function calls with FP save/restore calls is not good. However when compiler makes inlining, it knows if the inlined function is strictfp. If it is, FP state is not modified. If it isn't, the function expects default FP environment and putting save/restore call brings FP state to that expected by the inlined function.

[kpn]

Imagine a case where some function "X()" under the #pragma and a non-default FP environment calls a function "Y()" in a different TU and not under the #pragma. Then the programmer moves "Y()" into a header file. Under your proposal Y() would get different FP environments at run time simply because it was moved from a different TU into a header file. Surprise!

You'd have the same issues if the programmer enabled LTO. Imagine having functions executing with differing FP environments depending on whether or not LTO was enabled.

No, the compiler should never change the FP environment implicitly. That way the compiler avoids introducing ugly surprises. The programmer would be left with just the mess they made themselves.

[sepavloff]

Updated patch

[andrew.w.kaylor]

I had forgotten that this patch never landed, but I was investigating a bug yesterday that I think this will help with (https://github.com/llvm/llvm-project/issues/48669).

@kpn are you happy with the current form. It's gotten stale in a few places, but I think it's basically correct.

[kpn]

In D69798#3408468, @andrew.w.kaylor wrote:

I had forgotten that this patch never landed, but I was investigating a bug yesterday that I think this will help with (https://github.com/llvm/llvm-project/issues/48669).

@kpn are you happy with the current form. It's gotten stale in a few places, but I think it's basically correct.

Yes, it looks fine. From reading my comments, I was worried about 'strictfp' not being attached to cloned calls. But it looks like it attaches it after the clone. So it's fine.

I'd love to see this go in the tree.

[andrew.w.kaylor]

@sepavloff I apologize for having lost track of this for so long. Do you have time to rebase this and the dependent patch?

[sepavloff]

Rebased and made some update

[sepavloff]

@andrew.w.kaylor Thank you for reviving this work!
I rebased the patches, the dependency patch has only small changes, this patch changed a bit more, because a new mechanism for intrinsic type parameters was used.

[andrew.w.kaylor]

lgtm

[sepavloff]

Thanks!

llvm/include/llvm/IR/Instruction.h

 };
 
 class Instruction : public User,
                     public ilist_node_with_parent<Instruction, BasicBlock> {
   BasicBlock *Parent;
   DebugLoc DbgLoc;                         // 'dbg' Metadata cache.
 
   enum {
+    /// Indicates that this instruction depends on floating point environment.
+    DependsOnFPEnvironment = 1 << 14,
     /// This is a bit stored in the SubClassData field which indicates whether
     /// this instruction has metadata attached to it or not.
     HasMetadataBit = 1 << 15
   };
 
 protected:
   ~Instruction(); // Use deleteValue() to delete a generic Instruction.
 
public:
   bool isFuncletPad() const { return isFuncletPad(getOpcode()); }
   bool isExceptionalTerminator() const {
     return isExceptionalTerminator(getOpcode());
   }
   bool isIndirectTerminator() const {
     return isIndirectTerminator(getOpcode());
   }
 
+  /// Returns true if the instruction depends on floating point environment.
+  bool dependsOnFPEnvironment() const {
+    return (getSubclassDataFromValue() & DependsOnFPEnvironment) != 0;
+  }
+
   static const char* getOpcodeName(unsigned OpCode);
 
   static inline bool isTerminator(unsigned OpCode) {
     return OpCode >= TermOpsBegin && OpCode < TermOpsEnd;
   }
 
   static inline bool isUnaryOp(unsigned Opcode) {
     return Opcode >= UnaryOpsBegin && Opcode < UnaryOpsEnd;
unsigned short getSubclassDataFromValue() const {
     return Value::getSubclassDataFromValue();
   }
 
   void setHasMetadataHashEntry(bool V) {
     setValueSubclassData((getSubclassDataFromValue() & ~HasMetadataBit) |
                          (V ? HasMetadataBit : 0));
   }
 
+  void setDependsOnFPEnvironment(bool V) {
+    setValueSubclassData(
+        (getSubclassDataFromValue() & ~DependsOnFPEnvironment) |
+        (V ? DependsOnFPEnvironment : 0));
+  }
+
   void setParent(BasicBlock *P);
 
 protected:
   // Instruction subclasses can stick up to 15 bits of stuff into the
   // SubclassData field of instruction with these members.
 
-  // Verify that only the low 15 bits are used.
+  // Verify that only the low 14 bits are used.
   void setInstructionSubclassData(unsigned short D) {
-    assert((D & HasMetadataBit) == 0 && "Out of range value put into field");
-    setValueSubclassData((getSubclassDataFromValue() & HasMetadataBit) | D);
+    assert((D & (DependsOnFPEnvironment | HasMetadataBit)) == 0 &&
+           "Out of range value put into field");
+    setValueSubclassData((getSubclassDataFromValue() &
+                          (DependsOnFPEnvironment | HasMetadataBit)) | D);
   }
 
   unsigned getSubclassDataFromInstruction() const {
-    return getSubclassDataFromValue() & ~HasMetadataBit;
+    return getSubclassDataFromValue() &
+           ~(DependsOnFPEnvironment | HasMetadataBit);
   }
 
   Instruction(Type *Ty, unsigned iType, Use *Ops, unsigned NumOps,
               Instruction *InsertBefore = nullptr);
   Instruction(Type *Ty, unsigned iType, Use *Ops, unsigned NumOps,
               BasicBlock *InsertAtEnd);
 
 private:

llvm/lib/IR/Instruction.cpp

 //===-- Instruction.cpp - Implement the Instruction class -----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the Instruction class for the IR library.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/FloatingPoint.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/Type.h"
 using namespace llvm;
 
 Instruction::Instruction(Type *ty, unsigned it, Use *Ops, unsigned NumOps,
                          Instruction *InsertBefore)
   : User(ty, Value::InstructionVal + it, Ops, NumOps), Parent(nullptr) {
 
+  if (getConstrainedIntrinsic(*this) != Intrinsic::not_intrinsic)
+    setDependsOnFPEnvironment(true);
+
   // If requested, insert this instruction into a basic block...
   if (InsertBefore) {
     BasicBlock *BB = InsertBefore->getParent();
     assert(BB && "Instruction to insert before is not in a basic block!");
     BB->getInstList().insert(InsertBefore->getIterator(), this);
   }
 }
 

llvm/lib/Transforms/Utils/CloneFunction.cpp

namespace {
   /// This is a private class used to implement CloneAndPruneFunctionInto.
   struct PruningFunctionCloner {
     Function *NewFunc;
     const Function *OldFunc;
     ValueToValueMapTy &VMap;
     bool ModuleLevelChanges;
     const char *NameSuffix;
     ClonedCodeInfo *CodeInfo;
+    bool HostFuncIsStrictFP;
+    bool InlinedFuncHasFPOps = false;
+
+    Instruction *cloneInstruction(BasicBlock::const_iterator II);
 
   public:
     PruningFunctionCloner(Function *newFunc, const Function *oldFunc,
                           ValueToValueMapTy &valueMap, bool moduleLevelChanges,
                           const char *nameSuffix, ClonedCodeInfo *codeInfo)
         : NewFunc(newFunc), OldFunc(oldFunc), VMap(valueMap),
           ModuleLevelChanges(moduleLevelChanges), NameSuffix(nameSuffix),
-          CodeInfo(codeInfo) {}
+          CodeInfo(codeInfo) {
+      HostFuncIsStrictFP = newFunc->getAttributes().hasAttribute(
+          AttributeList::FunctionIndex, Attribute::StrictFP);
+    }
 
     /// The specified block is found to be reachable, clone it and
     /// anything that it can reach.
     void CloneBlock(const BasicBlock *BB,
                     BasicBlock::const_iterator StartingInst,
                     std::vector<const BasicBlock*> &ToClone);
   };
 }
 
+Instruction *
+PruningFunctionCloner::cloneInstruction(BasicBlock::const_iterator II) {
+  const Instruction &OldInst = *II;
+  Instruction *NewInst;
+  if (HostFuncIsStrictFP && OldInst.dependsOnFPEnvironment()) {
+    // Instead of cloning the instruction, a call to constrained intrinsic
+    // should be created.
+    Intrinsic::ID CIID = getConstrainedIntrinsic(OldInst);
+    assert(CIID != Intrinsic::not_intrinsic);
+    Function *IFn = Intrinsic::getDeclaration(NewFunc->getParent(), CIID,
+                                              OldInst.getType());
+
+    // Assume that the first arguments of constrained intrinsics are the same as
+    // the operands of original instruction.
+    SmallVector<Value *, 4> Args;
+    for (auto &Op : OldInst.operands())
+      Args.push_back(Op);
+
+    // The last two arguments of a constrained intrinsic are metadata that
+    // represent rounding mode and exception behavior. The inlined function
+    // uses default settings.
+    LLVMContext &Ctx = NewFunc->getContext();
+    Value *RoundingMode = MetadataAsValue::get(Ctx, MDString::get(Ctx, "round.tonearest"));
+    Value *ExBehavior = MetadataAsValue::get(Ctx, MDString::get(Ctx, "fpexcept.ignore"));
+    Args.push_back(RoundingMode);
+    Args.push_back(ExBehavior);
+
+    NewInst = CallInst::Create(IFn, Args, OldInst.getName() + ".strict");
+  } else {
+    NewInst = II->clone();
+  }
+  return NewInst;
+}
+
 /// The specified block is found to be reachable, clone it and
 /// anything that it can reach.
 void PruningFunctionCloner::CloneBlock(const BasicBlock *BB,
                                        BasicBlock::const_iterator StartingInst,
                                        std::vector<const BasicBlock*> &ToClone){
   WeakTrackingVH &BBEntry = VMap[BB];
 
   // Have we already cloned this block?
void PruningFunctionCloner::CloneBlock(const BasicBlock *BB,
 
   bool hasCalls = false, hasDynamicAllocas = false, hasStaticAllocas = false;
 
   // Loop over all instructions, and copy them over, DCE'ing as we go.  This
   // loop doesn't include the terminator.
   for (BasicBlock::const_iterator II = StartingInst, IE = --BB->end();
        II != IE; ++II) {
 
-    Instruction *NewInst = II->clone();
+    Instruction *NewInst = cloneInstruction(II);
 
     // Eagerly remap operands to the newly cloned instruction, except for PHI
     // nodes for which we defer processing until we update the CFG.
     if (!isa<PHINode>(NewInst)) {
       RemapInstruction(NewInst, VMap,
                        ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges);
 
       // If we can simplify this instruction to some other value, simply add

llvm/lib/Transforms/Utils/InlineFunction.cpp

llvm::InlineResult llvm::InlineFunction(CallSite CS, InlineFunctionInfo &IFI,
 
   // If the call to the callee cannot throw, set the 'nounwind' flag on any
   // calls that we inline.
   bool MarkNoUnwind = CS.doesNotThrow();
 
   BasicBlock *OrigBB = TheCall->getParent();
   Function *Caller = OrigBB->getParent();
 
+  // Do not inline strictfp function into non-strictfp for now. It would require
+  // conversion of all FP operations in host function to constrained intrinsics.
+  if (CalledFunc->getAttributes().hasAttribute(
+          AttributeList::FunctionIndex, Attribute::StrictFP) &&
+      !Caller->getAttributes().hasAttribute(
+          AttributeList::FunctionIndex, Attribute::StrictFP)) {
+    return "incompatible strictfp attributes";
+  }
+
   // GC poses two hazards to inlining, which only occur when the callee has GC:
   //  1. If the caller has no GC, then the callee's GC must be propagated to the
   //     caller.
   //  2. If the caller has a differing GC, it is invalid to inline.
   if (CalledFunc->hasGC()) {
     if (!Caller->hasGC())
       Caller->setGC(CalledFunc->getGC());
     else if (CalledFunc->getGC() != Caller->getGC())

llvm/test/Transforms/CodeExtractor/PartialInlineAttributes.ll

 
 ; CHECK: define internal void @callee_writeonly.1.if.then(i32 %v, i32* %sub.out) { 
 ; CHECK: define internal void @callee_most.2.if.then(i32 %v, i32* %sub.out)  [[FN_ATTRS:#[0-9]+]]
 
 ; attributes to preserve
 attributes #0 = {
   inlinehint minsize noduplicate noimplicitfloat norecurse noredzone nounwind
   nonlazybind optsize safestack sanitize_address sanitize_hwaddress sanitize_memory
-  sanitize_thread ssp sspreq sspstrong strictfp uwtable "foo"="bar"
+  sanitize_thread ssp sspreq sspstrong uwtable "foo"="bar"
   "patchable-function"="prologue-short-redirect" "probe-stack"="_foo_guard" "stack-probe-size"="4096" }
 
-; CHECK: attributes [[FN_ATTRS]] = { inlinehint minsize noduplicate noimplicitfloat norecurse noredzone nounwind nonlazybind optsize safestack sanitize_address sanitize_hwaddress sanitize_memory sanitize_thread ssp sspreq sspstrong strictfp uwtable "foo"="bar" "patchable-function"="prologue-short-redirect" "probe-stack"="_foo_guard" "stack-probe-size"="4096" }
+; CHECK: attributes [[FN_ATTRS]] = { inlinehint minsize noduplicate noimplicitfloat norecurse noredzone nounwind nonlazybind optsize safestack sanitize_address sanitize_hwaddress sanitize_memory sanitize_thread ssp sspreq sspstrong uwtable "foo"="bar" "patchable-function"="prologue-short-redirect" "probe-stack"="_foo_guard" "stack-probe-size"="4096" }
 
 ; attributes to drop
 attributes #1 = {
   alignstack=16 convergent inaccessiblememonly inaccessiblemem_or_argmemonly naked
   noreturn readonly argmemonly returns_twice speculatable "thunk"
 }

llvm/test/Transforms/Inline/inline_strictfp.ll

+; RUN: opt -inline %s -S | FileCheck %s
+
+
+; Ordinary function is inlined into strictfp function.
+
+define float @func_01(float %a) {
+entry:
+  %add = fadd float %a, %a
+  ret float %add
+}
+
+define float @func_02(float %a) strictfp {
+entry:
+  %0 = tail call float @func_01(float %a)
+  %add = call float @llvm.experimental.constrained.fadd.f32(float %0, float 2.000000e+00, metadata !"round.dynamic", metadata !"fpexcept.strict")
+  ret float %add
+; CHECK_LABEL: @func_02
+; CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+; CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float 2.000000e+00, metadata !"round.dynamic", metadata !"fpexcept.strict")
+}
+
+
+; strictfp function is inlined into another strictfp function.
+
+define float @func_03(float %a) strictfp {
+entry:
+  %add = call float @llvm.experimental.constrained.fadd.f32(float %a, float %a, metadata !"round.downward", metadata !"fpexcept.maytrap")
+  ret float %add
+}
+
+define float @func_04(float %a) strictfp {
+entry:
+  %0 = tail call float @func_03(float %a)
+  %add = call float @llvm.experimental.constrained.fadd.f32(float %0, float 2.000000e+00, metadata !"round.dynamic", metadata !"fpexcept.strict")
+  ret float %add
+; CHECK_LABEL: @func_04
+; CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.downward", metadata !"fpexcept.maytrap")
+; CHECK: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float 2.000000e+00, metadata !"round.dynamic", metadata !"fpexcept.strict")
+}
+
+
+; strictfp function is NOT inlined into ordinary function.
+
+define float @func_05(float %a) strictfp {
+entry:
+  %add = call float @llvm.experimental.constrained.fadd.f32(float %a, float %a, metadata !"round.downward", metadata !"fpexcept.maytrap")
+  ret float %add
+}
+
+define float @func_06(float %a) {
+entry:
+  %0 = tail call float @func_05(float %a)
+  %add = fadd float %0, 2.000000e+00
+  ret float %add
+; CHECK_LABEL: @func_06
+; CHECK: call float @func_05(float %a)
+; CHECK: fadd float %0, 2.000000e+00
+}
+
+
+declare float @llvm.experimental.constrained.fadd.f32(float, float, metadata, metadata) #1
+
+