[Bitcode][NFC] Add abbrev for FUNC_CODE_DEBUG_LOC #146497
Conversation
DILocations that are not attached to instructions are encoded using METADATA_LOCATION records which have an abbrev. DILocations attached to instructions are interleaved with instruction records as FUNC_CODE_DEBUG_LOC records, which do not have an abbrev (and FUNC_CODE_DEBUG_LOC_AGAIN which have no operands). Add a new FUNCTION_BLOCK abbrev FUNCTION_DEBUG_LOC_ABBREV for FUNC_CODE_DEBUG_LOC records. This reduces the bc file size by up to 7% in CTMark, with many between 2-4% smaller. [per-file file size compile-time-tracker](https://llvm-compile-time-tracker.com/compare.php?from=75cf826849713c00829cdf657e330e24c1a2fd03&to=1e268ebd0a581016660d9d7e942495c1be041f7d&stat=size-file&details=on) (go to stage1-ReleaseLTO-g). This optimisation is motivated by llvm#144102, which adds the new Key Instructions fields to bitcode records. The combined patches still overall look to be a slight improvement over the base.
|
IMO this is good -- although it directly trades-off debug-info builds against normal builds because it's expanding the size of abbrev-records to five bits instead of four. By eye, it look like between a 0.5% and 1% cost per object file [edit: in a normal build]. I've previously discarded some removing-debug-intrinsics optimisations because of that limitation. I'm fine with saying that we should push through it to enable benefits in non-normal builds. With more effort (and the extra abbrev numberspace) we might recover the cost being added now. Further opinions would be most welcome! |
Huh, that seems like a lot for this one little record... I wonder if there's any way to reduce that impact - can't make the abbrev condition on debug info, I guess - too early to know that. The bitcode format does dynamically create abbrevs - but I guess if you don't create explicit ones like this, they aren't shared? Each record has its own abbrev? What if we had a way to add shared abbrevs dynamically? |
Alas I believe (96% confidence) it's the width of every abbreviated-code in a function-block, thus a very large number of records in the program grow by one bit, in both debug and non-debug modes.
Hhmmm -- the Customising the abbrev width based on whether the module contains debug-info feels awkward; but on the other hand, this is the benefit of bitcode abbrevs being emitted inline, we get to customise them without any kind of compatibility issues. Plus with that precedent, we can add some more abbrevs for RemoveDIs records to reduce filesize further, without non-debug builds paying a price. |
|
I feel like we'd probably gain more long term by switching to 5 bits now and thus enabling use of more abbreviations. But if you want to preserve the size, I strongly suspect that you can kick out FUNCTION_INST_UNOP_ABBREV and FUNCTION_INST_UNOP_FLAGS_ABBREV without much impact. These are used exclusively for encoding the fneg instruction, which shouldn't be particularly common... |
I just tried dynamic abbrevs (to be clear, creating them in function blocks on the fly rather than declaring them up front in function block-info) but it seems you still need to bump the CodeLen from 4 to 5 bits. And looking at the file sizes it seems Jeremy is right that it's the CodeLen increase that's causing the size increase for non-debug builds. Using dynamic abbrevs reduces the savings in debug info builds slightly compared to the initial patch, but it's still pretty good wins overall:
I'll happily give this a go, but this may not be necessary depending on the results below.
There's a slight saving in most non-debug files, but a reasonable regression in a few, e.g. stage1-ReleaseThinLTO CMakeFiles/lencod.dir/context_ini.c.o 83KiB -> 83KiB (+0.48%) (up to 0.9% for gim_tri_collision.cpp which is a small - 500loc - file with a bunch of fp stuff). By my count the average across the .bc files is -0.047% for the non-debug thinLTO build and -0.021% for the debug LTO build. So overall it's a slight win for CTMark looking at percentages (I didn't get numbers for sum size difference, but can if needed). If everyone's happy with us deferring smart conditional-on-debug-info stuff to a later date, in favour of Nikita's suggestion, then this seems like a good option to me. (I'll put up a PR shortly) |
Discussed in llvm#146497. Seems to have a positive impact on bitcode file size in CTMark LTO builds, and makes space for more abbrevs without bumping the CodeLen for function blocks. Alternatively, we could just bump the CodeLen to 5 bits.
|
Thanks @nikic for those numbers, that looks good. It seems like making CodeLen 5 bits is sensible and should be fairly uncontroversial in that case? @jmorse @dwblaikie are you happy with this patch as is? |
|
NB, as Orlando is away, I'm going to land this via #147211 with a test fixup. |
DILocations that are not attached to instructions are encoded using METADATA_LOCATION records which have an abbrev. DILocations attached to instructions are interleaved with instruction records as FUNC_CODE_DEBUG_LOC records, which do not have an abbrev (and FUNC_CODE_DEBUG_LOC_AGAIN which have no operands). Add a new FUNCTION_BLOCK abbrev FUNCTION_DEBUG_LOC_ABBREV for FUNC_CODE_DEBUG_LOC records. This reduces the bc file size by up to 7% in CTMark, with many between 2-4% smaller. [per-file file size compile-time-tracker](https://llvm-compile-time-tracker.com/compare.php?from=75cf826849713c00829cdf657e330e24c1a2fd03&to=1e268ebd0a581016660d9d7e942495c1be041f7d&stat=size-file&details=on) (go to stage1-ReleaseLTO-g). This optimisation is motivated by #144102, which adds the new Key Instructions fields to bitcode records. The combined patches still overall look to be a slight improvement over the base. (Originally reviewed in PR #146497) Co-authored-by: Orlando Cazalet-Hyams <[email protected]>
|
Merged in 51f4e2c , as discussed I think the size-cost is worth it, and it sounds like we'll gain it back with the additional abbrevs anyway. |
4 participants
DILocations that are not attached to instructions are encoded using METADATA_LOCATION records which have an abbrev. DILocations attached to instructions are interleaved with instruction records as FUNC_CODE_DEBUG_LOC records, which do not have an abbrev (and FUNC_CODE_DEBUG_LOC_AGAIN which have no operands).
Add a new FUNCTION_BLOCK abbrev FUNCTION_DEBUG_LOC_ABBREV for FUNC_CODE_DEBUG_LOC records.
This reduces the bc file size by up to 7% in CTMark, with many between 2-4% smaller.
per-file size comparison in compile-time-tracker (go to stage1-ReleaseLTO-g).
This optimisation is motivated by #144102, which adds the new Key Instructions fields to bitcode records. The combined patches still overall look to be a slight improvement over the base.