[Lldb-commits] [PATCH] D130054: [trace][intel pt] Introduce wall clock time for each trace item

wallace updated this revision to Diff 446303. wallace edited the summary of this revision. wallace added a comment. final version Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130054/new/ https://reviews.llvm.org/D130054 Files: lldb/include/lldb

[Lldb-commits] [PATCH] D130054: [trace][intel pt] Introduce wall clock time for each trace item

wallace updated this revision to Diff 446306. wallace added a comment. fix test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130054/new/ https://reviews.llvm.org/D130054 Files: lldb/include/lldb/Target/TraceCursor.h lldb/include/lldb/Target/T

[Lldb-commits] [PATCH] D130054: [trace][intel pt] Introduce wall clock time for each trace item

wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:288 +decoded_thread.NotifyTsc(execution.thread_execution.GetLowestKnownTSC()); decoded_thread.NotifyCPU(execution.thread_execution.cpu_id); jj10306 wrote: >

[Lldb-commits] [PATCH] D130054: [trace][intel pt] Introduce wall clock time for each trace item

wallace updated this revision to Diff 446541. wallace added a comment. fix a bug that happens when the initial trace items don't have timing information Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130054/new/ https://reviews.llvm.org/D130054 Fi

[Lldb-commits] [PATCH] D130320: Move GetControlFlowKind's logic to DisassemblerLLVMC.cpp

wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. great! thank you. I'll land this CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130320/new/ https://reviews.llvm.org/D130320 ___ lldb-com

[Lldb-commits] [PATCH] D130320: Move GetControlFlowKind's logic to DisassemblerLLVMC.cpp

wallace added a comment. Just one more change and good to go Comment at: lldb/include/lldb/Core/Disassembler.h:87 + virtual lldb::InstructionControlFlowKind + GetControlFlowKind(const ExecutionContext *exe_ctx) = 0; could add a default implementation here?

[Lldb-commits] [PATCH] D130580: Refactor string conversion for InstructionControlFlowKind enum

wallace accepted this revision. wallace added a comment. just remove that comment. I've just pushed Sujin's diff as well, so you can rebase Comment at: lldb/include/lldb/lldb-enumerations.h:975-976 /// A single instruction can match one or more of these categories. +/// The e

[Lldb-commits] [PATCH] D130924: [NFC][trace] Update TraceIntelPTBundleSaver.cpp to accommodate FileSpec API changes

wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130924/new/ https://reviews.llvm.org/D130924 __

[Lldb-commits] [PATCH] D130805: [trace][intel pt] Support a new kernel section in LLDB’s trace bundle schema

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. Are the files in `lldb/test/API/commands/trace/intelpt-kernel-trace/cores/` actual kernel traces? If not, just use some trace files that are already present in the repo. You can us

[Lldb-commits] [PATCH] D130925: [trace] Replace TraceCursorUP with TraceCursorSP

wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. great! that will make all the bindings very easy to handle Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130925/new/ https://reviews.llvm.org/

[Lldb-commits] [PATCH] D130930: [trace] Add SBTraceCursor bindings

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. pretty nice!! Just some few minor changes and good to go Comment at: lldb/bindings/interface/SBTraceCursor.i:1-7 +//===-- SWIG Interface for SBTraceCursor.h -

[Lldb-commits] [PATCH] D130805: [trace][intel pt] Support a new kernel section in LLDB’s trace bundle schema

wallace added a comment. > qq: Do we plan to add this kernel tracing support for live tracing as well? I think it won't be possible without major changes or at least a big discussion because LLDB is supposed to operated on processes it's debugging, so let's better leave it out of scope for now.

[Lldb-commits] [PATCH] D130805: [trace][intel pt] Support a new kernel section in LLDB’s trace bundle schema

wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:83 JSONTraceBundleDescription &bundle_description, ArrayRef traced_processes, -ArrayRef traced_threads) { +ArrayRef traced_threads, TraceMode trace_mode) { TraceIntelPT

[Lldb-commits] [PATCH] D130930: [trace] Add SBTraceCursor bindings

wallace added inline comments. Comment at: lldb/bindings/interface/SBTraceCursor.i:56 + + explicit operator bool() const; +}; +1 Comment at: lldb/include/lldb/API/SBTraceCursor.h:21 +public: + /// Default constructor for an invalid \a SBTrace

[Lldb-commits] [PATCH] D131005: [LLDB] Add SBInstruction::GetControlFlowKind()

wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. great! Comment at: lldb/source/API/SBInstruction.cpp:176 +if (target_sp) { + lock = std::unique_lock(target_sp->GetAPIMutex()); + this might be po

[Lldb-commits] [PATCH] D130805: [trace][intel pt] Support a new kernel section in LLDB’s trace bundle schema

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. just some minor cosmetic changes :) Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp:177-184 + if (bundle_description.kernel->load_ad

[Lldb-commits] [PATCH] D130930: [trace] Add SBTraceCursor bindings

wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. yaay Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130930/new/ https://reviews.llvm.org/D130930 _

[Lldb-commits] [PATCH] D130805: [trace][intel pt] Support a new kernel section in LLDB’s trace bundle schema

wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTConstants.h:26 +const lldb::addr_t kDefaultKernelLoadAddress = 0x8100; +const lldb::pid_t kDefaultKernelProcessID = 1; mention that github link here CHANGES

[Lldb-commits] [PATCH] D131081: [lldb] Prevent race condition when fetching /proc/cpuinfo

wallace created this revision. wallace added reviewers: clayborg, jj10306. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. @clayborg found a potential race condition when setting a static variable. The f

[Lldb-commits] [PATCH] D131081: [lldb] Prevent race condition when fetching /proc/cpuinfo

wallace updated this revision to Diff 449724. wallace added a comment. use an idea similar to labath's to simplify this code while still using call_once Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131081/new/ https://reviews.llvm.org/D131081 Fi

[Lldb-commits] [PATCH] D131081: [lldb] Prevent race condition when fetching /proc/cpuinfo

wallace added inline comments. Comment at: lldb/source/Plugins/Process/Linux/Procfs.cpp:12 #include "lldb/Host/linux/Support.h" + #include "llvm/Support/MemoryBuffer.h" labath wrote: > btw, llvm does not generally put blank lines between include headers. > omi

[Lldb-commits] [PATCH] D130805: [trace][intel pt] Support a new kernel section in LLDB’s trace bundle schema

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. almost there! Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:104-108 std::vector tids; for (const JSONProcess &process : bundle_descrip

[Lldb-commits] [PATCH] D130805: [trace][intel pt] Support a new kernel section in LLDB’s trace bundle schema

wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp:306 +- "processes" is provided if and only if "kernel" is not provided. +- If "kernel" is provided, then the "processes" section must be empty and the "cpus" section must b

[Lldb-commits] [PATCH] D130805: [trace][intel pt] Support a new kernel section in LLDB’s trace bundle schema

wallace accepted this revision. wallace added a comment. nice job! Now you just need to connect it with the collector :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130805/new/ https://reviews.llvm.org/D130805 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D131081: [lldb] Prevent race condition when fetching /proc/cpuinfo

wallace updated this revision to Diff 450226. wallace added a comment. simplify this diff following @labath's advice Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131081/new/ https://reviews.llvm.org/D131081 Files: lldb/source/Plugins/Process/Li

[Lldb-commits] [PATCH] D131081: [lldb] Prevent race condition when fetching /proc/cpuinfo

wallace added a comment. That's important to know. At least in this case this code only runs on Linux, so hopefully we are good with the atomic static initialization. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131081/new/ https://reviews.llvm.o

[Lldb-commits] [PATCH] D131630: [trace][intel pt] Fix per-psb packet decoding

wallace created this revision. wallace added reviewers: jj10306, persona0220. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The per-PSB packet decoding logic was wrong because it was assuming that pt_

[Lldb-commits] [PATCH] D131630: [trace][intel pt] Fix per-psb packet decoding

wallace updated this revision to Diff 451670. wallace edited the summary of this revision. wallace added a comment. nit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131630/new/ https://reviews.llvm.org/D131630 Files: lldb/include/lldb/Target/Tr

[Lldb-commits] [PATCH] D131630: [trace][intel pt] Fix per-psb packet decoding

wallace updated this revision to Diff 451685. wallace added a comment. improve documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131630/new/ https://reviews.llvm.org/D131630 Files: lldb/include/lldb/Target/TraceCursor.h lldb/include/

[Lldb-commits] [PATCH] D131630: [trace][intel pt] Fix per-psb packet decoding

wallace marked 4 inline comments as done. wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:476-478 + if (event.has_tsc) { +tsc = event.tsc; +break; jj10306 wrote: > so is this inner loop what's a

[Lldb-commits] [PATCH] D131630: [trace][intel pt] Fix per-psb packet decoding

wallace updated this revision to Diff 452313. wallace marked an inline comment as done. wallace added a comment. update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131630/new/ https://reviews.llvm.org/D131630 Files: lldb/include/lldb/Target/Tr

[Lldb-commits] [PATCH] D121711: [trace][intelpt] Rename IntelPTManager and change TraceCursor::GetTimestampCounter API to general trace counter API

wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. just fix these two simple comments and good to go Comment at: lldb/include/lldb/Target/TraceCursor.h:188-189 + /// \param[in] counter_type + ///The counter type //

[Lldb-commits] [PATCH] D121935: added intel-pt build instructions for lldb

wallace requested changes to this revision. wallace added a comment. Thank you very much for helping out with the documentaiton! As Jonas said, move the file to lldb/docs/intel_pt.rst and modify the necessary bits (some CMake changes) so that the documentation eventually appears in https://lldb

[Lldb-commits] [PATCH] D121935: added intel-pt build instructions for lldb

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. Pretty nice improvement! Now you need to reference this doc from the main page. See this reference patch https://reviews.llvm.org/D82064. According to it, you need to move this doc

[Lldb-commits] [PATCH] D121935: added intel-pt build instructions for lldb

wallace accepted this revision. wallace added a comment. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121935/new/ https://reviews.llvm.org/D121935 ___ lldb-commits mailing list lldb-commits@lists.l

[Lldb-commits] [PATCH] D122023: [trace][intelpt] fix some test failures

wallace created this revision. wallace added a reviewer: jj10306. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Minor fixes needed and now `./bin/lldb-dotest -p TestTrace` passes correctly. - There wa

[Lldb-commits] [PATCH] D122023: [trace][intelpt] fix some test failures

wallace updated this revision to Diff 416558. wallace added a comment. Herald added a subscriber: JDevlieghere. nit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122023/new/ https://reviews.llvm.org/D122023 Files: lldb/source/Commands/CommandObj

[Lldb-commits] [PATCH] D122073: [docs] Fixed minor ordering issue

wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. Herald added a subscriber: JDevlieghere. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122073/new/ https://reviews.llvm.org/D122073 _

[Lldb-commits] [PATCH] D122076: [trace][intelpt] Instruction count in trace info

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. just some minor details and good to go Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:108 Optional raw_size = GetRawTraceSize(thread); + size

[Lldb-commits] [PATCH] D122076: [trace][intelpt] Instruction count in trace info

wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. Lgtm! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122076/new/ https://reviews.llvm.org/D122076

[Lldb-commits] [PATCH] D122093: [trace][intelpt] Added total memory usage by decoded trace

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. Pretty nice! just some details i've noticed when reading your diff and good to go Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:121-126 + siz

[Lldb-commits] [PATCH] D122093: [trace][intelpt] Added total memory usage by decoded trace

wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:121-126 + size_t total = sizeof(m_raw_trace_size); + + for (const IntelPTInstruction &ins : m_instructions) +total += ins.GetMemoryUsage(); + + return total; wa

[Lldb-commits] [PATCH] D122093: [trace][intelpt] Added total memory usage by decoded trace

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. just some few remaining nits Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:85 + + /// Get the size in bytes of non-error instance of this clas

[Lldb-commits] [PATCH] D122093: [trace][intelpt] Added total memory usage by decoded trace

wallace requested changes to this revision. wallace added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:123-130 + size_t non_err_instruction_count = 0; + for (const IntelPTInstruction &insn : m_i

[Lldb-commits] [PATCH] D122093: [trace][intelpt] Added total memory usage by decoded trace

wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. great! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122093/new/ https://reviews.llvm.org/D122093 ___

[Lldb-commits] [PATCH] D122176: [trace] clear any existing tracing sessions before relaunching the binary

wallace created this revision. wallace added reviewers: jj10306, zrthxn. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. There's a bug caused when a process is relaunched: the target, which doesn't chang

[Lldb-commits] [PATCH] D122176: [trace] clear any existing tracing sessions before relaunching the binary

wallace updated this revision to Diff 417082. wallace added a comment. improve test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122176/new/ https://reviews.llvm.org/D122176 Files: lldb/source/Target/Target.cpp lldb/test/API/commands/trace/Te

[Lldb-commits] [PATCH] D122192: [trace] Use vector instead of ArrayRef when reading data

wallace created this revision. wallace added reviewers: jj10306, zrthxn. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. I incorrectly returned an ArrayRef when the underlying object didn't own the data.

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

wallace created this revision. wallace added reviewers: jj10306, clayborg, zrthxn. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. In order to support quick arbitrary access to instructions in the trace,

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

wallace updated this revision to Diff 417389. wallace marked 2 inline comments as done. wallace added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122254/new/ https://reviews.llvm.org/D122254 Files: lldb/include/lldb

[Lldb-commits] [PATCH] D122293: [wip][intelpt] Refactoring instruction decoding for flexibility

wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:172 std::vector m_instructions; + std::vector m_errors; + you need to have something like std::unordered_map m_errors; that way, you'll be able to quickly look fo

[Lldb-commits] [PATCH] D122293: [wip][intelpt] Refactoring instruction decoding for flexibility

wallace requested changes to this revision. wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:103-105 +std::unordered_map DecodedThread::GetErrors() const { + return m_errors; +} remove this Commen

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

wallace updated this revision to Diff 417827. wallace added a comment. - improve documentation - use lldb::user_id_t - add the new TraceInstructionDumperOptions struct Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122254/new/ https://reviews.llvm.o

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

wallace updated this revision to Diff 417828. wallace added a comment. nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122254/new/ https://reviews.llvm.org/D122254 Files: lldb/include/lldb/Target/TraceCursor.h lldb/include/lldb/Target/Trace

[Lldb-commits] [PATCH] D122293: [wip][intelpt] Refactoring instruction decoding for flexibility

wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:149 +thread.AppendError(insn_index, make_error(time_error, insn.ip)); +thread.AppendInstruction(IntelPTInstruction(insn)); break; zrthxn wro

[Lldb-commits] [PATCH] D122293: [wip][intelpt] Refactoring instruction decoding for flexibility

wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:103-105 +// llvm::Error DecodedThread::GetError(uint64_t idx) const { +// return m_errors.at(idx); +// } Errors can only be copied, that's why we need to create a ne

[Lldb-commits] [PATCH] D122293: [wip][intelpt] Refactoring instruction decoding for flexibility

wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:227 + +static Expected DecodeLiveThread(const ThreadSP &thread_sp, + TraceIntelPT &trace) { don't use expected. The D

[Lldb-commits] [PATCH] D122293: [wip][intelpt] Refactoring instruction decoding for flexibility

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. there are many comments from the previous versions of this diff that you didn't apply. Go through all of them first :) Comment at: lldb/source/Plugins/Trace/inte

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

wallace updated this revision to Diff 418032. wallace added a comment. fix some comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122254/new/ https://reviews.llvm.org/D122254 Files: lldb/include/lldb/Target/TraceCursor.h lldb/include/lldb

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

wallace added inline comments. Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:50 + /// Additional options for configuring the dumping. + TraceInstructionDumper(lldb::TraceCursorUP &&cursor_up, Stream &s, + const TraceInstructionDumperO

[Lldb-commits] [PATCH] D122293: [intelpt] Refactoring instruction decoding for flexibility

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. much closer! I'm glad you are starting to understand the patterns we use for this kind of code Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:1

[Lldb-commits] [PATCH] D122293: [intelpt] Refactoring instruction decoding for flexibility

wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:25 IntelPTError::IntelPTError(int libipt_error_code, lldb::addr_t address) : m_libipt_error_code(libipt_error_code), m_address(address) { assert(libipt_error_code < 0); --

[Lldb-commits] [PATCH] D122293: [intelpt] Refactoring instruction decoding for flexibility

wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:77 bool TraceCursorIntelPT::IsError() { return m_decoded_thread_sp->GetInstructions()[m_pos].IsError(); } jj10306 wrote: > zrthxn wrote: > > jj10306 wrote: >

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

wallace updated this revision to Diff 418361. wallace marked 2 inline comments as done. wallace added a comment. Some updates: - Modified `thread trace dump instructions` to accept one single thread instead of many. The reason is that, with the new --id argument, traversing multiple threads doe

[Lldb-commits] [PATCH] D122293: [intelpt] Refactoring instruction decoding for flexibility

wallace updated this revision to Diff 418404. wallace added a comment. - make tests pass - simplified the error handling. In fact, using Error objects might be too expensive and potentially provides little value in the API, because the user needs to consume the Error forcefully. Besides that, o

[Lldb-commits] [PATCH] D122603: [wip][intelpt] Refactor timestamps out of `IntelPTInstruction`

wallace added a comment. let's better use the word TSC instead of timestamps, which is more accurate Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:124 + /// Append a successfully decoded instruction. + void AppendInstruction(pt_insn instruction); + --

[Lldb-commits] [PATCH] D122603: [wip][intelpt] Refactor timestamps out of `IntelPTInstruction`

wallace requested changes to this revision. wallace added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:94-96 + auto last_ts = m_instruction_timestamps[m_instruction_timestamps.size() - 1]; + if

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

wallace updated this revision to Diff 418680. wallace added a comment. - Bring back the --continue command. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122254/new/ https://reviews.llvm.org/D122254 Files: lldb/include/lldb/Target/TraceCursor.h

[Lldb-commits] [PATCH] D122603: [wip][intelpt] Refactor timestamps out of `IntelPTInstruction`

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. I'm proposing a new interface for the TscRange. Let me know if you have questions Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:95 + m_instru

[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. Some calculations are wrong, but overall this is good. We are very close! Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:112-113 +DecodedThread:

[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. almost there! Mostly cosmetic changes needed Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:94-98 + m_instructions.emplace_back(insn); + if (!

[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

wallace added a comment. one last nit and good to go Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:82 + + auto dist = FindDistanceAndSetPos(); + m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos); don't use auto for simple typ

[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:145 + private: +friend class DecodedThread; + jj10306 wrote: > nit: No need to friend the enclosing class since C++11 - > https://en.cppreference.com/w/cpp/languag

[Lldb-commits] [PATCH] D122859: [trace] Show ideas for the main interfaces for new HTR

wallace created this revision. wallace added reviewers: davidca, clayborg, jj10306. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This diff tries to show the low level structures and the high level API

[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122603/new/ https://reviews.llvm.org/D122603 _

[Lldb-commits] [PATCH] D122867: [trace][intel pt] Handle better tsc in the decoder

wallace created this revision. wallace added reviewers: jj10306, zrthxn. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. A problem that I introduced in the decoder is that I was considering TSC decoding

[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

wallace added a comment. Don't forget to update the description of this diff and of the commit before pushing (you need to do both). Include the avg instruction size for a trace of at least 10k instructions as well :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[Lldb-commits] [PATCH] D122867: [trace][intel pt] Handle better tsc in the decoder

wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:120 + +void DecodedThread::ReportTscError(int libipt_error_code) { m_tsc_errors++; } + jj10306 wrote: > The parameter is unused, is there a reason to keep this or shou

[Lldb-commits] [PATCH] D122867: [trace][intel pt] Handle better tsc in the decoder

wallace updated this revision to Diff 419813. wallace added a comment. - Address comments - Also now using Format instead of Printf, which more idiomatic in this repo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122867/new/ https://reviews.llvm.o

[Lldb-commits] [PATCH] D122991: [lldb][intelpt] Remove `IntelPTInstruction` and move methods to `DecodedThread`

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. only mostly cosmetic changes needed. Thanks for this. I'm glad that we are bringing the usage down Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.c

[Lldb-commits] [PATCH] D123025: [lldb-vscode] Implement stderr/stdout on win32 and redirect lldb log to VSCode

wallace added inline comments. Comment at: lldb/tools/lldb-vscode/OutputRedirector.cpp:12 +#else +#include +#include mstorsjo wrote: > Minor style issue - I guess it'd be less of double negation, if we'd change > the ifdef to `#if defined(_WIN32) .. #else` fee

[Lldb-commits] [PATCH] D123106: [trace][intel pt] Create a class for the libipt decoder wrapper

wallace created this revision. wallace added reviewers: jj10306, zrthxn. Herald added a subscriber: mgorny. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. As we soon will need to decode multiple raw tra

[Lldb-commits] [PATCH] D123106: [trace][intel pt] Create a class for the libipt decoder wrapper

wallace updated this revision to Diff 420395. wallace added a comment. nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123106/new/ https://reviews.llvm.org/D123106 Files: lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt lldb/source/Plugins

[Lldb-commits] [PATCH] D122991: [lldb][intelpt] Remove `IntelPTInstruction` and move methods to `DecodedThread`

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. just remove a small comment and good to go! Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:143-146 + /// \param[in] next_load_address + ///

[Lldb-commits] [PATCH] D122991: [lldb][intelpt] Remove `IntelPTInstruction` and move methods to `DecodedThread`

wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. great job! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122991/new/ https://reviews.llvm.org/D122991 ___

[Lldb-commits] [PATCH] D123025: [lldb-vscode] Implement stderr/stdout on win32 and redirect lldb log to VSCode

wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123025/new/ https://reviews.llvm.org/D123025 ___

[Lldb-commits] [PATCH] D123025: [lldb-vscode] Implement stderr/stdout on win32 and redirect lldb log to VSCode

wallace added a comment. Let me know if you need help upstreaming this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123025/new/ https://reviews.llvm.org/D123025 ___ lldb-commits mailing list lldb

[Lldb-commits] [PATCH] D123025: [lldb-vscode] Implement stderr/stdout on win32 and redirect lldb log to VSCode

wallace added a comment. ok! I'll commit it for you Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123025/new/ https://reviews.llvm.org/D123025 ___ lldb-commits mailing list lldb-commits@lists.llvm.org ht

[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

wallace created this revision. wallace added reviewers: jj10306, zrthxn. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Some parts of the code have to distinguish between live and postmortem threads to

[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

wallace updated this revision to Diff 421093. wallace added a comment. nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123281/new/ https://reviews.llvm.org/D123281 Files: lldb/include/lldb/Target/Trace.h lldb/source/Plugins/Trace/common/Tra

[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

wallace added inline comments. Comment at: lldb/include/lldb/Target/Trace.h:265 + lldb::tid_t tid, llvm::StringRef kind, + std::function data)> callback); + jj10306 wrote: > typedef the callback to be cleaner and make the intention more clear? good idea

[Lldb-commits] [PATCH] D123106: [trace][intel pt] Create a class for the libipt decoder wrapper

wallace added a comment. thanks for the gotchas Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:41 + +/// Class that decodes a raw buffer for a single thread using the low level +/// libipt library. jj10306 wrote: > "for a single thread" > thin

[Lldb-commits] [PATCH] D123106: [trace][intel pt] Create a class for the libipt decoder wrapper

wallace updated this revision to Diff 421283. wallace marked 6 inline comments as done. wallace added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123106/new/ https://reviews.llvm.org/D123106 Files: lldb/source/Plugi

[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

wallace updated this revision to Diff 421291. wallace added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123281/new/ https://reviews.llvm.org/D123281 Files: lldb/include/lldb/Target/Trace.h lldb/source/Plugins/Trac

[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

wallace added inline comments. Comment at: lldb/include/lldb/Target/Trace.h:386-396 + llvm::DenseMap> m_live_thread_data; + /// data kind -> size std::unordered_map m_live_process_data; + /// \} + jj10306 wrote: > Why not change all the maps to Den

[Lldb-commits] [PATCH] D123356: add task timer

wallace created this revision. Herald added a subscriber: mgorny. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D123356 Files: lldb/so

[Lldb-commits] [PATCH] D123357: [trace][intelpt] Add task timer classes

wallace created this revision. Herald added a subscriber: mgorny. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. I'm adding two new classes that can be used to measure the duration of long tasks as proc

[Lldb-commits] [PATCH] D123358: [trace][intelpt] Remove code smell when printing the raw trace size

wallace created this revision. wallace added reviewers: jj10306, zrthxn. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Something ugly I did was to report the trace buffer size to the DecodedThread, whi

[Lldb-commits] [PATCH] D123375: [lldb][intelpt] Reduce trace memory usage by grouping instructions

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. I did a first pass on this diff. I'm asking to refactor a bit the InstructionBlock classes so that they are smarter. Besides that, if you use IDs more ubiquitously and stop using i

[Lldb-commits] [PATCH] D123358: [trace][intelpt] Remove code smell when printing the raw trace size

wallace added a comment. Well, not all theoretical trace plugins might have raw trace sizes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123358/new/ https://reviews.llvm.org/D123358 ___ lldb-commits ma

<    1   2   3   4   5   6   7   8   9   10   >