[PATCH] D113586: [DebugInfo][NFC] Add instr-ref documentation, migration guide

Orlando Cazalet-Hyams via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 17 09:03:55 PDT 2022


Orlando accepted this revision.
Orlando added a comment.
This revision is now accepted and ready to land.

LGTM with some nits addressed, thanks! I've hit accepted but please leave this up for a day or so in case anyone else has any further comments.



================
Comment at: llvm/docs/InstrRefDebugInfo.md:13
+describing what register or stack location a variable can be found in, for each
+instruction in that variables lexical scope. We could track the virtual
+register that the variable resides in through compilation, however this is
----------------
nit: variable's


================
Comment at: llvm/docs/InstrRefDebugInfo.md:42
+register the desired value is in. Instead, a more consistent way of identifying
+the instructions value is to refer to the MachineOperand where the value is
+defined: independently of which register is defined by that MachineOperand. In
----------------
nit: instruction's


================
Comment at: llvm/docs/InstrRefDebugInfo.md:91
+slots. This is performed in the [LiveDebugValues pass][LiveDebugValues], where
+the debug instructions and machine code is separated out into two independent
+functions:
----------------
nit: is separated -> are separated?


================
Comment at: llvm/docs/InstrRefDebugInfo.md:96
+
+LLVMs existing SSA tools are used to place PHIs for each function, between
+variable values and the values contained in machine locations, with value
----------------
nit: LLVM's


================
Comment at: llvm/docs/InstrRefDebugInfo.md:154-155
+
+With a substitution recorded in the MachineFunction that what was instruction
+number 1 operand 0, is now instruction number 2 operand 0. In LiveDebugValues,
+DBG_INSTR_REFs will be mapped through the substitution table to find the most
----------------
nit: I found the first sentence of this paragraph a little difficult to parse  - possible alternative suggested above.




================
Comment at: llvm/docs/InstrRefDebugInfo.md:161
+substitutions between an old and new instruction. It assumes that any operand
+that is a def in the old instruction, is a def in the new instruction at the
+same operand position. This works most of the time, for example in the example
----------------
nit: Removing this comma improves readability IMO.


================
Comment at: llvm/docs/InstrRefDebugInfo.md:168
+instruction, and MachineFunction::makeDebugValueSubstitution to record the
+mapping between register defines in the old and new instructions. If some
+values computed by the old instruction are no longer computed by the new
----------------
nit: should register defines be register definitions? I'm not sure if this is just terminology I'm not familiar with.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113586



More information about the llvm-commits mailing list