[PATCH] D53596: [ThinLTO] Fix a crash in lazy loading of Metadata

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D53596#1298826, @steven_wu wrote:

> My thought is that this patch should make bitcode reader more robust when lazy loading, which is always a good thing. If this is a performance regression, the regression is coming from how debug information is generated. If there are no such forward-ref in the metadata, there is no slowdown with this patch. We can investigate performance regression post commit if needed.


Right and note that if we are not doing lazy metadata loading, there is essentially no change here. Before, the code called MDLoader->getMDNodeFwdRefOrNull(), which boils down to dyn_cast_or_null<MDNode>(getMetadataFwdRef(Idx)). With this change we instead call MDLoader->getMetadataFwdRefOrLoad, and if there is no lazy loading enabled this will end up returning getMetadataFwdRef(ID), to which the caller is now applying the dyn_cast_or_null<MDNode>.

Steven - you had a suggestion around callers to getMDNodeFwdRefOrNull, but after this change there will be no more callers to that. So should I go ahead and submit this one, then remove that interface completely in a follow up? Or do it in this patch?


Repository:
  rL LLVM

https://reviews.llvm.org/D53596