[llvm-commits] [llvm] r89176 - in /llvm/trunk: include/llvm/Target/TargetData.h lib/Target/TargetData.cpp

[Chris Lattner]

On Dec 2, 2009, at 4:14 PM, Bill Wendling wrote:
>>> +    insert(STy);
>>> +    return LayoutInfo[STy];
>>
>> Don't do a redundant map lookup after 'insert'.
>>
> I reworked a lot of this so that it's arguably more correct now...I  
> hope at least.

Thanks Bill, definite improvement.  Next round:

TD.cpp:402:

} // end namespace llvm

-> end anonymous namespace.




typedef DenseMap<const StructType*, StructLayout*> LayoutInfoTy;

Please pull this into the StructLayoutMap class.


void TargetData::InvalidateStructLayoutInfo(const StructType *Ty)  
const {
   if (!LayoutMap) return;  // No cache.

   StructLayoutMap *STM = static_cast<StructLayoutMap*>(LayoutMap);
   LayoutInfoTy::iterator I = STM->find(Ty);
   if (I == STM->end()) return;

   I->second->~StructLayout();
   free(I->second);
   STM->erase(I);

   if (Ty->isAbstract())
     Ty->removeAbstractTypeUser(STM);
}

Most of this code should be in StructLayoutMap.  The body should be  
something like:

{
   if (!LayoutMap) return;  // No cache.

   StructLayoutMap *STM = static_cast<StructLayoutMap*>(LayoutMap);
   STM->InvalidateEntry(Ty);
}

and all the goop should be sucked into InvalidateEntry.  This will  
allow removing a bunch of public methods from StructLayoutMap.

   virtual void refineAbstractType(const DerivedType *OldTy,
                                   const Type *) {

This should just call the new 'InvalidateEntry' method instead of  
duplicating the logic.  Likewise with typeBecameConcrete.


     const StructType *STy = dyn_cast<const StructType>(AbsTy);
     assert(STy && "This can only track struct types.");

Just use 'cast' instead of dyn_cast + assert.  It is more efficient in  
a release-assert build.


   virtual ~StructLayoutMap() {
     // Remove any layouts.
     for (LayoutInfoTy::iterator
            I = LayoutInfo.begin(), E = LayoutInfo.end(); I != E; ++I) {
       const Type *Key = I->first;
       StructLayout *Value = I->second;

       if (Key && Key->isAbstract())
         Key->removeAbstractTypeUser(this);

How can Key be null?  Code aggressively with strong invariants, not  
defensively.

       if (Value) {

How can Value be null?


Much of the logic in TargetData::getStructLayout should be sucked into  
StructLayoutMap.

-Chris