[BOLT] Mark fragments related to split jump table as non-simple

[nhuhuan]

Summary

Mark fragments related to split jump table as non-simple.

A function could be splitted into hot and cold fragments. A split jump table is
challenging for correctly reconstructing control flow graphs, so it was marked
as ignored. This update marks those fragments as non-simple, allowing them
to be printed and partial control flow graph construction.

Test Plan:

llvm-lit -a tools/bolt/test/X86/split-func-icf.s

This test has two functions (main, main2), each has a jump table target to the
same cold portion main2.cold.1(*2). We try to print out only this cold portion.
If it is ignored, it cannot be printed. If it is non-simple, it can be printed. We
verify that it can be printed.

[nhuhuan]

Update the warning messages

[nhuhuan]

update FileCheck rules

[Amir]

Please address test crashes: https://buildkite.com/llvm-project/premerge-checks/builds/97009#01814b67-0bb2-48c7-b45e-ed4f32025c12/494-3664

[nhuhuan]

Propagate hasSplitJumpTable property to all relevant fragments.
Update assertion to address split jump table cases.

[Amir]

bolt/lib/Core/BinaryContext.cpp

709

This lambda is for adding to a worklist and filtering out repetitions. Let's keep it this way and move this line down to setSimple

[nhuhuan]

Move the propagation of hasSplitJumpTable out of the lambda.

[Amir]

Please update the test plan (test name).
LGTM otherwise

[nhuhuan]

In D127464#3574478, @Amir wrote:

Please address test crashes: https://buildkite.com/llvm-project/premerge-checks/builds/97009#01814b67-0bb2-48c7-b45e-ed4f32025c12/494-3664

Fixed.
The assertion guarantees that non-simple functions are either an empty function or has no relocation, which is not the case for split jump table.
We propagate hasSplitJumpTable to all fragments that can reach or be reached by the fragment with split jump table. Then we use this property to make an exception for the assertion.

Example:

FragmentA:

• has split jump table target FragmentA.cold

FragmentB:

• has direct jump to FragmentA.cold

FragmentA.cold.1:

• some code

Initially, the property HasSplitJumpTable of FragmentA is set to true.
Propagate to FragmentA.cold.1, so the property of FragmentA.cold.1 is also set to true.
FragmentB, despite not having split jump table, is also set to true because it reaches the non-simple fragment FragmentA.cold.1.

[hzq]

@nhuhuan Hello, see issue 62307, add this patch in the use of a problem, can you help to see?

[hzq]

@nhuhuan

bolt/lib/Core/BinaryContext.cpp

std::unordered_set<BinaryFunction *> UniqueFunctions(FragmentsToSkip.begin(),
                                                        FragmentsToSkip.end());
   // Copy the functions back to FragmentsToSkip.
   FragmentsToSkip.assign(UniqueFunctions.begin(), UniqueFunctions.end());
   auto addToWorklist = [&](BinaryFunction *Function) -> void {
     if (UniqueFunctions.count(Function))
       return;
     FragmentsToSkip.push_back(Function);
     UniqueFunctions.insert(Function);
   };
   // Functions containing split jump tables need to be skipped with all
   // fragments (transitively).
   for (size_t I = 0; I != FragmentsToSkip.size(); I++) {
     BinaryFunction *BF = FragmentsToSkip[I];
     assert(UniqueFunctions.count(BF) &&
            "internal error in traversing function fragments");
     if (opts::Verbosity >= 1)
       errs() << "BOLT-WARNING: Ignoring " << BF->getPrintName() << '\n';
-    BF->setIgnored();
+    BF->setSimple(false);
+    BF->setHasSplitJumpTable(true);
+
     std::for_each(BF->Fragments.begin(), BF->Fragments.end(), addToWorklist);
     std::for_each(BF->ParentFragments.begin(), BF->ParentFragments.end(),
                   addToWorklist);
   }
   if (!FragmentsToSkip.empty())
-    errs() << "BOLT-WARNING: ignored " << FragmentsToSkip.size() << " function"
+    errs() << "BOLT-WARNING: skipped " << FragmentsToSkip.size() << " function"
            << (FragmentsToSkip.size() == 1 ? "" : "s")
            << " due to cold fragments\n";
   FragmentsToSkip.clear();
 }
 
 MCSymbol *BinaryContext::getOrCreateGlobalSymbol(uint64_t Address, Twine Prefix,
                                                  uint64_t Size,
                                                  uint16_t Alignment,

bolt/lib/Rewrite/RewriteInstance.cpp

void RewriteInstance::disassembleFunctions() {
 
   for (auto &BFI : BC->getBinaryFunctions()) {
     BinaryFunction &Function = BFI.second;
 
     if (!shouldDisassemble(Function))
       continue;
 
     if (!Function.isSimple()) {
-      assert((!BC->HasRelocations || Function.getSize() == 0) &&
+      assert((!BC->HasRelocations || Function.getSize() == 0 ||
+              Function.hasSplitJumpTable()) &&
              "unexpected non-simple function in relocation mode");
       continue;
     }
 
     // Fill in CFI information for this function
     if (!Function.trapsOnEntry() && !CFIRdWrt->fillCFIInfoFor(Function)) {
       if (BC->HasRelocations) {
         BC->exitWithBugReport("unable to fill CFI.", Function);

bolt/test/X86/split-func-icf.s

 # This reproduces an issue where two cold fragments are folded into one, so the
 # fragment has two parents.
 # The fragment is only reachable through a jump table, so all functions must be
 # ignored.
 
 # REQUIRES: system-linux
 
 # RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s -o %t.o
 # RUN: llvm-strip --strip-unneeded %t.o
 # RUN: %clang %cflags %t.o -o %t.exe -Wl,-q
-# RUN: llvm-bolt %t.exe -o %t.out -lite=0 -v=1 2>&1 | FileCheck %s
+# RUN: llvm-bolt %t.exe -o %t.out -v=1 --print-only=main2.cold.1 --print-disasm 2>&1 | FileCheck %s
 
 # CHECK-NOT: unclaimed PC-relative relocations left in data
 # CHECK-DAG: BOLT-INFO: marking main2.cold.1(*2) as a fragment of main
 # CHECK-DAG: BOLT-INFO: marking main2.cold.1(*2) as a fragment of main2
+# CHECK: Binary Function "main2.cold.1(*2)" after disassembly
+# CHECK: End of Function "main2.cold.1(*2)"
 # CHECK-DAG: BOLT-WARNING: Ignoring main2
 # CHECK-DAG: BOLT-WARNING: Ignoring main
 # CHECK-DAG: BOLT-WARNING: Ignoring main2.cold.1(*2)
-# CHECK: BOLT-WARNING: ignored 3 functions due to cold fragments
+# CHECK: BOLT-WARNING: skipped 3 functions due to cold fragments
   .text
   .globl main
   .type main, %function
   .p2align 2
 main:
 LBB0:
   andl $0xf, %ecx
   cmpb $0x4, %cl