[PATCH] D67723: [CodeView] Add option to disable inline line tables.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:1943
+def gno_inline_line_tables : Flag<["-"], "gno-inline-line-tables">,
+  Flags<[CC1Option, CoreOption]>, HelpText<"Don't emit inline line tables">;
 
----------------
aprantl wrote:
> As a DWARF person, this option name is a little confusing since in DWARF inline info is part of debug info, not the line table, but few end-users would actually know. I would probably have called it -gno-inline-info or -gno-inlined-functions. I don't have strong feelings about it though.
The other two options we have that control this stuff are `-gmlt` / `-gline-tables-only`. gmlt stands for "g minimal line tables". So, our command line interface talks about "line tables" already, and IMO we should stick with it, even if it's not really a table after all.

And, technically, this option will greatly affect the `.debug_line` section. The inlined source locations are normally present in `.debug_line`, and this change suppresses them. Instead, the debugger will appear to be stopped at the inlined call site.


================
Comment at: llvm/docs/LangRef.rst:1437
+``"no-inline-line-tables"``
+    When this attribute is set to true, inline line tables are not generated
+    for this function if it is inlined and the location of the inlined code
----------------
aprantl wrote:
> Same comment for the attribute.
Maybe we could use more precise wording rather than talking about tables. Maybe we should describe what happens from the user perspective, something like:

When this attribute is present and set to true, the inliner will discard source locations while inlining code into the current function. Instead, the source location of the call site will be used for all inlined code. Breakpoints set on code that was inlined into the current function will not fire during the execution of the inlined call sites. If the debugger stops inside an inlined call site, it will appear to be stopped at the outermost inlined call site.


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1433-1446
       if (CalleeHasDebugInfo)
         continue;
 
       // If the inlined instruction has no line number, make it look as if it
       // originates from the call location. This is important for
       // ((__always_inline__, __nodebug__)) functions which must use caller
       // location for all instructions in their function body.
----------------
rnk wrote:
> Let's actually try to reuse this `!CalleeHasDebugInfo` code path when this function attribute is present. They should do the same thing.
This suggestion makes less sense in light of the need to remove variable information. Use your best judgement.


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1426
+          // If we are not generating inline line tables, set the debug location
+          // of the inlined code to be the call site.
+          DebugLoc IDL =
----------------
aprantl wrote:
> aprantl wrote:
> > This will probably cause some IR Verifier failures and very confusing debug info when inlining dbg.value intrinsics. The correct thing to do here is probably to assign line 0 to the inlined instructions and remove all debug info intrinsics. Otherwise the inlined variables will show up in the parent frame, which will screw up debugging.
> cf. getMergedLocation() for how to do this.
Ah, yes, we should erase all debug info for inlined variables while inlining in this mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67723