[PATCH] D24632: Preserve the debug location when sinking compare instructions

Pieb, Wolfgang via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 15 16:01:52 PDT 2016


The original test case was the following:

/////
int foo(unsigned x, unsigned y, int *z, int *h) {
  int Result = 0;
  for (unsigned i = 0; i < x; i++) {    // line 3
    int Val = 0;
    for (unsigned j = 0; j < y; ++j) {  // line 5
      if (Val == z[j]) {                // line 6
        Val += 42;                      // line 7
        break;
      }
    }
    Result += Val;                      // line 9
  }

  return Result;
}
/////

Quoting Andrea’s investigation:


At –O2, the inner loop at line 5 is rotated. The comparison `j < y` at iteration 0 of the inner loop becomes a comparison between 'y' and zero.

Since 'y' is a loop invariant (for both loops), the compiler moves the icmp between 'y' and zero in

the outer loop preheader.


Later on, CodeGenPrepare decides to sink the icmp within the basic block that uses it. So, we end

up rematerializing the computation in its original place (after loop rotation).

However, this time, the rematerialized instruction doesn't seem to be associated with any debug info.



Compiling the test case with –O2 –g –s you see assembly code:



.LBB0_3:                                # %for.cond1.preheader.prol

        movl    $1, %r8d

        xorl    %eax, %eax

        testl   %esi, %esi

        je      .LBB0_7





Instead of:



.LBB0_3:                                # %for.cond1.preheader.prol

        movl    $1, %r8d

        xorl    %eax, %eax

.Ltmp4:

        .loc    1 5 28 is_stmt 1 discriminator 1 # test.cpp:5:28

        testl   %esi, %esi

        je      .LBB0_7







-- wolfgang


From: Dehao Chen [mailto:danielcdh at gmail.com]
Sent: Thursday, September 15, 2016 3:32 PM
To: David Blaikie
Cc: reviews+D24632+public+5e33fe097ec6bcd7 at reviews.llvm.org; Pieb, Wolfgang; aprantl at apple.com; andrea.dibiagio at gmail.com; llvm-commits; Junbum Lim
Subject: Re: [PATCH] D24632: Preserve the debug location when sinking compare instructions

For AutoFDO, we want to remove(or set 0-line) debug info for the instructions that could be executed speculatively (i.e. instruction gets more count that it should). For this patch, it's more like code cloning without speculation. Basically the cloned code will split the profile counts among different copies, so its effect will be: make the profiled count smaller than it should have. When we post-process samples, we intend to use the max_count we get from all cloned instructions (well, technically not exactly "max", there is some voting to prevent from speculated case). As a result, we don't want to lose debug info for clones (as soon as it's not speculated clones) because it gives us better chance to capture the "max".

The same applies to debugging. We don't want to see gdb go to speculated source locations as we don't expect it getting executed. But for clones, as soon as it's actually logically executed, we want to have the source location reserved.

I'm wondering what's the source code that has been affected by this.

Dehao

On Thu, Sep 15, 2016 at 2:56 PM, David Blaikie <dblaikie at gmail.com<mailto:dblaikie at gmail.com>> wrote:
This seems like the opposite of the other recent change to remove/zero out the debug location of an instruction being commoned into a preceeding basic block.

Adding Dehao.

Wouldn't this hurt profile based optimizations by attributing code passing through other.bb<http://other.bb> to the entry? Making the Entry seem more common than it is?
On Thu, Sep 15, 2016 at 2:52 PM Wolfgang Pieb <wolfgang.pieb at sony.com<mailto:wolfgang.pieb at sony.com>> wrote:
wolfgangp created this revision.
wolfgangp added reviewers: aprantl, dblaikie.
wolfgangp added subscribers: llvm-commits, andreadb.

When the CodeGenPrepare pass sinks a compare instruction into the basic block of a user, it should preserve its debug location. Not doing so negatively affects source line attribution for debugging and AutoFDO.

Patch by Andrea Di Biagio

https://reviews.llvm.org/D24632

Files:
  lib/CodeGen/CodeGenPrepare.cpp
  test/DebugInfo/Generic/sunk-compare.ll

Index: test/DebugInfo/Generic/sunk-compare.ll
===================================================================
--- test/DebugInfo/Generic/sunk-compare.ll
+++ test/DebugInfo/Generic/sunk-compare.ll
@@ -0,0 +1,46 @@
+; RUN: opt -S -codegenprepare < %s | FileCheck %s
+;
+; This test case has been generated by hand but is inspired by the
+; observation that compares that are sunk into the basic blocks where
+; their results are used did not retain their debug locs. This caused
+; sample profiling to attribute code to the wrong source lines.
+;
+; We check that the compare instruction retains its debug loc after
+; it is sunk into other.bb<http://other.bb> by the codegen prepare pass.
+;
+; CHECK:       other.bb<http://other.bb>:
+; CHECK-NEXT:  icmp{{.*}}%x, 0, !dbg ![[MDHANDLE:[0-9]*]]
+; CHECK:       ![[MDHANDLE]] = !DILocation(line: 2
+;
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+define i32 @_Z3fooii(i32 %x, i32 %y) !dbg !5 {
+entry:
+  %cmp17 = icmp sgt i32 %x, 0, !dbg !6
+  br label %other.bb<http://other.bb>, !dbg !6
+
+other.bb<http://other.bb>:
+  br i1 %cmp17, label %exit1.bb<http://exit1.bb>, label %exit2.bb<http://exit2.bb>, !dbg !7
+
+exit1.bb<http://exit1.bb>:
+  %0 = add i32 %y, 42, !dbg !8
+  ret i32 %0, !dbg !8
+
+exit2.bb<http://exit2.bb>:
+  ret i32 44, !dbg !9
+
+}
+
+!llvm.dbg.cu<http://llvm.dbg.cu> = !{!0}
+!llvm.module.flags = !{!3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !2)
+!1 = !DIFile(filename: "test.cpp", directory: "/debuginfo/bug/cgp")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !1, file: !1, line: 1, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
+!6 = !DILocation(line: 2, column: 0, scope: !5)
+!7 = !DILocation(line: 3, column: 0, scope: !5)
+!8 = !DILocation(line: 4, column: 0, scope: !5)
+!9 = !DILocation(line: 5, column: 0, scope: !5)
Index: lib/CodeGen/CodeGenPrepare.cpp
===================================================================
--- lib/CodeGen/CodeGenPrepare.cpp
+++ lib/CodeGen/CodeGenPrepare.cpp
@@ -926,6 +926,8 @@
       InsertedCmp =
           CmpInst::Create(CI->getOpcode(), CI->getPredicate(),
                           CI->getOperand(0), CI->getOperand(1), "", &*InsertPt);
+      // Propagate the debug info.
+      InsertedCmp->setDebugLoc(CI->getDebugLoc());
     }

     // Replace a use of the cmp with a use of the new cmp.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160915/2968d382/attachment-0001.html>


More information about the llvm-commits mailing list