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

[Stephen Tozer via llvm-commits]

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

>From 12ca4ee046828a916256c7282263a96aa7df3529 Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Thu, 7 Nov 2024 17:31:31 +0000
Subject: [PATCH 1/3] [DebugInfo] Update policy for when to merge locations

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.
---
 llvm/docs/HowToUpdateDebugInfo.rst | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

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

>From 5faa5972d557feb41d28aa5af08405fa23b17752 Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Fri, 8 Nov 2024 15:31:42 +0000
Subject: [PATCH 2/3] Add clearer description and example for multiple merge
 instructions

---
 llvm/docs/HowToUpdateDebugInfo.rst | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/llvm/docs/HowToUpdateDebugInfo.rst b/llvm/docs/HowToUpdateDebugInfo.rst
index 2d2323634b4f07..4796a3fc784a62 100644
--- a/llvm/docs/HowToUpdateDebugInfo.rst
+++ b/llvm/docs/HowToUpdateDebugInfo.rst
@@ -78,11 +78,11 @@ When to merge instruction locations
 A transformation should merge instruction locations if it replaces multiple
 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.
+use is ``Instruction::applyMergedLocation``, and the new location for each new
+instruction should be a merge of the locations of all the instructions whose
+output is produced 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
@@ -108,7 +108,12 @@ Examples of transformations that should follow this rule include:
 * 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.
+  simultaneously, it should use a merge of the two locations. Similarly, if
+  prior optimizations have already produced vectors ``(A1, A2)`` and
+  ``(B2, B1)``, then we might create a ``(shufflevector (1, 0), (B2, B1))``
+  instruction to produce ``(B1, B2)`` for the vector ``add``; in this case we've
+  created two instructions to replace the original ``adds``, so both new
+  instructions should use the merged location.
 
 Examples of transformations for which this rule *does not* apply include:
 

>From a778df89aebd9a0cb7717d657574f6df7d7f3947 Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Fri, 8 Nov 2024 15:39:29 +0000
Subject: [PATCH 3/3] Use the better wording already suggested

---
 llvm/docs/HowToUpdateDebugInfo.rst | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/llvm/docs/HowToUpdateDebugInfo.rst b/llvm/docs/HowToUpdateDebugInfo.rst
index 4796a3fc784a62..d8c300f2f3a703 100644
--- a/llvm/docs/HowToUpdateDebugInfo.rst
+++ b/llvm/docs/HowToUpdateDebugInfo.rst
@@ -77,12 +77,12 @@ When to merge instruction locations
 
 A transformation should merge instruction locations if it replaces multiple
 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 for each new
-instruction should be a merge of the locations of all the instructions whose
-output is produced 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.
+produce the output of more than one of the original instructions. The API to use
+is ``Instruction::applyMergedLocation``. For each new instruction I, its new
+location should be a merge of the locations of all instructions whose output is
+produced by I. 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