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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
