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 <aau...@fb.com>
Date: Thu, 28 Mar 2024 10:16:15 -0700
Subject: [PATCH 1/8] 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 <aau...@fb.com>
Date: Thu, 28 Mar 2024 22:37:24 -0700
Subject: [PATCH 2/8] 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 <aau...@fb.com>
Date: Mon, 1 Apr 2024 18:30:25 -0700
Subject: [PATCH 3/8] 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 <aau...@fb.com>
Date: Tue, 2 Apr 2024 10:38:27 -0700
Subject: [PATCH 4/8] 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 <aau...@fb.com>
Date: Thu, 4 Apr 2024 17:16:57 -0700
Subject: [PATCH 5/8] 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 <aau...@fb.com>
Date: Fri, 5 Apr 2024 14:45:01 -0700
Subject: [PATCH 6/8] 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 <aau...@fb.com>
Date: Fri, 5 Apr 2024 14:47:31 -0700
Subject: [PATCH 7/8] 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
>From 8ac541395fa6910a1c408596573209e7332605b7 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aau...@fb.com>
Date: Fri, 5 Apr 2024 14:50:33 -0700
Subject: [PATCH 8/8] Remove unused include
Created using spr 1.3.4
---
bolt/lib/Profile/YAMLProfileWriter.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/bolt/lib/Profile/YAMLProfileWriter.cpp
b/bolt/lib/Profile/YAMLProfileWriter.cpp
index f1cdfc09a8a2f7..ef04ba0d21ad75 100644
--- a/bolt/lib/Profile/YAMLProfileWriter.cpp
+++ b/bolt/lib/Profile/YAMLProfileWriter.cpp
@@ -13,7 +13,6 @@
#include "bolt/Profile/ProfileReaderBase.h"
#include "bolt/Rewrite/RewriteInstance.h"
#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/raw_ostream.h"
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
