This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG05b4bf257124: [trace][intelpt] Introduce instruction Ids 
(authored by Walter Erquinigo <wall...@fb.com>).

Changed prior to commit:
  https://reviews.llvm.org/D122254?vs=418680&id=420969#toc

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/TraceInstructionDumper.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  lldb/source/Target/TraceInstructionDumper.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceStartStop.py
  lldb/test/API/commands/trace/TestTraceTSC.py
  lldb/test/API/commands/trace/TestTraceTimestampCounters.py

Index: lldb/test/API/commands/trace/TestTraceTSC.py
===================================================================
--- lldb/test/API/commands/trace/TestTraceTSC.py
+++ lldb/test/API/commands/trace/TestTraceTSC.py
@@ -19,7 +19,35 @@
 
         self.expect("n")
         self.expect("thread trace dump instructions --tsc -c 1",
-            patterns=["\[0\] \[tsc=0x[0-9a-fA-F]+\] 0x0000000000400511    movl"])
+            patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x0000000000400511    movl"])
+
+    @testSBAPIAndCommands
+    @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
+    def testMultipleTscsPerThread(self):
+        self.expect("file " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+        self.expect("b main")
+        self.expect("r")
+
+        self.traceStartThread(enableTsc=True)
+
+        # After each stop there'll be a new TSC
+        self.expect("n")
+        self.expect("n")
+        self.expect("n")
+
+        # We'll get the most recent instructions, with at least 3 different TSCs
+        self.runCmd("thread trace dump instructions --tsc --raw")
+        id_to_tsc = {}
+        for line in self.res.GetOutput().splitlines():
+            m = re.search("    (.+): \[tsc=(.+)\].*", line)
+            if m:
+                id_to_tsc[int(m.group(1))] = m.group(2)
+        self.assertEqual(len(id_to_tsc), 6)
+
+        # We check that the values are right when dumping a specific id
+        for id in range(0, 6):
+            self.expect(f"thread trace dump instructions --tsc --id {id} -c 1",
+                substrs=[f"{id}: [tsc={id_to_tsc[id]}]"])
 
     @testSBAPIAndCommands
     @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -32,7 +60,7 @@
 
         self.expect("n")
         self.expect("thread trace dump instructions --tsc -c 1",
-            patterns=["\[0\] \[tsc=0x[0-9a-fA-F]+\] 0x0000000000400511    movl"])
+            patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x0000000000400511    movl"])
 
     @testSBAPIAndCommands
     @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -45,7 +73,7 @@
 
         self.expect("n")
         self.expect("thread trace dump instructions --tsc -c 1",
-            patterns=["\[0\] \[tsc=unavailable\] 0x0000000000400511    movl"])
+            patterns=["0: \[tsc=unavailable\] 0x0000000000400511    movl"])
 
     @testSBAPIAndCommands
     @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
Index: lldb/test/API/commands/trace/TestTraceStartStop.py
===================================================================
--- lldb/test/API/commands/trace/TestTraceStartStop.py
+++ lldb/test/API/commands/trace/TestTraceStartStop.py
@@ -114,29 +114,29 @@
         self.expect("thread trace dump instructions -f",
             patterns=[f'''thread #1: tid = .*
   a.out`main \+ 4 at main.cpp:2
-    \[ 0\] {ADDRESS_REGEX}    movl'''])
+    0: {ADDRESS_REGEX}    movl'''])
 
         # We can reconstruct the instructions up to the second line
         self.expect("n")
         self.expect("thread trace dump instructions -f",
             patterns=[f'''thread #1: tid = .*
   a.out`main \+ 4 at main.cpp:2
-    \[ 0\] {ADDRESS_REGEX}    movl .*
+    0: {ADDRESS_REGEX}    movl .*
   a.out`main \+ 11 at main.cpp:4
-    \[ 1\] {ADDRESS_REGEX}    movl .*
-    \[ 2\] {ADDRESS_REGEX}    jmp  .* ; <\+28> at main.cpp:4
-    \[ 3\] {ADDRESS_REGEX}    cmpl .*
-    \[ 4\] {ADDRESS_REGEX}    jle  .* ; <\+20> at main.cpp:5'''])
+    1: {ADDRESS_REGEX}    movl .*
+    2: {ADDRESS_REGEX}    jmp  .* ; <\+28> at main.cpp:4
+    3: {ADDRESS_REGEX}    cmpl .*
+    4: {ADDRESS_REGEX}    jle  .* ; <\+20> at main.cpp:5'''])
 
         self.expect("thread trace dump instructions",
             patterns=[f'''thread #1: tid = .*
   a.out`main \+ 32 at main.cpp:4
-    \[  0\] {ADDRESS_REGEX}    jle  .* ; <\+20> at main.cpp:5
-    \[ -1\] {ADDRESS_REGEX}    cmpl .*
-    \[ -2\] {ADDRESS_REGEX}    jmp  .* ; <\+28> at main.cpp:4
-    \[ -3\] {ADDRESS_REGEX}    movl .*
+    4: {ADDRESS_REGEX}    jle  .* ; <\+20> at main.cpp:5
+    3: {ADDRESS_REGEX}    cmpl .*
+    2: {ADDRESS_REGEX}    jmp  .* ; <\+28> at main.cpp:4
+    1: {ADDRESS_REGEX}    movl .*
   a.out`main \+ 4 at main.cpp:2
-    \[ -4\] {ADDRESS_REGEX}    movl .* '''])
+    0: {ADDRESS_REGEX}    movl .* '''])
 
         # We stop tracing
         self.expect("thread trace stop")
@@ -152,12 +152,12 @@
         self.expect("thread trace dump instructions -f",
             patterns=[f'''thread #1: tid = .*
   a.out`main \+ 20 at main.cpp:5
-    \[ 0\] {ADDRESS_REGEX}    xorl'''])
+    0: {ADDRESS_REGEX}    xorl'''])
 
         self.expect("thread trace dump instructions",
             patterns=[f'''thread #1: tid = .*
   a.out`main \+ 20 at main.cpp:5
-    \[  0\] {ADDRESS_REGEX}    xorl'''])
+    0: {ADDRESS_REGEX}    xorl'''])
 
         self.expect("c")
         # Now the process has finished, so the commands should fail
Index: lldb/test/API/commands/trace/TestTraceDumpInstructions.py
===================================================================
--- lldb/test/API/commands/trace/TestTraceDumpInstructions.py
+++ lldb/test/API/commands/trace/TestTraceDumpInstructions.py
@@ -37,49 +37,67 @@
 
         self.expect("thread trace dump instructions --raw --count 21 --forwards",
             substrs=['''thread #1: tid = 3842849
-    [ 0] 0x0000000000400511
-    [ 1] 0x0000000000400518
-    [ 2] 0x000000000040051f
-    [ 3] 0x0000000000400529
-    [ 4] 0x000000000040052d
-    [ 5] 0x0000000000400521
-    [ 6] 0x0000000000400525
-    [ 7] 0x0000000000400529
-    [ 8] 0x000000000040052d
-    [ 9] 0x0000000000400521
-    [10] 0x0000000000400525
-    [11] 0x0000000000400529
-    [12] 0x000000000040052d
-    [13] 0x0000000000400521
-    [14] 0x0000000000400525
-    [15] 0x0000000000400529
-    [16] 0x000000000040052d
-    [17] 0x0000000000400521
-    [18] 0x0000000000400525
-    [19] 0x0000000000400529
-    [20] 0x000000000040052d'''])
+    0: 0x0000000000400511
+    1: 0x0000000000400518
+    2: 0x000000000040051f
+    3: 0x0000000000400529
+    4: 0x000000000040052d
+    5: 0x0000000000400521
+    6: 0x0000000000400525
+    7: 0x0000000000400529
+    8: 0x000000000040052d
+    9: 0x0000000000400521
+    10: 0x0000000000400525
+    11: 0x0000000000400529
+    12: 0x000000000040052d
+    13: 0x0000000000400521
+    14: 0x0000000000400525
+    15: 0x0000000000400529
+    16: 0x000000000040052d
+    17: 0x0000000000400521
+    18: 0x0000000000400525
+    19: 0x0000000000400529
+    20: 0x000000000040052d'''])
 
         # We check if we can pass count and skip
         self.expect("thread trace dump instructions --count 5 --skip 6 --raw --forwards",
             substrs=['''thread #1: tid = 3842849
-    [ 6] 0x0000000000400525
-    [ 7] 0x0000000000400529
-    [ 8] 0x000000000040052d
-    [ 9] 0x0000000000400521
-    [10] 0x0000000000400525'''])
+    6: 0x0000000000400525
+    7: 0x0000000000400529
+    8: 0x000000000040052d
+    9: 0x0000000000400521
+    10: 0x0000000000400525'''])
 
         self.expect("thread trace dump instructions --count 5 --skip 6 --raw",
             substrs=['''thread #1: tid = 3842849
-    [ -6] 0x0000000000400525
-    [ -7] 0x0000000000400521
-    [ -8] 0x000000000040052d
-    [ -9] 0x0000000000400529
-    [-10] 0x0000000000400525'''])
+    14: 0x0000000000400525
+    13: 0x0000000000400521
+    12: 0x000000000040052d
+    11: 0x0000000000400529
+    10: 0x0000000000400525'''])
+
+        # We check if we can pass count and skip and instruction id in hex
+        self.expect("thread trace dump instructions --count 5 --skip 6 --raw --id 0xA",
+            substrs=['''thread #1: tid = 3842849
+    4: 0x000000000040052d
+    3: 0x0000000000400529
+    2: 0x000000000040051f
+    1: 0x0000000000400518
+    0: 0x0000000000400511'''])
+
+        # We check if we can pass count and skip and instruction id in decimal
+        self.expect("thread trace dump instructions --count 5 --skip 6 --raw --id 10",
+            substrs=['''thread #1: tid = 3842849
+    4: 0x000000000040052d
+    3: 0x0000000000400529
+    2: 0x000000000040051f
+    1: 0x0000000000400518
+    0: 0x0000000000400511'''])
 
         # We check if we can access the thread by index id
         self.expect("thread trace dump instructions 1 --raw",
             substrs=['''thread #1: tid = 3842849
-    [  0] 0x000000000040052d'''])
+    20: 0x000000000040052d'''])
 
         # We check that we get an error when using an invalid thread index id
         self.expect("thread trace dump instructions 10", error=True,
@@ -90,53 +108,35 @@
         self.expect("trace load -v " +
             os.path.join(self.getSourceDir(), "intelpt-trace", "trace_2threads.json"))
 
-        # We print the instructions of two threads simultaneously
-        self.expect("thread trace dump instructions 1 2 --count 2",
-            substrs=['''thread #1: tid = 3842849
-  a.out`main + 32 at main.cpp:4
-    [ 0] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
-    [-1] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
-thread #2: tid = 3842850
+        # We print the instructions of a specific thread
+        self.expect("thread trace dump instructions 2 --count 2",
+            substrs=['''thread #2: tid = 3842850
   a.out`main + 32 at main.cpp:4
-    [ 0] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
-    [-1] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)'''])
+    20: 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
+    19: 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)'''])
 
         # We use custom --count and --skip, saving the command to history for later
-        self.expect("thread trace dump instructions 1 2 --count 2 --skip 2", inHistory=True,
-            substrs=['''thread #1: tid = 3842849
+        self.expect("thread trace dump instructions 2 --count 2 --skip 2", inHistory=True,
+            substrs=['''thread #2: tid = 3842850
   a.out`main + 24 at main.cpp:4
-    [-2] 0x0000000000400525    addl   $0x1, -0x8(%rbp)
+    18: 0x0000000000400525    addl   $0x1, -0x8(%rbp)
   a.out`main + 20 at main.cpp:5
-    [-3] 0x0000000000400521    xorl   $0x1, -0x4(%rbp)
-thread #2: tid = 3842850
-  a.out`main + 24 at main.cpp:4
-    [-2] 0x0000000000400525    addl   $0x1, -0x8(%rbp)
-  a.out`main + 20 at main.cpp:5
-    [-3] 0x0000000000400521    xorl   $0x1, -0x4(%rbp)'''])
+    17: 0x0000000000400521    xorl   $0x1, -0x4(%rbp)'''])
 
         # We use a repeat command twice and ensure the previous count is used and the
         # start position moves with each command.
         self.expect("", inHistory=True,
-            substrs=['''thread #1: tid = 3842849
-  a.out`main + 32 at main.cpp:4
-    [-4] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
-    [-5] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
-thread #2: tid = 3842850
+            substrs=['''thread #2: tid = 3842850
   a.out`main + 32 at main.cpp:4
-    [-4] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
-    [-5] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)'''])
+    16: 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
+    15: 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)'''])
 
         self.expect("", inHistory=True,
-            substrs=['''thread #1: tid = 3842849
-  a.out`main + 24 at main.cpp:4
-    [-6] 0x0000000000400525    addl   $0x1, -0x8(%rbp)
-  a.out`main + 20 at main.cpp:5
-    [-7] 0x0000000000400521    xorl   $0x1, -0x4(%rbp)
-thread #2: tid = 3842850
+            substrs=['''thread #2: tid = 3842850
   a.out`main + 24 at main.cpp:4
-    [-6] 0x0000000000400525    addl   $0x1, -0x8(%rbp)
+    14: 0x0000000000400525    addl   $0x1, -0x8(%rbp)
   a.out`main + 20 at main.cpp:5
-    [-7] 0x0000000000400521    xorl   $0x1, -0x4(%rbp)'''])
+    13: 0x0000000000400521    xorl   $0x1, -0x4(%rbp)'''])
 
     def testInvalidBounds(self):
         self.expect("trace load -v " +
@@ -146,10 +146,10 @@
         self.expect("thread trace dump instructions --count 20 --forwards",
             substrs=['''thread #1: tid = 3842849
   a.out`main + 4 at main.cpp:2
-    [ 0] 0x0000000000400511    movl   $0x0, -0x4(%rbp)
+    0: 0x0000000000400511    movl   $0x0, -0x4(%rbp)
   a.out`main + 11 at main.cpp:4
-    [ 1] 0x0000000000400518    movl   $0x0, -0x8(%rbp)
-    [ 2] 0x000000000040051f    jmp    0x400529                  ; <+28> at main.cpp:4'''])
+    1: 0x0000000000400518    movl   $0x0, -0x8(%rbp)
+    2: 0x000000000040051f    jmp    0x400529                  ; <+28> at main.cpp:4'''])
 
         # Should print no instructions if the position is out of bounds
         self.expect("thread trace dump instructions --skip 23",
@@ -164,15 +164,15 @@
             os.path.join(self.getSourceDir(), "intelpt-trace", "trace_bad_image.json"))
         self.expect("thread trace dump instructions --forwards",
             substrs=['''thread #1: tid = 3842849
-    [ 0] 0x0000000000400511    error: no memory mapped at this address
-    [ 1] 0x0000000000400518    error: no memory mapped at this address'''])
+    0: 0x0000000000400511    error: no memory mapped at this address
+    1: 0x0000000000400518    error: no memory mapped at this address'''])
 
     def testWrongCPU(self):
         self.expect("trace load " +
             os.path.join(self.getSourceDir(), "intelpt-trace", "trace_wrong_cpu.json"))
         self.expect("thread trace dump instructions --forwards",
             substrs=['''thread #1: tid = 3842849
-    [ 0] error: unknown cpu'''])
+    0: error: unknown cpu'''])
 
     def testMultiFileTraceWithMissingModule(self):
         self.expect("trace load " +
@@ -195,147 +195,147 @@
         self.expect("thread trace dump instructions --count 50 --forwards",
             substrs=['''thread #1: tid = 815455
   a.out`main + 15 at main.cpp:10
-    [ 0] 0x000000000040066f    callq  0x400540                  ; symbol stub for: foo()
+    0: 0x000000000040066f    callq  0x400540                  ; symbol stub for: foo()
   a.out`symbol stub for: foo()
-    [ 1] 0x0000000000400540    jmpq   *0x200ae2(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 40
-    [ 2] 0x0000000000400546    pushq  $0x2
-    [ 3] 0x000000000040054b    jmp    0x400510
+    1: 0x0000000000400540    jmpq   *0x200ae2(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 40
+    2: 0x0000000000400546    pushq  $0x2
+    3: 0x000000000040054b    jmp    0x400510
   a.out`(none)
-    [ 4] 0x0000000000400510    pushq  0x200af2(%rip)            ; _GLOBAL_OFFSET_TABLE_ + 8
-    [ 5] 0x0000000000400516    jmpq   *0x200af4(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 16
-    [ 6] 0x00007ffff7df1950    error: no memory mapped at this address
+    4: 0x0000000000400510    pushq  0x200af2(%rip)            ; _GLOBAL_OFFSET_TABLE_ + 8
+    5: 0x0000000000400516    jmpq   *0x200af4(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 16
+    6: 0x00007ffff7df1950    error: no memory mapped at this address
     ...missing instructions
   a.out`main + 20 at main.cpp:10
-    [ 7] 0x0000000000400674    movl   %eax, -0xc(%rbp)
+    7: 0x0000000000400674    movl   %eax, -0xc(%rbp)
   a.out`main + 23 at main.cpp:12
-    [ 8] 0x0000000000400677    movl   -0xc(%rbp), %eax
-    [ 9] 0x000000000040067a    addl   $0x1, %eax
-    [10] 0x000000000040067f    movl   %eax, -0xc(%rbp)
+    8: 0x0000000000400677    movl   -0xc(%rbp), %eax
+    9: 0x000000000040067a    addl   $0x1, %eax
+    10: 0x000000000040067f    movl   %eax, -0xc(%rbp)
   a.out`main + 34 [inlined] inline_function() at main.cpp:4
-    [11] 0x0000000000400682    movl   $0x0, -0x4(%rbp)
+    11: 0x0000000000400682    movl   $0x0, -0x4(%rbp)
   a.out`main + 41 [inlined] inline_function() + 7 at main.cpp:5
-    [12] 0x0000000000400689    movl   -0x4(%rbp), %eax
-    [13] 0x000000000040068c    addl   $0x1, %eax
-    [14] 0x0000000000400691    movl   %eax, -0x4(%rbp)
+    12: 0x0000000000400689    movl   -0x4(%rbp), %eax
+    13: 0x000000000040068c    addl   $0x1, %eax
+    14: 0x0000000000400691    movl   %eax, -0x4(%rbp)
   a.out`main + 52 [inlined] inline_function() + 18 at main.cpp:6
-    [15] 0x0000000000400694    movl   -0x4(%rbp), %eax
+    15: 0x0000000000400694    movl   -0x4(%rbp), %eax
   a.out`main + 55 at main.cpp:14
-    [16] 0x0000000000400697    movl   -0xc(%rbp), %ecx
-    [17] 0x000000000040069a    addl   %eax, %ecx
-    [18] 0x000000000040069c    movl   %ecx, -0xc(%rbp)
+    16: 0x0000000000400697    movl   -0xc(%rbp), %ecx
+    17: 0x000000000040069a    addl   %eax, %ecx
+    18: 0x000000000040069c    movl   %ecx, -0xc(%rbp)
   a.out`main + 63 at main.cpp:16
-    [19] 0x000000000040069f    callq  0x400540                  ; symbol stub for: foo()
+    19: 0x000000000040069f    callq  0x400540                  ; symbol stub for: foo()
   a.out`symbol stub for: foo()
-    [20] 0x0000000000400540    jmpq   *0x200ae2(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 40
+    20: 0x0000000000400540    jmpq   *0x200ae2(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 40
   libfoo.so`foo() at foo.cpp:3
-    [21] 0x00007ffff7bd96e0    pushq  %rbp
-    [22] 0x00007ffff7bd96e1    movq   %rsp, %rbp
+    21: 0x00007ffff7bd96e0    pushq  %rbp
+    22: 0x00007ffff7bd96e1    movq   %rsp, %rbp
   libfoo.so`foo() + 4 at foo.cpp:4
-    [23] 0x00007ffff7bd96e4    subq   $0x10, %rsp
-    [24] 0x00007ffff7bd96e8    callq  0x7ffff7bd95d0            ; symbol stub for: bar()
+    23: 0x00007ffff7bd96e4    subq   $0x10, %rsp
+    24: 0x00007ffff7bd96e8    callq  0x7ffff7bd95d0            ; symbol stub for: bar()
   libfoo.so`symbol stub for: bar()
-    [25] 0x00007ffff7bd95d0    jmpq   *0x200a4a(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 32
+    25: 0x00007ffff7bd95d0    jmpq   *0x200a4a(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 32
   libbar.so`bar() at bar.cpp:1
-    [26] 0x00007ffff79d7690    pushq  %rbp
-    [27] 0x00007ffff79d7691    movq   %rsp, %rbp
+    26: 0x00007ffff79d7690    pushq  %rbp
+    27: 0x00007ffff79d7691    movq   %rsp, %rbp
   libbar.so`bar() + 4 at bar.cpp:2
-    [28] 0x00007ffff79d7694    movl   $0x1, -0x4(%rbp)
+    28: 0x00007ffff79d7694    movl   $0x1, -0x4(%rbp)
   libbar.so`bar() + 11 at bar.cpp:3
-    [29] 0x00007ffff79d769b    movl   -0x4(%rbp), %eax
-    [30] 0x00007ffff79d769e    addl   $0x1, %eax
-    [31] 0x00007ffff79d76a3    movl   %eax, -0x4(%rbp)
+    29: 0x00007ffff79d769b    movl   -0x4(%rbp), %eax
+    30: 0x00007ffff79d769e    addl   $0x1, %eax
+    31: 0x00007ffff79d76a3    movl   %eax, -0x4(%rbp)
   libbar.so`bar() + 22 at bar.cpp:4
-    [32] 0x00007ffff79d76a6    movl   -0x4(%rbp), %eax
-    [33] 0x00007ffff79d76a9    popq   %rbp
-    [34] 0x00007ffff79d76aa    retq''',
+    32: 0x00007ffff79d76a6    movl   -0x4(%rbp), %eax
+    33: 0x00007ffff79d76a9    popq   %rbp
+    34: 0x00007ffff79d76aa    retq''',
   '''libfoo.so`foo() + 13 at foo.cpp:4
-    [35] 0x00007ffff7bd96ed    movl   %eax, -0x4(%rbp)
+    35: 0x00007ffff7bd96ed    movl   %eax, -0x4(%rbp)
   libfoo.so`foo() + 16 at foo.cpp:5
-    [36] 0x00007ffff7bd96f0    movl   -0x4(%rbp), %eax
-    [37] 0x00007ffff7bd96f3    addl   $0x1, %eax
-    [38] 0x00007ffff7bd96f8    movl   %eax, -0x4(%rbp)
+    36: 0x00007ffff7bd96f0    movl   -0x4(%rbp), %eax
+    37: 0x00007ffff7bd96f3    addl   $0x1, %eax
+    38: 0x00007ffff7bd96f8    movl   %eax, -0x4(%rbp)
   libfoo.so`foo() + 27 at foo.cpp:6
-    [39] 0x00007ffff7bd96fb    movl   -0x4(%rbp), %eax
-    [40] 0x00007ffff7bd96fe    addq   $0x10, %rsp
-    [41] 0x00007ffff7bd9702    popq   %rbp
-    [42] 0x00007ffff7bd9703    retq''',
+    39: 0x00007ffff7bd96fb    movl   -0x4(%rbp), %eax
+    40: 0x00007ffff7bd96fe    addq   $0x10, %rsp
+    41: 0x00007ffff7bd9702    popq   %rbp
+    42: 0x00007ffff7bd9703    retq''',
   '''a.out`main + 68 at main.cpp:16
-    [43] 0x00000000004006a4    movl   -0xc(%rbp), %ecx
-    [44] 0x00000000004006a7    addl   %eax, %ecx
-    [45] 0x00000000004006a9    movl   %ecx, -0xc(%rbp)'''])
+    43: 0x00000000004006a4    movl   -0xc(%rbp), %ecx
+    44: 0x00000000004006a7    addl   %eax, %ecx
+    45: 0x00000000004006a9    movl   %ecx, -0xc(%rbp)'''])
 
 
         self.expect("thread trace dump instructions --count 50",
             substrs=['''thread #1: tid = 815455
   a.out`main + 73 at main.cpp:16
-    [  0] 0x00000000004006a9    movl   %ecx, -0xc(%rbp)
-    [ -1] 0x00000000004006a7    addl   %eax, %ecx
-    [ -2] 0x00000000004006a4    movl   -0xc(%rbp), %ecx
+    45: 0x00000000004006a9    movl   %ecx, -0xc(%rbp)
+    44: 0x00000000004006a7    addl   %eax, %ecx
+    43: 0x00000000004006a4    movl   -0xc(%rbp), %ecx
   libfoo.so`foo() + 35 at foo.cpp:6
-    [ -3] 0x00007ffff7bd9703    retq''',
-    '''[ -4] 0x00007ffff7bd9702    popq   %rbp
-    [ -5] 0x00007ffff7bd96fe    addq   $0x10, %rsp
-    [ -6] 0x00007ffff7bd96fb    movl   -0x4(%rbp), %eax
+    42: 0x00007ffff7bd9703    retq''',
+    '''41: 0x00007ffff7bd9702    popq   %rbp
+    40: 0x00007ffff7bd96fe    addq   $0x10, %rsp
+    39: 0x00007ffff7bd96fb    movl   -0x4(%rbp), %eax
   libfoo.so`foo() + 24 at foo.cpp:5
-    [ -7] 0x00007ffff7bd96f8    movl   %eax, -0x4(%rbp)
-    [ -8] 0x00007ffff7bd96f3    addl   $0x1, %eax
-    [ -9] 0x00007ffff7bd96f0    movl   -0x4(%rbp), %eax
+    38: 0x00007ffff7bd96f8    movl   %eax, -0x4(%rbp)
+    37: 0x00007ffff7bd96f3    addl   $0x1, %eax
+    36: 0x00007ffff7bd96f0    movl   -0x4(%rbp), %eax
   libfoo.so`foo() + 13 at foo.cpp:4
-    [-10] 0x00007ffff7bd96ed    movl   %eax, -0x4(%rbp)
+    35: 0x00007ffff7bd96ed    movl   %eax, -0x4(%rbp)
   libbar.so`bar() + 26 at bar.cpp:4
-    [-11] 0x00007ffff79d76aa    retq''',
-    '''[-12] 0x00007ffff79d76a9    popq   %rbp
-    [-13] 0x00007ffff79d76a6    movl   -0x4(%rbp), %eax
+    34: 0x00007ffff79d76aa    retq''',
+    '''33: 0x00007ffff79d76a9    popq   %rbp
+    32: 0x00007ffff79d76a6    movl   -0x4(%rbp), %eax
   libbar.so`bar() + 19 at bar.cpp:3
-    [-14] 0x00007ffff79d76a3    movl   %eax, -0x4(%rbp)
-    [-15] 0x00007ffff79d769e    addl   $0x1, %eax
-    [-16] 0x00007ffff79d769b    movl   -0x4(%rbp), %eax
+    31: 0x00007ffff79d76a3    movl   %eax, -0x4(%rbp)
+    30: 0x00007ffff79d769e    addl   $0x1, %eax
+    29: 0x00007ffff79d769b    movl   -0x4(%rbp), %eax
   libbar.so`bar() + 4 at bar.cpp:2
-    [-17] 0x00007ffff79d7694    movl   $0x1, -0x4(%rbp)
+    28: 0x00007ffff79d7694    movl   $0x1, -0x4(%rbp)
   libbar.so`bar() + 1 at bar.cpp:1
-    [-18] 0x00007ffff79d7691    movq   %rsp, %rbp
-    [-19] 0x00007ffff79d7690    pushq  %rbp
+    27: 0x00007ffff79d7691    movq   %rsp, %rbp
+    26: 0x00007ffff79d7690    pushq  %rbp
   libfoo.so`symbol stub for: bar()
-    [-20] 0x00007ffff7bd95d0    jmpq   *0x200a4a(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 32
+    25: 0x00007ffff7bd95d0    jmpq   *0x200a4a(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 32
   libfoo.so`foo() + 8 at foo.cpp:4
-    [-21] 0x00007ffff7bd96e8    callq  0x7ffff7bd95d0            ; symbol stub for: bar()
-    [-22] 0x00007ffff7bd96e4    subq   $0x10, %rsp
+    24: 0x00007ffff7bd96e8    callq  0x7ffff7bd95d0            ; symbol stub for: bar()
+    23: 0x00007ffff7bd96e4    subq   $0x10, %rsp
   libfoo.so`foo() + 1 at foo.cpp:3
-    [-23] 0x00007ffff7bd96e1    movq   %rsp, %rbp
-    [-24] 0x00007ffff7bd96e0    pushq  %rbp
+    22: 0x00007ffff7bd96e1    movq   %rsp, %rbp
+    21: 0x00007ffff7bd96e0    pushq  %rbp
   a.out`symbol stub for: foo()
-    [-25] 0x0000000000400540    jmpq   *0x200ae2(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 40
+    20: 0x0000000000400540    jmpq   *0x200ae2(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 40
   a.out`main + 63 at main.cpp:16
-    [-26] 0x000000000040069f    callq  0x400540                  ; symbol stub for: foo()
+    19: 0x000000000040069f    callq  0x400540                  ; symbol stub for: foo()
   a.out`main + 60 at main.cpp:14
-    [-27] 0x000000000040069c    movl   %ecx, -0xc(%rbp)
-    [-28] 0x000000000040069a    addl   %eax, %ecx
-    [-29] 0x0000000000400697    movl   -0xc(%rbp), %ecx
+    18: 0x000000000040069c    movl   %ecx, -0xc(%rbp)
+    17: 0x000000000040069a    addl   %eax, %ecx
+    16: 0x0000000000400697    movl   -0xc(%rbp), %ecx
   a.out`main + 52 [inlined] inline_function() + 18 at main.cpp:6
-    [-30] 0x0000000000400694    movl   -0x4(%rbp), %eax
+    15: 0x0000000000400694    movl   -0x4(%rbp), %eax
   a.out`main + 49 [inlined] inline_function() + 15 at main.cpp:5
-    [-31] 0x0000000000400691    movl   %eax, -0x4(%rbp)
-    [-32] 0x000000000040068c    addl   $0x1, %eax
-    [-33] 0x0000000000400689    movl   -0x4(%rbp), %eax
+    14: 0x0000000000400691    movl   %eax, -0x4(%rbp)
+    13: 0x000000000040068c    addl   $0x1, %eax
+    12: 0x0000000000400689    movl   -0x4(%rbp), %eax
   a.out`main + 34 [inlined] inline_function() at main.cpp:4
-    [-34] 0x0000000000400682    movl   $0x0, -0x4(%rbp)
+    11: 0x0000000000400682    movl   $0x0, -0x4(%rbp)
   a.out`main + 31 at main.cpp:12
-    [-35] 0x000000000040067f    movl   %eax, -0xc(%rbp)
-    [-36] 0x000000000040067a    addl   $0x1, %eax
-    [-37] 0x0000000000400677    movl   -0xc(%rbp), %eax
+    10: 0x000000000040067f    movl   %eax, -0xc(%rbp)
+    9: 0x000000000040067a    addl   $0x1, %eax
+    8: 0x0000000000400677    movl   -0xc(%rbp), %eax
   a.out`main + 20 at main.cpp:10
-    [-38] 0x0000000000400674    movl   %eax, -0xc(%rbp)
+    7: 0x0000000000400674    movl   %eax, -0xc(%rbp)
     ...missing instructions
-    [-39] 0x00007ffff7df1950    error: no memory mapped at this address
+    6: 0x00007ffff7df1950    error: no memory mapped at this address
   a.out`(none)
-    [-40] 0x0000000000400516    jmpq   *0x200af4(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 16
-    [-41] 0x0000000000400510    pushq  0x200af2(%rip)            ; _GLOBAL_OFFSET_TABLE_ + 8
+    5: 0x0000000000400516    jmpq   *0x200af4(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 16
+    4: 0x0000000000400510    pushq  0x200af2(%rip)            ; _GLOBAL_OFFSET_TABLE_ + 8
   a.out`symbol stub for: foo() + 11
-    [-42] 0x000000000040054b    jmp    0x400510
-    [-43] 0x0000000000400546    pushq  $0x2
-    [-44] 0x0000000000400540    jmpq   *0x200ae2(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 40
+    3: 0x000000000040054b    jmp    0x400510
+    2: 0x0000000000400546    pushq  $0x2
+    1: 0x0000000000400540    jmpq   *0x200ae2(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 40
   a.out`main + 15 at main.cpp:10
-    [-45] 0x000000000040066f    callq  0x400540                  ; symbol stub for: foo()'''])
+    0: 0x000000000040066f    callq  0x400540                  ; symbol stub for: foo()'''])
 
         self.expect("thread trace dump instructions --skip 100 --forwards", inHistory=True,
             substrs=['''thread #1: tid = 815455
Index: lldb/source/Target/TraceInstructionDumper.cpp
===================================================================
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -18,11 +18,34 @@
 using namespace lldb_private;
 using namespace llvm;
 
-TraceInstructionDumper::TraceInstructionDumper(lldb::TraceCursorUP &&cursor_up,
-                                               int initial_index, bool raw,
-                                               bool show_tsc)
-    : m_cursor_up(std::move(cursor_up)), m_index(initial_index), m_raw(raw),
-      m_show_tsc(show_tsc) {}
+TraceInstructionDumper::TraceInstructionDumper(
+    lldb::TraceCursorUP &&cursor_up, Stream &s,
+    const TraceInstructionDumperOptions &options)
+    : m_cursor_up(std::move(cursor_up)), m_options(options), m_s(s) {
+  // We first set the cursor in its initial position
+  if (m_options.id) {
+    if (!m_cursor_up->GoToId(*m_options.id)) {
+      s.PutCString("    invalid instruction id\n");
+      SetNoMoreData();
+      return;
+    }
+  } else if (m_options.forwards) {
+    m_cursor_up->Seek(0, TraceCursor::SeekType::Beginning);
+  } else {
+    m_cursor_up->Seek(0, TraceCursor::SeekType::End);
+  }
+
+  m_cursor_up->SetForwards(m_options.forwards);
+  if (m_options.skip) {
+    uint64_t to_skip = m_options.skip.getValue();
+    if (m_cursor_up->Seek((m_options.forwards ? 1 : -1) * to_skip,
+                          TraceCursor::SeekType::Current) < to_skip) {
+      // This happens when the skip value was more than the number of
+      // available instructions.
+      SetNoMoreData();
+    }
+  }
+}
 
 /// \return
 ///     Return \b true if the cursor could move one step.
@@ -31,19 +54,9 @@
     SetNoMoreData();
     return false;
   }
-  m_index += m_cursor_up->IsForwards() ? 1 : -1;
   return true;
 }
 
-/// \return
-///     The number of characters that would be needed to print the given
-///     integer.
-static int GetNumberOfChars(int num) {
-  if (num == 0)
-    return 1;
-  return (num < 0 ? 1 : 0) + static_cast<int>(log10(abs(num))) + 1;
-}
-
 /// Helper struct that holds symbol, disassembly and address information of an
 /// instruction.
 struct InstructionSymbolInfo {
@@ -159,36 +172,31 @@
 
 bool TraceInstructionDumper::HasMoreData() { return !m_no_more_data; }
 
-void TraceInstructionDumper::DumpInstructions(Stream &s, size_t count) {
+Optional<lldb::tid_t> TraceInstructionDumper::DumpInstructions(size_t count) {
   ThreadSP thread_sp = m_cursor_up->GetExecutionContextRef().GetThreadSP();
   if (!thread_sp) {
-    s.Printf("invalid thread");
-    return;
+    m_s.Printf("invalid thread");
+    return None;
   }
 
-  s.Printf("thread #%u: tid = %" PRIu64 "\n", thread_sp->GetIndexID(),
-           thread_sp->GetID());
-
-  int digits_count = GetNumberOfChars(
-      m_cursor_up->IsForwards() ? m_index + count - 1 : m_index - count + 1);
   bool was_prev_instruction_an_error = false;
 
   auto printMissingInstructionsMessage = [&]() {
-    s.Printf("    ...missing instructions\n");
+    m_s.Printf("    ...missing instructions\n");
   };
 
-  auto printInstructionIndex = [&]() {
-    s.Printf("    [%*d] ", digits_count, m_index);
+  auto printInstructionHeader = [&](uint64_t id) {
+    m_s.Printf("    %" PRIu64 ": ", id);
 
-    if (m_show_tsc) {
-      s.Printf("[tsc=");
+    if (m_options.show_tsc) {
+      m_s.Printf("[tsc=");
 
       if (Optional<uint64_t> timestamp = m_cursor_up->GetCounter(lldb::eTraceCounterTSC))
-        s.Printf("0x%016" PRIx64, *timestamp);
+        m_s.Printf("0x%016" PRIx64, *timestamp);
       else
-        s.Printf("unavailable");
+        m_s.Printf("unavailable");
 
-      s.Printf("] ");
+      m_s.Printf("] ");
     }
   };
 
@@ -244,11 +252,13 @@
                                         : InstructionSP());
   };
 
+  Optional<lldb::user_id_t> last_id;
   for (size_t i = 0; i < count; i++) {
     if (!HasMoreData()) {
-      s.Printf("    no more data\n");
+      m_s.Printf("    no more data\n");
       break;
     }
+    last_id = m_cursor_up->GetId();
 
     if (const char *err = m_cursor_up->GetError()) {
       if (!m_cursor_up->IsForwards() && !was_prev_instruction_an_error)
@@ -256,8 +266,8 @@
 
       was_prev_instruction_an_error = true;
 
-      printInstructionIndex();
-      s << err;
+      printInstructionHeader(m_cursor_up->GetId());
+      m_s << err;
     } else {
       if (m_cursor_up->IsForwards() && was_prev_instruction_an_error)
         printMissingInstructionsMessage();
@@ -266,7 +276,7 @@
 
       InstructionSymbolInfo insn_info;
 
-      if (!m_raw) {
+      if (!m_options.raw) {
         insn_info.load_address = m_cursor_up->GetLoadAddress();
         insn_info.exe_ctx = exe_ctx;
         insn_info.address.SetLoadAddress(insn_info.load_address, &target);
@@ -274,19 +284,20 @@
         std::tie(insn_info.disassembler, insn_info.instruction) =
             calculateDisass(insn_info.address, insn_info.sc);
 
-        DumpInstructionSymbolContext(s, prev_insn_info, insn_info);
+        DumpInstructionSymbolContext(m_s, prev_insn_info, insn_info);
       }
 
-      printInstructionIndex();
-      s.Printf("0x%016" PRIx64, m_cursor_up->GetLoadAddress());
+      printInstructionHeader(m_cursor_up->GetId());
+      m_s.Printf("0x%016" PRIx64, m_cursor_up->GetLoadAddress());
 
-      if (!m_raw)
-        DumpInstructionDisassembly(s, insn_info);
+      if (!m_options.raw)
+        DumpInstructionDisassembly(m_s, insn_info);
 
       prev_insn_info = insn_info;
     }
 
-    s.Printf("\n");
+    m_s.Printf("\n");
     TryMoveOneStep();
   }
+  return last_id;
 }
Index: lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
===================================================================
--- lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
+++ lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
@@ -130,7 +130,7 @@
 
   // Move cursor to the first instruction in the trace
   cursor.SetForwards(true);
-  cursor.Seek(0, TraceCursor::SeekType::Set);
+  cursor.Seek(0, TraceCursor::SeekType::Beginning);
 
   Target &target = thread.GetProcess()->GetTarget();
   auto function_name_from_load_address =
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -20,7 +20,7 @@
   TraceCursorIntelPT(lldb::ThreadSP thread_sp,
                      DecodedThreadSP decoded_thread_sp);
 
-  size_t Seek(int64_t offset, SeekType origin) override;
+  uint64_t Seek(int64_t offset, SeekType origin) override;
 
   virtual bool Next() override;
 
@@ -35,6 +35,10 @@
 
   bool IsError() override;
 
+  bool GoToId(lldb::user_id_t id) override;
+
+  lldb::user_id_t GetId() const override;
+
 private:
   size_t GetInternalInstructionSize();
 
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,7 +23,8 @@
   assert(m_decoded_thread_sp->GetInstructionsCount() > 0 &&
          "a trace should have at least one instruction or error");
   m_pos = m_decoded_thread_sp->GetInstructionsCount() - 1;
-  m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
+  m_tsc_range =
+      m_decoded_thread_sp->CalculateTscRange(m_pos, /*hint_range*/ None);
 }
 
 size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -56,7 +57,7 @@
   return false;
 }
 
-size_t TraceCursorIntelPT::Seek(int64_t offset, SeekType origin) {
+uint64_t TraceCursorIntelPT::Seek(int64_t offset, SeekType origin) {
   int64_t last_index = GetInternalInstructionSize() - 1;
 
   auto fitPosToBounds = [&](int64_t raw_pos) -> int64_t {
@@ -65,7 +66,7 @@
 
   auto FindDistanceAndSetPos = [&]() -> int64_t {
     switch (origin) {
-    case TraceCursor::SeekType::Set:
+    case TraceCursor::SeekType::Beginning:
       m_pos = fitPosToBounds(offset);
       return m_pos;
     case TraceCursor::SeekType::End:
@@ -80,7 +81,7 @@
   };
 
   int64_t dist = FindDistanceAndSetPos();
-  m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
+  m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos, m_tsc_range);
   return dist;
 }
 
@@ -111,3 +112,14 @@
 TraceCursorIntelPT::GetInstructionControlFlowType() {
   return m_decoded_thread_sp->GetInstructionControlFlowType(m_pos);
 }
+
+bool TraceCursorIntelPT::GoToId(user_id_t id) {
+  if (m_decoded_thread_sp->GetInstructionsCount() <= id)
+    return false;
+  m_pos = id;
+  m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos, m_tsc_range);
+
+  return true;
+}
+
+user_id_t TraceCursorIntelPT::GetId() const { return m_pos; }
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -71,13 +71,13 @@
   class TscRange {
   public:
     /// Check if this TSC range includes the given instruction index.
-    bool InRange(size_t insn_index);
+    bool InRange(size_t insn_index) const;
 
     /// Get the next range chronologically.
-    llvm::Optional<TscRange> Next();
+    llvm::Optional<TscRange> Next() const;
 
     /// Get the previous range chronologically.
-    llvm::Optional<TscRange> Prev();
+    llvm::Optional<TscRange> Prev() const;
 
     /// Get the TSC value.
     size_t GetTsc() const;
@@ -150,7 +150,17 @@
   /// If the trace was collected with TSC support, all the instructions of
   /// the trace will have associated TSCs. This means that this method will
   /// only return \b llvm::None if there are no TSCs whatsoever in the trace.
-  llvm::Optional<TscRange> CalculateTscRange(size_t insn_index) const;
+  ///
+  /// \param[in] insn_index
+  ///   The instruction index in question.
+  ///
+  /// \param[in] hint_range
+  ///   An optional range that might include the given index or might be a
+  ///   neighbor of it. It might help speed it traversals of the trace with
+  ///   short jumps.
+  llvm::Optional<TscRange> CalculateTscRange(
+      size_t insn_index,
+      const llvm::Optional<DecodedThread::TscRange> &hint_range) const;
 
   /// Check if an instruction given by its index is an error.
   bool IsInstructionAnError(size_t insn_idx) const;
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -112,7 +112,7 @@
   m_errors.try_emplace(m_instruction_ips.size(), toString(std::move(error)));
   m_instruction_ips.emplace_back(LLDB_INVALID_ADDRESS);
   m_instruction_sizes.emplace_back(0);
-  m_instruction_classes.emplace_back(pt_insn_class::ptic_unknown);
+  m_instruction_classes.emplace_back(pt_insn_class::ptic_error);
 }
 
 void DecodedThread::AppendError(llvm::Error &&error, uint64_t tsc) {
@@ -133,8 +133,25 @@
   return m_tsc_errors;
 }
 
-Optional<DecodedThread::TscRange>
-DecodedThread::CalculateTscRange(size_t insn_index) const {
+Optional<DecodedThread::TscRange> DecodedThread::CalculateTscRange(
+    size_t insn_index,
+    const Optional<DecodedThread::TscRange> &hint_range) const {
+  // We first try to check the given hint range in case we are traversing the
+  // trace in short jumps. If that fails, then we do the more expensive
+  // arbitrary lookup.
+  if (hint_range) {
+    Optional<TscRange> candidate_range;
+    if (insn_index < hint_range->GetStartInstructionIndex())
+      candidate_range = hint_range->Prev();
+    else if (insn_index > hint_range->GetEndInstructionIndex())
+      candidate_range = hint_range->Next();
+    else
+      candidate_range = hint_range;
+
+    if (candidate_range && candidate_range->InRange(insn_index))
+      return candidate_range;
+  }
+  // Now we do a more expensive lookup
   auto it = m_instruction_timestamps.upper_bound(insn_index);
   if (it == m_instruction_timestamps.begin())
     return None;
@@ -199,12 +216,12 @@
   return m_end_index;
 }
 
-bool DecodedThread::TscRange::InRange(size_t insn_index) {
+bool DecodedThread::TscRange::InRange(size_t insn_index) const {
   return GetStartInstructionIndex() <= insn_index &&
          insn_index <= GetEndInstructionIndex();
 }
 
-Optional<DecodedThread::TscRange> DecodedThread::TscRange::Next() {
+Optional<DecodedThread::TscRange> DecodedThread::TscRange::Next() const {
   auto next_it = m_it;
   ++next_it;
   if (next_it == m_decoded_thread->m_instruction_timestamps.end())
@@ -212,7 +229,7 @@
   return TscRange(next_it, *m_decoded_thread);
 }
 
-Optional<DecodedThread::TscRange> DecodedThread::TscRange::Prev() {
+Optional<DecodedThread::TscRange> DecodedThread::TscRange::Prev() const {
   if (m_it == m_decoded_thread->m_instruction_timestamps.begin())
     return None;
   auto prev_it = m_it;
Index: lldb/source/Commands/Options.td
===================================================================
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -1097,7 +1097,8 @@
 }
 
 let Command = "thread trace dump instructions" in {
-  def thread_trace_dump_instructions_forwards: Option<"forwards", "f">, Group<1>,
+  def thread_trace_dump_instructions_forwards: Option<"forwards", "f">,
+    Group<1>,
     Desc<"If specified, the trace is traversed forwards chronologically "
     "starting at the oldest instruction. Otherwise, it starts at the most "
     "recent one and the traversal is backwards.">;
@@ -1105,17 +1106,21 @@
     Arg<"Count">,
     Desc<"The number of instructions to display starting at the most recent "
     "instruction, or the oldest if --forwards is provided.">;
-  def thread_trace_dump_instructions_skip: Option<"skip", "s">,
-    Group<1>,
+  def thread_trace_dump_instructions_id: Option<"id", "i">, Group<1>,
     Arg<"Index">,
-    Desc<"How many instruction to skip from the end of the trace to start "
-    "dumping instructions, or from the beginning if --forwards is provided">;
+    Desc<"Custom starting instruction id from where to start traversing. This "
+    "id can be provided in decimal or hexadecimal representation.">;
+  def thread_trace_dump_instructions_skip: Option<"skip", "s">, Group<1>,
+    Arg<"Index">,
+    Desc<"How many instruction to skip from the starting position of the trace "
+    "before starting the traversal.">;
   def thread_trace_dump_instructions_raw : Option<"raw", "r">,
     Group<1>,
-    Desc<"Dump only instruction address without disassembly nor symbol information.">;
-  def thread_trace_dump_instructions_show_tsc : Option<"tsc", "t">,
-    Group<1>,
-    Desc<"For each instruction, print the corresponding timestamp counter if available.">;
+    Desc<"Dump only instruction address without disassembly nor symbol "
+    "information.">;
+  def thread_trace_dump_instructions_show_tsc : Option<"tsc", "t">, Group<1>,
+    Desc<"For each instruction, print the corresponding timestamp counter if "
+    "available.">;
   def thread_trace_dump_instructions_continue: Option<"continue", "C">,
     Group<1>,
     Desc<"Continue dumping instructions right where the previous invocation of this "
Index: lldb/source/Commands/CommandObjectThread.cpp
===================================================================
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -2099,8 +2099,7 @@
 #define LLDB_OPTIONS_thread_trace_dump_instructions
 #include "CommandOptions.inc"
 
-class CommandObjectTraceDumpInstructions
-    : public CommandObjectIterateOverThreads {
+class CommandObjectTraceDumpInstructions : public CommandObjectParsed {
 public:
   class CommandOptions : public Options {
   public:
@@ -2132,19 +2131,29 @@
               "invalid integer value for option '%s'",
               option_arg.str().c_str());
         else
-          m_skip = skip;
+          m_dumper_options.skip = skip;
+        break;
+      }
+      case 'i': {
+        uint64_t id;
+        if (option_arg.empty() || option_arg.getAsInteger(0, id))
+          error.SetErrorStringWithFormat(
+              "invalid integer value for option '%s'",
+              option_arg.str().c_str());
+        else
+          m_dumper_options.id = id;
         break;
       }
       case 'r': {
-        m_raw = true;
+        m_dumper_options.raw = true;
         break;
       }
       case 'f': {
-        m_forwards = true;
+        m_dumper_options.forwards = true;
         break;
       }
       case 't': {
-        m_show_tsc = true;
+        m_dumper_options.show_tsc = true;
         break;
       }
       case 'C': {
@@ -2159,11 +2168,8 @@
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
       m_count = kDefaultCount;
-      m_skip = 0;
-      m_raw = false;
-      m_forwards = false;
-      m_show_tsc = false;
       m_continue = false;
+      m_dumper_options = {};
     }
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
@@ -2174,23 +2180,19 @@
 
     // Instance variables to hold the values for command options.
     size_t m_count;
-    size_t m_skip;
-    bool m_raw;
-    bool m_forwards;
-    bool m_show_tsc;
-    bool m_continue;
+    size_t m_continue;
+    TraceInstructionDumperOptions m_dumper_options;
   };
 
   CommandObjectTraceDumpInstructions(CommandInterpreter &interpreter)
-      : CommandObjectIterateOverThreads(
+      : CommandObjectParsed(
             interpreter, "thread trace dump instructions",
-            "Dump the traced instructions for one or more threads. If no "
-            "threads are specified, show the current thread. Use the "
-            "thread-index \"all\" to see all threads.",
+            "Dump the traced instructions for one thread. If no "
+            "thread is specified, show the current thread.",
             nullptr,
-            eCommandRequiresProcess | eCommandTryTargetAPILock |
-                eCommandProcessMustBeLaunched | eCommandProcessMustBePaused |
-                eCommandProcessMustBeTraced) {}
+            eCommandRequiresProcess | eCommandRequiresThread |
+                eCommandTryTargetAPILock | eCommandProcessMustBeLaunched |
+                eCommandProcessMustBePaused | eCommandProcessMustBeTraced) {}
 
   ~CommandObjectTraceDumpInstructions() override = default;
 
@@ -2200,57 +2202,63 @@
                                                uint32_t index) override {
     std::string cmd;
     current_command_args.GetCommandString(cmd);
-    if (cmd.find("--continue") == std::string::npos)
+    if (cmd.find(" --continue") == std::string::npos)
       cmd += " --continue";
     return cmd;
   }
 
 protected:
-  bool DoExecute(Args &args, CommandReturnObject &result) override {
-    if (!m_options.m_continue)
-      m_dumpers.clear();
+  ThreadSP GetThread(Args &args, CommandReturnObject &result) {
+    if (args.GetArgumentCount() == 0)
+      return m_exe_ctx.GetThreadSP();
 
-    return CommandObjectIterateOverThreads::DoExecute(args, result);
-  }
-
-  bool HandleOneThread(lldb::tid_t tid, CommandReturnObject &result) override {
-    Stream &s = result.GetOutputStream();
+    const char *arg = args.GetArgumentAtIndex(0);
+    uint32_t thread_idx;
 
-    const TraceSP &trace_sp = m_exe_ctx.GetTargetSP()->GetTrace();
+    if (!llvm::to_integer(arg, thread_idx)) {
+      result.AppendErrorWithFormat("invalid thread specification: \"%s\"\n",
+                                   arg);
+      return nullptr;
+    }
     ThreadSP thread_sp =
-        m_exe_ctx.GetProcessPtr()->GetThreadList().FindThreadByID(tid);
-
-    if (!m_dumpers.count(thread_sp->GetID())) {
-      lldb::TraceCursorUP cursor_up = trace_sp->GetCursor(*thread_sp);
-      // Set up the cursor and return the presentation index of the first
-      // instruction to dump after skipping instructions.
-      auto setUpCursor = [&]() {
-        cursor_up->SetForwards(m_options.m_forwards);
-        if (m_options.m_forwards)
-          return cursor_up->Seek(m_options.m_skip, TraceCursor::SeekType::Set);
-        return -cursor_up->Seek(-m_options.m_skip, TraceCursor::SeekType::End);
-      };
-
-      int initial_index = setUpCursor();
+        m_exe_ctx.GetProcessRef().GetThreadList().FindThreadByIndexID(
+            thread_idx);
+    if (!thread_sp)
+      result.AppendErrorWithFormat("no thread with index: \"%s\"\n", arg);
+    return thread_sp;
+  }
 
-      auto dumper = std::make_unique<TraceInstructionDumper>(
-          std::move(cursor_up), initial_index, m_options.m_raw,
-          m_options.m_show_tsc);
+  bool DoExecute(Args &args, CommandReturnObject &result) override {
+    ThreadSP thread_sp = GetThread(args, result);
+    if (!thread_sp)
+      return false;
 
-      // This happens when the seek value was more than the number of available
-      // instructions.
-      if (std::abs(initial_index) < (int)m_options.m_skip)
-        dumper->SetNoMoreData();
+    Stream &s = result.GetOutputStream();
+    s.Printf("thread #%u: tid = %" PRIu64 "\n", thread_sp->GetIndexID(),
+             thread_sp->GetID());
 
-      m_dumpers[thread_sp->GetID()] = std::move(dumper);
+    if (m_options.m_continue) {
+      if (!m_last_id) {
+        result.AppendMessage("    no more data\n");
+        return true;
+      }
+      // We set up the options to continue one instruction past where
+      // the previous iteration stopped.
+      m_options.m_dumper_options.skip = 1;
+      m_options.m_dumper_options.id = m_last_id;
     }
 
-    m_dumpers[thread_sp->GetID()]->DumpInstructions(s, m_options.m_count);
+    const TraceSP &trace_sp = m_exe_ctx.GetTargetSP()->GetTrace();
+    TraceInstructionDumper dumper(trace_sp->GetCursor(*thread_sp), s,
+                                  m_options.m_dumper_options);
+    m_last_id = dumper.DumpInstructions(m_options.m_count);
     return true;
   }
 
   CommandOptions m_options;
-  std::map<lldb::tid_t, std::unique_ptr<TraceInstructionDumper>> m_dumpers;
+  // Last traversed id used to continue a repeat command. None means
+  // that all the trace has been consumed.
+  llvm::Optional<lldb::user_id_t> m_last_id;
 };
 
 // CommandObjectTraceDumpInfo
Index: lldb/include/lldb/Target/TraceInstructionDumper.h
===================================================================
--- lldb/include/lldb/Target/TraceInstructionDumper.h
+++ lldb/include/lldb/Target/TraceInstructionDumper.h
@@ -13,6 +13,26 @@
 
 namespace lldb_private {
 
+/// Class that holds the configuration used by \a TraceInstructionDumper for
+/// traversing and dumping instructions.
+struct TraceInstructionDumperOptions {
+  /// If \b true, the cursor will be iterated forwards starting from the
+  /// oldest instruction. Otherwise, the iteration starts from the most
+  /// recent instruction.
+  bool forwards = false;
+  /// Dump only instruction addresses without disassembly nor symbol
+  /// information.
+  bool raw = false;
+  /// For each instruction, print the corresponding timestamp counter if
+  /// available.
+  bool show_tsc = false;
+  /// Optional custom id to start traversing from.
+  llvm::Optional<uint64_t> id = llvm::None;
+  /// Optional number of instructions to skip from the starting position
+  /// of the cursor.
+  llvm::Optional<size_t> skip = llvm::None;
+};
+
 /// Class used to dump the instructions of a \a TraceCursor using its current
 /// state and granularity.
 class TraceInstructionDumper {
@@ -22,20 +42,13 @@
   /// \param[in] cursor
   ///     The cursor whose instructions will be dumped.
   ///
-  /// \param[in] initial_index
-  ///     Presentation index to use for referring to the current instruction
-  ///     of the cursor. If the direction is forwards, the index will increase,
-  ///     and if the direction is backwards, the index will decrease.
-  ///
-  /// \param[in] raw
-  ///     Dump only instruction addresses without disassembly nor symbol
-  ///     information.
+  /// \param[in] s
+  ///     The stream where to dump the instructions to.
   ///
-  /// \param[in] show_tsc
-  ///     For each instruction, print the corresponding timestamp counter if
-  ///     available.
-  TraceInstructionDumper(lldb::TraceCursorUP &&cursor_up, int initial_index = 0,
-                         bool raw = false, bool show_tsc = false);
+  /// \param[in] options
+  ///     Additional options for configuring the dumping.
+  TraceInstructionDumper(lldb::TraceCursorUP &&cursor_up, Stream &s,
+                         const TraceInstructionDumperOptions &options);
 
   /// Dump \a count instructions of the thread trace starting at the current
   /// cursor position.
@@ -43,21 +56,23 @@
   /// This effectively moves the cursor to the next unvisited position, so that
   /// a subsequent call to this method continues where it left off.
   ///
-  /// \param[in] s
-  ///     The stream object where the instructions are printed.
-  ///
   /// \param[in] count
   ///     The number of instructions to print.
-  void DumpInstructions(Stream &s, size_t count);
-
-  /// Indicate the dumper that no more data is available in the trace.
-  void SetNoMoreData();
+  ///
+  /// \return
+  ///     The instruction id of the last traversed instruction, or \b llvm::None
+  ///     if no instructions were visited.
+  llvm::Optional<lldb::user_id_t> DumpInstructions(size_t count);
 
   /// \return
   ///     \b true if there's still more data to traverse in the trace.
   bool HasMoreData();
 
 private:
+  /// Indicate to the dumper that no more data is available in the trace.
+  /// This will prevent further iterations.
+  void SetNoMoreData();
+
   /// Move the cursor one step.
   ///
   /// \return
@@ -65,9 +80,8 @@
   bool TryMoveOneStep();
 
   lldb::TraceCursorUP m_cursor_up;
-  int m_index;
-  bool m_raw;
-  bool m_show_tsc;
+  TraceInstructionDumperOptions m_options;
+  Stream &m_s;
   /// If \b true, all the instructions have been traversed.
   bool m_no_more_data = false;
 };
Index: lldb/include/lldb/Target/TraceCursor.h
===================================================================
--- lldb/include/lldb/Target/TraceCursor.h
+++ lldb/include/lldb/Target/TraceCursor.h
@@ -74,9 +74,10 @@
 public:
   /// Helper enum to indicate the reference point when invoking
   /// \a TraceCursor::Seek().
+  /// The following values are inspired by \a std::istream::seekg.
   enum class SeekType {
     /// The beginning of the trace, i.e the oldest item.
-    Set = 0,
+    Beginning = 0,
     /// The current position in the trace.
     Current,
     /// The end of the trace, i.e the most recent item.
@@ -133,6 +134,51 @@
   ///     \b true if the cursor effectively moved, \b false otherwise.
   virtual bool Next() = 0;
 
+  /// Instruction identifiers:
+  ///
+  /// When building complex higher level tools, fast random accesses in the
+  /// trace might be needed, for which each instruction requires a unique
+  /// identifier within its thread trace. For example, a tool might want to
+  /// repeatedly inspect random consecutive portions of a trace. This means that
+  /// it will need to first move quickly to the beginning of each section and
+  /// then start its iteration. Given that the number of instructions can be in
+  /// the order of hundreds of millions, fast random access is necessary.
+  ///
+  /// An example of such a tool could be an inspector of the call graph of a
+  /// trace, where each call is represented with its start and end instructions.
+  /// Inspecting all the instructions of a call requires moving to its first
+  /// instruction and then iterating until the last instruction, which following
+  /// the pattern explained above.
+  ///
+  /// Instead of using 0-based indices as identifiers, each Trace plug-in can
+  /// decide the nature of these identifiers and thus no assumptions can be made
+  /// regarding their ordering and sequentiality. The reason is that an
+  /// instruction might be encoded by the plug-in in a way that hides its actual
+  /// 0-based index in the trace, but it's still possible to efficiently find
+  /// it.
+  ///
+  /// Requirements:
+  /// - For a given thread, no two instructions have the same id.
+  /// - In terms of efficiency, moving the cursor to a given id should be as
+  ///   fast as possible, but not necessarily O(1). That's why the recommended
+  ///   way to traverse sequential instructions is to use the \a
+  ///   TraceCursor::Next() method and only use \a TraceCursor::GoToId(id)
+  ///   sparingly.
+
+  /// Make the cursor point to the item whose identifier is \p id.
+  ///
+  /// \return
+  ///     \b true if the given identifier exists and the cursor effectively
+  ///     moved. Otherwise, \b false is returned and the cursor doesn't change
+  ///     its position.
+  virtual bool GoToId(lldb::user_id_t id) = 0;
+
+  /// \return
+  ///     A unique identifier for the instruction or error this cursor is
+  ///     pointing to.
+  virtual lldb::user_id_t GetId() const = 0;
+  /// \}
+
   /// Make the cursor point to an item in the trace based on an origin point and
   /// an offset. This API doesn't distinguishes instruction types nor errors in
   /// the trace, unlike the \a TraceCursor::Next() method.
@@ -152,7 +198,7 @@
   ///
   /// \return
   ///     The number of trace items moved from the origin.
-  virtual size_t Seek(ssize_t offset, SeekType origin) = 0;
+  virtual uint64_t Seek(int64_t offset, SeekType origin) = 0;
 
   /// \return
   ///   The \a ExecutionContextRef of the backing thread from the creation time
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to