Skip to content
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

[RemoveDIs][DebugInfo] Perform some pre-turn-on test maintenence #80885

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

jmorse
Copy link
Member

@jmorse jmorse commented Feb 6, 2024

As we'll hopefully move away from using intrinsics for debug-info shortly, this commit stabilizes a few tests to avoid spurious changes in the process. Briefly, there are differences in output when we don't use intrinsics that we're going to suppress in case we have to revert, these are:

  • The attributor test gets different attributes for the dbg.value intrinsic because it's not present during optimisation. This has no functional effect and there's no need to test for it.
  • The Scalarizer test exposes a "debug-info affects codegen" problem, but fixing it is fiddly (updating 20 IRBuilder object calls). Pin this test to not change with RemoveDIs, we can relax it later and get the correct behaviour.
  • DIDefaultTemplateParam.ll tests for explicit metadata node numbers which is generally bad. Add explicit node-number capturing CHECK lines.

As we'll hopefully move away from using intrinsics for debug-info shortly,
this commit stabilizes a few tests to avoid spurious changes in the
process. Briefly, there are differences in output when we don't use
intrinsics that we're going to suppress in case we have to revert, these
are:
 * The attributor test gets different attributes for the dbg.value
   intrinsic because it's not present during optimisation. This has no
   functional effect and there's no need to test for it.
 * The Scalarizer test exposes a "debug-info affects codegen" problem, but
   fixing it is fiddly (updating 20 IRBuilder object calls). Pin this test
   to not change with RemoveDIs, we can relax it later and get the correct
   behaviour.
 * DIDefaultTemplateParam.ll tests for explicit metadata node numbers which
   is generally bad. Add explicit node-number capturing CHECK lines.
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 6, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-debuginfo

Author: Jeremy Morse (jmorse)

Changes

As we'll hopefully move away from using intrinsics for debug-info shortly, this commit stabilizes a few tests to avoid spurious changes in the process. Briefly, there are differences in output when we don't use intrinsics that we're going to suppress in case we have to revert, these are:

  • The attributor test gets different attributes for the dbg.value intrinsic because it's not present during optimisation. This has no functional effect and there's no need to test for it.
  • The Scalarizer test exposes a "debug-info affects codegen" problem, but fixing it is fiddly (updating 20 IRBuilder object calls). Pin this test to not change with RemoveDIs, we can relax it later and get the correct behaviour.
  • DIDefaultTemplateParam.ll tests for explicit metadata node numbers which is generally bad. Add explicit node-number capturing CHECK lines.

Full diff: https://github.com/llvm/llvm-project/pull/80885.diff

3 Files Affected:

  • (modified) llvm/test/Assembler/DIDefaultTemplateParam.ll (+13-9)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/pr33641_remove_arg_dbgvalue.ll (+5-2)
  • (modified) llvm/test/Transforms/Scalarizer/dbginfo.ll (+9-1)
diff --git a/llvm/test/Assembler/DIDefaultTemplateParam.ll b/llvm/test/Assembler/DIDefaultTemplateParam.ll
index 4ebd45a2749ef1..015da31629ad8b 100644
--- a/llvm/test/Assembler/DIDefaultTemplateParam.ll
+++ b/llvm/test/Assembler/DIDefaultTemplateParam.ll
@@ -29,6 +29,19 @@ attributes #1 = { nounwind readnone speculatable willreturn }
 !llvm.module.flags = !{!3, !4, !5}
 !llvm.ident = !{!6}
 
+;; Test that the templateParams for the DICompositeTypes are retained:
+; CHECK:      ![[COMPTAG:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "foo<int, 6>",
+; CHECK-SAME: templateParams: ![[PARAMLIST:[0-9]+]]
+; CHECK:      ![[PARAMLIST]] = !{![[PARAM1:[0-9]+]], ![[PARAM2:[0-9]+]]}
+; CHECK:      ![[PARAM1]] = !DITemplateTypeParameter(name: "T", type: !{{[0-9]*}})
+; CHECK:      ![[PARAM2]] = !DITemplateValueParameter(name: "i", type: !{{[0-9]*}}, value: i32 6)
+
+; CHECK:      ![[COMPTAG2:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "foo<char, 3>",
+; CHECK-SAME: templateParams: ![[PARAMLIST2:[0-9]+]]
+; CHECK:      ![[PARAMLIST2]] = !{![[PARAM3:[0-9]+]], ![[PARAM4:[0-9]+]]}
+; CHECK:      ![[PARAM3]] = !DITemplateTypeParameter({{.*}}, defaulted: true
+; CHECK:      ![[PARAM4]] = !DITemplateValueParameter({{.*}}, defaulted: true
+
 !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 11.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
 !1 = !DIFile(filename: "test.cpp", directory: "/dir/", checksumkind: CSK_MD5, checksum: "863d08522c2300490dea873efc4b2369")
 !2 = !{}
@@ -43,23 +56,14 @@ attributes #1 = { nounwind readnone speculatable willreturn }
 !11 = !DILocalVariable(name: "f1", scope: !7, file: !1, line: 30, type: !12)
 !12 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "foo<int, 6>", file: !1, line: 26, size: 8, flags: DIFlagTypePassByValue, elements: !2, templateParams: !13, identifier: "_ZTS3fooIiLi6EE")
 !13 = !{!14, !15}
-
-; CHECK: 14 = !DITemplateTypeParameter(name: "T", type: !{{[0-9]*}})
 !14 = !DITemplateTypeParameter(name: "T", type: !10)
-
-; CHECK: 15 = !DITemplateValueParameter(name: "i", type: !{{[0-9]*}}, value: i32 6)
 !15 = !DITemplateValueParameter(name: "i", type: !10, value: i32 6)
-
 !16 = !DILocation(line: 30, column: 14, scope: !7)
 !17 = !DILocalVariable(name: "f2", scope: !7, file: !1, line: 31, type: !18)
 !18 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "foo<char, 3>", file: !1, line: 26, size: 8, flags: DIFlagTypePassByValue, elements: !2, templateParams: !19, identifier: "_ZTS3fooIcLi3EE")
 !19 = !{!20, !22}
-
-; CHECK: 20 = !DITemplateTypeParameter({{.*}}, defaulted: true
 !20 = !DITemplateTypeParameter(name: "T", type: !21, defaulted: true)
 !21 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
-
-; CHECK: 22 = !DITemplateValueParameter({{.*}}, defaulted: true
 !22 = !DITemplateValueParameter(name: "i", type: !10, defaulted: true, value: i32 3)
 !23 = !DILocation(line: 31, column: 9, scope: !7)
 !24 = !DILocation(line: 32, column: 3, scope: !7)
diff --git a/llvm/test/Transforms/Attributor/ArgumentPromotion/pr33641_remove_arg_dbgvalue.ll b/llvm/test/Transforms/Attributor/ArgumentPromotion/pr33641_remove_arg_dbgvalue.ll
index d52340a3d89958..83226e56b5acd9 100644
--- a/llvm/test/Transforms/Attributor/ArgumentPromotion/pr33641_remove_arg_dbgvalue.ll
+++ b/llvm/test/Transforms/Attributor/ArgumentPromotion/pr33641_remove_arg_dbgvalue.ll
@@ -2,6 +2,9 @@
 ; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal  -attributor-annotate-decl-cs  -S < %s | FileCheck %s --check-prefixes=CHECK,TUNIT
 ; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal  -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,CGSCC
 
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal  -attributor-annotate-decl-cs  -S < %s --try-experimental-debuginfo-iterators | FileCheck %s --check-prefixes=CHECK,TUNIT
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal  -attributor-annotate-decl-cs -S < %s --try-experimental-debuginfo-iterators | FileCheck %s --check-prefixes=CHECK,CGSCC
+
 ; Fix for PR33641. ArgumentPromotion removed the argument to bar but left the call to
 ; dbg.value which still used the removed argument.
 
@@ -27,7 +30,8 @@ define internal void @bar(%p_t %p)  {
 ; CGSCC: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(none)
 ; CGSCC-LABEL: define {{[^@]+}}@bar
 ; CGSCC-SAME: (ptr nocapture nofree readnone [[P:%.*]]) #[[ATTR0]] {
-; CGSCC-NEXT:    call void @llvm.dbg.value(metadata ptr [[P]], metadata [[META3:![0-9]+]], metadata !DIExpression()) #[[ATTR2:[0-9]+]], !dbg [[DBG5:![0-9]+]]
+; CGSCC-NEXT:    call void @llvm.dbg.value(metadata ptr [[P]], metadata [[META3:![0-9]+]], metadata !DIExpression())
+; CGSCC-SAME:         !dbg [[DBG5:![0-9]+]]
 ; CGSCC-NEXT:    ret void
 ;
   call void @llvm.dbg.value(metadata %p_t %p, metadata !4, metadata !5), !dbg !6
@@ -52,7 +56,6 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
 ;.
 ; CGSCC: attributes #[[ATTR0]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) }
 ; CGSCC: attributes #[[ATTR1:[0-9]+]] = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
-; CGSCC: attributes #[[ATTR2]] = { nofree nosync willreturn }
 ;.
 ; TUNIT: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
 ; TUNIT: [[META1:![0-9]+]] = !DIFile(filename: "test.c", directory: "")
diff --git a/llvm/test/Transforms/Scalarizer/dbginfo.ll b/llvm/test/Transforms/Scalarizer/dbginfo.ll
index 62871b4e8ff608..3b48915b605d75 100644
--- a/llvm/test/Transforms/Scalarizer/dbginfo.ll
+++ b/llvm/test/Transforms/Scalarizer/dbginfo.ll
@@ -1,5 +1,13 @@
-; RUN: opt %s -passes='function(scalarizer)' -scalarize-load-store -S | FileCheck %s
+; RUN: opt %s -passes='function(scalarizer)' -scalarize-load-store -S --experimental-debuginfo-iterators=false | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+; FIXME: the test output here changes if we use the RemoveDIs non-intrinsic
+; debug-info format for the test. Specifically, the intrinsics no longer
+; interfere with the scalarizer, and we get the same code with/without
+; debug-info.
+; That's the behaviour we want; but fix this test to only use intrinsics as
+; we introduce the non-intrinsic format, to reduce the amount of spurious test
+; changes it comes with. We can turn that off once we're committed to using
+; the non-intrinsic format.
 
 ; Function Attrs: nounwind uwtable
 define void @f1(ptr nocapture %a, ptr nocapture readonly %b, ptr nocapture readonly %c) #0 !dbg !4 {

@jmorse jmorse merged commit 89ad31f into llvm:main Feb 7, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants