[NFC] Leave a comment in Local.cpp about debug info & sample profiling
#155296
+5
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Aug 25, 2025
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@llvm/pr-subscribers-llvm-transforms Author: Mircea Trofin (mtrofin) ChangesFull diff: https://github.com/llvm/llvm-project/pull/155296.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index ac344904f90f0..821a58caf10b2 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -3342,8 +3342,11 @@ void llvm::hoistAllInstructionsInto(BasicBlock *DomBlock, Instruction *InsertPt,
// retain their original debug locations (DILocations) and debug intrinsic
// instructions.
//
- // Doing so would degrade the debugging experience and adversely affect the
- // accuracy of profiling information.
+ // Doing so would degrade the debugging experience.
+ //
+ // FIXME: Issue #152767: debug info should also be the same as the
+ // original branch, **if** the user explicitly indicated that (for sampling
+ // PGO)
//
// Currently, when hoisting the instructions, we take the following actions:
// - Remove their debug intrinsic instructions.
|
|
Found this in a drive-by, worth stamping it with the issue so it's easy to find it later. |
dwblaikie
approved these changes
Aug 25, 2025
There was a problem hiding this comment.
I've still got concerns about this direction/divergence between users and profilers & would love to see some attempt at unification - but a comment that points to the issue at least centralizes the discusison and leaves breadcrumbs towards it when it needs to be cleaned up in any direction later.
c255297 to
9038659
Compare
8422d7e to
088311a
Compare
9038659 to
9b6d2e7
Compare
088311a to
2e45cf2
Compare
alanzhao1
approved these changes
Aug 28, 2025
2e45cf2 to
e8a9ad0
Compare
9b6d2e7 to
8489a8a
Compare
e8a9ad0 to
d8edb04
Compare
03284b6 to
6f1eaf0
Compare
d8edb04 to
2c17eab
Compare
2c17eab to
e6959f2
Compare
6f1eaf0 to
29e3626
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #152767