[PATCH] D49053: [WIP][Docs] Debugging optimized code with debugify

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: docs/SourceLevelDebugging.rst:1482
+
+The first applies synthetic debug information to each non null instruction
+in the module, while the latter checks that this DI is still available after
----------------
"non null" is implied here, please omit it.


================
Comment at: docs/SourceLevelDebugging.rst:1485
+an optimization has occurred, reporting any errors/warnings while doing so.
+
+For example:
----------------
Could you add a section explaining what kind of debug information is attached? Specifically: instructions are assigned sequentially-increasing line locations, and are immediately used by debug value intrinsics when possible. It would help if the first example is a comparison of a short function before/after -debugify is applied.


================
Comment at: docs/SourceLevelDebugging.rst:1486
+
+For example:
+
----------------
This is a good example, but it should come after the one which shows a before/after with -debugify. Please explain what the error message means and how (at a high level) to fix it. You also only really need one ERROR line here, please omit the rest.


================
Comment at: docs/SourceLevelDebugging.rst:1527
+   $ opt -debugify-each -O2 sample.ll
+
+``debugify`` in regression tests
----------------
Please show an example of using -debugify to test a backend, e.g:
$ opt -debugify < sample.ll | llc -o -


================
Comment at: docs/SourceLevelDebugging.rst:1540
+
+.. code-block:: llvm
+
----------------
I think it'd be better to use a real, concrete example here. Please find one with stricter CHECK lines from the perspective of the actual optimization (e.g check the operands of the mul).


================
Comment at: docs/SourceLevelDebugging.rst:1551
+   }
+   ; DEBUGLOC:         ![[DbgLoc]] = DILocation(line: 2
+
----------------
Please justify the '2' here to readers (btw why isn't it 1?). It's worth adding a note that checking for a DILocation in this way is brittle. If someone updating the test inserts an instruction in the original IR before the second instruction, the test will break. Discuss the tradeoff between having a brittle test, and a test that is stringent enough to check for correct debug location preservation.


================
Comment at: docs/SourceLevelDebugging.rst:1554
+In the above example we are testing that the mul instruction has a debug location
+after the pass and it corresponds to line 2.
----------------
It's not quite enough to say that the mul corresponds to line 2. The test should be checking that the mul looks correct after `-pass-to-test` runs, and it should explain why line 2 is the correct line for the mul to be on.


Repository:
  rL LLVM

https://reviews.llvm.org/D49053