Skip to content

Conversation

aaupov
Copy link
Contributor

@aaupov aaupov commented Feb 21, 2025

BOLT used to mark multi-entry functions non-simple in non-relocation
mode with the reasoning that we can't move them due to potentially
undetected references. However, in aggregation mode it doesn't apply as
BOLT doesn't perform optimizations.

Relax this constraint in case of an aggregation job.

Test Plan: added entry-point-fallthru.s

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Feb 22, 2025

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

BOLT used to mark multi-entry functions non-simple in non-relocation
mode with the reasoning that we can't move them due to potentially
undetected references. However, in aggregation mode it doesn't apply as
BOLT doesn't perform optimizations.

Relax this constraint in case of an aggregation job.

Test Plan: added entry-point-fallthru.s


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

2 Files Affected:

  • (modified) bolt/lib/Core/BinaryFunction.cpp (+3-2)
  • (added) bolt/test/X86/entry-point-fallthru.s (+24)
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 8385551576098..ff5eb5cf6e1eb 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -15,6 +15,7 @@
 #include "bolt/Core/DynoStats.h"
 #include "bolt/Core/HashUtilities.h"
 #include "bolt/Core/MCPlusBuilder.h"
+#include "bolt/Utils/CommandLineOpts.h"
 #include "bolt/Utils/NameResolver.h"
 #include "bolt/Utils/NameShortener.h"
 #include "bolt/Utils/Utils.h"
@@ -1753,8 +1754,8 @@ void BinaryFunction::postProcessEntryPoints() {
     // In non-relocation mode there's potentially an external undetectable
     // reference to the entry point and hence we cannot move this entry
     // point. Optimizing without moving could be difficult.
-    // In BAT mode, register any known entry points for CFG construction.
-    if (!BC.HasRelocations && !BC.HasBATSection)
+    // In aggregation, register any known entry points for CFG construction.
+    if (!BC.HasRelocations && !opts::AggregateOnly)
       setSimple(false);
 
     const uint32_t Offset = KV.first;
diff --git a/bolt/test/X86/entry-point-fallthru.s b/bolt/test/X86/entry-point-fallthru.s
new file mode 100644
index 0000000000000..edf14247b0c43
--- /dev/null
+++ b/bolt/test/X86/entry-point-fallthru.s
@@ -0,0 +1,24 @@
+## Checks that fallthroughs spanning entry points are accepted in aggregation
+## mode.
+
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s -o %t.o
+# RUN: ld.lld %t.o -o %t
+# RUN: link_fdata %s %t %t.preagg PREAGG
+# RUN: perf2bolt %t -p %t.preagg --pa -o %t.fdata | FileCheck %s
+# CHECK: traces mismatching disassembled function contents: 0
+
+	.globl main
+main:
+	.cfi_startproc
+	vmovaps %zmm31,%zmm3
+
+next:
+	add    $0x4,%r9
+	add    $0x40,%r10
+	dec    %r14
+Ljmp:
+	jne    main
+# PREAGG: T #Ljmp# #main# #Ljmp# 1
+	ret
+	.cfi_endproc
+.size main,.-main

Copy link
Member

@dcci dcci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable, as long as it doesn't break internal binaries.

@aaupov aaupov merged commit 3968ebd into main Feb 25, 2025
12 checks passed
@aaupov aaupov deleted the users/aaupov/spr/bolt-keep-multi-entry-functions-simple-in-aggregation-mode branch February 25, 2025 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants