[llvm] [BOLT] Run PatchEntries pass before LongJmp (PR #137236)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-bolt

Author: Maksim Panchenko (maksfb)

<details>
<summary>Changes</summary>

With --force-patch option, every original function entry point is overwritten with a trampoline to a new version of the function to prevent the execution of the original code.

If the function size is too small for the trampoline code, we are forced to bail out on rewriting the function. That presented a problem on AArch64 due to LongJmp pass that assumed the presence of the new copy of the function. If the new copy was not emitted it could have lead to a relocation overflow.

Run PatchEntries pass before LongJmp and make the latter aware of the functions that are not going to be emitted. Make --force-patch option behavior on AArch64 consistent with other architectures.

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


2 Files Affected:

- (modified) bolt/lib/Passes/PatchEntries.cpp (+1-12) 
- (modified) bolt/lib/Rewrite/BinaryPassManager.cpp (+4-4) 


``````````diff
diff --git a/bolt/lib/Passes/PatchEntries.cpp b/bolt/lib/Passes/PatchEntries.cpp
index 8a2f0a39a56cc..4877e7dd8fdf3 100644
--- a/bolt/lib/Passes/PatchEntries.cpp
+++ b/bolt/lib/Passes/PatchEntries.cpp
@@ -98,21 +98,10 @@ Error PatchEntries::runOnFunctions(BinaryContext &BC) {
     });
 
     if (!Success) {
-      // We can't change output layout for AArch64 due to LongJmp pass
-      if (BC.isAArch64()) {
-        if (opts::ForcePatch) {
-          BC.errs() << "BOLT-ERROR: unable to patch entries in " << Function
-                    << "\n";
-          return createFatalBOLTError("");
-        }
-
-        continue;
-      }
-
       // If the original function entries cannot be patched, then we cannot
       // safely emit new function body.
       BC.errs() << "BOLT-WARNING: failed to patch entries in " << Function
-                << ". The function will not be optimized.\n";
+                << ". The function will not be optimized\n";
       Function.setIgnored();
       continue;
     }
diff --git a/bolt/lib/Rewrite/BinaryPassManager.cpp b/bolt/lib/Rewrite/BinaryPassManager.cpp
index dd48653931eb9..996d2e972599d 100644
--- a/bolt/lib/Rewrite/BinaryPassManager.cpp
+++ b/bolt/lib/Rewrite/BinaryPassManager.cpp
@@ -497,6 +497,10 @@ Error BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
   // memory profiling data.
   Manager.registerPass(std::make_unique<ReorderData>());
 
+  // Patch original function entries
+  if (BC.HasRelocations)
+    Manager.registerPass(std::make_unique<PatchEntries>());
+
   if (BC.isAArch64()) {
     Manager.registerPass(
         std::make_unique<ADRRelaxationPass>(PrintAdrRelaxation));
@@ -524,10 +528,6 @@ Error BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
   // Assign each function an output section.
   Manager.registerPass(std::make_unique<AssignSections>());
 
-  // Patch original function entries
-  if (BC.HasRelocations)
-    Manager.registerPass(std::make_unique<PatchEntries>());
-
   // This pass turns tail calls into jumps which makes them invisible to
   // function reordering. It's unsafe to use any CFG or instruction analysis
   // after this point.

``````````

</details>


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