[PATCH] D28423: Fix assertions on lazy-loading of Metadata TBAA attachments

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 7 08:07:25 PST 2017


tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

LGTM as a bug fix. But I have a few general questions below.



================
Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:704
       !DisableLazyLoading) {
     auto SuccessOrErr = lazyLoadModuleMetadataBlock(Placeholders);
     if (!SuccessOrErr)
----------------
I just noticed that Placeholders is never used in lazyLoadModuleMetadataBlock, so it should probably be removed (in a follow up patch). It does create forward refs so I guess resolveForwardRefsAndPlaceholders below still needs to be invoked, but should it be invoked from within lazyLoadModuleMetadataBlock instead (and passed a dummy PlaceholdersQueue)?


================
Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:1613
           lazyLoadOneMetadata(Idx, Placeholders);
+          resolveForwardRefsAndPlaceholders(Placeholders);
+        }
----------------
I suppose there is no loss in doing this more frequently since we know we need to resolve these anyway? Is there any disadvantage to doing the resolveForwardRefsAndPlaceholders more frequently? If so, should it only be done for TBAA attachments? If not, then should we be doing it more frequently elsewhere?


https://reviews.llvm.org/D28423





More information about the llvm-commits mailing list