[llvm] r257139 - Remove static global GCNames from Function.cpp and move it to the Context
Mehdi Amini via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 8 11:40:27 PST 2016
> On Jan 8, 2016, at 11:37 AM, Philip Reames <listmail at philipreames.com> wrote:
>
> I'm really not sure this patch is a good idea. Introducing a mapping of Functions inside the LLVMContext doesn't seem like the best idea from an ownership perspective. Putting something like this in the Module might make sense - since the Module owns the Function - but bare pointers in a long lived structure to shorter lived objects is a bad idea.
The pointer are removed from the context when a function is destroyed.
There is no increased lifetime as a result of this patch.
>
> Please revert this patch.
It seems like a strict improvement over the previous situation, why would you want to revert this?
—
Mehdi
>
> Philip
>
> On 01/07/2016 06:28 PM, Mehdi Amini via llvm-commits wrote:
>> Author: mehdi_amini
>> Date: Thu Jan 7 20:28:20 2016
>> New Revision: 257139
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=257139&view=rev
>> Log:
>> Remove static global GCNames from Function.cpp and move it to the Context
>>
>> This remove the need for locking when deleting a function.
>>
>> Differential Revision: http://reviews.llvm.org/D15988
>>
>> From: Mehdi Amini <mehdi.amini at apple.com>
>>
>> Modified:
>> llvm/trunk/include/llvm/IR/Function.h
>> llvm/trunk/include/llvm/IR/LLVMContext.h
>> llvm/trunk/lib/IR/Core.cpp
>> llvm/trunk/lib/IR/Function.cpp
>> llvm/trunk/lib/IR/LLVMContext.cpp
>> llvm/trunk/lib/IR/LLVMContextImpl.h
>> llvm/trunk/lib/Transforms/Scalar/PlaceSafepoints.cpp
>> llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
>>
>> Modified: llvm/trunk/include/llvm/IR/Function.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Function.h?rev=257139&r1=257138&r2=257139&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/IR/Function.h (original)
>> +++ llvm/trunk/include/llvm/IR/Function.h Thu Jan 7 20:28:20 2016
>> @@ -66,7 +66,8 @@ private:
>> * bit 2 : HasPrologueData
>> * bit 3 : HasPersonalityFn
>> * bits 4-13 : CallingConvention
>> - * bits 14-15 : [reserved]
>> + * bits 14 : HasGC
>> + * bits 15 : [reserved]
>> */
>> /// Bits from GlobalObject::GlobalObjectSubclassData.
>> @@ -220,9 +221,11 @@ public:
>> /// hasGC/getGC/setGC/clearGC - The name of the garbage collection algorithm
>> /// to use during code generation.
>> - bool hasGC() const;
>> - const char *getGC() const;
>> - void setGC(const char *Str);
>> + bool hasGC() const {
>> + return getSubclassDataFromValue() & (1<<14);
>> + }
>> + const std::string &getGC() const;
>> + void setGC(const std::string Str);
>> void clearGC();
>> /// @brief adds the attribute to the list of attributes.
>>
>> Modified: llvm/trunk/include/llvm/IR/LLVMContext.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/LLVMContext.h?rev=257139&r1=257138&r2=257139&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/IR/LLVMContext.h (original)
>> +++ llvm/trunk/include/llvm/IR/LLVMContext.h Thu Jan 7 20:28:20 2016
>> @@ -93,6 +93,17 @@ public:
>> /// tag registered with an LLVMContext has an unique ID.
>> uint32_t getOperandBundleTagID(StringRef Tag) const;
>> +
>> + /// Define the GC for a function
>> + void setGC(const Function &Fn, std::string GCName);
>> +
>> + /// Return the GC for a function
>> + const std::string &getGC(const Function &Fn);
>> +
>> + /// Remove the GC for a function
>> + void deleteGC(const Function &Fn);
>> +
>> +
>> typedef void (*InlineAsmDiagHandlerTy)(const SMDiagnostic&, void *Context,
>> unsigned LocCookie);
>>
>> Modified: llvm/trunk/lib/IR/Core.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Core.cpp?rev=257139&r1=257138&r2=257139&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/IR/Core.cpp (original)
>> +++ llvm/trunk/lib/IR/Core.cpp Thu Jan 7 20:28:20 2016
>> @@ -1722,7 +1722,7 @@ void LLVMSetFunctionCallConv(LLVMValueRe
>> const char *LLVMGetGC(LLVMValueRef Fn) {
>> Function *F = unwrap<Function>(Fn);
>> - return F->hasGC()? F->getGC() : nullptr;
>> + return F->hasGC()? F->getGC().c_str() : nullptr;
>> }
>> void LLVMSetGC(LLVMValueRef Fn, const char *GC) {
>>
>> Modified: llvm/trunk/lib/IR/Function.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Function.cpp?rev=257139&r1=257138&r2=257139&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/IR/Function.cpp (original)
>> +++ llvm/trunk/lib/IR/Function.cpp Thu Jan 7 20:28:20 2016
>> @@ -366,47 +366,21 @@ void Function::addDereferenceableOrNullA
>> setAttributes(PAL);
>> }
>> -// Maintain the GC name for each function in an on-the-side table. This saves
>> -// allocating an additional word in Function for programs which do not use GC
>> -// (i.e., most programs) at the cost of increased overhead for clients which do
>> -// use GC.
>> -static DenseMap<const Function*,PooledStringPtr> *GCNames;
>> -static StringPool *GCNamePool;
>> -static ManagedStatic<sys::SmartRWMutex<true> > GCLock;
>> -
>> -bool Function::hasGC() const {
>> - sys::SmartScopedReader<true> Reader(*GCLock);
>> - return GCNames && GCNames->count(this);
>> -}
>> -
>> -const char *Function::getGC() const {
>> +const std::string &Function::getGC() const {
>> assert(hasGC() && "Function has no collector");
>> - sys::SmartScopedReader<true> Reader(*GCLock);
>> - return *(*GCNames)[this];
>> + return getContext().getGC(*this);
>> }
>> -void Function::setGC(const char *Str) {
>> - sys::SmartScopedWriter<true> Writer(*GCLock);
>> - if (!GCNamePool)
>> - GCNamePool = new StringPool();
>> - if (!GCNames)
>> - GCNames = new DenseMap<const Function*,PooledStringPtr>();
>> - (*GCNames)[this] = GCNamePool->intern(Str);
>> +void Function::setGC(const std::string Str) {
>> + setValueSubclassDataBit(14, !Str.empty());
>> + getContext().setGC(*this, std::move(Str));
>> }
>> void Function::clearGC() {
>> - sys::SmartScopedWriter<true> Writer(*GCLock);
>> - if (GCNames) {
>> - GCNames->erase(this);
>> - if (GCNames->empty()) {
>> - delete GCNames;
>> - GCNames = nullptr;
>> - if (GCNamePool->empty()) {
>> - delete GCNamePool;
>> - GCNamePool = nullptr;
>> - }
>> - }
>> - }
>> + if (!hasGC())
>> + return;
>> + getContext().deleteGC(*this);
>> + setValueSubclassDataBit(14, false);
>> }
>> /// Copy all additional attributes (those not needed to create a Function) from
>>
>> Modified: llvm/trunk/lib/IR/LLVMContext.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContext.cpp?rev=257139&r1=257138&r2=257139&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/IR/LLVMContext.cpp (original)
>> +++ llvm/trunk/lib/IR/LLVMContext.cpp Thu Jan 7 20:28:20 2016
>> @@ -304,3 +304,19 @@ void LLVMContext::getOperandBundleTags(S
>> uint32_t LLVMContext::getOperandBundleTagID(StringRef Tag) const {
>> return pImpl->getOperandBundleTagID(Tag);
>> }
>> +
>> +void LLVMContext::setGC(const Function &Fn, std::string GCName) {
>> + auto It = pImpl->GCNames.find(&Fn);
>> +
>> + if (It == pImpl->GCNames.end()) {
>> + pImpl->GCNames.insert(std::make_pair(&Fn, std::move(GCName)));
>> + return;
>> + }
>> + It->second = std::move(GCName);
>> +}
>> +const std::string &LLVMContext::getGC(const Function &Fn) {
>> + return pImpl->GCNames[&Fn];
>> +}
>> +void LLVMContext::deleteGC(const Function &Fn) {
>> + pImpl->GCNames.erase(&Fn);
>> +}
>>
>> Modified: llvm/trunk/lib/IR/LLVMContextImpl.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.h?rev=257139&r1=257138&r2=257139&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/IR/LLVMContextImpl.h (original)
>> +++ llvm/trunk/lib/IR/LLVMContextImpl.h Thu Jan 7 20:28:20 2016
>> @@ -1027,6 +1027,13 @@ public:
>> void getOperandBundleTags(SmallVectorImpl<StringRef> &Tags) const;
>> uint32_t getOperandBundleTagID(StringRef Tag) const;
>> + /// Maintain the GC name for each function.
>> + ///
>> + /// This saves allocating an additional word in Function for programs which
>> + /// do not use GC (i.e., most programs) at the cost of increased overhead for
>> + /// clients which do use GC.
>> + DenseMap<const Function*, std::string> GCNames;
>> +
>> LLVMContextImpl(LLVMContext &C);
>> ~LLVMContextImpl();
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/PlaceSafepoints.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/PlaceSafepoints.cpp?rev=257139&r1=257138&r2=257139&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/PlaceSafepoints.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/PlaceSafepoints.cpp Thu Jan 7 20:28:20 2016
>> @@ -499,7 +499,7 @@ static bool isGCSafepointPoll(Function &
>> static bool shouldRewriteFunction(Function &F) {
>> // TODO: This should check the GCStrategy
>> if (F.hasGC()) {
>> - const char *FunctionGCName = F.getGC();
>> + const auto &FunctionGCName = F.getGC();
>> const StringRef StatepointExampleName("statepoint-example");
>> const StringRef CoreCLRName("coreclr");
>> return (StatepointExampleName == FunctionGCName) ||
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp?rev=257139&r1=257138&r2=257139&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp Thu Jan 7 20:28:20 2016
>> @@ -2558,7 +2558,7 @@ void RewriteStatepointsForGC::stripNonVa
>> static bool shouldRewriteStatepointsIn(Function &F) {
>> // TODO: This should check the GCStrategy
>> if (F.hasGC()) {
>> - const char *FunctionGCName = F.getGC();
>> + const auto &FunctionGCName = F.getGC();
>> const StringRef StatepointExampleName("statepoint-example");
>> const StringRef CoreCLRName("coreclr");
>> return (StatepointExampleName == FunctionGCName) ||
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list