[llvm-commits] CVS: llvm/lib/VMCore/BasicBlock.cpp Function.cpp Instruction.cpp SymbolTableListTraitsImpl.h Value.cpp ValueSymbolTable.cpp Verifier.cpp
Chris Lattner
sabre at nondot.org
Sun Feb 11 21:18:32 PST 2007
Changes in directory llvm/lib/VMCore:
BasicBlock.cpp updated: 1.74 -> 1.75
Function.cpp updated: 1.112 -> 1.113
Instruction.cpp updated: 1.63 -> 1.64
SymbolTableListTraitsImpl.h updated: 1.9 -> 1.10
Value.cpp updated: 1.65 -> 1.66
ValueSymbolTable.cpp updated: 1.10 -> 1.11
Verifier.cpp updated: 1.194 -> 1.195
---
Log message:
Switch ValueSymbolTable to use StringMap<Value*> instead of std::map<std::string, Value*>
as its main datastructure. There are many improvements yet to be made, but
this speeds up opt --std-compile-opts on 447.dealII by 7.3%.
---
Diffs of the changes: (+132 -85)
BasicBlock.cpp | 4 +
Function.cpp | 3 -
Instruction.cpp | 6 +-
SymbolTableListTraitsImpl.h | 12 ++---
Value.cpp | 69 +++++++++++++++++++++--------
ValueSymbolTable.cpp | 102 ++++++++++++++++++++++++++++----------------
Verifier.cpp | 21 ---------
7 files changed, 132 insertions(+), 85 deletions(-)
Index: llvm/lib/VMCore/BasicBlock.cpp
diff -u llvm/lib/VMCore/BasicBlock.cpp:1.74 llvm/lib/VMCore/BasicBlock.cpp:1.75
--- llvm/lib/VMCore/BasicBlock.cpp:1.74 Fri Sep 22 23:03:45 2006
+++ llvm/lib/VMCore/BasicBlock.cpp Sun Feb 11 23:18:08 2007
@@ -62,7 +62,7 @@
BasicBlock::BasicBlock(const std::string &Name, Function *Parent,
BasicBlock *InsertBefore)
- : Value(Type::LabelTy, Value::BasicBlockVal, Name) {
+ : Value(Type::LabelTy, Value::BasicBlockVal) {
// Initialize the instlist...
InstList.setItemParent(this);
@@ -76,6 +76,8 @@
} else if (Parent) {
Parent->getBasicBlockList().push_back(this);
}
+
+ setName(Name);
}
Index: llvm/lib/VMCore/Function.cpp
diff -u llvm/lib/VMCore/Function.cpp:1.112 llvm/lib/VMCore/Function.cpp:1.113
--- llvm/lib/VMCore/Function.cpp:1.112 Wed Feb 7 14:38:26 2007
+++ llvm/lib/VMCore/Function.cpp Sun Feb 11 23:18:08 2007
@@ -51,7 +51,7 @@
//===----------------------------------------------------------------------===//
Argument::Argument(const Type *Ty, const std::string &Name, Function *Par)
- : Value(Ty, Value::ArgumentVal, Name) {
+ : Value(Ty, Value::ArgumentVal) {
Parent = 0;
// Make sure that we get added to a function
@@ -59,6 +59,7 @@
if (Par)
Par->getArgumentList().push_back(this);
+ setName(Name);
}
void Argument::setParent(Function *parent) {
Index: llvm/lib/VMCore/Instruction.cpp
diff -u llvm/lib/VMCore/Instruction.cpp:1.63 llvm/lib/VMCore/Instruction.cpp:1.64
--- llvm/lib/VMCore/Instruction.cpp:1.63 Mon Feb 5 14:47:20 2007
+++ llvm/lib/VMCore/Instruction.cpp Sun Feb 11 23:18:08 2007
@@ -19,7 +19,7 @@
Instruction::Instruction(const Type *ty, unsigned it, Use *Ops, unsigned NumOps,
const std::string &Name, Instruction *InsertBefore)
- : User(ty, Value::InstructionVal + it, Ops, NumOps, Name), Parent(0) {
+ : User(ty, Value::InstructionVal + it, Ops, NumOps), Parent(0) {
// Make sure that we get added to a basicblock
LeakDetector::addGarbageObject(this);
@@ -29,17 +29,19 @@
"Instruction to insert before is not in a basic block!");
InsertBefore->getParent()->getInstList().insert(InsertBefore, this);
}
+ setName(Name);
}
Instruction::Instruction(const Type *ty, unsigned it, Use *Ops, unsigned NumOps,
const std::string &Name, BasicBlock *InsertAtEnd)
- : User(ty, Value::InstructionVal + it, Ops, NumOps, Name), Parent(0) {
+ : User(ty, Value::InstructionVal + it, Ops, NumOps), Parent(0) {
// Make sure that we get added to a basicblock
LeakDetector::addGarbageObject(this);
// append this instruction into the basic block
assert(InsertAtEnd && "Basic block to append to may not be NULL!");
InsertAtEnd->getInstList().push_back(this);
+ setName(Name);
}
// Out of line virtual method, so the vtable, etc has a home.
Index: llvm/lib/VMCore/SymbolTableListTraitsImpl.h
diff -u llvm/lib/VMCore/SymbolTableListTraitsImpl.h:1.9 llvm/lib/VMCore/SymbolTableListTraitsImpl.h:1.10
--- llvm/lib/VMCore/SymbolTableListTraitsImpl.h:1.9 Mon Feb 5 14:47:20 2007
+++ llvm/lib/VMCore/SymbolTableListTraitsImpl.h Sun Feb 11 23:18:08 2007
@@ -32,7 +32,7 @@
ValueSymbolTable &SymTab = SymTabObject->getValueSymbolTable();
for (typename iplist<ValueSubClass>::iterator I = List.begin();
I != List.end(); ++I)
- if (I->hasName()) SymTab.remove(I);
+ if (I->hasName()) SymTab.removeValueName(I->getValueName());
}
SymTabObject = STO;
@@ -42,7 +42,7 @@
ValueSymbolTable &SymTab = SymTabObject->getValueSymbolTable();
for (typename iplist<ValueSubClass>::iterator I = List.begin();
I != List.end(); ++I)
- if (I->hasName()) SymTab.insert(I);
+ if (I->hasName()) SymTab.reinsertValue(I);
}
}
@@ -53,7 +53,7 @@
assert(V->getParent() == 0 && "Value already in a container!!");
V->setParent(ItemParent);
if (V->hasName() && SymTabObject)
- SymTabObject->getValueSymbolTable().insert(V);
+ SymTabObject->getValueSymbolTable().reinsertValue(V);
}
template<typename ValueSubClass, typename ItemParentClass, typename SymTabClass,
@@ -62,7 +62,7 @@
::removeNodeFromList(ValueSubClass *V) {
V->setParent(0);
if (V->hasName() && SymTabObject)
- SymTabObject->getValueSymbolTable().remove(V);
+ SymTabObject->getValueSymbolTable().removeValueName(V->getValueName());
}
template<typename ValueSubClass, typename ItemParentClass, typename SymTabClass,
@@ -83,10 +83,10 @@
ValueSubClass &V = *first;
bool HasName = V.hasName();
if (OldSTO && HasName)
- OldSTO->getValueSymbolTable().remove(&V);
+ OldSTO->getValueSymbolTable().removeValueName(V.getValueName());
V.setParent(NewIP);
if (NewSTO && HasName)
- NewSTO->getValueSymbolTable().insert(&V);
+ NewSTO->getValueSymbolTable().reinsertValue(&V);
}
} else {
// Just transferring between blocks in the same function, simply update the
Index: llvm/lib/VMCore/Value.cpp
diff -u llvm/lib/VMCore/Value.cpp:1.65 llvm/lib/VMCore/Value.cpp:1.66
--- llvm/lib/VMCore/Value.cpp:1.65 Sun Feb 11 13:12:18 2007
+++ llvm/lib/VMCore/Value.cpp Sun Feb 11 23:18:08 2007
@@ -30,15 +30,13 @@
return Ty;
}
-Value::Value(const Type *ty, unsigned scid, const std::string &name)
+Value::Value(const Type *ty, unsigned scid)
: SubclassID(scid), SubclassData(0), Ty(checkType(ty)),
- UseList(0), Name(name) {
+ UseList(0), Name(0) {
if (!isa<Constant>(this) && !isa<BasicBlock>(this))
assert((Ty->isFirstClassType() || Ty == Type::VoidTy ||
isa<OpaqueType>(ty)) &&
"Cannot create non-first-class values except for constants!");
- if (ty == Type::VoidTy)
- assert(name.empty() && "Cannot have named void values!");
}
Value::~Value() {
@@ -114,29 +112,62 @@
return false;
}
-void Value::setName(const std::string &name) {
- if (Name == name) return; // Name is already set.
+std::string Value::getName() const {
+ if (Name == 0) return "";
+ return std::string(Name->getKeyData(),
+ Name->getKeyData()+Name->getKeyLength());
+}
+void Value::setName(const std::string &name) {
+ if (name.empty() && !hasName()) return;
+ if (getType() != Type::VoidTy && "Cannot assign a name to void values!");
+
+
// Get the symbol table to update for this object.
ValueSymbolTable *ST;
if (getSymTab(this, ST))
return; // Cannot set a name on this value (e.g. constant).
- if (!ST) // No symbol table to update? Just do the change.
- Name = name;
- else if (hasName()) {
- if (!name.empty()) { // Replacing name.
- ST->remove(this);
- Name = name;
- ST->insert(this);
- } else { // Transitioning from hasName -> noname.
- ST->remove(this);
- Name.clear();
+ if (!ST) { // No symbol table to update? Just do the change.
+ if (name.empty()) {
+ // Free the name for this value.
+ Name->Destroy();
+ Name = 0;
+ } else {
+ if (Name) {
+ // Name isn't changing.
+ if (name.size() == Name->getKeyLength() &&
+ !memcmp(Name->getKeyData(), &name[0], name.size()))
+ return;
+ Name->Destroy();
+ }
+
+ // Create the new name.
+ Name = ValueName::Create(&name[0], &name[name.size()]);
+ Name->setValue(this);
}
- } else { // Transitioning from noname -> hasName.
- Name = name;
- ST->insert(this);
+ return;
}
+
+ // NOTE: Could optimize for the case the name is shrinking to not deallocate
+ // then reallocated.
+ if (hasName()) {
+ // Name isn't changing?
+ if (name.size() == Name->getKeyLength() &&
+ !memcmp(Name->getKeyData(), &name[0], name.size()))
+ return;
+
+ // Remove old name.
+ ST->removeValueName(Name);
+ Name->Destroy();
+ Name = 0;
+
+ if (name.empty())
+ return;
+ }
+
+ // Name is changing to something new.
+ Name = ST->createValueName(&name[0], name.size(), this);
}
/// takeName - transfer the name from V to this value, setting V's name to
Index: llvm/lib/VMCore/ValueSymbolTable.cpp
diff -u llvm/lib/VMCore/ValueSymbolTable.cpp:1.10 llvm/lib/VMCore/ValueSymbolTable.cpp:1.11
--- llvm/lib/VMCore/ValueSymbolTable.cpp:1.10 Wed Feb 7 00:28:48 2007
+++ llvm/lib/VMCore/ValueSymbolTable.cpp Sun Feb 11 23:18:08 2007
@@ -17,7 +17,6 @@
#include "llvm/ValueSymbolTable.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Debug.h"
-#include <algorithm>
using namespace llvm;
// Class destructor
@@ -25,8 +24,8 @@
#ifndef NDEBUG // Only do this in -g mode...
for (iterator VI = vmap.begin(), VE = vmap.end(); VI != VE; ++VI)
DEBUG(DOUT << "Value still in symbol table! Type = '"
- << VI->second->getType()->getDescription() << "' Name = '"
- << VI->first << "'\n");
+ << VI->getValue()->getType()->getDescription() << "' Name = '"
+ << VI->getKeyData() << "'\n");
assert(vmap.empty() && "Values remain in symbol table!");
#endif
}
@@ -37,11 +36,11 @@
//
std::string ValueSymbolTable::getUniqueName(const std::string &BaseName) const {
std::string TryName = BaseName;
- const_iterator End = vmap.end();
// See if the name exists
- while (vmap.find(TryName) != End) // Loop until we find a free
- TryName = BaseName + utostr(++LastUnique); // name in the symbol table
+ while (vmap.find(&TryName[0], &TryName[TryName.size()]) != vmap.end())
+ // Loop until we find a free name in the symbol table.
+ TryName = BaseName + utostr(++LastUnique);
return TryName;
}
@@ -49,62 +48,95 @@
// lookup a value - Returns null on failure...
//
Value *ValueSymbolTable::lookup(const std::string &Name) const {
- const_iterator VI = vmap.find(Name);
+ const_iterator VI = vmap.find(&Name[0], &Name[Name.size()]);
if (VI != vmap.end()) // We found the symbol
- return const_cast<Value*>(VI->second);
+ return VI->getValue();
return 0;
}
// Insert a value into the symbol table with the specified name...
//
-void ValueSymbolTable::insert(Value* V) {
- assert(V && "Can't insert null Value into symbol table!");
+void ValueSymbolTable::reinsertValue(Value* V) {
assert(V->hasName() && "Can't insert nameless Value into symbol table");
// Try inserting the name, assuming it won't conflict.
- if (vmap.insert(make_pair(V->Name, V)).second) {
+ if (vmap.insert(V->Name)) {
DOUT << " Inserted value: " << V->Name << ": " << *V << "\n";
return;
}
+ // FIXME: this could be much more efficient.
+
// Otherwise, there is a naming conflict. Rename this value.
std::string UniqueName = V->getName();
+
+ V->Name->Destroy();
+
unsigned BaseSize = UniqueName.size();
- do {
+ while (1) {
// Trim any suffix off.
UniqueName.resize(BaseSize);
UniqueName += utostr(++LastUnique);
// Try insert the vmap entry with this suffix.
- } while (!vmap.insert(make_pair(UniqueName, V)).second);
-
- V->Name = UniqueName;
-
- DEBUG(DOUT << " Inserted value: " << UniqueName << ": " << *V << "\n");
+ ValueName &NewName = vmap.GetOrCreateValue(&UniqueName[0],
+ &UniqueName[UniqueName.size()]);
+ if (NewName.getValue() == 0) {
+ // Newly inserted name. Success!
+ NewName.setValue(V);
+ V->Name = &NewName;
+ DEBUG(DOUT << " Inserted value: " << UniqueName << ": " << *V << "\n");
+ return;
+ }
+ }
}
-// Remove a value
-void ValueSymbolTable::remove(Value *V) {
- assert(V->hasName() && "Value doesn't have name!");
- iterator Entry = vmap.find(V->getName());
- assert(Entry != vmap.end() && "Entry was not in the symtab!");
-
- DEBUG(DOUT << " Removing Value: " << Entry->second->getName() << "\n");
-
- // Remove the value from the plane...
- vmap.erase(Entry);
+void ValueSymbolTable::removeValueName(ValueName *V) {
+ DEBUG(DOUT << " Removing Value: " << V->getKeyData() << "\n");
+ // Remove the value from the plane.
+ vmap.remove(V);
+}
+
+/// createValueName - This method attempts to create a value name and insert
+/// it into the symbol table with the specified name. If it conflicts, it
+/// auto-renames the name and returns that instead.
+ValueName *ValueSymbolTable::createValueName(const char *NameStart,
+ unsigned NameLen, Value *V) {
+ ValueName &Entry = vmap.GetOrCreateValue(NameStart, NameStart+NameLen);
+ if (Entry.getValue() == 0) {
+ Entry.setValue(V);
+ DEBUG(DOUT << " Inserted value: " << Entry.getKeyData() << ": "
+ << *V << "\n");
+ return &Entry;
+ }
+
+ // FIXME: this could be much more efficient.
+
+ // Otherwise, there is a naming conflict. Rename this value.
+ std::string UniqueName(NameStart, NameStart+NameLen);
+ while (1) {
+ // Trim any suffix off.
+ UniqueName.resize(NameLen);
+ UniqueName += utostr(++LastUnique);
+ // Try insert the vmap entry with this suffix.
+ ValueName &NewName = vmap.GetOrCreateValue(&UniqueName[0],
+ &UniqueName[UniqueName.size()]);
+ if (NewName.getValue() == 0) {
+ // Newly inserted name. Success!
+ NewName.setValue(V);
+ DEBUG(DOUT << " Inserted value: " << UniqueName << ": " << *V << "\n");
+ return &NewName;
+ }
+ }
}
-// DumpVal - a std::for_each function for dumping a value
-//
-static void DumpVal(const std::pair<const std::string, Value *> &V) {
- DOUT << " '" << V.first << "' = ";
- V.second->dump();
- DOUT << "\n";
-}
// dump - print out the symbol table
//
void ValueSymbolTable::dump() const {
DOUT << "ValueSymbolTable:\n";
- for_each(vmap.begin(), vmap.end(), DumpVal);
+ for (const_iterator I = begin(), E = end(); I != E; ++I) {
+ DOUT << " '" << I->getKeyData() << "' = ";
+ I->getValue()->dump();
+ DOUT << "\n";
+ }
}
Index: llvm/lib/VMCore/Verifier.cpp
diff -u llvm/lib/VMCore/Verifier.cpp:1.194 llvm/lib/VMCore/Verifier.cpp:1.195
--- llvm/lib/VMCore/Verifier.cpp:1.194 Sat Feb 10 02:33:11 2007
+++ llvm/lib/VMCore/Verifier.cpp Sun Feb 11 23:18:08 2007
@@ -51,7 +51,6 @@
#include "llvm/Instructions.h"
#include "llvm/Intrinsics.h"
#include "llvm/PassManager.h"
-#include "llvm/ValueSymbolTable.h"
#include "llvm/Analysis/Dominators.h"
#include "llvm/Support/CFG.h"
#include "llvm/Support/InstVisitor.h"
@@ -102,7 +101,6 @@
bool doInitialization(Module &M) {
Mod = &M;
verifyTypeSymbolTable(M.getTypeSymbolTable());
- verifyValueSymbolTable(M.getValueSymbolTable());
// If this is a real pass, in a pass manager, we must abort before
// returning back to the pass manager, or else the pass manager may try to
@@ -177,7 +175,6 @@
// Verification methods...
void verifyTypeSymbolTable(TypeSymbolTable &ST);
- void verifyValueSymbolTable(ValueSymbolTable &ST);
void visitGlobalValue(GlobalValue &GV);
void visitGlobalVariable(GlobalVariable &GV);
void visitFunction(Function &F);
@@ -307,22 +304,6 @@
void Verifier::verifyTypeSymbolTable(TypeSymbolTable &ST) {
}
-// verifySymbolTable - Verify that a function or module symbol table is ok
-//
-void Verifier::verifyValueSymbolTable(ValueSymbolTable &ST) {
-
- // Loop over all of the values in the symbol table.
- for (ValueSymbolTable::const_iterator VI = ST.begin(), VE = ST.end();
- VI != VE; ++VI) {
- Value *V = VI->second;
- // Check that there are no void typed values in the symbol table. Values
- // with a void type cannot be put into symbol tables because they cannot
- // have names!
- Assert1(V->getType() != Type::VoidTy,
- "Values with void type are not allowed to have names!", V);
- }
-}
-
// visitFunction - Verify that a function is ok.
//
void Verifier::visitFunction(Function &F) {
@@ -375,8 +356,6 @@
Assert1(F.getName().substr(0, 5) != "llvm.",
"llvm intrinsics cannot be defined!", &F);
- verifyValueSymbolTable(F.getValueSymbolTable());
-
// Check the entry node
BasicBlock *Entry = &F.getEntryBlock();
Assert1(pred_begin(Entry) == pred_end(Entry),
More information about the llvm-commits
mailing list