[BOLT] Fix jump table issue for split functions

A jump table in a split function may contain an entry matching a start
address of another fragment of the function. While converting addresses
to labels, we used to ignore such entries resulting in underpopulated
jump table. Change that, so we always create one label per address.

Reviewed By: Amir

Differential Revision: https://reviews.llvm.org/D156013

diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 609da25..5b44a76 100644 (file)
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1639,16 +1639,20 @@ void BinaryFunction::postProcessJumpTables() {
         JT.Entries.push_back(Label);
         continue;
       }
-      // Create local label for targets cannot be reached by other fragments
-      // Otherwise, secondary entry point to target function
+      // Create a local label for targets that cannot be reached by other
+      // fragments. Otherwise, create a secondary entry point in the target
+      // function.
       BinaryFunction *TargetBF =
           BC.getBinaryFunctionContainingAddress(EntryAddress);
-      if (uint64_t Offset = EntryAddress - TargetBF->getAddress()) {
-        MCSymbol *Label = (HasOneParent && TargetBF == this)
-                              ? getOrCreateLocalLabel(EntryAddress, true)
-                              : TargetBF->addEntryPointAtOffset(Offset);
-        JT.Entries.push_back(Label);
+      MCSymbol *Label;
+      if (HasOneParent && TargetBF == this) {
+        Label = getOrCreateLocalLabel(EntryAddress, true);
+      } else {
+        const uint64_t Offset = EntryAddress - TargetBF->getAddress();
+        Label = Offset ? TargetBF->addEntryPointAtOffset(Offset)
+                       : TargetBF->getSymbol();
       }
+      JT.Entries.push_back(Label);
     }
   }
 

diff --git a/bolt/test/X86/split-func-jump-table-fragment.s b/bolt/test/X86/split-func-jump-table-fragment.s
index 6b6dc22..a92e673 100644 (file)
--- a/bolt/test/X86/split-func-jump-table-fragment.s
+++ b/bolt/test/X86/split-func-jump-table-fragment.s
@@ -1,12 +1,13 @@
-# This reproduces a bug with jump table identification where jump table has
-# entries pointing to code in function and its cold fragment.
+## This reproduces a bug with jump table identification where jump table has
+## entries pointing to code in function and its cold fragment.
 
 # 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 --lite=0 -v=1 --print-cfg --print-only=main \
+# RUN:   2>&1 | FileCheck %s
 
 # CHECK-NOT: unclaimed PC-relative relocations left in data
 # CHECK: BOLT-INFO: marking main.cold.1 as a fragment of main
@@ -36,13 +37,14 @@ LBB3:
   ret
 .size main, .-main
 
+# Insert padding between functions, so that the next instruction cannot be
+# treated as __builtin_unreachable destination for the jump table.
+  .quad 0
+
   .globl main.cold.1
   .type main.cold.1, %function
   .p2align 2
 main.cold.1:
-  # load bearing nop: pad LBB4 so that it can't be treated
-  # as __builtin_unreachable by analyzeJumpTable
-  nop
 LBB4:
   callq abort
 .size main.cold.1, .-main.cold.1
@@ -55,3 +57,12 @@ JUMP_TABLE:
   .long LBB3-JUMP_TABLE
   .long LBB4-JUMP_TABLE
   .long LBB3-JUMP_TABLE
+
+## Verify that the entry corresponding to the cold fragment was added to
+## the jump table.
+
+# CHECK:      PIC Jump table
+# CHECK-NEXT: 0x{{.*}} :
+# CHECK-NEXT: 0x{{.*}} :
+# CHECK-NEXT: 0x{{.*}} : main.cold.1
+# CHECK-NEXT: 0x{{.*}} :