This is an archive of the discontinued LLVM Phabricator instance.

TargetExtType: Use a schema-based abbreviation in bitcode
AcceptedPublic

Authored by nhaehnle on May 11 2023, 7:38 AM.

Details

Summary

The encoding of the structured data used in target types is quite
verbose, and most of its contents are redundant as they encode the
symbols that act as keys of the structured data fields (e.g., "layout")
as well as the self-description of the value types.

By defining an abbreviation that contains all this information as
literals, the overhead is amortized over multiple target types.

The main challenge of this change is the fact that the Array encoding
can only appear once in an abbreviation, at the end, but target types
need two arrays. Define a new ExtArray encoding that avoids this
limitation. Since the encoding space for the encoding itself is rather
tight (3 bits), this extended array encoding allows tuples inside of
arrays as a precautionary extension. This feature is not used at this
time, but it seems rather likely that it will become useful in the
future.

Diff Detail

Event Timeline

nhaehnle created this revision.May 11 2023, 7:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 11 2023, 7:38 AM
nhaehnle requested review of this revision.May 11 2023, 7:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 11 2023, 7:38 AM
Flakebi added inline comments.May 23 2023, 1:56 AM
llvm/lib/Bitstream/Reader/BitstreamReader.cpp
380–385

The code in BitstreamCursor::skipRecord and in BitstreamCursor::readRecord is almost identical. Maybe it makes sense to move that into a function that takes an optional Vals vector?

nhaehnle marked an inline comment as done.Jun 20 2023, 4:50 AM
nhaehnle added inline comments.
llvm/lib/Bitstream/Reader/BitstreamReader.cpp
380–385

Yeah, I was wondering the same thing. I suspect that the initial theory was that skipRecord can be faster. It's unclear how valid that theory is with modern branch predictors. And if it genuinely is an issue, we could still have a single implementation that we force to be specialized by writing it as a template. In any case, I'd rather do that in a separate change.

nhaehnle updated this revision to Diff 532903.Jun 20 2023, 6:51 AM
nhaehnle marked an inline comment as done.

Rebase, mostly changes related to APInt bitwidth being preserved.

Flakebi accepted this revision.Jun 20 2023, 8:36 AM

Looks good to me

This revision is now accepted and ready to land.Jun 20 2023, 8:36 AM
nhaehnle updated this revision to Diff 533141.Jun 20 2023, 11:13 PM

Drop symbols-as-values for now

nhaehnle updated this revision to Diff 533148.Jun 20 2023, 11:42 PM

Move SchemaField into this patch