[BOLT][Linux] Skip some functions

Skip some functions to simplify things, making BOLT for Linux more reliable
and saving development efforts.

We use "-bolt-function-list-file" to gather a list of C function when building
Linux kernel, then BOLT can choose to optimize them only.

BOLT can not handle some functions defined in assembly code reliably, since
they may have extra semantics/enforcements BOLT can never know. For example,
irq_entries_start defined in arch/x86/include/asm/idtentry.h is actually an
“array” but BOLT views it as an ordinary function. If BOLT applies basic
block reordering, instrumentation, etc to it, run time errors would happen.

We could explicitly specify those functions to skip, but to find all of them
we usually need to test & debug many times. That is a lot of work and we may
still miss some.

In my own experience, when adapting BOLT for Linux to a new architecture or
Linux version, we may spend lots of time on fixing runtime errors related to
functions defined in assembly code.

Only handling C functions makes BOLT for Linux more reliable, and may save
lots of development efforts.

"-bolt-function-list-file" can be used as follows:

* put "-mllvm -bolt-function-list-file=filename" in KCFLAGS when
  buildling Linux
* specify "-funcs-file-no-regex=filename" when invoking llvm-bolt

Some functions's address are used in add/sub/comparision. To make things
simple, they should keep their address (after relocation mode is
supported).

See __bpf_call_base in kernel/bpf/core.c for an example.

Currently, we just skip them.

"bolt-keep-address-function-list-file" can be used as follows:

* put "-mllvm -bolt-keep-address-function-list-file=filename" in
  KCFLAGS when buildling Linux
* specify "-keep-address-funcs-file-no-regex=filename" when
  invoking llvm-bolt

Functions in sections other than .text (e.g. .head.text, .init.text,
.exit.text) are (mostly) run during initialization and shutdown, and
not (much) relevant to application performance.

Skipping them also helps to avoid runtime errors, especially those
even before the first message is printed out, which are not easy to
debug.

Only handling functions in .text also make sure no function is moved to
a different section (after relocation mode is supported). Linux kernel
code may compare function pointers with section boundary symbols, and
if we move functions to another section, runtime errors may happen.

Author: FLZ101

bolt/include/bolt/Core/BinaryContext.h

@@ -433,6 +433,13 @@ class BinaryContext {
         Address);
   }
 
+  bool isInRange(StringRef NameStart, StringRef NameEnd,
+                 uint64_t Address) const {
+    ErrorOr<uint64_t> Start = getSymbolValue(NameStart);
+    ErrorOr<uint64_t> End = getSymbolValue(NameEnd);
+    return Start && End && *Start <= Address && Address < *End;
+  }
+
   /// Return size of an entry for the given jump table \p Type.
   uint64_t getJumpTableEntrySize(JumpTable::JumpTableType Type) const {
     return Type == JumpTable::JTT_PIC ? 4 : AsmInfo->getCodePointerSize();
@@ -911,7 +918,11 @@ class BinaryContext {
   /// Return a value of the global \p Symbol or an error if the value
   /// was not set.
   ErrorOr<uint64_t> getSymbolValue(const MCSymbol &Symbol) const {
-    const BinaryData *BD = getBinaryDataByName(Symbol.getName());
+    return getSymbolValue(Symbol.getName());
+  }
+
+  ErrorOr<uint64_t> getSymbolValue(StringRef Name) const {
+    const BinaryData *BD = getBinaryDataByName(Name);
     if (!BD)
       return std::make_error_code(std::errc::bad_address);
     return BD->getAddress();

bolt/include/bolt/Core/BinaryFunction.h

@@ -294,6 +294,9 @@ class BinaryFunction {
   /// Pseudo functions should not be disassembled or emitted.
   bool IsPseudo{false};
 
+  // True if address of this function can not be changed
+  bool KeepAddress{false};
+
   /// True if the original function code has all necessary relocations to track
   /// addresses of functions emitted to new locations. Typically set for
   /// functions that we are not going to emit.
@@ -1318,6 +1321,9 @@ class BinaryFunction {
   /// otherwise processed.
   bool isPseudo() const { return IsPseudo; }
 
+  /// Return true if address of this function can not be changed
+  bool mustKeepAddress() const { return KeepAddress; }
+
   /// Return true if the function contains explicit or implicit indirect branch
   /// to its split fragments, e.g., split jump table, landing pad in split
   /// fragment.

bolt/lib/Rewrite/LinuxKernelRewriter.cpp

@@ -351,6 +351,33 @@ class LinuxKernelRewriter final : public MetadataRewriter {
     if (Error E = detectLinuxKernelVersion())
       return E;
 
+    auto ShouldIgnore = [this](const BinaryFunction &Function) {
+      std::optional<StringRef> SectionName = Function.getOriginSectionName();
+      if (!SectionName || *SectionName != ".text")
+        return true;
+
+      uint64_t Address = Function.getAddress();
+
+      if (BC.isX86()) {
+        BinaryData *BDStart = BC.getBinaryDataByName("irq_entries_start");
+        if (BDStart && BDStart->containsAddress(Address))
+          return true;
+
+        if (BC.isInRange("__static_call_text_start", "__static_call_text_end",
+                         Address))
+          return true;
+      }
+
+      if (BC.isInRange("__noinstr_text_start", "__noinstr_text_end", Address))
+        return true;
+
+      return false;
+    };
+
+    for (BinaryFunction *Function : BC.getAllBinaryFunctions())
+      if (ShouldIgnore(*Function))
+        Function->setIgnored();
+
     processLKSections();
 
     if (Error E = processSMPLocks())

bolt/lib/Rewrite/RewriteInstance.cpp

@@ -137,6 +137,16 @@ static cl::opt<std::string> FunctionNamesFileNR(
     cl::desc("file with list of functions to optimize (non-regex)"), cl::Hidden,
     cl::cat(BoltCategory));
 
+static cl::list<std::string> KeepAddressFunctionNamesNR(
+    "keep-address-funcs-no-regex", cl::CommaSeparated,
+    cl::desc("KeepAddress functions from the list (non-regex)"),
+    cl::value_desc("func1,func2,func3,..."), cl::Hidden, cl::cat(BoltCategory));
+
+static cl::opt<std::string> KeepAddressFunctionNamesFileNR(
+    "keep-address-funcs-file-no-regex",
+    cl::desc("file with list of KeepAddress functions to optimize (non-regex)"),
+    cl::Hidden, cl::cat(BoltCategory));
+
 cl::opt<bool>
 KeepTmp("keep-tmp",
   cl::desc("preserve intermediate .o file"),
@@ -2982,6 +2992,8 @@ void RewriteInstance::selectFunctionsToProcess() {
   populateFunctionNames(opts::FunctionNamesFile, opts::ForceFunctionNames);
   populateFunctionNames(opts::SkipFunctionNamesFile, opts::SkipFunctionNames);
   populateFunctionNames(opts::FunctionNamesFileNR, opts::ForceFunctionNamesNR);
+  populateFunctionNames(opts::KeepAddressFunctionNamesFileNR,
+                        opts::KeepAddressFunctionNamesNR);
 
   // Make a set of functions to process to speed up lookups.
   std::unordered_set<std::string> ForceFunctionsNR(
@@ -2996,6 +3008,10 @@ void RewriteInstance::selectFunctionsToProcess() {
     exit(1);
   }
 
+  std::unordered_set<std::string> KeepAddressFunctionsNR(
+      opts::KeepAddressFunctionNamesNR.begin(),
+      opts::KeepAddressFunctionNamesNR.end());
+
   uint64_t LiteThresholdExecCount = 0;
   if (opts::LiteThresholdPct) {
     if (opts::LiteThresholdPct > 100)
@@ -3043,7 +3059,8 @@ void RewriteInstance::selectFunctionsToProcess() {
     for (std::string &Name : opts::SkipFunctionNames)
       if (Function.hasNameRegex(Name))
         return true;
-
+    if (Function.mustKeepAddress())
+      return true;
     return false;
   };
 
@@ -3094,6 +3111,10 @@ void RewriteInstance::selectFunctionsToProcess() {
   for (auto &BFI : BC->getBinaryFunctions()) {
     BinaryFunction &Function = BFI.second;
 
+    for (const StringRef Name : Function.getNames())
+      if (KeepAddressFunctionsNR.count(Name.str()))
+        Function.KeepAddress = true;
+
     // Pseudo functions are explicitly marked by us not to be processed.
     if (Function.isPseudo()) {
       Function.IsIgnored = true;

llvm/lib/CodeGen/CodeGenPrepare.cpp

@@ -85,6 +85,7 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
@@ -274,6 +275,14 @@ static cl::opt<bool>
     DisableDeletePHIs("disable-cgp-delete-phis", cl::Hidden, cl::init(false),
                       cl::desc("Disable elimination of dead PHI nodes."));
 
+cl::opt<std::string>
+    BoltFunctionListFile("bolt-function-list-file", cl::Hidden,
+                         cl::desc("Specify BOLT function list file"));
+
+cl::opt<std::string> BoltKeepAddressFunctionListFile(
+    "bolt-keep-address-function-list-file", cl::Hidden,
+    cl::desc("Specify BOLT KeepAddress function list file"));
+
 namespace {
 
 enum ExtType {
@@ -505,7 +514,43 @@ class CodeGenPrepareLegacyPass : public FunctionPass {
 
 char CodeGenPrepareLegacyPass::ID = 0;
 
+template <typename T> void GatherForBoltKA(raw_fd_ostream &OS, T &I) {
+  switch (I.getOpcode()) {
+  case Instruction::ICmp:
+  case Instruction::PtrToInt:
+    for (Use &U : I.operands())
+      if (auto *FF = dyn_cast<Function>(U.get()))
+        OS << FF->getName() << "\n";
+    break;
+  default:;
+  }
+  for (Use &U : I.operands())
+    if (auto *CE = dyn_cast<ConstantExpr>(U.get()))
+      GatherForBoltKA(OS, *CE);
+}
+
 bool CodeGenPrepareLegacyPass::runOnFunction(Function &F) {
+  if (!BoltFunctionListFile.empty()) {
+    std::error_code EC;
+    raw_fd_ostream OS(BoltFunctionListFile, EC, sys::fs::OpenFlags::OF_Append);
+    if (EC)
+      report_fatal_error(Twine(BoltFunctionListFile) + ": " + EC.message());
+    OS << F.getName() << "\n";
+  }
+
+  if (!BoltKeepAddressFunctionListFile.empty()) {
+    std::error_code EC;
+    raw_fd_ostream OS(BoltKeepAddressFunctionListFile, EC,
+                      sys::fs::OpenFlags::OF_Append);
+    if (EC)
+      report_fatal_error(Twine(BoltKeepAddressFunctionListFile) + ": " +
+                         EC.message());
+
+    for (BasicBlock &BB : F)
+      for (Instruction &I : BB)
+        GatherForBoltKA(OS, I);
+  }
+
   if (skipFunction(F))
     return false;
   auto TM = &getAnalysis<TargetPassConfig>().getTM<TargetMachine>();