This is an archive of the discontinued LLVM Phabricator instance.

Bitcode reader: Inline readAbbreviatedField in readRecord and move the enclosing loop in each case (NFC)
ClosedPublic

Authored by mehdi_amini on Mar 5 2016, 11:18 PM.

Details

Summary

This make readRecord 20% faster, measured on an LTO build

Diff Detail

Event Timeline

mehdi_amini retitled this revision from to Bitcode reader: Inline readAbbreviatedField in readRecord and move the enclosing loop in each case (NFC).
mehdi_amini updated this object.
mehdi_amini added a reviewer: rafael.
mehdi_amini added a subscriber: llvm-commits.

Nice improvement. Do you know how much this improves the overall parse time or compile time?

lib/Bitcode/Reader/BitstreamReader.cpp
135

Similar loop unswitching optimization could be done here, but presumably not as hot. Consider implementing here too for consistency?

209

Move these into the default case in your new switch below. Otherwise essentially checking for the error case multiple times.

Take Teresa's comment into account.

To answer the question on the impact on the total time: my profile shows readRecord for ~3% for an LTO link and close to %10 for a ThinLTO link.

tejohnson accepted this revision.Mar 6 2016, 3:57 PM
tejohnson added a reviewer: tejohnson.

LGTM with fix noted below.

lib/Bitcode/Reader/BitstreamReader.cpp
147

ReadVBR64?

This revision is now accepted and ready to land.Mar 6 2016, 3:57 PM
mehdi_amini updated this revision to Diff 49926.Mar 6 2016, 4:24 PM
mehdi_amini edited edge metadata.

ReadVBR64(), good catch!