diff options
author | Roman Tereshin <rtereshin@apple.com> | 2018-04-13 21:22:24 +0000 |
---|---|---|
committer | Roman Tereshin <rtereshin@apple.com> | 2018-04-13 21:22:24 +0000 |
commit | 883750c46eb4606e648c9c5170cfe0507f02491c (patch) | |
tree | 1843046c7c38d45dae050650335d7021d8eb9d9a /tools/opt | |
parent | 129fc8ca3cbd6350b1a256e0da356e98761a6499 (diff) |
[DebugInfo][OPT] Fixing a couple of DI duplication bugs of CloneModule
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
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@330069 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'tools/opt')
-rw-r--r-- | tools/opt/opt.cpp | 20 |
1 files changed, 11 insertions, 9 deletions
diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index 32260effd2e..eda61e7c051 100644 --- a/tools/opt/opt.cpp +++ b/tools/opt/opt.cpp @@ -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()) != |