RPCS3/llvm-mirror
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2025-02-09 05:47:13 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

[DebugInfo][OPT] Fixing a couple of DI duplication bugs of CloneModule

Browse Source 
As demonstrated by the regression tests added in this patch, the
following cases are valid cases:

1. A Function with no DISubprogram attached, but various debug info
  related to its instructions, coming, for instance, from an inlined
  function, also defined somewhere else in the same module;
2. ... or coming exclusively from the functions inlined and eliminated
  from the module entirely.

The ValueMap shared between CloneFunctionInto calls within CloneModule
needs to contain identity mappings for all of the DISubprogram's to
prevent them from being duplicated by MapMetadata / RemapInstruction
calls, this is achieved via DebugInfoFinder collecting all the
DISubprogram's. However, CloneFunctionInto was missing calls into
DebugInfoFinder for functions w/o DISubprogram's attached, but still
referring DISubprogram's from within (case 1). This patch fixes that.

The fix above, however, exposes another issue: if a module contains a
DISubprogram referenced only indirectly from other debug info
metadata, but not attached to any Function defined within the module
(case 2), cloning such a module causes a DICompileUnit duplication: it
will be moved in indirecty via a DISubprogram by DebugInfoFinder first
(because of the first bug fix described above), without being
self-mapped within the shared ValueMap, and then will be copied during
named metadata cloning. So this patch makes sure DebugInfoFinder
visits DICompileUnit's referenced from DISubprogram's as it goes w/o
re-processing llvm.dbg.cu list over and over again for every function
cloned, and makes sure that CloneFunctionInto self-maps
DICompileUnit's referenced from the entire function, not just its own
DISubprogram attached that may also be missing.

The most convenient way of tesing CloneModule I found is to rely on
CloneModule call from `opt -run-twice`, instead of writing tedious
unit tests. That feature has a couple of properties that makes it hard
to use for this purpose though:

1. CloneModule doesn't copy source filename, making `opt -run-twice`
  report it as a difference.
2. `opt -run-twice` does the second run on the original module, not
  its clone, making the result of cloning completely invisible in opt's
  actual output with and without `-run-twice` both, which directly
  contradicts `opt -run-twice`s own error message.

This patch fixes this as well.

Reviewed By: aprantl

Reviewers: loladiro, GorNishanov, espindola, echristo, dexonsmith

Subscribers: vsk, debug-info, JDevlieghere, llvm-commits

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

llvm-svn: 330069
This commit is contained in:
Roman Tereshin 2018-04-13 21:22:24 +00:00
parent 7c421b4ae9
commit eecdf22144
7 changed files with 199 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/llvm/IR/DebugInfo.h
View File

@ -82,6 +82,7 @@ private:
void processType(DIType *DT);
void processSubprogram(DISubprogram *SP);
void processCompileUnit(DICompileUnit *CU);
void processScope(DIScope *Scope);
bool addCompileUnit(DICompileUnit *CU);
bool addGlobalVariable(DIGlobalVariableExpression *DIG);

53
lib/IR/DebugInfo.cpp
View File

@ -87,6 +87,8 @@ void DebugInfoFinder::processModule(const Module &M) {
processScope(NS->getScope());
else if (auto *M = dyn_cast<DIModule>(Entity))
processScope(M->getScope());
else
llvm_unreachable("unexpected imported entity type");
}
}
for (auto &F : M.functions()) {
@ -96,14 +98,50 @@ void DebugInfoFinder::processModule(const Module &M) {
// instructions only. Walk the function to find them.
for (const BasicBlock &BB : F) {
for (const Instruction &I : BB) {
if (!I.getDebugLoc())
continue;
processLocation(M, I.getDebugLoc().get());
if (auto *DDI = dyn_cast<DbgDeclareInst>(&I))
processDeclare(M, DDI);
else if (auto *DVI = dyn_cast<DbgValueInst>(&I))
processValue(M, DVI);
if (auto DbgLoc = I.getDebugLoc())
processLocation(M, DbgLoc.get());
}
}
}
}
void DebugInfoFinder::processCompileUnit(DICompileUnit *CU) {
if (!addCompileUnit(CU))
return;
for (auto DIG : CU->getGlobalVariables()) {
if (!addGlobalVariable(DIG))
continue;
auto *GV = DIG->getVariable();
processScope(GV->getScope());
processType(GV->getType().resolve());
}
for (auto *ET : CU->getEnumTypes())
processType(ET);
for (auto *RT : CU->getRetainedTypes())
if (auto *T = dyn_cast<DIType>(RT))
processType(T);
else
processSubprogram(cast<DISubprogram>(RT));
for (auto *Import : CU->getImportedEntities()) {
auto *Entity = Import->getEntity().resolve();
if (auto *T = dyn_cast<DIType>(Entity))
processType(T);
else if (auto *SP = dyn_cast<DISubprogram>(Entity))
processSubprogram(SP);
else if (auto *NS = dyn_cast<DINamespace>(Entity))
processScope(NS->getScope());
else if (auto *M = dyn_cast<DIModule>(Entity))
processScope(M->getScope());
else
llvm_unreachable("unexpected imported entity type");
}
}
void DebugInfoFinder::processLocation(const Module &M, const DILocation *Loc) {
if (!Loc)
return;
@ -165,6 +203,15 @@ void DebugInfoFinder::processSubprogram(DISubprogram *SP) {
if (!addSubprogram(SP))
return;
processScope(SP->getScope().resolve());
// Some of the users, e.g. CloneFunctionInto / CloneModule, need to set up a
// ValueMap containing identity mappings for all of the DICompileUnit's, not
// just DISubprogram's, referenced from anywhere within the Function being
// cloned prior to calling MapMetadata / RemapInstruction to avoid their
// duplication later as DICompileUnit's are also directly referenced by
// llvm.dbg.cu list. Thefore we need to collect DICompileUnit's here as well.
// Also, DICompileUnit's may reference DISubprogram's too and therefore need
// to be at least looked through.
processCompileUnit(SP->getUnit());
processType(SP->getType());
for (auto *Element : SP->getTemplateParams()) {
if (auto *TType = dyn_cast<DITemplateTypeParameter>(Element)) {

5
lib/Transforms/Utils/CloneFunction.cpp
View File

@ -175,7 +175,7 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
// Create a new basic block and copy instructions into it!
BasicBlock *CBB = CloneBasicBlock(&BB, VMap, NameSuffix, NewFunc, CodeInfo,
SP ? &DIFinder : nullptr);
ModuleLevelChanges ? &DIFinder : nullptr);
// Add basic block mapping.
VMap[&BB] = CBB;
@ -203,6 +203,9 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
}
}
for (DICompileUnit *CU : DIFinder.compile_units())
VMap.MD()[CU].reset(CU);
for (auto *Type : DIFinder.types()) {
VMap.MD()[Type].reset(Type);
}

1
lib/Transforms/Utils/CloneModule.cpp
View File

@ -50,6 +50,7 @@ std::unique_ptr<Module> llvm::CloneModule(
// First off, we need to create the new module.
std::unique_ptr<Module> New =
llvm::make_unique<Module>(M.getModuleIdentifier(), M.getContext());
New->setSourceFileName(M.getSourceFileName());
New->setDataLayout(M.getDataLayout());
New->setTargetTriple(M.getTargetTriple());
New->setModuleInlineAsm(M.getModuleInlineAsm());

52
test/DebugInfo/X86/clone-module-2.ll Normal file
View File

@ -0,0 +1,52 @@
; RUN: opt -run-twice -verify -S -o - %s | FileCheck %s
; If a module contains a DISubprogram referenced only indirectly from
; instruction-level debug info metadata, but not attached to any Function
; defined within the module, cloning such a module with CloneModule was
; causing a DICompileUnit duplication: it would be moved in indirecty via a
; DISubprogram by DebugInfoFinder (making sure DISubprogram's don't get
; duplicated) first without being explicitly self-mapped within the ValueMap
; shared among CloneFunctionInto calls, and then it would get copied during
; named metadata cloning.
;
; This is to make sure we don't regress on that.
; Derived from the following C-snippet
;
; static int eliminated(int j);
; __attribute__((nodebug)) int nodebug(int k) { return eliminated(k); }
; __attribute__((always_inline)) static int eliminated(int j) { return j * 2; }
;
; compiled with `clang -O1 -g1 -emit-llvm -S`
; CHECK: DICompileUnit
; CHECK-NOT: DICompileUnit
source_filename = "clone-module.c"
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx"
; Function Attrs: norecurse nounwind readnone ssp uwtable
define i32 @nodebug(i32 %k) local_unnamed_addr #0 {
entry:
%mul.i = shl nsw i32 %k, 1, !dbg !8
ret i32 %mul.i
}
attributes #0 = { norecurse nounwind readnone ssp uwtable }
!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!3, !4, !5, !6}
!llvm.ident = !{!7}
!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 7.0.0 (https://git.llvm.org/git/clang.git/ 195459d046e795f5952f7d2121e505eeb59c5574) (https://git.llvm.org/git/llvm.git/ e9dc5b5ade57869d1a443c568c6cf556ccf3b7af)", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
!1 = !DIFile(filename: "test.c", directory: "/Volumes/Data/llvm/build/obj")
!2 = !{}
!3 = !{i32 2, !"Dwarf Version", i32 4}
!4 = !{i32 2, !"Debug Info Version", i32 3}
!5 = !{i32 1, !"wchar_size", i32 4}
!6 = !{i32 7, !"PIC Level", i32 2}
!7 = !{!"clang version 7.0.0 (https://git.llvm.org/git/clang.git/ 195459d046e795f5952f7d2121e505eeb59c5574) (https://git.llvm.org/git/llvm.git/ e9dc5b5ade57869d1a443c568c6cf556ccf3b7af)"}
!8 = !DILocation(line: 3, column: 72, scope: !9)
!9 = distinct !DISubprogram(name: "eliminated", scope: !1, file: !1, line: 3, type: !10, isLocal: true, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !2)
!10 = !DISubroutineType(types: !2)

80
test/DebugInfo/X86/clone-module.ll Normal file
View File

@ -0,0 +1,80 @@
; RUN: opt -run-twice -verify -S -o - %s | FileCheck %s
; The ValueMap shared between CloneFunctionInto calls within CloneModule needs
; to contain identity mappings for all of the DISubprogram's to prevent them
; from being duplicated by MapMetadata / RemapInstruction calls, this is
; achieved via DebugInfoFinder collecting all the DISubprogram's. However,
; CloneFunctionInto was missing calls into DebugInfoFinder for functions w/o
; DISubprogram's attached, but still referring DISubprogram's from within.
;
; This is to make sure we don't regress on that.
; Derived from the following C-snippet
;
; int inlined(int j);
; __attribute__((nodebug)) int nodebug(int k) { return inlined(k); }
; __attribute__((always_inline)) int inlined(int j) { return j * 2; }
;
; compiled with `clang -O1 -g3 -emit-llvm -S` by removing
;
; call void @llvm.dbg.value(metadata i32 %k, metadata !8, metadata !DIExpression()), !dbg !14
;
; line from @nodebug function.
; The @llvm.dbg.value call is manually removed from @nodebug as not having
; it there also may cause an incorrect remapping of the call in a case of a
; regression, not just a duplication of a DISubprogram. Namely, the call's
; metadata !8 2nd argument and the !dbg !14 debug location may get remapped
; to reference different copies of the DISubprogram, which is verified by IR
; Verifier, while having DISubprogram duplicates is not.
; CHECK: DISubprogram
; CHECK-NOT: DISubprogram
source_filename = "clone-module.c"
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx"
; Function Attrs: nounwind readnone ssp uwtable
define i32 @nodebug(i32 %k) local_unnamed_addr #0 {
entry:
%mul.i = shl nsw i32 %k, 1, !dbg !15
ret i32 %mul.i
}
; Function Attrs: alwaysinline nounwind readnone ssp uwtable
define i32 @inlined(i32 %j) local_unnamed_addr #1 !dbg !9 {
entry:
call void @llvm.dbg.value(metadata i32 %j, metadata !8, metadata !DIExpression()), !dbg !14
%mul = shl nsw i32 %j, 1, !dbg !15
ret i32 %mul, !dbg !16
}
; Function Attrs: nounwind readnone speculatable
declare void @llvm.dbg.value(metadata, metadata, metadata) #2
attributes #0 = { nounwind readnone ssp uwtable }
attributes #1 = { alwaysinline nounwind readnone ssp uwtable }
attributes #2 = { nounwind readnone speculatable }
!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!3, !4, !5, !6}
!llvm.ident = !{!7}
!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 7.0.0 (https://git.llvm.org/git/clang.git/ 195459d046e795f5952f7d2121e505eeb59c5574) (https://git.llvm.org/git/llvm.git/ 69ec7d5667e9928db8435bfbee0da151c85a91c9)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
!1 = !DIFile(filename: "clone-module.c", directory: "/somewhere")
!2 = !{}
!3 = !{i32 2, !"Dwarf Version", i32 4}
!4 = !{i32 2, !"Debug Info Version", i32 3}
!5 = !{i32 1, !"wchar_size", i32 4}
!6 = !{i32 7, !"PIC Level", i32 2}
!7 = !{!"clang version 7.0.0 (https://git.llvm.org/git/clang.git/ 195459d046e795f5952f7d2121e505eeb59c5574) (https://git.llvm.org/git/llvm.git/ 69ec7d5667e9928db8435bfbee0da151c85a91c9)"}
!8 = !DILocalVariable(name: "j", arg: 1, scope: !9, file: !1, line: 3, type: !12)
!9 = distinct !DISubprogram(name: "inlined", scope: !1, file: !1, line: 3, type: !10, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !13)
!10 = !DISubroutineType(types: !11)
!11 = !{!12, !12}
!12 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
!13 = !{!8}
!14 = !DILocation(line: 3, column: 48, scope: !9)
!15 = !DILocation(line: 3, column: 62, scope: !9)
!16 = !DILocation(line: 3, column: 53, scope: !9)

20
tools/opt/opt.cpp
View File

@ -755,20 +755,22 @@ int main(int argc, char **argv) {
// Before executing passes, print the final values of the LLVM options.
cl::PrintOptionValues();
// If requested, run all passes again with the same pass manager to catch
// bugs caused by persistent state in the passes
if (RunTwice) {
if (!RunTwice) {
// Now that we have all of the passes ready, run them.
Passes.run(*M);
} else {
// If requested, run all passes twice with the same pass manager to catch
// bugs caused by persistent state in the passes.
std::unique_ptr<Module> M2(CloneModule(*M));
Passes.run(*M2);
// Run all passes on the original module first, so the second run processes
// the clone to catch CloneModule bugs.
Passes.run(*M);
CompileTwiceBuffer = Buffer;
Buffer.clear();
}
// Now that we have all of the passes ready, run them.
Passes.run(*M);
Passes.run(*M2);
// Compare the two outputs and make sure they're the same
if (RunTwice) {
// Compare the two outputs and make sure they're the same
assert(Out);
if (Buffer.size() != CompileTwiceBuffer.size() ||
(memcmp(Buffer.data(), CompileTwiceBuffer.data(), Buffer.size()) !=