[AddDiscriminators] Assign discriminators to MemIntrinsic calls.

Before this patch, pass AddDiscriminators always avoided to assign
discriminators to intrinsic calls. This was done mainly for two reasons:
 1) We wanted to minimize the number of based discriminators used.
 2) We wanted to avoid non-deterministic discriminator assignment for
    different debug levels.

Unfortunately, that approach was problematic for MemIntrinsic calls.
MemIntrinsic calls can be split by SROA into loads and stores, and each new
load/store instruction would obtain the debug location from the original
intrinsic call.
If we don't assign a discriminator to MemIntrinsic calls, then we cannot
correctly set the discriminator for the newly created loads and stores.
This may have a negative impact on the basic block weight computation
performed by the SampleLoader.

This patch fixes the issue by letting MemIntrinsic calls have a discriminator.

Differential Revision: https://reviews.llvm.org/D31900

llvm-svn: 299972

llvm/lib/Transforms/Utils/AddDiscriminators.cpp

@@ -102,6 +102,10 @@ FunctionPass *llvm::createAddDiscriminatorsPass() {
   return new AddDiscriminatorsLegacyPass();
 }
 
+static bool shouldHaveDiscriminator(const Instruction *I) {
+  return !isa<IntrinsicInst>(I) || isa<MemIntrinsic>(I);
+}
+
 /// \brief Assign DWARF discriminators.
 ///
 /// To assign discriminators, we examine the boundaries of every
@@ -176,7 +180,13 @@ static bool addDiscriminators(Function &F) {
   // discriminator for this instruction.
   for (BasicBlock &B : F) {
     for (auto &I : B.getInstList()) {
-      if (isa<IntrinsicInst>(&I))
+      // Not all intrinsic calls should have a discriminator.
+      // We want to avoid a non-deterministic assignment of discriminators at
+      // different debug levels. We still allow discriminators on memory
+      // intrinsic calls because those can be early expanded by SROA into
+      // pairs of loads and stores, and the expanded load/store instructions
+      // should have a valid discriminator.
+      if (!shouldHaveDiscriminator(&I))
         continue;
       const DILocation *DIL = I.getDebugLoc();
       if (!DIL)
@@ -207,6 +217,10 @@ static bool addDiscriminators(Function &F) {
     LocationSet CallLocations;
     for (auto &I : B.getInstList()) {
       CallInst *Current = dyn_cast<CallInst>(&I);
+      // We bypass intrinsic calls for the following two reasons:
+      //  1) We want to avoid a non-deterministic assigment of
+      //     discriminators.
+      //  2) We want to minimize the number of base discriminators used.
       if (!Current || isa<IntrinsicInst>(&I))
         continue;
 

llvm/test/Transforms/AddDiscriminators/memcpy-discriminator.ll

@@ -0,0 +1,104 @@
+; RUN: opt < %s -add-discriminators -sroa -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+; Test case obtained from the following C code:
+
+; struct A {
+;  int field1;
+;  short field2;
+; };
+;
+; struct B {
+;   struct A field1;
+;   int field2;
+; };
+;
+;
+; extern struct B g_b;
+; extern int bar(struct B b, int c);
+;
+; int foo(int cond) {
+;   int result = cond ? bar(g_b, 33) : 42;
+;   return result;
+; }
+
+; In this test, global variable g_b is passed by copy to function bar. That
+; copy is located on the stack (see alloca %g_b.coerce), and it is initialized
+; by a memcpy call.
+;
+; SROA would split alloca %g_b.coerce into two (smaller disjoint) slices:
+; slice [0,8) and slice [8, 12). Users of the original alloca are rewritten
+; as users of the new alloca slices.
+; In particular, the memcpy is rewritten by SROA as two load/store pairs.
+;
+; Later on, mem2reg successfully promotes the new alloca slices to registers,
+; and loads %3 and %5 are made redundant by the loads obtained from the memcpy
+; intrinsic expansion.
+;
+; If pass AddDiscriminators doesn't assign a discriminator to the intrinsic
+; memcpy call, then the loads obtained from the memcpy expansion would not have
+; a correct discriminator.
+;
+; This test checks that the two new loads inserted by SROA in %cond.true
+; correctly reference a debug location with a non-zero discriminator. This test
+; also checks that the same discriminator is used by all instructions from
+; basic block %cond.true.
+
+%struct.B = type { %struct.A, i32 }
+%struct.A = type { i32, i16 }
+
+@g_b = external global %struct.B, align 4
+
+define i32 @foo(i32 %cond) #0 !dbg !5 {
+entry:
+  %g_b.coerce = alloca { i64, i32 }, align 4
+  %tobool = icmp ne i32 %cond, 0, !dbg !7
+  br i1 %tobool, label %cond.true, label %cond.end, !dbg !7
+
+cond.true:
+; CHECK-LABEL: cond.true:
+; CHECK:       load i64, {{.*}}, !dbg ![[LOC:[0-9]+]]
+; CHECK-NEXT:  load i32, {{.*}}, !dbg ![[LOC]]
+; CHECK-NEXT:  %call = call i32 @bar({{.*}}), !dbg ![[LOC]]
+; CHECK-NEXT:  br label %cond.end, !dbg ![[BR_LOC:[0-9]+]]
+
+; CHECK-DAG: ![[LOC]] = !DILocation(line: 16, column: 23, scope: ![[SCOPE:[0-9]+]])
+; CHECK-DAG: ![[SCOPE]] = !DILexicalBlockFile({{.*}}, discriminator: 2)
+; CHECK-DAG: ![[BR_LOC]] = !DILocation(line: 16, column: 16, scope: ![[SCOPE]])
+
+  %0 = bitcast { i64, i32 }* %g_b.coerce to i8*, !dbg !8
+  %1 = bitcast %struct.B* @g_b to i8*, !dbg !8
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %0, i8* %1, i64 12, i32 4, i1 false), !dbg !8
+  %2 = getelementptr inbounds { i64, i32 }, { i64, i32 }* %g_b.coerce, i32 0, i32 0, !dbg !8
+  %3 = load i64, i64* %2, align 4, !dbg !8
+  %4 = getelementptr inbounds { i64, i32 }, { i64, i32 }* %g_b.coerce, i32 0, i32 1, !dbg !8
+  %5 = load i32, i32* %4, align 4, !dbg !8
+  %call = call i32 @bar(i64 %3, i32 %5, i32 33), !dbg !8
+  br label %cond.end, !dbg !7
+
+cond.end:                                         ; preds = %entry, %cond.true
+  %cond1 = phi i32 [ %call, %cond.true ], [ 42, %entry ], !dbg !7
+  ret i32 %cond1, !dbg !9
+}
+
+declare i32 @bar(i64, i32, i32)
+
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i32, i1) #1
+
+attributes #0 = { noinline nounwind uwtable }
+attributes #1 = { argmemonly nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
+!1 = !DIFile(filename: "test.c", directory: ".")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 15, type: !6, isLocal: false, isDefinition: true, scopeLine: 15, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
+!6 = !DISubroutineType(types: !2)
+!7 = !DILocation(line: 16, column: 16, scope: !5)
+!8 = !DILocation(line: 16, column: 23, scope: !5)
+!9 = !DILocation(line: 17, column: 3, scope: !5)