[PATCH] D19987: Allow the LTO code generator to drop malformed debug info from the input

[Mehdi AMINI via llvm-commits]

joker.eph added inline comments.

================
Comment at: lib/IR/Verifier.cpp:4434
@@ +4433,3 @@
+bool llvm::verifyModule(bool &BrokenDebugInfo, const Module &M,
+                        raw_ostream *OS) {
+  // Don't use a raw_null_ostream.  Printing IR is expensive.
----------------
The only difference between this function and the one above is the output parameter `BrokenDebugInfo` right?

What about having a single function with an optional argument, something like:

```
// Returns true if the module is broken. If BrokenDebugInfo is supplied, DebugInfo 
// verification failures won't be considered as error and instead *BrokenDebugInfo
// will be set to true.
bool llvm::verifyModule(const Module &M, raw_ostream *OS, bool *BrokenDebugInfo) {
  // Don't use a raw_null_ostream.  Printing IR is expensive.
  Verifier V(OS, /*TreatBrokenDebugInfoAsError=*/!BrokenDebugInfo);

  bool Broken = false;
  for (const Function &F : M)
    if (!F.isDeclaration() && !F.isMaterializable())
      Broken |= !V.verify(F);
  Broken |= !V.verify(M)
  if (BrokenDebugInfo) *BrokenDebugInfo = V.hasBrokenDebugInfo();

  // Note that this function's return value is inverted from what you would
  // expect of a function called "verify".
  return Broken;
}
```

================
Comment at: lib/IR/Verifier.cpp:4436
@@ +4435,3 @@
+  // Don't use a raw_null_ostream.  Printing IR is expensive.
+ Verifier V(OS, /*treatBrokenDebugInfoAsError=*/false);
+  bool Broken = false;
----------------
`s/treatBrokenDebugInfoAsError/TreatBrokenDebugInfoAsError/`

================
Comment at: lib/IR/Verifier.cpp:4442
@@ +4441,3 @@
+
+  Broken = !V.verify(M);
+  BrokenDebugInfo = V.hasBrokenDebugInfo();
----------------
This statement unconditionally overwrite `Broken`, I doubt this is what you intended?

================
Comment at: lib/LTO/LTOCodeGenerator.cpp:85
@@ +84,3 @@
+    "lto-strip-invalid-debug-info",
+    cl::desc("Strip invalid debug info metadata during LTO."),
+#ifdef NDEBUG
----------------
What about:
```
  cl::desc("Strip invalid debug info metadata during LTO instead of aborting."),
```
?


http://reviews.llvm.org/D19987