[LTO] Add optimization remarks for removed functions

[xazax.hun]

Summary

Some functions are not getting added to the combined module that will be optimized later. Knowing which functions were removed could be useful for debugging or for detecting dead code. While getting the list of removed functions is not hard, optimizations remarks were not set up until just before opt was run. I moved this setup process a bit earlier for regular LTO so I can emit remarks. See http://lists.llvm.org/pipermail/llvm-dev/2020-January/138660.html for the discussion.
Note that, I did not change ThinLTO, as those modules will only be available later on.

[thegameg]

llvm/lib/LTO/LTO.cpp

789

This is not the usual way we add strings with arguments to remarks. Please use << "STR: " << ore::NV("Function", F).

[xazax.hun]

• More idiomatic optimization remark is emitted.

[xazax.hun]

llvm/lib/LTO/LTO.cpp

789

Thanks!

[tejohnson]

llvm/lib/LTO/LTO.cpp

967

Can you add a FIXME here noting that this is checking both Regular and Thin LTO and should be split into two, one checking Regular here and the other checking the ThinLTO index in runThinLTO?

This does work since we always go through this path, but logically it would make more sense to do the ThinLTO checking in runThinLTO. Unfortunately, the regular LTO checking this is doing must be after linkRegularLTO.

llvm/test/LTO/Resolution/X86/dead-strip-fulllto.ll

19

Is the yaml remark more verbose than this (i.e. it includes your message)? Would be good to include more of it.

[xazax.hun]

• Addressed more review comments.

[xazax.hun]

llvm/lib/LTO/LTO.cpp

967

Let me know if you prefer to reword this. I am not a native speaker and not familiar with the nomenclature of LLVM.

llvm/test/LTO/Resolution/X86/dead-strip-fulllto.ll

19

Good point, thanks!

[tejohnson]

lgtm

llvm/lib/LTO/LTO.cpp

967

It looks fine, thanks.

[thegameg]

llvm/lib/LTO/LTO.cpp

789

These changes and the test updates look good to me fwiw! Thanks!

llvm/include/llvm/LTO/LTOBackend.h

Error backend(const Config &C, AddStreamFn AddStream,
               std::unique_ptr<Module> M, ModuleSummaryIndex &CombinedIndex);
 
 /// Runs a ThinLTO backend.
 Error thinBackend(const Config &C, unsigned Task, AddStreamFn AddStream,
                   Module &M, const ModuleSummaryIndex &CombinedIndex,
                   const FunctionImporter::ImportMapTy &ImportList,
                   const GVSummaryMapTy &DefinedGlobals,
                   MapVector<StringRef, BitcodeModule> &ModuleMap);
+
+Error finalizeOptimizationRemarks(
+    std::unique_ptr<ToolOutputFile> DiagOutputFile);
 }
 }
 
 #endif

llvm/lib/IR/DiagnosticInfo.cpp

 
 OptimizationRemark::OptimizationRemark(const char *PassName,
                                        StringRef RemarkName,
                                        const Instruction *Inst)
     : DiagnosticInfoIROptimization(DK_OptimizationRemark, DS_Remark, PassName,
                                    RemarkName, *Inst->getParent()->getParent(),
                                    Inst->getDebugLoc(), Inst->getParent()) {}
 
-// Helper to allow for an assert before attempting to return an invalid
-// reference.
-static const BasicBlock &getFirstFunctionBlock(const Function *Func) {
-  assert(!Func->empty() && "Function does not have a body");
-  return Func->front();
+static const BasicBlock *getFirstFunctionBlock(const Function *Func) {
+  return Func->empty() ? nullptr : &Func->front();
 }
 
 OptimizationRemark::OptimizationRemark(const char *PassName,
                                        StringRef RemarkName,
                                        const Function *Func)
     : DiagnosticInfoIROptimization(DK_OptimizationRemark, DS_Remark, PassName,
                                    RemarkName, *Func, Func->getSubprogram(),
-                                   &getFirstFunctionBlock(Func)) {}
+                                   getFirstFunctionBlock(Func)) {}
 
 bool OptimizationRemark::isEnabled() const {
   const Function &Fn = getFunction();
   LLVMContext &Ctx = Fn.getContext();
   return Ctx.getDiagHandlerPtr()->isPassedOptRemarkEnabled(getPassName());
 }
 
 OptimizationRemarkMissed::OptimizationRemarkMissed(

llvm/lib/LTO/LTO.cpp

 //===-LTO.cpp - LLVM Link Time Optimizer ----------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements functions and classes used to support LTO.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/LTO/LTO.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/CodeGen/Analysis.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/AutoUpgrade.h"
 #include "llvm/IR/DiagnosticPrinter.h"
LTO::addRegularLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
   assert(MsymI == MsymE);
   return std::move(Mod);
 }
 
 Error LTO::linkRegularLTO(RegularLTOState::AddedModule Mod,
                           bool LivenessFromIndex) {
   std::vector<GlobalValue *> Keep;
   for (GlobalValue *GV : Mod.Keep) {
-    if (LivenessFromIndex && !ThinLTO.CombinedIndex.isGUIDLive(GV->getGUID()))
+    if (LivenessFromIndex && !ThinLTO.CombinedIndex.isGUIDLive(GV->getGUID())) {
+      if (Function *F = dyn_cast<Function>(GV)) {
+        OptimizationRemarkEmitter ORE(F);
+        ORE.emit(OptimizationRemark(DEBUG_TYPE, "deadfunction", F)
+                 << ore::NV("Function", F)
+                 << " not added to the combined module ");
+      }
       continue;
+    }
 
     if (!GV->hasAvailableExternallyLinkage()) {
       Keep.push_back(GV);
       continue;
     }
 
     // Only link available_externally definitions if we don't already have a
     // definition.
computeDeadSymbolsWithConstProp(ThinLTO.CombinedIndex, GUIDPreservedSymbols,
                                   isPrevailing, Conf.OptLevel > 0);
 
   // Setup output file to emit statistics.
   auto StatsFileOrErr = setupStatsFile(Conf.StatsFile);
   if (!StatsFileOrErr)
     return StatsFileOrErr.takeError();
   std::unique_ptr<ToolOutputFile> StatsFile = std::move(StatsFileOrErr.get());
 
-  // Finalize linking of regular LTO modules containing summaries now that
-  // we have computed liveness information.
-  for (auto &M : RegularLTO.ModsWithSummaries)
-    if (Error Err = linkRegularLTO(std::move(M),
-                                   /*LivenessFromIndex=*/true))
-      return Err;
-
-  // Ensure we don't have inconsistently split LTO units with type tests.
-  if (Error Err = checkPartiallySplit())
-    return Err;
-
   Error Result = runRegularLTO(AddStream);
   if (!Result)
     Result = runThinLTO(AddStream, Cache, GUIDPreservedSymbols);
 
   if (StatsFile)
     PrintStatisticsJSON(StatsFile->os());
 
   return Result;
 }
 
 Error LTO::runRegularLTO(AddStreamFn AddStream) {
+  // Setup optimization remarks.
+  auto DiagFileOrErr = lto::setupOptimizationRemarks(
+      RegularLTO.CombinedModule->getContext(), Conf.RemarksFilename,
+      Conf.RemarksPasses, Conf.RemarksFormat, Conf.RemarksWithHotness);
+  if (!DiagFileOrErr)
+    return DiagFileOrErr.takeError();
+
+  // Finalize linking of regular LTO modules containing summaries now that
+  // we have computed liveness information.
+  for (auto &M : RegularLTO.ModsWithSummaries)
+    if (Error Err = linkRegularLTO(std::move(M),
+                                   /*LivenessFromIndex=*/true))
+      return Err;
+
+  // Ensure we don't have inconsistently split LTO units with type tests.
+  // FIXME: this checks both LTO and ThinLTO. It happens to work as we take
+  // this path both cases but eventually this should be split into two and
+  // do the ThinLTO checks in `runThinLTO`.
+  if (Error Err = checkPartiallySplit())
+    return Err;
+
   // Make sure commons have the right size/alignment: we kept the largest from
   // all the prevailing when adding the inputs, and we apply it here.
   const DataLayout &DL = RegularLTO.CombinedModule->getDataLayout();
   for (auto &I : RegularLTO.Commons) {
     if (!I.second.Prevailing)
       // Don't do anything if no instance of this common was prevailing.
       continue;
     GlobalVariable *OldGV = RegularLTO.CombinedModule->getNamedGlobal(I.first);
if (!Conf.CodeGenOnly) {
     }
 
     RegularLTO.CombinedModule->addModuleFlag(Module::Error, "LTOPostLink", 1);
 
     if (Conf.PostInternalizeModuleHook &&
         !Conf.PostInternalizeModuleHook(0, *RegularLTO.CombinedModule))
       return Error::success();
   }
-  return backend(Conf, AddStream, RegularLTO.ParallelCodeGenParallelismLevel,
-                 std::move(RegularLTO.CombinedModule), ThinLTO.CombinedIndex);
+  if (Error Err =
+          backend(Conf, AddStream, RegularLTO.ParallelCodeGenParallelismLevel,
+                  std::move(RegularLTO.CombinedModule), ThinLTO.CombinedIndex))
+    return Err;
+
+  return finalizeOptimizationRemarks(std::move(*DiagFileOrErr));
 }
 
 static const char *libcallRoutineNames[] = {
 #define HANDLE_LIBCALL(code, name) name,
 #include "llvm/IR/RuntimeLibcalls.def"
 #undef HANDLE_LIBCALL
 };
 

llvm/lib/LTO/LTOBackend.cpp

Expected<const Target *> initAndLookupTarget(const Config &C, Module &Mod) {
   std::string Msg;
   const Target *T = TargetRegistry::lookupTarget(Mod.getTargetTriple(), Msg);
   if (!T)
     return make_error<StringError>(Msg, inconvertibleErrorCode());
   return T;
 }
 }
 
-static Error
-finalizeOptimizationRemarks(std::unique_ptr<ToolOutputFile> DiagOutputFile) {
+Error lto::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 Error::success();
   DiagOutputFile->keep();
   DiagOutputFile->os().flush();
   return Error::success();
 }
 
 Error lto::backend(const 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.RemarksPasses, C.RemarksFormat,
-      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))
-      return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
+      return Error::success();
   }
 
   if (ParallelCodeGenParallelismLevel == 1) {
     codegen(C, TM.get(), AddStream, 0, *Mod);
   } else {
     splitCodeGen(C, TM.get(), AddStream, ParallelCodeGenParallelismLevel,
                  std::move(Mod));
   }
-  return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
+  return Error::success();
 }
 
 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)) {

llvm/test/LTO/Resolution/X86/dead-strip-fulllto.ll

 ; RUN: opt -module-summary -o %t %s
 ; RUN: opt -module-summary -o %t2 %S/Inputs/dead-strip-fulllto.ll
 
-; RUN: llvm-lto2 run %t -r %t,main,px -r %t,live1, -r %t,live2,p -r %t,dead2,p \
+; RUN: llvm-lto2 run --pass-remarks-output=%t4.yaml --pass-remarks-filter=. \
+; RUN:               %t -r %t,main,px -r %t,live1, -r %t,live2,p -r %t,dead2,p \
 ; RUN:               %t2 -r %t2,live1,p -r %t2,live2, -r %t2,dead1,p -r %t2,dead2, -r %t2,odr, \
-; RUN: -save-temps -o %t3
+; RUN:               -save-temps -o %t3
+
+; RUN: cat %t4.yaml | FileCheck %s -check-prefix=REMARK
 ; RUN: llvm-nm %t3.0 | FileCheck --check-prefix=FULL %s
 ; RUN: llvm-nm %t3.1 | FileCheck --check-prefix=THIN %s
 
 ; RUN: llvm-lto2 run %t -r %t,main,px -r %t,live1, -r %t,live2,p -r %t,dead2,p \
 ; RUN:               %t2 -r %t2,live1,p -r %t2,live2, -r %t2,dead1,p -r %t2,dead2, -r %t2,odr, \
-; RUN: -save-temps -o %t3 -O0
+; RUN:               -save-temps -o %t3 -O0
 ; RUN: llvm-nm %t3.0 | FileCheck --check-prefix=FULL %s
 ; RUN: llvm-nm %t3.1 | FileCheck --check-prefix=THIN %s
 
+; REMARK:      Pass:            lto
+; REMARK-NEXT: Name:            deadfunction
+; REMARK-NEXT: Function:        dead2
+; REMARK-NEXT: Args:
+; REMARK-NEXT:   - Function:        dead2
+; REMARK-NEXT:   - String:          ' not added to the combined module '
+
 ; FULL-NOT: dead
 ; FULL: U live1
 ; FULL: T live2
 ; FULL: T main
 
 ; THIN-NOT: dead
 ; THIN: T live1
 ; THIN: U live2