[PATCH] D127168: [llvm-reduce] Add support for LTO bitcode files

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: tools/llvm-reduce/ReducerWorkItem.cpp:425
+    if (std::error_code EC = MB.getError()) {
+      WithColor::error(errs(), ToolName) << Filename << ": " << EC.message() << "\n";
       return std::unique_ptr<ReducerWorkItem>();
----------------
Single quotes around newline


================
Comment at: tools/llvm-reduce/ReducerWorkItem.cpp:434
+        Err.print(ToolName, errs());
+        return std::unique_ptr<ReducerWorkItem>();
+      }
----------------
Why is this returning a default constructed ReducerWorkItem instead of null? I see some other places are doing this but I don't see how this makes sense


================
Comment at: tools/llvm-reduce/ReducerWorkItem.cpp:441
+      if (MMM->LTOInfo->IsThinLTO && MMM->LTOInfo->EnableSplitLTOUnit)
+       initializeTargetInfo();
+    }
----------------
Why does an IR reduction need to initialize target info?


================
Comment at: tools/llvm-reduce/ReducerWorkItem.cpp:455
   auto CloneMMM = std::make_unique<ReducerWorkItem>();
-  if (TM) {
+  if (MMM.MMI) {
     // We're assuming the Module IR contents are always unchanged by MIR
----------------
Why this change?


================
Comment at: tools/llvm-reduce/llvm-reduce.cpp:121
+    legacy::PassManager PM;
+    PM.add(llvm::createWriteThinLTOBitcodePass(OutStream));
+    PM.run(*(M.M));
----------------
Do you really need a pass manager (a legacy one at that) just to write out the file? Why isn't there a simple function to use?


================
Comment at: tools/llvm-reduce/llvm-reduce.cpp:137
+  if (!IF) {
+    WithColor::error(errs()) << IF.takeError();
+    exit(1);
----------------
Other places propagate the tool name from argv


================
Comment at: tools/llvm-reduce/llvm-reduce.cpp:144
+  if (!LI || !MOrErr) {
+    WithColor::error(errs()) << IF.takeError();
+    exit(1);
----------------
Ditto


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127168/new/

https://reviews.llvm.org/D127168