[PATCH] D71244: [DWARF5][DWARFVerifier] Check that Skeleton compilation unit does not have children.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3542
       Args.hasFlag(options::OPT_fsplit_dwarf_inlining,
-                   options::OPT_fno_split_dwarf_inlining, true);
+                   options::OPT_fno_split_dwarf_inlining, false);
 
----------------
aprantl wrote:
> probinson wrote:
> > aprantl wrote:
> > > What does this have to do with the dwarfdump patch?
> > So that by default we emit DWARF that passes the verifier?
> Sorry for being terse earlier. The review says this is a DWARFVerifier patch so it shouldn't touch clang. The clang part should be split into a separate commit with a different commit message.
+1 to this needing to be separate patches (& if there's any automation that's testing that some clang compiled binary passes the verifier that this verifier change would break, then the change in clang's defaults should go in first & then the verifier change after that to ensure things aren't broken between them)


================
Comment at: llvm/test/DebugInfo/skeleton-unit-verify.ll:1
+; RUN: llc %s -mtriple=x86_64-linux -filetype=obj -split-dwarf-cross-cu-references --split-dwarf-file=test.dwo -O3 -o %t 
+; RUN: llvm-dwarfdump --verify %t | FileCheck %s
----------------
What does this functionality/test have to do with split-dwarf-cross-cu-references? That seems orthogonal/unrelated to this change? (& that feature is only related to LTO - this code change seems unrelated to LTO & the problematic cases would appear with or without LTO)


================
Comment at: llvm/test/DebugInfo/skeleton-unit-verify.ll:9-21
+;int foo ( int p1 ) {
+;    return p1 + 10;
+;}
+;
+;int bar ( int p1 ) {
+;    return foo(p1);
+;}
----------------
The simplest example would be:

  void f1();
  __attribute__((always_inline)) void f2() {
    f1();
  }
  void f3() {
    f2();
  }

That produces inlining without extra parameter types, etc, etc. Doesn't require optimizations & I'm not sure that "-gno-column-info" would be related/relevant (nor DWARFv5 - though I'm happy for split-dwarf related tests to be written in DWARFv5 by default so we can maybe remove the v4 support one day) - presumably the command line should have -gsplit-dwarf on it?


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

https://reviews.llvm.org/D71244