[PATCH] D159285: [BOLT][RISCV] Prevent adding secondary entry points for BB labels

[Job Noorman via Phabricator via llvm-commits]

jobnoorman created this revision.
jobnoorman added reviewers: rafauler, maksfb, yota9, Amir.
Herald added subscribers: asb, luke, treapster, sunshaoce, pmatos, ayermolo, VincentWu, vkmr, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, arichardson.
Herald added a project: All.
jobnoorman requested review of this revision.
Herald added subscribers: llvm-commits, wangpc, eopXD, MaskRay.
Herald added a project: LLVM.

When linker relaxation is enabled, every branch has a relocation and a
corresponding symbol in the symbol table. BOLT currently registers all
these symbols as secondary entry points causing almost every function to
be marked as multi entry on RISC-V.

This patch modifies `adjustFunctionBoundaries` to ignore these symbols.
Note that I currently try to detect them by checking if a symbol's name
starts with the private label prefix as defined by `MCAsmInfo`. Since
I'm not entirely sure what multi-entry functions look like on different
targets, please check if this condition is correct. Maybe it could make
sense to only check this on RISC-V?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159285

Files:
  bolt/lib/Rewrite/RewriteInstance.cpp
  bolt/test/RISCV/branch-no-secondary-entry.s


Index: bolt/test/RISCV/branch-no-secondary-entry.s
===================================================================
--- /dev/null
+++ bolt/test/RISCV/branch-no-secondary-entry.s
@@ -0,0 +1,18 @@
+/// Test that no secondary entry points are created for basic block labels used
+/// by branches.
+// RUN: %clang %cflags -o %t %s
+// RUN: llvm-bolt -print-cfg -o /dev/null %t 2>&1 | FileCheck %s
+
+// CHECK: Binary Function "_start" after building cfg {
+// CHECK: IsMultiEntry: 0
+// CHECK: beq t0, t1, .Ltmp0
+// CHECK: {{^}}.Ltmp0
+// CHECK: ret
+
+    .globl _start
+_start:
+    beq t0, t1, 1f
+1:
+    ret
+    .size _start, .-_start
+
Index: bolt/lib/Rewrite/RewriteInstance.cpp
===================================================================
--- bolt/lib/Rewrite/RewriteInstance.cpp
+++ bolt/lib/Rewrite/RewriteInstance.cpp
@@ -1586,6 +1586,15 @@
       if (!Function.isSymbolValidInScope(Symbol, SymbolSize))
         break;
 
+      // Skip basic block labels. This happens on RISC-V with linker relaxation
+      // enabled because every branch needs a relocation and corresponding
+      // symbol. We don't want to add such symbols as entry points.
+      if (cantFail(Symbol.getName())
+              .starts_with(BC->AsmInfo->getPrivateLabelPrefix())) {
+        ++NextSymRefI;
+        continue;
+      }
+
       // This is potentially another entry point into the function.
       uint64_t EntryOffset = NextSymRefI->first - Function.getAddress();
       LLVM_DEBUG(dbgs() << "BOLT-DEBUG: adding entry point to function "


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D159285.555042.patch
Type: text/x-patch
Size: 1545 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230831/67c28aa8/attachment.bin>