-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[DWARF] Emit a worst-case prologue_end flag for pathological inputs #107849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,10 @@ | ||
; RUN: llc -filetype=asm -mtriple=x86_64-apple-macosx12.0.0 -O0 %s -o - | FileCheck %s | ||
; RUN: llc -filetype=asm -mtriple=x86_64-apple-macosx12.0.0 -O0 %s -o - | FileCheck %s --implicit-check-not=prologue_end | ||
; CHECK: Lfunc_begin0: | ||
; CHECK-NEXT: .file{{.+}} | ||
; CHECK-NEXT: .loc 1 1 0 ## test-small.c:1:0{{$}} | ||
; CHECK-NEXT: .cfi_startproc | ||
; CHECK-NEXT: ## %bb.{{[0-9]+}}: | ||
; CHECK-NEXT: .loc 1 0 1 prologue_end{{.*}} | ||
; CHECK-NEXT: .loc 1 0 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (thinking out loud) cc @rastogishubham as the author of this test |
||
define void @test() #0 !dbg !9 { | ||
ret void, !dbg !12 | ||
} | ||
|
@@ -19,4 +19,4 @@ define void @test() #0 !dbg !9 { | |
!9 = distinct !DISubprogram(name: "test", scope: !10, file: !10, line: 1, type: !11, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !5, retainedNodes: !7) | ||
!10 = !DIFile(filename: "test-small.c", directory: "/Users/shubham/Development/test") | ||
!11 = !DISubroutineType(types: !7) | ||
!12 = !DILocation(line: 0, column: 1, scope: !9) | ||
!12 = !DILocation(line: 0, column: 1, scope: !9) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
# RUN: llc %s -start-before=livedebugvalues -o - | \ | ||
# RUN: FileCheck %s --implicit-check-not=prologue_end | ||
# | ||
## When picking a "backup" location of the first non-trivial instruction in | ||
## a function, don't select a location outside of the entry block. We have to | ||
## give it the function's scope-line, and installing that outside of the entry | ||
## block is liable to be misleading. | ||
## | ||
## Produced from the C below with "clang -O2 -g -mllvm | ||
## -stop-before=livedebugvalues", then modified to unrotate and shift early | ||
## insts into the loop block. This means the MIR is meaningless, we only test | ||
## whether the scope-line will leak into the loop block or not. | ||
## | ||
## int glob = 0; | ||
## int foo(int arg, int sum) { | ||
## arg += sum; | ||
## while (arg) { | ||
## glob--; | ||
## arg %= glob; | ||
## } | ||
## return 0; | ||
## } | ||
# | ||
# CHECK-LABEL: foo: | ||
# CHECK: .loc 0 2 0 | ||
# CHECK: # %bb.0: | ||
# CHECK-NEXT: movl %edi, %edx | ||
# CHECK-NEXT: .loc 0 0 0 is_stmt 0 | ||
# CHECK-NEXT: .Ltmp0: | ||
# CHECK-NEXT: .p2align 4, 0x90 | ||
# CHECK-NEXT: .LBB0_1: | ||
# CHECK-LABEL: addl %esi, %edx | ||
|
||
|
||
--- | | ||
; ModuleID = 'out2.ll' | ||
source_filename = "foo.c" | ||
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128" | ||
target triple = "x86_64-unknown-linux-gnu" | ||
|
||
@glob = dso_local local_unnamed_addr global i32 0, align 4, !dbg !0 | ||
|
||
define dso_local noundef i32 @foo(i32 noundef %arg, i32 noundef %sum) local_unnamed_addr !dbg !9 { | ||
entry: | ||
%add = add nsw i32 %sum, %arg | ||
br label %while.body.preheader | ||
|
||
while.body.preheader: ; preds = %entry | ||
%glob.promoted = load i32, ptr @glob, align 4 | ||
br label %while.body, !dbg !13 | ||
|
||
while.body: ; preds = %while.body, %while.body.preheader | ||
%arg.addr.06 = phi i32 [ %rem, %while.body ], [ %add, %while.body.preheader ] | ||
%dec35 = phi i32 [ %dec, %while.body ], [ %glob.promoted, %while.body.preheader ] | ||
%dec = add nsw i32 %dec35, -1, !dbg !14 | ||
%0 = add i32 %dec35, -1, !dbg !16 | ||
%rem = srem i32 %arg.addr.06, %0, !dbg !16 | ||
%tobool.not = icmp eq i32 %rem, 0, !dbg !13 | ||
br i1 %tobool.not, label %while.cond.while.end_crit_edge, label %while.body, !dbg !13 | ||
|
||
while.cond.while.end_crit_edge: ; preds = %while.body | ||
store i32 %dec, ptr @glob, align 4, !dbg !14 | ||
br label %while.end, !dbg !13 | ||
|
||
while.end: ; preds = %while.cond.while.end_crit_edge | ||
ret i32 0, !dbg !17 | ||
} | ||
|
||
!llvm.dbg.cu = !{!2} | ||
!llvm.module.flags = !{!6, !7} | ||
!llvm.ident = !{!8} | ||
|
||
!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression()) | ||
!1 = distinct !DIGlobalVariable(name: "glob", scope: !2, file: !3, line: 1, type: !5, isLocal: false, isDefinition: true) | ||
!2 = distinct !DICompileUnit(language: DW_LANG_C11, file: !3, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false, nameTableKind: None) | ||
!3 = !DIFile(filename: "foo.c", directory: "") | ||
!4 = !{!0} | ||
!5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) | ||
!6 = !{i32 7, !"Dwarf Version", i32 5} | ||
!7 = !{i32 2, !"Debug Info Version", i32 3} | ||
!8 = !{!"clang"} | ||
!9 = distinct !DISubprogram(name: "foo", scope: !3, file: !3, line: 2, type: !10, scopeLine: 2, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !12) | ||
!10 = !DISubroutineType(types: !11) | ||
!11 = !{!5, !5, !5} | ||
!12 = !{} | ||
!13 = !DILocation(line: 4, column: 3, scope: !9) | ||
!14 = !DILocation(line: 5, column: 9, scope: !15) | ||
!15 = distinct !DILexicalBlock(scope: !9, file: !3, line: 4, column: 15) | ||
!16 = !DILocation(line: 6, column: 9, scope: !15) | ||
!17 = !DILocation(line: 8, column: 3, scope: !9) | ||
|
||
... | ||
--- | ||
name: foo | ||
alignment: 16 | ||
tracksRegLiveness: true | ||
debugInstrRef: true | ||
tracksDebugUserValues: true | ||
liveins: | ||
- { reg: '$edi' } | ||
- { reg: '$esi' } | ||
frameInfo: | ||
maxAlignment: 1 | ||
maxCallFrameSize: 0 | ||
isCalleeSavedInfoValid: true | ||
machineFunctionInfo: | ||
amxProgModel: None | ||
body: | | ||
bb.0.entry: | ||
liveins: $edi, $esi | ||
|
||
$edx = MOV32rr $edi | ||
|
||
bb.1.while.body (align 16): | ||
successors: %bb.2(0x04000000), %bb.1(0x7c000000) | ||
liveins: $ecx, $edx, $esi | ||
|
||
renamable $edx = nsw ADD32rr killed renamable $edx, renamable $esi, implicit-def dead $eflags | ||
renamable $ecx = MOV32rm $rip, 1, $noreg, @glob, $noreg :: (dereferenceable load (s32) from @glob) | ||
renamable $ecx = DEC32r killed renamable $ecx, implicit-def dead $eflags | ||
$eax = MOV32rr killed $edx | ||
CDQ implicit-def $eax, implicit-def $edx, implicit $eax | ||
IDIV32r renamable $ecx, implicit-def dead $eax, implicit-def $edx, implicit-def dead $eflags, implicit $eax, implicit $edx | ||
TEST32rr renamable $edx, renamable $edx, implicit-def $eflags | ||
JCC_1 %bb.1, 5, implicit killed $eflags | ||
|
||
bb.2.while.cond.while.end_crit_edge: | ||
liveins: $ecx, $esi | ||
|
||
MOV32mr $rip, 1, $noreg, @glob, $noreg, killed renamable $ecx, debug-location !14 :: (store (s32) into @glob) | ||
$eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, debug-location !17 | ||
RET64 $eax, debug-location !17 | ||
|
||
... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel slightly confused reading the comment and the current
if
expression. I read the comment as saying "... Thus, only early-return with this location if it's a valid location", but!DL
is not a location we can use?Should this be
if (DL && DL->getLine() != 0)
instead ofif (!DL || DL->getLine() != 0)
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's not clear that the real purpose is simply filtering out line-zero-locations -- those with an empty DebugLoc get given the scope line due to an earlier patch. (It's unfortunate that we've got "empty" nullptr DebugLocs, but also "empty" source locations in the form of line-zero, but here we are).
Without this awkward logic we would end up with zero-length linetable entries for the scope line, before then having a prologue_end linetable entry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, yep I see that now, thanks. This all feels a bit tangled up in an unfortunate way, but I've not got any immediate ideas (possibly there's some way of refactoring this, but maybe not), so LGTM.