[llvm] 600c129 - [DebugInfo][RemoveDIs] Reverse order of DPValues from findDbgUsers (#74099)

via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 5 03:59:31 PST 2023


Author: Jeremy Morse
Date: 2023-12-05T11:59:26Z
New Revision: 600c12987e17d8be174bb4e4cd95244c598676e6

URL: https://github.com/llvm/llvm-project/commit/600c12987e17d8be174bb4e4cd95244c598676e6
DIFF: https://github.com/llvm/llvm-project/commit/600c12987e17d8be174bb4e4cd95244c598676e6.diff

LOG: [DebugInfo][RemoveDIs] Reverse order of DPValues from findDbgUsers (#74099)

The order of dbg.value intrinsics appearing in the output can affect the
order of tables in DWARF sections. This means that DPValues, our
dbg.value replacement, needs to obey the same ordering rules. For
dbg.values returned by findDbgUsers it's reverse order of creation (due
to how they're put on use-lists). Produce that order from findDbgUsers
for DPValues.

I've got a few IR files where the order of dbg.values flips, but it's a
fragile test -- ultimately it needs the number of times a DPValue is
handled by findDbgValues to be odd.

Added: 
    

Modified: 
    llvm/lib/IR/Metadata.cpp
    llvm/lib/Transforms/Utils/Local.cpp
    llvm/unittests/IR/DebugInfoTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/IR/Metadata.cpp b/llvm/lib/IR/Metadata.cpp
index 415e256c817b8..7bc25e30b8932 100644
--- a/llvm/lib/IR/Metadata.cpp
+++ b/llvm/lib/IR/Metadata.cpp
@@ -249,8 +249,13 @@ SmallVector<DPValue *> ReplaceableMetadataImpl::getAllDPValueUsers() {
       continue;
     DPVUsersWithID.push_back(&UseMap[Pair.first]);
   }
+  // Order DPValue users in reverse-creation order. Normal dbg.value users
+  // of MetadataAsValues are ordered by their UseList, i.e. reverse order of
+  // when they were added: we need to replicate that here. The structure of
+  // debug-info output depends on the ordering of intrinsics, thus we need
+  // to keep them consistent for comparisons sake.
   llvm::sort(DPVUsersWithID, [](auto UserA, auto UserB) {
-    return UserA->second < UserB->second;
+    return UserA->second > UserB->second;
   });
   SmallVector<DPValue *> DPVUsers;
   for (auto UserWithID : DPVUsersWithID)

diff  --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index e399329a58873..dfc78aa589ef8 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -2587,14 +2587,7 @@ static bool rewriteDebugUsers(
     }
 
     // DPValue implementation of the above.
-    // RemoveDIs misery: The above loop of intrinsic-users are ordered by the
-    // use-list of the corresponding metadata-as-value: in reverse order of when
-    // they were added. Wheras DPUsers are ordered by when they were added to
-    // the replaceable-metadata map, i.e., in the order they were added. Thus to
-    // have matching orders between the two, we have to reverse here. For
-    // RemoveDIs we might in the long run need to consider whether this implicit
-    // ordering is relied upon by any other part of LLVM.
-    for (auto *DPV : llvm::reverse(DPUsers)) {
+    for (auto *DPV : DPUsers) {
       Instruction *MarkedInstr = DPV->getMarker()->MarkedInstr;
       Instruction *NextNonDebug = MarkedInstr;
       // The next instruction might still be a dbg.declare, skip over it.

diff  --git a/llvm/unittests/IR/DebugInfoTest.cpp b/llvm/unittests/IR/DebugInfoTest.cpp
index a408c0af5623b..be8f590a27eb4 100644
--- a/llvm/unittests/IR/DebugInfoTest.cpp
+++ b/llvm/unittests/IR/DebugInfoTest.cpp
@@ -284,6 +284,71 @@ TEST(MetadataTest, DeleteInstUsedByDPValue) {
   UseNewDbgInfoFormat = OldDbgValueMode;
 }
 
+// Ensure that the order of dbg.value intrinsics returned by findDbgValues, and
+// their corresponding DPValue representation, are consistent.
+TEST(MetadataTest, OrderingOfDPValues) {
+  LLVMContext C;
+  std::unique_ptr<Module> M = parseIR(C, R"(
+    define i16 @f(i16 %a) !dbg !6 {
+      %b = add i16 %a, 1, !dbg !11
+      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+      call void @llvm.dbg.value(metadata i16 %b, metadata !12, metadata !DIExpression()), !dbg !11
+      ret i16 0, !dbg !11
+    }
+    declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+    attributes #0 = { nounwind readnone speculatable willreturn }
+
+    !llvm.dbg.cu = !{!0}
+    !llvm.module.flags = !{!5}
+
+    !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+    !1 = !DIFile(filename: "t.ll", directory: "/")
+    !2 = !{}
+    !5 = !{i32 2, !"Debug Info Version", i32 3}
+    !6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+    !7 = !DISubroutineType(types: !2)
+    !8 = !{!9}
+    !9 = !DILocalVariable(name: "foo", scope: !6, file: !1, line: 1, type: !10)
+    !10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned)
+    !11 = !DILocation(line: 1, column: 1, scope: !6)
+    !12 = !DILocalVariable(name: "bar", scope: !6, file: !1, line: 1, type: !10)
+)");
+
+  bool OldDbgValueMode = UseNewDbgInfoFormat;
+  UseNewDbgInfoFormat = true;
+  Instruction &I = *M->getFunction("f")->getEntryBlock().getFirstNonPHI();
+
+  SmallVector<DbgValueInst *, 2> DVIs;
+  SmallVector<DPValue *, 2> DPVs;
+  findDbgValues(DVIs, &I, &DPVs);
+  ASSERT_EQ(DVIs.size(), 2u);
+  ASSERT_EQ(DPVs.size(), 0u);
+
+  // The correct order of dbg.values is given by their use-list, which becomes
+  // the reverse order of creation. Thus the dbg.values should come out as
+  // "bar" and then "foo".
+  DILocalVariable *Var0 = DVIs[0]->getVariable();
+  EXPECT_TRUE(Var0->getName() == "bar");
+  DILocalVariable *Var1 = DVIs[1]->getVariable();
+  EXPECT_TRUE(Var1->getName() == "foo");
+
+  // Now try again, but in DPValue form.
+  DVIs.clear();
+
+  M->convertToNewDbgValues();
+  findDbgValues(DVIs, &I, &DPVs);
+  ASSERT_EQ(DVIs.size(), 0u);
+  ASSERT_EQ(DPVs.size(), 2u);
+
+  Var0 = DPVs[0]->getVariable();
+  EXPECT_TRUE(Var0->getName() == "bar");
+  Var1 = DPVs[1]->getVariable();
+  EXPECT_TRUE(Var1->getName() == "foo");
+
+  M->convertFromNewDbgValues();
+  UseNewDbgInfoFormat = OldDbgValueMode;
+}
+
 TEST(DIBuiler, CreateFile) {
   LLVMContext Ctx;
   std::unique_ptr<Module> M(new Module("MyModule", Ctx));


        


More information about the llvm-commits mailing list