From: Vedant Kumar Date: Sat, 19 Dec 2015 08:29:51 +0000 (+0000) Subject: Reapply "[IR] Move optional data in llvm::Function into a hungoff uselist" X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=commitdiff_plain;h=cd6381c4d45ff989a25731fedbda0bb8818499fc Reapply "[IR] Move optional data in llvm::Function into a hungoff uselist" Make personality functions, prefix data, and prologue data hungoff operands of Function. This is based on the email thread "[RFC] Clean up the way we store optional Function data" on llvm-dev. Thanks to sanjoyd, majnemer, rnk, loladiro, and dexonsmith for feedback! Includes a fix to scrub value subclass data in dropAllReferences. Differential Revision: http://reviews.llvm.org/D13829 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@256093 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/IR/Function.h b/include/llvm/IR/Function.h index 2843cf2cce3..2a983930bf4 100644 --- a/include/llvm/IR/Function.h +++ b/include/llvm/IR/Function.h @@ -64,7 +64,7 @@ private: * bit 0 : HasLazyArguments * bit 1 : HasPrefixData * bit 2 : HasPrologueData - * bit 3 : [reserved] + * bit 3 : HasPersonalityFn * bits 4-13 : CallingConvention * bits 14-15 : [reserved] */ @@ -110,7 +110,7 @@ private: public: static Function *Create(FunctionType *Ty, LinkageTypes Linkage, const Twine &N = "", Module *M = nullptr) { - return new(1) Function(Ty, Linkage, N, M); + return new Function(Ty, Linkage, N, M); } ~Function() override; @@ -118,14 +118,6 @@ public: /// \brief Provide fast operand accessors DECLARE_TRANSPARENT_OPERAND_ACCESSORS(Value); - /// \brief Get the personality function associated with this function. - bool hasPersonalityFn() const { return getNumOperands() != 0; } - Constant *getPersonalityFn() const { - assert(hasPersonalityFn()); - return cast(Op<0>()); - } - void setPersonalityFn(Constant *C); - Type *getReturnType() const; // Return the type of the ret val FunctionType *getFunctionType() const; // Return the FunctionType for me @@ -525,17 +517,30 @@ public: size_t arg_size() const; bool arg_empty() const; + /// \brief Check whether this function has a personality function. + bool hasPersonalityFn() const { + return getSubclassDataFromValue() & (1<<3); + } + + /// \brief Get the personality function associated with this function. + Constant *getPersonalityFn() const; + void setPersonalityFn(Constant *Fn); + + /// \brief Check whether this function has prefix data. bool hasPrefixData() const { return getSubclassDataFromValue() & (1<<1); } + /// \brief Get the prefix data associated with this function. Constant *getPrefixData() const; void setPrefixData(Constant *PrefixData); + /// \brief Check whether this function has prologue data. bool hasPrologueData() const { return getSubclassDataFromValue() & (1<<2); } + /// \brief Get the prologue data associated with this function. Constant *getPrologueData() const; void setPrologueData(Constant *PrologueData); @@ -630,11 +635,15 @@ public: DISubprogram *getSubprogram() const; private: + void allocHungoffUselist(); + template void setHungoffOperand(Constant *C); + // Shadow Value::setValueSubclassData with a private forwarding method so that // subclasses cannot accidentally use it. void setValueSubclassData(unsigned short D) { Value::setValueSubclassData(D); } + void setValueSubclassDataBit(unsigned Bit, bool On); bool hasMetadataHashEntry() const { return getGlobalObjectSubClassData() & HasMetadataHashEntryBit; @@ -647,7 +656,7 @@ private: }; template <> -struct OperandTraits : public OptionalOperandTraits {}; +struct OperandTraits : public HungoffOperandTraits<3> {}; DEFINE_TRANSPARENT_OPERAND_ACCESSORS(Function, Value) diff --git a/include/llvm/IR/User.h b/include/llvm/IR/User.h index 639dc5c01c8..885ae197d22 100644 --- a/include/llvm/IR/User.h +++ b/include/llvm/IR/User.h @@ -170,19 +170,6 @@ public: NumUserOperands = NumOps; } - /// Set the number of operands on a Function. - /// - /// Function always allocates space for a single operands, but - /// doesn't always use it. - /// - /// FIXME: As that the number of operands is used to find the start of - /// the allocated memory in operator delete, we need to always think we have - /// 1 operand before delete. - void setFunctionNumOperands(unsigned NumOps) { - assert(NumOps <= 1 && "Function can only have 0 or 1 operands"); - NumUserOperands = NumOps; - } - /// \brief Subclasses with hung off uses need to manage the operand count /// themselves. In these instances, the operand count isn't used to find the /// OperandList, so there's no issue in having the operand count change. diff --git a/lib/Bitcode/Writer/ValueEnumerator.cpp b/lib/Bitcode/Writer/ValueEnumerator.cpp index 2a9b4490ffb..faf82a7926e 100644 --- a/lib/Bitcode/Writer/ValueEnumerator.cpp +++ b/lib/Bitcode/Writer/ValueEnumerator.cpp @@ -87,15 +87,9 @@ static OrderMap orderModule(const Module &M) { if (!isa(A.getAliasee())) orderValue(A.getAliasee(), OM); for (const Function &F : M) { - if (F.hasPrefixData()) - if (!isa(F.getPrefixData())) - orderValue(F.getPrefixData(), OM); - if (F.hasPrologueData()) - if (!isa(F.getPrologueData())) - orderValue(F.getPrologueData(), OM); - if (F.hasPersonalityFn()) - if (!isa(F.getPersonalityFn())) - orderValue(F.getPersonalityFn(), OM); + for (const Use &U : F.operands()) + if (!isa(U.get())) + orderValue(U.get(), OM); } OM.LastGlobalConstantID = OM.size(); @@ -273,12 +267,8 @@ static UseListOrderStack predictUseListOrder(const Module &M) { for (const GlobalAlias &A : M.aliases()) predictValueUseListOrder(A.getAliasee(), nullptr, OM, Stack); for (const Function &F : M) { - if (F.hasPrefixData()) - predictValueUseListOrder(F.getPrefixData(), nullptr, OM, Stack); - if (F.hasPrologueData()) - predictValueUseListOrder(F.getPrologueData(), nullptr, OM, Stack); - if (F.hasPersonalityFn()) - predictValueUseListOrder(F.getPersonalityFn(), nullptr, OM, Stack); + for (const Use &U : F.operands()) + predictValueUseListOrder(U.get(), nullptr, OM, Stack); } return Stack; @@ -321,20 +311,10 @@ ValueEnumerator::ValueEnumerator(const Module &M, for (const GlobalAlias &GA : M.aliases()) EnumerateValue(GA.getAliasee()); - // Enumerate the prefix data constants. + // Enumerate any optional Function data. for (const Function &F : M) - if (F.hasPrefixData()) - EnumerateValue(F.getPrefixData()); - - // Enumerate the prologue data constants. - for (const Function &F : M) - if (F.hasPrologueData()) - EnumerateValue(F.getPrologueData()); - - // Enumerate the personality functions. - for (Module::const_iterator I = M.begin(), E = M.end(); I != E; ++I) - if (I->hasPersonalityFn()) - EnumerateValue(I->getPersonalityFn()); + for (const Use &U : F.operands()) + EnumerateValue(U.get()); // Enumerate the metadata type. // diff --git a/lib/IR/AsmWriter.cpp b/lib/IR/AsmWriter.cpp index 51a0faee81c..124fc56e78f 100644 --- a/lib/IR/AsmWriter.cpp +++ b/lib/IR/AsmWriter.cpp @@ -103,17 +103,9 @@ static OrderMap orderModule(const Module *M) { orderValue(&A, OM); } for (const Function &F : *M) { - if (F.hasPrefixData()) - if (!isa(F.getPrefixData())) - orderValue(F.getPrefixData(), OM); - - if (F.hasPrologueData()) - if (!isa(F.getPrologueData())) - orderValue(F.getPrologueData(), OM); - - if (F.hasPersonalityFn()) - if (!isa(F.getPersonalityFn())) - orderValue(F.getPersonalityFn(), OM); + for (const Use &U : F.operands()) + if (!isa(U.get())) + orderValue(U.get(), OM); orderValue(&F, OM); @@ -263,8 +255,8 @@ static UseListOrderStack predictUseListOrder(const Module *M) { for (const GlobalAlias &A : M->aliases()) predictValueUseListOrder(A.getAliasee(), nullptr, OM, Stack); for (const Function &F : *M) - if (F.hasPrefixData()) - predictValueUseListOrder(F.getPrefixData(), nullptr, OM, Stack); + for (const Use &U : F.operands()) + predictValueUseListOrder(U.get(), nullptr, OM, Stack); return Stack; } diff --git a/lib/IR/Function.cpp b/lib/IR/Function.cpp index 5e4d2d2054e..b6199b827bb 100644 --- a/lib/IR/Function.cpp +++ b/lib/IR/Function.cpp @@ -279,9 +279,6 @@ Function::~Function() { // Remove the function from the on-the-side GC table. clearGC(); - - // FIXME: needed by operator delete - setFunctionNumOperands(1); } void Function::BuildLazyArguments() const { @@ -328,14 +325,15 @@ void Function::dropAllReferences() { while (!BasicBlocks.empty()) BasicBlocks.begin()->eraseFromParent(); - // Prefix and prologue data are stored in a side table. - setPrefixData(nullptr); - setPrologueData(nullptr); + // Drop uses of any optional data (real or placeholder). + if (getNumOperands()) { + User::dropAllReferences(); + setNumHungOffUseOperands(0); + setValueSubclassData(getSubclassDataFromValue() & ~0b1110); + } // Metadata is stored in a side-table. clearMetadata(); - - setPersonalityFn(nullptr); } void Function::addAttribute(unsigned i, Attribute::AttrKind attr) { @@ -425,18 +423,12 @@ void Function::copyAttributesFrom(const GlobalValue *Src) { setGC(SrcF->getGC()); else clearGC(); + if (SrcF->hasPersonalityFn()) + setPersonalityFn(SrcF->getPersonalityFn()); if (SrcF->hasPrefixData()) setPrefixData(SrcF->getPrefixData()); - else - setPrefixData(nullptr); if (SrcF->hasPrologueData()) setPrologueData(SrcF->getPrologueData()); - else - setPrologueData(nullptr); - if (SrcF->hasPersonalityFn()) - setPersonalityFn(SrcF->getPersonalityFn()); - else - setPersonalityFn(nullptr); } /// \brief This does the actual lookup of an intrinsic ID which @@ -944,61 +936,67 @@ bool Function::callsFunctionThatReturnsTwice() const { return false; } -static Constant * -getFunctionData(const Function *F, - const LLVMContextImpl::FunctionDataMapTy &Map) { - const auto &Entry = Map.find(F); - assert(Entry != Map.end()); - return cast(Entry->second->getReturnValue()); -} - -/// setFunctionData - Set "Map[F] = Data". Return an updated SubclassData value -/// in which Bit is low iff Data is null. -static unsigned setFunctionData(Function *F, - LLVMContextImpl::FunctionDataMapTy &Map, - Constant *Data, unsigned SCData, unsigned Bit) { - ReturnInst *&Holder = Map[F]; - if (Data) { - if (Holder) - Holder->setOperand(0, Data); - else - Holder = ReturnInst::Create(F->getContext(), Data); - return SCData | (1 << Bit); - } else { - delete Holder; - Map.erase(F); - return SCData & ~(1 << Bit); - } +Constant *Function::getPersonalityFn() const { + assert(hasPersonalityFn() && getNumOperands()); + return cast(Op<0>()); +} + +void Function::setPersonalityFn(Constant *Fn) { + if (Fn) + setHungoffOperand<0>(Fn); + setValueSubclassDataBit(3, Fn != nullptr); } Constant *Function::getPrefixData() const { - assert(hasPrefixData()); - return getFunctionData(this, getContext().pImpl->PrefixDataMap); + assert(hasPrefixData() && getNumOperands()); + return cast(Op<1>()); } void Function::setPrefixData(Constant *PrefixData) { - if (!PrefixData && !hasPrefixData()) - return; - - unsigned SCData = getSubclassDataFromValue(); - SCData = setFunctionData(this, getContext().pImpl->PrefixDataMap, PrefixData, - SCData, /*Bit=*/1); - setValueSubclassData(SCData); + if (PrefixData) + setHungoffOperand<1>(PrefixData); + setValueSubclassDataBit(1, PrefixData != nullptr); } Constant *Function::getPrologueData() const { - assert(hasPrologueData()); - return getFunctionData(this, getContext().pImpl->PrologueDataMap); + assert(hasPrologueData() && getNumOperands()); + return cast(Op<2>()); } void Function::setPrologueData(Constant *PrologueData) { - if (!PrologueData && !hasPrologueData()) + if (PrologueData) + setHungoffOperand<2>(PrologueData); + setValueSubclassDataBit(2, PrologueData != nullptr); +} + +void Function::allocHungoffUselist() { + // If we've already allocated a uselist, stop here. + if (getNumOperands()) return; - unsigned SCData = getSubclassDataFromValue(); - SCData = setFunctionData(this, getContext().pImpl->PrologueDataMap, - PrologueData, SCData, /*Bit=*/2); - setValueSubclassData(SCData); + allocHungoffUses(3, /*IsPhi=*/ false); + setNumHungOffUseOperands(3); + + // Initialize the uselist with placeholder operands to allow traversal. + auto *CPN = ConstantPointerNull::get(Type::getInt1PtrTy(getContext(), 0)); + Op<0>().set(CPN); + Op<1>().set(CPN); + Op<2>().set(CPN); +} + +template +void Function::setHungoffOperand(Constant *C) { + assert(C && "Cannot set hungoff operand to nullptr"); + allocHungoffUselist(); + Op().set(C); +} + +void Function::setValueSubclassDataBit(unsigned Bit, bool On) { + assert(Bit < 16 && "SubclassData contains only 16 bits"); + if (On) + setValueSubclassData(getSubclassDataFromValue() | (1 << Bit)); + else + setValueSubclassData(getSubclassDataFromValue() & ~(1 << Bit)); } void Function::setEntryCount(uint64_t Count) { @@ -1016,22 +1014,3 @@ Optional Function::getEntryCount() const { } return None; } - -void Function::setPersonalityFn(Constant *C) { - if (!C) { - if (hasPersonalityFn()) { - // Note, the num operands is used to compute the offset of the operand, so - // the order here matters. Clearing the operand then clearing the num - // operands ensures we have the correct offset to the operand. - Op<0>().set(nullptr); - setFunctionNumOperands(0); - } - } else { - // Note, the num operands is used to compute the offset of the operand, so - // the order here matters. We need to set num operands to 1 first so that - // we get the correct offset to the first operand when we set it. - if (!hasPersonalityFn()) - setFunctionNumOperands(1); - Op<0>().set(C); - } -} diff --git a/lib/IR/LLVMContextImpl.h b/lib/IR/LLVMContextImpl.h index ae987e65bcb..a24114d0a0a 100644 --- a/lib/IR/LLVMContextImpl.h +++ b/lib/IR/LLVMContextImpl.h @@ -1014,17 +1014,6 @@ public: /// instructions in different blocks at the same location. DenseMap, unsigned> DiscriminatorTable; - typedef DenseMap FunctionDataMapTy; - - /// \brief Mapping from a function to its prefix data, which is stored as the - /// operand of an unparented ReturnInst so that the prefix data has a Use. - FunctionDataMapTy PrefixDataMap; - - /// \brief Mapping from a function to its prologue data, which is stored as - /// the operand of an unparented ReturnInst so that the prologue data has a - /// Use. - FunctionDataMapTy PrologueDataMap; - int getOrAddScopeRecordIdxEntry(MDNode *N, int ExistingIdx); int getOrAddScopeInlinedAtIdxEntry(MDNode *Scope, MDNode *IA,int ExistingIdx); diff --git a/lib/IR/TypeFinder.cpp b/lib/IR/TypeFinder.cpp index 2ea0550ba45..b5bdab0865b 100644 --- a/lib/IR/TypeFinder.cpp +++ b/lib/IR/TypeFinder.cpp @@ -44,14 +44,8 @@ void TypeFinder::run(const Module &M, bool onlyNamed) { for (Module::const_iterator FI = M.begin(), E = M.end(); FI != E; ++FI) { incorporateType(FI->getType()); - if (FI->hasPrefixData()) - incorporateValue(FI->getPrefixData()); - - if (FI->hasPrologueData()) - incorporateValue(FI->getPrologueData()); - - if (FI->hasPersonalityFn()) - incorporateValue(FI->getPersonalityFn()); + for (const Use &U : FI->operands()) + incorporateValue(U.get()); // First incorporate the arguments. for (Function::const_arg_iterator AI = FI->arg_begin(), diff --git a/lib/Transforms/IPO/GlobalDCE.cpp b/lib/Transforms/IPO/GlobalDCE.cpp index 1cc657458d6..9b276ed28e2 100644 --- a/lib/Transforms/IPO/GlobalDCE.cpp +++ b/lib/Transforms/IPO/GlobalDCE.cpp @@ -215,14 +215,8 @@ void GlobalDCE::GlobalIsNeeded(GlobalValue *G) { // any globals used will be marked as needed. Function *F = cast(G); - if (F->hasPrefixData()) - MarkUsedGlobalsAsNeeded(F->getPrefixData()); - - if (F->hasPrologueData()) - MarkUsedGlobalsAsNeeded(F->getPrologueData()); - - if (F->hasPersonalityFn()) - MarkUsedGlobalsAsNeeded(F->getPersonalityFn()); + for (Use &U : F->operands()) + MarkUsedGlobalsAsNeeded(cast(U.get())); for (BasicBlock &BB : *F) for (Instruction &I : BB)