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

Mon Oct 14 13:15:52 PDT 2019

rnk added a comment.

Based on what we learned in https://llvm.org/PR43530, I think we still want to use the location of the call site and not line zero. :(

================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1431
+        }
+        BI->setDebugLoc(TheCallDL);
+        continue;
----------------
aprantl wrote:
> I still think an artificial (line 0) location would be less misleading for debuggers, profilers, and optimization remarks.
That will cause problems for us in practice. There's discussion about this in D68747. Since that change, we treat line zero the same as "no location". If there are no locations in a basic block, then the whole block inherits the line number from the block layout predecessor, which could be unrelated. Keeping the inlined call site location gives us the highest likelihood that "step over" will stop at the next statement. Widely applying line zero to entire basic blocks will put us in that situation more often. We could certainly write a pass to backfill better source locations, but it seems preferable to not put ourselves in that position in the first place.

However, the effect you mention on profilers and optimization remarks is real and concerning. Users should have the power to work around it by removing the flag that applies this attribute, which makes me feel like we should go forward with this as is. If this develops into a real usability problem, we can leave the attribute as is and move the implementation into the backend.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67723