[llvm-branch-commits] [BOLT][NFC] Print timers in perf2bolt invocation (PR #101270)
https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/101270 When BOLT is run in AggregateOnly mode (perf2bolt), it exits with code zero so destructors are not run thus TimerGroup never prints the timers. Add explicit printing just before the exit to honor options requesting timers (`--time-rewrite`, `--time-aggr`). Test Plan: updated bolt/test/timers.c ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Add profile density computation (PR #101094)
@@ -223,6 +223,22 @@ static cl::opt TopCalledLimit(
"functions section"),
cl::init(100), cl::Hidden, cl::cat(BoltCategory));
+// Profile density options, synced with llvm-profgen/ProfileGenerator.cpp
+static cl::opt ShowDensity("show-density", cl::init(false),
aaupov wrote:
Let's keep it disabled by default until we find good threshold value.
https://github.com/llvm/llvm-project/pull/101094
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Add profile density computation (PR #101094)
@@ -223,6 +223,22 @@ static cl::opt TopCalledLimit(
"functions section"),
cl::init(100), cl::Hidden, cl::cat(BoltCategory));
+// Profile density options, synced with llvm-profgen/ProfileGenerator.cpp
+static cl::opt ShowDensity("show-density", cl::init(false),
aaupov wrote:
I see. We don't report density by default but I'm not against it considering
it's pretty cheap to calculate.
https://github.com/llvm/llvm-project/pull/101094
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [MC][NFC] Store MCPseudoProbeFuncDesc::FuncName as StringRef (PR #100655)
https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/100655 Reduces peak RSS in `perf2bolt --profile-use-pseudo-probes` to 16.04GiB. ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with pseudo probes (PR #99891)
@@ -45,6 +45,8 @@ namespace opts {
extern cl::opt TimeRewrite;
extern cl::OptionCategory BoltOptCategory;
+extern cl::opt Verbosity;
+extern cl::opt ProfileUsePseudoProbes;
aaupov wrote:
> The profile (not stale matching) is clearly using probe as probe exists in
> profile
But in a similar way, the profile can contain BB hashes, but they won't be used
unless `-infer-stale-profile` is set. (We produce them unconditionally so
there's no corresponding generation flag).
There's no use of probes outside stale matching.
The confusion argument for me is flipped – if there were separate flags for
generation and consumption for probes. It's as if we had
`ProfileEmitDFSIndices` and `ProfileAssumeDFSIndices` instead of
`ProfileUseDFS` which covers both.
https://github.com/llvm/llvm-project/pull/99891
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with pseudo probes (PR #99891)
@@ -45,6 +45,8 @@ namespace opts {
extern cl::opt TimeRewrite;
extern cl::OptionCategory BoltOptCategory;
+extern cl::opt Verbosity;
+extern cl::opt ProfileUsePseudoProbes;
aaupov wrote:
> Ok, I think we need a dedicated flag to control matching strategy, otherwise
> it can be confusing. Generating profile with probe and using profile for
> stale profile matching are two different things.
Agree they're different but both require parsing pseudo probes, which is
controlled by the flag. `ProfileUseDFS` is another flag which controls both
profile generation and consumption.
> Like profile contains probe doesn't mean stale matching has to use probe.
If the profile contains probe information, but ProfileUsePseudoProbes is not
set, we will not use the information for matching.
https://github.com/llvm/llvm-project/pull/99891
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with pseudo probes (PR #99891)
@@ -208,11 +212,33 @@ class StaleMatcher {
}
}
- /// Find the most similar block for a given hash.
- const FlowBlock *matchBlock(BlendedBlockHash BlendedHash,
- uint64_t CallHash) const {
+ /// Creates a mapping from a pseudo probe index to pseudo probe.
+ void mapIndexToProbe(uint64_t Index, const MCDecodedPseudoProbe *Probe) {
+IndexToBBPseudoProbes[Index].push_back(Probe);
+ }
+
+ /// Creates a mapping from a pseudo probe to a flow block.
+ void mapProbeToBB(const MCDecodedPseudoProbe *Probe, FlowBlock *Block) {
+BBPseudoProbeToBlock[Probe] = Block;
+ }
+
+ /// Find the most similar flow block for a profile block given its hashes and
+ /// pseudo probe information.
+ const FlowBlock *
+ matchBlock(BlendedBlockHash BlendedHash, uint64_t CallHash,
+ const std::vector ) {
const FlowBlock *BestBlock = matchWithOpcodes(BlendedHash);
-return BestBlock ? BestBlock : matchWithCalls(BlendedHash, CallHash);
+if (BestBlock) {
+ ++MatchedWithOpcodes;
+ return BestBlock;
+}
+BestBlock = matchWithCalls(BlendedHash, CallHash);
+if (BestBlock)
+ return BestBlock;
+BestBlock = matchWithPseudoProbes(BlendedHash, PseudoProbes);
aaupov wrote:
> I'm a bit unsure about the strategy here. This is assuming matching of each
> individual block/node is an isolated problem. But the reality is, it's not,
> instead we're matching two sets of nodes (or rather two graphs) holistically.
Agree but we're trying to lay the foundation with only block matching belonging
to the current function. Your comment makes me rethink probe filtering based on
GUID - cc @shawbyoung.
> I.e. if we function's CFG hash matches that from probe desc, we should be
> able to have high confidence exact mapping for all blocks based on probe, and
> shouldn't need to treat probe based matching as a fallback?
Exact hash provides a stronger signal than probes and is available for all
binary basic blocks, so it makes sense to put it to the top. I'm on the fence
about the relative strengths of call hash and probes: call hash is supposedly
cheaper to construct but might not be available for all blocks. Stats from both
should help us make the right call.
https://github.com/llvm/llvm-project/pull/99891
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with pseudo probes (PR #99891)
@@ -45,6 +45,8 @@ namespace opts {
extern cl::opt TimeRewrite;
extern cl::OptionCategory BoltOptCategory;
+extern cl::opt Verbosity;
+extern cl::opt ProfileUsePseudoProbes;
aaupov wrote:
No, I intended `ProfileUsePseudoProbes` to cover both generation and use of
pseudo probes. The option means we parse pseudo probes for use in Profile, and
that covers both YAMLProfileReader and Writer.
https://github.com/llvm/llvm-project/pull/99891
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with pseudo probes (PR #99891)
@@ -266,6 +313,65 @@ class StaleMatcher {
}
return BestBlock;
}
+ // Uses pseudo probe information to attach the profile to the appropriate
+ // block.
+ const FlowBlock *matchWithPseudoProbes(
+ BlendedBlockHash BlendedHash,
+ const std::vector ) const {
+if (!YamlBFGUID)
+ return nullptr;
+
+if (opts::Verbosity >= 3)
+ outs() << "BOLT-INFO: attempting to match block with pseudo probes\n";
aaupov wrote:
Not throughout Profile component (yet).
https://github.com/llvm/llvm-project/pull/99891
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with pseudo probes (PR #99891)
https://github.com/aaupov approved this pull request. LG but let's give some time for other reviewers https://github.com/llvm/llvm-project/pull/99891 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with pseudo probes (PR #99891)
@@ -206,13 +213,30 @@ class StaleMatcher {
CallHashToBlocks[CallHashes[I]].push_back(
std::make_pair(Hashes[I], Block));
}
+this->IndexToBBPseudoProbes = IndexToBBPseudoProbes;
+this->BBPseudoProbeToBlock = BBPseudoProbeToBlock;
+this->YamlBFGUID = YamlBFGUID;
aaupov wrote:
Yes, that looks good.
https://github.com/llvm/llvm-project/pull/99891
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Add profile-use-pseudo-probes option (PR #100299)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/100299 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Add profile-use-pseudo-probes option (PR #100299)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/100299 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Add profile-use-pseudo-probes option (PR #100299)
https://github.com/aaupov ready_for_review https://github.com/llvm/llvm-project/pull/100299 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Support more than two jump table parents (PR #99988)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/99988 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Support more than two jump table parents (PR #99988)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/99988 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with pseudo probes (PR #99891)
@@ -555,6 +574,10 @@ size_t matchWeightsByHashes(
ProbeMap.lower_bound(FuncAddr + BlockRange.second));
for (const auto &[_, Probes] : BlockProbes) {
for (const MCDecodedPseudoProbe : Probes) {
+ if (Probe.getInlineTreeNode()->hasInlineSite())
aaupov wrote:
What do we prune with this check? Don't we discard valid probes belonging to
the current function?
https://github.com/llvm/llvm-project/pull/99891
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with pseudo probes (PR #99891)
@@ -306,26 +310,41 @@ class StaleMatcher {
BlockPseudoProbes.push_back();
}
-
// Returns nullptr if there is not a 1:1 mapping of the yaml block pseudo
// probe and binary pseudo probe.
-if (BlockPseudoProbes.size() == 0 || BlockPseudoProbes.size() > 1)
+if (BlockPseudoProbes.size() == 0) {
+ if (opts::Verbosity >= 2)
+errs() << "BOLT-WARNING: no pseudo probes in profile block\n";
aaupov wrote:
Bump verbosity for this logging to >=3.
Add aggregated counters – those could be printed for BF at v>=2.
BC-level aggregated counters can be printed at v>=1.
https://github.com/llvm/llvm-project/pull/99891
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with pseudo probes (PR #99891)
@@ -266,6 +287,47 @@ class StaleMatcher {
}
return BestBlock;
}
+ // Uses pseudo probe information to attach the profile to the appropriate
+ // block.
+ const FlowBlock *matchWithPseudoProbes(
+ const std::vector ) const {
+// Searches for the pseudo probe attached to the matched function's block,
+// ignoring pseudo probes attached to function calls and inlined functions'
+// blocks.
+std::vector BlockPseudoProbes;
+for (const auto : PseudoProbes) {
+ // Ensures that pseudo probe information belongs to the appropriate
+ // function and not an inlined function.
+ if (PseudoProbe.GUID != YamlBFGUID)
+continue;
+ // Skips pseudo probes attached to function calls.
+ if (PseudoProbe.Type != static_cast(PseudoProbeType::Block))
+continue;
+
+ BlockPseudoProbes.push_back();
+}
+
+// Returns nullptr if there is not a 1:1 mapping of the yaml block pseudo
+// probe and binary pseudo probe.
+if (BlockPseudoProbes.size() == 0 || BlockPseudoProbes.size() > 1)
+ return nullptr;
+
+uint64_t Index = BlockPseudoProbes[0]->Index;
+assert(Index <= Blocks.size() && "Invalid pseudo probe index");
+
+auto It = IndexToBinaryPseudoProbes.find(Index);
+assert(It != IndexToBinaryPseudoProbes.end() &&
+ "All blocks should have a pseudo probe");
aaupov wrote:
This assert should become a check as it's possible to have blocks without probes
https://github.com/llvm/llvm-project/pull/99891
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with pseudo probes (PR #99891)
@@ -266,6 +287,47 @@ class StaleMatcher {
}
return BestBlock;
}
+ // Uses pseudo probe information to attach the profile to the appropriate
+ // block.
+ const FlowBlock *matchWithPseudoProbes(
+ const std::vector ) const {
+// Searches for the pseudo probe attached to the matched function's block,
+// ignoring pseudo probes attached to function calls and inlined functions'
+// blocks.
+std::vector BlockPseudoProbes;
+for (const auto : PseudoProbes) {
+ // Ensures that pseudo probe information belongs to the appropriate
+ // function and not an inlined function.
+ if (PseudoProbe.GUID != YamlBFGUID)
+continue;
+ // Skips pseudo probes attached to function calls.
+ if (PseudoProbe.Type != static_cast(PseudoProbeType::Block))
+continue;
+
+ BlockPseudoProbes.push_back();
+}
+
+// Returns nullptr if there is not a 1:1 mapping of the yaml block pseudo
+// probe and binary pseudo probe.
+if (BlockPseudoProbes.size() == 0 || BlockPseudoProbes.size() > 1)
+ return nullptr;
+
+uint64_t Index = BlockPseudoProbes[0]->Index;
+assert(Index <= Blocks.size() && "Invalid pseudo probe index");
aaupov wrote:
Invalid assertion
https://github.com/llvm/llvm-project/pull/99891
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT][NFC] Track fragment relationships using EquivalenceClasses (PR #99979)
@@ -241,6 +242,10 @@ class BinaryContext {
/// Function fragments to skip.
std::unordered_set FragmentsToSkip;
+ /// Fragment equivalence classes to query belonging to the same "family" in
+ /// presence of multiple fragments/multiple parents.
+ EquivalenceClasses FragmentClasses;
aaupov wrote:
It's a set that only keeps a copy of entry (a pointer in our case):
https://github.com/llvm/llvm-project/blob/73ffeeab12d54211fd838d6ff988d111369ea196/llvm/include/llvm/ADT/EquivalenceClasses.h#L26-L35
Moreover, we will only insert fragments into it, so it shouldn't be too
expensive even for the largest binaries (we split up to 10s of thousands of
functions).
https://github.com/llvm/llvm-project/pull/99979
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Support more than two jump table parents (PR #99988)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/99988 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Support more than two jump table parents (PR #99988)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/99988 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT][NFC] Track fragment relationships using EquivalenceClasses (PR #99979)
https://github.com/aaupov updated
https://github.com/llvm/llvm-project/pull/99979
>From f6478e36a962843329c519ba35ad2a132ffd8c9e Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Mon, 22 Jul 2024 16:34:02 -0700
Subject: [PATCH] fix getOrCreateJumpTable
Created using spr 1.3.4
---
bolt/lib/Core/BinaryContext.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index bdfd91417a696..874cdd26ce6ea 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -841,7 +841,7 @@ BinaryContext::getOrCreateJumpTable(BinaryFunction
, uint64_t Address,
// Prevent associating a jump table to a specific fragment twice.
// This simple check arises from the assumption: no more than 2 fragments.
if (JT->Parents.size() == 1 && JT->Parents[0] != ) {
- assert(JT->Parents[0]->isParentOrChildOf(Function) &&
+ assert(areRelatedFragments(JT->Parents[0], ) &&
"cannot re-use jump table of a different function");
// Duplicate the entry for the parent function for easy access
JT->Parents.push_back();
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Support more than two jump table parents (PR #99988)
https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/99988 Multi-way splitting can cause multiple fragments to access the same jump table. Relax the assumption that a jump table can only have up to two parents. Test Plan: added bolt/test/X86/three-way-split-jt.s ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT][NFC] Track fragment relationships using EquivalenceClasses (PR #99979)
https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/99979 Three-way splitting can create references between split fragments (warm to cold or vice versa) that are not handled by `isChildOf/isParentOf/isChildOrParentOf`. Generalize fragment relationships to allow checking if two functions belong to one group, potentially in presence of ICF which can join multiple groups. Test Plan: NFC for existing tests ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [MC][NFC] Use std::map for AddressProbesMap (PR #99553)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/99553 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [MC][NFC] Use std::map for AddressProbesMap (PR #99553)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/99553 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Support POSSIBLE_PIC_FIXED_BRANCH (PR #91667)
https://github.com/aaupov closed https://github.com/llvm/llvm-project/pull/91667 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Support POSSIBLE_PIC_FIXED_BRANCH (PR #91667)
https://github.com/aaupov updated
https://github.com/llvm/llvm-project/pull/91667
>From dd4d0de42048c063d5e5095a0c2594c7cc578df5 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Thu, 9 May 2024 19:35:26 -0700
Subject: [PATCH 1/6] Fix RISCVMCPlusBuilder
Created using spr 1.3.4
---
bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
index 74f2f0aae91e6..020e62463ee2f 100644
--- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
+++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
@@ -177,13 +177,14 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
MCInst , InstructionIterator Begin, InstructionIterator End,
const unsigned PtrSize, MCInst *, unsigned ,
unsigned , int64_t , const MCExpr *,
- MCInst *) const override {
+ MCInst *, MCInst *) const override {
MemLocInstr = nullptr;
BaseRegNum = 0;
IndexRegNum = 0;
DispValue = 0;
DispExpr = nullptr;
PCRelBaseOut = nullptr;
+FixedEntryLoadInst = nullptr;
// Check for the following long tail call sequence:
// 1: auipc xi, %pcrel_hi(sym)
>From 62391bb5aa01f2b77d4315d1e72a9924eec9ecc0 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Fri, 5 Jul 2024 14:54:51 -0700
Subject: [PATCH 2/6] Drop deregisterJumpTable
Created using spr 1.3.4
---
bolt/lib/Core/BinaryFunction.cpp | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 09a6ca1d68730..f587d5a2cadd4 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -899,17 +899,9 @@ BinaryFunction::processIndirectBranch(MCInst ,
unsigned Size,
TargetAddress = ArrayStart + *Value;
-// Remove spurious JumpTable at EntryAddress caused by PIC reference from
-// the load instruction.
-JumpTable *JT = BC.getJumpTableContainingAddress(EntryAddress);
-assert(JT && "Must have a jump table at fixed entry address");
-BC.deregisterJumpTable(EntryAddress);
-JumpTables.erase(EntryAddress);
-delete JT;
-
// Replace FixedEntryDispExpr used in target address calculation with outer
// jump table reference.
-JT = BC.getJumpTableContainingAddress(ArrayStart);
+JumpTable *JT = BC.getJumpTableContainingAddress(ArrayStart);
assert(JT && "Must have a containing jump table for PIC fixed branch");
BC.MIB->replaceMemOperandDisp(*FixedEntryLoadInstr, JT->getFirstLabel(),
EntryAddress - ArrayStart, &*BC.Ctx);
>From 5336879ab68aedb1217e2c6c139d171f31e89e03 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Sat, 6 Jul 2024 22:26:14 -0700
Subject: [PATCH 3/6] Surgically drop spurious jump table
Created using spr 1.3.4
---
bolt/include/bolt/Core/BinaryContext.h | 5 +
bolt/lib/Core/BinaryFunction.cpp| 12 ++--
bolt/test/X86/jump-table-fixed-ref-pic.test | 11 ---
3 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/bolt/include/bolt/Core/BinaryContext.h
b/bolt/include/bolt/Core/BinaryContext.h
index 73932c4ca2fb3..c5e2c6cd02179 100644
--- a/bolt/include/bolt/Core/BinaryContext.h
+++ b/bolt/include/bolt/Core/BinaryContext.h
@@ -431,6 +431,11 @@ class BinaryContext {
return nullptr;
}
+ /// Deregister JumpTable registered at a given \p Address.
+ bool deregisterJumpTable(uint64_t Address) {
+return JumpTables.erase(Address);
+ }
+
unsigned getDWARFEncodingSize(unsigned Encoding) {
if (Encoding == dwarf::DW_EH_PE_omit)
return 0;
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index f587d5a2cadd4..2ecca32a5985c 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -899,9 +899,17 @@ BinaryFunction::processIndirectBranch(MCInst ,
unsigned Size,
TargetAddress = ArrayStart + *Value;
+// Remove spurious JumpTable at EntryAddress caused by PIC reference from
+// the load instruction.
+JumpTable *JT = BC.getJumpTableContainingAddress(EntryAddress);
+assert(JT && "Must have a jump table at fixed entry address");
+BC.deregisterJumpTable(EntryAddress);
+JumpTables.erase(EntryAddress);
+delete JT;
+
// Replace FixedEntryDispExpr used in target address calculation with outer
// jump table reference.
-JumpTable *JT = BC.getJumpTableContainingAddress(ArrayStart);
+JT = BC.getJumpTableContainingAddress(ArrayStart);
assert(JT && "Must have a containing jump table for PIC fixed branch");
BC.MIB->replaceMemOperandDisp(*FixedEntryLoadInstr, JT->getFirstLabel(),
EntryAddress - ArrayStart, &*BC.Ctx);
@@ -1158,10 +1166,10 @@ void BinaryFunction::handleIndirectBranch(MCInst
, uint64_t Size,
}
case IndirectBranchType::POSSIBLE_JUMP_TABLE:
case
[llvm-branch-commits] [llvm] [BOLT] Support POSSIBLE_PIC_FIXED_BRANCH (PR #91667)
@@ -1876,6 +1876,13 @@ class X86MCPlusBuilder : public MCPlusBuilder {
//add %r2, %r1
//jmp *%r1
//
+// or a fixed indirect jump template:
+//
+//movslq En(%rip), {%r2|%r1}
+//lea PIC_JUMP_TABLE(%rip), {%r1|%r2} <- MemLocInstr
aaupov wrote:
Updated the comment. There's still MemLocInstr as a return argument.
https://github.com/llvm/llvm-project/pull/91667
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Support POSSIBLE_PIC_FIXED_BRANCH (PR #91667)
https://github.com/aaupov updated
https://github.com/llvm/llvm-project/pull/91667
>From dd4d0de42048c063d5e5095a0c2594c7cc578df5 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Thu, 9 May 2024 19:35:26 -0700
Subject: [PATCH 1/5] Fix RISCVMCPlusBuilder
Created using spr 1.3.4
---
bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
index 74f2f0aae91e6..020e62463ee2f 100644
--- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
+++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
@@ -177,13 +177,14 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
MCInst , InstructionIterator Begin, InstructionIterator End,
const unsigned PtrSize, MCInst *, unsigned ,
unsigned , int64_t , const MCExpr *,
- MCInst *) const override {
+ MCInst *, MCInst *) const override {
MemLocInstr = nullptr;
BaseRegNum = 0;
IndexRegNum = 0;
DispValue = 0;
DispExpr = nullptr;
PCRelBaseOut = nullptr;
+FixedEntryLoadInst = nullptr;
// Check for the following long tail call sequence:
// 1: auipc xi, %pcrel_hi(sym)
>From 62391bb5aa01f2b77d4315d1e72a9924eec9ecc0 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Fri, 5 Jul 2024 14:54:51 -0700
Subject: [PATCH 2/5] Drop deregisterJumpTable
Created using spr 1.3.4
---
bolt/lib/Core/BinaryFunction.cpp | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 09a6ca1d68730..f587d5a2cadd4 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -899,17 +899,9 @@ BinaryFunction::processIndirectBranch(MCInst ,
unsigned Size,
TargetAddress = ArrayStart + *Value;
-// Remove spurious JumpTable at EntryAddress caused by PIC reference from
-// the load instruction.
-JumpTable *JT = BC.getJumpTableContainingAddress(EntryAddress);
-assert(JT && "Must have a jump table at fixed entry address");
-BC.deregisterJumpTable(EntryAddress);
-JumpTables.erase(EntryAddress);
-delete JT;
-
// Replace FixedEntryDispExpr used in target address calculation with outer
// jump table reference.
-JT = BC.getJumpTableContainingAddress(ArrayStart);
+JumpTable *JT = BC.getJumpTableContainingAddress(ArrayStart);
assert(JT && "Must have a containing jump table for PIC fixed branch");
BC.MIB->replaceMemOperandDisp(*FixedEntryLoadInstr, JT->getFirstLabel(),
EntryAddress - ArrayStart, &*BC.Ctx);
>From 5336879ab68aedb1217e2c6c139d171f31e89e03 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Sat, 6 Jul 2024 22:26:14 -0700
Subject: [PATCH 3/5] Surgically drop spurious jump table
Created using spr 1.3.4
---
bolt/include/bolt/Core/BinaryContext.h | 5 +
bolt/lib/Core/BinaryFunction.cpp| 12 ++--
bolt/test/X86/jump-table-fixed-ref-pic.test | 11 ---
3 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/bolt/include/bolt/Core/BinaryContext.h
b/bolt/include/bolt/Core/BinaryContext.h
index 73932c4ca2fb3..c5e2c6cd02179 100644
--- a/bolt/include/bolt/Core/BinaryContext.h
+++ b/bolt/include/bolt/Core/BinaryContext.h
@@ -431,6 +431,11 @@ class BinaryContext {
return nullptr;
}
+ /// Deregister JumpTable registered at a given \p Address.
+ bool deregisterJumpTable(uint64_t Address) {
+return JumpTables.erase(Address);
+ }
+
unsigned getDWARFEncodingSize(unsigned Encoding) {
if (Encoding == dwarf::DW_EH_PE_omit)
return 0;
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index f587d5a2cadd4..2ecca32a5985c 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -899,9 +899,17 @@ BinaryFunction::processIndirectBranch(MCInst ,
unsigned Size,
TargetAddress = ArrayStart + *Value;
+// Remove spurious JumpTable at EntryAddress caused by PIC reference from
+// the load instruction.
+JumpTable *JT = BC.getJumpTableContainingAddress(EntryAddress);
+assert(JT && "Must have a jump table at fixed entry address");
+BC.deregisterJumpTable(EntryAddress);
+JumpTables.erase(EntryAddress);
+delete JT;
+
// Replace FixedEntryDispExpr used in target address calculation with outer
// jump table reference.
-JumpTable *JT = BC.getJumpTableContainingAddress(ArrayStart);
+JT = BC.getJumpTableContainingAddress(ArrayStart);
assert(JT && "Must have a containing jump table for PIC fixed branch");
BC.MIB->replaceMemOperandDisp(*FixedEntryLoadInstr, JT->getFirstLabel(),
EntryAddress - ArrayStart, &*BC.Ctx);
@@ -1158,10 +1166,10 @@ void BinaryFunction::handleIndirectBranch(MCInst
, uint64_t Size,
}
case IndirectBranchType::POSSIBLE_JUMP_TABLE:
case
[llvm-branch-commits] [llvm] [BOLT] Attach pseudo probes to blocks in YAML profile (PR #99554)
https://github.com/aaupov updated
https://github.com/llvm/llvm-project/pull/99554
>From 9498f8f38cea050fd363d5d4591e8401e5de8bd5 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Thu, 18 Jul 2024 12:49:23 -0700
Subject: [PATCH 1/2] Fix operator==
Created using spr 1.3.4
---
bolt/include/bolt/Profile/ProfileYAMLMapping.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bolt/include/bolt/Profile/ProfileYAMLMapping.h
b/bolt/include/bolt/Profile/ProfileYAMLMapping.h
index 2bc6901e5f591..2a0514d7d9304 100644
--- a/bolt/include/bolt/Profile/ProfileYAMLMapping.h
+++ b/bolt/include/bolt/Profile/ProfileYAMLMapping.h
@@ -100,7 +100,7 @@ struct PseudoProbeInfo {
uint8_t Type;
bool operator==(const PseudoProbeInfo ) const {
-return Index == Other.Index;
+return GUID == Other.GUID && Index == Other.Index;
}
bool operator!=(const PseudoProbeInfo ) const {
return !(*this == Other);
>From 956084a3b4078f10d953ecab20720e9cf2d25554 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Thu, 18 Jul 2024 12:55:50 -0700
Subject: [PATCH 2/2] Fix probe mapping to block in BAT mode
Created using spr 1.3.4
---
bolt/lib/Profile/DataAggregator.cpp | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/bolt/lib/Profile/DataAggregator.cpp
b/bolt/lib/Profile/DataAggregator.cpp
index 85e583c75728f..1da018c346d36 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -2419,14 +2419,7 @@ std::error_code
DataAggregator::writeBATYAML(BinaryContext ,
for (const auto &[OutputAddress, Probes] : FragmentProbes) {
const uint32_t InputOffset = BAT->translate(
FuncAddr, OutputAddress - FuncAddr, /*IsBranchSrc=*/true);
-if (!BlockMap.isInputBlock(InputOffset)) {
- if (opts::Verbosity >= 1)
-errs() << "BOLT-WARNING: Couldn't map pseudo probe at 0x"
- << Twine::utohexstr(InputOffset) << " to a block in "
- << F->getPrintName() << '\n';
- continue;
-}
-const unsigned BlockIndex = BlockMap.getBBIndex(InputOffset);
+const unsigned BlockIndex = getBlock(InputOffset).second;
for (const MCDecodedPseudoProbe : Probes)
YamlBF.Blocks[BlockIndex].PseudoProbes.emplace_back(
yaml::bolt::PseudoProbeInfo{Probe.getGuid(),
Probe.getIndex(),
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Attach pseudo probes to blocks in YAML profile (PR #99554)
https://github.com/aaupov updated
https://github.com/llvm/llvm-project/pull/99554
>From 9498f8f38cea050fd363d5d4591e8401e5de8bd5 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Thu, 18 Jul 2024 12:49:23 -0700
Subject: [PATCH] Fix operator==
Created using spr 1.3.4
---
bolt/include/bolt/Profile/ProfileYAMLMapping.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bolt/include/bolt/Profile/ProfileYAMLMapping.h
b/bolt/include/bolt/Profile/ProfileYAMLMapping.h
index 2bc6901e5f591..2a0514d7d9304 100644
--- a/bolt/include/bolt/Profile/ProfileYAMLMapping.h
+++ b/bolt/include/bolt/Profile/ProfileYAMLMapping.h
@@ -100,7 +100,7 @@ struct PseudoProbeInfo {
uint8_t Type;
bool operator==(const PseudoProbeInfo ) const {
-return Index == Other.Index;
+return GUID == Other.GUID && Index == Other.Index;
}
bool operator!=(const PseudoProbeInfo ) const {
return !(*this == Other);
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [MC][NFC] Use std::map for AddressProbesMap (PR #99553)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/99553 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Attach pseudo probes to blocks in YAML profile (PR #99554)
https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/99554 Read pseudo probes in regular and BAT YAML profile generation, and attach them to YAML profile basic blocks. This exposes GUID, probe id, and probe type in profile for future use in stale profile matching. Test Plan: updated pseudoprobe-decoding-inline.test ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [MC][NFC] Use std::map for AddressProbesMap (PR #99553)
https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/99553 AddressProbesMap is keyed by binary addresses, and it makes sense to treat them as ordered. This also enables slicing by binary function/ binary basic block, to be used in BOLT. Test Plan: NFC ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with call graph (PR #98125)
@@ -43,6 +43,57 @@ class YAMLProfileReader : public ProfileReaderBase {
using ProfileLookupMap =
DenseMap;
+ /// A class for matching binary functions in functions in the YAML profile.
+ /// First, a call graph is constructed for both profiled and binary
functions.
+ /// Then functions are hashed based on the names of their callee/caller
+ /// functions. Finally, functions are matched based on these neighbor hashes.
+ class CallGraphMatcher {
+ public:
+/// Constructs the call graphs for binary and profiled functions and
+/// computes neighbor hashes for binary functions.
+CallGraphMatcher(BinaryContext , yaml::bolt::BinaryProfile ,
+ ProfileLookupMap );
+
+/// Returns the YamlBFs adjacent to the parameter YamlBF in the call graph.
+std::set *
+getAdjacentYamlBFs(yaml::bolt::BinaryFunctionProfile ) {
+ auto It = YamlBFAdjacencyMap.find();
+ return It == YamlBFAdjacencyMap.end() ? nullptr : >second;
+}
+
+/// Returns the binary functions with the parameter neighbor hash.
+std::vector *
aaupov wrote:
Same here
https://github.com/llvm/llvm-project/pull/98125
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with call graph (PR #98125)
https://github.com/aaupov approved this pull request. LG with a couple of nits https://github.com/llvm/llvm-project/pull/98125 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with call graph (PR #98125)
@@ -43,6 +43,57 @@ class YAMLProfileReader : public ProfileReaderBase {
using ProfileLookupMap =
DenseMap;
+ /// A class for matching binary functions in functions in the YAML profile.
+ /// First, a call graph is constructed for both profiled and binary
functions.
+ /// Then functions are hashed based on the names of their callee/caller
+ /// functions. Finally, functions are matched based on these neighbor hashes.
+ class CallGraphMatcher {
+ public:
+/// Constructs the call graphs for binary and profiled functions and
+/// computes neighbor hashes for binary functions.
+CallGraphMatcher(BinaryContext , yaml::bolt::BinaryProfile ,
+ ProfileLookupMap );
+
+/// Returns the YamlBFs adjacent to the parameter YamlBF in the call graph.
+std::set *
aaupov wrote:
Please use `std::optional` type instead of a null-able pointer.
https://github.com/llvm/llvm-project/pull/98125
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with call graph (PR #98125)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/98125 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with call graph (PR #98125)
@@ -446,6 +503,56 @@ size_t YAMLProfileReader::matchWithLTOCommonName() {
return MatchedWithLTOCommonName;
}
+size_t YAMLProfileReader::matchWithCallGraph(BinaryContext ) {
+ if (!opts::MatchWithCallGraph)
+return 0;
+
+ size_t MatchedWithCallGraph = 0;
+ CGMatcher.computeBFNeighborHashes(BC);
+ CGMatcher.constructYAMLFCG(YamlBP, IdToYamLBF);
+
+ // Matches YAMLBF to BFs with neighbor hashes.
+ for (yaml::bolt::BinaryFunctionProfile : YamlBP.Functions) {
+if (YamlBF.Used)
+ continue;
+auto It = CGMatcher.YamlBFAdjacencyMap.find();
+if (It == CGMatcher.YamlBFAdjacencyMap.end())
+ continue;
+// Computes profiled function's neighbor hash.
+std::set =
+It->second;
+std::string AdjacentFunctionHashStr;
+for (auto : AdjacentFunctions) {
+ AdjacentFunctionHashStr += AdjacentFunction->Name;
+}
+uint64_t Hash = std::hash{}(AdjacentFunctionHashStr);
+auto NeighborHashToBFsIt = CGMatcher.NeighborHashToBFs.find(Hash);
+if (NeighborHashToBFsIt == CGMatcher.NeighborHashToBFs.end())
+ continue;
+// Finds the binary function with the closest block size to the profiled
aaupov wrote:
Two concerns here:
- worst-case runtime - please check the size of the largest NeighborHashToBFs
bucket in a large binary
- closest block count may not find the best match, we'll need to look at a
couple of examples from real binary+stale profile to find out a good proxy.
https://github.com/llvm/llvm-project/pull/98125
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with call graph (PR #98125)
@@ -568,12 +675,30 @@ Error YAMLProfileReader::readProfile(BinaryContext ) {
}
YamlProfileToFunction.resize(YamlBP.Functions.size() + 1);
- // Computes hash for binary functions.
+ // Map profiled function ids to names.
+ for (yaml::bolt::BinaryFunctionProfile : YamlBP.Functions)
+IdToYamLBF[YamlBF.Id] =
+
+ // Creates a vector of lamdas that preprocess binary functions for function
+ // matching to avoid multiple preprocessing passes over binary functions in
+ // different function matching techniques.
+ std::vector> BFPreprocessingFuncs;
aaupov wrote:
Let's move it into a separate NFC change.
https://github.com/llvm/llvm-project/pull/98125
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with call graph (PR #98125)
@@ -16,6 +16,37 @@
namespace llvm {
namespace bolt {
+/// A class for matching binary functions in functions in the YAML profile.
+struct CallGraphMatcher {
aaupov wrote:
Let's make it a proper class
https://github.com/llvm/llvm-project/pull/98125
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with call graph (PR #98125)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/98125 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with call graph (PR #98125)
https://github.com/aaupov commented: Thank you for working on this! It looks very good overall, left a couple of comments inline. Please run this new matching on a large binary to answer questions about runtime and matching quality in ambiguous cases. https://github.com/llvm/llvm-project/pull/98125 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Support POSSIBLE_PIC_FIXED_BRANCH (PR #91667)
https://github.com/aaupov updated
https://github.com/llvm/llvm-project/pull/91667
>From dd4d0de42048c063d5e5095a0c2594c7cc578df5 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Thu, 9 May 2024 19:35:26 -0700
Subject: [PATCH 1/4] Fix RISCVMCPlusBuilder
Created using spr 1.3.4
---
bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
index 74f2f0aae91e6..020e62463ee2f 100644
--- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
+++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
@@ -177,13 +177,14 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
MCInst , InstructionIterator Begin, InstructionIterator End,
const unsigned PtrSize, MCInst *, unsigned ,
unsigned , int64_t , const MCExpr *,
- MCInst *) const override {
+ MCInst *, MCInst *) const override {
MemLocInstr = nullptr;
BaseRegNum = 0;
IndexRegNum = 0;
DispValue = 0;
DispExpr = nullptr;
PCRelBaseOut = nullptr;
+FixedEntryLoadInst = nullptr;
// Check for the following long tail call sequence:
// 1: auipc xi, %pcrel_hi(sym)
>From 62391bb5aa01f2b77d4315d1e72a9924eec9ecc0 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Fri, 5 Jul 2024 14:54:51 -0700
Subject: [PATCH 2/4] Drop deregisterJumpTable
Created using spr 1.3.4
---
bolt/lib/Core/BinaryFunction.cpp | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 09a6ca1d68730..f587d5a2cadd4 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -899,17 +899,9 @@ BinaryFunction::processIndirectBranch(MCInst ,
unsigned Size,
TargetAddress = ArrayStart + *Value;
-// Remove spurious JumpTable at EntryAddress caused by PIC reference from
-// the load instruction.
-JumpTable *JT = BC.getJumpTableContainingAddress(EntryAddress);
-assert(JT && "Must have a jump table at fixed entry address");
-BC.deregisterJumpTable(EntryAddress);
-JumpTables.erase(EntryAddress);
-delete JT;
-
// Replace FixedEntryDispExpr used in target address calculation with outer
// jump table reference.
-JT = BC.getJumpTableContainingAddress(ArrayStart);
+JumpTable *JT = BC.getJumpTableContainingAddress(ArrayStart);
assert(JT && "Must have a containing jump table for PIC fixed branch");
BC.MIB->replaceMemOperandDisp(*FixedEntryLoadInstr, JT->getFirstLabel(),
EntryAddress - ArrayStart, &*BC.Ctx);
>From 5336879ab68aedb1217e2c6c139d171f31e89e03 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Sat, 6 Jul 2024 22:26:14 -0700
Subject: [PATCH 3/4] Surgically drop spurious jump table
Created using spr 1.3.4
---
bolt/include/bolt/Core/BinaryContext.h | 5 +
bolt/lib/Core/BinaryFunction.cpp| 12 ++--
bolt/test/X86/jump-table-fixed-ref-pic.test | 11 ---
3 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/bolt/include/bolt/Core/BinaryContext.h
b/bolt/include/bolt/Core/BinaryContext.h
index 73932c4ca2fb3..c5e2c6cd02179 100644
--- a/bolt/include/bolt/Core/BinaryContext.h
+++ b/bolt/include/bolt/Core/BinaryContext.h
@@ -431,6 +431,11 @@ class BinaryContext {
return nullptr;
}
+ /// Deregister JumpTable registered at a given \p Address.
+ bool deregisterJumpTable(uint64_t Address) {
+return JumpTables.erase(Address);
+ }
+
unsigned getDWARFEncodingSize(unsigned Encoding) {
if (Encoding == dwarf::DW_EH_PE_omit)
return 0;
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index f587d5a2cadd4..2ecca32a5985c 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -899,9 +899,17 @@ BinaryFunction::processIndirectBranch(MCInst ,
unsigned Size,
TargetAddress = ArrayStart + *Value;
+// Remove spurious JumpTable at EntryAddress caused by PIC reference from
+// the load instruction.
+JumpTable *JT = BC.getJumpTableContainingAddress(EntryAddress);
+assert(JT && "Must have a jump table at fixed entry address");
+BC.deregisterJumpTable(EntryAddress);
+JumpTables.erase(EntryAddress);
+delete JT;
+
// Replace FixedEntryDispExpr used in target address calculation with outer
// jump table reference.
-JumpTable *JT = BC.getJumpTableContainingAddress(ArrayStart);
+JT = BC.getJumpTableContainingAddress(ArrayStart);
assert(JT && "Must have a containing jump table for PIC fixed branch");
BC.MIB->replaceMemOperandDisp(*FixedEntryLoadInstr, JT->getFirstLabel(),
EntryAddress - ArrayStart, &*BC.Ctx);
@@ -1158,10 +1166,10 @@ void BinaryFunction::handleIndirectBranch(MCInst
, uint64_t Size,
}
case IndirectBranchType::POSSIBLE_JUMP_TABLE:
case
[llvm-branch-commits] [llvm] [BOLT] Added more details on heatmap docs. (PR #98162)
aaupov wrote: Please retitle as imperative statement and drop trailing dot, e.g. `[BOLT][docs] Expand Heatmaps.md` https://github.com/llvm/llvm-project/pull/98162 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Added more details on heatmap docs. (PR #98162)
https://github.com/aaupov approved this pull request. Awesome, thanks for updating the documentation https://github.com/llvm/llvm-project/pull/98162 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [NFC][BOLT] Rename createDummyReturnFunction to createReturnBody (PR #98448)
https://github.com/aaupov approved this pull request. Suggestion: `createReturnInstructionList` https://github.com/llvm/llvm-project/pull/98448 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
https://github.com/aaupov approved this pull request. https://github.com/llvm/llvm-project/pull/96596 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
@@ -63,8 +63,8 @@ class NameResolver {
}
// Removes a suffix from a function name.
- static StringRef removeSuffix(StringRef Name, StringRef Suffix) {
-const size_t Pos = Name.find(Suffix);
+ static StringRef unify(StringRef Name) {
aaupov wrote:
Let's use a self-descriptive name e.g. `dropNumNames`.
This would follow the convention used in BinaryFunction.h `getPrintName`.
https://github.com/llvm/llvm-project/pull/96596
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
https://github.com/aaupov commented: Thanks, LG with one nit. https://github.com/llvm/llvm-project/pull/96596 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/96596 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
@@ -40,6 +40,8 @@ class YAMLProfileReader : public ProfileReaderBase {
/// Check if the file contains YAML.
static bool isYAML(StringRef Filename);
+ using FunctionMap = DenseMap;
aaupov wrote:
```suggestion
using ProfileLookupMap = DenseMap;
```
https://github.com/llvm/llvm-project/pull/96596
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
@@ -181,20 +182,19 @@ std::string hashBlockCalls(BinaryContext , const
BinaryBasicBlock ) {
/// The same as the $hashBlockCalls function, but for profiled functions.
std::string
-hashBlockCalls(const DenseMap ,
+hashBlockCalls(const DenseMap
+ ,
const yaml::bolt::BinaryBasicBlockProfile ) {
- std::multiset FunctionNames;
+ std::vector FunctionNames;
for (const yaml::bolt::CallSiteInfo : YamlBB.CallSites) {
-auto It = IdToFunctionName.find(CallSiteInfo.DestId);
-if (It == IdToFunctionName.end())
+auto It = IdToYamlFunction.find(CallSiteInfo.DestId);
+if (It == IdToYamlFunction.end())
continue;
-StringRef Name = It->second;
-const size_t Pos = Name.find("(*");
-if (Pos != StringRef::npos)
- Name = Name.substr(0, Pos);
-FunctionNames.insert(std::string(Name));
+StringRef Name =
+NameResolver::removeSuffix(It->second->Name, StringRef("(*"));
aaupov wrote:
I think the intent of Maksim's comment was to hide an implementation detail
(using `(*N)` suffix) in relevant component (NameResolver).
https://github.com/llvm/llvm-project/pull/96596
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
https://github.com/aaupov approved this pull request. Thanks. https://github.com/llvm/llvm-project/pull/96596 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
@@ -35,6 +36,12 @@ std::string hashBlock(BinaryContext , const BinaryBasicBlock , std::string hashBlockLoose(BinaryContext , const BinaryBasicBlock ); +std::string hashBlockCalls(BinaryContext , const BinaryBasicBlock ); + +std::string +hashBlockCalls(const DenseMap , aaupov wrote: Please use StringRef. https://github.com/llvm/llvm-project/pull/96596 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Support POSSIBLE_PIC_FIXED_BRANCH (PR #91667)
https://github.com/aaupov updated
https://github.com/llvm/llvm-project/pull/91667
>From dd4d0de42048c063d5e5095a0c2594c7cc578df5 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Thu, 9 May 2024 19:35:26 -0700
Subject: [PATCH 1/3] Fix RISCVMCPlusBuilder
Created using spr 1.3.4
---
bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
index 74f2f0aae91e66..020e62463ee2f4 100644
--- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
+++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
@@ -177,13 +177,14 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
MCInst , InstructionIterator Begin, InstructionIterator End,
const unsigned PtrSize, MCInst *, unsigned ,
unsigned , int64_t , const MCExpr *,
- MCInst *) const override {
+ MCInst *, MCInst *) const override {
MemLocInstr = nullptr;
BaseRegNum = 0;
IndexRegNum = 0;
DispValue = 0;
DispExpr = nullptr;
PCRelBaseOut = nullptr;
+FixedEntryLoadInst = nullptr;
// Check for the following long tail call sequence:
// 1: auipc xi, %pcrel_hi(sym)
>From 62391bb5aa01f2b77d4315d1e72a9924eec9ecc0 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Fri, 5 Jul 2024 14:54:51 -0700
Subject: [PATCH 2/3] Drop deregisterJumpTable
Created using spr 1.3.4
---
bolt/lib/Core/BinaryFunction.cpp | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 09a6ca1d68730c..f587d5a2cadd49 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -899,17 +899,9 @@ BinaryFunction::processIndirectBranch(MCInst ,
unsigned Size,
TargetAddress = ArrayStart + *Value;
-// Remove spurious JumpTable at EntryAddress caused by PIC reference from
-// the load instruction.
-JumpTable *JT = BC.getJumpTableContainingAddress(EntryAddress);
-assert(JT && "Must have a jump table at fixed entry address");
-BC.deregisterJumpTable(EntryAddress);
-JumpTables.erase(EntryAddress);
-delete JT;
-
// Replace FixedEntryDispExpr used in target address calculation with outer
// jump table reference.
-JT = BC.getJumpTableContainingAddress(ArrayStart);
+JumpTable *JT = BC.getJumpTableContainingAddress(ArrayStart);
assert(JT && "Must have a containing jump table for PIC fixed branch");
BC.MIB->replaceMemOperandDisp(*FixedEntryLoadInstr, JT->getFirstLabel(),
EntryAddress - ArrayStart, &*BC.Ctx);
>From 5336879ab68aedb1217e2c6c139d171f31e89e03 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Sat, 6 Jul 2024 22:26:14 -0700
Subject: [PATCH 3/3] Surgically drop spurious jump table
Created using spr 1.3.4
---
bolt/include/bolt/Core/BinaryContext.h | 5 +
bolt/lib/Core/BinaryFunction.cpp| 12 ++--
bolt/test/X86/jump-table-fixed-ref-pic.test | 11 ---
3 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/bolt/include/bolt/Core/BinaryContext.h
b/bolt/include/bolt/Core/BinaryContext.h
index 73932c4ca2fb33..c5e2c6cd02179e 100644
--- a/bolt/include/bolt/Core/BinaryContext.h
+++ b/bolt/include/bolt/Core/BinaryContext.h
@@ -431,6 +431,11 @@ class BinaryContext {
return nullptr;
}
+ /// Deregister JumpTable registered at a given \p Address.
+ bool deregisterJumpTable(uint64_t Address) {
+return JumpTables.erase(Address);
+ }
+
unsigned getDWARFEncodingSize(unsigned Encoding) {
if (Encoding == dwarf::DW_EH_PE_omit)
return 0;
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index f587d5a2cadd49..2ecca32a5985c0 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -899,9 +899,17 @@ BinaryFunction::processIndirectBranch(MCInst ,
unsigned Size,
TargetAddress = ArrayStart + *Value;
+// Remove spurious JumpTable at EntryAddress caused by PIC reference from
+// the load instruction.
+JumpTable *JT = BC.getJumpTableContainingAddress(EntryAddress);
+assert(JT && "Must have a jump table at fixed entry address");
+BC.deregisterJumpTable(EntryAddress);
+JumpTables.erase(EntryAddress);
+delete JT;
+
// Replace FixedEntryDispExpr used in target address calculation with outer
// jump table reference.
-JumpTable *JT = BC.getJumpTableContainingAddress(ArrayStart);
+JT = BC.getJumpTableContainingAddress(ArrayStart);
assert(JT && "Must have a containing jump table for PIC fixed branch");
BC.MIB->replaceMemOperandDisp(*FixedEntryLoadInstr, JT->getFirstLabel(),
EntryAddress - ArrayStart, &*BC.Ctx);
@@ -1158,10 +1166,10 @@ void BinaryFunction::handleIndirectBranch(MCInst
, uint64_t Size,
}
case IndirectBranchType::POSSIBLE_JUMP_TABLE:
case
[llvm-branch-commits] [llvm] [BOLT] Support POSSIBLE_PIC_FIXED_BRANCH (PR #91667)
https://github.com/aaupov updated
https://github.com/llvm/llvm-project/pull/91667
>From dd4d0de42048c063d5e5095a0c2594c7cc578df5 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Thu, 9 May 2024 19:35:26 -0700
Subject: [PATCH 1/2] Fix RISCVMCPlusBuilder
Created using spr 1.3.4
---
bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
index 74f2f0aae91e66..020e62463ee2f4 100644
--- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
+++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
@@ -177,13 +177,14 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
MCInst , InstructionIterator Begin, InstructionIterator End,
const unsigned PtrSize, MCInst *, unsigned ,
unsigned , int64_t , const MCExpr *,
- MCInst *) const override {
+ MCInst *, MCInst *) const override {
MemLocInstr = nullptr;
BaseRegNum = 0;
IndexRegNum = 0;
DispValue = 0;
DispExpr = nullptr;
PCRelBaseOut = nullptr;
+FixedEntryLoadInst = nullptr;
// Check for the following long tail call sequence:
// 1: auipc xi, %pcrel_hi(sym)
>From 62391bb5aa01f2b77d4315d1e72a9924eec9ecc0 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Fri, 5 Jul 2024 14:54:51 -0700
Subject: [PATCH 2/2] Drop deregisterJumpTable
Created using spr 1.3.4
---
bolt/lib/Core/BinaryFunction.cpp | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 09a6ca1d68730c..f587d5a2cadd49 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -899,17 +899,9 @@ BinaryFunction::processIndirectBranch(MCInst ,
unsigned Size,
TargetAddress = ArrayStart + *Value;
-// Remove spurious JumpTable at EntryAddress caused by PIC reference from
-// the load instruction.
-JumpTable *JT = BC.getJumpTableContainingAddress(EntryAddress);
-assert(JT && "Must have a jump table at fixed entry address");
-BC.deregisterJumpTable(EntryAddress);
-JumpTables.erase(EntryAddress);
-delete JT;
-
// Replace FixedEntryDispExpr used in target address calculation with outer
// jump table reference.
-JT = BC.getJumpTableContainingAddress(ArrayStart);
+JumpTable *JT = BC.getJumpTableContainingAddress(ArrayStart);
assert(JT && "Must have a containing jump table for PIC fixed branch");
BC.MIB->replaceMemOperandDisp(*FixedEntryLoadInstr, JT->getFirstLabel(),
EntryAddress - ArrayStart, &*BC.Ctx);
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Eliminate dead jump tables (PR #91666)
https://github.com/aaupov closed https://github.com/llvm/llvm-project/pull/91666 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Support POSSIBLE_PIC_FIXED_BRANCH (PR #91667)
https://github.com/aaupov updated
https://github.com/llvm/llvm-project/pull/91667
>From dd4d0de42048c063d5e5095a0c2594c7cc578df5 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Thu, 9 May 2024 19:35:26 -0700
Subject: [PATCH] Fix RISCVMCPlusBuilder
Created using spr 1.3.4
---
bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
index 74f2f0aae91e6..020e62463ee2f 100644
--- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
+++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
@@ -177,13 +177,14 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
MCInst , InstructionIterator Begin, InstructionIterator End,
const unsigned PtrSize, MCInst *, unsigned ,
unsigned , int64_t , const MCExpr *,
- MCInst *) const override {
+ MCInst *, MCInst *) const override {
MemLocInstr = nullptr;
BaseRegNum = 0;
IndexRegNum = 0;
DispValue = 0;
DispExpr = nullptr;
PCRelBaseOut = nullptr;
+FixedEntryLoadInst = nullptr;
// Check for the following long tail call sequence:
// 1: auipc xi, %pcrel_hi(sym)
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Support POSSIBLE_PIC_FIXED_BRANCH (PR #91667)
https://github.com/aaupov updated
https://github.com/llvm/llvm-project/pull/91667
>From dd4d0de42048c063d5e5095a0c2594c7cc578df5 Mon Sep 17 00:00:00 2001
From: Amir Ayupov
Date: Thu, 9 May 2024 19:35:26 -0700
Subject: [PATCH] Fix RISCVMCPlusBuilder
Created using spr 1.3.4
---
bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
index 74f2f0aae91e6..020e62463ee2f 100644
--- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
+++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
@@ -177,13 +177,14 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
MCInst , InstructionIterator Begin, InstructionIterator End,
const unsigned PtrSize, MCInst *, unsigned ,
unsigned , int64_t , const MCExpr *,
- MCInst *) const override {
+ MCInst *, MCInst *) const override {
MemLocInstr = nullptr;
BaseRegNum = 0;
IndexRegNum = 0;
DispValue = 0;
DispExpr = nullptr;
PCRelBaseOut = nullptr;
+FixedEntryLoadInst = nullptr;
// Check for the following long tail call sequence:
// 1: auipc xi, %pcrel_hi(sym)
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/96596 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
@@ -220,17 +245,27 @@ class StaleMatcher {
return BestBlock;
}
- /// Returns true if the two basic blocks (in the binary and in the profile)
- /// corresponding to the given hashes are matched to each other with a high
- /// confidence.
- static bool isHighConfidenceMatch(BlendedBlockHash Hash1,
-BlendedBlockHash Hash2) {
-return Hash1.InstrHash == Hash2.InstrHash;
+ // Uses CallHash to find the most similar block for a given hash.
+ const FlowBlock *matchWithCalls(BlendedBlockHash ,
aaupov wrote:
ditto
https://github.com/llvm/llvm-project/pull/96596
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
@@ -193,18 +193,43 @@ class StaleMatcher {
public:
/// Initialize stale matcher.
void init(const std::vector ,
-const std::vector ) {
+const std::vector ,
+const std::vector ) {
assert(Blocks.size() == Hashes.size() &&
+ Hashes.size() == CallHashes.size() &&
"incorrect matcher initialization");
for (size_t I = 0; I < Blocks.size(); I++) {
FlowBlock *Block = Blocks[I];
uint16_t OpHash = Hashes[I].OpcodeHash;
OpHashToBlocks[OpHash].push_back(std::make_pair(Hashes[I], Block));
+ if (CallHashes[I])
+CallHashToBlocks[CallHashes[I]].push_back(
+std::make_pair(Hashes[I], Block));
}
}
/// Find the most similar block for a given hash.
- const FlowBlock *matchBlock(BlendedBlockHash BlendedHash) const {
+ const FlowBlock *matchBlock(BlendedBlockHash ,
+ uint64_t ) const {
+const FlowBlock *BestBlock = matchWithOpcodes(BlendedHash);
+return BestBlock ? BestBlock : matchWithCalls(BlendedHash, CallHash);
+ }
+
+ /// Returns true if the two basic blocks (in the binary and in the profile)
+ /// corresponding to the given hashes are matched to each other with a high
+ /// confidence.
+ static bool isHighConfidenceMatch(BlendedBlockHash Hash1,
+BlendedBlockHash Hash2) {
+return Hash1.InstrHash == Hash2.InstrHash;
+ }
+
+private:
+ using HashBlockPairType = std::pair;
+ std::unordered_map> OpHashToBlocks;
+ std::unordered_map>
CallHashToBlocks;
+
+ // Uses OpcodeHash to find the most similar block for a given hash.
+ const FlowBlock *matchWithOpcodes(BlendedBlockHash ) const {
aaupov wrote:
ditto
https://github.com/llvm/llvm-project/pull/96596
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/96596 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
@@ -193,18 +193,43 @@ class StaleMatcher {
public:
/// Initialize stale matcher.
void init(const std::vector ,
-const std::vector ) {
+const std::vector ,
+const std::vector ) {
assert(Blocks.size() == Hashes.size() &&
+ Hashes.size() == CallHashes.size() &&
"incorrect matcher initialization");
for (size_t I = 0; I < Blocks.size(); I++) {
FlowBlock *Block = Blocks[I];
uint16_t OpHash = Hashes[I].OpcodeHash;
OpHashToBlocks[OpHash].push_back(std::make_pair(Hashes[I], Block));
+ if (CallHashes[I])
+CallHashToBlocks[CallHashes[I]].push_back(
+std::make_pair(Hashes[I], Block));
}
}
/// Find the most similar block for a given hash.
- const FlowBlock *matchBlock(BlendedBlockHash BlendedHash) const {
+ const FlowBlock *matchBlock(BlendedBlockHash ,
+ uint64_t ) const {
aaupov wrote:
```suggestion
const FlowBlock *matchBlock(BlendedBlockHash BlendedHash,
uint64_t CallHash) const {
```
BlendedBlockHash is aliased to uint64_t, and integral types should be passed by
value.
https://github.com/llvm/llvm-project/pull/96596
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
@@ -412,33 +447,62 @@ createFlowFunction(const
BinaryFunction::BasicBlockOrderType ) {
/// of the basic blocks in the binary, the count is "matched" to the block.
/// Similarly, if both the source and the target of a count in the profile are
/// matched to a jump in the binary, the count is recorded in CFG.
-size_t matchWeightsByHashes(
-BinaryContext , const BinaryFunction::BasicBlockOrderType ,
-const yaml::bolt::BinaryFunctionProfile , FlowFunction ) {
+size_t
+matchWeightsByHashes(BinaryContext ,
+ const DenseMap ,
+ const BinaryFunction::BasicBlockOrderType ,
+ const yaml::bolt::BinaryFunctionProfile ,
+ FlowFunction , HashFunction HashFunction) {
aaupov wrote:
```suggestion
size_t
matchWeightsByHashes(BinaryContext ,
const BinaryFunction::BasicBlockOrderType ,
const yaml::bolt::BinaryFunctionProfile ,
FlowFunction , HashFunction HashFunction,
const DenseMap )
{
```
It's customary to add new parameters to the end
https://github.com/llvm/llvm-project/pull/96596
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
https://github.com/aaupov commented: Sorry, couple of final comments https://github.com/llvm/llvm-project/pull/96596 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT][NFC] Refactor function matching (PR #97502)
@@ -456,6 +435,39 @@ Error YAMLProfileReader::readProfile(BinaryContext ) {
++MatchedWithLTOCommonName;
}
}
+ return MatchedWithLTOCommonName;
+}
+
+Error YAMLProfileReader::readProfile(BinaryContext ) {
+ if (opts::Verbosity >= 1) {
+outs() << "BOLT-INFO: YAML profile with hash: ";
+switch (YamlBP.Header.HashFunction) {
+case HashFunction::StdHash:
+ outs() << "std::hash\n";
aaupov wrote:
@ayermolo, we didn't switch Profile component to BC logger class. That would be
a separate effort.
https://github.com/llvm/llvm-project/pull/97502
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT][NFC] Refactor function matching (PR #97502)
https://github.com/aaupov approved this pull request. LG % nit https://github.com/llvm/llvm-project/pull/97502 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT][NFC] Refactor function matching (PR #97502)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/97502 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT][NFC] Refactor function matching (PR #97502)
@@ -334,6 +334,13 @@ Error YAMLProfileReader::preprocessProfile(BinaryContext
) {
return Error::success();
}
+bool YAMLProfileReader::profileMatches(
+const yaml::bolt::BinaryFunctionProfile , BinaryFunction ) {
aaupov wrote:
```suggestion
const yaml::bolt::BinaryFunctionProfile , const BinaryFunction )
{
```
https://github.com/llvm/llvm-project/pull/97502
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
https://github.com/aaupov approved this pull request. LG with a couple of nits. https://github.com/llvm/llvm-project/pull/95884 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
@@ -342,6 +350,107 @@ bool YAMLProfileReader::mayHaveProfileData(const
BinaryFunction ) {
return false;
}
+uint64_t YAMLProfileReader::matchWithNameSimilarity(BinaryContext ) {
+ uint64_t MatchedWithNameSimilarity = 0;
+ ItaniumPartialDemangler Demangler;
+
+ // Demangle and derive namespace from function name.
+ auto DemangleName = [&](std::string ) {
+StringRef RestoredName = NameResolver::restore(FunctionName);
+return demangle(RestoredName);
+ };
+ auto DeriveNameSpace = [&](std::string ) {
+if (Demangler.partialDemangle(DemangledName.c_str()))
+ return std::string("");
+std::vector Buffer(DemangledName.begin(), DemangledName.end());
+size_t BufferSize = Buffer.size();
aaupov wrote:
```suggestion
size_t BufferSize;
```
https://github.com/llvm/llvm-project/pull/95884
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
@@ -342,6 +350,107 @@ bool YAMLProfileReader::mayHaveProfileData(const
BinaryFunction ) {
return false;
}
+uint64_t YAMLProfileReader::matchWithNameSimilarity(BinaryContext ) {
+ uint64_t MatchedWithNameSimilarity = 0;
+ ItaniumPartialDemangler Demangler;
+
+ // Demangle and derive namespace from function name.
+ auto DemangleName = [&](std::string ) {
+StringRef RestoredName = NameResolver::restore(FunctionName);
+return demangle(RestoredName);
+ };
+ auto DeriveNameSpace = [&](std::string ) {
+if (Demangler.partialDemangle(DemangledName.c_str()))
+ return std::string("");
+std::vector Buffer(DemangledName.begin(), DemangledName.end());
+size_t BufferSize = Buffer.size();
+char *NameSpace =
+Demangler.getFunctionDeclContextName([0], );
+return std::string(NameSpace, BufferSize);
+ };
+
+ // Maps namespaces to associated function block counts and gets profile
+ // function names and namespaces to minimize the number of BFs to process and
+ // avoid repeated name demangling/namespace derivation.
+ StringMap> NamespaceToProfiledBFSizes;
+ std::vector ProfileBFDemangledNames;
+ ProfileBFDemangledNames.reserve(YamlBP.Functions.size());
+ std::vector ProfiledBFNamespaces;
+ ProfiledBFNamespaces.reserve(YamlBP.Functions.size());
+
+ for (auto : YamlBP.Functions) {
+std::string YamlBFDemangledName = DemangleName(YamlBF.Name);
+ProfileBFDemangledNames.push_back(YamlBFDemangledName);
+std::string YamlBFNamespace = DeriveNameSpace(YamlBFDemangledName);
+ProfiledBFNamespaces.push_back(YamlBFNamespace);
+NamespaceToProfiledBFSizes[YamlBFNamespace].insert(YamlBF.NumBasicBlocks);
+ }
+
+ StringMap> NamespaceToBFs;
+
+ // Maps namespaces to BFs excluding binary functions with no equal sized
+ // profiled functions belonging to the same namespace.
+ for (BinaryFunction *BF : BC.getAllBinaryFunctions()) {
+std::string DemangledName = BF->getDemangledName();
+std::string Namespace = DeriveNameSpace(DemangledName);
+
+auto NamespaceToProfiledBFSizesIt =
+NamespaceToProfiledBFSizes.find(Namespace);
+if (NamespaceToProfiledBFSizesIt == NamespaceToProfiledBFSizes.end())
aaupov wrote:
```suggestion
// Skip if there are no ProfileBFs with a given \p Namespace.
if (NamespaceToProfiledBFSizesIt == NamespaceToProfiledBFSizes.end())
```
https://github.com/llvm/llvm-project/pull/95884
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
@@ -342,6 +350,107 @@ bool YAMLProfileReader::mayHaveProfileData(const
BinaryFunction ) {
return false;
}
+uint64_t YAMLProfileReader::matchWithNameSimilarity(BinaryContext ) {
+ uint64_t MatchedWithNameSimilarity = 0;
+ ItaniumPartialDemangler Demangler;
+
+ // Demangle and derive namespace from function name.
+ auto DemangleName = [&](std::string ) {
+StringRef RestoredName = NameResolver::restore(FunctionName);
+return demangle(RestoredName);
+ };
+ auto DeriveNameSpace = [&](std::string ) {
+if (Demangler.partialDemangle(DemangledName.c_str()))
+ return std::string("");
+std::vector Buffer(DemangledName.begin(), DemangledName.end());
+size_t BufferSize = Buffer.size();
+char *NameSpace =
+Demangler.getFunctionDeclContextName([0], );
+return std::string(NameSpace, BufferSize);
+ };
+
+ // Maps namespaces to associated function block counts and gets profile
+ // function names and namespaces to minimize the number of BFs to process and
+ // avoid repeated name demangling/namespace derivation.
+ StringMap> NamespaceToProfiledBFSizes;
+ std::vector ProfileBFDemangledNames;
+ ProfileBFDemangledNames.reserve(YamlBP.Functions.size());
+ std::vector ProfiledBFNamespaces;
+ ProfiledBFNamespaces.reserve(YamlBP.Functions.size());
+
+ for (auto : YamlBP.Functions) {
+std::string YamlBFDemangledName = DemangleName(YamlBF.Name);
+ProfileBFDemangledNames.push_back(YamlBFDemangledName);
+std::string YamlBFNamespace = DeriveNameSpace(YamlBFDemangledName);
+ProfiledBFNamespaces.push_back(YamlBFNamespace);
+NamespaceToProfiledBFSizes[YamlBFNamespace].insert(YamlBF.NumBasicBlocks);
+ }
+
+ StringMap> NamespaceToBFs;
+
+ // Maps namespaces to BFs excluding binary functions with no equal sized
+ // profiled functions belonging to the same namespace.
+ for (BinaryFunction *BF : BC.getAllBinaryFunctions()) {
+std::string DemangledName = BF->getDemangledName();
+std::string Namespace = DeriveNameSpace(DemangledName);
+
+auto NamespaceToProfiledBFSizesIt =
+NamespaceToProfiledBFSizes.find(Namespace);
+if (NamespaceToProfiledBFSizesIt == NamespaceToProfiledBFSizes.end())
+ continue;
+if (NamespaceToProfiledBFSizesIt->second.count(BF->size()) == 0)
aaupov wrote:
```suggestion
// Skip if there are no ProfileBFs in a given \p Namespace with
// equal number of blocks.
if (NamespaceToProfiledBFSizesIt->second.count(BF->size()) == 0)
```
https://github.com/llvm/llvm-project/pull/95884
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
@@ -342,6 +350,107 @@ bool YAMLProfileReader::mayHaveProfileData(const
BinaryFunction ) {
return false;
}
+uint64_t YAMLProfileReader::matchWithNameSimilarity(BinaryContext ) {
+ uint64_t MatchedWithNameSimilarity = 0;
+ ItaniumPartialDemangler Demangler;
+
+ // Demangle and derive namespace from function name.
+ auto DemangleName = [&](std::string ) {
+StringRef RestoredName = NameResolver::restore(FunctionName);
+return demangle(RestoredName);
+ };
+ auto DeriveNameSpace = [&](std::string ) {
+if (Demangler.partialDemangle(DemangledName.c_str()))
+ return std::string("");
+std::vector Buffer(DemangledName.begin(), DemangledName.end());
+size_t BufferSize = Buffer.size();
+char *NameSpace =
+Demangler.getFunctionDeclContextName([0], );
+return std::string(NameSpace, BufferSize);
+ };
+
+ // Maps namespaces to associated function block counts and gets profile
+ // function names and namespaces to minimize the number of BFs to process and
+ // avoid repeated name demangling/namespace derivation.
+ StringMap> NamespaceToProfiledBFSizes;
+ std::vector ProfileBFDemangledNames;
+ ProfileBFDemangledNames.reserve(YamlBP.Functions.size());
+ std::vector ProfiledBFNamespaces;
+ ProfiledBFNamespaces.reserve(YamlBP.Functions.size());
+
+ for (auto : YamlBP.Functions) {
+std::string YamlBFDemangledName = DemangleName(YamlBF.Name);
+ProfileBFDemangledNames.push_back(YamlBFDemangledName);
+std::string YamlBFNamespace = DeriveNameSpace(YamlBFDemangledName);
+ProfiledBFNamespaces.push_back(YamlBFNamespace);
+NamespaceToProfiledBFSizes[YamlBFNamespace].insert(YamlBF.NumBasicBlocks);
+ }
+
+ StringMap> NamespaceToBFs;
+
+ // Maps namespaces to BFs excluding binary functions with no equal sized
+ // profiled functions belonging to the same namespace.
+ for (BinaryFunction *BF : BC.getAllBinaryFunctions()) {
+std::string DemangledName = BF->getDemangledName();
+std::string Namespace = DeriveNameSpace(DemangledName);
+
+auto NamespaceToProfiledBFSizesIt =
+NamespaceToProfiledBFSizes.find(Namespace);
+if (NamespaceToProfiledBFSizesIt == NamespaceToProfiledBFSizes.end())
+ continue;
+if (NamespaceToProfiledBFSizesIt->second.count(BF->size()) == 0)
+ continue;
+auto NamespaceToBFsIt = NamespaceToBFs.find(Namespace);
+if (NamespaceToBFsIt == NamespaceToBFs.end())
+ NamespaceToBFs[Namespace] = {BF};
+else
+ NamespaceToBFsIt->second.push_back(BF);
+ }
+
+ // Iterates through all profiled functions and binary functions belonging to
+ // the same namespace and matches based on edit distance thresehold.
aaupov wrote:
```suggestion
// the same namespace and matches based on edit distance threshold.
```
https://github.com/llvm/llvm-project/pull/95884
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
@@ -342,6 +350,107 @@ bool YAMLProfileReader::mayHaveProfileData(const
BinaryFunction ) {
return false;
}
+uint64_t YAMLProfileReader::matchWithNameSimilarity(BinaryContext ) {
+ uint64_t MatchedWithNameSimilarity = 0;
+ ItaniumPartialDemangler Demangler;
+
+ // Demangle and derive namespace from function name.
+ auto DemangleName = [&](std::string ) {
+StringRef RestoredName = NameResolver::restore(FunctionName);
+return demangle(RestoredName);
+ };
+ auto DeriveNameSpace = [&](std::string ) {
+if (Demangler.partialDemangle(DemangledName.c_str()))
+ return std::string("");
+std::vector Buffer(DemangledName.begin(), DemangledName.end());
+size_t BufferSize = Buffer.size();
+char *NameSpace =
+Demangler.getFunctionDeclContextName([0], );
+return std::string(NameSpace, BufferSize);
+ };
+
+ // Maps namespaces to associated function block counts and gets profile
+ // function names and namespaces to minimize the number of BFs to process and
+ // avoid repeated name demangling/namespace derivation.
+ StringMap> NamespaceToProfiledBFSizes;
+ std::vector ProfileBFDemangledNames;
+ ProfileBFDemangledNames.reserve(YamlBP.Functions.size());
+ std::vector ProfiledBFNamespaces;
+ ProfiledBFNamespaces.reserve(YamlBP.Functions.size());
+
+ for (auto : YamlBP.Functions) {
+std::string YamlBFDemangledName = DemangleName(YamlBF.Name);
+ProfileBFDemangledNames.push_back(YamlBFDemangledName);
+std::string YamlBFNamespace = DeriveNameSpace(YamlBFDemangledName);
+ProfiledBFNamespaces.push_back(YamlBFNamespace);
+NamespaceToProfiledBFSizes[YamlBFNamespace].insert(YamlBF.NumBasicBlocks);
+ }
+
+ StringMap> NamespaceToBFs;
+
+ // Maps namespaces to BFs excluding binary functions with no equal sized
+ // profiled functions belonging to the same namespace.
+ for (BinaryFunction *BF : BC.getAllBinaryFunctions()) {
+std::string DemangledName = BF->getDemangledName();
+std::string Namespace = DeriveNameSpace(DemangledName);
+
+auto NamespaceToProfiledBFSizesIt =
+NamespaceToProfiledBFSizes.find(Namespace);
+if (NamespaceToProfiledBFSizesIt == NamespaceToProfiledBFSizes.end())
+ continue;
+if (NamespaceToProfiledBFSizesIt->second.count(BF->size()) == 0)
+ continue;
+auto NamespaceToBFsIt = NamespaceToBFs.find(Namespace);
+if (NamespaceToBFsIt == NamespaceToBFs.end())
+ NamespaceToBFs[Namespace] = {BF};
+else
+ NamespaceToBFsIt->second.push_back(BF);
+ }
+
+ // Iterates through all profiled functions and binary functions belonging to
+ // the same namespace and matches based on edit distance thresehold.
+ assert(YamlBP.Functions.size() == ProfiledBFNamespaces.size() &&
+ ProfiledBFNamespaces.size() == ProfileBFDemangledNames.size());
+ for (size_t I = 0; I < YamlBP.Functions.size(); ++I) {
+yaml::bolt::BinaryFunctionProfile = YamlBP.Functions[I];
+std::string = ProfiledBFNamespaces[I];
+if (YamlBF.Used)
+ continue;
+auto It = NamespaceToBFs.find(YamlBFNamespace);
aaupov wrote:
```suggestion
// Skip if there are no BFs in a given \p Namespace.
auto It = NamespaceToBFs.find(YamlBFNamespace);
```
https://github.com/llvm/llvm-project/pull/95884
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/95884 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
@@ -70,12 +70,16 @@ class YAMLProfileReader : public ProfileReaderBase {
std::vector ProfileBFs;
/// Populate \p Function profile with the one supplied in YAML format.
- bool parseFunctionProfile(BinaryFunction ,
-const yaml::bolt::BinaryFunctionProfile );
+ bool parseFunctionProfile(
+ const DenseMap ,
aaupov wrote:
Define the map as YAMLProfileReader member, with a comment about its use.
https://github.com/llvm/llvm-project/pull/96596
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match blocks with calls as anchors (PR #96596)
@@ -479,6 +481,11 @@ Error YAMLProfileReader::readProfile(BinaryContext ) {
NormalizeByInsnCount = usesEvent("cycles") || usesEvent("instructions");
NormalizeByCalls = usesEvent("branches");
+ // Map profiled function ids to names.
+ DenseMap IdToFunctionName;
aaupov wrote:
Do we need a map and not a vector?
https://github.com/llvm/llvm-project/pull/96596
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
@@ -342,6 +350,107 @@ bool YAMLProfileReader::mayHaveProfileData(const
BinaryFunction ) {
return false;
}
+uint64_t YAMLProfileReader::matchWithNameSimilarity(BinaryContext ) {
+ uint64_t MatchedWithNameSimilarity = 0;
+ ItaniumPartialDemangler ItaniumPartialDemangler;
+
+ // Demangle and derive namespace from function name.
+ auto DemangleName = [&](std::string ) {
+StringRef RestoredName = NameResolver::restore(FunctionName);
+return demangle(RestoredName);
+ };
+ auto DeriveNameSpace = [&](std::string ) {
+if (ItaniumPartialDemangler.partialDemangle(DemangledName.c_str()))
+ return std::string("");
+std::vector Buffer(DemangledName.begin(), DemangledName.end());
+size_t BufferSize = Buffer.size();
+char *NameSpace = ItaniumPartialDemangler.getFunctionDeclContextName(
+[0], );
+return NameSpace ? std::string(NameSpace) : std::string("");
aaupov wrote:
```suggestion
return std::string(NameSpace, BufferSize);
```
https://github.com/llvm/llvm-project/pull/95884
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
https://github.com/aaupov commented: LG in general with some comments https://github.com/llvm/llvm-project/pull/95884 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
@@ -342,6 +350,107 @@ bool YAMLProfileReader::mayHaveProfileData(const
BinaryFunction ) {
return false;
}
+uint64_t YAMLProfileReader::matchWithNameSimilarity(BinaryContext ) {
+ uint64_t MatchedWithNameSimilarity = 0;
+ ItaniumPartialDemangler ItaniumPartialDemangler;
aaupov wrote:
```suggestion
ItaniumPartialDemangler Demangler;
```
https://github.com/llvm/llvm-project/pull/95884
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
@@ -342,6 +350,107 @@ bool YAMLProfileReader::mayHaveProfileData(const
BinaryFunction ) {
return false;
}
+uint64_t YAMLProfileReader::matchWithNameSimilarity(BinaryContext ) {
+ uint64_t MatchedWithNameSimilarity = 0;
+ ItaniumPartialDemangler ItaniumPartialDemangler;
+
+ // Demangle and derive namespace from function name.
+ auto DemangleName = [&](std::string ) {
+StringRef RestoredName = NameResolver::restore(FunctionName);
+return demangle(RestoredName);
+ };
+ auto DeriveNameSpace = [&](std::string ) {
+if (ItaniumPartialDemangler.partialDemangle(DemangledName.c_str()))
+ return std::string("");
+std::vector Buffer(DemangledName.begin(), DemangledName.end());
+size_t BufferSize = Buffer.size();
+char *NameSpace = ItaniumPartialDemangler.getFunctionDeclContextName(
+[0], );
aaupov wrote:
What's bothering me here is the use of BufferSize – it's an output parameter
from `getFunctionDecContextName` and yet unused in the construction of output
string.
https://github.com/llvm/llvm-project/pull/95884
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
@@ -342,6 +350,107 @@ bool YAMLProfileReader::mayHaveProfileData(const
BinaryFunction ) {
return false;
}
+uint64_t YAMLProfileReader::matchWithNameSimilarity(BinaryContext ) {
+ uint64_t MatchedWithNameSimilarity = 0;
+ ItaniumPartialDemangler ItaniumPartialDemangler;
+
+ // Demangle and derive namespace from function name.
+ auto DemangleName = [&](std::string ) {
+StringRef RestoredName = NameResolver::restore(FunctionName);
+return demangle(RestoredName);
+ };
+ auto DeriveNameSpace = [&](std::string ) {
+if (ItaniumPartialDemangler.partialDemangle(DemangledName.c_str()))
+ return std::string("");
+std::vector Buffer(DemangledName.begin(), DemangledName.end());
+size_t BufferSize = Buffer.size();
+char *NameSpace = ItaniumPartialDemangler.getFunctionDeclContextName(
+[0], );
+return NameSpace ? std::string(NameSpace) : std::string("");
+ };
+
+ // Maps namespaces to associated function block counts and gets profile
+ // function names and namespaces to minimize the number of BFs to process and
+ // avoid repeated name demangling/namespace derivision.
+ StringMap> NamespaceToProfiledBFSizes;
+ std::vector ProfileBFDemangledNames;
+ ProfileBFDemangledNames.reserve(YamlBP.Functions.size());
+ std::vector ProfiledBFNamespaces;
+ ProfiledBFNamespaces.reserve(YamlBP.Functions.size());
+
+ for (auto : YamlBP.Functions) {
+std::string YamlBFDemangledName = DemangleName(YamlBF.Name);
+ProfileBFDemangledNames.push_back(YamlBFDemangledName);
+std::string YamlBFNamespace = DeriveNameSpace(YamlBFDemangledName);
+ProfiledBFNamespaces.push_back(YamlBFNamespace);
+NamespaceToProfiledBFSizes[YamlBFNamespace].insert(YamlBF.NumBasicBlocks);
+ }
+
+ StringMap> NamespaceToBFs;
+
+ // Maps namespaces to BFs disincluding binary functions with no equal sized
aaupov wrote:
```suggestion
// Maps namespaces to BFs excluding binary functions with no equal sized
```
https://github.com/llvm/llvm-project/pull/95884
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
@@ -342,6 +350,107 @@ bool YAMLProfileReader::mayHaveProfileData(const
BinaryFunction ) {
return false;
}
+uint64_t YAMLProfileReader::matchWithNameSimilarity(BinaryContext ) {
+ uint64_t MatchedWithNameSimilarity = 0;
+ ItaniumPartialDemangler ItaniumPartialDemangler;
+
+ // Demangle and derive namespace from function name.
+ auto DemangleName = [&](std::string ) {
+StringRef RestoredName = NameResolver::restore(FunctionName);
+return demangle(RestoredName);
+ };
+ auto DeriveNameSpace = [&](std::string ) {
+if (ItaniumPartialDemangler.partialDemangle(DemangledName.c_str()))
+ return std::string("");
+std::vector Buffer(DemangledName.begin(), DemangledName.end());
+size_t BufferSize = Buffer.size();
+char *NameSpace = ItaniumPartialDemangler.getFunctionDeclContextName(
+[0], );
+return NameSpace ? std::string(NameSpace) : std::string("");
+ };
+
+ // Maps namespaces to associated function block counts and gets profile
+ // function names and namespaces to minimize the number of BFs to process and
+ // avoid repeated name demangling/namespace derivision.
aaupov wrote:
```suggestion
// avoid repeated name demangling/namespace derivation.
```
https://github.com/llvm/llvm-project/pull/95884
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
@@ -23,6 +26,11 @@ extern cl::opt Verbosity;
extern cl::OptionCategory BoltOptCategory;
extern cl::opt InferStaleProfile;
+cl::opt NameSimilarityFunctionMatchingThreshold(
+"name-similarity-function-matching-threshold",
+cl::desc("Match functions using namespace and edit distance."),
cl::init(0),
aaupov wrote:
```suggestion
cl::desc("Match functions using namespace and edit distance"), cl::init(0),
```
https://github.com/llvm/llvm-project/pull/95884
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Match functions with name similarity (PR #95884)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/95884 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Drop macro-fusion alignment (PR #97358)
aaupov wrote: It's not. I landed to main manually. https://github.com/llvm/llvm-project/pull/97358 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Drop macro-fusion alignment (PR #97358)
https://github.com/aaupov closed https://github.com/llvm/llvm-project/pull/97358 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Drop macro-fusion alignment (PR #97358)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/97358 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Drop macro-fusion alignment (PR #97358)
https://github.com/aaupov ready_for_review https://github.com/llvm/llvm-project/pull/97358 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT] Drop macro-fusion alignment (PR #97358)
https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/97358 9d0754ada5dbbc0c009bcc2f7824488419cc5530 dropped MC support required for macro-fusion alignment in BOLT. Remove the support in BOLT. Test Plan: macro-fusion alignment was never upstreamed, so no upstream tests are affected. ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT][NFC] Refactoring CallGraph (PR #96922)
aaupov wrote: Please also retitle as an imperative statement, e.g. "Move CallGraph from Passes to Core" https://github.com/llvm/llvm-project/pull/96922 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT][NFC] Refactoring CallGraph (PR #96922)
https://github.com/aaupov approved this pull request. LGTM but please ensure that the diff passes NFC checks and shared build work. https://github.com/llvm/llvm-project/pull/96922 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT][NFC] Refactoring CallGraph (PR #96922)
@@ -10,7 +10,7 @@ // //===--===// -#include "bolt/Passes/CallGraph.h" +#include "bolt/Core/CallGraph.h" aaupov wrote: Please also update file headers (first line) https://github.com/llvm/llvm-project/pull/96922 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [BOLT][NFC] Refactoring CallGraph (PR #96922)
https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/96922 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
