[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro marked an inline comment as done. djtodoro added inline comments. Comment at: lib/Sema/SemaExpr.cpp:11292 +} + +/// Argument's value might be modified, so update the info. riccibruno wrote: > Hmm, I don't think that this will work. Suppose that you have

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

djtodoro created this revision. djtodoro added reviewers: shuaiwang, lebedev.ri. Herald added subscribers: Charusso, jdoerfert, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. We should track mutation of a variable within a comma operator

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

djtodoro added a comment. @lebedev.ri Thanks for your comment! > Is there any way to model this more generically? > I.e don't duplicate every naive matcher (ignoring possible presence of , op) > with an variant that does not ignore ,. > E.g. will this handle (a,b)+=1 ? Hmm, I am not sure if

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

djtodoro added a comment. >> @lebedev.ri I agree, thank you! I needed to be more precise in my previous >> reply, sorry for that. I thought it will be (somehow) overhead if I change >> existing, very basic, matchers. > > I indeed don't think the existing matchers should be changed to ignore

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

djtodoro updated this revision to Diff 189461. djtodoro added a comment. -add AST matcher that skips operands in comma expression -add unit tests for extended support CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58894/new/ https://reviews.llvm.org/D58894 Files:

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

djtodoro marked an inline comment as done. djtodoro added inline comments. Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:27 +AST_MATCHER_P(Expr, skipCommaOps, + ast_matchers::internal::Matcher, InnerMatcher) { lebedev.ri wrote: > djtodoro

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

djtodoro marked an inline comment as done. djtodoro added a comment. > Nice, i like this! > I think the test coverage is good. @lebedev.ri Thanks! > But what about other equalsNode() that you didn't change? > Do some of them need this change too? For sure some of them (maybe all of them) needs

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro marked an inline comment as done. djtodoro added a comment. @riccibruno Thanks for your comments! > Oh and I think that you will also have to update the serialization > code/de-serialization code in ASTReaderDecl.cpp / ASTWriterDecl.cpp. You > might also have to update TreeTransform

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro marked 2 inline comments as done. djtodoro added a comment. > I was under the impression that space inside VarDecl was quite constrained. > Pardon the likely naive question, but: is there any way to make the > representation more compact (maybe sneak a bit into ParmVarDeclBitfields)?

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro updated this revision to Diff 187931. djtodoro added a comment. - Add a field in `ParmVarDecl` instead of `VarDecl` - Use a bit in `ParmVarDeclBitfields` to indicate parameter modification - Add support for the bit in `ASTReaderDecl.cpp` / `ASTWriterDecl.cpp` - Add test case for

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro marked an inline comment as done. djtodoro added a comment. > I'm not quite sure what this differential is about, but i feel like > mentioning ExprMutationAnalyzer lib in clang-tidy / clang-tools-extra. >> Alternatively perhaps you could re-use getMemoryLocation() from D57660 >>

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro updated this revision to Diff 188110. djtodoro added a comment. Herald added a subscriber: jdoerfert. - Handle all kinds of expressions when mark a param's modification CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58035/new/ https://reviews.llvm.org/D58035 Files:

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

djtodoro added a comment. @lebedev.ri I agree, thank you! I needed to be more precise in my previous reply, sorry for that. I thought it will be (somehow) overhead if I change existing, very basic, matchers. I already implemented a static function that skips comma operands, and extended this

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

djtodoro updated this revision to Diff 189661. djtodoro added a comment. -add support for the rest of direct mutation cases CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58894/new/ https://reviews.llvm.org/D58894 Files: include/clang/AST/Expr.h

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

djtodoro marked an inline comment as done. djtodoro added a comment. > Anyways, this looks good in this state. > Thank you for working on this! Thanks! > If you don't intend to immediately work on fixing the rest of , cases here, > *please* do file a bug so it won't get lost. (and link it here)

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro updated this revision to Diff 186611. djtodoro added a comment. - Rename: `VariableNotChanged `===> `ArgumentNotModified` - Refactor a test case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58035/new/ https://reviews.llvm.org/D58035 Files: include/clang/AST/Decl.h

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro updated this revision to Diff 195192. djtodoro added a comment. -Rebase -Use `ExprMutationAnalyzer` for parameter's modification check CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58035/new/ https://reviews.llvm.org/D58035 Files: lib/CodeGen/CGDebugInfo.cpp

[PATCH] D58033: Add option for emitting dbg info for call sites

djtodoro updated this revision to Diff 195188. djtodoro edited the summary of this revision. djtodoro added a comment. -Rebase -Add all_call_sites flag in the case of GNU extensions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/ https://reviews.llvm.org/D58033 Files:

[PATCH] D58043: Add experimental options for call site related dbg info

djtodoro updated this revision to Diff 195198. djtodoro retitled this revision from "Add option for emitting DW_OP_entry_values" to "Add experimental options for call site related dbg info". djtodoro edited the summary of this revision. CHANGES SINCE LAST ACTION

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro updated this revision to Diff 195359. djtodoro added a comment. -Run clang-format -Use `cast` instead of '`dyn_cast`' CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58035/new/ https://reviews.llvm.org/D58035 Files: lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDebugInfo.h

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro marked 3 inline comments as done. djtodoro added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:4537 + CGM.getLangOpts().Optimize) { +for (auto : DeclCache) { + auto *D = SP.first; aprantl wrote: > Just looking at the type

[PATCH] D58033: Add option for emitting dbg info for call site parameters

djtodoro updated this revision to Diff 199213. djtodoro edited the summary of this revision. djtodoro added a comment. -Add an input in test/CodeGenCXX/dbg-info-all-calls-described.cpp -Rename the option CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro updated this revision to Diff 199214. djtodoro added a comment. -Rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58035/new/ https://reviews.llvm.org/D58035 Files: lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDebugInfo.h Index: lib/CodeGen/CGDebugInfo.h

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro marked 3 inline comments as done. djtodoro added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:4537 + CGM.getLangOpts().Optimize) { +for (auto : DeclCache) { + auto *D = SP.first; aprantl wrote: > djtodoro wrote: > > aprantl

[PATCH] D58033: Add option for emitting dbg info for call site parameters

djtodoro updated this revision to Diff 201503. djtodoro added a comment. -Rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/ https://reviews.llvm.org/D58033 Files: include/clang/Basic/CodeGenOptions.def include/clang/Driver/CC1Options.td lib/CodeGen/BackendUtil.cpp

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro updated this revision to Diff 201505. djtodoro added a comment. -Rebase -Add a test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58035/new/ https://reviews.llvm.org/D58035 Files: lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDebugInfo.h

[PATCH] D58033: Add option for emitting dbg info for call site parameters

djtodoro updated this revision to Diff 203335. djtodoro added a comment. -Explicitly enable the option only in the case of X86 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/ https://reviews.llvm.org/D58033 Files: include/clang/Basic/CodeGenOptions.def

[PATCH] D58033: Add option for emitting dbg info for call site parameters

djtodoro marked an inline comment as done. djtodoro added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:755 + (Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64)) +Opts.EnableDebugEntryValues = Args.hasArg(OPT_femit_debug_entry_values); +

[PATCH] D58033: Add option for emitting dbg info for call site parameters

djtodoro updated this revision to Diff 200943. djtodoro added a comment. -Add a negative test for -O0 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/ https://reviews.llvm.org/D58033 Files: include/clang/Basic/CodeGenOptions.def include/clang/Driver/CC1Options.td

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro updated this revision to Diff 200950. djtodoro added a comment. -Add `SPDefCache` to speed up the process -Add additional assertions that will improve quality of the code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58035/new/ https://reviews.llvm.org/D58035 Files:

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro updated this revision to Diff 200486. djtodoro added a comment. -Use `SPCache` instead of `DeclCache` -Refactor the code by addressing suggestions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58035/new/ https://reviews.llvm.org/D58035 Files: lib/CodeGen/CGDebugInfo.cpp

[PATCH] D58033: Add option for emitting dbg info for call site parameters

djtodoro updated this revision to Diff 200485. djtodoro added a comment. -Set `EnableDebugEntryValues` only for a higher level of optimizations (-O1 and higher) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/ https://reviews.llvm.org/D58033 Files:

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro updated this revision to Diff 199788. djtodoro added a comment. -Careful use of `dyn_cast` -Fill the `ParamCache` only in the case of `EnableDebugEntryValues` option CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58035/new/ https://reviews.llvm.org/D58035 Files:

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

djtodoro marked an inline comment as done. djtodoro added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:3885 + if (ArgNo) { +auto *PD = dyn_cast(VD); +ParmCache[PD].reset(D); aprantl wrote: > A `dyn_cast` followed by an unconditional use

[PATCH] D58033: Add option for emitting dbg info for call site parameters

djtodoro added a comment. @probinson @aprantl Thanks! @vsk Thanks for the comment! Sure, we will add that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/ https://reviews.llvm.org/D58033 ___ cfe-commits mailing list

[PATCH] D58033: Add option for emitting dbg info for call site parameters

This revision was automatically updated to reflect the committed changes. Closed by commit rL364399: [CC1Option] Add the option to enable the debug entry values (authored by djtodoro, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

This revision was automatically updated to reflect the committed changes. Closed by commit rL364424: [clang/DIVar] Emit the flag for params that have unmodified value (authored by djtodoro, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D64540: [CGDebugInfo] Simplfiy EmitFunctionDecl parameters, NFC

djtodoro accepted this revision. djtodoro added a comment. This revision is now accepted and ready to land. LGTM! Thanks @vsk! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64540/new/ https://reviews.llvm.org/D64540 ___ cfe-commits mailing

[PATCH] D58033: Add option for emitting dbg info for call site parameters

djtodoro added a comment. > Is there some kind of testcase? @aprantl Usage of the option is tested within following patches from the stack. I am not sure if we need some additional test here? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/ https://reviews.llvm.org/D58033

[PATCH] D58033: Add option for emitting dbg info for call site parameters

djtodoro marked 2 inline comments as done. djtodoro added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:3402 +CmdArgs.push_back("-femit-param-entry-values"); + RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC); probinson wrote: > If this

[PATCH] D58033: Add option for emitting dbg info for call site parameters

djtodoro updated this revision to Diff 196595. djtodoro marked an inline comment as done. djtodoro edited the summary of this revision. djtodoro added a comment. -Remove unneeded code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/ https://reviews.llvm.org/D58033 Files:

[PATCH] D58043: Add experimental options for call site related dbg info

djtodoro updated this revision to Diff 195569. djtodoro added a comment. -Rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58043/new/ https://reviews.llvm.org/D58043 Files: lib/Driver/ToolChains/Clang.cpp Index: lib/Driver/ToolChains/Clang.cpp

[PATCH] D58033: Add option for emitting dbg info for call sites

djtodoro marked 2 inline comments as done. djtodoro added a comment. @probinson @aprantl Thanks a lot for your comments! Let's clarify some things. I'm sorry about the confusion. Initial patch for the functionality can be restricted by this option (like we pushed here), since **LLDB** doesn't

[PATCH] D58033: Add option for emitting dbg info for call site parameters

djtodoro updated this revision to Diff 195568. djtodoro retitled this revision from "Add option for emitting dbg info for call sites" to "Add option for emitting dbg info for call site parameters". djtodoro added a comment. -Refactor -Remove `CC1` def CHANGES SINCE LAST ACTION

[PATCH] D58033: Add option for emitting dbg info for call sites

djtodoro marked an inline comment as done. djtodoro added inline comments. Comment at: include/clang/Driver/Options.td:919 HelpText<"Do not use jump tables for lowering switches">; +def emit_param_entry_values : Joined<["-"], "femit-param-entry-values">, +

[PATCH] D58033: Add option for emitting dbg info for call site parameters

djtodoro updated this revision to Diff 195571. djtodoro added a comment. -Fix comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/ https://reviews.llvm.org/D58033 Files: include/clang/Basic/CodeGenOptions.def include/clang/Driver/Options.td

[PATCH] D58043: Add experimental options for call site related dbg info

djtodoro updated this revision to Diff 195572. djtodoro added a comment. -Rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58043/new/ https://reviews.llvm.org/D58043 Files: lib/Driver/ToolChains/Clang.cpp Index: lib/Driver/ToolChains/Clang.cpp

[PATCH] D58033: Add option for emitting dbg info for call site parameters

djtodoro updated this revision to Diff 196053. djtodoro added a comment. -Add only cc1 option -Set up back end CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/ https://reviews.llvm.org/D58033 Files: include/clang/Basic/CodeGenOptions.def include/clang/Driver/CC1Options.td

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

djtodoro added a comment. I think that it sounds reasonable to avoid declaration subprograms for functions with reserved names, so that part looks good to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69970/new/ https://reviews.llvm.org/D69970

[PATCH] D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag

This revision was automatically updated to reflect the committed changes. Closed by commit rG1643734741d2: [clang] Remove the DIFlagArgumentNotModified debug info flag (authored by djtodoro). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github

[PATCH] D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag

djtodoro reopened this revision. djtodoro added a comment. This revision is now accepted and ready to land. After landing this, I see one LLDB test depends on this. I made a workaround to avoid testing it at the moment. I will remove the workaround when landing the D68209

[PATCH] D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag

djtodoro updated this revision to Diff 229495. djtodoro added a comment. -Add the workaround in the LLDB test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68206/new/ https://reviews.llvm.org/D68206 Files: clang/lib/CodeGen/CGDebugInfo.cpp clang/lib/CodeGen/CGDebugInfo.h

[PATCH] D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag

djtodoro added a comment. I will do that way. Yes, only the entry values test fails, so we should skip only that one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68206/new/ https://reviews.llvm.org/D68206 ___ cfe-commits mailing list

[PATCH] D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag

This revision was automatically updated to reflect the committed changes. Closed by commit rGce1f95a6e077: Reland [clang] Remove the DIFlagArgumentNotModified debug info flag (authored by djtodoro). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM

[PATCH] D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag

djtodoro added a comment. @vsk I have created the bug (llvm.org/pr44059), do you think this is OK? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68206/new/ https://reviews.llvm.org/D68206 ___ cfe-commits mailing list

[PATCH] D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag

djtodoro updated this revision to Diff 230054. djtodoro added a comment. -Update the lldb test for the entry values feature CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68206/new/ https://reviews.llvm.org/D68206 Files: clang/lib/CodeGen/CGDebugInfo.cpp

[PATCH] D69743: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood

djtodoro accepted this revision. djtodoro added a comment. This revision is now accepted and ready to land. @vsk Thanks for this! (lgtm) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69743/new/ https://reviews.llvm.org/D69743 ___

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

djtodoro added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:803 + if (Tag == dwarf::DW_TAG_typedef && DD->getDwarfVersion() >= 5) { +uint32_t AlignInBytes = DTy->getAlignInBytes(); Please add a comment here.

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

djtodoro added a comment. @awpandey Thanks for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70111/new/ https://reviews.llvm.org/D70111 ___ cfe-commits mailing list

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

djtodoro added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:169 +* C DebugInfo API ``LLVMDIBuilderCreateTypedef`` is updated to include an extra +argument ``AlignInBits``, to facilitate / propagate specified Alignment information

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

djtodoro added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:169 +* C DebugInfo API ``LLVMDIBuilderCreateTypedef`` is updated to include an extra +argument ``AlignInBits``, to facilitate / propagate specified Alignment information

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

This revision was automatically updated to reflect the committed changes. Closed by commit rG9f6ff07f8a39: [DebugInfo] Enable the debug entry values feature by default (authored by djtodoro). Herald added subscribers: lldb-commits, cfe-commits, jrtc27. Herald added projects: clang, LLDB. Changed

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro marked an inline comment as done. djtodoro added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:870 if (MI->isCandidateForCallSiteEntry() && -DAG->getTarget().Options.EnableDebugEntryValues) +

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. Reverted due to http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/21373/steps/build-stage3-compiler/logs/stdio. I will reland this as soon as I fix the issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D73261: [dwarf5] Support DebugInfo for constexpr for C++ variables and functions

djtodoro added a comment. I agree, we must pay attention and address the bitcode compatibility. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73261/new/ https://reviews.llvm.org/D73261 ___ cfe-commits mailing list

[PATCH] D73261: [dwarf5] Support DebugInfo for constexpr for C++ variables and functions

djtodoro added a comment. > Is it necessary to use DIFlags? I am willing to do that but generally, it is > not welcomed because we have a limited number of DIFlags and most of them are > currently in use. Are there any flags within `DIFlags` that could be applied only to functions? If so, we

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro updated this revision to Diff 245123. djtodoro added a comment. -Addressing the latest comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ https://reviews.llvm.org/D73534 Files: clang/include/clang/Basic/CodeGenOptions.def

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro updated this revision to Diff 245345. djtodoro added a comment. - Address the issue with ARM `describeLoadedValue()` (thanks to @vsk, I've reduced the test `llvm/test/DebugInfo/MIR/ARM/dbgcallsite-noreg-is-imm-check.mir`) CHANGES SINCE LAST ACTION

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. In D73534#1882118 , @dstenb wrote: > In D73534#1881353 , @nickdesaulniers > wrote: > > > As a heads up, Linaro's ToolChain Working Group's Linux kernel CI lit up on > > this change. I

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. Nice! Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ https://reviews.llvm.org/D73534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. @ostannard Thanks for reporting that! Please share the case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ https://reviews.llvm.org/D73534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. Reverted again with rG2f215cf36adc . The investigation is needed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ https://reviews.llvm.org/D73534

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. In D73534#1883022 , @nickdesaulniers wrote: > In D73534#1882136 , @djtodoro wrote: > > > - Address the issue with ARM `describeLoadedValue()` (thanks to @vsk, I've > > reduced the test

[PATCH] D73261: [dwarf5] Support DebugInfo for constexpr for C++ variables and functions

djtodoro added inline comments. Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:61 HANDLE_DI_FLAG((1 << 29), AllCallsDescribed) +HANDLE_DI_FLAG((1 << 30), ConstExpr) We are almost out of space here... I wouldn't add any additional flag here before we do

[PATCH] D73261: [dwarf5] Support DebugInfo for constexpr for C++ variables and functions

djtodoro added a comment. @awpandey Thanks for doing this, but could you please explain the motivation of implementing this? Can we use is (somehow) to generate more variables with the `DW_AT_const_value`? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73261/new/

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. Thanks for reporting that, are you sure this was the cause of the failure? I'll revert this while investigate, but it does not seem to me this is related to this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ https://reviews.llvm.org/D73534

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. I’ve already reverted the patch, but I’ll reland it again tomorrow. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ https://reviews.llvm.org/D73534 ___ cfe-commits mailing list

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

djtodoro added a comment. I guess this should be closed? :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69970/new/ https://reviews.llvm.org/D69970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

djtodoro added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:169 +* C DebugInfo API ``LLVMDIBuilderCreateTypedef`` is updated to include an extra +argument ``AlignInBits``, to facilitate / propagate specified Alignment information

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. In D73534#1916309 , @djtodoro wrote: > In D73534#1916291 , @djtodoro wrote: > > > In D73534#1915048 , @mstorsjo > > wrote: > > > > > This broke

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. In D73534#1915048 , @mstorsjo wrote: > This broke compiling for mingw, repro.c: > > a(short); > b() { a(1); } > > > `clang -target x86_64-w64-mingw32 -c repro.c -g -O2`, which gives `Assertion > '!MI.isMoveImmediate() &&

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. In D73534#1916291 , @djtodoro wrote: > In D73534#1915048 , @mstorsjo wrote: > > > This broke compiling for mingw, repro.c: > > > > a(short); > > b() { a(1); } > > > > > > `clang

[PATCH] D75175: [CallSiteInfo] Enable the call site info only for -g + optimizations

This revision was automatically updated to reflect the committed changes. Closed by commit rGc15c68abdc6f: [CallSiteInfo] Enable the call site info only for -g + optimizations (authored by djtodoro). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. Why reverting this one? Is it a different assertion? The D75036 was the problem with the previous issue reported. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ https://reviews.llvm.org/D73534

[PATCH] D79967: Fix debug info for NoDebug attr

djtodoro added a comment. This seems reasonable, so this change looks good to me! @dblaikie Thanks for pointing out to the potential problems of the usage of the func decl !dbg in the purpose of call sites debug info. It is currently being stored into CU's retainedTypes field. > That's why

[PATCH] D78105: [CSInfo][ISEL] Call site info generation support for Mips

This revision was automatically updated to reflect the committed changes. Closed by commit rG170ac4be3392: [CSInfo][ISEL] Call site info generation support for Mips (authored by djtodoro). Herald added subscribers: cfe-commits, jrtc27. Herald added a project: clang. Repository: rG LLVM Github

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. Since we landed the fix for the issue related to the D75036 , I'll reland this again. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ https://reviews.llvm.org/D73534 ___

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. Re-enabled with the d9b962100942 . If we face a failure again, since this enables the whole feature, I recommend reverting smaller pieces of the feature that was causing the problem, rather than

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. In D73534#1933985 , @djtodoro wrote: > Oh sorry, I thought it all has been fixed, since all the tests pass. > > We should revert the D75036 until we fix > all the issues. @dstenb do you agree?

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. Oh sorry, I thought it all has been fixed, since all the tests pass. We should revert the D75036 until we fix all the issues. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73534/new/ https://reviews.llvm.org/D73534

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

djtodoro added a comment. In D73534#1934105 , @vsk wrote: > In D73534#1933988 , @djtodoro wrote: > > > In D73534#1933985 , @djtodoro > > wrote: > > > > > Oh sorry, I

[PATCH] D80369: [DebugInfo] Remove decl subprograms from 'retainedTypes:'

djtodoro created this revision. djtodoro added reviewers: dblaikie, aprantl, vsk. djtodoro added projects: LLVM, debug-info. Herald added a project: clang. Herald added a subscriber: cfe-commits. After the D70350 , the `retainedTypes:` isn't being used for the

[PATCH] D80369: [DebugInfo][CallSites] Remove decl subprograms from 'retainedTypes:'

djtodoro added a comment. In D80369#2050022 , @dblaikie wrote: > In D80369#2048932 , @djtodoro wrote: > > > Still have test failing: > > > > Clang :: Modules/DebugInfoTransitiveImport.m > > Clang ::

[PATCH] D80369: [DebugInfo][CallSites] Remove decl subprograms from 'retainedTypes:'

djtodoro updated this revision to Diff 265722. djtodoro retitled this revision from "WIP: [DebugInfo] Remove decl subprograms from 'retainedTypes:'" to "[DebugInfo][CallSites] Remove decl subprograms from 'retainedTypes:'". djtodoro added a comment. -Remove the decls only in the case of

[PATCH] D83048: [LiveDebugValues] 3/4 Add Xclang and CodeGen options for using instr-ref variable locations

djtodoro accepted this revision. djtodoro added a comment. This revision is now accepted and ready to land. nit included, otherwise lgtm, thanks! Comment at: llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp:45 + ~LiveDebugValues() + { +if (TheImpl)

[PATCH] D82547: [VerifyDIPreserve] Expose original debuginfo preservation check as CC1 option

djtodoro updated this revision to Diff 294658. djtodoro added a comment. -Rebasing CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82547/new/ https://reviews.llvm.org/D82547 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Basic/CodeGenOptions.h

[PATCH] D82547: [Debugify] Expose debugify (original mode) as CC1 option

djtodoro updated this revision to Diff 292469. djtodoro added a comment. - Rebasing CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82547/new/ https://reviews.llvm.org/D82547 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Basic/CodeGenOptions.h

[PATCH] D80369: [DebugInfo][CallSites] Remove decl subprograms from 'retainedTypes:'

djtodoro added a comment. >> @dblaikie wrote: > > ... At least for the C++ test, this change makes it pass: > > diff --git clang/test/Modules/ModuleDebugInfo.cpp > clang/test/Modules/ModuleDebugInfo.cpp > index 26369c89605..b1ffe27ec22 100644 > --- clang/test/Modules/ModuleDebugInfo.cpp

[PATCH] D80369: [DebugInfo][CallSites] Remove decl subprograms from 'retainedTypes:'

djtodoro updated this revision to Diff 266776. djtodoro added a comment. -Tests clean up CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80369/new/ https://reviews.llvm.org/D80369 Files: clang/lib/CodeGen/CGDebugInfo.cpp clang/test/CodeGen/debug-info-extern-call.c

[PATCH] D80369: [DebugInfo][CallSites] Remove decl subprograms from 'retainedTypes:'

djtodoro added a comment. In D80369#2057866 , @dblaikie wrote: > Not sure I follow - why was it a problem that there was no DISubprogram at > all? Actually, since the DISubprograms from the retained types don't affect the final DWARF, it's not a

  1   2   >