[llvm] [DebugInfo] Update policy for when to merge locations (PR #115349)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-debuginfo

Author: Stephen Tozer (SLTozer)

<details>
<summary>Changes</summary>

Following discussions on PR #<!-- -->114231 this patch changes the policy on merging locations, making the rule that new instructions should use a merge of the locations of all the instructions whose output is produced by the new instructions; in the case where only one instruction's output is produced, as in most InstCombine optimizations, we use only that instruction's location.

Happy to take suggestions on the wording from this one - I've not found an up-front explanation that makes it totally clear, but the examples hopefully help to clarify.

---
Full diff: https://github.com/llvm/llvm-project/pull/115349.diff


1 Files Affected:

- (modified) llvm/docs/HowToUpdateDebugInfo.rst (+16-7) 


``````````diff
diff --git a/llvm/docs/HowToUpdateDebugInfo.rst b/llvm/docs/HowToUpdateDebugInfo.rst
index f7db92d58f4356..2d2323634b4f07 100644
--- a/llvm/docs/HowToUpdateDebugInfo.rst
+++ b/llvm/docs/HowToUpdateDebugInfo.rst
@@ -76,9 +76,13 @@ When to merge instruction locations
 -----------------------------------
 
 A transformation should merge instruction locations if it replaces multiple
-instructions with a single merged instruction, *and* that merged instruction
-does not correspond to any of the original instructions' locations. The API to
-use is ``Instruction::applyMergedLocation``.
+instructions with one or more new instructions, *and* the new instruction(s)
+produce the output of more than one of the original instructions. The API to
+use is ``Instruction::applyMergedLocation``, and the new location should be a
+merge of the locations of all the instructions whose output is produced in the
+new instructions; typically, this includes any instruction being RAUWed by a new
+instruction, and excludes any instruction that only produces an intermediate
+value used by the RAUWed instruction.
 
 The purpose of this rule is to ensure that a) the single merged instruction
 has a location with an accurate scope attached, and b) to prevent misleading
@@ -101,10 +105,10 @@ Examples of transformations that should follow this rule include:
 * Merging identical loop-invariant stores (see the LICM utility
   ``llvm::promoteLoopAccessesToScalars``).
 
-* Peephole optimizations which combine multiple instructions together, like
-  ``(add (mul A B) C) => llvm.fma.f32(A, B, C)``.  Note that the location of
-  the ``fma`` does not exactly correspond to the locations of either the
-  ``mul`` or the ``add`` instructions.
+* Scalar instructions being combined into a vector instruction, like
+  ``(add A1, B1), (add A2, B2) => (add (A1, A2), (B1, B2))``. As the new vector
+  ``add`` computes the result of both original ``add`` instructions
+  simultaneously, it should use a merge of the two locations.
 
 Examples of transformations for which this rule *does not* apply include:
 
@@ -113,6 +117,11 @@ Examples of transformations for which this rule *does not* apply include:
   ``zext`` is modified but remains in its block, so the rule for
   :ref:`preserving locations<WhenToPreserveLocation>` should apply.
 
+* Peephole optimizations which combine multiple instructions together, like
+  ``(add (mul A B) C) => llvm.fma.f32(A, B, C)``. Note that the result of the
+  ``mul`` no longer appears in the program, while the result of the ``add`` is
+  now produced by the ``fma``, so the ``add``'s location should be used.
+
 * Converting an if-then-else CFG diamond into a ``select``. Preserving the
   debug locations of speculated instructions can make it seem like a condition
   is true when it's not (or vice versa), which leads to a confusing

``````````

</details>


https://github.com/llvm/llvm-project/pull/115349