update_test_checks: Relax DIFile filename checks #135692
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-testing-tools Author: Scott Linder (slinder1) ChangesAvoid baking in absolute paths in check lines generated for DIFile metadata. Generated test checks cannot be sensitive to absolute paths anyway, as those vary with the environment, but there could be situations where some sensitivity to partial paths is required for certain tests. This implementation just assumes such tests aren't worth the effort to support, but it could be supported in the future. This is most useful for update_cc_test_checks with debug info enabled, where the test writer cannot manipulate the paths within the generated IR directly. Full diff: https://github.com/llvm/llvm-project/pull/135692.diff 6 Files Affected:
diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/difile_absolute_filenames.ll b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/difile_absolute_filenames.ll
new file mode 100644
index 0000000000000..0dbef0f75f34c
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/difile_absolute_filenames.ll
@@ -0,0 +1,34 @@
+; RUN: opt < %s -S | FileCheck %s
+
+; ModuleID = '/home/slinder1/llvm-project/foo.c'
+source_filename = "/home/slinder1/llvm-project/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"
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local void @f() #0 !dbg !10 {
+entry:
+ ret void, !dbg !14
+}
+
+attributes #0 = { noinline nounwind optnone uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 21.0.0git (git@github.com:AMD-Lightning-Internal/llvm-project ff31c4b867df0c606fba26eb1ec93996fc5f5920)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "/home/slinder1/llvm-project/foo.c", directory: "/home/slinder1/llvm-project/amd-staging", checksumkind: CSK_MD5, checksum: "e6ab1d5b7f82464c963a8522037dfa72")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{i32 7, !"frame-pointer", i32 2}
+!9 = !{!"clang version 21.0.0git (git@github.com:AMD-Lightning-Internal/llvm-project ff31c4b867df0c606fba26eb1ec93996fc5f5920)"}
+!10 = distinct !DISubprogram(name: "f", scope: !11, file: !11, line: 1, type: !12, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !0)
+!11 = !DIFile(filename: "foo.c", directory: "/home/slinder1/llvm-project", checksumkind: CSK_MD5, checksum: "e6ab1d5b7f82464c963a8522037dfa72")
+!12 = !DISubroutineType(types: !13)
+!13 = !{null}
+!14 = !DILocation(line: 1, column: 11, scope: !10)
diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/difile_absolute_filenames.ll.expected b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/difile_absolute_filenames.ll.expected
new file mode 100644
index 0000000000000..78a4b7304eba9
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/difile_absolute_filenames.ll.expected
@@ -0,0 +1,58 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals
+; RUN: opt < %s -S | FileCheck %s
+
+; ModuleID = '/home/slinder1/llvm-project/foo.c'
+source_filename = "/home/slinder1/llvm-project/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"
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local void @f() #0 !dbg !10 {
+; CHECK-LABEL: @f(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: ret void, !dbg [[DBG14:![0-9]+]]
+;
+entry:
+ ret void, !dbg !14
+}
+
+attributes #0 = { noinline nounwind optnone uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 21.0.0git (git@github.com:AMD-Lightning-Internal/llvm-project ff31c4b867df0c606fba26eb1ec93996fc5f5920)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "/home/slinder1/llvm-project/foo.c", directory: "/home/slinder1/llvm-project/amd-staging", checksumkind: CSK_MD5, checksum: "e6ab1d5b7f82464c963a8522037dfa72")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{i32 7, !"frame-pointer", i32 2}
+!9 = !{!"clang version 21.0.0git (git@github.com:AMD-Lightning-Internal/llvm-project ff31c4b867df0c606fba26eb1ec93996fc5f5920)"}
+!10 = distinct !DISubprogram(name: "f", scope: !11, file: !11, line: 1, type: !12, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !0)
+!11 = !DIFile(filename: "foo.c", directory: "/home/slinder1/llvm-project", checksumkind: CSK_MD5, checksum: "e6ab1d5b7f82464c963a8522037dfa72")
+!12 = !DISubroutineType(types: !13)
+!13 = !{null}
+!14 = !DILocation(line: 1, column: 11, scope: !10)
+;.
+; CHECK: attributes #[[ATTR0:[0-9]+]] = { noinline nounwind optnone uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+;.
+; CHECK: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C11, file: [[META1:![0-9]+]], producer: "{{.*}}clang version {{.*}}", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+; CHECK: [[META1]] = !DIFile(filename: "{{.*}}foo.c", directory: {{.*}})
+; CHECK: [[META2:![0-9]+]] = !{i32 7, !"Dwarf Version", i32 5}
+; CHECK: [[META3:![0-9]+]] = !{i32 2, !"Debug Info Version", i32 3}
+; CHECK: [[META4:![0-9]+]] = !{i32 1, !"wchar_size", i32 4}
+; CHECK: [[META5:![0-9]+]] = !{i32 8, !"PIC Level", i32 2}
+; CHECK: [[META6:![0-9]+]] = !{i32 7, !"PIE Level", i32 2}
+; CHECK: [[META7:![0-9]+]] = !{i32 7, !"uwtable", i32 2}
+; CHECK: [[META8:![0-9]+]] = !{i32 7, !"frame-pointer", i32 2}
+; CHECK: [[META9:![0-9]+]] = !{!"{{.*}}clang version {{.*}}"}
+; CHECK: [[META10:![0-9]+]] = distinct !DISubprogram(name: "f", scope: [[META11:![0-9]+]], file: [[META11]], line: 1, type: [[META12:![0-9]+]], scopeLine: 1, spFlags: DISPFlagDefinition, unit: [[META0]])
+; CHECK: [[META11]] = !DIFile(filename: "{{.*}}foo.c", directory: {{.*}})
+; CHECK: [[META12]] = !DISubroutineType(types: [[META13:![0-9]+]])
+; CHECK: [[META13]] = !{null}
+; CHECK: [[DBG14]] = !DILocation(line: 1, column: 11, scope: [[META10]])
+;.
diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values_dbgrecords.ll.funcsig.globals.expected b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values_dbgrecords.ll.funcsig.globals.expected
index 82f1442f30166..02575d4cff11e 100644
--- a/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values_dbgrecords.ll.funcsig.globals.expected
+++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values_dbgrecords.ll.funcsig.globals.expected
@@ -244,7 +244,7 @@ attributes #2 = { nounwind }
; CHECK: attributes #[[ATTR2]] = { nounwind }
;.
; CHECK: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C99, file: [[META1:![0-9]+]], producer: "{{.*}}clang version {{.*}}", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: [[META2:![0-9]+]], splitDebugInlining: false, nameTableKind: None)
-; CHECK: [[META1]] = !DIFile(filename: "various_ir_values.c", directory: {{.*}})
+; CHECK: [[META1]] = !DIFile(filename: "{{.*}}various_ir_values.c", directory: {{.*}})
; CHECK: [[META2]] = !{}
; CHECK: [[META3:![0-9]+]] = !{i32 7, !"Dwarf Version", i32 4}
; CHECK: [[META4:![0-9]+]] = !{i32 2, !"Debug Info Version", i32 3}
diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values_dbgrecords.ll.funcsig.transitiveglobals.expected b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values_dbgrecords.ll.funcsig.transitiveglobals.expected
index 9addb55bda98d..cd8dac743e2e0 100644
--- a/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values_dbgrecords.ll.funcsig.transitiveglobals.expected
+++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values_dbgrecords.ll.funcsig.transitiveglobals.expected
@@ -238,7 +238,7 @@ attributes #2 = { nounwind }
!62 = !DILocation(line: 11, column: 1, scope: !41)
;.
; CHECK: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C99, file: [[META1:![0-9]+]], producer: "{{.*}}clang version {{.*}}", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: [[META2:![0-9]+]], splitDebugInlining: false, nameTableKind: None)
-; CHECK: [[META1]] = !DIFile(filename: "various_ir_values.c", directory: {{.*}})
+; CHECK: [[META1]] = !DIFile(filename: "{{.*}}various_ir_values.c", directory: {{.*}})
; CHECK: [[META2]] = !{}
; CHECK: [[META7:![0-9]+]] = distinct !DISubprogram(name: "foo", scope: [[META1]], file: [[META1]], line: 1, type: [[META8:![0-9]+]], scopeLine: 1, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: [[META0]], retainedNodes: [[META12:![0-9]+]])
; CHECK: [[META8]] = !DISubroutineType(types: [[META9:![0-9]+]])
diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/difile_absolute_filenames.test b/llvm/test/tools/UpdateTestChecks/update_test_checks/difile_absolute_filenames.test
new file mode 100644
index 0000000000000..cea600d7b787b
--- /dev/null
+++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/difile_absolute_filenames.test
@@ -0,0 +1,7 @@
+## Test that update_test_checks.py is not sensitive to absolute paths in
+## DIFile's filename field.
+# RUN: cp -f %S/Inputs/difile_absolute_filenames.ll %t.ll && %update_test_checks %t.ll --check-globals
+# RUN: diff -u %t.ll %S/Inputs/difile_absolute_filenames.ll.expected
+## Check that running the script again does not change the result:
+# RUN: %update_test_checks %t.ll --check-globals
+# RUN: diff -u %t.ll %S/Inputs/difile_absolute_filenames.ll.expected
diff --git a/llvm/utils/UpdateTestChecks/common.py b/llvm/utils/UpdateTestChecks/common.py
index 274614c2bd272..e747ae3f385a9 100644
--- a/llvm/utils/UpdateTestChecks/common.py
+++ b/llvm/utils/UpdateTestChecks/common.py
@@ -2358,7 +2358,7 @@ def add(var):
r"(?<=\")(.+ )?(\w+ version )[\d.]+(?:[^\" ]*)(?: \([^)]+\))?",
r"{{.*}}\2{{.*}}",
), # preface with glob also, to capture optional CLANG_VENDOR
- (r'(!DIFile\(filename: ".+", directory: )".+"', r"\1{{.*}}"),
+ (r'(!DIFile\(filename: ")(.+/)?([^/]+", directory: )".+"', r"\1{{.*}}\3{{.*}}"),
]
METADATA_FILTERS_RE = [(re.compile(f), r) for (f, r) in METADATA_FILTERS]
|
OCHyams
approved these changes
Apr 17, 2025
Comment on lines
5
to
6
| 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" |
There was a problem hiding this comment.
I don't think we need the datalayout or triple, or function attributes?
There was a problem hiding this comment.
Right, they aren't required by any stretch, but I wanted to commit the unadulterated output of cc1, just to avoid biasing the test away from the ultimate goal for the change, which is to enable tests for CGDebugInfo related changes using update_cc_test_checks.py
I don't actually have an upstream test which will immediately benefit, and I wasn't sure how best to add a test which relies on clang but ultimately is testing UpdateTestChecks itself. If there is a better approach I'm happy to switch to it, and I am also fine stripping obviously unrelated bits from this test if I'm still not being convincing!
There was a problem hiding this comment.
I see, thanks for the background. I don't feel particularly strongly in any direction so I'm happy for you to go ahead either way. (patch still LGTM!)
There was a problem hiding this comment.
Personally, if I understand the intent here correctly, I think that since the test is set up specifically for filenames in debug info, it'd be better to trim the usual stuff from the test, e.g. target info, attributes, checksums, git hashes, etc, unless they are also going to be treated similarly (i.e. have their checks relaxed), in which case they can also be explicitly tested for.
There was a problem hiding this comment.
Fair enough, I can trim it down to focus just on the change, and I'll try to find another place to test that update_cc_test_checks round-trips with debug info enabled
There was a problem hiding this comment.
I tried to trim down the testcase, let me know if I missed anything!
| @@ -2358,7 +2358,7 @@ def add(var): | |||
| r"(?<=\")(.+ )?(\w+ version )[\d.]+(?:[^\" ]*)(?: \([^)]+\))?", | |||
| r"{{.*}}\2{{.*}}", | |||
| ), # preface with glob also, to capture optional CLANG_VENDOR | |||
| (r'(!DIFile\(filename: ".+", directory: )".+"', r"\1{{.*}}"), | |||
| (r'(!DIFile\(filename: ")(.+/)?([^/]+", directory: )".+"', r"\1{{.*}}\3{{.*}}"), | |||
There was a problem hiding this comment.
Do we get file paths in the filename field? (off the top of my head I can't remember - I assume that's why you've added the {{.*}} prefix to file base names here?)
There was a problem hiding this comment.
Yes, and in particular update_cc_test_checks.py seems to invoke clang in such a way as to always generate absolute paths.
The approach I took leaves some potential for ambiguity in some cases, but I think real issues would only crop up for largely contrived test cases (like having a test which depends on multiple input files at different relative paths but with the same filename) and even then the order of the metadata in the output should be stable, so it should be pretty inconsequential.
jmorse
reviewed
Apr 23, 2025
Avoid baking in absolute paths in check lines generated for DIFile metadata. Generated test checks cannot be sensitive to absolute paths anyway, as those vary with the environment, but there could be situations where some sensitivity to partial paths is required for certain tests. This implementation just assumes such tests aren't worth the effort to support, but it could be supported in the future. This is most useful for update_cc_test_checks with debug info enabled, where the test writer cannot manipulate the paths within the generated IR directly.
slinder1
force-pushed
the
main_difile_absolute_filenames
branch
from
2c95bb2 to
146b024
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/16587 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Avoid baking in absolute paths in check lines generated for DIFile metadata. Generated test checks cannot be sensitive to absolute paths anyway, as those vary with the environment, but there could be situations where some sensitivity to partial paths is required for certain tests. This implementation just assumes such tests aren't worth the effort to support, but it could be supported in the future.
This is most useful for update_cc_test_checks with debug info enabled, where the test writer cannot manipulate the paths within the generated IR directly.