[llvm] r238791 - Move the name pointer out of Value into a map that lives on the
Benjamin Kramer
benny.kra at gmail.com
Tue Jun 2 06:08:37 PDT 2015
> 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.
- 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
More information about the llvm-commits
mailing list