Skip to content

Add debug location to strlen in LoopIdiomRecognize pass #140164

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
May 16, 2025
Merged

Add debug location to strlen in LoopIdiomRecognize pass #140164

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
35fb19b
Add debug location to strlen in LoopIdiomRecognize pass
amykhuang May 14, 2025
276b552
Set debug loc on the IRBuilder instead of instruction
amykhuang May 16, 2025
d2e863e
Edit test case debug info
amykhuang May 16, 2025
6a672ac
Change debug location matching to a variable
amykhuang May 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1778,6 +1778,9 @@ bool LoopIdiomRecognize::recognizeAndInsertStrLen() {
}
assert(StrLenFunc && "Failed to emit strlen function.");

// Set debug location to the start of the loop.
cast<Instruction>(StrLenFunc)->setDebugLoc(CurLoop->getStartLoc());
Copy link
Collaborator

@dwblaikie dwblaikie May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the static type of StrLenFunc be updated to Instruction* so the cast isn't needed here?

Or, alternatively, perhaps it'd be better to set the debug location on the irbuilder, which will then be used for the instructions?

Copy link
Collaborator Author

@amykhuang amykhuang May 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to set the debug location on the IRbuilder.


const SCEV *StrlenEv = SE->getSCEV(StrLenFunc);
SmallVector<PHINode *, 4> Cleanup;
for (PHINode &PN : LoopExitBB->phis()) {
Expand Down
48 changes: 48 additions & 0 deletions llvm/test/Transforms/LoopIdiom/strlen.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,3 +612,51 @@ while.end:
ret i64 %sub.ptr.sub
}

define i64 @valid_basic_strlen_with_dbg(ptr %str) {
; CHECK-LABEL: define i64 @valid_basic_strlen_with_dbg(
; CHECK-SAME: ptr [[STR:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*]]:
; CHECK-NEXT: [[STRLEN:%.*]] = call i64 @strlen(ptr [[STR]]), !dbg !{{[0-9]+}}
Copy link
Collaborator

@dwblaikie dwblaikie May 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be good to test that the desired debug location was applied to the resulting strlen - might be this needs to be a test without autogenerated checks to do that.

(& what're all the other instructions here - are they instructions part of the transformation, but not part of the resulting strlen? Do any of those end up with the same location as the strlen and are they correct?)

Copy link
Collaborator Author

@amykhuang amykhuang May 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, yeah, I put in the general match because opt was reordering the debug info metadata, but I just reordered it to the "correct" order.

Not sure about the other instructions; they look unchanged so I assume the debug info would remain unchangd as well. Looks like in this particular test case only the compare instruction got replaced with a strlen call. will add the code author too, just realized the change is fairly recent.

; CHECK-NEXT: [[SCEVGEP:%.*]] = getelementptr i8, ptr [[STR]], i64 [[STRLEN]]
; CHECK-NEXT: br label %[[WHILE_COND:.*]]
; CHECK: [[WHILE_COND]]:
; CHECK-NEXT: [[STR_ADDR_0:%.*]] = phi ptr [ [[STR]], %[[ENTRY]] ], [ [[INCDEC_PTR:%.*]], %[[WHILE_COND]] ]
; CHECK-NEXT: [[TMP0:%.*]] = load i8, ptr [[STR_ADDR_0]], align 1
; CHECK-NEXT: [[CMP_NOT:%.*]] = icmp eq i8 [[TMP0]], 0
; CHECK-NEXT: [[INCDEC_PTR]] = getelementptr i8, ptr [[STR_ADDR_0]], i64 1
; CHECK-NEXT: br i1 true, label %[[WHILE_END:.*]], label %[[WHILE_COND]]
; CHECK: [[WHILE_END]]:
; CHECK-NEXT: [[SUB_PTR_LHS_CAST:%.*]] = ptrtoint ptr [[SCEVGEP]] to i64
; CHECK-NEXT: [[SUB_PTR_RHS_CAST:%.*]] = ptrtoint ptr [[STR]] to i64
; CHECK-NEXT: [[SUB_PTR_SUB:%.*]] = sub i64 [[SUB_PTR_LHS_CAST]], [[SUB_PTR_RHS_CAST]]
; CHECK-NEXT: ret i64 [[SUB_PTR_SUB]]
;
entry:
br label %while.cond

while.cond:
%str.addr.0 = phi ptr [ %str, %entry ], [ %incdec.ptr, %while.cond ]
%0 = load i8, ptr %str.addr.0, align 1, !dbg !6
%cmp.not = icmp eq i8 %0, 0, !dbg !6
%incdec.ptr = getelementptr i8, ptr %str.addr.0, i64 1
br i1 %cmp.not, label %while.end, label %while.cond, !dbg !6

while.end:
%sub.ptr.lhs.cast = ptrtoint ptr %str.addr.0 to i64
%sub.ptr.rhs.cast = ptrtoint ptr %str to i64
%sub.ptr.sub = sub i64 %sub.ptr.lhs.cast, %sub.ptr.rhs.cast
ret i64 %sub.ptr.sub
}


!llvm.module.flags = !{!7}
!llvm.dbg.cu = !{!2}

!0 = distinct !DISubprogram(name: "foo", line: 2, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !2, file: !1, scope: !1, type: !3)
!1 = !DIFile(filename: "strlen.c", directory: "/tmp")
!2 = distinct !DICompileUnit(language: DW_LANG_C99, producer: "clang version 2.9 (trunk 127165:127174)", isOptimized: true, emissionKind: FullDebug, file: !1, enums: !4, retainedTypes: !4)
!3 = !DISubroutineType(types: !4)
!4 = !{}
!5 = distinct !DILexicalBlock(line: 2, column: 21, file: !1, scope: !0)
!6 = !DILocation(line: 3, column: 3, scope: !5)
!7 = !{i32 1, !"Debug Info Version", i32 3}
Loading