CAS.o/FlatV1: Adopt data::TargetInfoList in CompileUnit #3556
Conversation
Pull addends out of blocks and use data::TargetInfoList to combine with the TargetIndex. This makes "block" encode exactly what is in data::BlockData and stops encoding "target"-related data there, fixing a data layering issue. Unfortunately this bloats the compile-unit a bit, since the data has nowhere else to go. Eventually we can/should outline data and references in logical chunks. This commit uniques by TargetInfoList, but there's 5-10% bloat in the size of a CU which makes this hard to justify on its own. A follow-up will optimize this down; this is not squashed so that it's easy to review in isolation.
As a follow-up to storing addends outside of blocks, inline the target lists into the block's list of indexes (rather than creating a TargetInfoList) if there are no non-zero addends. This avoids fixes the regression for the workloads I tested.
|
@cachemeifyoucan , is it easy to collect stats yet on ELF? I'm interested in what you've seen so far. |
I am fixing LinkGraphBuilder so it can consume all llvm-project objects. I can definitely collect early stats for swift-nio or part of llvm-project. |
|
So the reason why So these two are basically trading a small dedup with growth rate. It is hard to tell but I can surely collect some data for all the combo. |
@cachemeifyoucan, now that ELF is mostly working, can you confirm stats for both debug and release builds with I'd like to clean this up one way or the other, making flatv1::BlockRef a simple wrapper around data::BlockData and better aligning the data in nestedv1 and flatv1. The two approaches:
I'd like data to confirm that the addends in ELF are as inconsequential as they seem to be in Mach-O. |
|
I am not sure what information you need from ELF data to guide this patch. Currently, there is still one bug in ELF LinkGraph builder that should have minimal or no impact on size for one build but make size across multiple build invalid. I just run the current implementation on llvm-project for 1 build with I currently have no artifacts generated to benchmark |
2 participants
This is built on top of #3555; I split that out since I think it can/should land whatever happens here.
The first commit lifts addends out of the block and puts them in the compile unit, and the second one recovers from regressions caused by the former. They're split for ease of review.
My motivation is to make the content of
flatv1::BlockRefencapsulate (exactly)data::BlockData(which maybe could be renamed todata::Block...). The other option is to add addends todata::BlockData. I think it's important to do one of those two things. Thecasobjectformats::datanamespace has easily unit-testable code for the encodings; this also makes them reusable in different schemas, or in different parts of the same schema (e.g., inlined vs. outlined data). A pre-requisite for experimenting with outlining subgraphs in flatv1 is to make the encodings easy to reason about. If the "right" encoding for block data has the addends in it, then we should augmentdata::BlockData(and nestedv1). If it doesn't, then we should take this patch.Honestly, I'm not sure this patch is the right approach vs. adding Addends to data::BlockData. The stats for Mach-O don't support this direction; it's almost neutral when adding the second commit, but the other direction (changing
data::BlockData) is simpler. I'd like to see stats for ELF -- both-ffunction-sectionsand-fno-function-sections(although we care a lot more about the former, the latter would be good to understand) -- before landing this.