[llvm-branch-commits] [llvm] [BOLT] Map branch source address to the containing basic block in BAT YAML (PR #91273)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/91273 >From 2a848b1ac92203f7821d79ffe7cf0d04846542f3 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Mon, 6 May 2024 16:00:01 -0700 Subject: [PATCH] add blarge_new_bat_branchentry.preagg.txt Created using spr 1.3.4 --- bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt diff --git a/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt new file mode 100644 index 0..546da92f94dba --- /dev/null +++ b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt @@ -0,0 +1 @@ +B 80010c 800194 1 0 ___ 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] Map branch source address to the containing basic block in BAT YAML (PR #91273)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/91273 >From 2a848b1ac92203f7821d79ffe7cf0d04846542f3 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Mon, 6 May 2024 16:00:01 -0700 Subject: [PATCH] add blarge_new_bat_branchentry.preagg.txt Created using spr 1.3.4 --- bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt diff --git a/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt new file mode 100644 index 0..546da92f94dba --- /dev/null +++ b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt @@ -0,0 +1 @@ +B 80010c 800194 1 0 ___ 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] Map branch source address to the containing basic block in BAT YAML (PR #91273)
https://github.com/maksfb approved this pull request. Please see the nit for the error message (caps and new line). Otherwise LGTM. https://github.com/llvm/llvm-project/pull/91273 ___ 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] Map branch source address to the containing basic block in BAT YAML (PR #91273)
@@ -2378,21 +2379,27 @@ std::error_code
DataAggregator::writeBATYAML(BinaryContext ,
return CSI;
};
+ // Lookup containing basic block offset and index
+ auto getBlock = [](uint32_t Offset) {
+auto BlockIt = BlockMap.upper_bound(Offset);
+if (LLVM_UNLIKELY(BlockIt == BlockMap.begin())) {
+ errs() << "BOLT-ERROR: Invalid BAT section";
maksfb wrote:
```suggestion
errs() << "BOLT-ERROR: invalid BAT section\n";
```
https://github.com/llvm/llvm-project/pull/91273
___
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] Map branch source address to the containing basic block in BAT YAML (PR #91273)
https://github.com/maksfb edited https://github.com/llvm/llvm-project/pull/91273 ___ 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] Map branch source address to the containing basic block in BAT YAML (PR #91273)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/91273 >From 2a848b1ac92203f7821d79ffe7cf0d04846542f3 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Mon, 6 May 2024 16:00:01 -0700 Subject: [PATCH] add blarge_new_bat_branchentry.preagg.txt Created using spr 1.3.4 --- bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt diff --git a/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt new file mode 100644 index 0..546da92f94dba --- /dev/null +++ b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt @@ -0,0 +1 @@ +B 80010c 800194 1 0 ___ 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] Map branch source address to the containing basic block in BAT YAML (PR #91273)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/91273 >From 2a848b1ac92203f7821d79ffe7cf0d04846542f3 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Mon, 6 May 2024 16:00:01 -0700 Subject: [PATCH] add blarge_new_bat_branchentry.preagg.txt Created using spr 1.3.4 --- bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt diff --git a/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt new file mode 100644 index 0..546da92f94dba --- /dev/null +++ b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt @@ -0,0 +1 @@ +B 80010c 800194 1 0 ___ 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] Map branch source address to the containing basic block in BAT YAML (PR #91273)
@@ -2378,21 +2378,24 @@ std::error_code
DataAggregator::writeBATYAML(BinaryContext ,
return CSI;
};
+ // Lookup containing basic block offset and index
+ auto getBlock = [](uint32_t Offset) {
+auto BlockIt = BlockMap.upper_bound(Offset);
+assert(BlockIt != BlockMap.begin());
+--BlockIt;
+return std::pair(BlockIt->first, BlockIt->second.getBBIndex());
+ };
+
for (const auto &[FromOffset, SuccKV] : Branches.IntraIndex) {
-if (!BlockMap.isInputBlock(FromOffset))
- continue;
-const unsigned Index = BlockMap.getBBIndex(FromOffset);
+const auto &[_, Index] = getBlock(FromOffset);
maksfb wrote:
Even though we generate BAT in BOLT, when we view the invocation of BOLT on a
binary with embedded BAT, such input should be considered an external and
potentially malformed data. In this case, assertions will not provide adequate
enough protection since we can build BOLT without them.
https://github.com/llvm/llvm-project/pull/91273
___
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] Map branch source address to the containing basic block in BAT YAML (PR #91273)
@@ -2378,21 +2378,24 @@ std::error_code
DataAggregator::writeBATYAML(BinaryContext ,
return CSI;
};
+ // Lookup containing basic block offset and index
+ auto getBlock = [](uint32_t Offset) {
+auto BlockIt = BlockMap.upper_bound(Offset);
+assert(BlockIt != BlockMap.begin());
+--BlockIt;
+return std::pair(BlockIt->first, BlockIt->second.getBBIndex());
+ };
+
for (const auto &[FromOffset, SuccKV] : Branches.IntraIndex) {
-if (!BlockMap.isInputBlock(FromOffset))
- continue;
-const unsigned Index = BlockMap.getBBIndex(FromOffset);
+const auto &[_, Index] = getBlock(FromOffset);
aaupov wrote:
> Do we expect getBlock() to always return a good value?
Yes. It performs an `upper_bound` lookup with an unsigned value against a
BlockMap which should contain a zero key (entry basic block). As long as the
function has it, we're good. If not, it's worth looking why do we have such a
function in BAT.
> There's no chance for malformed input to trigger the assertion above?
No, any offset in the profile is >=0.
https://github.com/llvm/llvm-project/pull/91273
___
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] Map branch source address to the containing basic block in BAT YAML (PR #91273)
@@ -2378,21 +2378,24 @@ std::error_code
DataAggregator::writeBATYAML(BinaryContext ,
return CSI;
};
+ // Lookup containing basic block offset and index
+ auto getBlock = [](uint32_t Offset) {
+auto BlockIt = BlockMap.upper_bound(Offset);
+assert(BlockIt != BlockMap.begin());
+--BlockIt;
+return std::pair(BlockIt->first, BlockIt->second.getBBIndex());
+ };
+
for (const auto &[FromOffset, SuccKV] : Branches.IntraIndex) {
-if (!BlockMap.isInputBlock(FromOffset))
- continue;
-const unsigned Index = BlockMap.getBBIndex(FromOffset);
+const auto &[_, Index] = getBlock(FromOffset);
maksfb wrote:
Do we expect `getBlock()` to always return a good value? There's no chance for
malformed input to trigger the assertion above?
https://github.com/llvm/llvm-project/pull/91273
___
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] Map branch source address to the containing basic block in BAT YAML (PR #91273)
https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/91273 >From 2a848b1ac92203f7821d79ffe7cf0d04846542f3 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Mon, 6 May 2024 16:00:01 -0700 Subject: [PATCH] add blarge_new_bat_branchentry.preagg.txt Created using spr 1.3.4 --- bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt diff --git a/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt new file mode 100644 index 00..546da92f94dbae --- /dev/null +++ b/bolt/test/X86/Inputs/blarge_new_bat_branchentry.preagg.txt @@ -0,0 +1 @@ +B 80010c 800194 1 0 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
