[PATCH] D39953: [CodeGen] Do not lookup for cached TBAA metadata nodes twice

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev added a comment.

Indeed, DenseMap invalidates iterators on insertion. But then even the "technically correct" part of these changes make things more fragile while my original concern was reliability and not performance. I particularly don't like the repeating cache assignments.

What if we add a set of helper functions whose only purpose is to produce new nodes so we can handle cache-related things in a single place? Something like this:

  llvm::MDNode *CodeGenTBAA::getTypeInfoHelper(llvm::Type *Type) {
    ... Whatever we currently do in getTypeInfo(), except accesses to MetadataCache ...
  }
  
  llvm::MDNode *CodeGenTBAA::getTypeInfo(QualType QTy) {
    ...
    const Type *Ty = Context.getCanonicalType(QTy).getTypePtr();
    if (llvm::MDNode *N = MetadataCache[Ty])
      return N;
  
    return MetadataCache[Ty] = getTypeInfoHelper(Ty);
  }

If for any reasons it is undesirable, then I think I better abandon this diff. Maybe just add a comment explaining that we lookup twice for the same key intentionally.


Repository:
  rL LLVM

https://reviews.llvm.org/D39953