[PATCH] D27462: For functions with debug info, do not propagate the callsite debug location to inlined instructions.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb created this revision.
andreadb added reviewers: dblaikie, probinson, wolfgangp, aprantl.
andreadb added a subscriber: llvm-commits.

When a function F is inlined, `InlineFunction` extends the debug location of every instruction inlined from F by adding an `InlinedAt`.

However, if an instruction has a 'null' debug location, InlineFunction would propagate the callsite debug location to it. This behavior existed since revision 210459
(http://llvm.org/viewvc/llvm-project?view=revision&revision=210459).

Revision 210459 was originally committed specifically to workaround the lack of debug information for
instructions inlined from intrinsic functions (which are usually declared with attributes `__always_inline__, __nodebug__`).

The problem with revision 210459 is that it doesn't make any sort of distinction between instructions inlined from a 'nodebug' function and instructions which are inlined from a function built with debug info. When performing PGO on some game code, I noticed that r210459 was the main reason why some instructions in the sample had incorrect counts. As a consequence, a basic block was incorrectly seen as hot, and this was leading to a poor machine block placement.
Note that this issue may also lead to incorrect stepping in the debugger.

My fix works under the assumption that a __nodebug__ function does not have a DISubprogram. When a function F is inlined into another function G, InlineFunction checks if it has debug info associated with it.

For `nodebug` functions, the InlineFunction logic is unchanged (i.e. it would still propagate the callsite debugloc to the inlined instructions). Otherwise, InlineFunction no longer propagates the callsite debug location.

Please let me know if okay to commit.

Thanks,
Andrea


https://reviews.llvm.org/D27462

Files:
  lib/Transforms/Utils/InlineFunction.cpp
  test/DebugInfo/Generic/inline-debug-loc.ll


Index: test/DebugInfo/Generic/inline-debug-loc.ll
===================================================================
--- test/DebugInfo/Generic/inline-debug-loc.ll
+++ test/DebugInfo/Generic/inline-debug-loc.ll
@@ -0,0 +1,47 @@
+; RUN: opt -inline -S < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+; Function @bar contains instruction %cmp which is not associated to any debug
+; location. This test verifies that the inliner doesn't incorrectly attribute
+; the callsite debug location to %cmp.
+
+define i32 @bar(i32 %a, i32 %b) #0 !dbg !6 {
+entry:
+  %inc = add i32 %a, 1, !dbg !8
+  %cmp = icmp slt i32 %inc, %b
+  %select = select i1 %cmp, i32 %a, i32 %b, !dbg !8
+  ret i32 %select, !dbg !8
+}
+
+
+; CHECK-LABEL: define i32 @baz(
+; CHECK: entry:
+; CHECK:   %[[INC:[a-z0-9.]+]] = add i32 %a, 1, !dbg ![[DL:[0-9]+]]
+; CHECK:   %[[CMP:[a-z0-9.]+]] = icmp slt i32 %[[INC]], %b
+; CHECK-NOT: !dbg
+; CHECK:   %[[SELECT:[a-z0-9.]+]] = select i1 %[[CMP]], i32 %a, i32 %b, !dbg ![[DL]]
+;
+; ![[DL]] = !DILocation(line: 3, scope: !{{.*}}, inlinedAt: {{.*}})
+
+define i32 @baz(i32 %a, i32 %b) !dbg !9 {
+entry:
+  %call = tail call i32 @bar(i32 %a, i32 %b), !dbg !10
+  ret i32 %call, !dbg !10
+}
+
+attributes #0 = { alwaysinline }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, isOptimized: true, 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}
+!6 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 2, type: !7, isLocal: false, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !2)
+!7 = !DISubroutineType(types: !2)
+!8 = !DILocation(line: 3, scope: !6)
+!9 = distinct !DISubprogram(name: "baz", scope: !1, file: !1, line: 11, type: !7, isLocal: false, isDefinition: true, scopeLine: 11, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !2)
+!10 = !DILocation(line: 12, scope: !9)
Index: lib/Transforms/Utils/InlineFunction.cpp
===================================================================
--- lib/Transforms/Utils/InlineFunction.cpp
+++ lib/Transforms/Utils/InlineFunction.cpp
@@ -1350,7 +1350,7 @@
 /// Update inlined instructions' line numbers to
 /// to encode location where these instructions are inlined.
 static void fixupLineNumbers(Function *Fn, Function::iterator FI,
-                             Instruction *TheCall) {
+                             Instruction *TheCall, bool CalleeHasDebugInfo) {
   const DebugLoc &TheCallDL = TheCall->getDebugLoc();
   if (!TheCallDL)
     return;
@@ -1374,6 +1374,8 @@
          BI != BE; ++BI) {
       DebugLoc DL = BI->getDebugLoc();
       if (!DL) {
+        if (CalleeHasDebugInfo)
+          continue;
         // If the inlined instruction has no line number, make it look as if it
         // originates from the call location. This is important for
         // ((__always_inline__, __nodebug__)) functions which must use caller
@@ -1643,8 +1645,11 @@
     if (IFI.CG)
       UpdateCallGraphAfterInlining(CS, FirstNewBlock, VMap, IFI);
 
-    // Update inlined instructions' line number information.
-    fixupLineNumbers(Caller, FirstNewBlock, TheCall);
+    // For 'nodebug' functions, the associated DISubprogram is always null.
+    // Conservatively avoid propagating the callsite debug location to
+    // instructions inlined from a function whose DISubprogram is not null.
+    fixupLineNumbers(Caller, FirstNewBlock, TheCall,
+                     CalledFunc->getSubprogram() != nullptr);
 
     // Clone existing noalias metadata if necessary.
     CloneAliasScopeMetadata(CS, VMap);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27462.80416.patch
Type: text/x-patch
Size: 3817 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161206/467f4073/attachment.bin>