[FPEnv] The inliner shouldn't mix strictfp and non-strictfp functions.

[kpn]

Summary

Constrained FP intrinsic rules prohibit the mixing of constrained FP and non-constrained. When inlining functions this rule should not be violated.

This patch disables the "inline" and "alwaysinline" keywords when function attributes don't match.

[craig.topper]

llvm/lib/Analysis/InlineCost.cpp

2039

Should this be a FIXME or TODO so it stands out?

[kpn]

llvm/lib/Analysis/InlineCost.cpp

2039

Sure, that makes sense.

[kpn]

Address review comment: rework comment to stand out.

[kpn]

It looks like D69798 may be more complete patch. Let me take this off everyone's queue while that one is in flight.

[kpn]

Not needed anymore after D69798 landed.

llvm/lib/Analysis/InlineCost.cpp

InlineCost llvm::getInlineCost(
   for (unsigned I = 0, E = Call.arg_size(); I != E; ++I)
     if (Call.isByValArgument(I)) {
       PointerType *PTy = cast<PointerType>(Call.getArgOperand(I)->getType());
       if (PTy->getAddressSpace() != AllocaAS)
         return llvm::InlineCost::getNever("byval arguments without alloca"
                                           " address space");
     }
 
+  // Currently, we can't mix strictfp and non-strictfp functions. This
+  // overrides the AlwaysInline attribute!
+  // This should be improved at some point.
+  if (Call.getCaller()->hasFnAttribute(Attribute::StrictFP) !=
+         Callee->hasFnAttribute(Attribute::StrictFP))
+    return llvm::InlineCost::getNever("conflicting strictfp attributes");
+
   // Calls to functions with always-inline attributes should be inlined
   // whenever possible.
   if (Call.hasFnAttr(Attribute::AlwaysInline)) {
     auto IsViable = isInlineViable(*Callee);
     if (IsViable)
       return llvm::InlineCost::getAlways("always inline attribute");
     return llvm::InlineCost::getNever(IsViable.message);
   }

llvm/test/Transforms/Inline/inline-strictfp.ll

+; RUN: opt < %s -inline -S | FileCheck %s
+
+; Test that strictfp and non-strictfp functions are not inlined.
+; Also that that strictfp overrides alwaysinline.
+
+; Function Attrs: alwaysinline nounwind readnone strictfp uwtable
+define double @alwaysInlineStrictFPFunction(double %a) #0 {
+entry:
+  %mul = call double @llvm.experimental.constrained.fmul.f64(
+                                               double %a, double %a,
+                                               metadata !"round.dynamic",
+                                               metadata !"fpexcept.strict") #2
+  ret double %mul
+}
+
+; Function Attrs: alwaysinline nounwind readnone uwtable
+define double @alwaysInlineVanillaFPFunction(double %a) #1 {
+entry:
+  %mul = fmul double %a, %a
+  ret double %mul
+}
+
+; Function Attrs: nounwind readnone strictfp uwtable
+define double @simpleFunction(double %a) #2 {
+entry:
+  %add = call double @llvm.experimental.constrained.fadd.f64(
+                                               double %a, double %a,
+                                               metadata !"round.dynamic",
+                                               metadata !"fpexcept.strict") #2
+  ret double %add
+}
+
+; Function Attrs: nounwind readnone uwtable
+define double @VanillaFPFunction(double %a) #3 {
+entry:
+  %0 = tail call double @alwaysInlineStrictFPFunction(double %a)
+  %1 = tail call double @simpleFunction(double %a)
+  %add = fadd double %0, %1
+  ret double %add
+}
+
+; CHECK-LABEL: @VanillaFPFunction
+; CHECK: call double @alwaysInlineStrictFPFunction(double %a)
+; CHECK: call double @simpleFunction(double %a)
+; CHECK: ret
+
+; Function Attrs: nounwind readnone strictfp uwtable
+define double @bar(double %a) #2 {
+entry:
+  %0 = tail call double @VanillaFPFunction(double 5.0)
+  %1 = tail call double @simpleFunction(double 6.0)
+  %2 = tail call double @alwaysInlineStrictFPFunction(double %1)
+  %3 = tail call double @alwaysInlineVanillaFPFunction(double %2)
+  %add = fadd double %0, %3
+  ret double %add
+}
+
+; CHECK-LABEL: @bar
+; CHECK: call double @VanillaFPFunction(double 5
+; CHECK-NOT: call double @simpleFunction(double 6
+; CHECK-NOT: call double @alwaysInlineStrictFPFunction(double
+; CHECK: call double @alwaysInlineVanillaFPFunction(double
+; CHECK: ret
+
+attributes #0 = { alwaysinline nounwind readnone strictfp uwtable }
+attributes #1 = { alwaysinline nounwind readnone uwtable }
+attributes #2 = { nounwind readnone strictfp uwtable }
+attributes #3 = { nounwind readnone uwtable }
+
+declare double @llvm.experimental.constrained.fadd.f64(double, double, metadata, metadata);
+declare double @llvm.experimental.constrained.fmul.f64(double, double, metadata, metadata);