Page MenuHomePhabricator

added intel-pt build instructions for lldb
ClosedPublic

Authored by zrthxn on Mar 17 2022, 11:07 AM.

Diff Detail

Event Timeline

zrthxn created this revision.Mar 17 2022, 11:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 17 2022, 11:07 AM
zrthxn requested review of this revision.Mar 17 2022, 11:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 17 2022, 11:07 AM
JDevlieghere requested changes to this revision.Mar 17 2022, 11:10 AM

Can we include this in the docs folder instead? Having READMEs spread throughout the sources isn't going to be particularly discoverable.

This revision now requires changes to proceed.Mar 17 2022, 11:10 AM

Can we include this in the docs folder instead? Having READMEs spread throughout the sources isn't going to be particularly discoverable.

Sure can. Should I also use RST instead of MD? Also which particular folder in docs?

wallace requested changes to this revision.Mar 17 2022, 11:40 AM

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.llvm.org/index.html as an "Tracing with Intel PT" section under the "Testing LLDB using QEMU" section.
You can follow the format of https://lldb.llvm.org/use/qemu-testing.html for the document.

lldb/source/Plugins/Trace/intel-pt/README.md
1 ↗(On Diff #416259)

Tracing with Intel Processor Trace (Intel PT)

2 ↗(On Diff #416259)

Intel PT is a technology available in modern Intel CPUs that allows efficient tracing of all the instructions executed by a process. LLDB can collect traces and dump them using its symbolication stack. You can read more here https://easyperf.net/blog/2019/08/23/Intel-Processor-Trace.

3 ↗(On Diff #416259)

Prerequisites

5 ↗(On Diff #416259)
11 ↗(On Diff #416259)
15 ↗(On Diff #416259)
15–40 ↗(On Diff #416259)

Let's keep it concise and direct

20–23 ↗(On Diff #416259)

remove this, because you can assume the developer already knows how to build lldb

25–27 ↗(On Diff #416259)

only keep libipt-build

42–60 ↗(On Diff #416259)
62–81 ↗(On Diff #416259)

Instead of these instructions, you can use the internal documentation I have for using Intel PT.

https://pastebin.com/UiJJ32uh

Copy paste it here and add the appropriate formatting as a "How to use" section :)

85–86 ↗(On Diff #416259)
90 ↗(On Diff #416259)

just delete this one

zrthxn marked 12 inline comments as done.Mar 17 2022, 12:21 PM
zrthxn updated this revision to Diff 416287.EditedMar 17 2022, 12:32 PM

Switched to RST. Although I'm not very familiar with RST, I've done my best to match the existing docs. Please let me know if changes are needed.

wallace requested changes to this revision.Mar 17 2022, 5:01 PM

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 to lldb/docs/use/intel_pt.rst and reference it from lldb/docs/index.rst.

Besides that, you should generate the documentation and make sure that the output html looks correct. I haven't done that myself, but according to https://reviews.llvm.org/D55376, you should be able to do ninja docs-lldb-html after setting as true the cmake flag -DLLVM_ENABLE_SPHINX. Then, include a screenshot of the generated documentation.

lldb/docs/intel_pt.rst
17 ↗(On Diff #416287)
48 ↗(On Diff #416287)

just remove this, as it's a little bit obvious

51 ↗(On Diff #416287)

let's put this as a one liner as it's common to everyone

This revision now requires changes to proceed.Mar 17 2022, 5:01 PM
zrthxn marked 3 inline comments as done.Mar 18 2022, 7:31 AM

Here is the screenshot as well as the HTML file
The screenshot doesn't have the CSS theming...

zrthxn updated this revision to Diff 416506.Mar 18 2022, 7:49 AM

Referenced IPT doc from main page

wallace accepted this revision.Mar 18 2022, 8:34 AM

lgtm

JDevlieghere accepted this revision.Mar 18 2022, 9:26 AM

Please reflow the document to match the 80 column limit observed in the other docs. LGTM with that addressed.

This revision is now accepted and ready to land.Mar 18 2022, 9:26 AM
zrthxn updated this revision to Diff 416532.EditedMar 18 2022, 9:47 AM

Reflow to 80 cols

Please reflow the document to match the 80 column limit observed in the other docs. LGTM with that addressed.

I've tried my best to do this, it wasn't possible on some lines

zrthxn closed this revision.Mar 19 2022, 12:26 AM
zrthxn reopened this revision.Mar 19 2022, 2:29 AM
This revision is now accepted and ready to land.Mar 19 2022, 2:29 AM
zrthxn closed this revision.Mar 19 2022, 2:54 AM