From ea376dae2597d580e335df43370833003818c129 Mon Sep 17 00:00:00 2001 From: Matt Arsenault Date: Fri, 28 Apr 2017 20:25:27 +0000 Subject: [PATCH] Add speculatable function attribute This attribute tells the optimizer that the function may be speculated. Patch by Tom Stellard git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@301680 91177308-0d34-0410-b5e6-96231b3b80d8 --- docs/LangRef.rst | 11 +++++++ include/llvm/Bitcode/LLVMBitCodes.h | 3 +- include/llvm/IR/Attributes.td | 3 ++ include/llvm/IR/Function.h | 8 +++++ include/llvm/IR/Intrinsics.td | 3 ++ lib/AsmParser/LLLexer.cpp | 1 + lib/AsmParser/LLParser.cpp | 1 + lib/AsmParser/LLToken.h | 1 + lib/Bitcode/Reader/BitcodeReader.cpp | 3 ++ lib/Bitcode/Writer/BitcodeWriter.cpp | 2 ++ lib/IR/Attributes.cpp | 2 ++ lib/IR/Verifier.cpp | 32 ++++++++++++------- test/Bitcode/attributes.ll | 10 ++++-- test/Bitcode/compatibility.ll | 10 ++++-- .../Verifier/speculatable-callsite-invalid.ll | 24 ++++++++++++++ test/Verifier/speculatable-callsite.ll | 20 ++++++++++++ utils/TableGen/CodeGenIntrinsics.h | 3 ++ utils/TableGen/CodeGenTarget.cpp | 3 ++ utils/TableGen/IntrinsicEmitter.cpp | 11 ++++++- 19 files changed, 133 insertions(+), 18 deletions(-) create mode 100644 test/Verifier/speculatable-callsite-invalid.ll create mode 100644 test/Verifier/speculatable-callsite.ll diff --git a/docs/LangRef.rst b/docs/LangRef.rst index b0a31589cc4..bf4973ca9ae 100644 --- a/docs/LangRef.rst +++ b/docs/LangRef.rst @@ -1535,6 +1535,17 @@ example: ``sanitize_thread`` This attribute indicates that ThreadSanitizer checks (dynamic thread safety analysis) are enabled for this function. +``speculatable`` + This function attribute indicates that the function does not have any + effects besides calculating its result and does not have undefined behavior. + Note that ``speculatable`` is not enough to conclude that along any + particular exection path the number of calls to this function will not be + externally observable. This attribute is only valid on functions + and declarations, not on individual call sites. If a function is + incorrectly marked as speculatable and really does exhibit + undefined behavior, the undefined behavior may be observed even + if the call site is dead code. + ``ssp`` This attribute indicates that the function should emit a stack smashing protector. It is in the form of a "canary" --- a random value diff --git a/include/llvm/Bitcode/LLVMBitCodes.h b/include/llvm/Bitcode/LLVMBitCodes.h index 03eac80bc1e..8ee1e4b583b 100644 --- a/include/llvm/Bitcode/LLVMBitCodes.h +++ b/include/llvm/Bitcode/LLVMBitCodes.h @@ -545,7 +545,8 @@ enum AttributeKindCodes { ATTR_KIND_INACCESSIBLEMEM_ONLY = 49, ATTR_KIND_INACCESSIBLEMEM_OR_ARGMEMONLY = 50, ATTR_KIND_ALLOC_SIZE = 51, - ATTR_KIND_WRITEONLY = 52 + ATTR_KIND_WRITEONLY = 52, + ATTR_KIND_SPECULATABLE = 53 }; enum ComdatSelectionKindCodes { diff --git a/include/llvm/IR/Attributes.td b/include/llvm/IR/Attributes.td index 7b63638a3f6..75867a6e583 100644 --- a/include/llvm/IR/Attributes.td +++ b/include/llvm/IR/Attributes.td @@ -137,6 +137,9 @@ def SExt : EnumAttr<"signext">; /// +1 bias 0 means unaligned (different from alignstack=(1)). def StackAlignment : EnumAttr<"alignstack">; +/// Function can be speculated. +def Speculatable : EnumAttr<"speculatable">; + /// Stack protection. def StackProtect : EnumAttr<"ssp">; diff --git a/include/llvm/IR/Function.h b/include/llvm/IR/Function.h index f48d436e691..4ba34d94b09 100644 --- a/include/llvm/IR/Function.h +++ b/include/llvm/IR/Function.h @@ -418,6 +418,14 @@ public: removeFnAttr(Attribute::Convergent); } + /// @brief Determine if the call has sideeffects. + bool isSpeculatable() const { + return hasFnAttribute(Attribute::Speculatable); + } + void setSpeculatable() { + addFnAttr(Attribute::Speculatable); + } + /// Determine if the function is known not to recurse, directly or /// indirectly. bool doesNotRecurse() const { diff --git a/include/llvm/IR/Intrinsics.td b/include/llvm/IR/Intrinsics.td index 309b2148922..cf60f79ba40 100644 --- a/include/llvm/IR/Intrinsics.td +++ b/include/llvm/IR/Intrinsics.td @@ -98,6 +98,9 @@ def IntrNoDuplicate : IntrinsicProperty; // Parallels the convergent attribute on LLVM IR functions. def IntrConvergent : IntrinsicProperty; +// This property indicates that the intrinsic is safe to speculate. +def IntrSpeculatable : IntrinsicProperty; + //===----------------------------------------------------------------------===// // Types used by intrinsics. //===----------------------------------------------------------------------===// diff --git a/lib/AsmParser/LLLexer.cpp b/lib/AsmParser/LLLexer.cpp index 49a8ce4bed0..85152ff60f9 100644 --- a/lib/AsmParser/LLLexer.cpp +++ b/lib/AsmParser/LLLexer.cpp @@ -648,6 +648,7 @@ lltok::Kind LLLexer::LexIdentifier() { KEYWORD(returned); KEYWORD(returns_twice); KEYWORD(signext); + KEYWORD(speculatable); KEYWORD(sret); KEYWORD(ssp); KEYWORD(sspreq); diff --git a/lib/AsmParser/LLParser.cpp b/lib/AsmParser/LLParser.cpp index a6a9b32d093..f422787460e 100644 --- a/lib/AsmParser/LLParser.cpp +++ b/lib/AsmParser/LLParser.cpp @@ -1095,6 +1095,7 @@ bool LLParser::ParseFnAttributeValuePairs(AttrBuilder &B, case lltok::kw_readonly: B.addAttribute(Attribute::ReadOnly); break; case lltok::kw_returns_twice: B.addAttribute(Attribute::ReturnsTwice); break; + case lltok::kw_speculatable: B.addAttribute(Attribute::Speculatable); break; case lltok::kw_ssp: B.addAttribute(Attribute::StackProtect); break; case lltok::kw_sspreq: B.addAttribute(Attribute::StackProtectReq); break; case lltok::kw_sspstrong: diff --git a/lib/AsmParser/LLToken.h b/lib/AsmParser/LLToken.h index 33f8e63daa0..9f9f4f5b429 100644 --- a/lib/AsmParser/LLToken.h +++ b/lib/AsmParser/LLToken.h @@ -198,6 +198,7 @@ enum Kind { kw_returned, kw_returns_twice, kw_signext, + kw_speculatable, kw_ssp, kw_sspreq, kw_sspstrong, diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index 6b29f6d66a1..782738f5359 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -1119,6 +1119,7 @@ static uint64_t getRawAttributeMask(Attribute::AttrKind Val) { case Attribute::SwiftSelf: return 1ULL << 51; case Attribute::SwiftError: return 1ULL << 52; case Attribute::WriteOnly: return 1ULL << 53; + case Attribute::Speculatable: return 1ULL << 54; case Attribute::Dereferenceable: llvm_unreachable("dereferenceable attribute not supported in raw format"); break; @@ -1315,6 +1316,8 @@ static Attribute::AttrKind getAttrFromCode(uint64_t Code) { return Attribute::ReturnsTwice; case bitc::ATTR_KIND_S_EXT: return Attribute::SExt; + case bitc::ATTR_KIND_SPECULATABLE: + return Attribute::Speculatable; case bitc::ATTR_KIND_STACK_ALIGNMENT: return Attribute::StackAlignment; case bitc::ATTR_KIND_STACK_PROTECT: diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp index eb8b43219da..54839b48723 100644 --- a/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -688,6 +688,8 @@ static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) { return bitc::ATTR_KIND_RETURNS_TWICE; case Attribute::SExt: return bitc::ATTR_KIND_S_EXT; + case Attribute::Speculatable: + return bitc::ATTR_KIND_SPECULATABLE; case Attribute::StackAlignment: return bitc::ATTR_KIND_STACK_ALIGNMENT; case Attribute::StackProtect: diff --git a/lib/IR/Attributes.cpp b/lib/IR/Attributes.cpp index 6c4d0f0c6a7..f0db6b4e71a 100644 --- a/lib/IR/Attributes.cpp +++ b/lib/IR/Attributes.cpp @@ -315,6 +315,8 @@ std::string Attribute::getAsString(bool InAttrGrp) const { return "returns_twice"; if (hasAttribute(Attribute::SExt)) return "signext"; + if (hasAttribute(Attribute::Speculatable)) + return "speculatable"; if (hasAttribute(Attribute::StackProtect)) return "ssp"; if (hasAttribute(Attribute::StackProtectReq)) diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp index 5f60774c302..b65f3ef386a 100644 --- a/lib/IR/Verifier.cpp +++ b/lib/IR/Verifier.cpp @@ -1203,9 +1203,9 @@ void Verifier::visitComdat(const Comdat &C) { void Verifier::visitModuleIdents(const Module &M) { const NamedMDNode *Idents = M.getNamedMetadata("llvm.ident"); - if (!Idents) + if (!Idents) return; - + // llvm.ident takes a list of metadata entry. Each entry has only one string. // Scan each llvm.ident entry and make sure that this requirement is met. for (const MDNode *N : Idents->operands()) { @@ -1215,7 +1215,7 @@ void Verifier::visitModuleIdents(const Module &M) { ("invalid value for llvm.ident metadata entry operand" "(the operand should be a string)"), N->getOperand(0)); - } + } } void Verifier::visitModuleFlags(const Module &M) { @@ -1352,6 +1352,7 @@ static bool isFuncOnlyAttr(Attribute::AttrKind Kind) { case Attribute::InaccessibleMemOnly: case Attribute::InaccessibleMemOrArgMemOnly: case Attribute::AllocSize: + case Attribute::Speculatable: return true; default: break; @@ -1837,7 +1838,7 @@ void Verifier::verifyStatepoint(ImmutableCallSite CS) { Assert(ExpectedNumArgs <= (int)CS.arg_size(), "gc.statepoint too few arguments according to length fields", &CI); - // Check that the only uses of this gc.statepoint are gc.result or + // Check that the only uses of this gc.statepoint are gc.result or // gc.relocate calls which are tied to this statepoint and thus part // of the same statepoint sequence for (const User *U : CI.users()) { @@ -2610,6 +2611,15 @@ void Verifier::verifyCallSite(CallSite CS) { Assert(verifyAttributeCount(Attrs, CS.arg_size()), "Attribute after last parameter!", I); + if (Attrs.hasAttribute(AttributeList::FunctionIndex, Attribute::Speculatable)) { + // Don't allow speculatable on call sites, unless the underlying function + // declaration is also speculatable. + Function *Callee + = dyn_cast(CS.getCalledValue()->stripPointerCasts()); + Assert(Callee && Callee->isSpeculatable(), + "speculatable attribute may not apply to call sites", I); + } + // Verify call attributes. verifyFunctionAttrs(FTy, Attrs, I); @@ -3908,7 +3918,7 @@ void Verifier::visitIntrinsicCallSite(Intrinsic::ID ID, CallSite CS) { // If the intrinsic takes MDNode arguments, verify that they are either global // or are local to *this* function. - for (Value *V : CS.args()) + for (Value *V : CS.args()) if (auto *MD = dyn_cast(V)) visitMetadataAsValue(*MD, CS.getCaller()); @@ -3981,7 +3991,7 @@ void Verifier::visitIntrinsicCallSite(Intrinsic::ID ID, CallSite CS) { auto IsValidAlignment = [&](uint64_t Alignment) { return isPowerOf2_64(Alignment) && ElementSizeVal.ule(Alignment); }; - + uint64_t DstAlignment = CS.getParamAlignment(1), SrcAlignment = CS.getParamAlignment(2); @@ -4220,7 +4230,7 @@ void Verifier::visitIntrinsicCallSite(Intrinsic::ID ID, CallSite CS) { } case Intrinsic::masked_load: { Assert(CS.getType()->isVectorTy(), "masked_load: must return a vector", CS); - + Value *Ptr = CS.getArgOperand(0); //Value *Alignment = CS.getArgOperand(1); Value *Mask = CS.getArgOperand(2); @@ -4230,12 +4240,12 @@ void Verifier::visitIntrinsicCallSite(Intrinsic::ID ID, CallSite CS) { // DataTy is the overloaded type Type *DataTy = cast(Ptr->getType())->getElementType(); - Assert(DataTy == CS.getType(), + Assert(DataTy == CS.getType(), "masked_load: return must match pointer type", CS); Assert(PassThru->getType() == DataTy, "masked_load: pass through and data type must match", CS); Assert(Mask->getType()->getVectorNumElements() == - DataTy->getVectorNumElements(), + DataTy->getVectorNumElements(), "masked_load: vector mask must be same length as data", CS); break; } @@ -4249,10 +4259,10 @@ void Verifier::visitIntrinsicCallSite(Intrinsic::ID ID, CallSite CS) { // DataTy is the overloaded type Type *DataTy = cast(Ptr->getType())->getElementType(); - Assert(DataTy == Val->getType(), + Assert(DataTy == Val->getType(), "masked_store: storee must match pointer type", CS); Assert(Mask->getType()->getVectorNumElements() == - DataTy->getVectorNumElements(), + DataTy->getVectorNumElements(), "masked_store: vector mask must be same length as data", CS); break; } diff --git a/test/Bitcode/attributes.ll b/test/Bitcode/attributes.ll index 9fdf54b7b30..18aa12c7af9 100644 --- a/test/Bitcode/attributes.ll +++ b/test/Bitcode/attributes.ll @@ -204,7 +204,7 @@ define void @f34() ; CHECK: define void @f34() { call void @nobuiltin() nobuiltin -; CHECK: call void @nobuiltin() #33 +; CHECK: call void @nobuiltin() #34 ret void; } @@ -334,6 +334,11 @@ define void @f56() writeonly ret void } +; CHECK: define void @f57() #33 +define void @f57() speculatable { + ret void +} + ; CHECK: attributes #0 = { noreturn } ; CHECK: attributes #1 = { nounwind } ; CHECK: attributes #2 = { readnone } @@ -367,4 +372,5 @@ define void @f56() writeonly ; CHECK: attributes #30 = { allocsize(0) } ; CHECK: attributes #31 = { allocsize(0,1) } ; CHECK: attributes #32 = { writeonly } -; CHECK: attributes #33 = { nobuiltin } +; CHECK: attributes #33 = { speculatable } +; CHECK: attributes #34 = { nobuiltin } diff --git a/test/Bitcode/compatibility.ll b/test/Bitcode/compatibility.ll index b1f52bbe059..bc77e05d409 100644 --- a/test/Bitcode/compatibility.ll +++ b/test/Bitcode/compatibility.ll @@ -3,7 +3,7 @@ ; Please update this file when making any IR changes. Information on the ; release process for this file is available here: ; -; http://llvm.org/docs/DeveloperPolicy.html#ir-backwards-compatibility +; http://llvm.org/docs/DeveloperPolicy.html#ir-backwards-compatibility ; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s ; RUN-PR24755: verify-uselistorder < %s @@ -1246,7 +1246,7 @@ exit: ; CHECK: select <2 x i1> , <2 x i8> , <2 x i8> call void @f.nobuiltin() builtin - ; CHECK: call void @f.nobuiltin() #41 + ; CHECK: call void @f.nobuiltin() #42 call fastcc noalias i32* @f.noalias() noinline ; CHECK: call fastcc noalias i32* @f.noalias() #12 @@ -1613,6 +1613,9 @@ normal: declare void @f.writeonly() writeonly ; CHECK: declare void @f.writeonly() #40 +declare void @f.speculatable() speculatable +; CHECK: declare void @f.speculatable() #41 + ;; Constant Expressions define i8** @constexpr() { @@ -1661,7 +1664,8 @@ define i8** @constexpr() { ; CHECK: attributes #38 = { nounwind readonly } ; CHECK: attributes #39 = { inaccessiblemem_or_argmemonly nounwind } ; CHECK: attributes #40 = { writeonly } -; CHECK: attributes #41 = { builtin } +; CHECK: attributes #41 = { speculatable } +; CHECK: attributes #42 = { builtin } ;; Metadata diff --git a/test/Verifier/speculatable-callsite-invalid.ll b/test/Verifier/speculatable-callsite-invalid.ll new file mode 100644 index 00000000000..f9a1adfe947 --- /dev/null +++ b/test/Verifier/speculatable-callsite-invalid.ll @@ -0,0 +1,24 @@ +; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s + +; Make sure that speculatable is not allowed on a call site if the +; declaration is not also speculatable. + +declare i32 @not_speculatable() + +; CHECK: speculatable attribute may not apply to call sites +; CHECK-NEXT: %ret = call i32 @not_speculatable() #0 +define i32 @call_not_speculatable() { + %ret = call i32 @not_speculatable() #0 + ret i32 %ret +} + +@gv = internal unnamed_addr constant i32 0 + +; CHECK: speculatable attribute may not apply to call sites +; CHECK-NEXT: %ret = call float bitcast (i32* @gv to float ()*)() #0 +define float @call_bitcast_speculatable() { + %ret = call float bitcast (i32* @gv to float()*)() #0 + ret float %ret +} + +attributes #0 = { speculatable } diff --git a/test/Verifier/speculatable-callsite.ll b/test/Verifier/speculatable-callsite.ll new file mode 100644 index 00000000000..fafed831cf9 --- /dev/null +++ b/test/Verifier/speculatable-callsite.ll @@ -0,0 +1,20 @@ +; RUN: llvm-as %s -o /dev/null + +; Make sure speculatable is accepted on a call site if the declaration +; is also speculatable. + +declare i32 @speculatable() #0 + +; Make sure this the attribute is accepted on the call site if the +; declaration matches. +define i32 @call_speculatable() { + %ret = call i32 @speculatable() #0 + ret i32 %ret +} + +define float @call_bitcast_speculatable() { + %ret = call float bitcast (i32()* @speculatable to float()*)() #0 + ret float %ret +} + +attributes #0 = { speculatable } diff --git a/utils/TableGen/CodeGenIntrinsics.h b/utils/TableGen/CodeGenIntrinsics.h index 6df0e6a62ca..d126f273a1a 100644 --- a/utils/TableGen/CodeGenIntrinsics.h +++ b/utils/TableGen/CodeGenIntrinsics.h @@ -123,6 +123,9 @@ struct CodeGenIntrinsic { /// True if the intrinsic is marked as convergent. bool isConvergent; + // True if the intrinsic is marked as speculatable. + bool isSpeculatable; + enum ArgAttribute { NoCapture, Returned, ReadOnly, WriteOnly, ReadNone }; std::vector> ArgumentAttributes; diff --git a/utils/TableGen/CodeGenTarget.cpp b/utils/TableGen/CodeGenTarget.cpp index 03c58ac09c2..aad9005de20 100644 --- a/utils/TableGen/CodeGenTarget.cpp +++ b/utils/TableGen/CodeGenTarget.cpp @@ -515,6 +515,7 @@ CodeGenIntrinsic::CodeGenIntrinsic(Record *R) { isNoReturn = false; isNoDuplicate = false; isConvergent = false; + isSpeculatable = false; if (DefName.size() <= 4 || std::string(DefName.begin(), DefName.begin() + 4) != "int_") @@ -653,6 +654,8 @@ CodeGenIntrinsic::CodeGenIntrinsic(Record *R) { isConvergent = true; else if (Property->getName() == "IntrNoReturn") isNoReturn = true; + else if (Property->getName() == "IntrSpeculatable") + isSpeculatable = true; else if (Property->isSubClassOf("NoCapture")) { unsigned ArgNo = Property->getValueAsInt("ArgNo"); ArgumentAttributes.push_back(std::make_pair(ArgNo, NoCapture)); diff --git a/utils/TableGen/IntrinsicEmitter.cpp b/utils/TableGen/IntrinsicEmitter.cpp index e979b94e46d..72f2dce401e 100644 --- a/utils/TableGen/IntrinsicEmitter.cpp +++ b/utils/TableGen/IntrinsicEmitter.cpp @@ -476,6 +476,9 @@ struct AttributeComparator { if (L->isConvergent != R->isConvergent) return R->isConvergent; + if (L->isSpeculatable != R->isSpeculatable) + return R->isSpeculatable; + // Try to order by readonly/readnone attribute. CodeGenIntrinsic::ModRefBehavior LK = L->ModRef; CodeGenIntrinsic::ModRefBehavior RK = R->ModRef; @@ -600,7 +603,7 @@ void IntrinsicEmitter::EmitAttributes(const CodeGenIntrinsicTable &Ints, if (!intrinsic.canThrow || intrinsic.ModRef != CodeGenIntrinsic::ReadWriteMem || intrinsic.isNoReturn || intrinsic.isNoDuplicate || - intrinsic.isConvergent) { + intrinsic.isConvergent || intrinsic.isSpeculatable) { OS << " const Attribute::AttrKind Atts[] = {"; bool addComma = false; if (!intrinsic.canThrow) { @@ -625,6 +628,12 @@ void IntrinsicEmitter::EmitAttributes(const CodeGenIntrinsicTable &Ints, OS << "Attribute::Convergent"; addComma = true; } + if (intrinsic.isSpeculatable) { + if (addComma) + OS << ","; + OS << "Attribute::Speculatable"; + addComma = true; + } switch (intrinsic.ModRef) { case CodeGenIntrinsic::NoMem: -- 2.40.0