This is an archive of the discontinued LLVM Phabricator instance.

Refactor BitcodeReader: move Metadata and ValueId handling in their own class/file
ClosedPublic

Authored by mehdi_amini on Dec 9 2016, 5:07 PM.

Details

Summary

I'm planning on changing the way we load metadata to enable laziness.
I'm getting lost in this gigantic files, and gigantic class that is the bitcode
reader. This is a first toward splitting it in a few coarse components that
are more easily understandable.

Diff Detail

Repository
rL LLVM

Event Timeline

mehdi_amini updated this revision to Diff 80983.Dec 9 2016, 5:07 PM
mehdi_amini retitled this revision from to Refactor BitcodeReader: move Metadata and ValueId handling in their own class/file.
mehdi_amini updated this object.
mehdi_amini added reviewers: pcc, tejohnson.
mehdi_amini added subscribers: dexonsmith, llvm-commits.
mehdi_amini updated this revision to Diff 80986.Dec 9 2016, 5:56 PM

Remove IsMetadataMaterialized, didn't find a use for it?

Address Duncan's comments.

tejohnson accepted this revision.Dec 12 2016, 8:49 AM
tejohnson edited edge metadata.

LGTM, other than a nit and one suggestion below.

llvm/lib/Bitcode/Reader/BitcodeReader.cpp
3076 ↗(On Diff #81046)

Why not create this within the BitcodeReader constructor instead? AFAICT we only ever invoke this right after we construct a BitcodeReader anyway (from within BitcodeModule::getModuleImpl).

llvm/lib/Bitcode/Reader/MetadataLoader.h
67 ↗(On Diff #81046)

Nit: s/Perse/Parse/

This revision is now accepted and ready to land.Dec 12 2016, 8:49 AM
mehdi_amini added inline comments.Dec 12 2016, 9:05 AM
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
3076 ↗(On Diff #81046)

The MetadataLoader needs a Module, which we only have here.

tejohnson added inline comments.Dec 12 2016, 9:07 AM
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
3076 ↗(On Diff #81046)

Oh, of course, that makes sense.

This revision was automatically updated to reflect the committed changes.