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/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 <aau...@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
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
