[DebugInfo] Redundant `.loc` due to Clang issues in codegen of `for` statement

[pogo59]

I'm filing this issue as a follow-up to PR108300. Ordinarily I'd track this as an internal issue within Sony, but as I'm retiring from Sony effective today, I won't have access to my test case or explanation. I do intend to look at this in the next couple of weeks.

Here's my example. It's the simplest form of the for statement; more complicated forms might have additional issues, but let's start small.

int i = 2;
void test_for() {
#line 300
  for (;          i < 10; ++i)
    i += 2;
}

Compile with -g -gno-column-info -S -emit-llvm and we see this IR and location metadata:

for.cond:                                         ; preds = %for.inc, %entry
  %0 = load i32, ptr @i, align 4, !dbg !18
  %cmp = icmp slt i32 %0, 10, !dbg !18
  br i1 %cmp, label %for.body, label %for.end, !dbg !21

!14 = distinct !DISubprogram(....)
!17 = !DILocation(line: 300, scope: !14)
!18 = !DILocation(line: 300, scope: !19)
!19 = distinct !DILexicalBlock(scope: !20, file: !3, line: 300)
!20 = distinct !DILexicalBlock(scope: !14, file: !3, line: 300)
!21 = !DILocation(line: 300, scope: !20)

Observe that the evaluation of the condition uses !18 as its DILocation, while the conditional branch uses !21. These have the same source location, but different scopes. When it comes time to emit the .loc directives for the machine instructions (in llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp), the fact that the branch has a different DILocation causes us to generate a new .loc directive. The fact that the only difference is the scope (which is irrelevant to .loc) is not considered.

Look at the assembly code for the same block:

.Ltmp0:
	.loc	0 300 0 prologue_end            # tc13837-for.cpp:300:0
	jmp	.LBB0_1
.LBB0_1:                                # %for.cond
                                        # =>This Inner Loop Header: Depth=1
.Ltmp1:
	.loc	0 300 0 is_stmt 0               # tc13837-for.cpp:300:0
	cmpl	$10, i(%rip)
.Ltmp2:
	.loc	0 300 0                         # tc13837-for.cpp:300:0
	jge	.LBB0_4

The .loc at .Ltmp1 is emitted because it is at the start of a new basic block. But the .loc at .Ltmp2 is 100% redundant; it's emitted only because of the scope confusion described above.

Arguably we could filter out the redundant .loc directives in LLVM, but I claim the root cause is in Clang codegen; see the PR cited above, which fixed the equivalent problem with if statements simply by correcting how Clang sets up its notion of DebugLoc.

[llvmbot]

@llvm/issue-subscribers-clang-codegen

Author: Paul T Robinson (pogo59)

I'm filing this issue as a follow-up to PR108300. Ordinarily I'd track this as an internal issue within Sony, but as I'm retiring from Sony effective today, I won't have access to my test case or explanation. I do intend to look at this in the next couple of weeks.

Here's my example. It's the simplest form of the for statement; more complicated forms might have additional issues, but let's start small.

int i = 2;
void test_for() {
#line 300
  for (;          i < 10; ++i)
    i += 2;
}

Compile with -g -gno-column-info -S -emit-llvm and we see this IR and location metadata:

for.cond:                                         ; preds = %for.inc, %entry
  %0 = load i32, ptr @<!-- -->i, align 4, !dbg !18
  %cmp = icmp slt i32 %0, 10, !dbg !18
  br i1 %cmp, label %for.body, label %for.end, !dbg !21

!14 = distinct !DISubprogram(....)
!17 = !DILocation(line: 300, scope: !14)
!18 = !DILocation(line: 300, scope: !19)
!19 = distinct !DILexicalBlock(scope: !20, file: !3, line: 300)
!20 = distinct !DILexicalBlock(scope: !14, file: !3, line: 300)
!21 = !DILocation(line: 300, scope: !20)

Observe that the evaluation of the condition uses !18 as its DILocation, while the conditional branch uses !21. These have the same source location, but different scopes. When it comes time to emit the .loc directives for the machine instructions (in llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp), the fact that the branch has a different DILocation causes us to generate a new .loc directive. The fact that the only difference is the scope (which is irrelevant to .loc) is not considered.

Look at the assembly code for the same block:

.Ltmp0:
	.loc	0 300 0 prologue_end            # tc13837-for.cpp:300:0
	jmp	.LBB0_1
.LBB0_1:                                # %for.cond
                                        # =>This Inner Loop Header: Depth=1
.Ltmp1:
	.loc	0 300 0 is_stmt 0               # tc13837-for.cpp:300:0
	cmpl	$10, i(%rip)
.Ltmp2:
	.loc	0 300 0                         # tc13837-for.cpp:300:0
	jge	.LBB0_4

The .loc at .Ltmp1 is emitted because it is at the start of a new basic block. But the .loc at .Ltmp2 is 100% redundant; it's emitted only because of the scope confusion described above.

Arguably we could filter out the redundant .loc directives in LLVM, but I claim the root cause is in Clang codegen; see the PR cited above, which fixed the equivalent problem with if statements simply by correcting how Clang sets up its notion of DebugLoc.

[llvmbot]

@llvm/issue-subscribers-debuginfo

Author: Paul T Robinson (pogo59)

I'm filing this issue as a follow-up to PR108300. Ordinarily I'd track this as an internal issue within Sony, but as I'm retiring from Sony effective today, I won't have access to my test case or explanation. I do intend to look at this in the next couple of weeks.

Here's my example. It's the simplest form of the for statement; more complicated forms might have additional issues, but let's start small.

int i = 2;
void test_for() {
#line 300
  for (;          i < 10; ++i)
    i += 2;
}

Compile with -g -gno-column-info -S -emit-llvm and we see this IR and location metadata:

for.cond:                                         ; preds = %for.inc, %entry
  %0 = load i32, ptr @<!-- -->i, align 4, !dbg !18
  %cmp = icmp slt i32 %0, 10, !dbg !18
  br i1 %cmp, label %for.body, label %for.end, !dbg !21

!14 = distinct !DISubprogram(....)
!17 = !DILocation(line: 300, scope: !14)
!18 = !DILocation(line: 300, scope: !19)
!19 = distinct !DILexicalBlock(scope: !20, file: !3, line: 300)
!20 = distinct !DILexicalBlock(scope: !14, file: !3, line: 300)
!21 = !DILocation(line: 300, scope: !20)

Observe that the evaluation of the condition uses !18 as its DILocation, while the conditional branch uses !21. These have the same source location, but different scopes. When it comes time to emit the .loc directives for the machine instructions (in llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp), the fact that the branch has a different DILocation causes us to generate a new .loc directive. The fact that the only difference is the scope (which is irrelevant to .loc) is not considered.

Look at the assembly code for the same block:

.Ltmp0:
	.loc	0 300 0 prologue_end            # tc13837-for.cpp:300:0
	jmp	.LBB0_1
.LBB0_1:                                # %for.cond
                                        # =>This Inner Loop Header: Depth=1
.Ltmp1:
	.loc	0 300 0 is_stmt 0               # tc13837-for.cpp:300:0
	cmpl	$10, i(%rip)
.Ltmp2:
	.loc	0 300 0                         # tc13837-for.cpp:300:0
	jge	.LBB0_4

The .loc at .Ltmp1 is emitted because it is at the start of a new basic block. But the .loc at .Ltmp2 is 100% redundant; it's emitted only because of the scope confusion described above.

Arguably we could filter out the redundant .loc directives in LLVM, but I claim the root cause is in Clang codegen; see the PR cited above, which fixed the equivalent problem with if statements simply by correcting how Clang sets up its notion of DebugLoc.

[pogo59]

Note that fixing the if issue resulted in a noticeable reduction in object size, as reported here. I'd expect the for statement fix to cause some reduction as well.

I did not find any similar issues with the while statement. I think that if/for/while are the only places that implicitly introduce lexical scopes, although C++ has become so huge it's hard to be certain.

[pogo59]

Weirdly, the !dbg metadata is different depending on whether we're generating DWARF or Codeview. Hopefully that will help narrow down where the difference is occurring. It actually looks right (or at least better) in Codeview mode.