[llvm-commits] [llvm] r81861 - in /llvm/trunk: include/llvm/AbstractTypeUser.h include/llvm/Value.h lib/VMCore/Constants.cpp lib/VMCore/ConstantsContext.h lib/VMCore/Type.cpp test/Linker/partial-type-refinement-link.ll test/Linker/partial-type-refinement.ll

Dan Gohman gohman at apple.com
Tue Sep 15 08:58:07 PDT 2009


Author: djg
Date: Tue Sep 15 10:58:07 2009
New Revision: 81861

URL: http://llvm.org/viewvc/llvm-project?rev=81861&view=rev
Log:
When a constant's type is refined, update the constant in place
instead of cloning and RAUWing it.

 - Make AbstractTypeUser a friend of Value so that it can offer
   its subclasses a way to update a Value's type in place. This
   is better than a universally visible setType method on Value,
   and it's sufficient for the immediate need.

 - Eliminate the constant "convert" functions. This eliminates a
   lot of logic duplication, and fixes a complicated bug where a
   constant can't actually be cloned during the type refinement
   process because some of the types that its folder needs are
   half-destroyed, being in the middle of refinement themselves.

 - Move the getValType functions from being static overloaded
   functions in Constants.cpp to be members of class template
   specializations in ConstantsContext.h. This means that the
   code ends up getting instantiated twice, however it also
   makes it possible to eliminate all "convert" functions, so
   it's not a big net code size increase. And if desired, the
   duplicate instantiations could be eliminated with some
   reorganization.

Added:
    llvm/trunk/test/Linker/partial-type-refinement-link.ll
    llvm/trunk/test/Linker/partial-type-refinement.ll
Modified:
    llvm/trunk/include/llvm/AbstractTypeUser.h
    llvm/trunk/include/llvm/Value.h
    llvm/trunk/lib/VMCore/Constants.cpp
    llvm/trunk/lib/VMCore/ConstantsContext.h
    llvm/trunk/lib/VMCore/Type.cpp

Modified: llvm/trunk/include/llvm/AbstractTypeUser.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/AbstractTypeUser.h?rev=81861&r1=81860&r2=81861&view=diff

==============================================================================
--- llvm/trunk/include/llvm/AbstractTypeUser.h (original)
+++ llvm/trunk/include/llvm/AbstractTypeUser.h Tue Sep 15 10:58:07 2009
@@ -31,6 +31,7 @@
 
 namespace llvm {
 
+class Value;
 class Type;
 class DerivedType;
 template<typename T> struct simplify_type;
@@ -55,6 +56,12 @@
 class AbstractTypeUser {
 protected:
   virtual ~AbstractTypeUser();                        // Derive from me
+
+  /// setType - It's normally not possible to change a Value's type in place,
+  /// but an AbstractTypeUser subclass that knows what its doing can be
+  /// permitted to do so with care.
+  void setType(Value *V, const Type *NewTy);
+
 public:
 
   /// refineAbstractType - The callback method invoked when an abstract type is

Modified: llvm/trunk/include/llvm/Value.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Value.h?rev=81861&r1=81860&r2=81861&view=diff

==============================================================================
--- llvm/trunk/include/llvm/Value.h (original)
+++ llvm/trunk/include/llvm/Value.h Tue Sep 15 10:58:07 2009
@@ -81,6 +81,7 @@
   friend class ValueSymbolTable; // Allow ValueSymbolTable to directly mod Name.
   friend class SymbolTable;      // Allow SymbolTable to directly poke Name.
   friend class ValueHandleBase;
+  friend class AbstractTypeUser;
   ValueName *Name;
 
   void operator=(const Value &);     // Do not implement

Modified: llvm/trunk/lib/VMCore/Constants.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Constants.cpp?rev=81861&r1=81860&r2=81861&view=diff

==============================================================================
--- llvm/trunk/lib/VMCore/Constants.cpp (original)
+++ llvm/trunk/lib/VMCore/Constants.cpp Tue Sep 15 10:58:07 2009
@@ -878,8 +878,6 @@
 //===----------------------------------------------------------------------===//
 //                      Factory Function Implementation
 
-static char getValType(ConstantAggregateZero *CPZ) { return 0; }
-
 ConstantAggregateZero* ConstantAggregateZero::get(const Type* Ty) {
   assert((isa<StructType>(Ty) || isa<ArrayType>(Ty) || isa<VectorType>(Ty)) &&
          "Cannot create an aggregate zero of non-aggregate type!");
@@ -1008,11 +1006,6 @@
 //---- ConstantPointerNull::get() implementation...
 //
 
-static char getValType(ConstantPointerNull *) {
-  return 0;
-}
-
-
 ConstantPointerNull *ConstantPointerNull::get(const PointerType *Ty) {
   // Implicitly locked.
   return Ty->getContext().pImpl->NullPtrConstants.getOrCreate(Ty, 0);
@@ -1030,10 +1023,6 @@
 //---- UndefValue::get() implementation...
 //
 
-static char getValType(UndefValue *) {
-  return 0;
-}
-
 UndefValue *UndefValue::get(const Type *Ty) {
   // Implicitly locked.
   return Ty->getContext().pImpl->UndefValueConstants.getOrCreate(Ty, 0);
@@ -1050,18 +1039,6 @@
 //---- ConstantExpr::get() implementations...
 //
 
-static ExprMapKeyType getValType(ConstantExpr *CE) {
-  std::vector<Constant*> Operands;
-  Operands.reserve(CE->getNumOperands());
-  for (unsigned i = 0, e = CE->getNumOperands(); i != e; ++i)
-    Operands.push_back(cast<Constant>(CE->getOperand(i)));
-  return ExprMapKeyType(CE->getOpcode(), Operands,
-      CE->isCompare() ? CE->getPredicate() : 0,
-      CE->getRawSubclassOptionalData(),
-      CE->hasIndices() ?
-        CE->getIndices() : SmallVector<unsigned, 4>());
-}
-
 /// This is a utility function to handle folding of casts and lookup of the
 /// cast in the ExprConstants map. It is used by the various get* methods below.
 static inline Constant *getFoldedCast(
@@ -1878,15 +1855,6 @@
 /// work, but would be really slow because it would have to unique each updated
 /// array instance.
 
-static std::vector<Constant*> getValType(ConstantArray *CA) {
-  std::vector<Constant*> Elements;
-  Elements.reserve(CA->getNumOperands());
-  for (unsigned i = 0, e = CA->getNumOperands(); i != e; ++i)
-    Elements.push_back(cast<Constant>(CA->getOperand(i)));
-  return Elements;
-}
-
-
 void ConstantArray::replaceUsesOfWithOnConstant(Value *From, Value *To,
                                                 Use *U) {
   assert(isa<Constant>(To) && "Cannot make Constant refer to non-constant!");
@@ -1895,7 +1863,7 @@
   LLVMContext &Context = getType()->getContext();
   LLVMContextImpl *pImpl = Context.pImpl;
 
-  std::pair<LLVMContextImpl::ArrayConstantsTy::MapKey, Constant*> Lookup;
+  std::pair<LLVMContextImpl::ArrayConstantsTy::MapKey, ConstantArray*> Lookup;
   Lookup.first.first = getType();
   Lookup.second = this;
 
@@ -1973,14 +1941,6 @@
   destroyConstant();
 }
 
-static std::vector<Constant*> getValType(ConstantStruct *CS) {
-  std::vector<Constant*> Elements;
-  Elements.reserve(CS->getNumOperands());
-  for (unsigned i = 0, e = CS->getNumOperands(); i != e; ++i)
-    Elements.push_back(cast<Constant>(CS->getOperand(i)));
-  return Elements;
-}
-
 void ConstantStruct::replaceUsesOfWithOnConstant(Value *From, Value *To,
                                                  Use *U) {
   assert(isa<Constant>(To) && "Cannot make Constant refer to non-constant!");
@@ -1989,7 +1949,7 @@
   unsigned OperandToUpdate = U-OperandList;
   assert(getOperand(OperandToUpdate) == From && "ReplaceAllUsesWith broken!");
 
-  std::pair<LLVMContextImpl::StructConstantsTy::MapKey, Constant*> Lookup;
+  std::pair<LLVMContextImpl::StructConstantsTy::MapKey, ConstantStruct*> Lookup;
   Lookup.first.first = getType();
   Lookup.second = this;
   std::vector<Constant*> &Values = Lookup.first.second;
@@ -2049,14 +2009,6 @@
   destroyConstant();
 }
 
-static std::vector<Constant*> getValType(ConstantVector *CP) {
-  std::vector<Constant*> Elements;
-  Elements.reserve(CP->getNumOperands());
-  for (unsigned i = 0, e = CP->getNumOperands(); i != e; ++i)
-    Elements.push_back(CP->getOperand(i));
-  return Elements;
-}
-
 void ConstantVector::replaceUsesOfWithOnConstant(Value *From, Value *To,
                                                  Use *U) {
   assert(isa<Constant>(To) && "Cannot make Constant refer to non-constant!");

Modified: llvm/trunk/lib/VMCore/ConstantsContext.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/ConstantsContext.h?rev=81861&r1=81860&r2=81861&view=diff

==============================================================================
--- llvm/trunk/lib/VMCore/ConstantsContext.h (original)
+++ llvm/trunk/lib/VMCore/ConstantsContext.h Tue Sep 15 10:58:07 2009
@@ -350,10 +350,11 @@
   }
 };
 
-template<class ConstantClass, class TypeClass>
-struct ConvertConstantType {
-  static void convert(ConstantClass *OldC, const TypeClass *NewTy) {
-    llvm_unreachable("This type cannot be converted!");
+template<class ConstantClass>
+struct ConstantKeyData {
+  typedef void ValType;
+  static ValType getValType(ConstantClass *C) {
+    llvm_unreachable("Unknown Constant type!");
   }
 };
 
@@ -404,50 +405,18 @@
 };
 
 template<>
-struct ConvertConstantType<ConstantExpr, Type> {
-  static void convert(ConstantExpr *OldC, const Type *NewTy) {
-    Constant *New;
-    switch (OldC->getOpcode()) {
-    case Instruction::Trunc:
-    case Instruction::ZExt:
-    case Instruction::SExt:
-    case Instruction::FPTrunc:
-    case Instruction::FPExt:
-    case Instruction::UIToFP:
-    case Instruction::SIToFP:
-    case Instruction::FPToUI:
-    case Instruction::FPToSI:
-    case Instruction::PtrToInt:
-    case Instruction::IntToPtr:
-    case Instruction::BitCast:
-      New = ConstantExpr::getCast(OldC->getOpcode(), OldC->getOperand(0), 
-                                  NewTy);
-      break;
-    case Instruction::Select:
-      New = ConstantExpr::getSelectTy(NewTy, OldC->getOperand(0),
-                                      OldC->getOperand(1),
-                                      OldC->getOperand(2));
-      break;
-    default:
-      assert(OldC->getOpcode() >= Instruction::BinaryOpsBegin &&
-             OldC->getOpcode() <  Instruction::BinaryOpsEnd);
-      New = ConstantExpr::getTy(NewTy, OldC->getOpcode(), OldC->getOperand(0),
-                                OldC->getOperand(1));
-      break;
-    case Instruction::GetElementPtr:
-      // Make everyone now use a constant of the new type...
-      std::vector<Value*> Idx(OldC->op_begin()+1, OldC->op_end());
-      New = cast<GEPOperator>(OldC)->isInBounds() ?
-        ConstantExpr::getInBoundsGetElementPtrTy(NewTy, OldC->getOperand(0),
-                                                 &Idx[0], Idx.size()) :
-        ConstantExpr::getGetElementPtrTy(NewTy, OldC->getOperand(0),
-                                         &Idx[0], Idx.size());
-      break;
-    }
-
-    assert(New != OldC && "Didn't replace constant??");
-    OldC->uncheckedReplaceAllUsesWith(New);
-    OldC->destroyConstant();    // This constant is now dead, destroy it.
+struct ConstantKeyData<ConstantExpr> {
+  typedef ExprMapKeyType ValType;
+  static ValType getValType(ConstantExpr *CE) {
+    std::vector<Constant*> Operands;
+    Operands.reserve(CE->getNumOperands());
+    for (unsigned i = 0, e = CE->getNumOperands(); i != e; ++i)
+      Operands.push_back(cast<Constant>(CE->getOperand(i)));
+    return ExprMapKeyType(CE->getOpcode(), Operands,
+        CE->isCompare() ? CE->getPredicate() : 0,
+        CE->getRawSubclassOptionalData(),
+        CE->hasIndices() ?
+          CE->getIndices() : SmallVector<unsigned, 4>());
   }
 };
 
@@ -460,56 +429,46 @@
 };
 
 template<>
-struct ConvertConstantType<ConstantVector, VectorType> {
-  static void convert(ConstantVector *OldC, const VectorType *NewTy) {
-    // Make everyone now use a constant of the new type...
-    std::vector<Constant*> C;
-    for (unsigned i = 0, e = OldC->getNumOperands(); i != e; ++i)
-      C.push_back(cast<Constant>(OldC->getOperand(i)));
-    Constant *New = ConstantVector::get(NewTy, C);
-    assert(New != OldC && "Didn't replace constant??");
-    OldC->uncheckedReplaceAllUsesWith(New);
-    OldC->destroyConstant();    // This constant is now dead, destroy it.
+struct ConstantKeyData<ConstantVector> {
+  typedef std::vector<Constant*> ValType;
+  static ValType getValType(ConstantVector *CP) {
+    std::vector<Constant*> Elements;
+    Elements.reserve(CP->getNumOperands());
+    for (unsigned i = 0, e = CP->getNumOperands(); i != e; ++i)
+      Elements.push_back(CP->getOperand(i));
+    return Elements;
   }
 };
 
 template<>
-struct ConvertConstantType<ConstantAggregateZero, Type> {
-  static void convert(ConstantAggregateZero *OldC, const Type *NewTy) {
-    // Make everyone now use a constant of the new type...
-    Constant *New = ConstantAggregateZero::get(NewTy);
-    assert(New != OldC && "Didn't replace constant??");
-    OldC->uncheckedReplaceAllUsesWith(New);
-    OldC->destroyConstant();     // This constant is now dead, destroy it.
+struct ConstantKeyData<ConstantAggregateZero> {
+  typedef char ValType;
+  static ValType getValType(ConstantAggregateZero *C) {
+    return 0;
   }
 };
 
 template<>
-struct ConvertConstantType<ConstantArray, ArrayType> {
-  static void convert(ConstantArray *OldC, const ArrayType *NewTy) {
-    // Make everyone now use a constant of the new type...
-    std::vector<Constant*> C;
-    for (unsigned i = 0, e = OldC->getNumOperands(); i != e; ++i)
-      C.push_back(cast<Constant>(OldC->getOperand(i)));
-    Constant *New = ConstantArray::get(NewTy, C);
-    assert(New != OldC && "Didn't replace constant??");
-    OldC->uncheckedReplaceAllUsesWith(New);
-    OldC->destroyConstant();    // This constant is now dead, destroy it.
+struct ConstantKeyData<ConstantArray> {
+  typedef std::vector<Constant*> ValType;
+  static ValType getValType(ConstantArray *CA) {
+    std::vector<Constant*> Elements;
+    Elements.reserve(CA->getNumOperands());
+    for (unsigned i = 0, e = CA->getNumOperands(); i != e; ++i)
+      Elements.push_back(cast<Constant>(CA->getOperand(i)));
+    return Elements;
   }
 };
 
 template<>
-struct ConvertConstantType<ConstantStruct, StructType> {
-  static void convert(ConstantStruct *OldC, const StructType *NewTy) {
-    // Make everyone now use a constant of the new type...
-    std::vector<Constant*> C;
-    for (unsigned i = 0, e = OldC->getNumOperands(); i != e; ++i)
-      C.push_back(cast<Constant>(OldC->getOperand(i)));
-    Constant *New = ConstantStruct::get(NewTy, C);
-    assert(New != OldC && "Didn't replace constant??");
-
-    OldC->uncheckedReplaceAllUsesWith(New);
-    OldC->destroyConstant();    // This constant is now dead, destroy it.
+struct ConstantKeyData<ConstantStruct> {
+  typedef std::vector<Constant*> ValType;
+  static ValType getValType(ConstantStruct *CS) {
+    std::vector<Constant*> Elements;
+    Elements.reserve(CS->getNumOperands());
+    for (unsigned i = 0, e = CS->getNumOperands(); i != e; ++i)
+      Elements.push_back(cast<Constant>(CS->getOperand(i)));
+    return Elements;
   }
 };
 
@@ -522,13 +481,10 @@
 };
 
 template<>
-struct ConvertConstantType<ConstantPointerNull, PointerType> {
-  static void convert(ConstantPointerNull *OldC, const PointerType *NewTy) {
-    // Make everyone now use a constant of the new type...
-    Constant *New = ConstantPointerNull::get(NewTy);
-    assert(New != OldC && "Didn't replace constant??");
-    OldC->uncheckedReplaceAllUsesWith(New);
-    OldC->destroyConstant();     // This constant is now dead, destroy it.
+struct ConstantKeyData<ConstantPointerNull> {
+  typedef char ValType;
+  static ValType getValType(ConstantPointerNull *C) {
+    return 0;
   }
 };
 
@@ -541,13 +497,10 @@
 };
 
 template<>
-struct ConvertConstantType<UndefValue, Type> {
-  static void convert(UndefValue *OldC, const Type *NewTy) {
-    // Make everyone now use a constant of the new type.
-    Constant *New = UndefValue::get(NewTy);
-    assert(New != OldC && "Didn't replace constant??");
-    OldC->uncheckedReplaceAllUsesWith(New);
-    OldC->destroyConstant();     // This constant is now dead, destroy it.
+struct ConstantKeyData<UndefValue> {
+  typedef char ValType;
+  static ValType getValType(UndefValue *C) {
+    return 0;
   }
 };
 
@@ -555,10 +508,11 @@
          bool HasLargeKey = false /*true for arrays and structs*/ >
 class ValueMap : public AbstractTypeUser {
 public:
-  typedef std::pair<const Type*, ValType> MapKey;
-  typedef std::map<MapKey, Constant *> MapTy;
-  typedef std::map<Constant*, typename MapTy::iterator> InverseMapTy;
-  typedef std::map<const Type*, typename MapTy::iterator> AbstractTypeMapTy;
+  typedef std::pair<const TypeClass*, ValType> MapKey;
+  typedef std::map<MapKey, ConstantClass *> MapTy;
+  typedef std::map<ConstantClass *, typename MapTy::iterator> InverseMapTy;
+  typedef std::map<const DerivedType*, typename MapTy::iterator>
+    AbstractTypeMapTy;
 private:
   /// Map - This is the main map from the element descriptor to the Constants.
   /// This is the primary way we avoid creating two of the same shape
@@ -599,7 +553,7 @@
   /// I->second == 0, and should be filled in.
   /// NOTE: This function is not locked.  It is the caller's responsibility
   // to enforce proper synchronization.
-  typename MapTy::iterator InsertOrGetItem(std::pair<MapKey, Constant *>
+  typename MapTy::iterator InsertOrGetItem(std::pair<MapKey, ConstantClass *>
                                  &InsertVal,
                                  bool &Exists) {
     std::pair<typename MapTy::iterator, bool> IP = Map.insert(InsertVal);
@@ -619,7 +573,7 @@
       
     typename MapTy::iterator I =
       Map.find(MapKey(static_cast<const TypeClass*>(CP->getRawType()),
-                      getValType(CP)));
+                      ConstantKeyData<ConstantClass>::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.
@@ -629,6 +583,22 @@
     return I;
   }
     
+  void AddAbstractTypeUser(const Type *Ty, typename MapTy::iterator I) {
+    // If the type of the constant is abstract, make sure that an entry
+    // exists for it in the AbstractTypeMap.
+    if (Ty->isAbstract()) {
+      const DerivedType *DTy = static_cast<const DerivedType *>(Ty);
+      typename AbstractTypeMapTy::iterator TI = AbstractTypeMap.find(DTy);
+
+      if (TI == AbstractTypeMap.end()) {
+        // Add ourselves to the ATU list of the type.
+        cast<DerivedType>(DTy)->addAbstractTypeUser(this);
+
+        AbstractTypeMap.insert(TI, std::make_pair(DTy, I));
+      }
+    }
+  }
+
   ConstantClass* Create(const TypeClass *Ty, const ValType &V,
                         typename MapTy::iterator I) {
     ConstantClass* Result =
@@ -640,19 +610,7 @@
     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()) {
-      typename AbstractTypeMapTy::iterator TI = 
-                                               AbstractTypeMap.find(Ty);
-
-      if (TI == AbstractTypeMap.end()) {
-        // Add ourselves to the ATU list of the type.
-        cast<DerivedType>(Ty)->addAbstractTypeUser(this);
-
-        AbstractTypeMap.insert(TI, std::make_pair(Ty, I));
-      }
-    }
+    AddAbstractTypeUser(Ty, I);
       
     return Result;
   }
@@ -668,7 +626,7 @@
     typename MapTy::iterator I = Map.find(Lookup);
     // Is it in the map?  
     if (I != Map.end())
-      Result = static_cast<ConstantClass *>(I->second);
+      Result = I->second;
         
     if (!Result) {
       // If no preexisting value, create one now...
@@ -678,6 +636,43 @@
     return Result;
   }
 
+  void UpdateAbstractTypeMap(const DerivedType *Ty,
+                             typename MapTy::iterator I) {
+    assert(AbstractTypeMap.count(Ty) &&
+           "Abstract type not in AbstractTypeMap?");
+    typename MapTy::iterator &ATMEntryIt = AbstractTypeMap[Ty];
+    if (ATMEntryIt == I) {
+      // Yes, we are removing the representative entry for this type.
+      // See if there are any other entries of the same type.
+      typename MapTy::iterator TmpIt = ATMEntryIt;
+
+      // First check the entry before this one...
+      if (TmpIt != Map.begin()) {
+        --TmpIt;
+        if (TmpIt->first.first != Ty) // Not the same type, move back...
+          ++TmpIt;
+      }
+
+      // If we didn't find the same type, try to move forward...
+      if (TmpIt == ATMEntryIt) {
+        ++TmpIt;
+        if (TmpIt == Map.end() || TmpIt->first.first != Ty)
+          --TmpIt;   // No entry afterwards with the same type
+      }
+
+      // If there is another entry in the map of the same abstract type,
+      // update the AbstractTypeMap entry now.
+      if (TmpIt != ATMEntryIt) {
+        ATMEntryIt = TmpIt;
+      } else {
+        // Otherwise, we are removing the last instance of this type
+        // from the table.  Remove from the ATM, and from user list.
+        cast<DerivedType>(Ty)->removeAbstractTypeUser(this);
+        AbstractTypeMap.erase(Ty);
+      }
+    }
+  }
+
   void remove(ConstantClass *CP) {
     sys::SmartScopedLock<true> Lock(ValueMapLock);
     typename MapTy::iterator I = FindExistingElement(CP);
@@ -689,47 +684,13 @@
       
     // Now that we found the entry, make sure this isn't the entry that
     // the AbstractTypeMap points to.
-    const TypeClass *Ty = static_cast<const TypeClass *>(I->first.first);
-    if (Ty->isAbstract()) {
-      assert(AbstractTypeMap.count(Ty) &&
-             "Abstract type not in AbstractTypeMap?");
-      typename MapTy::iterator &ATMEntryIt = AbstractTypeMap[Ty];
-      if (ATMEntryIt == I) {
-        // Yes, we are removing the representative entry for this type.
-        // See if there are any other entries of the same type.
-        typename MapTy::iterator TmpIt = ATMEntryIt;
-
-        // First check the entry before this one...
-        if (TmpIt != Map.begin()) {
-          --TmpIt;
-          if (TmpIt->first.first != Ty) // Not the same type, move back...
-            ++TmpIt;
-        }
-
-        // If we didn't find the same type, try to move forward...
-        if (TmpIt == ATMEntryIt) {
-          ++TmpIt;
-          if (TmpIt == Map.end() || TmpIt->first.first != Ty)
-            --TmpIt;   // No entry afterwards with the same type
-        }
-
-        // If there is another entry in the map of the same abstract type,
-        // update the AbstractTypeMap entry now.
-        if (TmpIt != ATMEntryIt) {
-          ATMEntryIt = TmpIt;
-        } else {
-          // Otherwise, we are removing the last instance of this type
-          // from the table.  Remove from the ATM, and from user list.
-          cast<DerivedType>(Ty)->removeAbstractTypeUser(this);
-          AbstractTypeMap.erase(Ty);
-        }
-      }
-    }
+    const TypeClass *Ty = I->first.first;
+    if (Ty->isAbstract())
+      UpdateAbstractTypeMap(static_cast<const DerivedType *>(Ty), I);
 
     Map.erase(I);
   }
 
-    
   /// MoveConstantToNewSlot - If we are about to change C to be the element
   /// specified by I, update our internal data structures to reflect this
   /// fact.
@@ -765,8 +726,7 @@
     
   void refineAbstractType(const DerivedType *OldTy, const Type *NewTy) {
     sys::SmartScopedLock<true> Lock(ValueMapLock);
-    typename AbstractTypeMapTy::iterator I =
-      AbstractTypeMap.find(cast<Type>(OldTy));
+    typename AbstractTypeMapTy::iterator I = AbstractTypeMap.find(OldTy);
 
     assert(I != AbstractTypeMap.end() &&
            "Abstract type not in AbstractTypeMap?");
@@ -775,12 +735,39 @@
     // leaving will remove() itself, causing the AbstractTypeMapEntry to be
     // eliminated eventually.
     do {
-      ConvertConstantType<ConstantClass,
-                          TypeClass>::convert(
-                              static_cast<ConstantClass *>(I->second->second),
-                                              cast<TypeClass>(NewTy));
-
-      I = AbstractTypeMap.find(cast<Type>(OldTy));
+      ConstantClass *C = I->second->second;
+      MapKey Key(cast<TypeClass>(NewTy),
+                 ConstantKeyData<ConstantClass>::getValType(C));
+
+      std::pair<typename MapTy::iterator, bool> IP =
+        Map.insert(std::make_pair(Key, C));
+      if (IP.second) {
+        // The map didn't previously have an appropriate constant in the
+        // new type.
+        
+        // Remove the old entry.
+        typename MapTy::iterator OldI =
+          Map.find(MapKey(cast<TypeClass>(OldTy), IP.first->first.second));
+        assert(OldI != Map.end() && "Constant not in map!");
+        UpdateAbstractTypeMap(OldTy, OldI);
+        Map.erase(OldI);
+
+        // Set the constant's type. This is done in place!
+        setType(C, NewTy);
+
+        // Update the inverse map so that we know that this constant is now
+        // located at descriptor I.
+        if (HasLargeKey)
+          InverseMap[C] = IP.first;
+
+        AddAbstractTypeUser(NewTy, IP.first);
+      } else {
+        // The map already had an appropriate constant in the new type, so
+        // there's no longer a need for the old constant.
+        C->uncheckedReplaceAllUsesWith(IP.first->second);
+        C->destroyConstant();    // This constant is now dead, destroy it.
+      }
+      I = AbstractTypeMap.find(OldTy);
     } while (I != AbstractTypeMap.end());
   }
 

Modified: llvm/trunk/lib/VMCore/Type.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Type.cpp?rev=81861&r1=81860&r2=81861&view=diff

==============================================================================
--- llvm/trunk/lib/VMCore/Type.cpp (original)
+++ llvm/trunk/lib/VMCore/Type.cpp Tue Sep 15 10:58:07 2009
@@ -42,6 +42,9 @@
 
 AbstractTypeUser::~AbstractTypeUser() {}
 
+void AbstractTypeUser::setType(Value *V, const Type *NewTy) {
+  V->VTy = NewTy;
+}
 
 //===----------------------------------------------------------------------===//
 //                         Type Class Implementation

Added: llvm/trunk/test/Linker/partial-type-refinement-link.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/partial-type-refinement-link.ll?rev=81861&view=auto

==============================================================================
--- llvm/trunk/test/Linker/partial-type-refinement-link.ll (added)
+++ llvm/trunk/test/Linker/partial-type-refinement-link.ll Tue Sep 15 10:58:07 2009
@@ -0,0 +1,20 @@
+; This file is used by first.ll, so it doesn't actually do anything itself
+; RUN: true
+
+%AnalysisResolver = type { i8, %PMDataManager* }
+%"DenseMap<P*,AU*>" = type { i64, %"pair<P*,AU*>"*, i64, i64 }
+%PMDataManager = type { i8, %PMTopLevelManager*, i8, i8, i8, i8, i8, i64, i8 }
+%PMTopLevelManager = type { i8, i8, i8, i8, i8, i8, i8, i8, %"DenseMap<P*,AU*>" }
+%P = type { i8, %AnalysisResolver*, i64 }
+%PI = type { i8, i8, i8, i8, i8, i8, %"vector<const PI*>", %P* }
+%"SmallVImpl<const PI*>" = type { i8, %PI* }
+%"_V_base<const PI*>" = type { %"_V_base<const PI*>::_V_impl" }
+%"_V_base<const PI*>::_V_impl" = type { %PI*, i8, i8 }
+%"pair<P*,AU*>" = type opaque
+%"vector<const PI*>" = type { %"_V_base<const PI*>" }
+
+define void @f(%"SmallVImpl<const PI*>"* %this) {
+entry:
+  %x = getelementptr inbounds %"SmallVImpl<const PI*>"* %this, i64 0, i32 1
+  ret void
+}

Added: llvm/trunk/test/Linker/partial-type-refinement.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/partial-type-refinement.ll?rev=81861&view=auto

==============================================================================
--- llvm/trunk/test/Linker/partial-type-refinement.ll (added)
+++ llvm/trunk/test/Linker/partial-type-refinement.ll Tue Sep 15 10:58:07 2009
@@ -0,0 +1,24 @@
+; RUN: llvm-link %s %p/partial-type-refinement-link.ll -S | FileCheck %s
+; PR4954
+
+; CHECK: load %PI** getelementptr inbounds (%"RegisterP<LowerArrayLength>"* @_ZN3mvmL1XE, i64 0, i32 0, i32 6, i32 0, i32 0, i32 0), align 16
+
+%AnalysisResolver = type { i8, %PMDataManager* }
+%"DenseMap<P*,AU*>" = type { i64, %"pair<P*,AU*>"*, i64, i64 }
+%PMDataManager = type { i8, %PMTopLevelManager*, i8, i8, i8, i8, i8, i64, i8 }
+%PMTopLevelManager = type { i8, i8, i8, i8, i8, i8, i8, i8, %"DenseMap<P*,AU*>" }
+%P = type { i8, %AnalysisResolver*, i64 }
+%PI = type { i8, i8, i8, i8, i8, i8, %"vector<const PI*>", %P* }
+%"RegisterP<LowerArrayLength>" = type { %PI }
+%"_V_base<const PI*>" = type { %"_V_base<const PI*>::_V_impl" }
+%"_V_base<const PI*>::_V_impl" = type { %PI*, i8, i8 }
+%"pair<P*,AU*>" = type opaque
+%"vector<const PI*>" = type { %"_V_base<const PI*>" }
+
+ at _ZN3mvmL1XE = external global %"RegisterP<LowerArrayLength>"
+
+define void @__tcf_0() nounwind {
+entry:
+  %0 = load %PI** getelementptr inbounds (%"RegisterP<LowerArrayLength>"* @_ZN3mvmL1XE, i64 0, i32 0, i32 6, i32 0, i32 0, i32 0), align 16
+  ret void
+}





More information about the llvm-commits mailing list