wallace marked 4 inline comments as done.
wallace added inline comments.

================
Comment at: lldb/docs/lldb-gdb-remote.txt:248
 //     "name": <string>,
-//         Tracing technology name, e.g. intel-pt, arm-coresight.
+//         Tracing technology name, e.g. intel-pt, arm-etm.
 //     "description": <string>,
----------------
this should be "Embedded Trace Macrocells" (ETM) instead of coresight


================
Comment at: lldb/docs/lldb-gdb-remote.txt:465-474
 //      "tid": <decimal integer>,
 //      "binaryData": [
 //        {
 //          "kind": <string>,
 //              Identifier for some binary data related to this thread to
 //              fetch with the jLLDBTraceGetBinaryData packet.
 //          "size": <decimal integer>,
----------------
wallace wrote:
> jj10306 wrote:
> > Should the thread info be optional now that we have an optional `cores`? If 
> > not, can you explain how this output works in the case that you are doing 
> > per core tracing? Will both the tid binary data and the cores binary data 
> > section be populated?
> I'll make this field optional and include more documentation below
in the end i didn't make this optional but instead forced the per-core case to 
return all threads 


================
Comment at: lldb/include/lldb/Utility/TraceGDBRemotePackets.h:157-160
+  std::vector<TraceThreadState> traced_threads;
+  std::vector<TraceBinaryData> process_binary_data;
+  llvm::Optional<std::vector<TraceCoreState>> cores;
 };
----------------
jj10306 wrote:
> Similar question here as above on the GetState documentation - how do the 
> cores and traced_threads options play together?
traced_threads will contain all the threads in the per core case but without 
any binary data. The cores field will then be returned with per-core trace 
buffers


================
Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp:149
 
 #ifdef PERF_ATTR_SIZE_VER5
 static Expected<uint64_t>
----------------
i'm the location of this ifdef so that perf_event_attr is used only if 
PERF_ATTR_SIZE_VER5 is set


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124858/new/

https://reviews.llvm.org/D124858

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to