[llvm] 07f3351 - [strictfp] Replace dangling strictfp attrs with nobuiltin
Kevin P. Neal via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 15 07:06:18 PDT 2020
Author: Kevin P. Neal
Date: 2020-06-15T10:05:35-04:00
New Revision: 07f335128410ca2dbb4b78050132ecd070e2e425
URL: https://github.com/llvm/llvm-project/commit/07f335128410ca2dbb4b78050132ecd070e2e425
DIFF: https://github.com/llvm/llvm-project/commit/07f335128410ca2dbb4b78050132ecd070e2e425.diff
LOG: [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
Added:
Modified:
llvm/include/llvm/IR/AutoUpgrade.h
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/IR/AutoUpgrade.cpp
llvm/test/Bitcode/compatibility-5.0.ll
llvm/test/Bitcode/compatibility-6.0.ll
llvm/unittests/Bitcode/BitReaderTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/AutoUpgrade.h b/llvm/include/llvm/IR/AutoUpgrade.h
index 9ddec28f0d64..f331fc3c413f 100644
--- a/llvm/include/llvm/IR/AutoUpgrade.h
+++ b/llvm/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/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 2f9331e51f3d..a428416244f9 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/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/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 4beb9b642ef3..3e0c3b91a04c 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/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)
diff --git a/llvm/test/Bitcode/compatibility-5.0.ll b/llvm/test/Bitcode/compatibility-5.0.ll
index dffe593ac698..a38bf2d9675c 100644
--- a/llvm/test/Bitcode/compatibility-5.0.ll
+++ b/llvm/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/llvm/test/Bitcode/compatibility-6.0.ll b/llvm/test/Bitcode/compatibility-6.0.ll
index 2b160558dd7a..5af30ddc8635 100644
--- a/llvm/test/Bitcode/compatibility-6.0.ll
+++ b/llvm/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/llvm/unittests/Bitcode/BitReaderTest.cpp b/llvm/unittests/Bitcode/BitReaderTest.cpp
index e803677a6eee..c4f9b672ac91 100644
--- a/llvm/unittests/Bitcode/BitReaderTest.cpp
+++ b/llvm/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;
More information about the llvm-commits
mailing list