[llvm] [AArch64][BOLT] Ensure tentative code layout for cold BBs runs. (PR #96609)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-bolt

Author: Paschalis Mpeis (paschalis-mpeis)

<details>
<summary>Changes</summary>

When BOLT is not processing all functions (e.g., when lite mode is
enabled or when multiple functions are ignored via the 'skip-funcs' flag),
we might skip the tentative code layout estimation for cold basic blocks.
However, due to the `-split-functions`` flag, we still need to compute
PC-relative distances between hot and cold basic blocks.

In such cases, BOLT will use '0x0' as the first address of the basic
block, leading to incorrect estimations of the necessary PC-relative
addresses. Consequently, the relaxStub method expands all those branches
to the short-jump sequence unnecessarily.

Such an unnecessary expansion by `relaxStub` is from:
```armasm
b       .Ltmp1234
```

to:
```armasm
adrp    x16, .Ltmp1234
add     x16, x16, :lo12:.Ltmp1234
br      x16
```

---
Full diff: https://github.com/llvm/llvm-project/pull/96609.diff


2 Files Affected:

- (modified) bolt/lib/Passes/LongJmp.cpp (+11-3) 
- (added) bolt/test/AArch64/split-funcs-lite.test (+14) 


``````````diff
diff --git a/bolt/lib/Passes/LongJmp.cpp b/bolt/lib/Passes/LongJmp.cpp
index c483f70a836ee..053650f8da16c 100644
--- a/bolt/lib/Passes/LongJmp.cpp
+++ b/bolt/lib/Passes/LongJmp.cpp
@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "bolt/Passes/LongJmp.h"
+#include "bolt/Utils/CommandLineOpts.h"
 
 #define DEBUG_TYPE "longjmp"
 
@@ -324,7 +325,6 @@ uint64_t LongJmpPass::tentativeLayoutRelocColdPart(
 uint64_t LongJmpPass::tentativeLayoutRelocMode(
     const BinaryContext &BC, std::vector<BinaryFunction *> &SortedFunctions,
     uint64_t DotAddress) {
-
   // Compute hot cold frontier
   uint32_t LastHotIndex = -1u;
   uint32_t CurrentIndex = 0;
@@ -354,9 +354,12 @@ uint64_t LongJmpPass::tentativeLayoutRelocMode(
   for (BinaryFunction *Func : SortedFunctions) {
     if (!BC.shouldEmit(*Func)) {
       HotAddresses[Func] = Func->getAddress();
-      continue;
+      // Don't perform any tentative address estimation of a function's cold
+      // layout if it won't be emitted, unless we are ignoring a large number of
+      // functions (ie, on lite mode) and we haven't done such estimation yet.
+      if (opts::processAllFunctions() || ColdLayoutDone)
+        continue;
     }
-
     if (!ColdLayoutDone && CurrentIndex >= LastHotIndex) {
       DotAddress =
           tentativeLayoutRelocColdPart(BC, SortedFunctions, DotAddress);
@@ -382,6 +385,11 @@ uint64_t LongJmpPass::tentativeLayoutRelocMode(
     DotAddress += Func->estimateConstantIslandSize();
     ++CurrentIndex;
   }
+  if (!ColdLayoutDone) {
+    errs() << "BOLT-ERROR: Did not perform tentative code layout for cold functions.\n";
+    exit(1);
+  }
+
   // BBs
   for (BinaryFunction *Func : SortedFunctions)
     tentativeBBLayout(*Func);
diff --git a/bolt/test/AArch64/split-funcs-lite.test b/bolt/test/AArch64/split-funcs-lite.test
new file mode 100644
index 0000000000000..c7a11918fe061
--- /dev/null
+++ b/bolt/test/AArch64/split-funcs-lite.test
@@ -0,0 +1,14 @@
+// This test checks that tentative code layout for cold basic blocks runs
+// at least once, even when each function after the hot/cold frontier is not
+// emittable. This is done by ignoring each function using the 'skip-funcs' flag.
+// In a realistic scenario, this may happen when lite mode is enabled along
+// with a bolt profile.
+
+REQUIRES: system-linux
+
+RUN: %clang %cflags %p/../Inputs/asm_main.c -Wl,-q -o %t
+
+RUN: llvm-bolt %t -o %t.bolt -lite=1 -split-functions -split-all-cold \
+RUN: --skip-funcs="_init,_start,call_weak_fn/1,deregister_tm_clones/1,register_tm_clones/1,__do_global_dtors_aux/1,frame_dummy/1,main,foo,_fini" 2>&1 | FileCheck %s
+
+CHECK-NOT: BOLT-ERROR: Did not perform tentative code layout for cold functions.
\ No newline at end of file

``````````

</details>


https://github.com/llvm/llvm-project/pull/96609