[PATCH] D70350: [DWARF] Allow cross-CU references of subprogram definitions

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:201
     return false;
-  return (isa<DIType>(D) ||
-          (isa<DISubprogram>(D) && !cast<DISubprogram>(D)->isDefinition())) &&
-         !DD->generateTypeUnits();
+  return (isa<DIType>(D) || isa<DISubprogram>(D)) && !DD->generateTypeUnits();
 }
----------------
Oh, hmm - seems this function isn't used for cross-CU inlining. Perhaps it should be refactored to go through here?

Also - any idea what the history was about the !Definition test in this function? What was that designed to avoid/handle?


================
Comment at: llvm/test/DebugInfo/X86/lto-cross-cu-call-origin-ref.ll:5-26
+; // a.c
+; extern void helper_from_b(void);
+; __attribute__((optnone)) void bar() {}
+; __attribute__((optnone)) static void baz() {}
+; __attribute__((always_inline)) void foo() {
+;   bar();
+;   baz();
----------------
This test case still seems a bit complicated - I'd probably avoid having "main" in it if not needed (because it has arguments and return values that then complicate the DWARF, etc - and extra semantics that probably aren't relevant to the test/readers trying to understand it)

Probably leave functions declared-but-not-defined (since you don't need to link the whole thing into an executable - juts to an object file, to look at the DWARF) rather than optnone, for one thing. & probably doesn't need to be compiled with optimizations enabled? (always_inline is probably enough to get the inlinings you're interested in) & some functions have "()" and others have "(void)" - prefer the former uniformly. You can use llvm-link to link together two llvm IR files, then run it back through clang (to go IR->bitcode-or-IR & just get the always_inline behavior at -O0), rather than needing save-temps, for what it's worth.

What are the cases this test is intended to exercise?


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

https://reviews.llvm.org/D70350