[PATCH] D28751: [ThinLTO] Add a recursive step in Metadata lazy-loading
Teresa Johnson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 17 06:52:12 PST 2017
tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.
Nice improvement - is this related to the regression you saw in your measurements on the effectiveness of lazy loading metadata? A few comments/suggestions below.
In https://reviews.llvm.org/D28751#646918, @mehdi_amini wrote:
> Add a comment, and handles correctly uniquing cycles (even if I didn't encounter any when bootstrapping clang)
Do we have a test case that exposes this issue? If not, it would be good to add
================
Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:836
+ // If lazy-loading is enabled, we try recursively to load the operand
+ // instead of creating a temporary.
+ if (ID < (MDStringRef.size() + GlobalMetadataBitPosIndex.size())) {
----------------
The comment is a little confusing, since we are in fact creating a temporary just above. Maybe change to something like "instead of keeping the temporary which requires a later RAUW".
https://reviews.llvm.org/D28751
More information about the llvm-commits
mailing list