[llvm-branch-commits] [llvm] [BOLT] Use BAT interfaces in YAMLProfileWriter::convert (PR #86219)

Amir Ayupov via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon Apr 1 16:02:24 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/2] 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/2] 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



More information about the llvm-branch-commits mailing list