[llvm-commits] CVS: llvm/lib/VMCore/Constants.cpp
Chris Lattner
lattner at cs.uiuc.edu
Tue Oct 4 10:48:58 PDT 2005
Changes in directory llvm/lib/VMCore:
Constants.cpp updated: 1.134 -> 1.135
---
Log message:
For large constants (e.g. arrays and structs with many elements) just
creating the keys and doing comparisons to index into 'Map' takes a lot
of time. For these large constants, keep an inverse map so that 'remove'
and move operations are much faster.
This speeds up a release build of the bc reader on Eric's nasty python
bytecode file from 1:39 to 1:00s.
---
Diffs of the changes: (+56 -23)
Constants.cpp | 79 +++++++++++++++++++++++++++++++++++++++++-----------------
1 files changed, 56 insertions(+), 23 deletions(-)
Index: llvm/lib/VMCore/Constants.cpp
diff -u llvm/lib/VMCore/Constants.cpp:1.134 llvm/lib/VMCore/Constants.cpp:1.135
--- llvm/lib/VMCore/Constants.cpp:1.134 Tue Oct 4 11:52:46 2005
+++ llvm/lib/VMCore/Constants.cpp Tue Oct 4 12:48:46 2005
@@ -513,7 +513,8 @@
}
namespace {
- template<class ValType, class TypeClass, class ConstantClass>
+ template<class ValType, class TypeClass, class ConstantClass,
+ bool HasLargeKey = false /*true for arrays and structs*/ >
class ValueMap : public AbstractTypeUser {
public:
typedef std::pair<const TypeClass*, ValType> MapKey;
@@ -524,6 +525,12 @@
/// This is the primary way we avoid creating two of the same shape
/// constant.
MapTy Map;
+
+ /// InverseMap - If "HasLargeKey" is true, this contains an inverse mapping
+ /// from the constants to their element in Map. This is important for
+ /// removal of constants from the array, which would otherwise have to scan
+ /// through the map with very large keys.
+ std::map<ConstantClass*, MapIterator> InverseMap;
typedef std::map<const TypeClass*, MapIterator> AbstractTypeMapTy;
AbstractTypeMapTy AbstractTypeMap;
@@ -535,11 +542,19 @@
Constants.push_back(I->second);
Map.clear();
AbstractTypeMap.clear();
+ InverseMap.clear();
}
public:
MapIterator map_end() { return Map.end(); }
+ void UpdateInverseMap(ConstantClass *C, MapIterator I) {
+ if (HasLargeKey) {
+ assert(I->second == C && "Bad inversemap entry!");
+ InverseMap[C] = I;
+ }
+ }
+
/// InsertOrGetItem - Return an iterator for the specified element.
/// If the element exists in the map, the returned iterator points to the
/// entry and Exists=true. If not, the iterator points to the newly
@@ -552,19 +567,35 @@
return IP.first;
}
- /// SimpleRemove - This method removes the specified constant from the map,
- /// without updating type information. This should only be used when we're
- /// changing an element in the map, making this the second half of a 'move'
- /// operation.
- void SimpleRemove(ConstantClass *CP) {
- MapIterator I = Map.find(MapKey((TypeClass*)CP->getRawType(),
- getValType(CP)));
+private:
+ MapIterator FindExistingElement(ConstantClass *CP) {
+ if (HasLargeKey) {
+ typename std::map<ConstantClass*, MapIterator>::iterator
+ IMI = InverseMap.find(CP);
+ assert(IMI != InverseMap.end() && IMI->second != Map.end() &&
+ IMI->second->second == CP &&
+ "InverseMap corrupt!");
+ return IMI->second;
+ }
+
+ MapIterator I =
+ Map.find(MapKey((TypeClass*)CP->getRawType(), getValType(CP)));
if (I == Map.end() || I->second != CP) {
// FIXME: This should not use a linear scan. If this gets to be a
// performance problem, someone should look at this.
for (I = Map.begin(); I != Map.end() && I->second != CP; ++I)
/* empty */;
}
+ return I;
+ }
+public:
+
+ /// SimpleRemove - This method removes the specified constant from the map,
+ /// without updating type information. This should only be used when we're
+ /// changing an element in the map, making this the second half of a 'move'
+ /// operation.
+ void SimpleRemove(ConstantClass *CP) {
+ MapIterator I = FindExistingElement(CP);
assert(I != Map.end() && "Constant not found in constant table!");
assert(I->second == CP && "Didn't find correct element?");
Map.erase(I);
@@ -586,6 +617,9 @@
//assert(Result->getType() == Ty && "Type specified is not correct!");
I = Map.insert(I, std::make_pair(MapKey(Ty, V), Result));
+ if (HasLargeKey) // Remember the reverse mapping if needed.
+ InverseMap.insert(std::make_pair(Result, I));
+
// If the type of the constant is abstract, make sure that an entry exists
// for it in the AbstractTypeMap.
if (Ty->isAbstract()) {
@@ -603,18 +637,13 @@
}
void remove(ConstantClass *CP) {
- MapIterator I = Map.find(MapKey((TypeClass*)CP->getRawType(),
- getValType(CP)));
- if (I == Map.end() || I->second != CP) {
- // FIXME: This should not use a linear scan. If this gets to be a
- // performance problem, someone should look at this.
- for (I = Map.begin(); I != Map.end() && I->second != CP; ++I)
- /* empty */;
- }
-
+ MapIterator I = FindExistingElement(CP);
assert(I != Map.end() && "Constant not found in constant table!");
assert(I->second == CP && "Didn't find correct element?");
+ if (HasLargeKey) // Remember the reverse mapping if needed.
+ InverseMap.erase(CP);
+
// Now that we found the entry, make sure this isn't the entry that
// the AbstractTypeMap points to.
const TypeClass *Ty = I->first.first;
@@ -821,7 +850,7 @@
}
typedef ValueMap<std::vector<Constant*>, ArrayType,
- ConstantArray> ArrayConstantsTy;
+ ConstantArray, true /*largekey*/> ArrayConstantsTy;
static ArrayConstantsTy ArrayConstants;
Constant *ConstantArray::get(const ArrayType *Ty,
@@ -911,7 +940,7 @@
}
typedef ValueMap<std::vector<Constant*>, StructType,
- ConstantStruct> StructConstantsTy;
+ ConstantStruct, true /*largekey*/> StructConstantsTy;
static StructConstantsTy StructConstants;
static std::vector<Constant*> getValType(ConstantStruct *CS) {
@@ -1402,9 +1431,11 @@
// creating a new constant array, inserting it, replaceallusesof'ing the
// old with the new, then deleting the old... just update the current one
// in place!
- if (I != ArrayConstants.map_end() && I->second == this)
- ++I; // Do not invalidate iterator!
ArrayConstants.SimpleRemove(this); // Remove old shape from the map.
+
+ // Update the inverse map so that we know that this constant is now
+ // located at descriptor I.
+ ArrayConstants.UpdateInverseMap(this, I);
// Update to the new values.
for (unsigned i = 0, e = getNumOperands(); i != e; ++i)
@@ -1464,9 +1495,11 @@
// creating a new constant struct, inserting it, replaceallusesof'ing the
// old with the new, then deleting the old... just update the current one
// in place!
- if (I != StructConstants.map_end() && I->second == this)
- ++I; // Do not invalidate iterator!
StructConstants.SimpleRemove(this); // Remove old shape from the map.
+
+ // Update the inverse map so that we know that this constant is now
+ // located at descriptor I.
+ StructConstants.UpdateInverseMap(this, I);
// Update to the new values.
for (unsigned i = 0, e = getNumOperands(); i != e; ++i)
More information about the llvm-commits
mailing list