Skip to content

Commit

Permalink
[LICM] Try to merge debug locations when sinking.
Browse files Browse the repository at this point in the history
The current strategy LICM uses when sinking for debuginfo is
that of picking the debug location of one of the uses.
This causes stepping to be wrong sometimes, see, e.g. PR45523.

This patch introduces a generalization of getMergedLocation(),
that operates on a vector of locations instead of two, and try
to merge all them together, and use the new API in LICM.

<rdar://problem/61750950>
  • Loading branch information
Davide Italiano committed Apr 15, 2020
1 parent b2dff0d commit 5f87415
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 5 deletions.
7 changes: 7 additions & 0 deletions llvm/include/llvm/IR/DebugInfoMetadata.h
Expand Up @@ -1546,6 +1546,13 @@ class DILocation : public MDNode {
static const DILocation *getMergedLocation(const DILocation *LocA,
const DILocation *LocB);

/// Try to combine the vector of locations passed as input in a single one.
/// This function applies getMergedLocation() repeatedly left-to-right.
///
/// \p Locs: The locations to be merged.
static
const DILocation *getMergedLocations(ArrayRef<const DILocation *> Locs);

/// Returns the base discriminator for a given encoded discriminator \p D.
static unsigned getBaseDiscriminatorFromDiscriminator(unsigned D) {
return getUnsignedFromPrefixEncoding(D);
Expand Down
15 changes: 15 additions & 0 deletions llvm/lib/IR/DebugInfoMetadata.cpp
Expand Up @@ -75,6 +75,21 @@ DILocation *DILocation::getImpl(LLVMContext &Context, unsigned Line,
Storage, Context.pImpl->DILocations);
}

const
DILocation *DILocation::getMergedLocations(ArrayRef<const DILocation *> Locs) {
if (Locs.empty())
return nullptr;
if (Locs.size() == 1)
return Locs[0];
auto *Merged = Locs[0];
for (auto I = std::next(Locs.begin()), E = Locs.end(); I != E; ++I) {
Merged = getMergedLocation(Merged, *I);
if (Merged == nullptr)
break;
}
return Merged;
}

const DILocation *DILocation::getMergedLocation(const DILocation *LocA,
const DILocation *LocB) {
if (!LocA || !LocB)
Expand Down
10 changes: 5 additions & 5 deletions llvm/lib/Transforms/Scalar/LICM.cpp
Expand Up @@ -2065,11 +2065,11 @@ bool llvm::promoteLoopAccessesToScalars(
});
++NumPromoted;

// Grab a debug location for the inserted loads/stores; given that the
// inserted loads/stores have little relation to the original loads/stores,
// this code just arbitrarily picks a location from one, since any debug
// location is better than none.
DebugLoc DL = LoopUses[0]->getDebugLoc();
// Look at all the loop uses, and try to merge their locations.
std::vector<const DILocation *> LoopUsesLocs;
for (auto U : LoopUses)
LoopUsesLocs.push_back(U->getDebugLoc().get());
auto DL = DebugLoc(DILocation::getMergedLocations(LoopUsesLocs));

// We use the SSAUpdater interface to insert phi nodes as required.
SmallVector<PHINode *, 16> NewPHIs;
Expand Down
147 changes: 147 additions & 0 deletions llvm/test/Transforms/LICM/sink-debuginfo-preserve.ll
@@ -0,0 +1,147 @@
; RUN: opt -S -licm < %s | FileCheck %s

; LICM is trying to merge the two `store` in block %14 and %17, but given their
; locations disagree, it sets a line zero location instead instead of picking a
; random one (the DILocation picked the nearest enclosing scope of the two stores).

; Original C testcase.
; volatile int a;
; extern int g;
; int g;
; void f1() {
; while (a) {
; g = 0;
; if (a)
; g = 0;
; }
; }

; CHECK: %.lcssa = phi i32
; CHECK-NEXT: store i32 %.lcssa, i32* getelementptr inbounds ([2 x i32], [2 x i32]* @g_390, i64 0, i64 1), align 4, !dbg [[storeLocation:![0-9]+]]
; CHECK: [[storeLocation]] = !DILocation(line: 0

target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.15.0"

@b = global i32 0, align 4, !dbg !0
@c = global i32 0, align 4, !dbg !10
@g_390 = local_unnamed_addr global [2 x i32] zeroinitializer, align 4, !dbg !12
@a = local_unnamed_addr global i32 0, align 4, !dbg !6

define i32 @main() local_unnamed_addr !dbg !22 {
%1 = load volatile i32, i32* @b, align 4, !dbg !37, !tbaa !40
%2 = icmp sgt i32 %1, -9, !dbg !44
br i1 %2, label %3, label %5, !dbg !45

3: ; preds = %0
br label %9, !dbg !45

4: ; preds = %9
br label %5, !dbg !45

5: ; preds = %4, %0
%6 = load volatile i32, i32* @c, align 4, !dbg !46, !tbaa !40
%7 = icmp slt i32 %6, 6, !dbg !47
br i1 %7, label %8, label %24, !dbg !48

8: ; preds = %5
br label %14, !dbg !48

9: ; preds = %3, %9
%10 = load volatile i32, i32* @b, align 4, !dbg !49, !tbaa !40
%11 = add nsw i32 %10, -1, !dbg !49
store volatile i32 %11, i32* @b, align 4, !dbg !49, !tbaa !40
%12 = load volatile i32, i32* @b, align 4, !dbg !37, !tbaa !40
%13 = icmp sgt i32 %12, -9, !dbg !44
br i1 %13, label %9, label %4, !dbg !45, !llvm.loop !50

14: ; preds = %8, %18
store i32 0, i32* getelementptr inbounds ([2 x i32], [2 x i32]* @g_390, i64 0, i64 1), align 4, !dbg !53, !tbaa !40
%15 = load volatile i32, i32* @b, align 4, !dbg !54, !tbaa !40
%16 = icmp eq i32 %15, 0, !dbg !54
br i1 %16, label %17, label %18, !dbg !55

17: ; preds = %14
store i32 0, i32* getelementptr inbounds ([2 x i32], [2 x i32]* @g_390, i64 0, i64 1), align 4, !dbg !56, !tbaa !40
br label %18

18: ; preds = %14, %17
%19 = load volatile i32, i32* @c, align 4, !dbg !57, !tbaa !40
%20 = add nsw i32 %19, 1, !dbg !57
store volatile i32 %20, i32* @c, align 4, !dbg !57, !tbaa !40
%21 = load volatile i32, i32* @c, align 4, !dbg !46, !tbaa !40
%22 = icmp slt i32 %21, 6, !dbg !47
br i1 %22, label %14, label %23, !dbg !48, !llvm.loop !58

23: ; preds = %18
br label %24, !dbg !48

24: ; preds = %23, %5
ret i32 0, !dbg !60
}

!llvm.dbg.cu = !{!2}
!llvm.module.flags = !{!17, !18, !19, !20}
!llvm.ident = !{!21}

!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
!1 = distinct !DIGlobalVariable(name: "b", scope: !2, file: !3, line: 1, type: !8, isLocal: false, isDefinition: true)
!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project d3588d0814c4cbc7fca677b4d9634f6e1428a331)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, nameTableKind: None, sysroot: "/")
!3 = !DIFile(filename: "a.c", directory: "/Users/davide/work/build/bin")
!4 = !{}
!5 = !{!6, !0, !10, !12}
!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
!7 = distinct !DIGlobalVariable(name: "a", scope: !2, file: !3, line: 1, type: !8, isLocal: false, isDefinition: true)
!8 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !9)
!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
!10 = !DIGlobalVariableExpression(var: !11, expr: !DIExpression())
!11 = distinct !DIGlobalVariable(name: "c", scope: !2, file: !3, line: 1, type: !8, isLocal: false, isDefinition: true)
!12 = !DIGlobalVariableExpression(var: !13, expr: !DIExpression())
!13 = distinct !DIGlobalVariable(name: "g_390", scope: !2, file: !3, line: 2, type: !14, isLocal: false, isDefinition: true)
!14 = !DICompositeType(tag: DW_TAG_array_type, baseType: !9, size: 64, elements: !15)
!15 = !{!16}
!16 = !DISubrange(count: 2)
!17 = !{i32 7, !"Dwarf Version", i32 4}
!18 = !{i32 2, !"Debug Info Version", i32 3}
!19 = !{i32 1, !"wchar_size", i32 4}
!20 = !{i32 7, !"PIC Level", i32 2}
!21 = !{!"clang version 11.0.0 (https://github.com/llvm/llvm-project d3588d0814c4cbc7fca677b4d9634f6e1428a331)"}
!22 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 3, type: !23, scopeLine: 3, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !25)
!23 = !DISubroutineType(types: !24)
!24 = !{!9}
!25 = !{!26}
!26 = !DILocalVariable(name: "l_1546", scope: !27, file: !3, line: 11, type: !32)
!27 = distinct !DILexicalBlock(scope: !28, file: !3, line: 10, column: 10)
!28 = distinct !DILexicalBlock(scope: !29, file: !3, line: 8, column: 9)
!29 = distinct !DILexicalBlock(scope: !30, file: !3, line: 6, column: 23)
!30 = distinct !DILexicalBlock(scope: !31, file: !3, line: 6, column: 3)
!31 = distinct !DILexicalBlock(scope: !22, file: !3, line: 6, column: 3)
!32 = !DICompositeType(tag: DW_TAG_array_type, baseType: !33, size: 288, elements: !34)
!33 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
!34 = !{!35, !36, !35}
!35 = !DISubrange(count: 3)
!36 = !DISubrange(count: 4)
!37 = !DILocation(line: 4, column: 10, scope: !38)
!38 = distinct !DILexicalBlock(scope: !39, file: !3, line: 4, column: 3)
!39 = distinct !DILexicalBlock(scope: !22, file: !3, line: 4, column: 3)
!40 = !{!41, !41, i64 0}
!41 = !{!"int", !42, i64 0}
!42 = !{!"omnipotent char", !43, i64 0}
!43 = !{!"Simple C/C++ TBAA"}
!44 = !DILocation(line: 4, column: 12, scope: !38)
!45 = !DILocation(line: 4, column: 3, scope: !39)
!46 = !DILocation(line: 6, column: 10, scope: !30)
!47 = !DILocation(line: 6, column: 12, scope: !30)
!48 = !DILocation(line: 6, column: 3, scope: !31)
!49 = !DILocation(line: 4, column: 19, scope: !38)
!50 = distinct !{!50, !45, !51, !52}
!51 = !DILocation(line: 5, column: 5, scope: !39)
!52 = !{!"llvm.loop.unroll.disable"}
!53 = !DILocation(line: 7, column: 14, scope: !29)
!54 = !DILocation(line: 8, column: 9, scope: !28)
!55 = !DILocation(line: 8, column: 9, scope: !29)
!56 = !DILocation(line: 12, column: 16, scope: !27)
!57 = !DILocation(line: 6, column: 19, scope: !30)
!58 = distinct !{!58, !48, !59, !52}
!59 = !DILocation(line: 14, column: 3, scope: !31)
!60 = !DILocation(line: 15, column: 1, scope: !22)

0 comments on commit 5f87415

Please sign in to comment.