[PATCH] D90717: [llvm] Add a test for debug info generated with split functions.

[Snehasish Kumar via Phabricator via llvm-commits]

snehasish added a comment.

Thanks for the review all. I've updated the test to use llvm-dwarfdump to check that the line table is as expected for the hot and cold parts.

PTAL @dblaikie @MaskRay 
Thanks!



================
Comment at: llvm/test/DebugInfo/X86/machine-function-splitter.ll:2
+;; Basic block sections used by MFS is only supported on x86, elf based systems.
+; UNSUPPORTED: system-windows, system-darwin 
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -split-machine-functions -filetype=obj %s -o %t
----------------
dblaikie wrote:
> This doesn't sound right - we can run ELF tests on non-ELF platforms. (I assume that's what's trying to be avoided/addressed by this?) Note all the other tests in this directory that don't have such UNSUPPORTED tagging.
Removed, I think this was failing on windows due to the use of llvm-objdump. I'll keep an eye on the windows bot and double check.


================
Comment at: llvm/test/DebugInfo/X86/machine-function-splitter.ll:30
+;;   if (i % 100) {
+;;     return i + 1;
+;;   } else {
----------------
tmsriram wrote:
> Isn't this the cold basic block? What is split out here?
The cold block is the one below, the modulus yields the remainder which is interpreted as true for the if condition.


================
Comment at: llvm/test/DebugInfo/X86/machine-function-splitter.ll:48
+;; llvm-profdata merge -o default.profdata default_*.profraw
+;; clang -fprofile-use -O2 -g -S -emit-llvm split_functions.c
+;; ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 
----------------
dblaikie wrote:
> tmsriram wrote:
> > dblaikie wrote:
> > > MaskRay wrote:
> > > > tmsriram wrote:
> > > > > Do we need a -gsplit-dwarf fission test too?
> > > > The current test only checks -g1 line table. If it targets -g2 (and I think it probably should), I expect that more stuff should be tested.
> > > > Adding @dblaikie who knows better how to construct an interesting test.
> > > If this is a general discussion of "What debug info related things should we test for bb-sections in the LLVM project" it'd be good to get a quick summary of what testing already exists.
> > > 
> > > But otherwise, broad testing I'd think:
> > > * llvm-symbolizer testing (check a couple of addresses in different bb sections, covering an inlined function)
> > > * llvm-dwarfdump of a linked executable showing bb section line table, DW_AT_ranges on the DW_TAG_subprogram
> > > 
> > > But maybe both of these tests are already checked in/implemented?
> > > 
> > > I don't think there's much need for Split DWARF-specific testing, as the Split DWARF handling of ranges is fairly orthogonal to where they appear (shouldn't be interesting to Split DWARF that the ranges appear on a DW_TAG_subprogram, I don't think).
> > Yes the llvm-dwarfdump and the split dwarf testing of DW_AT_ranges is already there:  test/DebugInfo/X86/basic-block-sections_1.ll
> > I didnt add a llvm-symbolizer test, only did lldb tests.  I can add a symbolizer test.
> Could the coverage this new test is adding be added to the existing test? Looks like this new test is intended to cover the line table - which could be added to expanded coverage in the basic-block-sections_1.ll by dumping debug_line as well as debug_info (adding "-debug-line" to those existing dwarfdump commands)
I think there is value in having a simple test for the line table which many downstream tools rely on. We should add a line table test for basic block sections too. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90717/new/

https://reviews.llvm.org/D90717