[llvm-branch-commits] [llvm] [BOLT] Use BAT for YAML profile call target information (PR #86219)

Amir Ayupov via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Fri Apr 5 14:47:55 PDT 2024


https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/86219

>From 685d3f5fa6ae75d6c3e22873a52ea8347e170c1e Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Thu, 28 Mar 2024 10:16:15 -0700
Subject: [PATCH 1/7] Get rid of std::map::at

Created using spr 1.3.4
---
 bolt/lib/Profile/BoltAddressTranslation.cpp | 5 ++++-
 bolt/lib/Profile/YAMLProfileWriter.cpp      | 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/bolt/lib/Profile/BoltAddressTranslation.cpp b/bolt/lib/Profile/BoltAddressTranslation.cpp
index 6d3f83efbe5f5a..7c54ba1971cbac 100644
--- a/bolt/lib/Profile/BoltAddressTranslation.cpp
+++ b/bolt/lib/Profile/BoltAddressTranslation.cpp
@@ -600,7 +600,10 @@ BoltAddressTranslation::getBFBranches(uint64_t OutputAddress) const {
 unsigned
 BoltAddressTranslation::getSecondaryEntryPointId(uint64_t Address,
                                                  uint32_t Offset) const {
-  const std::vector<uint32_t> &Offsets = SecondaryEntryPointsMap.at(Address);
+  auto FunctionIt = SecondaryEntryPointsMap.find(Address);
+  if (FunctionIt == SecondaryEntryPointsMap.end())
+    return UINT_MAX;
+  const std::vector<uint32_t> &Offsets = FunctionIt->second;
   auto OffsetIt = std::find(Offsets.begin(), Offsets.end(), Offset);
   if (OffsetIt == Offsets.end())
     return UINT_MAX;
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index 78fb1e8539d477..bacee136de3f87 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -48,7 +48,8 @@ setCSIDestination(const BinaryContext &BC, yaml::bolt::CallSiteInfo &CSI,
     if (SymbolValue.getError())
       return Callee;
     if (uint32_t Offset = SymbolValue.get() - Callee->getAddress())
-      EntryID = (*GetBATSecondaryEntryPointId)(Callee->getAddress(), Offset);
+      EntryID =
+          (*GetBATSecondaryEntryPointId)(Callee->getAddress(), Offset) + 1;
   } else {
     BC.getFunctionForSymbol(Symbol, &EntryID);
   }

>From 03520283ff38a47bc44cfa395534837d8da66934 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Thu, 28 Mar 2024 22:37:24 -0700
Subject: [PATCH 2/7] Fixed setting of BAT secondary entry point, updated test

Created using spr 1.3.4
---
 bolt/include/bolt/Profile/YAMLProfileWriter.h | 11 +--
 bolt/lib/Profile/DataAggregator.cpp           | 11 +--
 bolt/lib/Profile/YAMLProfileWriter.cpp        | 71 ++++++++++++-------
 .../X86/yaml-secondary-entry-discriminator.s  | 52 +++++++++++++-
 4 files changed, 97 insertions(+), 48 deletions(-)

diff --git a/bolt/include/bolt/Profile/YAMLProfileWriter.h b/bolt/include/bolt/Profile/YAMLProfileWriter.h
index 7db581652a5b73..0db2e3fd90f9f1 100644
--- a/bolt/include/bolt/Profile/YAMLProfileWriter.h
+++ b/bolt/include/bolt/Profile/YAMLProfileWriter.h
@@ -15,6 +15,7 @@
 
 namespace llvm {
 namespace bolt {
+class BoltAddressTranslation;
 class RewriteInstance;
 
 class YAMLProfileWriter {
@@ -31,17 +32,9 @@ class YAMLProfileWriter {
   /// Save execution profile for that instance.
   std::error_code writeProfile(const RewriteInstance &RI);
 
-  /// Callback to determine if a function is covered by BAT.
-  using IsBATCallbackTy = std::optional<function_ref<bool(uint64_t Address)>>;
-  /// Callback to get secondary entry point id for a given function and offset.
-  using GetBATSecondaryEntryPointIdCallbackTy =
-      std::optional<function_ref<unsigned(uint64_t Address, uint32_t Offset)>>;
-
   static yaml::bolt::BinaryFunctionProfile
   convert(const BinaryFunction &BF, bool UseDFS,
-          IsBATCallbackTy IsBATFunction = std::nullopt,
-          GetBATSecondaryEntryPointIdCallbackTy GetBATSecondaryEntryPointId =
-              std::nullopt);
+          const BoltAddressTranslation *BAT = nullptr);
 };
 
 } // namespace bolt
diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index 5b5ce5532ffdb9..71824e2cc0e97a 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -2324,13 +2324,6 @@ std::error_code DataAggregator::writeBATYAML(BinaryContext &BC,
   BP.Header.Flags = opts::BasicAggregation ? BinaryFunction::PF_SAMPLE
                                            : BinaryFunction::PF_LBR;
 
-  auto IsBATFunction = [&](uint64_t Address) {
-    return BAT->isBATFunction(Address);
-  };
-  auto GetSecondaryEntryPointId = [&](uint64_t Address, uint32_t Offset) {
-    return BAT->getSecondaryEntryPointId(Address, Offset);
-  };
-
   if (!opts::BasicAggregation) {
     // Convert profile for functions not covered by BAT
     for (auto &BFI : BC.getBinaryFunctions()) {
@@ -2339,8 +2332,8 @@ std::error_code DataAggregator::writeBATYAML(BinaryContext &BC,
         continue;
       if (BAT->isBATFunction(Function.getAddress()))
         continue;
-      BP.Functions.emplace_back(YAMLProfileWriter::convert(
-          Function, /*UseDFS=*/false, IsBATFunction, GetSecondaryEntryPointId));
+      BP.Functions.emplace_back(
+          YAMLProfileWriter::convert(Function, /*UseDFS=*/false, BAT));
     }
 
     for (const auto &KV : NamesToBranches) {
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index bacee136de3f87..6694890fa6900f 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -9,6 +9,7 @@
 #include "bolt/Profile/YAMLProfileWriter.h"
 #include "bolt/Core/BinaryBasicBlock.h"
 #include "bolt/Core/BinaryFunction.h"
+#include "bolt/Profile/BoltAddressTranslation.h"
 #include "bolt/Profile/ProfileReaderBase.h"
 #include "bolt/Rewrite/RewriteInstance.h"
 #include "llvm/Support/CommandLine.h"
@@ -29,37 +30,53 @@ namespace bolt {
 /// BinaryFunction for that symbol.
 static const BinaryFunction *
 setCSIDestination(const BinaryContext &BC, yaml::bolt::CallSiteInfo &CSI,
-                  const MCSymbol *Symbol,
-                  YAMLProfileWriter::IsBATCallbackTy IsBATFunction,
-                  YAMLProfileWriter::GetBATSecondaryEntryPointIdCallbackTy
-                      GetBATSecondaryEntryPointId) {
+                  const MCSymbol *Symbol, const BoltAddressTranslation *BAT) {
   CSI.DestId = 0; // designated for unknown functions
   CSI.EntryDiscriminator = 0;
-  if (!Symbol)
-    return nullptr;
-  uint64_t EntryID = 0;
-  const BinaryFunction *const Callee = BC.getFunctionForSymbol(Symbol);
-  if (!Callee)
-    return nullptr;
-  CSI.DestId = Callee->getFunctionNumber();
-  if (IsBATFunction && (*IsBATFunction)(Callee->getAddress())) {
-    assert(GetBATSecondaryEntryPointId);
+  auto setBATSecondaryEntry = [&](const BinaryFunction *const Callee) {
+    // The symbol could be a secondary entry in a cold fragment.
     ErrorOr<uint64_t> SymbolValue = BC.getSymbolValue(*Symbol);
     if (SymbolValue.getError())
+      return;
+
+    // Containing function, not necessarily the same as symbol value.
+    const uint64_t CalleeAddress = Callee->getAddress();
+    const uint32_t OutputOffset = SymbolValue.get() - CalleeAddress;
+
+    const uint64_t ParentAddress = BAT->fetchParentAddress(CalleeAddress);
+    const uint64_t HotAddress = ParentAddress ? ParentAddress : CalleeAddress;
+
+    if (const BinaryFunction *ParentBF =
+            BC.getBinaryFunctionAtAddress(HotAddress))
+      CSI.DestId = ParentBF->getFunctionNumber();
+
+    const uint32_t InputOffset =
+        BAT->translate(CalleeAddress, OutputOffset, /*IsBranchSrc*/ false);
+
+    if (!InputOffset)
+      return;
+
+    CSI.EntryDiscriminator =
+        BAT->getSecondaryEntryPointId(HotAddress, InputOffset) + 1;
+  };
+
+  if (Symbol) {
+    uint64_t EntryID = 0;
+    if (const BinaryFunction *const Callee =
+            BC.getFunctionForSymbol(Symbol, &EntryID)) {
+      CSI.DestId = Callee->getFunctionNumber();
+      CSI.EntryDiscriminator = EntryID;
+      if (BAT && BAT->isBATFunction(Callee->getAddress()))
+        setBATSecondaryEntry(Callee);
       return Callee;
-    if (uint32_t Offset = SymbolValue.get() - Callee->getAddress())
-      EntryID =
-          (*GetBATSecondaryEntryPointId)(Callee->getAddress(), Offset) + 1;
-  } else {
-    BC.getFunctionForSymbol(Symbol, &EntryID);
+    }
   }
-  CSI.EntryDiscriminator = EntryID;
-  return Callee;
+  return nullptr;
 }
 
-yaml::bolt::BinaryFunctionProfile YAMLProfileWriter::convert(
-    const BinaryFunction &BF, bool UseDFS, IsBATCallbackTy IsBATFunction,
-    GetBATSecondaryEntryPointIdCallbackTy GetBATSecondaryEntryPointId) {
+yaml::bolt::BinaryFunctionProfile
+YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
+                           const BoltAddressTranslation *BAT) {
   yaml::bolt::BinaryFunctionProfile YamlBF;
   const BinaryContext &BC = BF.getBinaryContext();
 
@@ -112,8 +129,8 @@ yaml::bolt::BinaryFunctionProfile YAMLProfileWriter::convert(
           continue;
         for (const IndirectCallProfile &CSP : ICSP.get()) {
           StringRef TargetName = "";
-          const BinaryFunction *Callee = setCSIDestination(
-              BC, CSI, CSP.Symbol, IsBATFunction, GetBATSecondaryEntryPointId);
+          const BinaryFunction *Callee =
+              setCSIDestination(BC, CSI, CSP.Symbol, BAT);
           if (Callee)
             TargetName = Callee->getOneName();
           CSI.Count = CSP.Count;
@@ -123,8 +140,8 @@ yaml::bolt::BinaryFunctionProfile YAMLProfileWriter::convert(
       } else { // direct call or a tail call
         StringRef TargetName = "";
         const MCSymbol *CalleeSymbol = BC.MIB->getTargetSymbol(Instr);
-        const BinaryFunction *Callee = setCSIDestination(
-            BC, CSI, CalleeSymbol, IsBATFunction, GetBATSecondaryEntryPointId);
+        const BinaryFunction *const Callee =
+            setCSIDestination(BC, CSI, CalleeSymbol, BAT);
         if (Callee)
           TargetName = Callee->getOneName();
 
diff --git a/bolt/test/X86/yaml-secondary-entry-discriminator.s b/bolt/test/X86/yaml-secondary-entry-discriminator.s
index 43c2e2a7f05549..6e7c84edaa785a 100644
--- a/bolt/test/X86/yaml-secondary-entry-discriminator.s
+++ b/bolt/test/X86/yaml-secondary-entry-discriminator.s
@@ -11,17 +11,17 @@
 # RUN: FileCheck %s -input-file %t.yaml
 # CHECK:      - name:    main
 # CHECK-NEXT:   fid:     2
-# CHECK-NEXT:   hash:    0xADF270D550151185
+# CHECK-NEXT:   hash:    {{.*}}
 # CHECK-NEXT:   exec:    0
 # CHECK-NEXT:   nblocks: 4
 # CHECK-NEXT:   blocks:
 # CHECK:          - bid:   1
 # CHECK-NEXT:       insns: 1
-# CHECK-NEXT:       hash:  0x36A303CBA4360014
+# CHECK-NEXT:       hash:  {{.*}}
 # CHECK-NEXT:       calls: [ { off: 0x0, fid: 1, disc: 1, cnt: 1 } ]
 # CHECK:          - bid:   2
 # CHECK-NEXT:       insns: 5
-# CHECK-NEXT:       hash:  0x8B2F5747CD0019
+# CHECK-NEXT:       hash:  {{.*}}
 # CHECK-NEXT:       calls: [ { off: 0x0, fid: 1, disc: 1, cnt: 1, mis: 1 } ]
 
 # Make sure that the profile is attached correctly
@@ -33,15 +33,57 @@
 # CHECK-CFG:      callq *%rax # Offset: [[#]] # CallProfile: 1 (1 misses) :
 # CHECK-CFG-NEXT:     { secondary_entry: 1 (1 misses) }
 
+# YAML BAT test of calling BAT secondary entry from non-BAT function
+# Now force-split func and skip main (making it call secondary entries)
+# RUN: llvm-bolt %t.exe -o %t.bat --data %t.fdata --funcs=func \
+# RUN:   --split-functions --split-strategy=all --split-all-cold --enable-bat
+
+# Prepare pre-aggregated profile using %t.bat
+# RUN: link_fdata %s %t.bat %t.preagg PREAGG
+# Strip labels used for pre-aggregated profile
+# RUN: llvm-strip -NLcall -NLindcall %t.bat
+
+# Convert pre-aggregated profile using BAT
+# RUN: perf2bolt %t.bat -p %t.preagg --pa -o %t.bat.fdata -w %t.bat.yaml
+
+# Convert BAT fdata into YAML
+# RUN: llvm-bolt %t.exe -data %t.bat.fdata -w %t.bat.fdata-yaml -o /dev/null
+
+# Check fdata YAML - make sure that a direct call has discriminator field
+# RUN: FileCheck %s --input-file %t.bat.fdata-yaml -check-prefix CHECK-BAT-YAML
+
+# Check BAT YAML - make sure that a direct call has discriminator field
+# RUN: FileCheck %s --input-file %t.bat.yaml --check-prefix CHECK-BAT-YAML
+
+# CHECK-BAT-YAML:      - name:    main
+# CHECK-BAT-YAML-NEXT:   fid:     2
+# CHECK-BAT-YAML-NEXT:   hash:    0xADF270D550151185
+# CHECK-BAT-YAML-NEXT:   exec:    0
+# CHECK-BAT-YAML-NEXT:   nblocks: 4
+# CHECK-BAT-YAML-NEXT:   blocks:
+# CHECK-BAT-YAML:          - bid:   1
+# CHECK-BAT-YAML-NEXT:       insns: [[#]]
+# CHECK-BAT-YAML-NEXT:       hash:  0x36A303CBA4360018
+# CHECK-BAT-YAML-NEXT:       calls: [ { off: 0x0, fid: [[#]], disc: 1, cnt: 1 } ]
+
 .globl func
 .type	func, @function
 func:
 # FDATA: 0 [unknown] 0 1 func 0 1 0
+# PREAGG: B X:0 #func# 1 1
   .cfi_startproc
   pushq   %rbp
   movq    %rsp, %rbp
+  # Placeholder code to make splitting profitable
+.rept 5
+  testq   %rax, %rax
+.endr
 .globl secondary_entry
 secondary_entry:
+  # Placeholder code to make splitting profitable
+.rept 5
+  testq   %rax, %rax
+.endr
   popq    %rbp
   retq
   nopl    (%rax)
@@ -58,12 +100,16 @@ main:
   movl    $0, -4(%rbp)
   testq   %rax, %rax
   jne     Lindcall
+.globl Lcall
 Lcall:
   call    secondary_entry
 # FDATA: 1 main #Lcall# 1 secondary_entry 0 1 1
+# PREAGG: B #Lcall# #secondary_entry# 1 1
+.globl Lindcall
 Lindcall:
   callq   *%rax
 # FDATA: 1 main #Lindcall# 1 secondary_entry 0 1 1
+# PREAGG: B #Lindcall# #secondary_entry# 1 1
   xorl    %eax, %eax
   addq    $16, %rsp
   popq    %rbp

>From e64c676d4b44db41b3df4307a43a667ba0e7c4e8 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Mon, 1 Apr 2024 18:30:25 -0700
Subject: [PATCH 3/7] Define BAT::translateSymbol

Created using spr 1.3.4
---
 .../bolt/Profile/BoltAddressTranslation.h     | 11 +++++--
 bolt/lib/Profile/BoltAddressTranslation.cpp   | 29 +++++++++++++++++
 bolt/lib/Profile/YAMLProfileWriter.cpp        | 31 ++-----------------
 3 files changed, 41 insertions(+), 30 deletions(-)

diff --git a/bolt/include/bolt/Profile/BoltAddressTranslation.h b/bolt/include/bolt/Profile/BoltAddressTranslation.h
index 6d389f26c05b04..271a67dcc0562b 100644
--- a/bolt/include/bolt/Profile/BoltAddressTranslation.h
+++ b/bolt/include/bolt/Profile/BoltAddressTranslation.h
@@ -19,6 +19,7 @@
 #include <unordered_map>
 
 namespace llvm {
+class MCSymbol;
 class raw_ostream;
 
 namespace object {
@@ -123,8 +124,10 @@ class BoltAddressTranslation {
   std::unordered_map<uint32_t, std::vector<uint32_t>>
   getBFBranches(uint64_t FuncOutputAddress) const;
 
-  /// Returns a secondary entry point id for a given \p Address and \p Offset.
-  unsigned getSecondaryEntryPointId(uint64_t Address, uint32_t Offset) const;
+  /// For a given \p Symbol in the output binary, returns a corresponding pair
+  /// of parent BinaryFunction and secondary entry point in it.
+  std::pair<const BinaryFunction *, unsigned>
+  translateSymbol(const BinaryContext &BC, const MCSymbol &Symbol) const;
 
 private:
   /// Helper to update \p Map by inserting one or more BAT entries reflecting
@@ -161,6 +164,10 @@ class BoltAddressTranslation {
   /// Map a function to its secondary entry points vector
   std::unordered_map<uint64_t, std::vector<uint32_t>> SecondaryEntryPointsMap;
 
+  /// Translates a given \p Symbol into a BinaryFunction and
+  /// Returns a secondary entry point id for a given \p Address and \p Offset.
+  unsigned getSecondaryEntryPointId(uint64_t Address, uint32_t Offset) const;
+
   /// Links outlined cold bocks to their original function
   std::map<uint64_t, uint64_t> ColdPartSource;
 
diff --git a/bolt/lib/Profile/BoltAddressTranslation.cpp b/bolt/lib/Profile/BoltAddressTranslation.cpp
index 614ca9a08ff6aa..87d2ce40df7f01 100644
--- a/bolt/lib/Profile/BoltAddressTranslation.cpp
+++ b/bolt/lib/Profile/BoltAddressTranslation.cpp
@@ -611,5 +611,34 @@ BoltAddressTranslation::getSecondaryEntryPointId(uint64_t Address,
   // enumeration for secondary entry points starts with 1.
   return OffsetIt - Offsets.begin() + 1;
 }
+
+std::pair<const BinaryFunction *, unsigned>
+BoltAddressTranslation::translateSymbol(const BinaryContext &BC,
+                                        const MCSymbol &Symbol) const {
+  // The symbol could be a secondary entry in a cold fragment.
+  uint64_t SymbolValue = cantFail(errorOrToExpected(BC.getSymbolValue(Symbol)));
+
+  const BinaryFunction *Callee = BC.getFunctionForSymbol(&Symbol);
+  assert(Callee);
+
+  // Containing function, not necessarily the same as symbol value.
+  const uint64_t CalleeAddress = Callee->getAddress();
+  const uint32_t OutputOffset = SymbolValue - CalleeAddress;
+
+  const uint64_t ParentAddress = fetchParentAddress(CalleeAddress);
+  const uint64_t HotAddress = ParentAddress ? ParentAddress : CalleeAddress;
+
+  const BinaryFunction *ParentBF = BC.getBinaryFunctionAtAddress(HotAddress);
+
+  const uint32_t InputOffset =
+      translate(CalleeAddress, OutputOffset, /*IsBranchSrc*/ false);
+
+  unsigned SecondaryEntryId{0};
+  if (InputOffset)
+    SecondaryEntryId = getSecondaryEntryPointId(HotAddress, InputOffset);
+
+  return std::pair(ParentBF, SecondaryEntryId);
+}
+
 } // namespace bolt
 } // namespace llvm
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index d432d92228cc93..88a2a149e7bca9 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -34,40 +34,15 @@ setCSIDestination(const BinaryContext &BC, yaml::bolt::CallSiteInfo &CSI,
                   const MCSymbol *Symbol, const BoltAddressTranslation *BAT) {
   CSI.DestId = 0; // designated for unknown functions
   CSI.EntryDiscriminator = 0;
-  auto setBATSecondaryEntry = [&](const BinaryFunction &Callee) {
-    // The symbol could be a secondary entry in a cold fragment.
-    uint64_t SymbolValue =
-        cantFail(errorOrToExpected(BC.getSymbolValue(*Symbol)));
-
-    // Containing function, not necessarily the same as symbol value.
-    const uint64_t CalleeAddress = Callee.getAddress();
-    const uint32_t OutputOffset = SymbolValue - CalleeAddress;
-
-    const uint64_t ParentAddress = BAT->fetchParentAddress(CalleeAddress);
-    const uint64_t HotAddress = ParentAddress ? ParentAddress : CalleeAddress;
-
-    if (const BinaryFunction *ParentBF =
-            BC.getBinaryFunctionAtAddress(HotAddress))
-      CSI.DestId = ParentBF->getFunctionNumber();
-
-    const uint32_t InputOffset =
-        BAT->translate(CalleeAddress, OutputOffset, /*IsBranchSrc*/ false);
-
-    if (!InputOffset)
-      return;
-
-    CSI.EntryDiscriminator =
-        BAT->getSecondaryEntryPointId(HotAddress, InputOffset);
-  };
 
   if (Symbol) {
     uint64_t EntryID = 0;
-    if (const BinaryFunction *const Callee =
+    if (const BinaryFunction *Callee =
             BC.getFunctionForSymbol(Symbol, &EntryID)) {
+      if (BAT && BAT->isBATFunction(Callee->getAddress()))
+        std::tie(Callee, EntryID) = BAT->translateSymbol(BC, *Symbol);
       CSI.DestId = Callee->getFunctionNumber();
       CSI.EntryDiscriminator = EntryID;
-      if (BAT && BAT->isBATFunction(Callee->getAddress()))
-        setBATSecondaryEntry(*Callee);
       return Callee;
     }
   }

>From 8bf63a7e21ecf260caba79f59a91b7ad9cfceb80 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Tue, 2 Apr 2024 10:38:27 -0700
Subject: [PATCH 4/7] Remove stray comment

Created using spr 1.3.4
---
 bolt/include/bolt/Profile/BoltAddressTranslation.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/bolt/include/bolt/Profile/BoltAddressTranslation.h b/bolt/include/bolt/Profile/BoltAddressTranslation.h
index 271a67dcc0562b..356193e321688c 100644
--- a/bolt/include/bolt/Profile/BoltAddressTranslation.h
+++ b/bolt/include/bolt/Profile/BoltAddressTranslation.h
@@ -164,7 +164,6 @@ class BoltAddressTranslation {
   /// Map a function to its secondary entry points vector
   std::unordered_map<uint64_t, std::vector<uint32_t>> SecondaryEntryPointsMap;
 
-  /// Translates a given \p Symbol into a BinaryFunction and
   /// Returns a secondary entry point id for a given \p Address and \p Offset.
   unsigned getSecondaryEntryPointId(uint64_t Address, uint32_t Offset) const;
 

>From 77b7722dcbdde80d197a059c4c0bbf7093e8dd8b Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Thu, 4 Apr 2024 17:16:57 -0700
Subject: [PATCH 5/7] Expose setCSIDestination

Created using spr 1.3.4
---
 .../bolt/Profile/BoltAddressTranslation.h     |  3 +-
 bolt/include/bolt/Profile/YAMLProfileWriter.h |  7 ++++
 bolt/lib/Profile/BoltAddressTranslation.cpp   |  5 ++-
 bolt/lib/Profile/DataAggregator.cpp           | 37 ++-----------------
 bolt/lib/Profile/YAMLProfileWriter.cpp        | 11 +++---
 5 files changed, 20 insertions(+), 43 deletions(-)

diff --git a/bolt/include/bolt/Profile/BoltAddressTranslation.h b/bolt/include/bolt/Profile/BoltAddressTranslation.h
index 356193e321688c..92c23b9d909b12 100644
--- a/bolt/include/bolt/Profile/BoltAddressTranslation.h
+++ b/bolt/include/bolt/Profile/BoltAddressTranslation.h
@@ -127,7 +127,8 @@ class BoltAddressTranslation {
   /// For a given \p Symbol in the output binary, returns a corresponding pair
   /// of parent BinaryFunction and secondary entry point in it.
   std::pair<const BinaryFunction *, unsigned>
-  translateSymbol(const BinaryContext &BC, const MCSymbol &Symbol) const;
+  translateSymbol(const BinaryContext &BC, const MCSymbol &Symbol,
+                  uint32_t Offset) const;
 
 private:
   /// Helper to update \p Map by inserting one or more BAT entries reflecting
diff --git a/bolt/include/bolt/Profile/YAMLProfileWriter.h b/bolt/include/bolt/Profile/YAMLProfileWriter.h
index 0db2e3fd90f9f1..4a9355dfceac9e 100644
--- a/bolt/include/bolt/Profile/YAMLProfileWriter.h
+++ b/bolt/include/bolt/Profile/YAMLProfileWriter.h
@@ -35,6 +35,13 @@ class YAMLProfileWriter {
   static yaml::bolt::BinaryFunctionProfile
   convert(const BinaryFunction &BF, bool UseDFS,
           const BoltAddressTranslation *BAT = nullptr);
+
+  /// Set CallSiteInfo destination fields from \p Symbol and return a target
+  /// BinaryFunction for that symbol.
+  static const BinaryFunction *
+  setCSIDestination(const BinaryContext &BC, yaml::bolt::CallSiteInfo &CSI,
+                    const MCSymbol *Symbol, const BoltAddressTranslation *BAT,
+                    uint32_t Offset = 0);
 };
 
 } // namespace bolt
diff --git a/bolt/lib/Profile/BoltAddressTranslation.cpp b/bolt/lib/Profile/BoltAddressTranslation.cpp
index 87d2ce40df7f01..6b808257e9ab26 100644
--- a/bolt/lib/Profile/BoltAddressTranslation.cpp
+++ b/bolt/lib/Profile/BoltAddressTranslation.cpp
@@ -614,7 +614,8 @@ BoltAddressTranslation::getSecondaryEntryPointId(uint64_t Address,
 
 std::pair<const BinaryFunction *, unsigned>
 BoltAddressTranslation::translateSymbol(const BinaryContext &BC,
-                                        const MCSymbol &Symbol) const {
+                                        const MCSymbol &Symbol,
+                                        uint32_t Offset) const {
   // The symbol could be a secondary entry in a cold fragment.
   uint64_t SymbolValue = cantFail(errorOrToExpected(BC.getSymbolValue(Symbol)));
 
@@ -631,7 +632,7 @@ BoltAddressTranslation::translateSymbol(const BinaryContext &BC,
   const BinaryFunction *ParentBF = BC.getBinaryFunctionAtAddress(HotAddress);
 
   const uint32_t InputOffset =
-      translate(CalleeAddress, OutputOffset, /*IsBranchSrc*/ false);
+      translate(CalleeAddress, OutputOffset, /*IsBranchSrc*/ false) + Offset;
 
   unsigned SecondaryEntryId{0};
   if (InputOffset)
diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index 71824e2cc0e97a..d0289d551c6a69 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -2386,40 +2386,9 @@ std::error_code DataAggregator::writeBATYAML(BinaryContext &BC,
             YamlCSI.Count = BI.Branches;
             YamlCSI.Mispreds = BI.Mispreds;
             YamlCSI.Offset = BranchOffset - Offset;
-            BinaryData *CallTargetBD = BC.getBinaryDataByName(CallToLoc.Name);
-            if (!CallTargetBD) {
-              YamlBB.CallSites.emplace_back(YamlCSI);
-              continue;
-            }
-            uint64_t CallTargetAddress = CallTargetBD->getAddress();
-            BinaryFunction *CallTargetBF =
-                BC.getBinaryFunctionAtAddress(CallTargetAddress);
-            if (!CallTargetBF) {
-              YamlBB.CallSites.emplace_back(YamlCSI);
-              continue;
-            }
-            // Calls between hot and cold fragments must be handled in
-            // fixupBATProfile.
-            assert(CallTargetBF != BF && "invalid CallTargetBF");
-            YamlCSI.DestId = CallTargetBF->getFunctionNumber();
-            if (CallToLoc.Offset) {
-              if (BAT->isBATFunction(CallTargetAddress)) {
-                LLVM_DEBUG(dbgs() << "BOLT-DEBUG: Unsupported secondary "
-                                     "entry point in BAT function "
-                                  << CallToLoc.Name << '\n');
-              } else if (const BinaryBasicBlock *CallTargetBB =
-                             CallTargetBF->getBasicBlockAtOffset(
-                                 CallToLoc.Offset)) {
-                // Only record true call information, ignoring returns (normally
-                // won't have a target basic block) and jumps to the landing
-                // pads (not an entry point).
-                if (CallTargetBB->isEntryPoint()) {
-                  YamlCSI.EntryDiscriminator =
-                      CallTargetBF->getEntryIDForSymbol(
-                          CallTargetBB->getLabel());
-                }
-              }
-            }
+            if (BinaryData *BD = BC.getBinaryDataByName(CallToLoc.Name))
+              YAMLProfileWriter::setCSIDestination(BC, YamlCSI, BD->getSymbol(),
+                                                   BAT, CallToLoc.Offset);
             YamlBB.CallSites.emplace_back(YamlCSI);
           }
         }
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp b/bolt/lib/Profile/YAMLProfileWriter.cpp
index 88a2a149e7bca9..f1cdfc09a8a2f7 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -27,11 +27,10 @@ extern llvm::cl::opt<bool> ProfileUseDFS;
 namespace llvm {
 namespace bolt {
 
-/// Set CallSiteInfo destination fields from \p Symbol and return a target
-/// BinaryFunction for that symbol.
-static const BinaryFunction *
-setCSIDestination(const BinaryContext &BC, yaml::bolt::CallSiteInfo &CSI,
-                  const MCSymbol *Symbol, const BoltAddressTranslation *BAT) {
+const BinaryFunction *YAMLProfileWriter::setCSIDestination(
+    const BinaryContext &BC, yaml::bolt::CallSiteInfo &CSI,
+    const MCSymbol *Symbol, const BoltAddressTranslation *BAT,
+    uint32_t Offset) {
   CSI.DestId = 0; // designated for unknown functions
   CSI.EntryDiscriminator = 0;
 
@@ -40,7 +39,7 @@ setCSIDestination(const BinaryContext &BC, yaml::bolt::CallSiteInfo &CSI,
     if (const BinaryFunction *Callee =
             BC.getFunctionForSymbol(Symbol, &EntryID)) {
       if (BAT && BAT->isBATFunction(Callee->getAddress()))
-        std::tie(Callee, EntryID) = BAT->translateSymbol(BC, *Symbol);
+        std::tie(Callee, EntryID) = BAT->translateSymbol(BC, *Symbol, Offset);
       CSI.DestId = Callee->getFunctionNumber();
       CSI.EntryDiscriminator = EntryID;
       return Callee;

>From 502b6b7c976c671a011772afd7663248c240c212 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Fri, 5 Apr 2024 14:45:01 -0700
Subject: [PATCH 6/7] Drop changes to DataAggregator

Created using spr 1.3.4
---
 bolt/lib/Profile/DataAggregator.cpp | 37 ++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index d0289d551c6a69..71824e2cc0e97a 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -2386,9 +2386,40 @@ std::error_code DataAggregator::writeBATYAML(BinaryContext &BC,
             YamlCSI.Count = BI.Branches;
             YamlCSI.Mispreds = BI.Mispreds;
             YamlCSI.Offset = BranchOffset - Offset;
-            if (BinaryData *BD = BC.getBinaryDataByName(CallToLoc.Name))
-              YAMLProfileWriter::setCSIDestination(BC, YamlCSI, BD->getSymbol(),
-                                                   BAT, CallToLoc.Offset);
+            BinaryData *CallTargetBD = BC.getBinaryDataByName(CallToLoc.Name);
+            if (!CallTargetBD) {
+              YamlBB.CallSites.emplace_back(YamlCSI);
+              continue;
+            }
+            uint64_t CallTargetAddress = CallTargetBD->getAddress();
+            BinaryFunction *CallTargetBF =
+                BC.getBinaryFunctionAtAddress(CallTargetAddress);
+            if (!CallTargetBF) {
+              YamlBB.CallSites.emplace_back(YamlCSI);
+              continue;
+            }
+            // Calls between hot and cold fragments must be handled in
+            // fixupBATProfile.
+            assert(CallTargetBF != BF && "invalid CallTargetBF");
+            YamlCSI.DestId = CallTargetBF->getFunctionNumber();
+            if (CallToLoc.Offset) {
+              if (BAT->isBATFunction(CallTargetAddress)) {
+                LLVM_DEBUG(dbgs() << "BOLT-DEBUG: Unsupported secondary "
+                                     "entry point in BAT function "
+                                  << CallToLoc.Name << '\n');
+              } else if (const BinaryBasicBlock *CallTargetBB =
+                             CallTargetBF->getBasicBlockAtOffset(
+                                 CallToLoc.Offset)) {
+                // Only record true call information, ignoring returns (normally
+                // won't have a target basic block) and jumps to the landing
+                // pads (not an entry point).
+                if (CallTargetBB->isEntryPoint()) {
+                  YamlCSI.EntryDiscriminator =
+                      CallTargetBF->getEntryIDForSymbol(
+                          CallTargetBB->getLabel());
+                }
+              }
+            }
             YamlBB.CallSites.emplace_back(YamlCSI);
           }
         }

>From df9ff6607024044cedf8b1e5cee4c516223f400b Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Fri, 5 Apr 2024 14:47:31 -0700
Subject: [PATCH 7/7] Drop changes to yaml-secondary-entry-discriminator.s

Created using spr 1.3.4
---
 .../X86/yaml-secondary-entry-discriminator.s  | 52 ++-----------------
 1 file changed, 3 insertions(+), 49 deletions(-)

diff --git a/bolt/test/X86/yaml-secondary-entry-discriminator.s b/bolt/test/X86/yaml-secondary-entry-discriminator.s
index 6e7c84edaa785a..43c2e2a7f05549 100644
--- a/bolt/test/X86/yaml-secondary-entry-discriminator.s
+++ b/bolt/test/X86/yaml-secondary-entry-discriminator.s
@@ -11,17 +11,17 @@
 # RUN: FileCheck %s -input-file %t.yaml
 # CHECK:      - name:    main
 # CHECK-NEXT:   fid:     2
-# CHECK-NEXT:   hash:    {{.*}}
+# CHECK-NEXT:   hash:    0xADF270D550151185
 # CHECK-NEXT:   exec:    0
 # CHECK-NEXT:   nblocks: 4
 # CHECK-NEXT:   blocks:
 # CHECK:          - bid:   1
 # CHECK-NEXT:       insns: 1
-# CHECK-NEXT:       hash:  {{.*}}
+# CHECK-NEXT:       hash:  0x36A303CBA4360014
 # CHECK-NEXT:       calls: [ { off: 0x0, fid: 1, disc: 1, cnt: 1 } ]
 # CHECK:          - bid:   2
 # CHECK-NEXT:       insns: 5
-# CHECK-NEXT:       hash:  {{.*}}
+# CHECK-NEXT:       hash:  0x8B2F5747CD0019
 # CHECK-NEXT:       calls: [ { off: 0x0, fid: 1, disc: 1, cnt: 1, mis: 1 } ]
 
 # Make sure that the profile is attached correctly
@@ -33,57 +33,15 @@
 # CHECK-CFG:      callq *%rax # Offset: [[#]] # CallProfile: 1 (1 misses) :
 # CHECK-CFG-NEXT:     { secondary_entry: 1 (1 misses) }
 
-# YAML BAT test of calling BAT secondary entry from non-BAT function
-# Now force-split func and skip main (making it call secondary entries)
-# RUN: llvm-bolt %t.exe -o %t.bat --data %t.fdata --funcs=func \
-# RUN:   --split-functions --split-strategy=all --split-all-cold --enable-bat
-
-# Prepare pre-aggregated profile using %t.bat
-# RUN: link_fdata %s %t.bat %t.preagg PREAGG
-# Strip labels used for pre-aggregated profile
-# RUN: llvm-strip -NLcall -NLindcall %t.bat
-
-# Convert pre-aggregated profile using BAT
-# RUN: perf2bolt %t.bat -p %t.preagg --pa -o %t.bat.fdata -w %t.bat.yaml
-
-# Convert BAT fdata into YAML
-# RUN: llvm-bolt %t.exe -data %t.bat.fdata -w %t.bat.fdata-yaml -o /dev/null
-
-# Check fdata YAML - make sure that a direct call has discriminator field
-# RUN: FileCheck %s --input-file %t.bat.fdata-yaml -check-prefix CHECK-BAT-YAML
-
-# Check BAT YAML - make sure that a direct call has discriminator field
-# RUN: FileCheck %s --input-file %t.bat.yaml --check-prefix CHECK-BAT-YAML
-
-# CHECK-BAT-YAML:      - name:    main
-# CHECK-BAT-YAML-NEXT:   fid:     2
-# CHECK-BAT-YAML-NEXT:   hash:    0xADF270D550151185
-# CHECK-BAT-YAML-NEXT:   exec:    0
-# CHECK-BAT-YAML-NEXT:   nblocks: 4
-# CHECK-BAT-YAML-NEXT:   blocks:
-# CHECK-BAT-YAML:          - bid:   1
-# CHECK-BAT-YAML-NEXT:       insns: [[#]]
-# CHECK-BAT-YAML-NEXT:       hash:  0x36A303CBA4360018
-# CHECK-BAT-YAML-NEXT:       calls: [ { off: 0x0, fid: [[#]], disc: 1, cnt: 1 } ]
-
 .globl func
 .type	func, @function
 func:
 # FDATA: 0 [unknown] 0 1 func 0 1 0
-# PREAGG: B X:0 #func# 1 1
   .cfi_startproc
   pushq   %rbp
   movq    %rsp, %rbp
-  # Placeholder code to make splitting profitable
-.rept 5
-  testq   %rax, %rax
-.endr
 .globl secondary_entry
 secondary_entry:
-  # Placeholder code to make splitting profitable
-.rept 5
-  testq   %rax, %rax
-.endr
   popq    %rbp
   retq
   nopl    (%rax)
@@ -100,16 +58,12 @@ main:
   movl    $0, -4(%rbp)
   testq   %rax, %rax
   jne     Lindcall
-.globl Lcall
 Lcall:
   call    secondary_entry
 # FDATA: 1 main #Lcall# 1 secondary_entry 0 1 1
-# PREAGG: B #Lcall# #secondary_entry# 1 1
-.globl Lindcall
 Lindcall:
   callq   *%rax
 # FDATA: 1 main #Lindcall# 1 secondary_entry 0 1 1
-# PREAGG: B #Lindcall# #secondary_entry# 1 1
   xorl    %eax, %eax
   addq    $16, %rsp
   popq    %rbp



More information about the llvm-branch-commits mailing list