[PATCH] D42254: Add optional DICompileUnit to DIBuilder and make outlined debug info use that CU

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lib/CodeGen/MachineOutliner.cpp:116
+  /// \p Candidate lives in, return it.
+  DICompileUnit *getCUIfExists() const {
+    assert(MF && "Candidate has no MF!");
----------------
aprantl wrote:
> How about `getCompileUnitOrNull`?
Maybe "OrNull" rather than "IfExists". Maybe check if one's more prevalent than the other across the LLVM codebase - but either's fine, really.


================
Comment at: lib/CodeGen/MachineOutliner.cpp:182-188
+    DICompileUnit *CU = nullptr;
+    for (auto &C : Candidates) {
+      CU = C->getCUIfExists();
+      if (CU)
+        break;
+    }
+    return CU;
----------------
I'd probably write that as:

  for (const auto &C  : Candidates)
    if (auto *CU = C->getCUIfExists())
      return CU;
  return nullptr;


================
Comment at: lib/CodeGen/MachineOutliner.cpp:1288-1295
+      DISubprogram *SP = DB->createFunction(
+          Unit /* Context */, F->getName(),
+          StringRef() /* Empty linkage name. */, Unit /* File */,
+          0 /* Line numbers don't matter*/,
+          DB->createSubroutineType(DB->getOrCreateTypeArray(None)), /* void */
+          false, true, 0, /* Line in scope doesn't matter*/
+          DINode::DIFlags::FlagArtificial /* Compiler-generated code. */,
----------------
Any chance of cloning the DISubprogram from the original function, then modifying it? Or does it seem better to create a separate one like this? I guess this is fine (Adrian?)


https://reviews.llvm.org/D42254