Merge DebugLoc on combined stores

[probinson]

Summary

When combining stores at the ends of two blocks, merge the debug location
instead of arbitrarily picking one.

[andreadb]

test/DebugInfo/Generic/store-tail-merge.ll

5

This test only requires -gline-tables-only. You don't need any @llvm.dbg.value calls and/or extra metadata for debug info types and variables.

69

This entry is not needed. You can get rid of !6. also replace the producer string with an empty string.

[aprantl]

LGTM with outstanding comments addressed.

[probinson]

test/DebugInfo/Generic/store-tail-merge.ll

5

Oh, good point.

lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

 //===----------------------------------------------------------------------===//
 
 #include "InstCombineInternal.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/Loads.h"
 #include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/DataLayout.h"
+#include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Local.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "instcombine"
bool InstCombiner::SimplifyStoreAtEndOfBlock(StoreInst &SI) {
   // insert it.
   BBI = DestBB->getFirstInsertionPt();
   StoreInst *NewSI = new StoreInst(MergedVal, SI.getOperand(1),
                                    SI.isVolatile(),
                                    SI.getAlignment(),
                                    SI.getOrdering(),
                                    SI.getSynchScope());
   InsertNewInstBefore(NewSI, *BBI);
-  NewSI->setDebugLoc(OtherStore->getDebugLoc());
+  // The debug locations of the original instructions might differ; merge them.
+  NewSI->setDebugLoc(DILocation::getMergedLocation(SI.getDebugLoc(),
+                                                   OtherStore->getDebugLoc()));
 
   // If the two stores had AA tags, merge them.
   AAMDNodes AATags;
   SI.getAAMetadata(AATags);
   if (AATags) {
     OtherStore->getAAMetadata(AATags, /* Merge = */ true);
     NewSI->setAAMetadata(AATags);
   }
 
   // Nuke the old stores.
   eraseInstFromFunction(SI);
   eraseInstFromFunction(*OtherStore);
   return true;
 }

test/DebugInfo/Generic/store-tail-merge.ll

+; RUN: opt -instcombine -S < %s | FileCheck %s
+;
+; Generated with:
+;
+; clang -S -g -emit-llvm test.c -o 1.ll
+; opt -sroa -S 1.ll -o test.ll
+;
+; extern int bar(int i);
+; extern int bar2(int i);
+;
+; int foo(int a, int *d) {
+;   if(a) {
+;       *d = bar(a);
+;   } else {
+;       *d = bar2(a);
+;   }
+;
+;   return a;
+; }
+;
+; CHECK:       define {{.*}}foo
+; CHECK:       if.end:
+; CHECK-NEXT:  %storemerge = phi
+; CHECK-NEXT:  store i32 %storemerge{{.*}}, align 4{{$}}
+;
+; ModuleID = 'test.ll'
+source_filename = "test.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+; target triple = "x86_64-scei-ps4"
+
+; Function Attrs: nounwind sspstrong uwtable
+define i32 @foo(i32 %a, i32* %d) local_unnamed_addr #0 !dbg !7 {
+entry:
+  call void @llvm.dbg.value(metadata i32 %a, i64 0, metadata !13, metadata !15), !dbg !16
+  call void @llvm.dbg.value(metadata i32* %d, i64 0, metadata !14, metadata !15), !dbg !16
+  %tobool = icmp ne i32 %a, 0, !dbg !17
+  br i1 %tobool, label %if.then, label %if.else, !dbg !19
+
+if.then:                                          ; preds = %entry
+  %call = call i32 @bar(i32 %a), !dbg !20
+  store i32 %call, i32* %d, align 4, !dbg !20
+  br label %if.end, !dbg !22
+
+if.else:                                          ; preds = %entry
+  %call1 = call i32 @bar2(i32 %a), !dbg !23
+  store i32 %call1, i32* %d, align 4, !dbg !23
+  br label %if.end
+
+if.end:                                           ; preds = %if.else, %if.then
+  ret i32 %a, !dbg !25
+}
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+declare i32 @bar(i32) local_unnamed_addr #2
+
+declare i32 @bar2(i32) local_unnamed_addr #2
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.value(metadata, i64, metadata, metadata) #1
+
+attributes #0 = { nounwind sspstrong uwtable }
+attributes #1 = { nounwind readnone }
+attributes #2 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 4.0.0 (PS4 clang version 99.99.0.840 a5ef1c5c checking)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "test.c", directory: "D:\test")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"PIC Level", i32 2}
+!6 = !{!"clang version 4.0.0 (PS4 clang version 99.99.0.840 a5ef1c5c checking)"}
+!7 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 4, type: !8, isLocal: false, isDefinition: true, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !12)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10, !10, !11}
+!10 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!11 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !10, size: 64, align: 64)
+!12 = !{!13, !14}
+!13 = !DILocalVariable(name: "a", arg: 1, scope: !7, file: !1, line: 4, type: !10)
+!14 = !DILocalVariable(name: "d", arg: 2, scope: !7, file: !1, line: 4, type: !11)
+!15 = !DIExpression()
+!16 = !DILocation(line: 4, scope: !7)
+!17 = !DILocation(line: 5, scope: !18)
+!18 = distinct !DILexicalBlock(scope: !7, file: !1, line: 5)
+!19 = !DILocation(line: 5, scope: !7)
+!20 = !DILocation(line: 6, scope: !21)
+!21 = distinct !DILexicalBlock(scope: !18, file: !1, line: 5)
+!22 = !DILocation(line: 7, scope: !21)
+!23 = !DILocation(line: 8, scope: !24)
+!24 = distinct !DILexicalBlock(scope: !18, file: !1, line: 7)
+!25 = !DILocation(line: 11, scope: !7)

test/Transforms/SampleProfile/calls.ll

entry:
   br label %while.cond, !dbg !13
 
 while.cond:                                       ; preds = %if.end, %entry
   %0 = load i32, i32* %i, align 4, !dbg !14
   %inc = add nsw i32 %0, 1, !dbg !14
   store i32 %inc, i32* %i, align 4, !dbg !14
   %cmp = icmp slt i32 %0, 400000000, !dbg !14
   br i1 %cmp, label %while.body, label %while.end, !dbg !14
-; CHECK: edge while.cond -> while.body probability is 0x7d9eb367 / 0x80000000 = 98.14% [HOT edge]
-; CHECK: edge while.cond -> while.end probability is 0x02614c99 / 0x80000000 = 1.86%
+; CHECK: edge while.cond -> while.body probability is 0x75bcbf1b / 0x80000000 = 91.98% [HOT edge]
+; CHECK: edge while.cond -> while.end probability is 0x0a4340e5 / 0x80000000 = 8.02%
 
 while.body:                                       ; preds = %while.cond
   %1 = load i32, i32* %i, align 4, !dbg !16
   %cmp1 = icmp ne i32 %1, 100, !dbg !16
   br i1 %cmp1, label %if.then, label %if.else, !dbg !16
 ; Without discriminator information, the profiler used to think that
 ; both branches out of while.body had the same weight. In reality,
 ; the edge while.body->if.then is taken most of the time.