[PATCH] D16424: [ThinLTO] Do metadata linking during batch function importing

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 21 13:42:38 PST 2016


joker.eph added inline comments.

================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:308
@@ +307,3 @@
+    // The modules were created with lazy metadata loading. Materialize it
+    // now, before linking it.
+    SrcModule->materializeMetadata();
----------------
tejohnson wrote:
> joker.eph wrote:
> > I'd question why did we load them with lazy metadata in the first place? If we have materialize the metadata anyway, we can as well load them up-front.
> I think delaying it until here should save memory - otherwise we have all metadata for all source modules in memory at once. Materializing it here just before we link, after which the SrcModule is destroyed, should mean we only have one source module worth of metadata materialized and in memory at once (other than what is linked into the dest module, which will be a subset of the metadata, even smaller once I stop linking in the other things hanging off the DICompileUnit).
I'm fine with leaving the call to `materializeMetadata` as it will turn to be a no-op if they were loaded up-front.
Can you reflect this in the comment above? Just prepending an "If" at the beginning looks good to me.


http://reviews.llvm.org/D16424





More information about the llvm-commits mailing list