From 4f1ca018aa9a835ccfcb1f77fad76370831b1065 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Sat, 3 Nov 2001 03:27:53 +0000 Subject: [PATCH] Fix major bugs in type resolution llvm-svn: 1092 --- llvm/include/llvm/AbstractTypeUser.h | 26 ++++- llvm/include/llvm/DerivedTypes.h | 8 +- llvm/lib/Bytecode/Reader/ConstantReader.cpp | 10 +- llvm/lib/VMCore/SymbolTable.cpp | 44 +++++--- llvm/lib/VMCore/Type.cpp | 113 +++++++++++++++++--- 5 files changed, 167 insertions(+), 34 deletions(-) diff --git a/llvm/include/llvm/AbstractTypeUser.h b/llvm/include/llvm/AbstractTypeUser.h index 24c061a3bb5d..a36bd49e23bb 100644 --- a/llvm/include/llvm/AbstractTypeUser.h +++ b/llvm/include/llvm/AbstractTypeUser.h @@ -35,6 +35,16 @@ public: // after this method is invoked, OldType shall be deleted, so referencing it // is quite unwise. // + // Another case that is important to consider is when a type is refined, but + // stays in the same place in memory. In this case OldTy will equal NewTy. + // This callback just notifies ATU's that the underlying structure of the type + // has changed... but any previously used properties are still valid. + // + // Note that it is possible to refine a type with parameters OldTy==NewTy, and + // OldTy is no longer abstract. In this case, abstract type users should + // release their hold on a type, because it went from being abstract to + // concrete. + // virtual void refineAbstractType(const DerivedType *OldTy, const Type *NewTy) = 0; }; @@ -94,6 +104,14 @@ public: // operator-> - Allow user to dereference handle naturally... inline const TypeSubClass *operator->() const { return Ty; } + + // removeUserFromConcrete - This function should be called when the User is + // notified that our type is refined... and the type is being refined to + // itself, which is now a concrete type. When a type becomes concrete like + // this, we MUST remove ourself from the AbstractTypeUser list, even though + // the type is apparently concrete. + // + inline void removeUserFromConcrete(); }; @@ -113,7 +131,13 @@ public: // virtual void refineAbstractType(const DerivedType *OldTy, const Type *NewTy) { assert(get() == OldTy && "Can't refine to unknown value!"); - PATypeHandle::operator=((const TypeSC*)NewTy); + + // Check to see if the type just became concrete. If so, we have to + // removeUser to get off its AbstractTypeUser list + removeUserFromConcrete(); + + if (OldTy != NewTy) + PATypeHandle::operator=((const TypeSC*)NewTy); } // operator= - Allow assignment to handle diff --git a/llvm/include/llvm/DerivedTypes.h b/llvm/include/llvm/DerivedTypes.h index fdc654d7fcee..8c6b4a533b71 100644 --- a/llvm/include/llvm/DerivedTypes.h +++ b/llvm/include/llvm/DerivedTypes.h @@ -160,7 +160,6 @@ protected: // Private ctor - Only can be created by a static member... ArrayType(const Type *ElType, int NumEl); - public: inline const Type *getElementType() const { return ElementType; } @@ -252,7 +251,6 @@ protected: // Private ctor - Only can be created by a static member... PointerType(const Type *ElType); - public: inline const Type *getValueType() const { return ValueType; } @@ -326,4 +324,10 @@ template void PATypeHandle::removeUser() { cast(Ty)->removeAbstractTypeUser(User); } +template +void PATypeHandle::removeUserFromConcrete() { + if (!Ty->isAbstract()) + cast(Ty)->removeAbstractTypeUser(User); +} + #endif diff --git a/llvm/lib/Bytecode/Reader/ConstantReader.cpp b/llvm/lib/Bytecode/Reader/ConstantReader.cpp index fcf92fa53668..ec39a9f64c7c 100644 --- a/llvm/lib/Bytecode/Reader/ConstantReader.cpp +++ b/llvm/lib/Bytecode/Reader/ConstantReader.cpp @@ -103,7 +103,8 @@ const Type *BytecodeParser::parseTypeConstant(const uchar *&Buf, // void BytecodeParser::refineAbstractType(const DerivedType *OldType, const Type *NewType) { - if (OldType == NewType) return; // Type is modified, but same + if (OldType == NewType && + OldType->isAbstract()) return; // Type is modified, but same TypeValuesListTy::iterator I = find(MethodTypeValues.begin(), MethodTypeValues.end(), OldType); @@ -113,7 +114,12 @@ void BytecodeParser::refineAbstractType(const DerivedType *OldType, "Can't refine a type I don't know about!"); } - *I = NewType; // Update to point to new, more refined type. + if (OldType == NewType) { + assert(!OldType->isAbstract()); + I->removeUserFromConcrete(); + } else { + *I = NewType; // Update to point to new, more refined type. + } } diff --git a/llvm/lib/VMCore/SymbolTable.cpp b/llvm/lib/VMCore/SymbolTable.cpp index 7a6854c240a8..8d6df2c787e5 100644 --- a/llvm/lib/VMCore/SymbolTable.cpp +++ b/llvm/lib/VMCore/SymbolTable.cpp @@ -25,6 +25,10 @@ SymbolTable::~SymbolTable() { cast(Ty)->removeAbstractTypeUser(this); } } + + // TODO: FIXME: BIG ONE: This doesn't unreference abstract types for the planes + // that could still have entries! + #ifndef NDEBUG // Only do this in -g mode... bool LeftoverValues = true; for (iterator i = begin(); i != end(); ++i) { @@ -173,27 +177,27 @@ void SymbolTable::insertEntry(const string &Name, const Type *VTy, Value *V) { // This function is called when one of the types in the type plane are refined void SymbolTable::refineAbstractType(const DerivedType *OldType, const Type *NewType) { - if (OldType == NewType) return; // Noop, don't waste time dinking around - - // Get a handle to the new type plane... - iterator NewTypeIt = find(NewType); - if (NewTypeIt == super::end()) { // If no plane exists, add one - NewTypeIt = super::insert(make_pair(NewType, VarMap())).first; - - if (NewType->isAbstract()) { - cast(NewType)->addAbstractTypeUser(this); -#if DEBUG_ABSTYPE - cerr << "refined to abstype: " << NewType->getDescription() <second; + if (OldType == NewType && OldType->isAbstract()) + return; // Noop, don't waste time dinking around // Search to see if we have any values of the type oldtype. If so, we need to // move them into the newtype plane... iterator TPI = find(OldType); - if (TPI != end()) { + if (OldType != NewType && TPI != end()) { + // Get a handle to the new type plane... + iterator NewTypeIt = find(NewType); + if (NewTypeIt == super::end()) { // If no plane exists, add one + NewTypeIt = super::insert(make_pair(NewType, VarMap())).first; + + if (NewType->isAbstract()) { + cast(NewType)->addAbstractTypeUser(this); +#if DEBUG_ABSTYPE + cerr << "[Added] refined to abstype: "<getDescription()<second; VarMap &OldPlane = TPI->second; while (!OldPlane.empty()) { pair V = *OldPlane.begin(); @@ -256,6 +260,12 @@ void SymbolTable::refineAbstractType(const DerivedType *OldType, // Remove the plane that is no longer used erase(TPI); + } else if (TPI != end()) { + assert(OldType == NewType); +#if DEBUG_ABSTYPE + cerr << "Removing SELF type " << OldType->getDescription() << endl; +#endif + OldType->removeAbstractTypeUser(this); } TPI = find(Type::TypeTy); diff --git a/llvm/lib/VMCore/Type.cpp b/llvm/lib/VMCore/Type.cpp index ba3f5e0f226c..29d80fc950c6 100644 --- a/llvm/lib/VMCore/Type.cpp +++ b/llvm/lib/VMCore/Type.cpp @@ -151,6 +151,11 @@ ArrayType::ArrayType(const Type *ElType, int NumEl) NumElements = NumEl; setDerivedTypeProperties(); } +ArrayType::~ArrayType() { +#ifdef DEBUG_MERGE_TYPES + cerr << "Destroyed type: " << getDescription() << endl; +#endif +} StructType::StructType(const vector &Types) : DerivedType("", StructTyID) { @@ -166,6 +171,11 @@ PointerType::PointerType(const Type *E) : DerivedType("", PointerTyID), ValueType(PATypeHandle(E, this)) { setDerivedTypeProperties(); } +PointerType::~PointerType() { +#ifdef DEBUG_MERGE_TYPES + cerr << "Destoyed type: " << getDescription() << endl; +#endif +} OpaqueType::OpaqueType() : DerivedType("", OpaqueTyID) { setAbstract(true); @@ -381,7 +391,16 @@ public: // corrected. // virtual void refineAbstractType(const DerivedType *OldTy, const Type *NewTy) { - if (OldTy == NewTy) return; + if (OldTy == NewTy) { + if (!OldTy->isAbstract()) { + // Check to see if the type just became concrete. + // If so, remove self from user list. + for (MapTy::iterator I = Map.begin(), E = Map.end(); I != E; ++I) + if (I->second == OldTy) + I->second.removeUserFromConcrete(); + } + return; + } #ifdef DEBUG_MERGE_TYPES cerr << "Removing Old type from Tab: " << (void*)OldTy << ", " << OldTy->getDescription() << " replacement == " << (void*)NewTy @@ -426,9 +445,19 @@ protected: // Subclass should override this... to update self as usual virtual void doRefinement(const DerivedType *OldTy, const Type *NewTy) = 0; + + // typeBecameConcrete - This callback occurs when a contained type refines + // to itself, but becomes concrete in the process. Our subclass should remove + // itself from the ATU list of the specified type. + // + virtual void typeBecameConcrete(const DerivedType *Ty) = 0; virtual void refineAbstractType(const DerivedType *OldTy, const Type *NewTy) { - if (OldTy == NewTy) return; + if (OldTy == NewTy) { + if (!OldTy->isAbstract()) + typeBecameConcrete(OldTy); + return; + } TypeMap &Table = MyTable; // Copy MyTable reference ValType Tmp(*(ValType*)this); // Copy this. PATypeHandle OldType(Table.get(*(ValType*)this), this); @@ -481,6 +510,13 @@ public: if (ArgTypes[i] == OldType) ArgTypes[i] = NewType; } + virtual void typeBecameConcrete(const DerivedType *Ty) { + if (RetTy == Ty) RetTy.removeUserFromConcrete(); + + for (unsigned i = 0; i < ArgTypes.size(); ++i) + if (ArgTypes[i] == Ty) ArgTypes[i].removeUserFromConcrete(); + } + inline bool operator<(const MethodValType &MTV) const { if (RetTy.get() < MTV.RetTy.get()) return true; if (RetTy.get() > MTV.RetTy.get()) return false; @@ -531,6 +567,12 @@ public: if (ValTy == OldType) ValTy = NewType; } + virtual void typeBecameConcrete(const DerivedType *Ty) { + assert(ValTy == Ty && + "Contained type became concrete but we're not using it!"); + ValTy.removeUserFromConcrete(); + } + inline bool operator<(const ArrayValType &MTV) const { if (Size < MTV.Size) return true; return Size == MTV.Size && ValTy.get() < MTV.ValTy.get(); @@ -587,6 +629,11 @@ public: if (ElTypes[i] == OldType) ElTypes[i] = NewType; } + virtual void typeBecameConcrete(const DerivedType *Ty) { + for (unsigned i = 0; i < ElTypes.size(); ++i) + if (ElTypes[i] == Ty) ElTypes[i].removeUserFromConcrete(); + } + inline bool operator<(const StructValType &STV) const { return ElTypes < STV.ElTypes; } @@ -631,6 +678,12 @@ public: if (ValTy == OldType) ValTy = NewType; } + virtual void typeBecameConcrete(const DerivedType *Ty) { + assert(ValTy == Ty && + "Contained type became concrete but we're not using it!"); + ValTy.removeUserFromConcrete(); + } + inline bool operator<(const PointerValType &MTV) const { return ValTy.get() < MTV.ValTy.get(); } @@ -675,22 +728,20 @@ void DerivedType::removeAbstractTypeUser(AbstractTypeUser *U) const { AbstractTypeUsers.erase(AbstractTypeUsers.begin()+i-1); #ifdef DEBUG_MERGE_TYPES - cerr << " removeAbstractTypeUser[" << (void*)this << ", " - << getDescription() << "][" << AbstractTypeUsers.size() - << "] User = " << U << endl; + cerr << " removeAbstractTypeUser<" << (void*)this << ", " + << getDescription() << ">[" << i << "] User = " << U << endl; #endif - if (AbstractTypeUsers.empty()) { + if (AbstractTypeUsers.empty() && isAbstract()) { #ifdef DEBUG_MERGE_TYPES - cerr << "DELETEing unused abstract type: " << getDescription() - << "[" << (void*)this << "]" << endl; + cerr << "DELETEing unused abstract type: <" << getDescription() + << ">[" << (void*)this << "]" << endl; #endif delete this; // No users of this abstract type! } return; } } - assert(isAbstract() && "removeAbstractTypeUser: Type not abstract!"); assert(0 && "AbstractTypeUser not in user list!"); } @@ -746,6 +797,9 @@ void DerivedType::refineAbstractTypeTo(const Type *NewType) { #endif User->refineAbstractType(this, NewTy); + if (AbstractTypeUsers.size() == OldSize) { + User->refineAbstractType(this, NewTy); + } assert(AbstractTypeUsers.size() != OldSize && "AbsTyUser did not remove self from user list!"); } @@ -786,6 +840,19 @@ void DerivedType::typeIsRefined() { } --isRefining; + +#ifndef _NDEBUG + if (!(isAbstract() || AbstractTypeUsers.empty())) + for (unsigned i = 0; i < AbstractTypeUsers.size(); ++i) { + if (AbstractTypeUsers[i] != this) { + // Debugging hook + cerr << "FOUND FAILURE\n"; + AbstractTypeUsers[i]->refineAbstractType(this, this); + assert(0 && "Type became concrete," + " but it still has abstract type users hanging around!"); + } + } +#endif } @@ -803,6 +870,12 @@ void MethodType::refineAbstractType(const DerivedType *OldType, << NewType->getDescription() << "])\n"; #endif + if (!OldType->isAbstract()) { + if (ResultType == OldType) ResultType.removeUserFromConcrete(); + for (unsigned i = 0; i < ParamTys.size(); ++i) + if (ParamTys[i] == OldType) ParamTys[i].removeUserFromConcrete(); + } + if (OldType != NewType) { if (ResultType == OldType) ResultType = NewType; @@ -812,10 +885,10 @@ void MethodType::refineAbstractType(const DerivedType *OldType, const MethodType *MT = MethodTypes.containsEquivalent(this); if (MT && MT != this) { - refineAbstractTypeTo(MT); // Different type altogether... + refineAbstractTypeTo(MT); // Different type altogether... } else { setDerivedTypeProperties(); // Update the name and isAbstract - typeIsRefined(); // Same type, different contents... + typeIsRefined(); // Same type, different contents... } } @@ -832,6 +905,11 @@ void ArrayType::refineAbstractType(const DerivedType *OldType, << NewType->getDescription() << "])\n"; #endif + if (!OldType->isAbstract()) { + assert(ElementType == OldType); + ElementType.removeUserFromConcrete(); + } + ElementType = NewType; const ArrayType *AT = ArrayTypes.containsEquivalent(this); if (AT && AT != this) { @@ -854,12 +932,18 @@ void StructType::refineAbstractType(const DerivedType *OldType, << OldType->getDescription() << "], " << (void*)NewType << " [" << NewType->getDescription() << "])\n"; #endif + if (!OldType->isAbstract()) { + for (unsigned i = 0; i < ETypes.size(); ++i) + if (ETypes[i] == OldType) + ETypes[i].removeUserFromConcrete(); + } + if (OldType != NewType) { // Update old type to new type in the array... for (unsigned i = 0; i < ETypes.size(); ++i) if (ETypes[i] == OldType) ETypes[i] = NewType; - } + } const StructType *ST = StructTypes.containsEquivalent(this); if (ST && ST != this) { @@ -882,6 +966,11 @@ void PointerType::refineAbstractType(const DerivedType *OldType, << NewType->getDescription() << "])\n"; #endif + if (!OldType->isAbstract()) { + assert(ValueType == OldType); + ValueType.removeUserFromConcrete(); + } + ValueType = NewType; const PointerType *PT = PointerTypes.containsEquivalent(this);