[PATCH] D25742: Remove debug location from common tail when tail-merging

[Robert Lougher via llvm-commits]

rob.lougher created this revision.
rob.lougher added reviewers: dblaikie, danielcdh, probinson, wolfgangp, aprantl.
rob.lougher added subscribers: llvm-commits, andreadb, gbedwell.

Consider the following simple if-then-else:

  define i32 @test(i32 %a, i32 %b) !dbg !6 {
  entry:
    %tobool = icmp ne i32 %b, 0, !dbg !8
    br i1 %tobool, label %if.then, label %if.else, !dbg !8
  
  if.then:                                          ; preds = %entry
    %call = call i32 @foo(i32 %a), !dbg !9
    %add = add nsw i32 %a, %call, !dbg !10
    br label %if.end, !dbg !11
  
  if.else:                                          ; preds = %entry
    %call1 = call i32 @bar(i32 %a), !dbg !12
    %add2 = add nsw i32 %a, %call1, !dbg !13
    br label %if.end
  
  if.end:                                           ; preds = %if.else, %if.then
    %a.addr.0 = phi i32 [ %add, %if.then ], [ %add2, %if.else ]
    ret i32 %a.addr.0, !dbg !14
  }

With debug line information as follows:

  !8 = !DILocation(line: 5, column: 6, scope: !6)
  !9 = !DILocation(line: 6, column: 10, scope: !6)
  !10 = !DILocation(line: 6, column: 7, scope: !6)
  !11 = !DILocation(line: 6, column: 5, scope: !6)
  !12 = !DILocation(line: 8, column: 10, scope: !6)
  !13 = !DILocation(line: 8, column: 7, scope: !6)
  !14 = !DILocation(line: 9, column: 3, scope: !6)

If this is passed to llc the branch folding pass will merge the two add instructions into a common tail (.LBB0_3):

  test:                                   # @test
          .loc    1 4 0                   # test.c:4:0
          pushq   %rbx
          movl    %edi, %ebx
          .loc    1 5 6 prologue_end      # test.c:5:6
          testl   %esi, %esi
          je      .LBB0_2
          .loc    1 6 10                  # test.c:6:10
          callq   foo
          jmp     .LBB0_3
  .LBB0_2:                                # %if.else
          .loc    1 8 10                  # test.c:8:10
          callq   bar
  .LBB0_3:                                # %if.end
          addl    %ebx, %eax
          .loc    1 9 3                   # test.c:9:3
          popq    %rbx
          retq

However, the common tail retains the debug information from its original location (randomly taken from one of the merge inputs).  In this case the else-part has been taken and the add appears to occur at line 8.  This is a problem for sample-based PGO as the else-part will now appear to have been executed irrespective of which side of the if-then-else was actually taken.  It will also affect the optimized debugging experience leading to odd stepping in the debugger.

This issue was first seen with the 3.9 release compiler.  The issue still exists on trunk, however it is masked by the recent changes to simplify CFG (SinkThenElseCodeToEnd).  If the above IR is passed to clang, the add will already have been sunk by the time it gets to the branch folding pass.  However it should be fixed as tail-merging will handle cases not handled by simplify CFG.

This patch removes the debug location from the common tail.  As this does not cause a new location to be emitted the testcase forces line 0 by using the //-use-unknown-locations// flag to use line 0 for all instructions with no debug locations.  The pending line 0 patch (review https://reviews.llvm.org/D24180), which turns "no debug info" at the start of a basic-block into line 0 will also emit line 0 for the common-tail.

One test required fixing after the change (DebugInfo/COFF/local-variables.ll).  This test has an if-then-else with a common-tail. After the change the common-tail no longer has the original debug location which decreases the size of the else (the end of the else is now the same as the end of the second inline site, i.e. the original labels //inline_site2_end// and //else_end// are the same and have been merged in the test).  This also changes the size of various ranges.

OK to commit?

Thanks,
Rob.


https://reviews.llvm.org/D25742

Files:
  lib/CodeGen/BranchFolding.cpp
  test/DebugInfo/COFF/local-variables.ll
  test/DebugInfo/X86/tail-merge.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D25742.75010.patch
Type: text/x-patch
Size: 5509 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161018/58c3bd93/attachment.bin>