<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Oct 14, 2015 at 3:08 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
> On 2015-Oct-13, at 22:32, Akira Hatanaka <<a href="mailto:ahatanak@gmail.com">ahatanak@gmail.com</a>> wrote:<br>
><br>
> ahatanak updated this revision to Diff 37320.<br>
> ahatanak added a comment.<br>
><br>
> Rebase and add rules to merge caller's and callee's attributes to Attributes.td.<br>
><br>
> Merge rules enable modifying the caller's attribute set if the caller and callee have incompatible attribute sets, rather than blocking inlining. For example, if the callee has attribute "noimplicitfloat", but the caller doesn't, the merge rule defined in Attributes.td attaches "noimplicitfloat" to the caller.<br>
><br>
><br>
> <a href="http://reviews.llvm.org/D7802" rel="noreferrer" target="_blank">http://reviews.llvm.org/D7802</a><br>
><br>
> Files:<br>
> include/llvm/IR/Attributes.h<br>
> include/llvm/IR/Attributes.td<br>
> include/llvm/IR/CMakeLists.txt<br>
> lib/Analysis/InlineCost.cpp<br>
> lib/IR/Attributes.cpp<br>
> lib/IR/AttributesCompatFunc.td<br>
> lib/IR/CMakeLists.txt<br>
> lib/IR/Makefile<br>
> lib/Transforms/IPO/Inliner.cpp<br>
> test/Analysis/BasicAA/intrinsics.ll<br>
> test/Analysis/TypeBasedAliasAnalysis/intrinsics.ll<br>
> test/Bitcode/compatibility-3.6.ll<br>
> test/Bitcode/compatibility-3.7.ll<br>
> test/Bitcode/compatibility.ll<br>
> test/Transforms/Inline/attributes.ll<br>
> test/Transforms/Inline/inline_invoke.ll<br>
> test/Transforms/MemCpyOpt/memcpy.ll<br>
> test/Transforms/ObjCARC/nested.ll<br>
> utils/TableGen/Attribute.cpp<br>
> utils/TableGen/CMakeLists.txt<br>
> utils/TableGen/TableGen.cpp<br>
> utils/TableGen/TableGenBackends.h<br>
><br>
</div></div>> <D7802.37320.patch><br>
<br>
This looks like a much better approach. I have a few minor things.<br>
LGTM after that.<br>
<br>
> Index: utils/TableGen/TableGenBackends.h<br>
> ===================================================================<br>
> --- utils/TableGen/TableGenBackends.h<br>
> +++ utils/TableGen/TableGenBackends.h<br>
> @@ -78,6 +78,7 @@<br>
> void EmitMapTable(RecordKeeper &RK, raw_ostream &OS);<br>
> void EmitOptParser(RecordKeeper &RK, raw_ostream &OS);<br>
> void EmitCTags(RecordKeeper &RK, raw_ostream &OS);<br>
> +void EmitAttribute(RecordKeeper &RK, raw_ostream &OS);<br>
<br>
Should this be `EmitAttributes()`? Or am I missing some convention?<br>
<br></blockquote><div><br></div><div>It should be plural. I'll correct the other places that use singular too.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
><br>
> } // End llvm namespace<br>
><br>
> Index: utils/TableGen/TableGen.cpp<br>
> ===================================================================<br>
> --- utils/TableGen/TableGen.cpp<br>
> +++ utils/TableGen/TableGen.cpp<br>
> @@ -41,7 +41,8 @@<br>
> PrintEnums,<br>
> PrintSets,<br>
> GenOptParserDefs,<br>
> - GenCTags<br>
> + GenCTags,<br>
> + GenAttribute<br>
<br>
I'll mark up all the other places I thought plural made sense<br>
(attributes instead of attribute). If there's a good reason for these<br>
to be singular instead of plural, please just correct me.<br>
<br>
=> plural<br>
<br>
> };<br>
><br>
> namespace {<br>
> @@ -85,6 +86,8 @@<br>
> "Generate option definitions"),<br>
> clEnumValN(GenCTags, "gen-ctags",<br>
> "Generate ctags-compatible index"),<br>
> + clEnumValN(GenAttribute, "gen-attr",<br>
> + "Generate attribute"),<br>
<br>
=> plural<br>
<br>
> clEnumValEnd));<br>
><br>
> cl::opt<std::string><br>
> @@ -165,6 +168,9 @@<br>
> case GenCTags:<br>
> EmitCTags(Records, OS);<br>
> break;<br>
> + case GenAttribute:<br>
> + EmitAttribute(Records, OS);<br>
> + break;<br>
> }<br>
><br>
> return false;<br>
> Index: utils/TableGen/CMakeLists.txt<br>
> ===================================================================<br>
> --- utils/TableGen/CMakeLists.txt<br>
> +++ utils/TableGen/CMakeLists.txt<br>
> @@ -4,6 +4,7 @@<br>
> AsmMatcherEmitter.cpp<br>
> AsmWriterEmitter.cpp<br>
> AsmWriterInst.cpp<br>
> + Attribute.cpp<br>
<br>
=> plural<br>
<br>
> CallingConvEmitter.cpp<br>
> CodeEmitterGen.cpp<br>
> CodeGenDAGPatterns.cpp<br>
> Index: utils/TableGen/Attribute.cpp<br>
> ===================================================================<br>
> --- /dev/null<br>
> +++ utils/TableGen/Attribute.cpp<br>
<br>
=> plural<br>
<br>
> @@ -0,0 +1,188 @@<br>
> +//===- Attribute.cpp - Generate attributes --------------------------------===//<br>
<br>
=> plural<br>
<br>
> +//<br>
> +// The LLVM Compiler Infrastructure<br>
> +//<br>
> +// This file is distributed under the University of Illinois Open Source<br>
> +// License. See LICENSE.TXT for details.<br>
> +//<br>
> +//===----------------------------------------------------------------------===//<br>
> +<br>
> +#include "llvm/Support/SourceMgr.h"<br>
> +#include "llvm/Support/MemoryBuffer.h"<br>
> +#include "llvm/TableGen/Error.h"<br>
> +#include "llvm/TableGen/Record.h"<br>
> +#include <algorithm><br>
> +#include <string><br>
> +#include <vector><br>
> +using namespace llvm;<br>
> +<br>
> +#define DEBUG_TYPE "attr-enum"<br>
> +<br>
> +namespace {<br>
> +<br>
> +class Attribute {<br>
<br>
=> plural<br>
<br>
> +public:<br>
> + Attribute(RecordKeeper &R) : Records(R) {}<br>
> + void emit(raw_ostream &OS);<br>
> +<br>
> +private:<br>
> + void emitTargetIndependentEnums(raw_ostream &OS);<br>
> + void emitFnAttrCompatCheck(raw_ostream &OS, bool IsStringAttr);<br>
> +<br>
> + void printEnumAttrClasses(raw_ostream &OS,<br>
> + const std::vector<Record *> &Records);<br>
> + void printStrAttrClasses(raw_ostream &OS,<br>
> + const std::vector<Record *> &Records);<br>
> + void printStrBoolAttrClasses(raw_ostream &OS,<br>
> + const std::vector<Record *> &Records);<br>
> +<br>
> + RecordKeeper &Records;<br>
> +};<br>
> +<br>
> +} // End anonymous namespace.<br>
> +<br>
> +static bool isStringAttr(const Record &A) {<br>
> + return A.isSubClassOf("StrAttr");<br>
> +}<br>
> +<br>
> +void Attribute::emitTargetIndependentEnums(raw_ostream &OS) {<br>
> + OS << "#ifdef GET_ATTR_ENUM\n";<br>
> + OS << "#undef GET_ATTR_ENUM\n";<br>
> +<br>
> + const std::vector<Record*> &Attrs =<br>
> + Records.getAllDerivedDefinitions("EnumAttr");<br>
> +<br>
> + for (auto A : Attrs)<br>
> + if (!isStringAttr(*A))<br>
> + OS << A->getName() << ",\n";<br>
> +<br>
> + OS << "#endif\n";<br>
> +}<br>
> +<br>
> +void Attribute::emitFnAttrCompatCheck(raw_ostream &OS, bool IsStringAttr) {<br>
> + OS << "#ifdef GET_ATTR_COMPAT_FUNC\n";<br>
> + OS << "#undef GET_ATTR_COMPAT_FUNC\n";<br>
> +<br>
> + OS << "struct EnumAttr {\n";<br>
> + OS << " static bool isSet(const Function &Fn,\n";<br>
> + OS << " Attribute::AttrKind Kind) {\n";<br>
> + OS << " return Fn.hasFnAttribute(Kind);\n";<br>
> + OS << " }\n\n";<br>
> + OS << " static void set(Function &Fn,\n";<br>
> + OS << " Attribute::AttrKind Kind, bool Val) {\n";<br>
> + OS << " if (Val)\n";<br>
> + OS << " Fn.addFnAttr(Kind);\n";<br>
> + OS << " else\n";<br>
> + OS << " Fn.removeFnAttr(Kind);\n";<br>
> + OS << " }\n";<br>
> + OS << "};\n\n";<br>
> +<br>
> + OS << "struct StrAttr {\n";<br>
> + OS << " static bool isSet(const Function &Fn,\n";<br>
> + OS << " StringRef Kind) {\n";<br>
> + OS << " return Fn.hasFnAttribute(Kind);\n";<br>
> + OS << " }\n\n";<br>
> + OS << " static void set(Function &Fn,\n";<br>
> + OS << " StringRef Kind, bool Val) {\n";<br>
> + OS << " if (Val)\n";<br>
> + OS << " Fn.addFnAttr(Kind);\n";<br>
> + OS << " else {\n";<br>
> + OS << " auto &Ctx = Fn.getContext();\n";<br>
> + OS << " AttributeSet As;\n";<br>
> + OS << " As.addAttribute(Ctx, AttributeSet::FunctionIndex, Kind);\n";<br>
> + OS << " Fn.removeAttributes(AttributeSet::FunctionIndex, As);\n";<br>
> + OS << " }\n";<br>
> + OS << " }\n";<br>
> + OS << "};\n\n";<br>
> +<br>
> + OS << "struct StrBoolAttr {\n";<br>
> + OS << " static bool isSet(const Function &Fn,\n";<br>
> + OS << " StringRef Kind) {\n";<br>
> + OS << " auto A = Fn.getFnAttribute(Kind);\n";<br>
> + OS << " return A.getValueAsString().equals(\"true\");\n";<br>
> + OS << " }\n\n";<br>
> + OS << " static void set(Function &Fn,\n";<br>
> + OS << " StringRef Kind, bool Val) {\n";<br>
> + OS << " Fn.addFnAttr(Kind, Val ? \"true\" : \"false\");\n";<br>
> + OS << " }\n";<br>
> + OS << "};\n\n";<br>
> +<br>
> + printEnumAttrClasses(OS ,Records.getAllDerivedDefinitions("EnumAttr"));<br>
> + //printStrAttrClasses(OS, Records.getAllDerivedDefinitions("StrAttr"));<br>
> + printStrBoolAttrClasses(OS , Records.getAllDerivedDefinitions("StrBoolAttr"));<br>
> +<br>
> + OS << "static inline bool hasCompatibleFnAttrs(const Function &Caller,\n"<br>
> + << " const Function &Callee) {\n";<br>
> + OS << " bool Ret = true;\n\n";<br>
> +<br>
> + const std::vector<Record *> &CompatRules =<br>
> + Records.getAllDerivedDefinitions("CompatRule");<br>
> +<br>
> + for (auto *Rule : CompatRules) {<br>
> + StringRef FuncName = Rule->getValueAsString("CompatFunc");<br>
> + OS << " Ret &= " << FuncName << "(Caller, Callee);\n";<br>
> + }<br>
> +<br>
> + OS << "\n";<br>
> + OS << " return Ret;\n";<br>
> + OS << "}\n\n";<br>
> +<br>
> + const std::vector<Record *> &MergeRules =<br>
> + Records.getAllDerivedDefinitions("MergeRule");<br>
> + OS << "static inline void mergeFnAttrs(Function &Caller,\n"<br>
> + << " const Function &Callee) {\n";<br>
> +<br>
> + for (auto *Rule : MergeRules) {<br>
> + StringRef FuncName = Rule->getValueAsString("MergeFunc");<br>
> + OS << " " << FuncName << "(Caller, Callee);\n";<br>
> + }<br>
> +<br>
> + OS << "}\n\n";<br>
> +<br>
> + OS << "#endif\n";<br>
> +}<br>
> +<br>
> +void Attribute::printEnumAttrClasses(raw_ostream &OS,<br>
> + const std::vector<Record *> &Records) {<br>
> + OS << "// EnumAttr classes\n";<br>
> + for (const auto *R : Records) {<br>
> + OS << "struct " << R->getName() << "Attr : EnumAttr {\n";<br>
> + OS << " constexpr static const enum Attribute::AttrKind Kind = ";<br>
> + OS << "Attribute::" << R->getName() << ";\n";<br>
> + OS << "};\n";<br>
> + }<br>
> + OS << "\n";<br>
> +}<br>
> +<br>
> +void Attribute::printStrAttrClasses(raw_ostream &OS,<br>
> + const std::vector<Record *> &Records) {<br>
> + OS << "// StrAttr classes\n";<br>
> + for (const auto *R : Records)<br>
> + OS << "// class " << R->getName() << "\n";<br>
> + OS << "\n";<br>
> +}<br>
> +<br>
> +void Attribute::printStrBoolAttrClasses(raw_ostream &OS,<br>
> + const std::vector<Record *> &Records) {<br>
> + OS << "// StrBoolAttr classes\n";<br>
> + for (const auto *R : Records) {<br>
> + OS << "struct " << R->getName() << "Attr : StrBoolAttr {\n";<br>
> + OS << " constexpr static const char * const Kind = \"";<br>
> + OS << R->getValueAsString("AttrString") << "\";\n";<br>
> + OS << "};\n";<br>
> + }<br>
> + OS << "\n";<br>
> +}<br>
> +<br>
> +void Attribute::emit(raw_ostream &OS) {<br>
> + emitTargetIndependentEnums(OS);<br>
> + emitFnAttrCompatCheck(OS, false);<br>
> +}<br>
> +<br>
> +namespace llvm {<br>
> +<br>
> +void EmitAttribute(RecordKeeper &RK, raw_ostream &OS) {<br>
> + Attribute(RK).emit(OS);<br>
> +}<br>
> +<br>
> +} // End llvm namespace.<br>
> Index: test/Transforms/ObjCARC/nested.ll<br>
> ===================================================================<br>
> --- test/Transforms/ObjCARC/nested.ll<br>
> +++ test/Transforms/ObjCARC/nested.ll<br>
> @@ -820,6 +820,6 @@<br>
> }<br>
><br>
><br>
> -; CHECK: attributes #0 = { nounwind argmemonly }<br>
> +; CHECK: attributes #0 = { argmemonly nounwind }<br>
<br>
Is this just because you've sorted the attributes in the .td file? If<br>
so, maybe you should leave them in the other order as a starting point,<br>
and then sort them as a follow-up if you think it's worth it. That<br>
should separate the random testcase output changes into a separate<br>
commit. (If they *have* to be sorted by name, then I guess ignore<br>
this.)<br>
<br></blockquote><div><br></div><div>Yes, it's because I've sorted the attributes into alphabetical order in the .td file. I'll keep them in the same order as in the original header file, (Attributes.h).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> ; CHECK: attributes #1 = { nonlazybind }<br>
> ; CHECK: attributes [[NUW]] = { nounwind }<br>
> Index: lib/Transforms/IPO/Inliner.cpp<br>
> ===================================================================<br>
> --- lib/Transforms/IPO/Inliner.cpp<br>
> +++ lib/Transforms/IPO/Inliner.cpp<br>
> @@ -86,39 +86,6 @@<br>
> typedef DenseMap<ArrayType*, std::vector<AllocaInst*> ><br>
> InlinedArrayAllocasTy;<br>
><br>
> -/// \brief If the inlined function had a higher stack protection level than the<br>
> -/// calling function, then bump up the caller's stack protection level.<br>
> -static void AdjustCallerSSPLevel(Function *Caller, Function *Callee) {<br>
> - // If upgrading the SSP attribute, clear out the old SSP Attributes first.<br>
> - // Having multiple SSP attributes doesn't actually hurt, but it adds useless<br>
> - // clutter to the IR.<br>
> - AttrBuilder B;<br>
> - B.addAttribute(Attribute::StackProtect)<br>
> - .addAttribute(Attribute::StackProtectStrong)<br>
> - .addAttribute(Attribute::StackProtectReq);<br>
> - AttributeSet OldSSPAttr = AttributeSet::get(Caller->getContext(),<br>
> - AttributeSet::FunctionIndex,<br>
> - B);<br>
> -<br>
> - if (Callee->hasFnAttribute(Attribute::SafeStack)) {<br>
> - Caller->removeAttributes(AttributeSet::FunctionIndex, OldSSPAttr);<br>
> - Caller->addFnAttr(Attribute::SafeStack);<br>
> - } else if (Callee->hasFnAttribute(Attribute::StackProtectReq) &&<br>
> - !Caller->hasFnAttribute(Attribute::SafeStack)) {<br>
> - Caller->removeAttributes(AttributeSet::FunctionIndex, OldSSPAttr);<br>
> - Caller->addFnAttr(Attribute::StackProtectReq);<br>
> - } else if (Callee->hasFnAttribute(Attribute::StackProtectStrong) &&<br>
> - !Caller->hasFnAttribute(Attribute::SafeStack) &&<br>
> - !Caller->hasFnAttribute(Attribute::StackProtectReq)) {<br>
> - Caller->removeAttributes(AttributeSet::FunctionIndex, OldSSPAttr);<br>
> - Caller->addFnAttr(Attribute::StackProtectStrong);<br>
> - } else if (Callee->hasFnAttribute(Attribute::StackProtect) &&<br>
> - !Caller->hasFnAttribute(Attribute::SafeStack) &&<br>
> - !Caller->hasFnAttribute(Attribute::StackProtectReq) &&<br>
> - !Caller->hasFnAttribute(Attribute::StackProtectStrong))<br>
> - Caller->addFnAttr(Attribute::StackProtect);<br>
> -}<br>
> -<br>
> /// If it is possible to inline the specified call site,<br>
> /// do so and update the CallGraph for this operation.<br>
> ///<br>
> @@ -146,7 +113,7 @@<br>
> if (!InlineFunction(CS, IFI, &AAR, InsertLifetime))<br>
> return false;<br>
><br>
> - AdjustCallerSSPLevel(Caller, Callee);<br>
> + AttributeFuncs::mergeAttributesForInlining(*Caller, *Callee);<br>
><br>
> // Look at all of the allocas that we inlined through this call site. If we<br>
> // have already inlined other allocas through other calls into this function,<br>
> Index: lib/IR/Makefile<br>
> ===================================================================<br>
> --- lib/IR/Makefile<br>
> +++ lib/IR/Makefile<br>
> @@ -10,14 +10,20 @@<br>
> LIBRARYNAME = LLVMCore<br>
> BUILD_ARCHIVE = 1<br>
><br>
> -BUILT_SOURCES = $(PROJ_OBJ_ROOT)/include/llvm/IR/Intrinsics.gen<br>
> +BUILT_SOURCES = $(PROJ_OBJ_ROOT)/include/llvm/IR/Intrinsics.gen \<br>
> + $(PROJ_OBJ_ROOT)/include/llvm/IR/Attributes.inc \<br>
> + $(PROJ_OBJ_ROOT)/lib/IR/AttributesCompatFunc.inc<br>
><br>
> include $(LEVEL)/Makefile.common<br>
><br>
> GENFILE:=$(PROJ_OBJ_ROOT)/include/llvm/IR/Intrinsics.gen<br>
> +ATTRINCFILE:=$(PROJ_OBJ_ROOT)/include/llvm/IR/Attributes.inc<br>
> +ATTRCOMPATFUNCINCFILE:=$(PROJ_OBJ_ROOT)/lib/IR/AttributesCompatFunc.inc<br>
><br>
> INTRINSICTD := $(PROJ_SRC_ROOT)/include/llvm/IR/Intrinsics.td<br>
> INTRINSICTDS := $(wildcard $(PROJ_SRC_ROOT)/include/llvm/IR/Intrinsics*.td)<br>
> +ATTRIBUTESTD := $(PROJ_SRC_ROOT)/include/llvm/IR/Attributes.td<br>
> +ATTRCOMPATFUNCTD := $(PROJ_SRC_ROOT)/lib/IR/AttributesCompatFunc.td<br>
><br>
> $(ObjDir)/Intrinsics.gen.tmp: $(ObjDir)/.dir $(INTRINSICTDS) $(LLVM_TBLGEN)<br>
> $(Echo) Building Intrinsics.gen.tmp from Intrinsics.td<br>
> @@ -28,6 +34,32 @@<br>
> $(EchoCmd) Updated Intrinsics.gen because Intrinsics.gen.tmp \<br>
> changed significantly. )<br>
><br>
> +$(ObjDir)/Attributes.inc.tmp: $(ObjDir)/.dir $(ATTRIBUTESTD) $(LLVM_TBLGEN)<br>
> + $(Echo) Building Attributes.inc.tmp from $(ATTRIBUTESTD)<br>
> + $(Verb) $(LLVMTableGen) $(call SYSPATH, $(ATTRIBUTESTD)) -o $(call SYSPATH, $@) -gen-attr<br>
> +<br>
> +$(ATTRINCFILE): $(ObjDir)/Attributes.inc.tmp $(PROJ_OBJ_ROOT)/include/llvm/IR/.dir<br>
> + $(Verb) $(CMP) -s $@ $< || ( $(CP) $< $@ && \<br>
> + $(EchoCmd) Updated Attributes.inc because Attributes.inc.tmp \<br>
> + changed significantly. )<br>
> +<br>
> +$(ObjDir)/AttributesCompatFunc.inc.tmp: $(ObjDir)/.dir $(ATTRCOMPATFUNCTD) $(LLVM_TBLGEN)<br>
> + $(Echo) Building AttributesCompatFunc.inc.tmp from $(ATTRCOMPATFUNCTD)<br>
> + $(Verb) $(LLVMTableGen) $(call SYSPATH, $(ATTRCOMPATFUNCTD)) -o $(call SYSPATH, $@) -gen-attr<br>
> +<br>
> +$(ATTRCOMPATFUNCINCFILE): $(ObjDir)/AttributesCompatFunc.inc.tmp $(PROJ_OBJ_ROOT)/include/llvm/IR/.dir<br>
> + $(Verb) $(CMP) -s $@ $< || ( $(CP) $< $@ && \<br>
> + $(EchoCmd) Updated AttributesCompatFunc.inc because AttributesCompatFunc.inc.tmp \<br>
> + changed significantly. )<br>
> +<br>
> install-local:: $(GENFILE)<br>
> $(Echo) Installing $(DESTDIR)$(PROJ_includedir)/llvm/IR/Intrinsics.gen<br>
> $(Verb) $(DataInstall) $(GENFILE) $(DESTDIR)$(PROJ_includedir)/llvm/IR/Intrinsics.gen<br>
> +<br>
> +install-local:: $(ATTRINCFILE)<br>
> + $(Echo) Installing $(DESTDIR)$(PROJ_includedir)/llvm/IR/Attributes.inc<br>
> + $(Verb) $(DataInstall) $(ATTRINCFILE) $(DESTDIR)$(PROJ_includedir)/llvm/IR/Attributes.inc<br>
> +<br>
> +install-local:: $(ATTRCOMPATFUNCINCFILE)<br>
> + $(Echo) Installing $(DESTDIR)$(PROJ_libdir)/IR/AttributesCompatFunc.inc<br>
> + $(Verb) $(DataInstall) $(ATTRCOMPATFUNCINCFILE) $(DESTDIR)$(PROJ_libdir)/IR/AttributesCompatFunc.inc<br>
> Index: lib/IR/CMakeLists.txt<br>
> ===================================================================<br>
> --- lib/IR/CMakeLists.txt<br>
> +++ lib/IR/CMakeLists.txt<br>
> @@ -1,3 +1,7 @@<br>
> +set(LLVM_TARGET_DEFINITIONS AttributesCompatFunc.td)<br>
> +tablegen(LLVM AttributesCompatFunc.inc -gen-attr)<br>
> +add_public_tablegen_target(AttributeCompatFuncTableGen)<br>
> +<br>
> add_llvm_library(LLVMCore<br>
> AsmWriter.cpp<br>
> Attributes.cpp<br>
> Index: lib/IR/AttributesCompatFunc.td<br>
> ===================================================================<br>
> --- /dev/null<br>
> +++ lib/IR/AttributesCompatFunc.td<br>
> @@ -0,0 +1 @@<br>
> +include "llvm/IR/Attributes.td"<br>
> Index: lib/IR/Attributes.cpp<br>
> ===================================================================<br>
> --- lib/IR/Attributes.cpp<br>
> +++ lib/IR/Attributes.cpp<br>
> @@ -14,6 +14,7 @@<br>
> //===----------------------------------------------------------------------===//<br>
><br>
> #include "llvm/IR/Attributes.h"<br>
> +#include "llvm/IR/Function.h"<br>
> #include "AttributeImpl.h"<br>
> #include "LLVMContextImpl.h"<br>
> #include "llvm/ADT/STLExtras.h"<br>
> @@ -1404,3 +1405,80 @@<br>
><br>
> return Incompatible;<br>
> }<br>
> +<br>
> +template<typename AttrClass><br>
> +static bool isEqual(const Function &Caller, const Function &Callee) {<br>
> + return Caller.getFnAttribute(AttrClass::Kind) ==<br>
> + Callee.getFnAttribute(AttrClass::Kind);<br>
> +}<br>
> +<br>
> +/// \brief Compute the logical AND of the attributes of the caller and the<br>
> +/// callee.<br>
> +///<br>
> +/// This function sets the caller's attribute to false if the callee's attribute<br>
> +/// is false.<br>
> +template<typename AttrClass><br>
> +static void setAND(Function &Caller, const Function &Callee) {<br>
> + if (AttrClass::isSet(Caller, AttrClass::Kind) &&<br>
> + !AttrClass::isSet(Callee, AttrClass::Kind))<br>
> + AttrClass::set(Caller, AttrClass::Kind, false);<br>
> +}<br>
> +<br>
> +/// \brief Compute the logical OR of the attributes of the caller and the<br>
> +/// callee.<br>
> +///<br>
> +/// This function sets the caller's attribute to true if the callee's attribute<br>
> +/// is true.<br>
> +template<typename AttrClass><br>
> +static void setOR(Function &Caller, const Function &Callee) {<br>
> + if (!AttrClass::isSet(Caller, AttrClass::Kind) &&<br>
> + AttrClass::isSet(Callee, AttrClass::Kind))<br>
> + AttrClass::set(Caller, AttrClass::Kind, true);<br>
> +}<br>
> +<br>
> +/// \brief If the inlined function had a higher stack protection level than the<br>
> +/// calling function, then bump up the caller's stack protection level.<br>
> +static void adjustCallerSSPLevel(Function &Caller, const Function &Callee) {<br>
> + // If upgrading the SSP attribute, clear out the old SSP Attributes first.<br>
> + // Having multiple SSP attributes doesn't actually hurt, but it adds useless<br>
> + // clutter to the IR.<br>
> + AttrBuilder B;<br>
> + B.addAttribute(Attribute::StackProtect)<br>
> + .addAttribute(Attribute::StackProtectStrong)<br>
> + .addAttribute(Attribute::StackProtectReq);<br>
> + AttributeSet OldSSPAttr = AttributeSet::get(Caller.getContext(),<br>
> + AttributeSet::FunctionIndex,<br>
> + B);<br>
> +<br>
> + if (Callee.hasFnAttribute(Attribute::SafeStack)) {<br>
> + Caller.removeAttributes(AttributeSet::FunctionIndex, OldSSPAttr);<br>
> + Caller.addFnAttr(Attribute::SafeStack);<br>
> + } else if (Callee.hasFnAttribute(Attribute::StackProtectReq) &&<br>
> + !Caller.hasFnAttribute(Attribute::SafeStack)) {<br>
> + Caller.removeAttributes(AttributeSet::FunctionIndex, OldSSPAttr);<br>
> + Caller.addFnAttr(Attribute::StackProtectReq);<br>
> + } else if (Callee.hasFnAttribute(Attribute::StackProtectStrong) &&<br>
> + !Caller.hasFnAttribute(Attribute::SafeStack) &&<br>
> + !Caller.hasFnAttribute(Attribute::StackProtectReq)) {<br>
> + Caller.removeAttributes(AttributeSet::FunctionIndex, OldSSPAttr);<br>
> + Caller.addFnAttr(Attribute::StackProtectStrong);<br>
> + } else if (Callee.hasFnAttribute(Attribute::StackProtect) &&<br>
> + !Caller.hasFnAttribute(Attribute::SafeStack) &&<br>
> + !Caller.hasFnAttribute(Attribute::StackProtectReq) &&<br>
> + !Caller.hasFnAttribute(Attribute::StackProtectStrong))<br>
> + Caller.addFnAttr(Attribute::StackProtect);<br>
> +}<br>
> +<br>
> +#define GET_ATTR_COMPAT_FUNC<br>
> +#include "AttributesCompatFunc.inc"<br>
> +<br>
> +bool AttributeFuncs::areInlineCompatible(const Function &Caller,<br>
> + const Function &Callee) {<br>
> + return hasCompatibleFnAttrs(Caller, Callee);<br>
> +}<br>
> +<br>
> +<br>
> +void AttributeFuncs::mergeAttributesForInlining(Function &Caller,<br>
> + const Function &Callee) {<br>
> + mergeFnAttrs(Caller, Callee);<br>
> +}<br>
> Index: lib/Analysis/InlineCost.cpp<br>
> ===================================================================<br>
> --- lib/Analysis/InlineCost.cpp<br>
> +++ lib/Analysis/InlineCost.cpp<br>
> @@ -1362,9 +1362,7 @@<br>
> Function *Callee,<br>
> TargetTransformInfo &TTI) {<br>
> return TTI.areInlineCompatible(Caller, Callee) &&<br>
> - attributeMatches(Caller, Callee, Attribute::SanitizeAddress) &&<br>
> - attributeMatches(Caller, Callee, Attribute::SanitizeMemory) &&<br>
> - attributeMatches(Caller, Callee, Attribute::SanitizeThread);<br>
> + AttributeFuncs::areInlineCompatible(*Caller, *Callee);<br>
> }<br>
><br>
> InlineCost InlineCostAnalysis::getInlineCost(CallSite CS, Function *Callee,<br>
> Index: include/llvm/IR/CMakeLists.txt<br>
> ===================================================================<br>
> --- include/llvm/IR/CMakeLists.txt<br>
> +++ include/llvm/IR/CMakeLists.txt<br>
> @@ -1,5 +1,6 @@<br>
> -set(LLVM_TARGET_DEFINITIONS Intrinsics.td)<br>
> +set(LLVM_TARGET_DEFINITIONS Attributes.td)<br>
> +tablegen(LLVM Attributes.inc -gen-attr)<br>
><br>
> +set(LLVM_TARGET_DEFINITIONS Intrinsics.td)<br>
> tablegen(LLVM Intrinsics.gen -gen-intrinsic)<br>
> -<br>
> add_public_tablegen_target(intrinsics_gen)<br>
> Index: include/llvm/IR/Attributes.td<br>
> ===================================================================<br>
> --- /dev/null<br>
> +++ include/llvm/IR/Attributes.td<br>
> @@ -0,0 +1,187 @@<br>
> +/// Attribute base class.<br>
> +class Attr<string S> {<br>
> + // String representation of this attribute in the IR.<br>
> + string AttrString = S;<br>
> +}<br>
> +<br>
> +/// Enum attribute.<br>
> +class EnumAttr<string S> : Attr<S>;<br>
> +<br>
> +/// StringBool attribute.<br>
> +class StrBoolAttr<string S> : Attr<S>;<br>
> +<br>
> +/// Target-independent enum attributes.<br>
> +<br>
> +/// Alignment of parameter (5 bits) stored as log2 of alignment with +1 bias.<br>
> +/// 0 means unaligned (different from align(1)).<br>
> +def Alignment : EnumAttr<"align">;<br>
> +<br>
> +/// inline=always.<br>
> +def AlwaysInline : EnumAttr<"alwaysinline">;<br>
> +<br>
> +/// Funciton can access memory only using pointers based on its arguments.<br>
> +def ArgMemOnly : EnumAttr<"argmemonly">;<br>
> +<br>
> +/// Callee is recognized as a builtin, despite nobuiltin attribute on its<br>
> +/// declaration.<br>
> +def Builtin : EnumAttr<"builtin">;<br>
> +<br>
> +/// Pass structure by value.<br>
> +def ByVal : EnumAttr<"byval">;<br>
> +<br>
> +/// Marks function as being in a cold path.<br>
> +def Cold : EnumAttr<"cold">;<br>
> +<br>
> +/// Can only be moved to control-equivalent blocks.<br>
> +def Convergent : EnumAttr<"convergent">;<br>
> +<br>
> +/// Pointer is known to be dereferenceable.<br>
> +def Dereferenceable : EnumAttr<"dereferenceable">;<br>
> +<br>
> +/// Pointer is either null or dereferenceable.<br>
> +def DereferenceableOrNull : EnumAttr<"dereferenceable_or_null">;<br>
> +<br>
> +/// Pass structure in an alloca.<br>
> +def InAlloca : EnumAttr<"inalloca">;<br>
> +<br>
> +/// Source said inlining was desirable.<br>
> +def InlineHint : EnumAttr<"inlinehint">;<br>
> +<br>
> +/// Force argument to be passed in register.<br>
> +def InReg : EnumAttr<"inreg">;<br>
> +<br>
> +/// Build jump-instruction tables and replace refs.<br>
> +def JumpTable : EnumAttr<"jumptable">;<br>
> +<br>
> +/// Function must be optimized for size first.<br>
> +def MinSize : EnumAttr<"minsize">;<br>
> +<br>
> +/// Naked function.<br>
> +def Naked : EnumAttr<"naked">;<br>
> +<br>
> +/// Nested function static chain.<br>
> +def Nest : EnumAttr<"nest">;<br>
> +<br>
> +/// Considered to not alias after call.<br>
> +def NoAlias : EnumAttr<"noalias">;<br>
> +<br>
> +/// Callee isn't recognized as a builtin.<br>
> +def NoBuiltin : EnumAttr<"nobuiltin">;<br>
> +<br>
> +/// Function creates no aliases of pointer.<br>
> +def NoCapture : EnumAttr<"nocapture">;<br>
> +<br>
> +/// Call cannot be duplicated.<br>
> +def NoDuplicate : EnumAttr<"noduplicate">;<br>
> +<br>
> +/// Disable implicit floating point insts.<br>
> +def NoImplicitFloat : EnumAttr<"noimplicitfloat">;<br>
> +<br>
> +/// inline=never.<br>
> +def NoInline : EnumAttr<"noinline">;<br>
> +<br>
> +/// Function is called early and/or often, so lazy binding isn't worthwhile.<br>
> +def NonLazyBind : EnumAttr<"nonlazybind">;<br>
> +<br>
> +/// Pointer is known to be not null.<br>
> +def NonNull : EnumAttr<"nonnull">;<br>
> +<br>
> +/// Disable redzone.<br>
> +def NoRedZone : EnumAttr<"noredzone">;<br>
> +<br>
> +/// Mark the function as not returning.<br>
> +def NoReturn : EnumAttr<"noreturn">;<br>
> +<br>
> +/// Function doesn't unwind stack.<br>
> +def NoUnwind : EnumAttr<"nounwind">;<br>
> +<br>
> +/// opt_size.<br>
> +def OptimizeForSize : EnumAttr<"optsize">;<br>
> +<br>
> +/// Function must not be optimized.<br>
> +def OptimizeNone : EnumAttr<"optnone">;<br>
> +<br>
> +/// Function does not access memory.<br>
> +def ReadNone : EnumAttr<"readnone">;<br>
> +<br>
> +/// Function only reads from memory.<br>
> +def ReadOnly : EnumAttr<"readonly">;<br>
> +<br>
> +/// Return value is always equal to this argument.<br>
> +def Returned : EnumAttr<"returned">;<br>
> +<br>
> +/// Function can return twice.<br>
> +def ReturnsTwice : EnumAttr<"returns_twice">;<br>
> +<br>
> +/// Safe Stack protection.<br>
> +def SafeStack : EnumAttr<"safestack">;<br>
> +<br>
> +/// Sign extended before/after call.<br>
> +def SExt : EnumAttr<"signext">;<br>
> +<br>
> +/// Alignment of stack for function (3 bits) stored as log2 of alignment with<br>
> +/// +1 bias 0 means unaligned (different from alignstack=(1)).<br>
> +def StackAlignment : EnumAttr<"alignstack">;<br>
> +<br>
> +/// Stack protection.<br>
> +def StackProtect : EnumAttr<"ssp">;<br>
> +<br>
> +/// Stack protection required.<br>
> +def StackProtectReq : EnumAttr<"sspreq">;<br>
> +<br>
> +/// Strong Stack protection.<br>
> +def StackProtectStrong : EnumAttr<"sspstrong">;<br>
> +<br>
> +/// Hidden pointer to structure to return.<br>
> +def StructRet : EnumAttr<"sret">;<br>
> +<br>
> +/// AddressSanitizer is on.<br>
> +def SanitizeAddress : EnumAttr<"sanitize_address">;<br>
> +<br>
> +/// ThreadSanitizer is on.<br>
> +def SanitizeThread : EnumAttr<"sanitize_thread">;<br>
> +<br>
> +/// MemorySanitizer is on.<br>
> +def SanitizeMemory : EnumAttr<"sanitize_memory">;<br>
> +<br>
> +/// Function must be in a unwind table.<br>
> +def UWTable : EnumAttr<"uwtable">;<br>
> +<br>
> +/// Zero extended before/after call.<br>
> +def ZExt : EnumAttr<"zeroext">;<br>
> +<br>
> +/// Target-independent string attributes.<br>
> +def LessPreciseFPMAD : StrBoolAttr<"less-precise-fpmad">;<br>
> +def NoInfsFPMath : StrBoolAttr<"no-infs-fp-math">;<br>
> +def NoNansFPMath : StrBoolAttr<"no-nans-fp-math">;<br>
> +def UnsafeFPMath : StrBoolAttr<"unsafe-fp-math">;<br>
> +<br>
> +class CompatRule<string F> {<br>
> + // The name of the function called to check the attribute of the caller and<br>
> + // callee and decide whether inlining should be allowed. The function's<br>
> + // signature must match "bool(const Function&, const Function &)", where the<br>
> + // first parameter is the reference to the caller and the second parameter is<br>
> + // the reference to the callee. It must return false if the attributes of the<br>
> + // caller and callee are incompatible, and true otherwise.<br>
> + string CompatFunc = F;<br>
> +}<br>
> +<br>
> +def : CompatRule<"isEqual<SanitizeAddressAttr>">;<br>
> +def : CompatRule<"isEqual<SanitizeThreadAttr>">;<br>
> +def : CompatRule<"isEqual<SanitizeMemoryAttr>">;<br>
> +<br>
> +class MergeRule<string F> {<br>
> + // The name of the function called to merge the attributes of the caller and<br>
> + // callee. The function's signature must match<br>
> + // "void(Function&, const Function &)", where the first parameter is the<br>
> + // reference to the caller and the second parameter is the reference to the<br>
> + // callee.<br>
> + string MergeFunc = F;<br>
> +}<br>
> +<br>
> +def : MergeRule<"setAND<LessPreciseFPMADAttr>">;<br>
> +def : MergeRule<"setAND<NoInfsFPMathAttr>">;<br>
> +def : MergeRule<"setAND<NoNansFPMathAttr>">;<br>
> +def : MergeRule<"setAND<UnsafeFPMathAttr>">;<br>
> +def : MergeRule<"setOR<NoImplicitFloatAttr>">;<br>
> +def : MergeRule<"adjustCallerSSPLevel">;<br>
<br>
Are these the same rules that existed before? Ideally this commit (with<br>
the new infrastructure) would be no functionality change (NFC), and a<br>
follow-up would add any new rules. It's not a big deal though if it's<br>
hard to split up.<br>
<br></blockquote><div><br></div><div>Except for the ssp-level rule, all these rules didn't exist before. Adding or removing rules is easy, so I'll remove the new rules if this patch should be NFC.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> Index: include/llvm/IR/Attributes.h<br>
> ===================================================================<br>
> --- include/llvm/IR/Attributes.h<br>
> +++ include/llvm/IR/Attributes.h<br>
> @@ -33,6 +33,7 @@<br>
> class AttributeSetNode;<br>
> class Constant;<br>
> template<typename T> struct DenseMapInfo;<br>
> +class Function;<br>
> class LLVMContext;<br>
> class Type;<br>
><br>
> @@ -64,60 +65,8 @@<br>
> enum AttrKind {<br>
> // IR-Level Attributes<br>
> None, ///< No attributes have been set<br>
> - Alignment, ///< Alignment of parameter (5 bits)<br>
> - ///< stored as log2 of alignment with +1 bias<br>
> - ///< 0 means unaligned (different from align(1))<br>
> - AlwaysInline, ///< inline=always<br>
> - Builtin, ///< Callee is recognized as a builtin, despite<br>
> - ///< nobuiltin attribute on its declaration.<br>
> - ByVal, ///< Pass structure by value<br>
> - InAlloca, ///< Pass structure in an alloca<br>
> - Cold, ///< Marks function as being in a cold path.<br>
> - Convergent, ///< Can only be moved to control-equivalent blocks<br>
> - InlineHint, ///< Source said inlining was desirable<br>
> - InReg, ///< Force argument to be passed in register<br>
> - JumpTable, ///< Build jump-instruction tables and replace refs.<br>
> - MinSize, ///< Function must be optimized for size first<br>
> - Naked, ///< Naked function<br>
> - Nest, ///< Nested function static chain<br>
> - NoAlias, ///< Considered to not alias after call<br>
> - NoBuiltin, ///< Callee isn't recognized as a builtin<br>
> - NoCapture, ///< Function creates no aliases of pointer<br>
> - NoDuplicate, ///< Call cannot be duplicated<br>
> - NoImplicitFloat, ///< Disable implicit floating point insts<br>
> - NoInline, ///< inline=never<br>
> - NonLazyBind, ///< Function is called early and/or<br>
> - ///< often, so lazy binding isn't worthwhile<br>
> - NonNull, ///< Pointer is known to be not null<br>
> - Dereferenceable, ///< Pointer is known to be dereferenceable<br>
> - DereferenceableOrNull, ///< Pointer is either null or dereferenceable<br>
> - NoRedZone, ///< Disable redzone<br>
> - NoReturn, ///< Mark the function as not returning<br>
> - NoUnwind, ///< Function doesn't unwind stack<br>
> - OptimizeForSize, ///< opt_size<br>
> - OptimizeNone, ///< Function must not be optimized.<br>
> - ReadNone, ///< Function does not access memory<br>
> - ReadOnly, ///< Function only reads from memory<br>
> - ArgMemOnly, ///< Funciton can access memory only using pointers<br>
> - ///< based on its arguments.<br>
> - Returned, ///< Return value is always equal to this argument<br>
> - ReturnsTwice, ///< Function can return twice<br>
> - SExt, ///< Sign extended before/after call<br>
> - StackAlignment, ///< Alignment of stack for function (3 bits)<br>
> - ///< stored as log2 of alignment with +1 bias 0<br>
> - ///< means unaligned (different from<br>
> - ///< alignstack=(1))<br>
> - StackProtect, ///< Stack protection.<br>
> - StackProtectReq, ///< Stack protection required.<br>
> - StackProtectStrong, ///< Strong Stack protection.<br>
> - SafeStack, ///< Safe Stack protection.<br>
> - StructRet, ///< Hidden pointer to structure to return<br>
> - SanitizeAddress, ///< AddressSanitizer is on.<br>
> - SanitizeThread, ///< ThreadSanitizer is on.<br>
> - SanitizeMemory, ///< MemorySanitizer is on.<br>
> - UWTable, ///< Function must be in a unwind table<br>
> - ZExt, ///< Zero extended before/after call<br>
> -<br>
> + #define GET_ATTR_ENUM<br>
> + #include "llvm/IR/Attributes.inc"<br>
> EndAttrKinds ///< Sentinal value useful for loops<br>
> };<br>
><br>
> @@ -579,6 +528,13 @@<br>
> /// \brief Which attributes cannot be applied to a type.<br>
> AttrBuilder typeIncompatible(Type *Ty);<br>
><br>
> +/// \returns Return true if the two functions have compatible target-independent<br>
> +/// attributes for inlining purposes.<br>
> +bool areInlineCompatible(const Function &Caller, const Function &Callee);<br>
> +<br>
> +/// \brief Merge caller's and callee's attributes.<br>
> +void mergeAttributesForInlining(Function &Caller, const Function &Callee);<br>
> +<br>
> } // end AttributeFuncs namespace<br>
><br>
> } // end llvm namespace<br>
><br>
<br>
</blockquote></div><br></div></div>