[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)

2024-01-25 Thread Mircea Trofin via cfe-commits


@@ -858,62 +899,64 @@ struct BBAddrMap {
 bool hasIndirectBranch() const { return MD.HasIndirectBranch; }
   };
 
-  BBAddrMap(uint64_t Addr, std::vector BBEntries)
-  : Addr(Addr), BBEntries(std::move(BBEntries)) {}
+  // Struct representing the BBAddrMap information for a contiguous range of
+  // basic blocks (a function or a basic block section).
+  struct BBRangeEntry {
+uint64_t BaseAddress;   // Base address of the range.

mtrofin wrote:

(paranoia) could you initialize BaseAddress at decl? easier to maintain and 
avoid uninitialized error issues (sure, they can also be compiler-detected, but 
3 extra characters don't hurt either)

same for the fields above

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)

2024-01-11 Thread James Henderson via cfe-commits

https://github.com/jh7370 commented:

Sorry, I'm a bit snowed under with reviews. I'm happy to defer to others on 
this patch.

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)

2024-01-07 Thread Micah Weston via cfe-commits


@@ -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] [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)

2024-01-07 Thread Micah Weston via cfe-commits


@@ -719,60 +730,83 @@ decodeBBAddrMapImpl(const ELFFile ,
   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