[llvm] [clang] [lld] [SHT_LLVM_BB_ADDR_MAP] Allow basic-block-sections and labels be used together by decoupling the handling of the two features. (PR #74128)
@@ -719,60 +730,83 @@ decodeBBAddrMapImpl(const ELFFile &EF,
Feature = Data.getU8(Cur); // Feature byte
if (!Cur)
break;
- auto FeatEnableOrErr = PGOAnalysisMap::Features::decode(Feature);
+ auto FeatEnableOrErr = BBAddrMap::Features::decode(Feature);
if (!FeatEnableOrErr)
return FeatEnableOrErr.takeError();
- FeatEnable =
- FeatEnableOrErr ? *FeatEnableOrErr : PGOAnalysisMap::Features{};
+ FeatEnable = FeatEnableOrErr ? *FeatEnableOrErr : BBAddrMap::Features{};
if (Feature != 0 && Version < 2 && Cur)
return createError(
"version should be >= 2 for SHT_LLVM_BB_ADDR_MAP when "
"PGO features are enabled: version = " +
Twine(static_cast(Version)) +
" feature = " + Twine(static_cast(Feature)));
}
-uint64_t SectionOffset = Cur.tell();
-auto Address =
-static_cast::uintX_t>(Data.getAddress(Cur));
-if (!Cur)
- return Cur.takeError();
-if (IsRelocatable) {
- assert(Address == 0);
- auto FOTIterator = FunctionOffsetTranslations.find(SectionOffset);
- if (FOTIterator == FunctionOffsetTranslations.end()) {
-return createError("failed to get relocation data for offset: " +
- Twine::utohexstr(SectionOffset) + " in section " +
- describe(EF, Sec));
- }
- Address = FOTIterator->second;
-}
-uint32_t NumBlocks = readULEB128As(Data, Cur, ULEBSizeErr);
-
+uint32_t NumBlocksInBBRange = 0;
+uint32_t NumBBRanges = 1;
+typename ELFFile::uintX_t RangeBaseAddress = 0;
std::vector BBEntries;
-uint32_t PrevBBEndOffset = 0;
-for (uint32_t BlockIndex = 0;
- !MetadataDecodeErr && !ULEBSizeErr && Cur && (BlockIndex < NumBlocks);
- ++BlockIndex) {
- uint32_t ID = Version >= 2
-? readULEB128As(Data, Cur, ULEBSizeErr)
-: BlockIndex;
- uint32_t Offset = readULEB128As(Data, Cur, ULEBSizeErr);
- uint32_t Size = readULEB128As(Data, Cur, ULEBSizeErr);
- uint32_t MD = readULEB128As(Data, Cur, ULEBSizeErr);
- if (Version >= 1) {
-// Offset is calculated relative to the end of the previous BB.
-Offset += PrevBBEndOffset;
-PrevBBEndOffset = Offset + Size;
+if (FeatEnable.MultiBBRange) {
+ NumBBRanges = readULEB128As(Data, Cur, ULEBSizeErr);
+} else {
+ uint64_t RelocationOffsetInSection = Cur.tell();
+ RangeBaseAddress =
+ static_cast::uintX_t>(Data.getAddress(Cur));
+ if (!Cur)
+return Cur.takeError();
+ if (IsRelocatable) {
+Expected AddressOrErr =
+GetAddressForRelocation(RelocationOffsetInSection);
+if (!AddressOrErr)
+ return AddressOrErr.takeError();
+RangeBaseAddress = *AddressOrErr;
}
- Expected MetadataOrErr =
- BBAddrMap::BBEntry::Metadata::decode(MD);
- if (!MetadataOrErr) {
-MetadataDecodeErr = MetadataOrErr.takeError();
-break;
+ NumBlocksInBBRange = readULEB128As(Data, Cur, ULEBSizeErr);
+}
+std::vector BBRangeEntries;
+uint32_t TotalNumBlocks = 0;
+for (uint32_t BBRangeIndex = 0; BBRangeIndex < NumBBRanges;
+ ++BBRangeIndex) {
+ uint32_t PrevBBEndOffset = 0;
+ if (FeatEnable.MultiBBRange) {
+uint64_t RelocationOffsetInSection = Cur.tell();
+RangeBaseAddress =
+static_cast::uintX_t>(Data.getAddress(Cur));
+if (IsRelocatable) {
+ assert(RangeBaseAddress == 0);
+ Expected AddressOrErr =
+ GetAddressForRelocation(RelocationOffsetInSection);
+ if (!AddressOrErr)
+return AddressOrErr.takeError();
+ RangeBaseAddress = *AddressOrErr;
+}
+NumBlocksInBBRange = readULEB128As(Data, Cur, ULEBSizeErr);
red1bluelost wrote:
This seems nearly the same as at line 751. The differences seem to be this has
an assert on the RangeBaseAddress and 751 short circuits on Cur. If it is
possible, could it be moved into a lambda so that this and 751 can just call
the lambda?
https://github.com/llvm/llvm-project/pull/74128
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lld] [clang] [llvm] [SHT_LLVM_BB_ADDR_MAP] Allow basic-block-sections and labels be used together by decoupling the handling of the two features. (PR #74128)
https://github.com/red1bluelost edited https://github.com/llvm/llvm-project/pull/74128 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lld] [clang] [llvm] [SHT_LLVM_BB_ADDR_MAP] Allow basic-block-sections and labels be used together by decoupling the handling of the two features. (PR #74128)
https://github.com/red1bluelost commented: Overall looks good to me so far, just two minor things and the merge conflict. https://github.com/llvm/llvm-project/pull/74128 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [lld] [SHT_LLVM_BB_ADDR_MAP] Allow basic-block-sections and labels be used together by decoupling the handling of the two features. (PR #74128)
@@ -0,0 +1,89 @@
+; COM: Emitting basic-block-address-map when machine function splitting is
enabled.
+; RUN: llc < %s -mtriple=x86_64 -function-sections -split-machine-functions
-basic-block-address-map | FileCheck %s --check-prefixes=CHECK,BASIC
+
+; COM: Emitting basic-block-address-map with PGO analysis with machine
function splitting enabled.
+; RUN: llc < %s -mtriple=x86_64 -function-sections -split-machine-functions
-basic-block-address-map -pgo-analysis-map=func-entry-count,bb-freq,br-prob |
FileCheck %s --check-prefixes=CHECK,PGO
+
+define void @foo(i1 zeroext %0) nounwind !prof !14 {
+ br i1 %0, label %2, label %4, !prof !15
+
+2:; preds = %1
+ %3 = call i32 @bar()
+ br label %6
+
+4:; preds = %1
+ %5 = call i32 @baz()
+ br label %6
+
+6:; preds = %4, %2
+ %7 = tail call i32 @qux()
+ ret void
+}
+
+declare i32 @bar()
+declare i32 @baz()
+declare i32 @qux()
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"ProfileSummary", !1}
+!1 = !{!2, !3, !4, !5, !6, !7, !8, !9}
+!2 = !{!"ProfileFormat", !"InstrProf"}
+!3 = !{!"TotalCount", i64 1}
+!4 = !{!"MaxCount", i64 10}
+!5 = !{!"MaxInternalCount", i64 1}
+!6 = !{!"MaxFunctionCount", i64 1000}
+!7 = !{!"NumCounts", i64 3}
+!8 = !{!"NumFunctions", i64 5}
+!9 = !{!"DetailedSummary", !10}
+!10 = !{!11, !12, !13}
+!11 = !{i32 1, i64 100, i32 1}
+!12 = !{i32 00, i64 100, i32 1}
+!13 = !{i32 99, i64 1, i32 2}
+!14 = !{!"function_entry_count", i64 7000}
+!15 = !{!"branch_weights", i32 7000, i32 0}
+
+; CHECK: .section .text.hot.foo,"ax",@progbits
+; CHECK-LABEL: foo:
+; CHECK-LABEL: .Lfunc_begin0:
+; CHECK-LABEL: .LBB_END0_0:
+; CHECK-LABEL: .LBB0_1:
+; CHECK-LABEL: .LBB_END0_1:
+; CHECK: .section .text.split.foo,"ax",@progbits
+; CHECK-LABEL: foo.cold:
+; CHECK-LABEL: .LBB_END0_2:
+; CHECK-LABEL: .Lfunc_end0:
+
+; CHECK:.section
.llvm_bb_addr_map,"o",@llvm_bb_addr_map,.text.hot.foo
+; CHECK-NEXT: .byte 2 # version
+; BASIC-NEXT: .byte 8 # feature
+; PGO-NEXT: .byte 15 # feature
+; CHECK-NEXT: .byte 2 # number of basic block ranges
+; CHECK-NEXT: .quad .Lfunc_begin0 # base address
+; CHECK-NEXT: .byte 2 # number of basic blocks
+; CHECK-NEXT: .byte 0 # BB id
+; CHECK-NEXT: .uleb128 .Lfunc_begin0-.Lfunc_begin0
+; CHECK-NEXT: .uleb128 .LBB_END0_0-.Lfunc_begin0
+; CHECK-NEXT: .byte 8
+; CHECK-NEXT: .byte 1 # BB id
+; CHECK-NEXT: .uleb128 .LBB0_1-.LBB_END0_0
+; CHECK-NEXT: .uleb128 .LBB_END0_1-.LBB0_1
+; CHECK-NEXT: .byte 3
+; CHECK-NEXT: .quad foo.cold# base address
+; CHECK-NEXT: .byte 1 # number of basic blocks
+; CHECK-NEXT: .byte 2 # BB id
+; CHECK-NEXT: .uleb128 foo.cold-foo.cold
+; CHECK-NEXT: .uleb128 .LBB_END0_2-foo.cold
+; CHECK-NEXT: .byte 3
+
+;; PGO Analysis Map
red1bluelost wrote:
Thanks for including a test with PGO Analysis Map :)
https://github.com/llvm/llvm-project/pull/74128
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [lld] [clang] [SHT_LLVM_BB_ADDR_MAP] Allow basic-block-sections and labels be used together by decoupling the handling of the two features. (PR #74128)
@@ -719,60 +730,83 @@ decodeBBAddrMapImpl(const ELFFile &EF,
Feature = Data.getU8(Cur); // Feature byte
if (!Cur)
break;
- auto FeatEnableOrErr = PGOAnalysisMap::Features::decode(Feature);
+ auto FeatEnableOrErr = BBAddrMap::Features::decode(Feature);
if (!FeatEnableOrErr)
return FeatEnableOrErr.takeError();
- FeatEnable =
- FeatEnableOrErr ? *FeatEnableOrErr : PGOAnalysisMap::Features{};
+ FeatEnable = FeatEnableOrErr ? *FeatEnableOrErr : BBAddrMap::Features{};
red1bluelost wrote:
(Nit) I think this can just always assume true given that we return on error in
the line before. (woops, I think I wrote this wrong)
```suggestion
FeatEnable = *FeatEnableOrErr;
```
https://github.com/llvm/llvm-project/pull/74128
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [lld] [clang] [SHT_LLVM_BB_ADDR_MAP] Allow basic-block-sections and labels be used together by decoupling the handling of the two features. (PR #74128)
@@ -7574,30 +7573,38 @@ template void
LLVMELFDumper::printBBAddrMaps() {
continue;
}
for (const BBAddrMap &AM : *BBAddrMapOrErr) {
- DictScope D(W, "Function");
- W.printHex("At", AM.Addr);
+ DictScope FD(W, "Function");
+ if (AM.BBRanges.empty())
red1bluelost wrote:
Are there scenarios where BBRanges would be completely empty? If so, what are
those scenarios?
https://github.com/llvm/llvm-project/pull/74128
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
