[llvm] r238791 - Move the name pointer out of Value into a map that lives on the
Pete Cooper
peter_cooper at apple.com
Tue Jun 9 16:22:50 PDT 2015
> On Jun 2, 2015, at 6:08 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
>
>>
>> On 02.06.2015, at 00:24, Owen Anderson <resistor at mac.com> wrote:
>>
>> Author: resistor
>> Date: Mon Jun 1 17:24:01 2015
>> New Revision: 238791
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=238791&view=rev
>> Log:
>> Move the name pointer out of Value into a map that lives on the
>> LLVMContext. Production builds of clang do not set names on most
>> Value's, so this is wasted space on almost all subclasses of Value.
>> This reduces the size of all Value subclasses by 8 bytes on 64 bit
>> hosts.
>>
>> The one tricky part of this change is averting compile time regression
>> by keeping Value::hasName() fast. This required stealing bits out of
>> NumOperands.
>>
>> With this change, peak memory usage on verify-uselistorder-nodbg.lto.bc
>> is decreased by approximately 2.3% (~3MB absolute on my machine).
>
> Nice savings. PR889 is still there if you want to shave another pointer off Value, I think the PseudoSourceValue problem is solved so this may be a matter of rewriting virtual functions into switches.
I didn’t know that PR existed. I have a patch ready to do this that I can send out soon. I just need to clean up some of the switches to use a .def file instead of lots of duplication.
Cheers,
Pete
>
> - Ben
>>
>> Modified:
>> llvm/trunk/include/llvm/IR/Value.h
>> llvm/trunk/lib/IR/LLVMContextImpl.h
>> llvm/trunk/lib/IR/Metadata.cpp
>> llvm/trunk/lib/IR/Value.cpp
>>
>> Modified: llvm/trunk/include/llvm/IR/Value.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Value.h?rev=238791&r1=238790&r2=238791&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/IR/Value.h (original)
>> +++ llvm/trunk/include/llvm/IR/Value.h Mon Jun 1 17:24:01 2015
>> @@ -69,9 +69,8 @@ class Value {
>> Type *VTy;
>> Use *UseList;
>>
>> - friend class ValueAsMetadata; // Allow access to NameAndIsUsedByMD.
>> + friend class ValueAsMetadata; // Allow access to IsUsedByMD.
>> friend class ValueHandleBase;
>> - PointerIntPair<ValueName *, 1> NameAndIsUsedByMD;
>>
>> const unsigned char SubclassID; // Subclass identifier (for isa/dyn_cast)
>> unsigned char HasValueHandle : 1; // Has a ValueHandle pointing to this?
>> @@ -101,7 +100,10 @@ protected:
>> /// This is stored here to save space in User on 64-bit hosts. Since most
>> /// instances of Value have operands, 32-bit hosts aren't significantly
>> /// affected.
>> - unsigned NumOperands;
>> + unsigned NumOperands : 30;
>> +
>> + bool IsUsedByMD : 1;
>> + bool HasName : 1;
>>
>> private:
>> template <typename UseT> // UseT == 'Use' or 'const Use'
>> @@ -210,9 +212,9 @@ public:
>> LLVMContext &getContext() const;
>>
>> // \brief All values can potentially be named.
>> - bool hasName() const { return getValueName() != nullptr; }
>> - ValueName *getValueName() const { return NameAndIsUsedByMD.getPointer(); }
>> - void setValueName(ValueName *VN) { NameAndIsUsedByMD.setPointer(VN); }
>> + bool hasName() const { return HasName; }
>> + ValueName *getValueName() const;
>> + void setValueName(ValueName *VN);
>>
>> private:
>> void destroyValueName();
>> @@ -394,7 +396,7 @@ public:
>> bool hasValueHandle() const { return HasValueHandle; }
>>
>> /// \brief Return true if there is metadata referencing this value.
>> - bool isUsedByMetadata() const { return NameAndIsUsedByMD.getInt(); }
>> + bool isUsedByMetadata() const { return IsUsedByMD; }
>>
>> /// \brief Strip off pointer casts, all-zero GEPs, and aliases.
>> ///
>>
>> Modified: llvm/trunk/lib/IR/LLVMContextImpl.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.h?rev=238791&r1=238790&r2=238791&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/IR/LLVMContextImpl.h (original)
>> +++ llvm/trunk/lib/IR/LLVMContextImpl.h Mon Jun 1 17:24:01 2015
>> @@ -922,6 +922,8 @@ public:
>> DenseMap<Value *, ValueAsMetadata *> ValuesAsMetadata;
>> DenseMap<Metadata *, MetadataAsValue *> MetadataAsValues;
>>
>> + DenseMap<const Value*, ValueName*> ValueNames;
>> +
>> #define HANDLE_MDNODE_LEAF(CLASS) DenseSet<CLASS *, CLASS##Info> CLASS##s;
>> #include "llvm/IR/Metadata.def"
>>
>>
>> Modified: llvm/trunk/lib/IR/Metadata.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=238791&r1=238790&r2=238791&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/IR/Metadata.cpp (original)
>> +++ llvm/trunk/lib/IR/Metadata.cpp Mon Jun 1 17:24:01 2015
>> @@ -256,9 +256,9 @@ ValueAsMetadata *ValueAsMetadata::get(Va
>> if (!Entry) {
>> assert((isa<Constant>(V) || isa<Argument>(V) || isa<Instruction>(V)) &&
>> "Expected constant or function-local value");
>> - assert(!V->NameAndIsUsedByMD.getInt() &&
>> + assert(!V->IsUsedByMD &&
>> "Expected this to be the only metadata use");
>> - V->NameAndIsUsedByMD.setInt(true);
>> + V->IsUsedByMD = true;
>> if (auto *C = dyn_cast<Constant>(V))
>> Entry = new ConstantAsMetadata(C);
>> else
>> @@ -302,15 +302,15 @@ void ValueAsMetadata::handleRAUW(Value *
>> auto &Store = Context.pImpl->ValuesAsMetadata;
>> auto I = Store.find(From);
>> if (I == Store.end()) {
>> - assert(!From->NameAndIsUsedByMD.getInt() &&
>> + assert(!From->IsUsedByMD &&
>> "Expected From not to be used by metadata");
>> return;
>> }
>>
>> // Remove old entry from the map.
>> - assert(From->NameAndIsUsedByMD.getInt() &&
>> + assert(From->IsUsedByMD &&
>> "Expected From to be used by metadata");
>> - From->NameAndIsUsedByMD.setInt(false);
>> + From->IsUsedByMD = false;
>> ValueAsMetadata *MD = I->second;
>> assert(MD && "Expected valid metadata");
>> assert(MD->getValue() == From && "Expected valid mapping");
>> @@ -346,9 +346,9 @@ void ValueAsMetadata::handleRAUW(Value *
>> }
>>
>> // Update MD in place (and update the map entry).
>> - assert(!To->NameAndIsUsedByMD.getInt() &&
>> + assert(!To->IsUsedByMD &&
>> "Expected this to be the only metadata use");
>> - To->NameAndIsUsedByMD.setInt(true);
>> + To->IsUsedByMD = true;
>> MD->V = To;
>> Entry = MD;
>> }
>>
>> Modified: llvm/trunk/lib/IR/Value.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Value.cpp?rev=238791&r1=238790&r2=238791&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/IR/Value.cpp (original)
>> +++ llvm/trunk/lib/IR/Value.cpp Mon Jun 1 17:24:01 2015
>> @@ -46,8 +46,9 @@ static inline Type *checkType(Type *Ty)
>> }
>>
>> Value::Value(Type *ty, unsigned scid)
>> - : VTy(checkType(ty)), UseList(nullptr), SubclassID(scid), HasValueHandle(0),
>> - SubclassOptionalData(0), SubclassData(0), NumOperands(0) {
>> + : VTy(checkType(ty)), UseList(nullptr), SubclassID(scid),
>> + HasValueHandle(0), SubclassOptionalData(0), SubclassData(0),
>> + NumOperands(0), IsUsedByMD(false), HasName(false) {
>> // FIXME: Why isn't this in the subclass gunk??
>> // Note, we cannot call isa<CallInst> before the CallInst has been
>> // constructed.
>> @@ -157,11 +158,39 @@ static bool getSymTab(Value *V, ValueSym
>> return false;
>> }
>>
>> +ValueName *Value::getValueName() const {
>> + if (!HasName) return nullptr;
>> +
>> + LLVMContext &Ctx = getContext();
>> + auto I = Ctx.pImpl->ValueNames.find(this);
>> + assert(I != Ctx.pImpl->ValueNames.end() &&
>> + "No name entry found!");
>> +
>> + return I->second;
>> +}
>> +
>> +void Value::setValueName(ValueName *VN) {
>> + LLVMContext &Ctx = getContext();
>> +
>> + assert(HasName == Ctx.pImpl->ValueNames.count(this) &&
>> + "HasName bit out of sync!");
>> +
>> + if (!VN) {
>> + if (HasName)
>> + Ctx.pImpl->ValueNames.erase(this);
>> + HasName = false;
>> + return;
>> + }
>> +
>> + HasName = true;
>> + Ctx.pImpl->ValueNames[this] = VN;
>> +}
>> +
>> StringRef Value::getName() const {
>> // Make sure the empty string is still a C string. For historical reasons,
>> // some clients want to call .data() on the result and expect it to be null
>> // terminated.
>> - if (!getValueName())
>> + if (!hasName())
>> return StringRef("", 0);
>> return getValueName()->getKey();
>> }
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150609/d445635a/attachment.html>
More information about the llvm-commits
mailing list