diff --git a/include/llvm/IR/AutoUpgrade.h b/include/llvm/IR/AutoUpgrade.h index 9ddec28f0d6..f331fc3c413 100644 --- a/include/llvm/IR/AutoUpgrade.h +++ b/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. diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index 2f9331e51f3..a428416244f 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/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(); diff --git a/lib/IR/AutoUpgrade.cpp b/lib/IR/AutoUpgrade.cpp index 4beb9b642ef..3e0c3b91a04 100644 --- a/lib/IR/AutoUpgrade.cpp +++ b/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() {} + + void visitCallBase(CallBase &Call) { + if (!Call.isStrictFP()) + return; + if (dyn_cast(&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(MD); if (!T) diff --git a/test/Bitcode/compatibility-5.0.ll b/test/Bitcode/compatibility-5.0.ll index dffe593ac69..a38bf2d9675 100644 --- a/test/Bitcode/compatibility-5.0.ll +++ b/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 diff --git a/test/Bitcode/compatibility-6.0.ll b/test/Bitcode/compatibility-6.0.ll index 2b160558dd7..5af30ddc863 100644 --- a/test/Bitcode/compatibility-6.0.ll +++ b/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 diff --git a/unittests/Bitcode/BitReaderTest.cpp b/unittests/Bitcode/BitReaderTest.cpp index e803677a6ee..c4f9b672ac9 100644 --- a/unittests/Bitcode/BitReaderTest.cpp +++ b/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 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(&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 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(&I)) { + EXPECT_TRUE(Call->isStrictFP()); + EXPECT_FALSE(Call->isNoBuiltin()); + } + } + } + + EXPECT_FALSE(verifyModule(*M, &dbgs())); +} + TEST(BitReaderTest, MaterializeFunctionsForBlockAddr) { // PR11677 SmallString<1024> Mem;