[BOLT] Process cross references between ignored functions in BAT mode

[aaupov]

To align YAML and fdata profiles produced in BAT mode, lift two
restrictions applied in non-relocation mode when BAT is present:

1. register secondary entry points from ignored functions,
2. treat functions with secondary entry points as simple.

This allows constructing CFG for non-simple functions in non-relocation
mode and emitting YAML profile for them, which can then be used for
optimizations in relocation mode.

Test Plan: added test ignored-interprocedural-reference.s

[llvmbot]

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

To align YAML and fdata profiles produced in BAT mode, lift two
restrictions applied in non-relocation mode when BAT is present:

1. register secondary entry points from ignored functions,
2. treat functions with secondary entry points as simple.

This allows constructing CFG for non-simple functions in non-relocation
mode and emitting YAML profile for them, which can then be used for
optimizations in relocation mode.

Test Plan: added test ignored-interprocedural-reference.s

---

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

6 Files Affected:

• (modified) bolt/include/bolt/Core/BinaryContext.h (+3)
• (modified) bolt/lib/Core/BinaryContext.cpp (+3-1)
• (modified) bolt/lib/Core/BinaryFunction.cpp (+2-1)
• (modified) bolt/lib/Profile/YAMLProfileWriter.cpp (+4)
• (modified) bolt/lib/Rewrite/RewriteInstance.cpp (+1)
• (added) bolt/test/X86/ignored-interprocedural-reference.s (+49)

diff --git a/bolt/include/bolt/Core/BinaryContext.h b/bolt/include/bolt/Core/BinaryContext.h
index 8b1af9e815392..cef158074675b 100644
--- a/bolt/include/bolt/Core/BinaryContext.h
+++ b/bolt/include/bolt/Core/BinaryContext.h
@@ -676,6 +676,9 @@ class BinaryContext {
   /// have an origin file name available.
   bool HasSymbolsWithFileName{false};
 
+  /// Does the binary have BAT section.
+  bool HasBATSection{false};
+
   /// Sum of execution count of all functions
   uint64_t SumExecutionCount{0};
 
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index ad2eb18caf109..64d160adeee86 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -1322,7 +1322,9 @@ void BinaryContext::processInterproceduralReferences() {
        InterproceduralReferences) {
     BinaryFunction &Function = *It.first;
     uint64_t Address = It.second;
-    if (!Address || Function.isIgnored())
+    // Process interprocedural references from ignored functions in BAT mode
+    // (non-simple in non-relocation mode) to properly register entry points
+    if (!Address || (Function.isIgnored() && !HasBATSection))
       continue;
 
     BinaryFunction *TargetFunction =
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 1fa96dfaabde8..59b2e51e8a376 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1651,7 +1651,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.
-    if (!BC.HasRelocations)
+    // In BAT mode, register any known entry points for CFG construction.
+    if (!BC.HasRelocations && !BC.HasBATSection)
       setSimple(false);
 
     const uint32_t Offset = KV.first;
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index ef04ba0d21ad7..89087155ebb4a 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -39,6 +39,10 @@ const BinaryFunction *YAMLProfileWriter::setCSIDestination(
             BC.getFunctionForSymbol(Symbol, &EntryID)) {
       if (BAT && BAT->isBATFunction(Callee->getAddress()))
         std::tie(Callee, EntryID) = BAT->translateSymbol(BC, *Symbol, Offset);
+      else if (const BinaryBasicBlock *BB =
+                   Callee->getBasicBlockContainingOffset(Offset))
+        BC.getFunctionForSymbol(Callee->getSecondaryEntryPointSymbol(*BB),
+                                &EntryID);
       CSI.DestId = Callee->getFunctionNumber();
       CSI.EntryDiscriminator = EntryID;
       return Callee;
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 644d87eeca42e..d56ae4003464f 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -1959,6 +1959,7 @@ Error RewriteInstance::readSpecialSections() {
 
   if (ErrorOr<BinarySection &> BATSec =
           BC->getUniqueSectionByName(BoltAddressTranslation::SECTION_NAME)) {
+    BC->HasBATSection = true;
     // Do not read BAT when plotting a heatmap
     if (!opts::HeatmapMode) {
       if (std::error_code EC = BAT->parse(BC->outs(), BATSec->getContents())) {
diff --git a/bolt/test/X86/ignored-interprocedural-reference.s b/bolt/test/X86/ignored-interprocedural-reference.s
new file mode 100644
index 0000000000000..12e4fb92adcc0
--- /dev/null
+++ b/bolt/test/X86/ignored-interprocedural-reference.s
@@ -0,0 +1,49 @@
+# This reproduces a bug with not processing interprocedural references from
+# ignored functions.
+
+# REQUIRES: system-linux
+
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s -o %t.o
+# RUN: %clang %cflags %t.o -o %t.exe -nostdlib -Wl,-q
+# RUN: llvm-bolt %t.exe -o %t.out --enable-bat -funcs=main
+# RUN: link_fdata %s %t.out %t.preagg PREAGG
+# RUN: perf2bolt %t.out -p %t.preagg --pa -o %t.fdata -w %t.yaml
+# RUN: FileCheck %s --input-file=%t.fdata --check-prefix=CHECK-FDATA
+# RUN: FileCheck %s --input-file=%t.yaml --check-prefix=CHECK-YAML
+
+# CHECK-FDATA: 1 main 0 1 foo a 1 1
+# CHECK-YAML: name: main
+# CHECK-YAML: calls: {{.*}} disc: 1
+
+# PREAGG: B #main# #foo_secondary# 1 1
+# main calls foo at valid instruction offset past nops that are to be stripped.
+  .globl main
+main:
+  .cfi_startproc
+  call foo_secondary
+  ret
+  .cfi_endproc
+.size main,.-main
+
+# Placeholder cold fragment to force main to be ignored in non-relocation mode.
+  .globl main.cold
+main.cold:
+  .cfi_startproc
+  ud2
+  .cfi_endproc
+.size main.cold,.-main.cold
+
+# foo is set up to contain a valid instruction at called offset, and trapping
+# instructions past that.
+  .globl foo
+foo:
+  .cfi_startproc
+  .nops 10
+  .globl foo_secondary
+foo_secondary:
+  ret
+  .rept 20
+  int3
+  .endr
+  .cfi_endproc
+.size foo,.-foo

[rafaelauler]

LGTM

[aaupov]

Thanks!