This is an archive of the discontinued LLVM Phabricator instance.

Materialize metadata in IRLinker before value mapping
ClosedPublic

Authored by tejohnson on Mar 9 2016, 7:29 AM.

Details

Summary

Unless we plan to do later postpass metadata linking (ThinLTO special mode),
always invoke metadata materialization at the start of IRLinker::run().
This avoids the need for clients who use lazy metadata loading to
explicitly invoke materializeMetadata before the IRMover, which in
turn invokes IRLinker::run and needs materialized metadata for mapping.

Came up in the context of an LLD issue (D17982).

Diff Detail

Event Timeline

tejohnson updated this revision to Diff 50135.Mar 9 2016, 7:29 AM
tejohnson retitled this revision from to Materialize metadata in IRLinker before value mapping.
tejohnson updated this object.
tejohnson added a reviewer: rafael.
tejohnson added subscribers: llvm-commits, silvas.
rafael edited edge metadata.Mar 9 2016, 10:07 AM
rafael added a subscriber: rafael.

I am not sure this is better than just forcing the client to be non-lazy.

Cheers,
Rafael

My hope would be that it avoids errors and makes client lives simpler since they don't need to invoke this manually. Is there a reason why we shouldn't do it automatically?

It hides the fact that the metadata is actually read eagerly.
Cheers,
Rafael

It hides the fact that the metadata is actually read eagerly.
Cheers,
Rafael

Not sure I follow - when metadata is read eagarly (ShouldLazyLoadMetadata=false), this will be a nop.

If by "actually read eagerly" you are referring to the fact that metadata parsing is still all-or-nothing, I don't see how this patch really hides or affects that.

I'm not hugely wedded to this patch, but just want to understand why it would be a bad thing, since it would seem to make bitcode reading clients simpler.

rafael accepted this revision.Mar 9 2016, 4:20 PM
rafael edited edge metadata.
This revision is now accepted and ready to land.Mar 9 2016, 4:20 PM
This revision was automatically updated to reflect the committed changes.