[llvm] r299972 - [AddDiscriminators] Assign discriminators to MemIntrinsic calls.
Andrea Di Biagio via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 11 12:07:30 PDT 2017
Author: adibiagio
Date: Tue Apr 11 14:07:30 2017
New Revision: 299972
URL: http://llvm.org/viewvc/llvm-project?rev=299972&view=rev
Log:
[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
Added:
llvm/trunk/test/Transforms/AddDiscriminators/memcpy-discriminator.ll
Modified:
llvm/trunk/lib/Transforms/Utils/AddDiscriminators.cpp
Modified: llvm/trunk/lib/Transforms/Utils/AddDiscriminators.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/AddDiscriminators.cpp?rev=299972&r1=299971&r2=299972&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/AddDiscriminators.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/AddDiscriminators.cpp Tue Apr 11 14:07:30 2017
@@ -102,6 +102,10 @@ FunctionPass *llvm::createAddDiscriminat
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 &
// 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 &
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;
Added: llvm/trunk/test/Transforms/AddDiscriminators/memcpy-discriminator.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/AddDiscriminators/memcpy-discriminator.ll?rev=299972&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/AddDiscriminators/memcpy-discriminator.ll (added)
+++ llvm/trunk/test/Transforms/AddDiscriminators/memcpy-discriminator.ll Tue Apr 11 14:07:30 2017
@@ -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 }
+
+ at 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)
More information about the llvm-commits
mailing list