Fix cas-friendly line table emission in llvm #6718
Conversation
|
@swift-ci please test |
|
|
||
| int64_t LineDelta = static_cast<int64_t>(LineEntry.getLine()) - LastLine; | ||
|
|
||
| if (FileNum != LineEntry.getFileNum() || LineEntry.IsEndOfFunction) { | ||
| if (FileNum != LineEntry.getFileNum()) { |
There was a problem hiding this comment.
This looks like there could be some NFC refactoring done upstream first? Such as moving the LineEntry.IsEndOfFunction higher up? Might make maintaining the patch easier.
rastogishubham
force-pushed
the
LineTableFix
branch
2 times, most recently
from
0162d6a to
7a9f612
Compare
|
@swift-ci please test |
|
Unfortunately there is a problem with the CAS creation of the debug_line section when this PR is applied. Before this PR was applied, the size of the debug_line blocks were mc:debug_line 196,232 2.35% 8,211,642 1.84% 0 0.00% 30,164,670 0.24% 36,444,094 0.22% After this patch was applied the numbers are: mc:debug_line 214,869 2.57% 8,832,044 1.98% 0 0.00% 31,359,842 0.25% 38,235,650 0.23% The first step was to determine what exactly was wrong, this patch has two changes, one change removes the extra end_sequences at the end of the line table and the other one doesn’t reset the FileNum when the line table is being emitted so that there are no extra DW_LNS_set_line opcodes emitted for functions after the first function for the same header file. To quickly diagnose what was wrong, I put up two draft PRs: #6734 which re-adds the extra end_sequence in the line table whose build results look like mc:debug_line 214,870 2.57% 8,842,479 1.98% 0 0.00% 31,359,849 0.25% 38,235,689 0.23% and #6735 which does not restore the FileNum when generating the line table, thereby ensuring that if a function belongs in a file number that isn’t 1, a DW_LNS_set_file will be emitted. Whose build results look like mc:debug_line 196,367 2.35% 8,203,528 1.84% 0 0.00% 30,689,270 0.24% 36,973,014 0.23% We can see that re-adding the extra end_sequence doesn’t do any improvement, but re-adding the extra DW_LNS_set_file makes the numbers look fairly close to the base case without the PR #6718 |
rastogishubham
force-pushed
the
LineTableFix
branch
from
7a9f612 to
4e4b1e3
Compare
|
@swift-ci please test |
rastogishubham
force-pushed
the
LineTableFix
branch
from
4e4b1e3 to
4a1ab41
Compare
|
@swift-ci please test |
|
mc:debug_line 194,623 2.33% 8,204,512 1.84% 0 0.00% 30,547,505 0.24% 36,775,441 0.22% We can see that with the latest change that forces the DW_LNS_set_file to be emitted for every line table contribution, deduplication is the highest, but the overall size is marginally higher, this is because we are emitting an extra byte for the opcode DW_LNS_set_file, unfortunately there is no way to ensure a correct line table is emitted, that is debuggable and also maximises deduplication |
|
|
||
| int64_t LineDelta = static_cast<int64_t>(LineEntry.getLine()) - LastLine; | ||
|
|
||
| if (FileNum != LineEntry.getFileNum() || LineEntry.IsEndOfFunction) { |
There was a problem hiding this comment.
I'm confused about the previous code: was this || LineEntry.IsEndOfFunction ever relevant? Because right above we continue for this state 🤔
But I see now that most removals here are actually restoring the code to its upstream state
There was a problem hiding this comment.
Yes, that part was not needed, and I have removed it. I am sorry for the confusion on that.
When using the option -mllvm -cas-friendly-debug-info, we expect the line table to be emitted in such a way, that it is split-able in the CAS by every function's line table contribution. This was achieved by emitting a DW_LNE_end_sequence at the end of the function's contribution to the line table. This however, had some issues. Namely, there was an extra DW_LNE_end_sequence being emitted at the end of the line table. There was also the case where because of DW_LNE_end_sequence denoting a reset of the state machine. If the file number of the next line table is 1, then a new DW_LNS_set_file was not emitted, to fix this and maximize deduplication, a DW_LNS_set_file is always emitted after a DW_LNE_end_sequence. These issues have been addressed along with changes to associated tests.
rastogishubham
force-pushed
the
LineTableFix
branch
from
4a1ab41 to
fc66da1
Compare
|
@swift-ci please test |
When using the option -mllvm -cas-friendly-debug-info, we expect the line table to be emitted in such a way, that it is split-able in the CAS by every function's line table contribution. This was achieved by emitting a DW_LNE_end_sequence at the end of the function's contribution to the line table. This however, had some issues.
Namely, there was an extra DW_LNE_end_sequence being emitted at the end of the line table.
There was also the case where because of DW_LNE_end_sequence denoting a reset of the state machine, unnecessary DW_LNS_set_file directives were being emitted which was preventing deduplication.
These issues have been addressed along with changes to associated tests.