[strictfp] Replace dangling strictfp attrs with nobuiltin

In preparation for a patch that will enforce new rules for the usage of
the strictfp attribute, this patch introduces auto-upgrade behavior that
will replace the strictfp attribute on callsites with nobuiltin if the
enclosing function declaration doesn't also have the strictfp attribute.

This auto-upgrade isn't being performed on .ll files because that would
prevent us from writing a test for the forthcoming verifier behavior.

Differential Revision: https://reviews.llvm.org/D70096

include/llvm/IR/AutoUpgrade.h

@@ -61,6 +61,9 @@ namespace llvm {
 
   void UpgradeSectionAttributes(Module &M);
 
+  /// Correct any IR that is relying on old function attribute behavior.
+  void UpgradeFunctionAttributes(Function &F);
+
   /// If the given TBAA tag uses the scalar TBAA format, create a new node
   /// corresponding to the upgrade to the struct-path aware TBAA format.
   /// Otherwise return the \p TBAANode itself.

lib/Bitcode/Reader/BitcodeReader.cpp

@@ -3002,6 +3002,7 @@ Error BitcodeReader::globalCleanup() {
     return error("Malformed global initializer set");
 
   // Look for intrinsic functions which need to be upgraded at some point
+  // and functions that need to have their function attributes upgraded.
   for (Function &F : *TheModule) {
     MDLoader->upgradeDebugIntrinsics(F);
     Function *NewFn;
@@ -3012,6 +3013,8 @@ Error BitcodeReader::globalCleanup() {
       // loaded in the same LLVMContext (LTO scenario). In this case we should
       // remangle intrinsics names as well.
       RemangledIntrinsics[&F] = Remangled.getValue();
+    // Look for functions that rely on old function attribute behavior.
+    UpgradeFunctionAttributes(F);
   }
 
   // Look for global variables which need to be renamed.
@@ -5376,6 +5379,9 @@ Error BitcodeReader::materialize(GlobalValue *GV) {
     }
   }
 
+  // Look for functions that rely on old function attribute behavior.
+  UpgradeFunctionAttributes(*F);
+
   // Bring in any functions that this function forward-referenced via
   // blockaddresses.
   return materializeForwardReferencedFunctions();

lib/IR/AutoUpgrade.cpp

@@ -21,6 +21,7 @@
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Instruction.h"
+#include "llvm/IR/InstVisitor.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/IntrinsicsAArch64.h"
 #include "llvm/IR/IntrinsicsARM.h"
@@ -4166,6 +4167,42 @@ void llvm::UpgradeSectionAttributes(Module &M) {
   }
 }
 
+
+// Prior to LLVM 10.0, the strictfp attribute could be used on individual
+// callsites within a function that did not also have the strictfp attribute.
+// Since 10.0, if strict FP semantics are needed within a function, the
+// function must have the strictfp attribute and all calls within the function
+// must also have the strictfp attribute. This latter restriction is
+// necessary to prevent unwanted libcall simplification when a function is
+// being cloned (such as for inlining).
+//
+// The "dangling" strictfp attribute usage was only used to prevent constant
+// folding and other libcall simplification. The nobuiltin attribute on the
+// callsite has the same effect.
+struct StrictFPUpgradeVisitor : public InstVisitor<StrictFPUpgradeVisitor> {
+  StrictFPUpgradeVisitor() {}
+
+  void visitCallBase(CallBase &Call) {
+    if (!Call.isStrictFP())
+      return;
+    if (dyn_cast<ConstrainedFPIntrinsic>(&Call))
+      return;
+    // If we get here, the caller doesn't have the strictfp attribute
+    // but this callsite does. Replace the strictfp attribute with nobuiltin.
+    Call.removeAttribute(AttributeList::FunctionIndex, Attribute::StrictFP);
+    Call.addAttribute(AttributeList::FunctionIndex, Attribute::NoBuiltin);
+  }
+};
+
+void llvm::UpgradeFunctionAttributes(Function &F) {
+  // If a function definition doesn't have the strictfp attribute,
+  // convert any callsite strictfp attributes to nobuiltin.
+  if (!F.isDeclaration() && !F.hasFnAttribute(Attribute::StrictFP)) {
+    StrictFPUpgradeVisitor SFPV;
+    SFPV.visit(F);
+  }
+}
+
 static bool isOldLoopArgument(Metadata *MD) {
   auto *T = dyn_cast_or_null<MDTuple>(MD);
   if (!T)

test/Bitcode/compatibility-5.0.ll

@@ -1250,8 +1250,10 @@ exit:
   call void @f.nobuiltin() builtin
   ; CHECK: call void @f.nobuiltin() #43
 
+  ; When used in a non-strictfp function the strictfp callsite attribute
+  ; should get translated to nobuiltin.
   call void @f.strictfp() strictfp
-  ; CHECK: call void @f.strictfp() #44
+  ; CHECK: call void @f.strictfp() #9
 
   call fastcc noalias i32* @f.noalias() noinline
   ; CHECK: call fastcc noalias i32* @f.noalias() #12
@@ -1672,7 +1674,6 @@ define i8** @constexpr() {
 ; CHECK: attributes #41 = { speculatable }
 ; CHECK: attributes #42 = { inaccessiblemem_or_argmemonly nounwind willreturn }
 ; CHECK: attributes #43 = { builtin }
-; CHECK: attributes #44 = { strictfp }
 
 ;; Metadata
 

test/Bitcode/compatibility-6.0.ll

@@ -1261,8 +1261,10 @@ exit:
   call void @f.nobuiltin() builtin
   ; CHECK: call void @f.nobuiltin() #43
 
+  ; When used in a non-strictfp function the strictfp callsite attribute
+  ; should get translated to nobuiltin.
   call void @f.strictfp() strictfp
-  ; CHECK: call void @f.strictfp() #44
+  ; CHECK: call void @f.strictfp() #9
 
   call fastcc noalias i32* @f.noalias() noinline
   ; CHECK: call fastcc noalias i32* @f.noalias() #12
@@ -1683,7 +1685,6 @@ define i8** @constexpr() {
 ; CHECK: attributes #41 = { speculatable }
 ; CHECK: attributes #42 = { inaccessiblemem_or_argmemonly nounwind willreturn }
 ; CHECK: attributes #43 = { builtin }
-; CHECK: attributes #44 = { strictfp }
 
 ;; Metadata
 

unittests/Bitcode/BitReaderTest.cpp

@@ -11,6 +11,7 @@
 #include "llvm/AsmParser/Parser.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
+#include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
@@ -121,6 +122,70 @@ TEST(BitReaderTest, MaterializeFunctionsOutOfOrder) {
   EXPECT_FALSE(verifyModule(*M, &dbgs()));
 }
 
+TEST(BitReaderTest, MaterializeFunctionsStrictFP) {
+  SmallString<1024> Mem;
+
+  LLVMContext Context;
+  std::unique_ptr<Module> M = getLazyModuleFromAssembly(
+      Context, Mem, "define double @foo(double %a) {\n"
+                    "  %result = call double @bar(double %a) strictfp\n"
+                    "  ret double %result\n"
+                    "}\n"
+                    "declare double @bar(double)\n");
+  Function *Foo = M->getFunction("foo");
+  ASSERT_FALSE(Foo->materialize());
+  EXPECT_FALSE(Foo->empty());
+
+  for (auto &BB : *Foo) {
+    auto It = BB.begin();
+    while (It != BB.end()) {
+      Instruction &I = *It;
+      ++It;
+
+      if (auto *Call = dyn_cast<CallBase>(&I)) {
+        EXPECT_FALSE(Call->isStrictFP());
+        EXPECT_TRUE(Call->isNoBuiltin());
+      }
+    }
+  }
+
+  EXPECT_FALSE(verifyModule(*M, &dbgs()));
+}
+
+TEST(BitReaderTest, MaterializeConstrainedFPStrictFP) {
+  SmallString<1024> Mem;
+
+  LLVMContext Context;
+  std::unique_ptr<Module> M = getLazyModuleFromAssembly(
+      Context, Mem,
+      "define double @foo(double %a) {\n"
+      "  %result = call double @llvm.experimental.constrained.sqrt.f64(double "
+      "%a, metadata !\"round.tonearest\", metadata !\"fpexcept.strict\") "
+      "strictfp\n"
+      "  ret double %result\n"
+      "}\n"
+      "declare double @llvm.experimental.constrained.sqrt.f64(double, "
+      "metadata, metadata)\n");
+  Function *Foo = M->getFunction("foo");
+  ASSERT_FALSE(Foo->materialize());
+  EXPECT_FALSE(Foo->empty());
+
+  for (auto &BB : *Foo) {
+    auto It = BB.begin();
+    while (It != BB.end()) {
+      Instruction &I = *It;
+      ++It;
+
+      if (auto *Call = dyn_cast<CallBase>(&I)) {
+        EXPECT_TRUE(Call->isStrictFP());
+        EXPECT_FALSE(Call->isNoBuiltin());
+      }
+    }
+  }
+
+  EXPECT_FALSE(verifyModule(*M, &dbgs()));
+}
+
 TEST(BitReaderTest, MaterializeFunctionsForBlockAddr) { // PR11677
   SmallString<1024> Mem;