Skip to content

Refine the examples in the debug info document #86272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 9, 2024
Merged

Refine the examples in the debug info document #86272

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7727a9d
refine the examples of merging locations
Apochens Mar 22, 2024
6d5ad3f
refine description
Apochens Mar 22, 2024
c314066
refine the example
Apochens Aug 23, 2024
7488deb
add the counter-example
Apochens Aug 28, 2024
51ab177
refine wording
Apochens Aug 29, 2024
b146046
refine formatting
Apochens Aug 29, 2024
da54fa1
refine wording
Apochens Aug 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions llvm/docs/HowToUpdateDebugInfo.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,12 @@ misattributed to a block containing one of the instructions-to-be-merged.

Examples of transformations that should follow this rule include:

* Merging identical loads/stores which occur on both sides of a CFG diamond
(see the ``MergedLoadStoreMotion`` pass).
* Hoisting identical instructions from all successors of a conditional branch
or sinking those from all paths to a postdominating block. For example,
merging identical loads/stores which occur on both sides of a CFG diamond
(see the ``MergedLoadStoreMotion`` pass). For each group of identical
instructions being hoisted/sunk, the merge of all their locations should be
applied to the merged instruction.

* Merging identical loop-invariant stores (see the LICM utility
``llvm::promoteLoopAccessesToScalars``).
Expand All @@ -115,10 +119,12 @@ Examples of transformations for which this rule *does not* apply include:
single-stepping experience. The rule for
:ref:`dropping locations<WhenToDropLocation>` should apply here.

* Hoisting identical instructions which appear in several successor blocks into
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there wording we are preserving that covers the case of hoisting/sinking when some but not all successors/predecessors have the instruction? If not, we should probably keep this bullet point and clarify/correct it to refer to this case (of some but not all predecessors/successors)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll handle this.

a predecessor block (see ``BranchFolder::HoistCommonCodeInSuccs``). In this
case there is no single merged instruction. The rule for
:ref:`dropping locations<WhenToDropLocation>` applies.
* Hoisting/sinking a part of the identical instructions with the same location.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sentence above reads a bit broken, it's unclear what "a part of the identical instructions" is.
Maybe something like: "Hoisting/sinking that would make a location reachable when it previously wasn't"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This is a good suggestion.

Consider hoisting two identical instructions with the same location from
first two cases of a switch that has three cases. Merging their locations
would make the location from the first two cases reachable when the third
case is taken. The rule for :ref:`dropping locations<WhenToDropLocation>`
applies.

.. _WhenToDropLocation:

Expand Down
Loading