[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-16 Thread Phabricator via lldb-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb0aa70761b83: [trace][intel pt] Implement the Intel PT cursor (authored by Walter Erquinigo ). Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-16 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 359479. wallace added a comment. fix nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105531/new/ https://reviews.llvm.org/D105531 Files: lldb/include/lldb/Target/Trace.h lldb/include/lldb/Target/TraceCu

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-16 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Just switch all "-f" to the long option format as mentioned in inlined comments and this is good to go. Comment at: lldb/test/API/commands/trace/TestTraceDumpInstruction

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-16 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 359471. wallace added a comment. - Nwo using a single-step iterator method Next - Changed the direction to a boolean flag called "forwards" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105531/new/ https://revi

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Very close! Just a few nits. Comment at: lldb/include/lldb/Target/TraceCursor.h:101 + /// Set the direction to use in the \a TraceCursor::Move() method. + voi

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-15 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 359200. wallace added a comment. Followed the recommendations. Now I'm making sure that TraceInstructionDumper uses the state of the Cursor and doesn't modify it, this makes the class reusable for other purposes. Having the direction and granularity as part

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-08 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Check my current comments before I proceed to look at the rest. Now that we are using the API for something, it is showing some issues with the usability of the API in TraceCursor. Let me know what you think Comment at: lldb/include/lldb/Target/Trace

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-08 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 357371. wallace added a comment. Address comments. - I ended up creating new method Next(int count) and Prev(int count) in the cursor for the skip operation. It might be useful for other things. I didn't add the count parameter to the existing Prev and Next

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-08 Thread walter erquinigo via Phabricator via lldb-commits
wallace added inline comments. Comment at: lldb/source/Target/TraceInstructionDumper.cpp:291 +s.Printf("\n"); +TryMoveOneInstruction(); + } clayborg wrote: > You should be watching the return value of this right? What if this returns > false? that's tak

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-08 Thread walter erquinigo via Phabricator via lldb-commits
wallace marked 9 inline comments as done. wallace added inline comments. Comment at: lldb/source/Commands/CommandObjectThread.cpp:2111 bool m_create_repeat_command_just_invoked; - size_t m_consecutive_repetitions = 0; + std::map> m_dumpers; }; clayborg wrot

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-08 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Commands/CommandObjectThread.cpp:2111 bool m_create_repeat_command_just_invoked; - size_t m_consecutive_repetitions = 0; + std:

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-08 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 357331. wallace added a comment. Addressed all issues - Created a TraceInstructionDumper class that keeps the index to print, which leaves the TraceCursor unaffected. It also keeps some other useful state. Repository: rG LLVM Github Monorepo CHANGES SIN

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-08 Thread walter erquinigo via Phabricator via lldb-commits
wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:121 /// stopped at. See \a Trace::GetCursorPosition for more information. -class DecodedThread { +class DecodedThread : public std::enable_shared_from_this { public: c

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-07 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: lldb/include/lldb/Target/TraceCursor.h:64-65 + TraceCursor(lldb::ThreadSP thread_sp) : m_thread_sp(thread_sp) {} + /// Move the cursor to

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-06 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 356875. wallace added a comment. improve documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105531/new/ https://reviews.llvm.org/D105531 Files: lldb/include/lldb/Target/Trace.h lldb/include/lldb/T

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-06 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 356873. wallace added a comment. Herald added a subscriber: JDevlieghere. nit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105531/new/ https://reviews.llvm.org/D105531 Files: lldb/include/lldb/Target/Trace.

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

2021-07-06 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision. wallace added a reviewer: clayborg. Herald added subscribers: dang, mgorny. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. D104422 added the interface for TraceCurso