Skip to content

[DebugInfo] Preserve line and column number when merging debug info. #129960

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
Apr 4, 2025
Merged

[DebugInfo] Preserve line and column number when merging debug info. #129960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cb23baa
[DebugInfo] Preserve line and column number when merging debug info.
snehasish Mar 5, 2025
df04e25
Address comments.
snehasish Mar 5, 2025
e43ffd1
Update docs to mention SamplePGO debug info usage.
snehasish Mar 14, 2025
ffea531
Fix typo.
snehasish Mar 18, 2025
713ae85
Enhance the documentation for SamplePGO.
snehasish Mar 24, 2025
4ba8427
Address comments.
snehasish Mar 31, 2025
7129758
Fix formatting.
snehasish Apr 1, 2025
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
14 changes: 10 additions & 4 deletions llvm/docs/HowToUpdateDebugInfo.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ Introduction
============

Certain kinds of code transformations can inadvertently result in a loss of
debug info, or worse, make debug info misrepresent the state of a program.
debug info, or worse, make debug info misrepresent the state of a program. Debug
info availability is also essential for SamplePGO.

This document specifies how to correctly update debug info in various kinds of
code transformations, and offers suggestions for how to create targeted debug
Expand Down Expand Up @@ -89,9 +90,14 @@ has a location with an accurate scope attached, and b) to prevent misleading
single-stepping (or breakpoint) behavior. Often, merged instructions are memory
accesses which can trap: having an accurate scope attached greatly assists in
crash triage by identifying the (possibly inlined) function where the bad
memory access occurred. This rule is also meant to assist SamplePGO by banning
scenarios in which a sample of a block containing a merged instruction is
misattributed to a block containing one of the instructions-to-be-merged.
memory access occurred.

To maintain distinct source locations for SamplePGO, it is often beneficial to
retain an arbitrary but deterministic location instead of discarding line and
column information as part of merging. In particular, loss of location
information for calls inhibit optimizations such as indirect call promotion.
This behavior can be optionally enabled until support for accurately
representing merged instructions in the line table is implemented.

Examples of transformations that should follow this rule include:

Expand Down
13 changes: 13 additions & 0 deletions llvm/docs/SourceLevelDebugging.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ the stored debug information into source-language specific information. As
such, a debugger must be aware of the source-language, and is thus tied to a
specific language or family of languages.

.. _intro_consumers:

Debug information consumers
---------------------------

Expand All @@ -71,6 +73,17 @@ as Visual Studio and WinDBG. LLVM's debug information format is mostly derived
from and inspired by DWARF, but it is feasible to translate into other target
debug info formats such as STABS.

SamplePGO (also known as `AutoFDO <https://gcc.gnu.org/wiki/AutoFDO>`_)
is a variant of profile guided optimizations which uses hardware sampling based
profilers to collect branch frequency data with low overhead in production
environments. It relies on debug information to associate profile information
to LLVM IR which is then used to guide optimization heuristics. Maintaining
deterministic and distinct source locations is necessary to maximize the
accuracy of mapping hardware sample counts to LLVM IR. For example, DWARF
`discriminators <https://wiki.dwarfstd.org/Path_Discriminators.md>`_ allow
SamplePGO to distinguish between multiple paths of execution which map to the
same source line.

It would also be reasonable to use debug information to feed profiling tools
for analysis of generated code, or, tools for reconstructing the original
source from generated code.
Expand Down
21 changes: 21 additions & 0 deletions llvm/lib/IR/DebugInfoMetadata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "llvm/IR/IntrinsicInst.h"
#include "llvm/IR/Type.h"
#include "llvm/IR/Value.h"
#include "llvm/Support/CommandLine.h"

#include <numeric>
#include <optional>
Expand All @@ -34,6 +35,12 @@ cl::opt<bool> EnableFSDiscriminator(
cl::desc("Enable adding flow sensitive discriminators"));
} // namespace llvm

// When true, preserves line and column number by picking one of the merged
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like to have a more descriptive name for this option, since we are discarding information, we're not preserving half of the merged source location. I came up with some suggestions:

  • -merge-source-locs-by-picking
  • -pick-merged-source-locations
  • -merge-source-locations-with-selection
  • -merge-source-locations-by-choice
  • -no-approximate-merged-source-locs

I'm really just trying various synonyms for "pick", "select", "choose".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, "pick-merged-source-locations" sounds good to me.

// location info in a deterministic manner to assist sample based PGO.
static cl::opt<bool> PickMergedSourceLocations(
"pick-merged-source-locations", cl::init(false), cl::Hidden,
cl::desc("Preserve line and column number when merging locations."));

uint32_t DIType::getAlignInBits() const {
return (getTag() == dwarf::DW_TAG_LLVM_ptrauth_type ? 0 : SubclassData32);
}
Expand Down Expand Up @@ -125,6 +132,20 @@ DILocation *DILocation::getMergedLocation(DILocation *LocA, DILocation *LocB) {
if (LocA == LocB)
return LocA;

// For some use cases (SamplePGO), it is important to retain distinct source
// locations. When this flag is set, we choose arbitrarily between A and B,
// rather than computing a merged location using line 0, which is typically
// not useful for PGO.
if (PickMergedSourceLocations) {
auto A = std::make_tuple(LocA->getLine(), LocA->getColumn(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should filenames be compared here? DILocation has getFilename and getDirectory, which return StringRefs, so you can fill them in here in a reasonably straightforward way.

However, given the use case, would it be reasonable to always pick LocA?

Regardless, I think adding directories and files would follow the principle of least surprise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I've added getFilename and getDirectory to the comparison. Note that SamplePGO doesn't rely on file and directory today when using instruction locations.

would it be reasonable to always pick LocA

I think the pick policy should be independent of the order of the parameters passed to this function. So getMergedLocation(A, B) == getMergedLocation(B, A).

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Does std::forward_as_tuple work here (to avoid copies)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the only place it would be useful is if getFilename and getDirectory returned string, but they return StringRef. Also you'd have to rewrite this to do the comparison without the auto A and auto B vars or you'd introduce lifetime issues if you just replaced make_tuple with forward_as_tuple.

LocA->getDiscriminator(), LocA->getFilename(),
LocA->getDirectory());
auto B = std::make_tuple(LocB->getLine(), LocB->getColumn(),
LocB->getDiscriminator(), LocB->getFilename(),
LocB->getDirectory());
return A < B ? LocA : LocB;
}

LLVMContext &C = LocA->getContext();

using LocVec = SmallVector<const DILocation *>;
Expand Down
77 changes: 77 additions & 0 deletions llvm/test/DebugInfo/pick-merged-source-locations.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
;; This test verifies that we assign a deterministic location for merged
;; instructions when -pick-merged-source-locations is enabled. We use the
;; simplifycfg pass to test this behaviour since it was a common source of
;; merged instructions, however we intend this to apply to all users of the
;; getMergedLocation API.

;; Run simplifycfg and check that only 1 call to bar remains and it's debug
;; location has a valid line number (lexicographically smallest).
; RUN: opt %s -passes=simplifycfg -hoist-common-insts -pick-merged-source-locations -S | FileCheck %s --check-prefix=ENABLED
; ENABLED: call i32 @bar{{.*!dbg !}}[[TAG:[0-9]+]]
; ENABLED-NOT: call i32 @bar
; ENABLED: ![[TAG]] = !DILocation(line: 9, column: 16, scope: !9)

;; Run simplifycfg without the pass to ensure that we don't spuriously start
;; passing the test if simplifycfg behaviour changes.
; RUN: opt %s -passes=simplifycfg -hoist-common-insts -pick-merged-source-locations=false -S | FileCheck %s --check-prefix=DISABLED
; DISABLED: call i32 @bar{{.*!dbg !}}[[TAG:[0-9]+]]
; DISABLED-NOT: call i32 @bar
; DISABLED: ![[TAG]] = !DILocation(line: 0, scope: !9)

; ModuleID = '../llvm/test/DebugInfo/Inputs/debug-info-merge-call.c'
source_filename = "../llvm/test/DebugInfo/Inputs/debug-info-merge-call.c"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

; Function Attrs: nounwind uwtable
define dso_local i32 @test(i32 %n) !dbg !9 {
entry:
%call = call i32 @foo(i32 %n), !dbg !12
%cmp1 = icmp sgt i32 %n, 100, !dbg !13
br i1 %cmp1, label %if.then, label %if.else, !dbg !13

if.then: ; preds = %entry
%call2 = call i32 @bar(i32 %n), !dbg !14
%add = add nsw i32 %call2, %call, !dbg !15
br label %if.end, !dbg !16

if.else: ; preds = %entry
%call4 = call i32 @bar(i32 %n), !dbg !17
br label %if.end

if.end: ; preds = %if.else, %if.then
%r.0 = phi i32 [ %add, %if.then ], [ %call4, %if.else ], !dbg !18
ret i32 %r.0, !dbg !19
}

declare !dbg !20 i32 @foo(i32)

declare !dbg !21 i32 @bar(i32)

!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!2, !3, !4, !5, !6, !7}
!llvm.ident = !{!8}

!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 21.0.0git ([email protected]:snehasish/llvm-project.git 6ce41db6b0275d060d6e60f88b96a1657024345c)", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, splitDebugInlining: false, nameTableKind: None)
!1 = !DIFile(filename: "../llvm/test/DebugInfo/Inputs/debug-info-merge-call.c", directory: "/usr/local/google/home/snehasishk/working/llvm-project/build-assert", checksumkind: CSK_MD5, checksum: "ac1be6c40dad11691922d600f9d55c55")
!2 = !{i32 7, !"Dwarf Version", i32 5}
!3 = !{i32 2, !"Debug Info Version", i32 3}
!4 = !{i32 1, !"wchar_size", i32 4}
!5 = !{i32 8, !"PIC Level", i32 2}
!6 = !{i32 7, !"PIE Level", i32 2}
!7 = !{i32 7, !"uwtable", i32 2}
!8 = !{!"clang version 21.0.0git ([email protected]:snehasish/llvm-project.git 6ce41db6b0275d060d6e60f88b96a1657024345c)"}
!9 = distinct !DISubprogram(name: "test", scope: !1, file: !1, line: 5, type: !10, scopeLine: 5, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
!10 = !DISubroutineType(types: !11)
!11 = !{}
!12 = !DILocation(line: 7, column: 13, scope: !9)
!13 = !DILocation(line: 8, column: 8, scope: !9)
!14 = !DILocation(line: 9, column: 16, scope: !9)
!15 = !DILocation(line: 9, column: 14, scope: !9)
!16 = !DILocation(line: 10, column: 3, scope: !9)
!17 = !DILocation(line: 11, column: 10, scope: !9)
!18 = !DILocation(line: 0, scope: !9)
!19 = !DILocation(line: 13, column: 3, scope: !9)
!20 = !DISubprogram(name: "foo", scope: !1, file: !1, line: 2, type: !10, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
!21 = !DISubprogram(name: "bar", scope: !1, file: !1, line: 1, type: !10, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)