This is an archive of the discontinued LLVM Phabricator instance.

[DebugInfo] Try to make class memory layout more efficient
ClosedPublic

Authored by davide on Apr 8 2016, 7:08 PM.

Details

Summary

I started this effort after noticing that DI shows up as a considerable portion of memory usage during an LTO build (of clang, and others).
This is roughly the data I got (peak usage):
DISubprogram: 228MB
DILocalVariable: 164MB
The proposed patch packs field slightly more efficiently reducing the size of DISubprogram from 48bytes to 40bytes and of DILocalVariable from 40 bytes to 32 bytes.
The saving overall in memory is ~70MB.
This is still to be considered WIP as this breaks 4 tests in the LLVM suite (which I'm currently investigating), but I wanted to have some eyes on it as DebugInfo seems to be of interest of many these days.

Diff Detail

Event Timeline

davide updated this revision to Diff 53109.Apr 8 2016, 7:08 PM
davide retitled this revision from to [DebugInfo] Try to make class memory layout more efficient.
davide updated this object.
davide added a subscriber: llvm-commits.
dblaikie edited edge metadata.Apr 8 2016, 11:11 PM
dblaikie added a subscriber: dblaikie.

Certainly seems like a thing we might want to try - though it'd help to
have a reminder/description of why shrinking these fields to these specific
bit sizes is valid (are the already restricted to those sizes in other
places/ways, for example - point to where/how they're already so restricted)

probinson added inline comments.
include/llvm/IR/DebugInfoMetadata.h
1237

MSVC starts a new container field every time you change the base type, so this won't pack the way you want on Windows. Use unsigned as the base type in all of these.

davide added inline comments.Apr 11 2016, 10:21 AM
include/llvm/IR/DebugInfoMetadata.h
1237

Oh, sigh. I'll make sure to test with MSVC.

davide updated this revision to Diff 53497.Apr 12 2016, 5:04 PM
davide edited edge metadata.

Addressed reviewers comments. I decided to split the DILocalVariable changes and I'll post another patch soon.
Thanks for your feedback!

aprantl added inline comments.Apr 12 2016, 5:18 PM
include/llvm/IR/DebugInfoMetadata.h
1257

Should we also have a static assert that log2(DW_VIRTUALITY_max) < 4 here?

davide updated this revision to Diff 53505.Apr 12 2016, 6:16 PM

Addressed Adrian's comment.

aprantl added inline comments.Apr 13 2016, 8:00 AM
include/llvm/IR/DebugInfoMetadata.h
1257

Sorry, but it looks like my suggestion was incorrect: It's either log2(DW_VIRTUALITY_max) <= 2 or DW_VIRTUALITY_max < 4.

davide updated this revision to Diff 53584.Apr 13 2016, 10:03 AM

Done. Any other comments?

aprantl accepted this revision.Apr 13 2016, 11:15 AM
aprantl edited edge metadata.

LGTM with Duncan's comment about Flags addressed.

This revision is now accepted and ready to land.Apr 13 2016, 11:15 AM
This revision was automatically updated to reflect the committed changes.