[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg added a comment. In D110804#3035276 , @JDevlieghere wrote: > In D110804#3033689 , @JDevlieghere > wrote: > >> We have (short term) plans to start using the statistics to collect more >> data. Vedant cr

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg updated this revision to Diff 376421. clayborg added a comment. Changes: - Removed "target metrics" and took over "statistics dump" - Hooked old statistics into for expression evaluation and frame variable success and fails into this patch - No more textual output for "statistics dump",

[Lldb-commits] [PATCH] D110885: [NFC][AttributeList] Replace index_begin/end with an iterator

aeubanks updated this revision to Diff 376420. aeubanks added a comment. add comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110885/new/ https://reviews.llvm.org/D110885 Files: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScrip

[Lldb-commits] [PATCH] D110885: [NFC][AttributeList] Replace index_begin/end with an iterator

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: llvm/include/llvm/IR/Attributes.h:854 - /// Use these to iterate over the valid attribute indices. - unsigned index_begin() const { return AttributeList::Funct

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

jingham updated this revision to Diff 376410. jingham added a comment. My brain wants llvm::Error to be false when there's an error, (probably because false is the bad state.) Fix a case where I didn't catch myself... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[Lldb-commits] [PATCH] D110893: [lldb] Refactor statistics (NFC)

clayborg added a comment. I will have the new version that implements many of these changes submitted by tonight CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110893/new/ https://reviews.llvm.org/D110893 ___ lldb-commits mailing list lldb-co

[Lldb-commits] [PATCH] D110893: [lldb] Refactor statistics (NFC)

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. please don't commit this. I am in the process of doing very similar stuff in https://reviews.llvm.org/D110804 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110893/new/

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

jingham marked 9 inline comments as done. jingham added a comment. Addresses Jonas' first round of comments. Comment at: lldb/source/Commands/CommandCompletions.cpp:811-813 + sort(to_delete.begin(), to_delete.end(), std::greater()); + for (size_t idx : to_delete) +args.De

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

jingham updated this revision to Diff 376406. jingham marked an inline comment as done. jingham added a comment. Addressed Jonas' review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110298/new/ https://reviews.llvm.org/D110298 Files:

[Lldb-commits] [PATCH] D110893: [lldb] Refactor statistics (NFC)

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Mechanically, this sgtm. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110893/new/ https://reviews.llvm.org/D110893 ___ lldb-commits mai

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

JDevlieghere added a comment. In D110804#3033689 , @JDevlieghere wrote: > We have (short term) plans to start using the statistics to collect more > data. Vedant created some patches last year that I've rebased but haven't > landed yet. I'll try to get

[Lldb-commits] [PATCH] D110895: [lldb] Move ownership of analytics out of Target

JDevlieghere created this revision. JDevlieghere added reviewers: clayborg, labath, jingham. Herald added a subscriber: yaxunl. JDevlieghere requested review of this revision. Move ownership of analytics out of Target and instead create a global Analytics singleton. This is necessary to allow LLD

[Lldb-commits] [PATCH] D110893: [lldb] Refactor statistics (NFC)

JDevlieghere updated this revision to Diff 376398. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110893/new/ https://reviews.llvm.org/D110893 Files: lldb/include/lldb/Target/Target.h lldb/include/lldb/Utility/Analytics.h lldb/include/lldb/Utility/Metrics.def lldb/include/lldb/lld

[Lldb-commits] [PATCH] D110893: [lldb] Refactor statistics (NFC)

JDevlieghere created this revision. JDevlieghere added reviewers: clayborg, jingham, labath. Herald added a subscriber: mgorny. JDevlieghere requested review of this revision. Refactor LLDB's current statistics into a single class named Analytics which consists of different metrics. https://rev

[Lldb-commits] [PATCH] D110885: [NFC][AttributeList] Replace index_begin/end with an iterator

aeubanks updated this revision to Diff 376381. aeubanks added a comment. auto -> unsigned Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110885/new/ https://reviews.llvm.org/D110885 Files: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderS

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

jingham added a comment. In D110804#3035052 , @clayborg wrote: > Ok, so how does this sound: > > - This new command will replace the existing "statistics dump" command and > emit JSON only by default > - The "statistics enable/disable" will stay in place

[Lldb-commits] [PATCH] D110885: [NFC][AttributeList] Replace index_begin/end with an iterator

shafik added inline comments. Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:838 AttributeList AL = Attrs[i]; -for (unsigned i = AL.index_begin(), e = AL.index_end(); i != e; ++i) { +for (auto i : AL.indexes()) { AttributeSet AS = AL.getAttributes(i); -

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg added a comment. Ok, so how does this sound: - This new command will replace the existing "statistics dump" command and emit JSON only by default - The "statistics enable/disable" will stay in place and allow expensive metric gathering to be enabled/disabled if needed. If the gathering

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

jingham added a comment. In D110804#3034878 , @clayborg wrote: > I would like to get a consensus on if we should move this functionality to > "statistics" or not. The reasons that I didn't do it were: > > - "statistics" as a top level method doesn't real

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

JDevlieghere added a comment. In D110804#3034878 , @clayborg wrote: > I would like to get a consensus on if we should move this functionality to > "statistics" or not. The reasons that I didn't do it were: > > - "statistics" as a top level method doesn't

[Lldb-commits] [PATCH] D110885: [AttributeList] Replace index_begin/end with an iterator

aeubanks created this revision. Herald added subscribers: dexonsmith, jdoerfert, arphaman, hiraditya. aeubanks requested review of this revision. Herald added projects: LLDB, LLVM. Herald added subscribers: llvm-commits, lldb-commits. Herald added a subscriber: JDevlieghere. We expose the fact tha

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg added a comment. I would like to get a consensus on if we should move this functionality to "statistics" or not. The reasons that I didn't do it were: - "statistics" as a top level method doesn't really do what I wanted here, which was "target statistics", or stats that are related onl

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg marked 6 inline comments as done. clayborg added inline comments. Comment at: lldb/include/lldb/Core/Module.h:940 + /// ElapsedTime + double &GetSymtabParseTime() { return m_symtab_parse_time; } + double &GetSymtabNamesTime() { return m_symtab_names_time; } --

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg updated this revision to Diff 376358. clayborg added a comment. Changes: - Swithed from using "double" to using ElapsedTime::Duration - Stop using assertTrue in tests, and use better assertXXX methods Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. In D110571#3033685 , @jarin wrote: > Cache only global variables. I concur that we should only cache globals. So now we have unique variables for

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

clayborg added a comment. Should we be testing if these directories exist before trying to use them? Might be nice to avoid compiler warnings if the compiler will emit warnings or errors if these directories don't exist. LLDB also tests with compilers that were built, like when LLDB builds clan

[Lldb-commits] [PATCH] D110878: [lldb] Add a gdb_remote_client test for connecting to pty [WIP]

mgorny added a comment. @labath, this is the part where I ask for help ;-). An exception happened when receiving the response from the gdb server. Closing the client... Traceback (most recent call last): File "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client

[Lldb-commits] [PATCH] D110878: [lldb] Add a gdb_remote_client test for connecting to pty [WIP]

mgorny created this revision. mgorny added reviewers: labath, teemperor, krytarowski, emaste. mgorny requested review of this revision. Add a minimal mock server utilizing a pty, and add a client test connecting to that server. https://reviews.llvm.org/D110878 Files: lldb/test/API/functionali

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg added a comment. In D110804#3032741 , @labath wrote: > I have a feeling that there's a lot of overlap between this and the > "statistics" command. As a user, I don't think I would be able to tell the > difference between these two things. > > H

[Lldb-commits] [PATCH] D110721: [lldb] [Host] Refactor TerminalState

mgorny updated this revision to Diff 376271. mgorny added a comment. Move saving/restoring termios back into `TerminalState`, make `Impl` a trivial struct. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110721/new/ https://reviews.llvm.org/D110721 Files: lldb/include/lldb/Host/Termin

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

teemperor added a comment. Thanks for the patch! Out of curiosity: What distribution is using target-specific include paths? I can take a look at this tomorrow, but just from scrolling over I think the general direction of this patch seems fine, so I think I only have a few nits here and there

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

JDevlieghere added a comment. In D110804#3032741 , @labath wrote: > I have a feeling that there's a lot of overlap between this and the > "statistics" command. As a user, I don't think I would be able to tell the > difference between these two things. >

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin updated this revision to Diff 376235. jarin marked an inline comment as done. jarin added a comment. Cache only global variables. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110571/new/ https://reviews.llvm.org/D110571 Files: lldb/source

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

labath added a comment. In D110571#3033282 , @jarin wrote: > In D110571#3033078 , @labath wrote: > >> Here's one more question. AIUI, lldb relies on the order of formal parameter >> declarations in dwarf to estab

[Lldb-commits] [PATCH] D110721: [lldb] [Host] Refactor TerminalState

mgorny updated this revision to Diff 376191. mgorny edited the summary of this revision. mgorny added a comment. Move termios data to PImpl-style subclass. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110721/new/ https://reviews.llvm.org/D110721 Files: lldb/include/lldb/Host/Termina

[Lldb-commits] [PATCH] D110721: [lldb] [Host] Refactor TerminalState

mgorny marked 2 inline comments as done. mgorny added inline comments. Comment at: lldb/include/lldb/Host/Terminal.h:51 /// descriptor and later restore that state as it originally was. class TerminalState { public: labath wrote: > I guess it would be good to

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin added a comment. In D110571#3033078 , @labath wrote: > Here's one more question. AIUI, lldb relies on the order of formal parameter > declarations in dwarf to establish the the function signature (dwarf doesn't > leave us much choice. This then af

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

kpdev42 created this revision. kpdev42 added reviewers: clayborg, granata.enrico, labath. kpdev42 added a project: LLDB. Herald added subscribers: JDevlieghere, pengfei. kpdev42 requested review of this revision. Herald added a subscriber: lldb-commits. On Linux some C++ and C include files reside

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

labath added a comment. Here's one more question. AIUI, lldb relies on the order of formal parameter declarations in dwarf to establish the the function signature (dwarf doesn't leave us much choice. This then affects how the function is printed in the backtrace, for instance. What will be the

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

teemperor added inline comments. Comment at: lldb/include/lldb/Core/Module.h:940 + /// ElapsedTime + double &GetSymtabParseTime() { return m_symtab_parse_time; } + double &GetSymtabNamesTime() { return m_symtab_names_time; } labath wrote: > teemperor wrote: >

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

labath added inline comments. Comment at: lldb/include/lldb/Core/Module.h:940 + /// ElapsedTime + double &GetSymtabParseTime() { return m_symtab_parse_time; } + double &GetSymtabNamesTime() { return m_symtab_names_time; } teemperor wrote: > Could the `double`

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

teemperor added subscribers: vsk, teemperor. teemperor added a comment. +Vedant who had/has some plans with the statistics command at some point IIRC. FWIW, I wanted to throw away the current statistics implementation for quite a while but I didn't have anything to replace it with. If we can rep

Re: [Lldb-commits] [lldb] f939a32 - [lldb] Fix TestImportStdModule on some setups by testing minmax instead of abs

Cool, thanks! On Thu, 30 Sept 2021 at 11:12, Raphael Isemann wrote: > I'm pretty sure this isn't going to fix that specific bug. I've seen a > similar failure on one of my machines though, so let me see if fixing > that also fixes your bug. > > Am Do., 30. Sept. 2021 um 10:48 Uhr schrieb Diana P

Re: [Lldb-commits] [lldb] f939a32 - [lldb] Fix TestImportStdModule on some setups by testing minmax instead of abs

I'm pretty sure this isn't going to fix that specific bug. I've seen a similar failure on one of my machines though, so let me see if fixing that also fixes your bug. Am Do., 30. Sept. 2021 um 10:48 Uhr schrieb Diana Picus : > > Would this fix https://bugs.llvm.org/show_bug.cgi?id=51446 by any cha

Re: [Lldb-commits] [lldb] f939a32 - [lldb] Fix TestImportStdModule on some setups by testing minmax instead of abs

Would this fix https://bugs.llvm.org/show_bug.cgi?id=51446 by any chance? Should I give it a spin on the release branch? On Wed, 29 Sept 2021 at 17:04, Raphael Isemann via lldb-commits < lldb-commits@lists.llvm.org> wrote: > > Author: Raphael Isemann > Date: 2021-09-29T17:03:37+02:00 > New Revisi

[Lldb-commits] [PATCH] D110721: [lldb] [Host] Refactor TerminalState

labath added a comment. Overall, this seems like an improvement, though the class is still quite far from what I would consider an ideal state. Comment at: lldb/include/lldb/Host/Terminal.h:51 /// descriptor and later restore that state as it originally was. class TerminalSt

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

labath added a comment. I have a feeling that there's a lot of overlap between this and the "statistics" command. As a user, I don't think I would be able to tell the difference between these two things. Having two systems which do vaguely similar things does not seem like a good idea. Have yo

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg created this revision. clayborg added reviewers: jingham, labath, aprantl, JDevlieghere, aadsm, wallace. Herald added subscribers: dang, pengfei, arphaman, mgorny, emaste. clayborg requested review of this revision. Herald added subscribers: lldb-commits, sstefan1, MaskRay. Herald added a

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin updated this revision to Diff 376124. jarin marked an inline comment as done. jarin added a comment. Improved the comment, as Greg suggested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110571/new/ https://reviews.llvm.org/D110571 Files: