[BOLT] Respect -function-order in lite mode

[Amir]

Summary

Process functions listed in -function-order file even in lite mode.

[maksfb]

bolt/lib/Rewrite/RewriteInstance.cpp

2759

Maybe use StringSet<>?

bolt/test/X86/function-order-lite.s

30–41

What's the purpose of the jumps in the test?

[Amir]

bolt/test/X86/function-order-lite.s

30–41

The test was copied from bolt/test/X86/Inputs/jmpjmp.s. Let me simplify the function to a stub

[Amir]

Use StringSet<>, update test

bolt/include/bolt/Passes/ReorderFunctions.h

enum ReorderType : char {
     RT_USER
   };
 
   explicit ReorderFunctions(const cl::opt<bool> &PrintPass)
       : BinaryFunctionPass(PrintPass) {}
 
   const char *getName() const override { return "reorder-functions"; }
   void runOnFunctions(BinaryContext &BC) override;
+
+  static std::vector<std::string> readFunctionOrderFile();
 };
 
 } // namespace bolt
 } // namespace llvm
 
 #endif // BOLT_PASSES_REORDER_FUNCTIONS_H

bolt/lib/Passes/ReorderFunctions.cpp

if (TotalCalls)
     outs() << format("BOLT-INFO:  Total Calls within 64B = %.0lf (%.2lf%%)\n",
                      TotalCalls64B, 100 * TotalCalls64B / TotalCalls)
            << format("BOLT-INFO:  Total Calls within 4KB = %.0lf (%.2lf%%)\n",
                      TotalCalls4KB, 100 * TotalCalls4KB / TotalCalls)
            << format("BOLT-INFO:  Total Calls within 2MB = %.0lf (%.2lf%%)\n",
                      TotalCalls2MB, 100 * TotalCalls2MB / TotalCalls);
 }
 
-namespace {
-
-std::vector<std::string> readFunctionOrderFile() {
+std::vector<std::string> ReorderFunctions::readFunctionOrderFile() {
   std::vector<std::string> FunctionNames;
   std::ifstream FuncsFile(opts::FunctionOrderFile, std::ios::in);
   if (!FuncsFile) {
     errs() << "Ordered functions file \"" << opts::FunctionOrderFile
            << "\" can't be opened.\n";
     exit(1);
   }
   std::string FuncName;
   while (std::getline(FuncsFile, FuncName))
     FunctionNames.push_back(FuncName);
   return FunctionNames;
 }
 
-}
-
 void ReorderFunctions::runOnFunctions(BinaryContext &BC) {
   auto &BFs = BC.getBinaryFunctions();
   if (opts::ReorderFunctions != RT_NONE &&
       opts::ReorderFunctions != RT_EXEC_COUNT &&
       opts::ReorderFunctions != RT_USER) {
     Cg = buildCallGraph(
         BC,
         [](const BinaryFunction &BF) {

bolt/lib/Rewrite/RewriteInstance.cpp

if (Index)
       --Index;
     LiteThresholdExecCount = TopFunctions[Index]->getKnownExecutionCount();
     outs() << "BOLT-INFO: limiting processing to functions with at least "
            << LiteThresholdExecCount << " invocations\n";
   }
   LiteThresholdExecCount = std::max(
       LiteThresholdExecCount, static_cast<uint64_t>(opts::LiteThresholdCount));
 
+  StringSet<> ReorderFunctionsUserSet;
+  if (opts::ReorderFunctions == ReorderFunctions::RT_USER) {
+    for (const std::string &Function :
+         ReorderFunctions::readFunctionOrderFile())
+      ReorderFunctionsUserSet.insert(Function);
+  }
+
   uint64_t NumFunctionsToProcess = 0;
   auto shouldProcess = [&](const BinaryFunction &Function) {
     if (opts::MaxFunctions && NumFunctionsToProcess > opts::MaxFunctions)
       return false;
 
     // If the list is not empty, only process functions from the list.
     if (!opts::ForceFunctionNames.empty() || !ForceFunctionsNR.empty()) {
       // Regex check (-funcs and -funcs-file options).
if (!opts::ForceFunctionNames.empty() || !ForceFunctionsNR.empty()) {
       return Match.has_value();
     }
 
     for (std::string &Name : opts::SkipFunctionNames)
       if (Function.hasNameRegex(Name))
         return false;
 
     if (opts::Lite) {
+      // Forcibly include functions specified in the -function-order file.
+      if (opts::ReorderFunctions == ReorderFunctions::RT_USER) {
+        Optional<StringRef> Match = Function.forEachName([&](StringRef Name) {
+          return ReorderFunctionsUserSet.contains(Name);
+        });
+        if (Match.has_value())
+          return true;
+      }
+
       if (ProfileReader && !ProfileReader->mayHaveProfileData(Function))
         return false;
 
       if (Function.getKnownExecutionCount() < LiteThresholdExecCount)
         return false;
     }
 
     return true;

bolt/test/X86/Inputs/order-lite.txt

+main

bolt/test/X86/function-order-lite.s

+# Check that functions listed in -function-order list take precedence over
+# lite mode function filtering.
+
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s -o %t.o
+# RUN: link_fdata %s %t.o %t.fdata
+# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q
+# RUN: llvm-bolt %t.exe --data %t.fdata --lite --reorder-functions=user \
+# RUN:   --function-order=%p/Inputs/order-lite.txt -o %t -print-all 2>&1 \
+# RUN:   | FileCheck %s
+
+# CHECK: 1 out of 2 functions in the binary (50.0%) have non-empty execution profile
+# CHECK: Binary Function "main" after reorder-functions
+
+  .globl main
+  .type main, %function
+main:
+  .cfi_startproc
+.LBB06:
+  callq testfunc
+  retq
+  .cfi_endproc
+.size main, .-main
+
+  .globl testfunc
+  .type testfunc, %function
+testfunc:
+# FDATA: 0 [unknown] 0 1 testfunc 0 1 0
+  .cfi_startproc
+  pushq %rbp
+  movq  %rsp, %rbp
+  movl  $0x0, %eax
+  popq  %rbp
+  retq
+  .cfi_endproc
+.size testfunc, .-testfunc