[llvm] f09ccf8 - [ThinLTO] Fix a metadata lost issue with DICompileUnit import.

Hongtao Yu via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 2 14:40:56 PDT 2020


Author: Hongtao Yu
Date: 2020-09-02T14:40:41-07:00
New Revision: f09ccf89fbee976bcca77b374f69987c2e96e1ce

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

LOG: [ThinLTO] Fix a metadata lost issue with DICompileUnit import.

For ThinLTO importing we don't need to import all the fields of the DICompileUnit, such as enums, macros, retained types lists. The importation of those fields were previously disabled by setting their value map entries to nullptr. Unfortunately a metadata node can be shared by multiple metadata operands. Setting the map entry to nullptr might result in not importing other metadata unexpectedly.  The issue is fixed by explicitly setting the original DICompileUnit fields (still a copy of the source module metadata) to null.

Reviewed By: wenlei, dblaikie

Differential Revision: https://reviews.llvm.org/D86675

Added: 
    llvm/test/ThinLTO/X86/Inputs/import-metadata.ll
    llvm/test/ThinLTO/X86/import-metadata.ll

Modified: 
    llvm/lib/Linker/IRMover.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 055689b16e8f..186ddb3d2b81 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -1126,14 +1126,13 @@ void IRLinker::prepareCompileUnitsForImport() {
     assert(CU && "Expected valid compile unit");
     // Enums, macros, and retained types don't need to be listed on the
     // imported DICompileUnit. This means they will only be imported
-    // if reached from the mapped IR. Do this by setting their value map
-    // entries to nullptr, which will automatically prevent their importing
-    // when reached from the DICompileUnit during metadata mapping.
-    ValueMap.MD()[CU->getRawEnumTypes()].reset(nullptr);
-    ValueMap.MD()[CU->getRawMacros()].reset(nullptr);
-    ValueMap.MD()[CU->getRawRetainedTypes()].reset(nullptr);
+    // if reached from the mapped IR.
+    CU->replaceEnumTypes(nullptr);
+    CU->replaceMacros(nullptr);
+    CU->replaceRetainedTypes(nullptr);
+
     // The original definition (or at least its debug info - if the variable is
-    // internalized an optimized away) will remain in the source module, so
+    // internalized and optimized away) will remain in the source module, so
     // there's no need to import them.
     // If LLVM ever does more advanced optimizations on global variables
     // (removing/localizing write operations, for instance) that can track
@@ -1141,7 +1140,7 @@ void IRLinker::prepareCompileUnitsForImport() {
     // with care when it comes to debug info size. Emitting small CUs containing
     // only a few imported entities into every destination module may be very
     // size inefficient.
-    ValueMap.MD()[CU->getRawGlobalVariables()].reset(nullptr);
+    CU->replaceGlobalVariables(nullptr);
 
     // Imported entities only need to be mapped in if they have local
     // scope, as those might correspond to an imported entity inside a
@@ -1174,7 +1173,7 @@ void IRLinker::prepareCompileUnitsForImport() {
       else
         // If there were no local scope imported entities, we can map
         // the whole list to nullptr.
-        ValueMap.MD()[CU->getRawImportedEntities()].reset(nullptr);
+        CU->replaceImportedEntities(nullptr);
     }
   }
 }

diff  --git a/llvm/test/ThinLTO/X86/Inputs/import-metadata.ll b/llvm/test/ThinLTO/X86/Inputs/import-metadata.ll
new file mode 100644
index 000000000000..d8be887928a2
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/Inputs/import-metadata.ll
@@ -0,0 +1,23 @@
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-scei-ps4"
+
+define i32 @foo(i32 %goo) {
+entry:
+  %goo.addr = alloca i32, align 4
+  store i32 %goo, i32* %goo.addr, align 4
+  %0 = load i32, i32* %goo.addr, align 4
+  %1 = load i32, i32* %goo.addr, align 4
+  %mul = mul nsw i32 %0, %1
+  ret i32 %mul
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3}
+!llvm.md = !{!5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, enums: !4)
+!1 = !DIFile(filename: "foo.cpp", directory: "tmp")
+!2 = !{i32 2, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{}
+!5 = !{!4}

diff  --git a/llvm/test/ThinLTO/X86/import-metadata.ll b/llvm/test/ThinLTO/X86/import-metadata.ll
new file mode 100644
index 000000000000..f938fdd5c93c
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/import-metadata.ll
@@ -0,0 +1,40 @@
+; RUN: opt -thinlto-bc %s -o %t1.bc
+; RUN: opt -thinlto-bc %p/Inputs/import-metadata.ll -o %t2.bc
+; RUN: llvm-lto2 run -save-temps %t1.bc %t2.bc -o %t-out \
+; RUN:    -r=%t1.bc,main,plx \
+; RUN:    -r=%t1.bc,foo,l \
+; RUN:    -r=%t2.bc,foo,pl
+; RUN: llvm-dis %t-out.1.3.import.bc -o - | FileCheck %s
+
+;; Check the imported DICompileUnit doesn't have the enums operand.
+;; Also check the imported md metadata that shares a node with the 
+;; enums operand originally is not null.
+
+; CHECK: !llvm.dbg.cu = !{![[#CU1:]], ![[#CU2:]]}
+;; Note that MD1 comes from the current module. MD2 is from the imported module. 
+;; We are checking if the imported MD2 doesn't end up having a null operand.
+; CHECK: !llvm.md = !{![[#MD1:]], ![[#MD2:]]}
+; CHECK: ![[#MD3:]] = !{}
+; CHECK: ![[#CU2]] = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: ![[#FILE2:]], isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
+; CHECK: ![[#MD2]] = !{![[#MD3]]}
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-scei-ps4"
+
+declare i32 @foo(i32 %goo)
+
+define i32 @main() {
+  call i32 @foo(i32 0)
+  ret i32 0
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3}
+!llvm.md = !{!5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, enums: !4)
+!1 = !DIFile(filename: "main.cpp", directory: "tmp")
+!2 = !{i32 2, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{}
+!5 = !{!4}


        


More information about the llvm-commits mailing list