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

akhuang added a comment. Seems like this is good to be committed then. And it sounds like implementing more thresholds would be useful to do in the future. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67723/new/ https://reviews.llvm.org/D67723

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG6d0389038451: [CodeView] Add option to disable inline line tables. (authored by akhuang). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67723/new/ https://r

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

On Mon, Oct 28, 2019 at 4:42 PM David Blaikie via cfe-commits < cfe-commits@lists.llvm.org> wrote: > If the intended behavior is to show the crash at the call site like >> LangRef in the patch suggest then line 0 will certainly not achieve that. I >> implicitly assumed the wording in LangRef would

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

On Thu, Oct 24, 2019 at 3:24 PM Adrian Prantl via cfe-commits < cfe-commits@lists.llvm.org> wrote: > > > On Oct 24, 2019, at 3:02 PM, David Blaikie via Phabricator < > revi...@reviews.llvm.org> wrote: > > dblaikie added a comment. > > In D67723#1720509 , @a

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

> On Oct 24, 2019, at 3:02 PM, David Blaikie via Phabricator > wrote: > > dblaikie added a comment. > > In D67723#1720509 , @aprantl wrote: > >> In D67723#1720353 , @rnk wrote: >> >>> In D67723#1717468

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

dblaikie added a comment. In D67723#1720509 , @aprantl wrote: > In D67723#1720353 , @rnk wrote: > > > In D67723#1717468 , @aprantl wrote: > > > > > I agree that it would make

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

aprantl added a comment. tldr: my LGTM still stands. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67723/new/ https://reviews.llvm.org/D67723 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

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

aprantl added a comment. In D67723#1720353 , @rnk wrote: > In D67723#1717468 , @aprantl wrote: > > > I agree that it would make sense to have a > > `-ginline-info-threshold=<#insns>` or `-gno-small-inline-functions

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

probinson added a comment. > These experiments are convincing me that, in general, line zero isn't that > helpful for DWARF consumers. If the goal is to get smooth stepping, we may > want to refocus on getting reliable is_stmt bits in the line table. If you mean, it's not useful for identifying

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

rnk added a comment. In D67723#1717468 , @aprantl wrote: > I agree that it would make sense to have a `-ginline-info-threshold=<#insns>` > or `-gno-small-inline-functions` with a hardcoded threshold to implement the > feature Paul described, and this pat

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

aprantl added a comment. In D67723#1717416 , @rnk wrote: > In D67723#1710134 , @probinson wrote: > > > FTR, since @rnk has mentioned my years-ago writings, what Sony has > > internally nowadays is a little differen

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

rnk added a comment. In D67723#1710134 , @probinson wrote: > FTR, since @rnk has mentioned my years-ago writings, what Sony has internally > nowadays is a little different than what I said back then. We have an option > spelled `-gno-inlined-scopes` wh

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

probinson added a comment. Apologies for missing this until now. Our email system keeps dropping stuff sent by Phabricator. FTR, since @rnk has mentioned my years-ago writings, what Sony has internally nowadays is a little different than what I said back then. We have an option spelled `-gno

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

akhuang updated this revision to Diff 225104. akhuang marked an inline comment as done. akhuang added a comment. Address comment about bad decrementing iterator. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67723/new/ https://reviews.llvm.org/D677

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

rnk added a comment. In D67723#1708671 , @aprantl wrote: > Who is "we" in this context? The CodeView backend? Yes, the CodeView backend, sorry for the ambiguity. > As far as DWARF is concerned (and LLVM mostly inherits the DWARF semantics) > line 0 is

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

dblaikie added a comment. In D67723#1708671 , @aprantl wrote: > > 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 pre

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

aprantl added a comment. > 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 u

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

rnk added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1428 +if (isa(BI)) { + BI = --(BI->eraseFromParent()); + continue; Will this work if the dbg.value is the first instruction of a basic block? I'd expect

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

akhuang updated this revision to Diff 224909. akhuang added a comment. - Fixes for DbgInfoIntrinsic type and change test cmd Reverted the line 0 change - I wasn't sure if it would be an issue since the debugger doesn't step through those lines. Repository: rG LLVM Github Monorepo CHANGES SIN

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

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); +co

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

akhuang updated this revision to Diff 224888. akhuang added a comment. - Set inlined locations to line 0 - Fix to remove all debug info intrinsics Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67723/new/ https://reviews.llvm.org/D67723 Files: cl

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

akhuang updated this revision to Diff 224711. akhuang added a comment. - Set location to line 0 with getMergedLocation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67723/new/ https://reviews.llvm.org/D67723 Files: clang/include/clang/Basic/Code

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

akhuang updated this revision to Diff 224697. akhuang marked an inline comment as done. akhuang added a comment. Fix code so that -gno-inline-line-tables works when not codeview Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67723/new/ https://revie

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

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. I would still prefer no-inline-info or no-inline-debuginfo over no-inline-linetables and a line 0 location for the inlined instructions. Other than that the patch is now safe. ===