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

[Roman Tereshin via Phabricator via llvm-commits]

rtereshin added inline comments.


================
Comment at: include/llvm/IR/DebugInfo.h:70
+  /// \brief Process a single instruction and collect debug info anchors.
+  void processInstruction(const Module &M, const Instruction &I);
 
----------------
extracted from `processModule` and extended to sync with the `CloneBasicBlock`'s version of the events.


================
Comment at: include/llvm/IR/DebugInfo.h:85
 
-  void processType(DIType *DT);
-  void processSubprogram(DISubprogram *SP);
+  void processCompileUnit(DICompileUnit *CU);
   void processScope(DIScope *Scope);
----------------
extracted from `processModule`


================
Comment at: include/llvm/IR/DebugInfo.h:91
   bool addGlobalVariable(DIGlobalVariableExpression *DIG);
+  bool addScope(DIScope *Scope);
   bool addSubprogram(DISubprogram *SP);
----------------
NFC, just put in alphabetical order.


================
Comment at: lib/IR/DebugInfo.cpp:75
   }
 }
 
----------------
NFC


================
Comment at: lib/IR/DebugInfo.cpp:115
+  if (auto DbgLoc = I.getDebugLoc())
+    processLocation(M, DbgLoc.get());
+}
----------------
extracted from `processModule` and extended to sync with the `CloneBasicBlock`'s version of the events.


================
Comment at: lib/IR/DebugInfo.cpp:179
   processScope(SP->getScope().resolve());
+  processCompileUnit(SP->getUnit());
   processType(SP->getType());
----------------
Fixing case 2 (see the summary).


================
Comment at: lib/Transforms/Utils/CloneFunction.cpp:56
+    if (DIFinder && TheModule)
+      DIFinder->processInstruction(*TheModule, *II);
 
----------------
NFC


================
Comment at: lib/Transforms/Utils/CloneFunction.cpp:171
     BasicBlock *CBB = CloneBasicBlock(&BB, VMap, NameSuffix, NewFunc, CodeInfo,
-                                      SP ? &DIFinder : nullptr);
+                                      ModuleLevelChanges ? &DIFinder : nullptr);
 
----------------
Fixing case 1 (see the summary).


================
Comment at: lib/Transforms/Utils/CloneFunction.cpp:198
+  for (DICompileUnit *CU : DIFinder.compile_units())
+    VMap.MD()[CU].reset(CU);
+
----------------
Fixing case 2 (see the summary).


================
Comment at: lib/Transforms/Utils/CloneFunction.cpp:200
+
+  for (DIType *Type : DIFinder.types())
     VMap.MD()[Type].reset(Type);
----------------
NFC


================
Comment at: lib/Transforms/Utils/CloneModule.cpp:53
       llvm::make_unique<Module>(M.getModuleIdentifier(), M.getContext());
+  New->setSourceFileName(M.getSourceFileName());
   New->setDataLayout(M.getDataLayout());
----------------
Reducing the number of false-failure reports from `opt -run-twice`


================
Comment at: tools/opt/opt.cpp:772
+    Passes.run(*M);
+    FirstRunBuffer = Buffer;
     Buffer.clear();
----------------
Moving from
```
opt:
  Module -> first run -> visible output

opt -run-twice:
  Module -> CloneModule -> first run -> discarded
        \-> second run -> visible output
```
scheme to
```
opt:
  Module -> first run -> visible output

opt -run-twice:
  Module -> first run -> discarded
        \-> CloneModule -> second run -> visible output
```
to
1. Make sure that `opt -run-twice` only discards the output of the (full) run that is easily reproducible with a visible output (by just dropping `-run-twice` in this case)
2. `opt` could be used to debug `CloneModule`


================
Comment at: tools/opt/opt.cpp:782
+      errs()
+          << "Running the pass manager twice changed the output.\n"
+             "Writing the result of the second run to the specified output.\n"
----------------
NFC


Repository:
  rL LLVM

https://reviews.llvm.org/D45593