The Machine Function Splitter splits out cold basic block, it changes
dawrf range. Add Machine Function Splitter and DebugInfo composite
testcase to ensure debug information after the splitting work correctly.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Time | Test | |
---|---|---|
190 ms | x64 windows > Clang.Driver::undefined-libs.cpp |
Event Timeline
I referred llvm/test/CodeGen/X86/machine-function-splitter.ll and llvm/test/DebugInfo/X86/basic-block-sections_1.ll to add this machine function splitter and DebugInfo composite testcase.
Could you please have a review?
Can you elaborate on the motivation behind this test?
As the behaviour exists today, -split-machine-functions and -basic-block-sections are exclusive options and the former takes precedence. With D96392 we are planning on switching the precedence but they still remain exclusive. Thus in this test, the RUN invocations on line 2,3 and 4 are using -split-machine-functions. Furthermore, the underlying mechanism driving both options are the same so testing the generation of debug information together is redundant. For more information take a look at the discussion in D90989.
Remove redundant -basic-block-sections test as it's exclusive with -split-machine-functions.
Thanks for your review comment! I got the current exclusive relation, and -basic-block-section will take precedence over -split-machine-functions after D96392 is done.
I added motivation to Summary, removed redundant test related to -basic-block-sections.
Could you please have a look?
I'll take 1 week national holiday, 2/11~2/17, I'll handle the review comment soon after the holiday, thanks in advance!
We have discussed this approach and decided against adding a separate debug info test for function splitting (see the discussion in D90717). The underlying functionality is tested in test/DebugInfo/X86/basic-block-sections_1.ll. Based on the direction in your previous patch (D95209) - emitting dwarf based debug info for PE/COFF executables has limited utility as far as I can tell. While tooling under cygwin will be able to use this information most Windows native debuggers may not support reading from dwarf (eg. libdwarf started supporting PE mid-2019: https://www.prevanders.net/dwarf.html#libdwarf-20190104). For Windows, we need to look into the CodeView format (see the slides from https://llvm.org/devmtg/2016-11/Slides/Kleckner-CodeViewInLLVM.pdf).
Thanks for reminding the discussion of not adding a separate debug info test for function splitting! I’ll merge the test into test/DebugInfo/X86/basic-block-sections_1.ll.
Thanks for sharing information of dwarf, PE/COFF, CodeView, cygwin and Windows native debuggers! I think
add dwarf based test for Linux and Windows Cygwin
is independent on
(refer to your shared information and existing test of test/DebugInfo/PDB/Native/ to) add CodeView based test for Windows native debuggers, it’s depends on #D95209, it’s better be added in #D95209.
Please do not merge this test. Apologies if I was unclear earlier - there is no need for this test since the functionality is already tested in test/DebugInfo/X86/basic-block-sections_1.ll
The machine function splitter (MFS) pass uses basic block sections so if the dwarf debug info generated by basic block sections is correct then MFS is also correct.
Thanks for sharing information of dwarf, PE/COFF, CodeView, cygwin and Windows native debuggers! I think
add dwarf based test for Linux and Windows Cygwinis independent on
(refer to your shared information and existing test of test/DebugInfo/PDB/Native/ to) add CodeView based test for Windows native debuggers, it’s depends on #D95209, it’s better be added in #D95209.
Sounds good!
Thanks! I got it's MFS dependent basic block sections ensure dwarf debug info correct.
As for Windows COFF, is it also not MFS but MFS dependent basic block sections may be incorrect for Windows native debuggers, so need to extend basic block sections debug info test at first?
As for Windows COFF, is it also not MFS but MFS dependent basic block sections may be incorrect for Windows native debuggers, so need to extend basic block sections debug info test at first?
We have not added support for Windows debug info format so we need to add that before extending the test. :)
Lets discuss the approach offline before spending time on how to add this support.