[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

[walter erquinigo via Phabricator via lldb-commits]

wallace marked an inline comment as done.
wallace added inline comments.


================
Comment at: lldb/test/API/commands/trace/TestTraceDumpInstructions.py:48
+  [ 4] 0x40052d
+  [ 5] 0x400529
+  [ 6] 0x400525
----------------
clayborg wrote:
> If we reverse the direction, then hitting "enter" after doing one command won't flow as nicely as it does now. That being said I agree with Pavel that we should figure out what is expected. I generally think that earlier text is older.
> 
> I would not switch the indexes so that they change with any options that are specified. We currently have --start-position, but maybe this should be just --position? Or we specify:
> ```
> --from-end <offset> 
> ```
> <offset> would be the index offset from the end (newest) of the data?
> ```
> --from-start <offset>
> ```
> <offset> would be the index offset from the start (oldest) of the data?
> 
> I would be fine with:
> ```
> [--forwards | -f] [--backwards | -b]
> ```
> 
> but I think it would make sense to show the indexes in a consistent way regardless of what options are displayed. Maybe it makes sense to always show the true index where zero is the oldest and N is the newest?
> 
> We do need to make sure the auto repeat command looks good though which will be hard with oldest to newest ordering.
What about this:

We expose the indices in a chronologically increasing way, where [0] is the oldest instruction and [N] is the newest. Then we have the two options suggested by Greg

  --from-end <offset>

Where offset is an index or the string "end", meaning the last instruction of the trace, in case the user doesn't know the index of it. Then the instructions are printed

    [offset]
    [offset - 1]
    ...
    [offset - K]

And if there's a repeat command, this would be printed
    
    [offset - K - 1]
    [offset - K - 2]
    ...

Which would look nicely as a contiguous list of instructions if concatenated.

The other option would be

  --from-start<offset>

Where offset is an index.  Then the instructions are printed

    [offset]
    [offset + 1]
    [offset + 2]
    ...
    [offset + K]
    
And after a repeat command, you'd get

    [offset + K + 1]
    [offset + K + 2]
    ...


I think this would serve all purposes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89283