diff --git a/include/llvm/AbstractTypeUser.h b/include/llvm/AbstractTypeUser.h index 86862e790a1..b6cceb4011a 100644 --- a/include/llvm/AbstractTypeUser.h +++ b/include/llvm/AbstractTypeUser.h @@ -31,6 +31,7 @@ namespace llvm { +class Value; class Type; class DerivedType; template struct simplify_type; @@ -55,6 +56,12 @@ template struct simplify_type; 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 diff --git a/include/llvm/Value.h b/include/llvm/Value.h index 415b8fbb2ba..5aa2ea0e1be 100644 --- a/include/llvm/Value.h +++ b/include/llvm/Value.h @@ -81,6 +81,7 @@ private: 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 diff --git a/lib/VMCore/Constants.cpp b/lib/VMCore/Constants.cpp index fdc3eedf4ad..743462a4ed3 100644 --- a/lib/VMCore/Constants.cpp +++ b/lib/VMCore/Constants.cpp @@ -878,8 +878,6 @@ bool ConstantFP::isValueValidForType(const Type *Ty, const APFloat& Val) { //===----------------------------------------------------------------------===// // Factory Function Implementation -static char getValType(ConstantAggregateZero *CPZ) { return 0; } - ConstantAggregateZero* ConstantAggregateZero::get(const Type* Ty) { assert((isa(Ty) || isa(Ty) || isa(Ty)) && "Cannot create an aggregate zero of non-aggregate type!"); @@ -1008,11 +1006,6 @@ Constant *ConstantVector::getSplatValue() { //---- 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 @@ void ConstantPointerNull::destroyConstant() { //---- 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 @@ void UndefValue::destroyConstant() { //---- ConstantExpr::get() implementations... // -static ExprMapKeyType getValType(ConstantExpr *CE) { - std::vector Operands; - Operands.reserve(CE->getNumOperands()); - for (unsigned i = 0, e = CE->getNumOperands(); i != e; ++i) - Operands.push_back(cast(CE->getOperand(i))); - return ExprMapKeyType(CE->getOpcode(), Operands, - CE->isCompare() ? CE->getPredicate() : 0, - CE->getRawSubclassOptionalData(), - CE->hasIndices() ? - CE->getIndices() : SmallVector()); -} - /// 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 @@ const char *ConstantExpr::getOpcodeName() const { /// work, but would be really slow because it would have to unique each updated /// array instance. -static std::vector getValType(ConstantArray *CA) { - std::vector Elements; - Elements.reserve(CA->getNumOperands()); - for (unsigned i = 0, e = CA->getNumOperands(); i != e; ++i) - Elements.push_back(cast(CA->getOperand(i))); - return Elements; -} - - void ConstantArray::replaceUsesOfWithOnConstant(Value *From, Value *To, Use *U) { assert(isa(To) && "Cannot make Constant refer to non-constant!"); @@ -1895,7 +1863,7 @@ void ConstantArray::replaceUsesOfWithOnConstant(Value *From, Value *To, LLVMContext &Context = getType()->getContext(); LLVMContextImpl *pImpl = Context.pImpl; - std::pair Lookup; + std::pair Lookup; Lookup.first.first = getType(); Lookup.second = this; @@ -1973,14 +1941,6 @@ void ConstantArray::replaceUsesOfWithOnConstant(Value *From, Value *To, destroyConstant(); } -static std::vector getValType(ConstantStruct *CS) { - std::vector Elements; - Elements.reserve(CS->getNumOperands()); - for (unsigned i = 0, e = CS->getNumOperands(); i != e; ++i) - Elements.push_back(cast(CS->getOperand(i))); - return Elements; -} - void ConstantStruct::replaceUsesOfWithOnConstant(Value *From, Value *To, Use *U) { assert(isa(To) && "Cannot make Constant refer to non-constant!"); @@ -1989,7 +1949,7 @@ void ConstantStruct::replaceUsesOfWithOnConstant(Value *From, Value *To, unsigned OperandToUpdate = U-OperandList; assert(getOperand(OperandToUpdate) == From && "ReplaceAllUsesWith broken!"); - std::pair Lookup; + std::pair Lookup; Lookup.first.first = getType(); Lookup.second = this; std::vector &Values = Lookup.first.second; @@ -2049,14 +2009,6 @@ void ConstantStruct::replaceUsesOfWithOnConstant(Value *From, Value *To, destroyConstant(); } -static std::vector getValType(ConstantVector *CP) { - std::vector 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(To) && "Cannot make Constant refer to non-constant!"); diff --git a/lib/VMCore/ConstantsContext.h b/lib/VMCore/ConstantsContext.h index 4f55502ff94..526b4b1b7ee 100644 --- a/lib/VMCore/ConstantsContext.h +++ b/lib/VMCore/ConstantsContext.h @@ -350,10 +350,11 @@ struct ConstantCreator { } }; -template -struct ConvertConstantType { - static void convert(ConstantClass *OldC, const TypeClass *NewTy) { - llvm_unreachable("This type cannot be converted!"); +template +struct ConstantKeyData { + typedef void ValType; + static ValType getValType(ConstantClass *C) { + llvm_unreachable("Unknown Constant type!"); } }; @@ -404,50 +405,18 @@ struct ConstantCreator { }; template<> -struct ConvertConstantType { - 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 Idx(OldC->op_begin()+1, OldC->op_end()); - New = cast(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 { + typedef ExprMapKeyType ValType; + static ValType getValType(ConstantExpr *CE) { + std::vector Operands; + Operands.reserve(CE->getNumOperands()); + for (unsigned i = 0, e = CE->getNumOperands(); i != e; ++i) + Operands.push_back(cast(CE->getOperand(i))); + return ExprMapKeyType(CE->getOpcode(), Operands, + CE->isCompare() ? CE->getPredicate() : 0, + CE->getRawSubclassOptionalData(), + CE->hasIndices() ? + CE->getIndices() : SmallVector()); } }; @@ -460,56 +429,46 @@ struct ConstantCreator { }; template<> -struct ConvertConstantType { - static void convert(ConstantVector *OldC, const VectorType *NewTy) { - // Make everyone now use a constant of the new type... - std::vector C; - for (unsigned i = 0, e = OldC->getNumOperands(); i != e; ++i) - C.push_back(cast(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 { + typedef std::vector ValType; + static ValType getValType(ConstantVector *CP) { + std::vector 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 { - 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 { + typedef char ValType; + static ValType getValType(ConstantAggregateZero *C) { + return 0; } }; template<> -struct ConvertConstantType { - static void convert(ConstantArray *OldC, const ArrayType *NewTy) { - // Make everyone now use a constant of the new type... - std::vector C; - for (unsigned i = 0, e = OldC->getNumOperands(); i != e; ++i) - C.push_back(cast(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 { + typedef std::vector ValType; + static ValType getValType(ConstantArray *CA) { + std::vector Elements; + Elements.reserve(CA->getNumOperands()); + for (unsigned i = 0, e = CA->getNumOperands(); i != e; ++i) + Elements.push_back(cast(CA->getOperand(i))); + return Elements; } }; template<> -struct ConvertConstantType { - static void convert(ConstantStruct *OldC, const StructType *NewTy) { - // Make everyone now use a constant of the new type... - std::vector C; - for (unsigned i = 0, e = OldC->getNumOperands(); i != e; ++i) - C.push_back(cast(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 { + typedef std::vector ValType; + static ValType getValType(ConstantStruct *CS) { + std::vector Elements; + Elements.reserve(CS->getNumOperands()); + for (unsigned i = 0, e = CS->getNumOperands(); i != e; ++i) + Elements.push_back(cast(CS->getOperand(i))); + return Elements; } }; @@ -522,13 +481,10 @@ struct ConstantCreator { }; template<> -struct ConvertConstantType { - 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 { + typedef char ValType; + static ValType getValType(ConstantPointerNull *C) { + return 0; } }; @@ -541,13 +497,10 @@ struct ConstantCreator { }; template<> -struct ConvertConstantType { - 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 { + typedef char ValType; + static ValType getValType(UndefValue *C) { + return 0; } }; @@ -555,10 +508,11 @@ template class ValueMap : public AbstractTypeUser { public: - typedef std::pair MapKey; - typedef std::map MapTy; - typedef std::map InverseMapTy; - typedef std::map AbstractTypeMapTy; + typedef std::pair MapKey; + typedef std::map MapTy; + typedef std::map InverseMapTy; + typedef std::map + 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 @@ public: /// 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 + typename MapTy::iterator InsertOrGetItem(std::pair &InsertVal, bool &Exists) { std::pair IP = Map.insert(InsertVal); @@ -619,7 +573,7 @@ private: typename MapTy::iterator I = Map.find(MapKey(static_cast(CP->getRawType()), - getValType(CP))); + ConstantKeyData::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 @@ private: 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(Ty); + typename AbstractTypeMapTy::iterator TI = AbstractTypeMap.find(DTy); + + if (TI == AbstractTypeMap.end()) { + // Add ourselves to the ATU list of the type. + cast(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 @@ private: 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(Ty)->addAbstractTypeUser(this); - - AbstractTypeMap.insert(TI, std::make_pair(Ty, I)); - } - } + AddAbstractTypeUser(Ty, I); return Result; } @@ -668,7 +626,7 @@ public: typename MapTy::iterator I = Map.find(Lookup); // Is it in the map? if (I != Map.end()) - Result = static_cast(I->second); + Result = I->second; if (!Result) { // If no preexisting value, create one now... @@ -678,6 +636,43 @@ public: 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(Ty)->removeAbstractTypeUser(this); + AbstractTypeMap.erase(Ty); + } + } + } + void remove(ConstantClass *CP) { sys::SmartScopedLock Lock(ValueMapLock); typename MapTy::iterator I = FindExistingElement(CP); @@ -689,47 +684,13 @@ public: // Now that we found the entry, make sure this isn't the entry that // the AbstractTypeMap points to. - const TypeClass *Ty = static_cast(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(Ty)->removeAbstractTypeUser(this); - AbstractTypeMap.erase(Ty); - } - } - } + const TypeClass *Ty = I->first.first; + if (Ty->isAbstract()) + UpdateAbstractTypeMap(static_cast(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 @@ public: void refineAbstractType(const DerivedType *OldTy, const Type *NewTy) { sys::SmartScopedLock Lock(ValueMapLock); - typename AbstractTypeMapTy::iterator I = - AbstractTypeMap.find(cast(OldTy)); + typename AbstractTypeMapTy::iterator I = AbstractTypeMap.find(OldTy); assert(I != AbstractTypeMap.end() && "Abstract type not in AbstractTypeMap?"); @@ -775,12 +735,39 @@ public: // leaving will remove() itself, causing the AbstractTypeMapEntry to be // eliminated eventually. do { - ConvertConstantType::convert( - static_cast(I->second->second), - cast(NewTy)); + ConstantClass *C = I->second->second; + MapKey Key(cast(NewTy), + ConstantKeyData::getValType(C)); - I = AbstractTypeMap.find(cast(OldTy)); + std::pair 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(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()); } diff --git a/lib/VMCore/Type.cpp b/lib/VMCore/Type.cpp index 2ddb8c7d5ac..0883cc304b1 100644 --- a/lib/VMCore/Type.cpp +++ b/lib/VMCore/Type.cpp @@ -42,6 +42,9 @@ using namespace llvm; AbstractTypeUser::~AbstractTypeUser() {} +void AbstractTypeUser::setType(Value *V, const Type *NewTy) { + V->VTy = NewTy; +} //===----------------------------------------------------------------------===// // Type Class Implementation diff --git a/test/Linker/partial-type-refinement-link.ll b/test/Linker/partial-type-refinement-link.ll new file mode 100644 index 00000000000..320ef969f83 --- /dev/null +++ b/test/Linker/partial-type-refinement-link.ll @@ -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" = type { i64, %"pair"*, 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 = type { i8, %AnalysisResolver*, i64 } +%PI = type { i8, i8, i8, i8, i8, i8, %"vector", %P* } +%"SmallVImpl" = type { i8, %PI* } +%"_V_base" = type { %"_V_base::_V_impl" } +%"_V_base::_V_impl" = type { %PI*, i8, i8 } +%"pair" = type opaque +%"vector" = type { %"_V_base" } + +define void @f(%"SmallVImpl"* %this) { +entry: + %x = getelementptr inbounds %"SmallVImpl"* %this, i64 0, i32 1 + ret void +} diff --git a/test/Linker/partial-type-refinement.ll b/test/Linker/partial-type-refinement.ll new file mode 100644 index 00000000000..b995f11533f --- /dev/null +++ b/test/Linker/partial-type-refinement.ll @@ -0,0 +1,24 @@ +; RUN: llvm-link %s %p/partial-type-refinement-link.ll -S | FileCheck %s +; PR4954 + +; CHECK: load %PI** getelementptr inbounds (%"RegisterP"* @_ZN3mvmL1XE, i64 0, i32 0, i32 6, i32 0, i32 0, i32 0), align 16 + +%AnalysisResolver = type { i8, %PMDataManager* } +%"DenseMap" = type { i64, %"pair"*, 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 = type { i8, %AnalysisResolver*, i64 } +%PI = type { i8, i8, i8, i8, i8, i8, %"vector", %P* } +%"RegisterP" = type { %PI } +%"_V_base" = type { %"_V_base::_V_impl" } +%"_V_base::_V_impl" = type { %PI*, i8, i8 } +%"pair" = type opaque +%"vector" = type { %"_V_base" } + +@_ZN3mvmL1XE = external global %"RegisterP" + +define void @__tcf_0() nounwind { +entry: + %0 = load %PI** getelementptr inbounds (%"RegisterP"* @_ZN3mvmL1XE, i64 0, i32 0, i32 6, i32 0, i32 0, i32 0), align 16 + ret void +}