[llvm-branch-commits] [BOLT] Use aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Amir Ayupov via llvm-branch-commits 

https://github.com/aaupov updated 
https://github.com/llvm/llvm-project/pull/91289


___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Amir Ayupov via llvm-branch-commits 

https://github.com/aaupov updated 
https://github.com/llvm/llvm-project/pull/91289


___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Alexander Yermolovich via llvm-branch-commits 

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


https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Amir Ayupov via llvm-branch-commits 

https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Amir Ayupov via llvm-branch-commits 


@@ -2386,25 +2362,26 @@ std::error_code 
DataAggregator::writeBATYAML(BinaryContext ,
 return std::pair(BlockIt->first, BlockIt->second.getBBIndex());
   };
 
-  for (const auto &[FromOffset, SuccKV] : Branches.IntraIndex) {
-const auto &[_, Index] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = YamlBF.Blocks[Index];
-for (const auto &[SuccOffset, SuccDataIdx] : SuccKV)
-  if (BlockMap.isInputBlock(SuccOffset))
-YamlBB.Successors.emplace_back(
-getSuccessorInfo(SuccOffset, SuccDataIdx));
-  }
-  for (const auto &[FromOffset, CallTo] : Branches.InterIndex) {
-const auto &[BlockOffset, BlockIndex] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = 
YamlBF.Blocks[BlockIndex];
-const uint32_t Offset = FromOffset - BlockOffset;
-for (const auto &[CallToLoc, CallToIdx] : CallTo)
-  YamlBB.CallSites.emplace_back(
-  getCallSiteInfo(CallToLoc, CallToIdx, Offset));
-llvm::sort(YamlBB.CallSites, [](yaml::bolt::CallSiteInfo ,
-yaml::bolt::CallSiteInfo ) {
-  return A.Offset < B.Offset;
-});
+  for (const llvm::bolt::BranchInfo  : Branches.Data) {
+using namespace yaml::bolt;
+const auto &[BlockOffset, BlockIndex] = getBlock(BI.From.Offset);
+BinaryBasicBlockProfile  = YamlBF.Blocks[BlockIndex];
+if (BI.To.IsSymbol && BI.To.Name == BI.From.Name && BI.To.Offset != 0) 
{
+  // Internal branch
+  const unsigned SuccIndex = getBlock(BI.To.Offset).second;
+  auto  = YamlBB.Successors.emplace_back(SuccessorInfo{SuccIndex});
+  SI.Count = BI.Branches;

aaupov wrote:

For example, an explicit cast in the constructor due to type impedance: 
https://github.com/llvm/llvm-project/blob/31a203fa8af47a8b2e8e357857b114cf90638b2e/bolt/lib/Profile/DataAggregator.cpp#L1296-L1298

https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Amir Ayupov via llvm-branch-commits 


@@ -2386,25 +2362,26 @@ std::error_code 
DataAggregator::writeBATYAML(BinaryContext ,
 return std::pair(BlockIt->first, BlockIt->second.getBBIndex());
   };
 
-  for (const auto &[FromOffset, SuccKV] : Branches.IntraIndex) {
-const auto &[_, Index] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = YamlBF.Blocks[Index];
-for (const auto &[SuccOffset, SuccDataIdx] : SuccKV)
-  if (BlockMap.isInputBlock(SuccOffset))
-YamlBB.Successors.emplace_back(
-getSuccessorInfo(SuccOffset, SuccDataIdx));
-  }
-  for (const auto &[FromOffset, CallTo] : Branches.InterIndex) {
-const auto &[BlockOffset, BlockIndex] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = 
YamlBF.Blocks[BlockIndex];
-const uint32_t Offset = FromOffset - BlockOffset;
-for (const auto &[CallToLoc, CallToIdx] : CallTo)
-  YamlBB.CallSites.emplace_back(
-  getCallSiteInfo(CallToLoc, CallToIdx, Offset));
-llvm::sort(YamlBB.CallSites, [](yaml::bolt::CallSiteInfo ,
-yaml::bolt::CallSiteInfo ) {
-  return A.Offset < B.Offset;
-});
+  for (const llvm::bolt::BranchInfo  : Branches.Data) {
+using namespace yaml::bolt;
+const auto &[BlockOffset, BlockIndex] = getBlock(BI.From.Offset);
+BinaryBasicBlockProfile  = YamlBF.Blocks[BlockIndex];
+if (BI.To.IsSymbol && BI.To.Name == BI.From.Name && BI.To.Offset != 0) 
{
+  // Internal branch
+  const unsigned SuccIndex = getBlock(BI.To.Offset).second;
+  auto  = YamlBB.Successors.emplace_back(SuccessorInfo{SuccIndex});
+  SI.Count = BI.Branches;

aaupov wrote:

Type impedance means that if Count and Mispreds are passed to the constructor 
there needs to be an explicit cast (as the constructor doesn't accept uints). 
Whereas if the values are assigned to, there's an implicit cast.

https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Amir Ayupov via llvm-branch-commits 


@@ -2386,25 +2362,26 @@ std::error_code 
DataAggregator::writeBATYAML(BinaryContext ,
 return std::pair(BlockIt->first, BlockIt->second.getBBIndex());
   };
 
-  for (const auto &[FromOffset, SuccKV] : Branches.IntraIndex) {
-const auto &[_, Index] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = YamlBF.Blocks[Index];
-for (const auto &[SuccOffset, SuccDataIdx] : SuccKV)
-  if (BlockMap.isInputBlock(SuccOffset))
-YamlBB.Successors.emplace_back(
-getSuccessorInfo(SuccOffset, SuccDataIdx));
-  }
-  for (const auto &[FromOffset, CallTo] : Branches.InterIndex) {
-const auto &[BlockOffset, BlockIndex] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = 
YamlBF.Blocks[BlockIndex];
-const uint32_t Offset = FromOffset - BlockOffset;
-for (const auto &[CallToLoc, CallToIdx] : CallTo)
-  YamlBB.CallSites.emplace_back(
-  getCallSiteInfo(CallToLoc, CallToIdx, Offset));
-llvm::sort(YamlBB.CallSites, [](yaml::bolt::CallSiteInfo ,
-yaml::bolt::CallSiteInfo ) {
-  return A.Offset < B.Offset;
-});
+  for (const llvm::bolt::BranchInfo  : Branches.Data) {

aaupov wrote:

Dropped in follow-up diff https://github.com/llvm/llvm-project/pull/92017.

https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Amir Ayupov via llvm-branch-commits 

https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Amir Ayupov via llvm-branch-commits 


@@ -2386,25 +2362,26 @@ std::error_code 
DataAggregator::writeBATYAML(BinaryContext ,
 return std::pair(BlockIt->first, BlockIt->second.getBBIndex());
   };
 
-  for (const auto &[FromOffset, SuccKV] : Branches.IntraIndex) {
-const auto &[_, Index] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = YamlBF.Blocks[Index];
-for (const auto &[SuccOffset, SuccDataIdx] : SuccKV)
-  if (BlockMap.isInputBlock(SuccOffset))
-YamlBB.Successors.emplace_back(
-getSuccessorInfo(SuccOffset, SuccDataIdx));
-  }
-  for (const auto &[FromOffset, CallTo] : Branches.InterIndex) {
-const auto &[BlockOffset, BlockIndex] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = 
YamlBF.Blocks[BlockIndex];
-const uint32_t Offset = FromOffset - BlockOffset;
-for (const auto &[CallToLoc, CallToIdx] : CallTo)
-  YamlBB.CallSites.emplace_back(
-  getCallSiteInfo(CallToLoc, CallToIdx, Offset));
-llvm::sort(YamlBB.CallSites, [](yaml::bolt::CallSiteInfo ,
-yaml::bolt::CallSiteInfo ) {
-  return A.Offset < B.Offset;
-});
+  for (const llvm::bolt::BranchInfo  : Branches.Data) {

aaupov wrote:

Unfortunately we need a FQ name:
`llvm::bolt::BranchInfo`:
https://github.com/llvm/llvm-project/blob/5944579ab20cfcb6d1a9d1a2fe3d4b478ea24c64/bolt/include/bolt/Profile/DataReader.h#L78
```
`llvm::bolt::DataAggregator::BranchInfo`:
https://github.com/llvm/llvm-project/blob/5944579ab20cfcb6d1a9d1a2fe3d4b478ea24c64/bolt/include/bolt/Profile/DataAggregator.h#L125

I thought about renaming the latter to `TakenInfo` to match its counterpart 
`FTInfo`: 
https://github.com/llvm/llvm-project/blob/5944579ab20cfcb6d1a9d1a2fe3d4b478ea24c64/bolt/include/bolt/Profile/DataAggregator.h#L120
WDYT?

https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Maksim Panchenko via llvm-branch-commits 


@@ -2386,25 +2362,26 @@ std::error_code 
DataAggregator::writeBATYAML(BinaryContext ,
 return std::pair(BlockIt->first, BlockIt->second.getBBIndex());
   };
 
-  for (const auto &[FromOffset, SuccKV] : Branches.IntraIndex) {
-const auto &[_, Index] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = YamlBF.Blocks[Index];
-for (const auto &[SuccOffset, SuccDataIdx] : SuccKV)
-  if (BlockMap.isInputBlock(SuccOffset))
-YamlBB.Successors.emplace_back(
-getSuccessorInfo(SuccOffset, SuccDataIdx));
-  }
-  for (const auto &[FromOffset, CallTo] : Branches.InterIndex) {
-const auto &[BlockOffset, BlockIndex] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = 
YamlBF.Blocks[BlockIndex];
-const uint32_t Offset = FromOffset - BlockOffset;
-for (const auto &[CallToLoc, CallToIdx] : CallTo)
-  YamlBB.CallSites.emplace_back(
-  getCallSiteInfo(CallToLoc, CallToIdx, Offset));
-llvm::sort(YamlBB.CallSites, [](yaml::bolt::CallSiteInfo ,
-yaml::bolt::CallSiteInfo ) {
-  return A.Offset < B.Offset;
-});
+  for (const llvm::bolt::BranchInfo  : Branches.Data) {

maksfb wrote:

nit: no need for `llvm::bolt::`.

https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Maksim Panchenko via llvm-branch-commits 

https://github.com/maksfb edited https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Maksim Panchenko via llvm-branch-commits 

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

Looks good on my end.

https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Amir Ayupov via llvm-branch-commits 


@@ -2386,25 +2362,26 @@ std::error_code 
DataAggregator::writeBATYAML(BinaryContext ,
 return std::pair(BlockIt->first, BlockIt->second.getBBIndex());
   };
 
-  for (const auto &[FromOffset, SuccKV] : Branches.IntraIndex) {
-const auto &[_, Index] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = YamlBF.Blocks[Index];
-for (const auto &[SuccOffset, SuccDataIdx] : SuccKV)
-  if (BlockMap.isInputBlock(SuccOffset))
-YamlBB.Successors.emplace_back(
-getSuccessorInfo(SuccOffset, SuccDataIdx));
-  }
-  for (const auto &[FromOffset, CallTo] : Branches.InterIndex) {
-const auto &[BlockOffset, BlockIndex] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = 
YamlBF.Blocks[BlockIndex];
-const uint32_t Offset = FromOffset - BlockOffset;
-for (const auto &[CallToLoc, CallToIdx] : CallTo)
-  YamlBB.CallSites.emplace_back(
-  getCallSiteInfo(CallToLoc, CallToIdx, Offset));
-llvm::sort(YamlBB.CallSites, [](yaml::bolt::CallSiteInfo ,
-yaml::bolt::CallSiteInfo ) {
-  return A.Offset < B.Offset;
-});
+  for (const llvm::bolt::BranchInfo  : Branches.Data) {
+using namespace yaml::bolt;
+const auto &[BlockOffset, BlockIndex] = getBlock(BI.From.Offset);
+BinaryBasicBlockProfile  = YamlBF.Blocks[BlockIndex];
+if (BI.To.IsSymbol && BI.To.Name == BI.From.Name && BI.To.Offset != 0) 
{
+  // Internal branch
+  const unsigned SuccIndex = getBlock(BI.To.Offset).second;
+  auto  = YamlBB.Successors.emplace_back(SuccessorInfo{SuccIndex});
+  SI.Count = BI.Branches;

aaupov wrote:

It can, but I decided to keep it this way for two reasons:
1) type impedance (int vs uint) between BI and SI requiring a cast
2) making it explicit where Branches and Mispreds are assigned (vs implicitly 
in constructor parameter order).

https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-13 Thread Alexander Yermolovich via llvm-branch-commits 


@@ -2386,25 +2362,26 @@ std::error_code 
DataAggregator::writeBATYAML(BinaryContext ,
 return std::pair(BlockIt->first, BlockIt->second.getBBIndex());
   };
 
-  for (const auto &[FromOffset, SuccKV] : Branches.IntraIndex) {
-const auto &[_, Index] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = YamlBF.Blocks[Index];
-for (const auto &[SuccOffset, SuccDataIdx] : SuccKV)
-  if (BlockMap.isInputBlock(SuccOffset))
-YamlBB.Successors.emplace_back(
-getSuccessorInfo(SuccOffset, SuccDataIdx));
-  }
-  for (const auto &[FromOffset, CallTo] : Branches.InterIndex) {
-const auto &[BlockOffset, BlockIndex] = getBlock(FromOffset);
-yaml::bolt::BinaryBasicBlockProfile  = 
YamlBF.Blocks[BlockIndex];
-const uint32_t Offset = FromOffset - BlockOffset;
-for (const auto &[CallToLoc, CallToIdx] : CallTo)
-  YamlBB.CallSites.emplace_back(
-  getCallSiteInfo(CallToLoc, CallToIdx, Offset));
-llvm::sort(YamlBB.CallSites, [](yaml::bolt::CallSiteInfo ,
-yaml::bolt::CallSiteInfo ) {
-  return A.Offset < B.Offset;
-});
+  for (const llvm::bolt::BranchInfo  : Branches.Data) {
+using namespace yaml::bolt;
+const auto &[BlockOffset, BlockIndex] = getBlock(BI.From.Offset);
+BinaryBasicBlockProfile  = YamlBF.Blocks[BlockIndex];
+if (BI.To.IsSymbol && BI.To.Name == BI.From.Name && BI.To.Offset != 0) 
{
+  // Internal branch
+  const unsigned SuccIndex = getBlock(BI.To.Offset).second;
+  auto  = YamlBB.Successors.emplace_back(SuccessorInfo{SuccIndex});
+  SI.Count = BI.Branches;

ayermolo wrote:

Why can't this be part of constructor?

https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-10 Thread Amir Ayupov via llvm-branch-commits 

https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-10 Thread Amir Ayupov via llvm-branch-commits 

https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/91289
___
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 aggregated FuncBranchData in writeBATYAML (PR #91289)

2024-05-10 Thread Amir Ayupov via llvm-branch-commits 

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