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 ↗

(On Diff #86997)

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 ↗

(On Diff #86997)

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 ↗

(On Diff #86997)

Oh, good point.

llvm/trunk/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;
 }

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

+; RUN: opt -instcombine -S < %s | FileCheck %s
+;
+; Generated with:
+;
+; clang -S -gmlt -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
+; This final check is the "real" test, verify no !dbg on the store.
+; CHECK-NEXT:  store i32 %storemerge{{.*}}, align 4{{$}}
+;
+; ModuleID = 'test1.ll'
+source_filename = "test.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline nounwind uwtable
+define i32 @foo(i32 %a, i32* %d) !dbg !6 {
+entry:
+  %tobool = icmp ne i32 %a, 0, !dbg !8
+  br i1 %tobool, label %if.then, label %if.else, !dbg !8
+
+if.then:                                          ; preds = %entry
+  %call = call i32 @bar(i32 %a), !dbg !9
+  store i32 %call, i32* %d, align 4, !dbg !10
+  br label %if.end, !dbg !11
+
+if.else:                                          ; preds = %entry
+  %call1 = call i32 @bar2(i32 %a), !dbg !12
+  store i32 %call1, i32* %d, align 4, !dbg !13
+  br label %if.end
+
+if.end:                                           ; preds = %if.else, %if.then
+  ret i32 %a, !dbg !14
+}
+
+declare i32 @bar(i32)
+
+declare i32 @bar2(i32)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "", isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
+!1 = !DIFile(filename: "test.c", directory: "/home/probinson/projects/scratch")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!6 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 4, type: !7, isLocal: false, isDefinition: true, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
+!7 = !DISubroutineType(types: !2)
+!8 = !DILocation(line: 5, column: 6, scope: !6)
+!9 = !DILocation(line: 6, column: 12, scope: !6)
+!10 = !DILocation(line: 6, column: 10, scope: !6)
+!11 = !DILocation(line: 7, column: 3, scope: !6)
+!12 = !DILocation(line: 8, column: 12, scope: !6)
+!13 = !DILocation(line: 8, column: 10, scope: !6)
+!14 = !DILocation(line: 10, column: 3, scope: !6)

llvm/trunk/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.