[llvm-branch-commits] [BOLT][NFC] Print timers in perf2bolt invocation (PR #101270)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.

Thanks.

https://github.com/llvm/llvm-project/pull/101270
___
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)

[Davide Italiano via llvm-branch-commits]

@@ -223,10 +249,31 @@ class StaleMatcher {
 return Hash1.InstrHash == Hash2.InstrHash;
   }
 
+  /// Returns true if a profiled block was matched with its pseudo probe.
+  bool isPseudoProbeMatch(BlendedBlockHash YamlBBHash) {
+return MatchedWithPseudoProbes.find(YamlBBHash.combine()) !=
+   MatchedWithPseudoProbes.end();
+  }
+
+  /// Returns the number of blocks matched with opcodes.
+  size_t getNumBlocksMatchedWithOpcodes() const { return MatchedWithOpcodes; }
+
+  /// Returns the number of blocks matched with pseudo probes.
+  size_t getNumBlocksMatchedWithPseudoProbes() const {
+return MatchedWithPseudoProbes.size();
+  }
+
 private:
   using HashBlockPairType = std::pair;
   std::unordered_map> OpHashToBlocks;
   std::unordered_map> 
CallHashToBlocks;
+  std::unordered_map>

dcci wrote:

It's not better, but there's a use case for both of them. My question is mostly 
about why we use one instead of the other.

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)

[Davide Italiano via llvm-branch-commits]

@@ -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";
+
+// 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;

dcci wrote:

SmallVector?

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)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci commented:

some comments.

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)

[Davide Italiano via llvm-branch-commits]

@@ -223,10 +249,31 @@ class StaleMatcher {
 return Hash1.InstrHash == Hash2.InstrHash;
   }
 
+  /// Returns true if a profiled block was matched with its pseudo probe.
+  bool isPseudoProbeMatch(BlendedBlockHash YamlBBHash) {
+return MatchedWithPseudoProbes.find(YamlBBHash.combine()) !=
+   MatchedWithPseudoProbes.end();
+  }
+
+  /// Returns the number of blocks matched with opcodes.
+  size_t getNumBlocksMatchedWithOpcodes() const { return MatchedWithOpcodes; }
+
+  /// Returns the number of blocks matched with pseudo probes.
+  size_t getNumBlocksMatchedWithPseudoProbes() const {
+return MatchedWithPseudoProbes.size();
+  }
+
 private:
   using HashBlockPairType = std::pair;
   std::unordered_map> OpHashToBlocks;
   std::unordered_map> 
CallHashToBlocks;
+  std::unordered_map>

dcci wrote:

usual Q, how come we're using `std::unordered` rather than an LLVM ADT?

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)

[Davide Italiano via llvm-branch-commits]

@@ -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";
+
+// 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) {
+  if (opts::Verbosity >= 3)
+errs() << "BOLT-WARNING: no pseudo probes in profile block\n";
+  return nullptr;
+}
+if (BlockPseudoProbes.size() > 1) {
+  if (opts::Verbosity >= 3)
+errs() << "BOLT-WARNING: more than 1 pseudo probes in profile block\n";
+  return nullptr;
+}
+uint64_t Index = BlockPseudoProbes[0]->Index;
+auto It = IndexToBBPseudoProbes.find(Index);
+if (It == IndexToBBPseudoProbes.end()) {
+  if (opts::Verbosity >= 3)
+errs() << "BOLT-WARNING: no block pseudo probes found within binary "
+  "block at index\n";
+  return nullptr;
+}
+if (It->second.size() > 1) {
+  if (opts::Verbosity >= 3)

dcci wrote:

there's some repetition here 
```
if (verbosity >= 3) { ... }
```

I wonder if you can abstract this in a common helper that would help you 
simplifying the code.

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)

[Davide Italiano via llvm-branch-commits]

@@ -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";
+
+// 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) {
+  if (opts::Verbosity >= 3)
+errs() << "BOLT-WARNING: no pseudo probes in profile block\n";
+  return nullptr;
+}
+if (BlockPseudoProbes.size() > 1) {

dcci wrote:

you can probably hoist the computation of size and compute it only once.

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)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci edited 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] Support more than two jump table parents (PR #99988)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.


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 functions with call graph (PR #98125)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci commented:

LG

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)

[Davide Italiano via llvm-branch-commits]

@@ -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) {

dcci wrote:

Single line generally bodies of conditionals don't have braces in LLVM

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)

[Davide Italiano via llvm-branch-commits]

@@ -50,6 +54,59 @@ llvm::cl::opt ProfileUseDFS("profile-use-dfs",
 namespace llvm {
 namespace bolt {
 
+void CallGraphMatcher::addBFCGEdges(BinaryContext ,
+yaml::bolt::BinaryProfile ,
+BinaryFunction *BF) {
+  for (const BinaryBasicBlock  : BF->blocks()) {
+for (const MCInst  : BB) {
+  if (!BC.MIB->isCall(Instr))
+continue;
+  const MCSymbol *CallSymbol = BC.MIB->getTargetSymbol(Instr);
+  if (!CallSymbol)
+continue;
+  BinaryData *BD = BC.getBinaryDataByName(CallSymbol->getName());
+  if (!BD)
+continue;
+  BinaryFunction *CalleeBF = BC.getFunctionForSymbol(BD->getSymbol());

dcci wrote:

Is it possible to have a caller without a callee?
This sounds like it should be an assertion.

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)

[Davide Italiano via llvm-branch-commits]

@@ -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

dcci wrote:

`lamdas` -> `lambdas`

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)

[Davide Italiano via llvm-branch-commits]

@@ -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  =

dcci wrote:

why is this a set and not a `SmallSet`/`DenseSet` ?

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)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci 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)

[Davide Italiano via llvm-branch-commits]

@@ -16,6 +16,37 @@
 namespace llvm {
 namespace bolt {
 
+/// A class for matching binary functions in functions in the YAML profile.

dcci wrote:

I think it would be useful to add a general description of the algorithm, in 
particular if you have a reference to the paper, somewhere.

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)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci commented:

This is heading in the right direction.

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 blocks with calls as anchors (PR #96596)

[Davide Italiano via llvm-branch-commits]

dcci wrote:

(assuming Amir is happy)

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)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.

LGTM

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)

[Davide Italiano via llvm-branch-commits]

dcci wrote:

LG with the other comments addressed

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)

[Davide Italiano via llvm-branch-commits]

@@ -414,31 +449,59 @@ createFlowFunction(const 
BinaryFunction::BasicBlockOrderType ) {
 /// 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 ) {
+const yaml::bolt::BinaryFunctionProfile , FlowFunction ,
+HashFunction HashFunction,
+const DenseMap ) {
+
   assert(Func.Blocks.size() == BlockOrder.size() + 2);
 
+  std::vector CallHashes;
   std::vector Blocks;
   std::vector BlendedHashes;
   for (uint64_t I = 0; I < BlockOrder.size(); I++) {
 const BinaryBasicBlock *BB = BlockOrder[I];
 assert(BB->getHash() != 0 && "empty hash of BinaryBasicBlock");
+
+std::string CallHashStr = hashBlockCalls(BC, *BB);
+if (CallHashStr.empty()) {
+  CallHashes.push_back(0);
+} else if (HashFunction == HashFunction::StdHash) {
+  CallHashes.push_back(std::hash{}(CallHashStr));
+} else if (HashFunction == HashFunction::XXH3) {

dcci wrote:

then it should be
```
if (hash.empty()) {
  ...
} else {
  if (hash == xxh3) {}
  if (hash == stdhash) {}
}
```

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)

[Davide Italiano via llvm-branch-commits]

@@ -155,5 +155,51 @@ std::string hashBlockLoose(BinaryContext , const 
BinaryBasicBlock ) {
   return HashString;
 }
 
+/// An even looser hash of a basic block to use with stale profile matching,
+/// composed of the names of a block's called functions in lexicographic order.
+std::string hashBlockCalls(BinaryContext , const BinaryBasicBlock ) {
+  // The hash is computed by creating a string of all lexicographically ordered
+  // called function names.
+  std::multiset FunctionNames;
+  for (const MCInst  : BB) {
+// Skip non-call instructions.
+if (!BC.MIB->isCall(Instr))
+  continue;
+const MCSymbol *CallSymbol = BC.MIB->getTargetSymbol(Instr);
+if (!CallSymbol)

dcci wrote:

Did you check it can happen in the PLT pass, or you're copying what they do 
here?

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)

[Davide Italiano via llvm-branch-commits]

dcci wrote:

Can we have @rafaelauler looking at this when he's back?

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] Match blocks with calls as anchors (PR #96596)

[Davide Italiano via llvm-branch-commits]

@@ -155,5 +155,51 @@ std::string hashBlockLoose(BinaryContext , const 
BinaryBasicBlock ) {
   return HashString;
 }
 
+/// An even looser hash of a basic block to use with stale profile matching,
+/// composed of the names of a block's called functions in lexicographic order.
+std::string hashBlockCalls(BinaryContext , const BinaryBasicBlock ) {
+  // The hash is computed by creating a string of all lexicographically ordered
+  // called function names.
+  std::multiset FunctionNames;
+  for (const MCInst  : BB) {
+// Skip non-call instructions.
+if (!BC.MIB->isCall(Instr))
+  continue;
+const MCSymbol *CallSymbol = BC.MIB->getTargetSymbol(Instr);
+if (!CallSymbol)
+  continue;
+FunctionNames.insert(std::string(CallSymbol->getName()));
+  }
+
+  std::string HashString;
+  for (const std::string  : FunctionNames)
+HashString.append(FunctionName);
+
+  return HashString;
+}
+
+/// The same as the above function, but for profiled functions.

dcci wrote:

functions can be moved around the file, I wouldn't  say "same as above" -- as 
above can change.

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)

[Davide Italiano via llvm-branch-commits]

@@ -155,5 +155,51 @@ std::string hashBlockLoose(BinaryContext , const 
BinaryBasicBlock ) {
   return HashString;
 }
 
+/// An even looser hash of a basic block to use with stale profile matching,

dcci wrote:

You can say: a looser version of $SOMETHING and make it more explicit.

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)

[Davide Italiano via llvm-branch-commits]

@@ -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 ,

dcci wrote:

how come here you're using pointers to strings rather than references?

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)

[Davide Italiano via llvm-branch-commits]

@@ -414,31 +449,59 @@ createFlowFunction(const 
BinaryFunction::BasicBlockOrderType ) {
 /// 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 ) {
+const yaml::bolt::BinaryFunctionProfile , FlowFunction ,
+HashFunction HashFunction,
+const DenseMap ) {
+
   assert(Func.Blocks.size() == BlockOrder.size() + 2);
 
+  std::vector CallHashes;
   std::vector Blocks;
   std::vector BlendedHashes;
   for (uint64_t I = 0; I < BlockOrder.size(); I++) {
 const BinaryBasicBlock *BB = BlockOrder[I];
 assert(BB->getHash() != 0 && "empty hash of BinaryBasicBlock");
+
+std::string CallHashStr = hashBlockCalls(BC, *BB);
+if (CallHashStr.empty()) {
+  CallHashes.push_back(0);
+} else if (HashFunction == HashFunction::StdHash) {
+  CallHashes.push_back(std::hash{}(CallHashStr));
+} else if (HashFunction == HashFunction::XXH3) {
+  CallHashes.push_back(llvm::xxh3_64bits(CallHashStr));
+} else {
+  llvm_unreachable("Unhandled HashFunction");
+}
+
 Blocks.push_back([I + 1]);
 BlendedBlockHash BlendedHash(BB->getHash());
 BlendedHashes.push_back(BlendedHash);
 LLVM_DEBUG(dbgs() << "BB with index " << I << " has hash = "
   << Twine::utohexstr(BB->getHash()) << "\n");
   }
   StaleMatcher Matcher;
-  Matcher.init(Blocks, BlendedHashes);
+  Matcher.init(Blocks, BlendedHashes, CallHashes);
 
   // Index in yaml profile => corresponding (matched) block
   DenseMap MatchedBlocks;
   // Match blocks from the profile to the blocks in CFG
   for (const yaml::bolt::BinaryBasicBlockProfile  : YamlBF.Blocks) {
 assert(YamlBB.Hash != 0 && "empty hash of BinaryBasicBlockProfile");
 BlendedBlockHash YamlHash(YamlBB.Hash);
-const FlowBlock *MatchedBlock = Matcher.matchBlock(YamlHash);
-// Always match the entry block.
+
+const FlowBlock *MatchedBlock = nullptr;
+std::string CallHashStr = hashBlockCalls(IdToFunctionName, YamlBB);
+uint64_t CallHash = 0;
+if (CallHashStr.empty()) { // Noop
+} else if (HashFunction == HashFunction::StdHash) {
+  CallHash = std::hash{}(CallHashStr);
+} else if (HashFunction == HashFunction::XXH3) {
+  CallHash = llvm::xxh3_64bits(CallHashStr);
+} else {
+  llvm_unreachable("Unhandled HashFunction");

dcci wrote:

you might want to add a more informative message.

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)

[Davide Italiano via llvm-branch-commits]

@@ -414,31 +449,59 @@ createFlowFunction(const 
BinaryFunction::BasicBlockOrderType ) {
 /// 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 ) {
+const yaml::bolt::BinaryFunctionProfile , FlowFunction ,
+HashFunction HashFunction,
+const DenseMap ) {
+
   assert(Func.Blocks.size() == BlockOrder.size() + 2);
 
+  std::vector CallHashes;
   std::vector Blocks;
   std::vector BlendedHashes;
   for (uint64_t I = 0; I < BlockOrder.size(); I++) {
 const BinaryBasicBlock *BB = BlockOrder[I];
 assert(BB->getHash() != 0 && "empty hash of BinaryBasicBlock");
+
+std::string CallHashStr = hashBlockCalls(BC, *BB);
+if (CallHashStr.empty()) {
+  CallHashes.push_back(0);
+} else if (HashFunction == HashFunction::StdHash) {
+  CallHashes.push_back(std::hash{}(CallHashStr));
+} else if (HashFunction == HashFunction::XXH3) {
+  CallHashes.push_back(llvm::xxh3_64bits(CallHashStr));
+} else {
+  llvm_unreachable("Unhandled HashFunction");

dcci wrote:

maybe you can make this message slightly more verbose/explicative.

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)

[Davide Italiano via llvm-branch-commits]

@@ -414,31 +449,59 @@ createFlowFunction(const 
BinaryFunction::BasicBlockOrderType ) {
 /// 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 ) {
+const yaml::bolt::BinaryFunctionProfile , FlowFunction ,
+HashFunction HashFunction,
+const DenseMap ) {
+
   assert(Func.Blocks.size() == BlockOrder.size() + 2);
 
+  std::vector CallHashes;
   std::vector Blocks;
   std::vector BlendedHashes;
   for (uint64_t I = 0; I < BlockOrder.size(); I++) {
 const BinaryBasicBlock *BB = BlockOrder[I];
 assert(BB->getHash() != 0 && "empty hash of BinaryBasicBlock");
+
+std::string CallHashStr = hashBlockCalls(BC, *BB);
+if (CallHashStr.empty()) {
+  CallHashes.push_back(0);
+} else if (HashFunction == HashFunction::StdHash) {
+  CallHashes.push_back(std::hash{}(CallHashStr));
+} else if (HashFunction == HashFunction::XXH3) {

dcci wrote:

aren't these mutually exclusive? why do you need the `else` ?

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)

[Davide Italiano via llvm-branch-commits]

@@ -414,31 +449,59 @@ createFlowFunction(const 
BinaryFunction::BasicBlockOrderType ) {
 /// 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 ) {
+const yaml::bolt::BinaryFunctionProfile , FlowFunction ,
+HashFunction HashFunction,
+const DenseMap ) {
+
   assert(Func.Blocks.size() == BlockOrder.size() + 2);
 
+  std::vector CallHashes;
   std::vector Blocks;
   std::vector BlendedHashes;
   for (uint64_t I = 0; I < BlockOrder.size(); I++) {
 const BinaryBasicBlock *BB = BlockOrder[I];
 assert(BB->getHash() != 0 && "empty hash of BinaryBasicBlock");
+
+std::string CallHashStr = hashBlockCalls(BC, *BB);
+if (CallHashStr.empty()) {
+  CallHashes.push_back(0);
+} else if (HashFunction == HashFunction::StdHash) {
+  CallHashes.push_back(std::hash{}(CallHashStr));
+} else if (HashFunction == HashFunction::XXH3) {
+  CallHashes.push_back(llvm::xxh3_64bits(CallHashStr));
+} else {
+  llvm_unreachable("Unhandled HashFunction");
+}
+
 Blocks.push_back([I + 1]);
 BlendedBlockHash BlendedHash(BB->getHash());
 BlendedHashes.push_back(BlendedHash);
 LLVM_DEBUG(dbgs() << "BB with index " << I << " has hash = "
   << Twine::utohexstr(BB->getHash()) << "\n");
   }
   StaleMatcher Matcher;
-  Matcher.init(Blocks, BlendedHashes);
+  Matcher.init(Blocks, BlendedHashes, CallHashes);
 
   // Index in yaml profile => corresponding (matched) block
   DenseMap MatchedBlocks;
   // Match blocks from the profile to the blocks in CFG
   for (const yaml::bolt::BinaryBasicBlockProfile  : YamlBF.Blocks) {
 assert(YamlBB.Hash != 0 && "empty hash of BinaryBasicBlockProfile");
 BlendedBlockHash YamlHash(YamlBB.Hash);
-const FlowBlock *MatchedBlock = Matcher.matchBlock(YamlHash);
-// Always match the entry block.
+
+const FlowBlock *MatchedBlock = nullptr;
+std::string CallHashStr = hashBlockCalls(IdToFunctionName, YamlBB);
+uint64_t CallHash = 0;
+if (CallHashStr.empty()) { // Noop

dcci wrote:

```
if (!CallHashStr.empty()) {
  if (...)
}
```

maybe a better way of writing this

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)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci 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)

[Davide Italiano via llvm-branch-commits]

@@ -155,5 +155,51 @@ std::string hashBlockLoose(BinaryContext , const 
BinaryBasicBlock ) {
   return HashString;
 }
 
+/// An even looser hash of a basic block to use with stale profile matching,
+/// composed of the names of a block's called functions in lexicographic order.
+std::string hashBlockCalls(BinaryContext , const BinaryBasicBlock ) {
+  // The hash is computed by creating a string of all lexicographically ordered
+  // called function names.
+  std::multiset FunctionNames;
+  for (const MCInst  : BB) {
+// Skip non-call instructions.
+if (!BC.MIB->isCall(Instr))
+  continue;
+const MCSymbol *CallSymbol = BC.MIB->getTargetSymbol(Instr);
+if (!CallSymbol)

dcci wrote:

is this something that can happen, or should it be an assertion?

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)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci requested changes to this pull request.

First round of 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)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.


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] [BOLT][NFC] Refactor function matching (PR #97502)

[Davide Italiano via llvm-branch-commits]

dcci wrote:

> I have a couple of general comments about this. Can you also please add a 
> description explaining what this patch does?

i.e. why we're refactoring these functions.

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] [BOLT][NFC] Refactor function matching (PR #97502)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci 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] [BOLT][NFC] Refactor function matching (PR #97502)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci commented:

I have a couple of general comments about this.
Can you also please add a description explaining what this patch does?

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] [BOLT][NFC] Refactor function matching (PR #97502)

[Davide Italiano via llvm-branch-commits]

@@ -73,13 +73,26 @@ class YAMLProfileReader : public ProfileReaderBase {
   bool parseFunctionProfile(BinaryFunction ,
 const yaml::bolt::BinaryFunctionProfile );
 
+  /// Returns block cnt equality if IgnoreHash is true, otherwise, hash 
equality
+  bool profileMatches(const yaml::bolt::BinaryFunctionProfile ,

dcci wrote:

I think this comment talks about the implementation more than the definition. 
Can you rephrase it?

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] [BOLT][NFC] Refactor function matching (PR #97502)

[Davide Italiano via llvm-branch-commits]

@@ -73,13 +73,26 @@ class YAMLProfileReader : public ProfileReaderBase {
   bool parseFunctionProfile(BinaryFunction ,
 const yaml::bolt::BinaryFunctionProfile );
 
+  /// Returns block cnt equality if IgnoreHash is true, otherwise, hash 
equality
+  bool profileMatches(const yaml::bolt::BinaryFunctionProfile ,
+  BinaryFunction );
+
   /// Infer function profile from stale data (collected on older binaries).
   bool inferStaleProfile(BinaryFunction ,
  const yaml::bolt::BinaryFunctionProfile );
 
   /// Initialize maps for profile matching.
   void buildNameMaps(BinaryContext );
 
+  /// Matches functions using exact name.
+  size_t matchWithExactName();

dcci wrote:

why you need these 3 different functions?

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] [BOLT] Drop macro-fusion alignment (PR #97358)

[Davide Italiano via llvm-branch-commits]

dcci wrote:

is this the correct destination branch?

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)

[Davide Italiano via llvm-branch-commits]

dcci wrote:

Maybe you can add a comment to the title explaining *why* we're removing it.

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)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.

Thanks for measuring the impact and removing this code, Amir.

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][BAT] Add support for three-way split functions (PR #93760)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/93760
___
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][BAT] Add support for three-way split functions (PR #93760)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci edited https://github.com/llvm/llvm-project/pull/93760
___
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][BAT] Add support for three-way split functions (PR #93760)

[Davide Italiano via llvm-branch-commits]

@@ -129,6 +129,8 @@ void BoltAddressTranslation::write(const BinaryContext , 
raw_ostream ) {
 LLVM_DEBUG(dbgs() << " Cold part\n");
 for (const FunctionFragment  :
  Function.getLayout().getSplitFragments()) {
+  if (FF.empty())

dcci wrote:

can you add a comment explaining why we're skipping empty fragments?

https://github.com/llvm/llvm-project/pull/93760
___
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] Ignore hot markers as function references in updateELFSymbolTable (PR #92713)

[Davide Italiano via llvm-branch-commits]

@@ -4788,13 +4788,22 @@ void RewriteInstance::updateELFSymbolTable(
 if (!IsDynSym && shouldStrip(Symbol))
   continue;
 
+Expected SymbolName = Symbol.getName(StringSection);
+assert(SymbolName && "cannot get symbol name");

dcci wrote:

SG

https://github.com/llvm/llvm-project/pull/92713
___
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] Ignore hot markers as function references in updateELFSymbolTable (PR #92713)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.

LG

https://github.com/llvm/llvm-project/pull/92713
___
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] Ignore hot markers as function references in updateELFSymbolTable (PR #92713)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci edited https://github.com/llvm/llvm-project/pull/92713
___
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] Ignore hot markers as function references in updateELFSymbolTable (PR #92713)

[Davide Italiano via llvm-branch-commits]

@@ -30,12 +31,12 @@
 # CHECK-OUTPUT:   __hot_start
 # CHECK-OUTPUT-NEXT:  main
 # CHECK-OUTPUT-NEXT:  __hot_end
+# CHECK-OUTPUT-NOT:   __hot_start.cold
 
   .text
   .globl  main
   .type main, %function
   .globl  __hot_start
-  .type __hot_start, %object

dcci wrote:

why are you removing this?

https://github.com/llvm/llvm-project/pull/92713
___
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] Ignore hot markers as function references in updateELFSymbolTable (PR #92713)

[Davide Italiano via llvm-branch-commits]

@@ -4788,13 +4788,22 @@ void RewriteInstance::updateELFSymbolTable(
 if (!IsDynSym && shouldStrip(Symbol))
   continue;
 
+Expected SymbolName = Symbol.getName(StringSection);
+assert(SymbolName && "cannot get symbol name");

dcci wrote:

is this assert needed? if Expected returns null, it will blow up, no?

https://github.com/llvm/llvm-project/pull/92713
___
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] Ignore hot markers as function references in updateELFSymbolTable (PR #92713)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci requested changes to this pull request.

Some comments, thanks for working o this.

https://github.com/llvm/llvm-project/pull/92713
___
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] Ignore hot markers as function references in updateELFSymbolTable (PR #92713)

[Davide Italiano via llvm-branch-commits]

@@ -4788,13 +4788,22 @@ void RewriteInstance::updateELFSymbolTable(
 if (!IsDynSym && shouldStrip(Symbol))
   continue;
 
+Expected SymbolName = Symbol.getName(StringSection);
+assert(SymbolName && "cannot get symbol name");
+
 const BinaryFunction *Function =
 BC->getBinaryFunctionAtAddress(Symbol.st_value);
 // Ignore false function references, e.g. when the section address matches
 // the address of the function.
 if (Function && Symbol.getType() == ELF::STT_SECTION)
   Function = nullptr;
 
+// Ignore input hot markers as function aliases – markers are handled
+// separately.
+if (Function &&
+(*SymbolName == "__hot_start" || *SymbolName == "__hot_end"))

dcci wrote:

can you expand this comment -- explaining why we ignore input hot markers?

https://github.com/llvm/llvm-project/pull/92713
___
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] Rename DataAggregator::BranchInfo to TakenInfo (PR #92017)

[Davide Italiano via llvm-branch-commits]

dcci wrote:

I would suggest `BranchTakenInfo` rather than `TakenInfo`  but up to you/@maksfb

https://github.com/llvm/llvm-project/pull/92017
___
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] Rename DataAggregator::BranchInfo to TakenInfo (PR #92017)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.


https://github.com/llvm/llvm-project/pull/92017
___
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] Preserve Offset annotation in fixDoubleJumps (PR #91898)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.

LG

https://github.com/llvm/llvm-project/pull/91898
___
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] Set entry counts in BAT YAML profile (PR #91775)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.


https://github.com/llvm/llvm-project/pull/91775
___
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] Use disambiguated local names in BAT YAML (PR #91773)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.


https://github.com/llvm/llvm-project/pull/91773
___
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] Revise IDE folder structure (PR #89742)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.


https://github.com/llvm/llvm-project/pull/89742
___
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] Emit empty FDE for injected functions (PR #87967)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.


https://github.com/llvm/llvm-project/pull/87967
___
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] Emit empty FDE for injected functions (PR #87967)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci requested changes to this pull request.

Can you add a test? that way we can better understand what this function 
actually does, given it touches a core functionality.

https://github.com/llvm/llvm-project/pull/87967
___
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][BAT] Support multi-way split functions (PR #87123)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.

LG with the question answered/addressed.

https://github.com/llvm/llvm-project/pull/87123
___
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][BAT] Support multi-way split functions (PR #87123)

[Davide Italiano via llvm-branch-commits]

@@ -197,8 +197,10 @@ void BoltAddressTranslation::writeMaps(std::map ,
 ? SecondaryEntryPointsMap[Address].size()
 : 0;
 if (Cold) {
-  size_t HotIndex =
-  std::distance(ColdPartSource.begin(), ColdPartSource.find(Address));
+  // `Maps` is keyed by output addresses.

dcci wrote:

This comment seems self-explanatory. Maybe remove it?

https://github.com/llvm/llvm-project/pull/87123
___
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] Set EntryDiscriminator in YAML profile for indirect calls (PR #82128)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci commented:

> Also address the issue with enumeration of secondary entry points:
make them start with 1 instead of 0 (reserved for primary entry point).

^ do you want to split this in a separate commit?

https://github.com/llvm/llvm-project/pull/82128
___
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] Expose YAMLProfileWriter::convert function (PR #76909)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.


https://github.com/llvm/llvm-project/pull/76909
___
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] Expose YAMLProfileWriter::convert function (PR #76909)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci commented:

No problems with this, but do we really need 8 commits? It clutters history. 
Can you merge in a single one?

https://github.com/llvm/llvm-project/pull/76909
___
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] [flang] [libcxx] [mlir] [libc] [clang-tools-extra] [lldb] [clang] [openmp] [llvm] [BOLT] Deduplicate equal offsets in BAT (PR #76905)

[Davide Italiano via llvm-branch-commits]

https://github.com/dcci approved this pull request.

LG

https://github.com/llvm/llvm-project/pull/76905
___
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] [flang] [openmp] [clang-tools-extra] [lldb] [libc] [libcxx] [mlir] [clang] [llvm] [BOLT] Deduplicate equal offsets in BAT (PR #76905)

[Davide Italiano via llvm-branch-commits]

@@ -110,6 +111,34 @@ void BoltAddressTranslation::write(const BinaryContext 
, raw_ostream ) {
   outs() << "BOLT-INFO: Wrote " << Maps.size() << " BAT maps\n";
 }
 
+APInt BoltAddressTranslation::calculateBranchEntriesBitMask(MapTy ,
+size_t EqualElems) 
{
+  APInt BitMask(alignTo(EqualElems, 8), 0);
+  size_t Index = 0;
+  for (std::pair  : Map) {
+if (Index == EqualElems)
+  break;
+const uint32_t OutputOffset = KeyVal.second;
+if (OutputOffset & BRANCHENTRY)
+  BitMask.setBit(Index);
+++Index;
+  }
+  return BitMask;
+}
+
+size_t BoltAddressTranslation::getNumEqualOffsets(const MapTy ) const {
+  size_t EqualOffsets = 0;
+  for (const std::pair  : Map) {
+const uint32_t OutputOffset = KeyVal.first;
+const uint32_t InputOffset = KeyVal.second >> 1;
+if (OutputOffset == InputOffset)
+  ++EqualOffsets;
+else
+  break;

dcci wrote:

Do you want to stop here at the first offset that's different?

https://github.com/llvm/llvm-project/pull/76905
___
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] [libc] [openmp] [clang-tools-extra] [libcxx] [lldb] [flang] [clang] [mlir] [llvm] [BOLT] Deduplicate equal offsets in BAT (PR #76905)

[Davide Italiano via llvm-branch-commits]

@@ -207,15 +280,23 @@ void 
BoltAddressTranslation::parseMaps(std::vector ,
   const uint64_t OutputAddress = PrevAddress + OutputDelta;
   const uint64_t OutputOffset = OutputAddress - Address;
   PrevAddress = OutputAddress;
-  const int64_t InputDelta = DE.getSLEB128(, );
-  InputOffset += InputDelta;
+  int64_t InputDelta = 0;
+  if (J < EqualElems) {
+InputOffset = (OutputOffset << 1) | (*BEBitMask)[J];
+  } else {
+InputDelta = DE.getSLEB128(, );
+InputOffset += InputDelta;
+  }
   Map.insert(std::pair(OutputOffset, InputOffset));
   LLVM_DEBUG(
   dbgs() << formatv("{0:x} -> {1:x} ({2}/{3}b -> {4}/{5}b), {6:x}\n",
 OutputOffset, InputOffset, OutputDelta,
-encodeULEB128(OutputDelta, nulls()), InputDelta,
-encodeSLEB128(InputDelta, nulls()), 
OutputAddress));
+getULEB128Size(OutputDelta), InputDelta,
+(J < EqualElems) ? 0 : getSLEB128Size(InputDelta),
+OutputAddress));
 }
+if (BEBitMask)
+  delete BEBitMask;

dcci wrote:

Can you make this automatically memory managed?

https://github.com/llvm/llvm-project/pull/76905
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits