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