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
retqHowever, 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 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.
You could avoid the extra loop by putting this code into mergeOperations() (line 770 already has a check for isDebugValue()).