[ThinLTO] Add support for optimization remarks to thinBackend

[tejohnson]

Summary

Support was added to the regular LTO backend, but not thinBackend.
This patch adds that support.

[pcc]

LGTM

Maybe consider making finalizeOptimizationRemarks return an llvm::Error so that we can return from thinBackend with return finalizeOptimizationRemarks(...).

[tejohnson]

In D46376#1086811, @pcc wrote:

LGTM

Maybe consider making finalizeOptimizationRemarks return an llvm::Error so that we can return from thinBackend with return finalizeOptimizationRemarks(...).

Good idea, done.

[tejohnson]

Address comments

lib/LTO/LTOBackend.cpp

Expected<const Target *> initAndLookupTarget(Config &C, Module &Mod) {
   const Target *T = TargetRegistry::lookupTarget(Mod.getTargetTriple(), Msg);
   if (!T)
     return make_error<StringError>(Msg, inconvertibleErrorCode());
   return T;
 }
 
 }
 
-static void
+static Error
 finalizeOptimizationRemarks(std::unique_ptr<ToolOutputFile> DiagOutputFile) {
   // Make sure we flush the diagnostic remarks file in case the linker doesn't
   // call the global destructors before exiting.
   if (!DiagOutputFile)
-    return;
+    return Error::success();
   DiagOutputFile->keep();
   DiagOutputFile->os().flush();
+  return Error::success();
 }
 
 Error lto::backend(Config &C, AddStreamFn AddStream,
                    unsigned ParallelCodeGenParallelismLevel,
                    std::unique_ptr<Module> Mod,
                    ModuleSummaryIndex &CombinedIndex) {
   Expected<const Target *> TOrErr = initAndLookupTarget(C, *Mod);
   if (!TOrErr)
     return TOrErr.takeError();
 
   std::unique_ptr<TargetMachine> TM = createTargetMachine(C, *TOrErr, *Mod);
 
   // Setup optimization remarks.
   auto DiagFileOrErr = lto::setupOptimizationRemarks(
       Mod->getContext(), C.RemarksFilename, C.RemarksWithHotness);
   if (!DiagFileOrErr)
     return DiagFileOrErr.takeError();
   auto DiagnosticOutputFile = std::move(*DiagFileOrErr);
 
   if (!C.CodeGenOnly) {
     if (!opt(C, TM.get(), 0, *Mod, /*IsThinLTO=*/false,
-             /*ExportSummary=*/&CombinedIndex, /*ImportSummary=*/nullptr)) {
-      finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
-      return Error::success();
-    }
+             /*ExportSummary=*/&CombinedIndex, /*ImportSummary=*/nullptr))
+      return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
   }
 
   if (ParallelCodeGenParallelismLevel == 1) {
     codegen(C, TM.get(), AddStream, 0, *Mod);
   } else {
     splitCodeGen(C, TM.get(), AddStream, ParallelCodeGenParallelismLevel,
                  std::move(Mod));
   }
-  finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
-  return Error::success();
+  return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
 }
 
 static void dropDeadSymbols(Module &Mod, const GVSummaryMapTy &DefinedGlobals,
                             const ModuleSummaryIndex &Index) {
   std::vector<GlobalValue*> DeadGVs;
   for (auto &GV : Mod.global_values())
     if (GlobalValueSummary *GVS = DefinedGlobals.lookup(GV.getGUID()))
       if (!Index.isGlobalValueLive(GVS)) {
Error lto::thinBackend(Config &Conf, unsigned Task, AddStreamFn AddStream,
                        const GVSummaryMapTy &DefinedGlobals,
                        MapVector<StringRef, BitcodeModule> &ModuleMap) {
   Expected<const Target *> TOrErr = initAndLookupTarget(Conf, Mod);
   if (!TOrErr)
     return TOrErr.takeError();
 
   std::unique_ptr<TargetMachine> TM = createTargetMachine(Conf, *TOrErr, Mod);
 
+  // Setup optimization remarks.
+  auto DiagFileOrErr = lto::setupOptimizationRemarks(
+      Mod.getContext(), Conf.RemarksFilename, Conf.RemarksWithHotness, Task);
+  if (!DiagFileOrErr)
+    return DiagFileOrErr.takeError();
+  auto DiagnosticOutputFile = std::move(*DiagFileOrErr);
+
   if (Conf.CodeGenOnly) {
     codegen(Conf, TM.get(), AddStream, Task, Mod);
-    return Error::success();
+    return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
   }
 
   if (Conf.PreOptModuleHook && !Conf.PreOptModuleHook(Task, Mod))
-    return Error::success();
+    return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
 
   renameModuleForThinLTO(Mod, CombinedIndex);
 
   dropDeadSymbols(Mod, DefinedGlobals, CombinedIndex);
 
   thinLTOResolveWeakForLinkerModule(Mod, DefinedGlobals);
 
   if (Conf.PostPromoteModuleHook && !Conf.PostPromoteModuleHook(Task, Mod))
-    return Error::success();
+    return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
 
   if (!DefinedGlobals.empty())
     thinLTOInternalizeModule(Mod, DefinedGlobals);
 
   if (Conf.PostInternalizeModuleHook &&
       !Conf.PostInternalizeModuleHook(Task, Mod))
-    return Error::success();
+    return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
 
   auto ModuleLoader = [&](StringRef Identifier) {
     assert(Mod.getContext().isODRUniquingDebugTypes() &&
            "ODR Type uniquing should be enabled on the context");
     auto I = ModuleMap.find(Identifier);
     assert(I != ModuleMap.end());
     return I->second.getLazyModule(Mod.getContext(),
                                    /*ShouldLazyLoadMetadata=*/true,
                                    /*IsImporting*/ true);
   };
 
   FunctionImporter Importer(CombinedIndex, ModuleLoader);
   if (Error Err = Importer.importFunctions(Mod, ImportList).takeError())
     return Err;
 
   if (Conf.PostImportModuleHook && !Conf.PostImportModuleHook(Task, Mod))
-    return Error::success();
+    return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
 
   if (!opt(Conf, TM.get(), Task, Mod, /*IsThinLTO=*/true,
            /*ExportSummary=*/nullptr, /*ImportSummary=*/&CombinedIndex))
-    return Error::success();
+    return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
 
   codegen(Conf, TM.get(), AddStream, Task, Mod);
-  return Error::success();
+  return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
 }

test/LTO/Resolution/X86/diagnostic-handler-remarks.ll

-; RUN: llvm-as < %s >%t.bc
+; Test of LTO with opt remarks YAML output.
 
+; First try with Regular LTO
+; RUN: llvm-as < %s >%t.bc
 ; RUN: rm -f %t.yaml
 ; RUN: llvm-lto2 run -pass-remarks-output=%t.yaml \
 ; RUN:           -r %t.bc,tinkywinky,p \
 ; RUN:           -r %t.bc,patatino,px \
 ; RUN:           -r %t.bc,main,px -o %t.o %t.bc
 ; RUN: cat %t.yaml | FileCheck %s -check-prefix=YAML
 
+; Try again with ThinLTO
+; RUN: opt -module-summary %s -o %t.bc
+; RUN: rm -f %t.thin.1.yaml
+; RUN: llvm-lto2 run -pass-remarks-output=%t \
+; RUN:           -r %t.bc,tinkywinky,p \
+; RUN:           -r %t.bc,patatino,px \
+; RUN:           -r %t.bc,main,px -o %t.o %t.bc
+; RUN: cat %t.thin.1.yaml | FileCheck %s -check-prefix=YAML
+
 ; YAML:      --- !Passed
 ; YAML-NEXT: Pass:            inline
 ; YAML-NEXT: Name:            Inlined
 ; YAML-NEXT: Function:        main
 ; YAML-NEXT: Args:
 ; YAML-NEXT:   - Callee:          tinkywinky
 ; YAML-NEXT:   - String:          ' inlined into '
 ; YAML-NEXT:   - Caller:          main