[PATCH] D81198: [docs] Specify rules for updating debug locations

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/docs/HowToUpdateDebugInfo.rst:29
+
+A transformation must preserve the debug location of an instruction if the
+instruction either remains in its basic block, or if its basic block is folded
----------------
Is this kind of "must" language consistent with the rest of this document? In this case it's an aspirational, sort of "this is ideal/what we're striving for" rather than "if you don't do this it'll fail the verifier/break", etc?


================
Comment at: llvm/docs/HowToUpdateDebugInfo.rst:62-64
+A transformation must merge instruction locations if it replaces multiple
+instructions with a single merged instruction, *and* the merged instruction is
+in a different block than at least one of the instructions-to-be-merged. The
----------------
Should this be "or" rather than "and"? If you merge two instructions even in the same BB, I think the goal is you should use a merged location, right?


================
Comment at: llvm/docs/HowToUpdateDebugInfo.rst:121-123
+When creating a new instruction that doesn't map back to a source line, use an
+artificial line 0 location instead of setting no location at all. The API to
+use for this is ``DILocation::get()``.
----------------
Might be worth a separate header - not sure how consistently we do this or how problematic it is if this isn't done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81198