[llvm] [BOLT] Don't remove nops in non-simple functions (PR #101964)

[Vladislav Khmelevsky via llvm-commits]

https://github.com/yota9 updated https://github.com/llvm/llvm-project/pull/101964

>From 49f6f4c8819f86541c7f1032beb47a964c549935 Mon Sep 17 00:00:00 2001
From: Vladislav Khmelevsky <och95 at yandex.ru>
Date: Mon, 5 Aug 2024 15:57:06 +0400
Subject: [PATCH] [BOLT] Don't remove nops in non-simple functions

Follow the logic of https://reviews.llvm.org/D143887 patch (fixed later
by #71377) we don't want to remove nops in non-simple function just in
case there is undetected jump table to minimize chances to break offsets
in it.
---
 bolt/lib/Passes/BinaryPasses.cpp             |  3 +-
 bolt/test/AArch64/non-simple-nops-preserve.s | 38 ++++++++++++++++++++
 bolt/test/RISCV/reloc-label-diff.s           |  9 ++---
 3 files changed, 45 insertions(+), 5 deletions(-)
 create mode 100644 bolt/test/AArch64/non-simple-nops-preserve.s

diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp
index fa95ad7324ac1..8401658336daa 100644
--- a/bolt/lib/Passes/BinaryPasses.cpp
+++ b/bolt/lib/Passes/BinaryPasses.cpp
@@ -15,6 +15,7 @@
 #include "bolt/Core/ParallelUtilities.h"
 #include "bolt/Passes/ReorderAlgorithm.h"
 #include "bolt/Passes/ReorderFunctions.h"
+#include "bolt/Utils/CommandLineOpts.h"
 #include "llvm/Support/CommandLine.h"
 #include <atomic>
 #include <mutex>
@@ -1923,7 +1924,7 @@ Error RemoveNops::runOnFunctions(BinaryContext &BC) {
   };
 
   ParallelUtilities::PredicateTy SkipFunc = [&](const BinaryFunction &BF) {
-    return BF.shouldPreserveNops();
+    return BF.shouldPreserveNops() || (!opts::StrictMode && !BF.isSimple());
   };
 
   ParallelUtilities::runOnEachFunction(
diff --git a/bolt/test/AArch64/non-simple-nops-preserve.s b/bolt/test/AArch64/non-simple-nops-preserve.s
new file mode 100644
index 0000000000000..9e0fdad6f105a
--- /dev/null
+++ b/bolt/test/AArch64/non-simple-nops-preserve.s
@@ -0,0 +1,38 @@
+# This test checks that the nop instruction is preserved before the
+# ADRRelaxation pass. Otherwise the ADRRelaxation pass would
+# fail to replace nop+adr to adrp+add for non-simple function.
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown \
+# RUN:   %s -o %t.o
+# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q
+# RUN: llvm-bolt %t.exe -o %t.bolt --adr-relaxation=true \
+# RUN:   --print-cfg --print-only=_start | FileCheck %s
+# RUN: llvm-objdump -d -j .text %t.bolt | \
+# RUN:   FileCheck --check-prefix=DISASMCHECK %s
+
+# CHECK: Binary Function "_start" after building cfg
+# CHECK: IsSimple : 0
+
+# DISASMCHECK: {{.*}}<_start>:
+# DISASMCHECK-NEXT: adrp
+# DISASMCHECK-NEXT: add
+# DISASMCHECK-NEXT: adr
+
+  .text
+  .align 4
+  .global test
+  .type test, %function
+test:
+  ret
+  .size test, .-test
+
+  .global _start
+  .type _start, %function
+_start:
+  nop
+  adr x0, test
+  adr x1, 1f
+1:
+  mov x1, x0
+  br x0
+  .size _start, .-_start
diff --git a/bolt/test/RISCV/reloc-label-diff.s b/bolt/test/RISCV/reloc-label-diff.s
index 9ad38f86ff541..7d8c7c8772303 100644
--- a/bolt/test/RISCV/reloc-label-diff.s
+++ b/bolt/test/RISCV/reloc-label-diff.s
@@ -1,5 +1,5 @@
 // RUN: %clang %cflags -o %t %s
-// RUN: llvm-bolt -o %t.bolt %t
+// RUN: llvm-bolt --strict -o %t.bolt %t
 // RUN: llvm-readelf -x .data %t.bolt | FileCheck %s
 
   .text
@@ -9,15 +9,16 @@
 _start:
   // Force BOLT into relocation mode
   .reloc 0, R_RISCV_NONE
-  // BOLT removes this nop so the label difference is initially 8 but should be
-  // 4 after BOLT processes it.
+  // BOLT removes this nop so the label difference is initially 12 but should be
+  // 8 after BOLT processes it.
   nop
   beq x0, x0, _test_end
+  addi x1, x1, 1
 _test_end:
   ret
   .size _start, .-_start
 
   .data
 // CHECK: Hex dump of section '.data':
-// CHECK: 0x{{.*}} 04000000
+// CHECK: 0x{{.*}} 08000000
   .word _test_end - _start