Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SHT_LLVM_BB_ADDR_MAP] Allow basic-block-sections and labels be used together by decoupling the handling of the two features. #74128

Merged
merged 4 commits into from
Feb 2, 2024

Conversation

rlavaee
Copy link
Contributor

@rlavaee rlavaee commented Dec 1, 2023

Today -split-machine-functions and -fbasic-block-sections={all,list} cannot be combined with -basic-block-sections=labels (the labels option will be ignored).
The inconsistency comes from the way basic block address map -- the underlying mechanism for basic block labels -- encodes basic block addresses (https://lists.llvm.org/pipermail/llvm-dev/2020-July/143512.html). Specifically, basic block offsets are computed relative to the function begin symbol. This relies on functions being contiguous which is not the case for MFS and basic block section binaries. This means Propeller cannot use binary profiles collected from these binaries, which limits the applicability of Propeller for iterative optimization.

To make the SHT_LLVM_BB_ADDR_MAP feature work with basic block section binaries, we propose modifying the encoding of this section as follows.

First let us review the current encoding which emits the address of each function and its number of basic blocks, followed by basic block entries for each basic block.

Address of the function Function Address
Number of basic blocks in this function NumBlocks
BB entry 1
BB entry 2
...
BB entry #NumBlocks

To make this work for basic block sections, we treat each basic block section similar to a function, except that basic block sections of the same function must be encapsulated in the same structure so we can map all of them to their single function.

We modify the encoding to first emit the number of basic block sections (BB ranges) in the function. Then we emit the address map of each basic block section section as before: the base address of the section, its number of blocks, and BB entries for its basic block. The first section in the BB address map is always the function entry section.

Number of sections for this function NumBBRanges
Section 1 begin address BaseAddress[1]
Number of basic blocks in section 1 NumBlocks[1]
BB entries for Section 1
..................
Section #NumBBRanges begin address BaseAddress[NumBBRanges]
Number of basic blocks in section #NumBBRanges NumBlocks[NumBBRanges]
BB entries for Section #NumBBRanges

The encoding of basic block entries remains as before with the minor change that each basic block offset is now computed relative to the begin symbol of its containing BB section.

This patch adds a new boolean codegen option -basic-block-address-map. Correspondingly, the front-end flag -fbasic-block-address-map and LLD flag --lto-basic-block-address-map are introduced.
Analogously, we add a new TargetOption field BBAddrMap. This means BB address maps are either generated for all functions in the compiling unit, or for none (depending on TargetOptions::BBAddrMap).

This patch keeps the functionality of the old -fbasic-block-sections=labels option but does not remove it. A subsequent patch will remove the obsolete option.

We refactor the BasicBlockSections pass by separating the BB address map and BB sections handing to their own functions (named handleBBAddrMap and handleBBSections). handleBBSections renumbers basic blocks and places them in their assigned sections. handleBBAddrMap is invoked after handleBBSections (if requested) and only renumbers the blocks.

  • New tests added:
    • Two tests basic-block-address-map-with-basic-block-sections.ll and basic-block-address-map-with-mfs.ll to exercise the combination of -basic-block-address-map with -basic-block-sections=list and '-split-machine-functions`.
    • A driver sanity test for the -fbasic-block-address-map option (basic-block-address-map.c).
    • An LLD test for testing the --lto-basic-block-address-map option. This reuses the LLVM IR from lld/test/ELF/lto/basic-block-sections.ll.
  • Renamed and modified the two existing codegen tests for basic block address map (basic-block-sections-labels-functions-sections.ll and basic-block-sections-labels.ll)
  • Removed SHT_LLVM_BB_ADDR_MAP_V0 tests. Full deprecation of SHT_LLVM_BB_ADDR_MAP_V0 and SHT_LLVM_BB_ADDR_MAP version less than 2 will happen in a separate PR in a few months.

Copy link

github-actions bot commented Dec 1, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@rlavaee rlavaee force-pushed the bb-addr-map branch 2 times, most recently from 2f36518 to 0beecc8 Compare January 4, 2024 20:48
@rlavaee rlavaee changed the title [SHT_LLVM_BB_ADDR_MAP] Allow basic-block-sections and labels be used … [SHT_LLVM_BB_ADDR_MAP] Allow basic-block-sections and labels be used together by decoupling the handling of the two features. Jan 4, 2024
@rlavaee rlavaee marked this pull request as ready for review January 4, 2024 22:55
@llvmbot llvmbot added clang Clang issues not falling into any other category lld backend:X86 clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen mc Machine (object) code lld:ELF objectyaml llvm:binary-utilities labels Jan 4, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 4, 2024

@llvm/pr-subscribers-objectyaml
@llvm/pr-subscribers-mc
@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-lld-elf
@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-clang

Author: Rahman Lavaee (rlavaee)

Changes

Today -split-machine-functions and -fbasic-block-sections={all,list} cannot be combined with -basic-block-sections=labels (the labels option will be ignored).
The inconsistency comes from the way basic block address map -- the underlying mechanism for basic block labels -- encodes basic block addresses (https://lists.llvm.org/pipermail/llvm-dev/2020-July/143512.html). Specifically, basic block offsets are computed relative to the function begin symbol. This relies on functions being contiguous which is not the case for MFS and basic block section binaries. This means Propeller cannot use binary profiles collected from these binaries, which limits the applicability of Propeller for iterative optimization.

To make the SHT_LLVM_BB_ADDR_MAP feature work with basic block section binaries, we propose modifying the encoding of this section as follows.

First let us review the current encoding which emits the address of each function and its number of basic blocks, followed by basic block entries for each basic block.

Address of the function Function Address
Number of basic blocks in this function NumBlocks
BB entry 1
BB entry 2
...
BB entry #NumBlocks

To make this work for basic block sections, we treat each basic block section similar to a function, except that basic block sections of the same function must be encapsulated in the same structure so we can map all of them to their single function.

We modify the encoding to first emit the number of basic block sections (BB ranges) in the function. Then we emit the address map of each basic block section section as before: the base address of the section, its number of blocks, and BB entries for its basic block. The first section in the BB address map is always the function entry section.

Number of sections for this function NumBBRanges
Section 1 begin address BaseAddress[1]
Number of basic blocks in section 1 NumBlocks[1]
BB entries for Section 1
..................
Section #NumBBRanges begin address BaseAddress[NumBBRanges]
Number of basic blocks in section #NumBBRanges NumBlocks[NumBBRanges]
BB entries for Section #NumBBRanges

The encoding of basic block entries remains as before with the minor change that each basic block offset is now computed relative to the begin symbol of its containing BB section.

This patch adds a new boolean codegen option -basic-block-address-map. Correspondingly, the front-end flag -fbasic-block-address-map and LLD flag --lto-basic-block-address-map are introduced.
Analogously, we add a new TargetOption field BBAddrMap. This means BB address maps are either generated for all functions in the compiling unit, or for none (depending on TargetOptions::BBAddrMap).

This patch keeps the functionality of the old -fbasic-block-sections=labels option but does not remove it. A subsequent patch will remove the obsolete option.

We refactor the BasicBlockSections pass by separating the BB address map and BB sections handing to their own functions (named handleBBAddrMap and handleBBSections). handleBBSections renumbers basic blocks and places them in their assigned sections. handleBBAddrMap is invoked after handleBBSections (if requested) and only renumbers the blocks.

  • New tests added:
    • Two tests basic-block-address-map-with-basic-block-sections.ll and basic-block-address-map-with-mfs.ll to exercise the combination of -basic-block-address-map with -basic-block-sections=list and '-split-machine-functions`.
    • A driver sanity test for the -fbasic-block-address-map option (basic-block-address-map.c).
    • An LLD test for testing the --lto-basic-block-address-map option. This reuses the LLVM IR from lld/test/ELF/lto/basic-block-sections.ll.
  • Renamed and modified the two existing codegen tests for basic block address map (basic-block-sections-labels-functions-sections.ll and basic-block-sections-labels.ll)

Patch is 125.95 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/74128.diff

38 Files Affected:

  • (modified) clang/include/clang/Basic/CodeGenOptions.def (+1)
  • (modified) clang/include/clang/Driver/Options.td (+4)
  • (modified) clang/lib/CodeGen/BackendUtil.cpp (+1)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+11)
  • (added) clang/test/Driver/basic-block-address-map.c (+8)
  • (modified) lld/ELF/Config.h (+3)
  • (modified) lld/ELF/Driver.cpp (+3)
  • (modified) lld/ELF/LTO.cpp (+2)
  • (modified) lld/ELF/Options.td (+3)
  • (added) lld/test/ELF/lto/basic-block-address-map.ll (+28)
  • (modified) llvm/include/llvm/CodeGen/CommandFlags.h (+2)
  • (modified) llvm/include/llvm/Object/ELFTypes.h (+65-45)
  • (modified) llvm/include/llvm/ObjectYAML/ELFYAML.h (+22-5)
  • (modified) llvm/include/llvm/Target/TargetOptions.h (+5-1)
  • (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+66-31)
  • (modified) llvm/lib/CodeGen/BasicBlockSections.cpp (+31-6)
  • (modified) llvm/lib/CodeGen/CommandFlags.cpp (+7)
  • (modified) llvm/lib/CodeGen/MachineFunction.cpp (+1)
  • (modified) llvm/lib/CodeGen/TargetPassConfig.cpp (+16-12)
  • (modified) llvm/lib/MC/MCSectionELF.cpp (-2)
  • (modified) llvm/lib/Object/ELF.cpp (+93-43)
  • (modified) llvm/lib/ObjectYAML/ELFEmitter.cpp (+38-20)
  • (modified) llvm/lib/ObjectYAML/ELFYAML.cpp (+7-2)
  • (renamed) llvm/test/CodeGen/X86/basic-block-address-map-function-sections.ll (+1)
  • (added) llvm/test/CodeGen/X86/basic-block-address-map-with-basic-block-sections.ll (+71)
  • (added) llvm/test/CodeGen/X86/basic-block-address-map-with-mfs.ll (+89)
  • (renamed) llvm/test/CodeGen/X86/basic-block-address-map.ll (+2)
  • (modified) llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-disassemble-symbolize-operands.yaml (+62-58)
  • (modified) llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml (+12-10)
  • (modified) llvm/test/tools/llvm-readobj/ELF/bb-addr-map-relocatable.test (+63-44)
  • (modified) llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test (+99-173)
  • (modified) llvm/test/tools/obj2yaml/ELF/bb-addr-map.yaml (+73-143)
  • (modified) llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml (+21-18)
  • (modified) llvm/tools/llvm-objdump/llvm-objdump.cpp (+23-21)
  • (modified) llvm/tools/llvm-readobj/ELFDumper.cpp (+25-18)
  • (modified) llvm/tools/obj2yaml/elf2yaml.cpp (+34-12)
  • (modified) llvm/unittests/Object/ELFObjectFileTest.cpp (+242-164)
  • (modified) llvm/unittests/Object/ELFTypesTest.cpp (+20-18)
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 0acb5ae134ea24..1fc960010d0525 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -88,6 +88,7 @@ ENUM_CODEGENOPT(InlineAsmDialect, InlineAsmDialectKind, 1, IAD_ATT)
 CODEGENOPT(ForbidGuardVariables , 1, 0) ///< Issue errors if C++ guard variables
                                         ///< are required.
 CODEGENOPT(FunctionSections  , 1, 0) ///< Set when -ffunction-sections is enabled.
+CODEGENOPT(BBAddrMap  , 1, 0) ///< Set when -fbasic-block-address-map is enabled.
 CODEGENOPT(InstrumentFunctions , 1, 0) ///< Set when -finstrument-functions is
                                        ///< enabled.
 CODEGENOPT(InstrumentFunctionsAfterInlining , 1, 0) ///< Set when
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 2b93ddf033499c..8a12e41280d27c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3952,6 +3952,10 @@ defm function_sections : BoolFOption<"function-sections",
   PosFlag<SetTrue, [], [ClangOption, CC1Option],
           "Place each function in its own section">,
   NegFlag<SetFalse>>;
+defm basic_block_address_map : BoolFOption<"basic-block-address-map",
+  CodeGenOpts<"BBAddrMap">, DefaultFalse,
+  PosFlag<SetTrue, [], [CC1Option], "Emit the basic block address map section.">,
+  NegFlag<SetFalse>>;
 def fbasic_block_sections_EQ : Joined<["-"], "fbasic-block-sections=">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option, CC1AsOption]>,
   HelpText<"Place each function's basic blocks in unique sections (ELF Only)">,
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index a6142d99f3b688..1baff5b4fa7229 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -374,6 +374,7 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
                               LangOptions::FPModeKind::FPM_FastHonorPragmas);
   Options.ApproxFuncFPMath = LangOpts.ApproxFunc;
 
+  Options.BBAddrMap = CodeGenOpts.BBAddrMap;
   Options.BBSections =
       llvm::StringSwitch<llvm::BasicBlockSection>(CodeGenOpts.BBSections)
           .Case("all", llvm::BasicBlockSection::All)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index acfa119805068d..43882d9bb49854 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5884,6 +5884,17 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     CmdArgs.push_back("-ffunction-sections");
   }
 
+  if (Arg *A = Args.getLastArg(options::OPT_fbasic_block_address_map,
+                               options::OPT_fno_basic_block_address_map)) {
+    if (Triple.isX86() && Triple.isOSBinFormatELF()) {
+      if (A->getOption().matches(options::OPT_fbasic_block_address_map))
+        A->render(Args, CmdArgs);
+    } else {
+      D.Diag(diag::err_drv_unsupported_opt_for_target)
+          << A->getAsString(Args) << TripleStr;
+    }
+  }
+
   if (Arg *A = Args.getLastArg(options::OPT_fbasic_block_sections_EQ)) {
     StringRef Val = A->getValue();
     if (Triple.isX86() && Triple.isOSBinFormatELF()) {
diff --git a/clang/test/Driver/basic-block-address-map.c b/clang/test/Driver/basic-block-address-map.c
new file mode 100644
index 00000000000000..022f972b412d6b
--- /dev/null
+++ b/clang/test/Driver/basic-block-address-map.c
@@ -0,0 +1,8 @@
+// RUN: %clang -### -target x86_64 -fbasic-block-address-map %s -S 2>&1 | FileCheck -check-prefix=CHECK-PRESENT %s
+// CHECK-PRESENT: -fbasic-block-address-map
+
+// RUN: %clang -### -target x86_64 -fno-basic-block-address-map %s -S 2>&1 | FileCheck %s --check-prefix=CHECK-ABSENT
+// CHECK-ABSENT-NOT: -fbasic-block-address-map
+
+// RUN: not %clang -c -target x86_64-apple-darwin10 -fbasic-block-address-map %s -S 2>&1 | FileCheck -check-prefix=CHECK-TRIPLE %s
+// CHECK-TRIPLE: error: unsupported option '-fbasic-block-address-map' for target
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index 56229334f9a44a..ffdb6ca9584cd0 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -187,6 +187,7 @@ struct Config {
   llvm::StringRef cmseOutputLib;
   StringRef zBtiReport = "none";
   StringRef zCetReport = "none";
+  bool ltoBBAddrMap;
   llvm::StringRef ltoBasicBlockSections;
   std::pair<llvm::StringRef, llvm::StringRef> thinLTOObjectSuffixReplace;
   llvm::StringRef thinLTOPrefixReplaceOld;
@@ -249,6 +250,8 @@ struct Config {
   bool ltoPGOWarnMismatch;
   bool ltoDebugPassManager;
   bool ltoEmitAsm;
+  bool ltoNewPassManager;
+  bool ltoPseudoProbeForProfiling;
   bool ltoUniqueBasicBlockSectionNames;
   bool ltoValidateAllVtablesHaveTypeInfos;
   bool ltoWholeProgramVisibility;
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 6bef09eeca015a..9ff84048192e10 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1319,6 +1319,9 @@ static void readConfigs(opt::InputArgList &args) {
   config->ltoObjPath = args.getLastArgValue(OPT_lto_obj_path_eq);
   config->ltoPartitions = args::getInteger(args, OPT_lto_partitions, 1);
   config->ltoSampleProfile = args.getLastArgValue(OPT_lto_sample_profile);
+  config->ltoBBAddrMap =
+      args.hasFlag(OPT_lto_basic_block_address_map,
+                   OPT_no_lto_basic_block_address_map, false);
   config->ltoBasicBlockSections =
       args.getLastArgValue(OPT_lto_basic_block_sections);
   config->ltoUniqueBasicBlockSectionNames =
diff --git a/lld/ELF/LTO.cpp b/lld/ELF/LTO.cpp
index 504c12aac6c569..9fb0b18cd0ea67 100644
--- a/lld/ELF/LTO.cpp
+++ b/lld/ELF/LTO.cpp
@@ -59,6 +59,8 @@ static lto::Config createConfig() {
   c.Options.FunctionSections = true;
   c.Options.DataSections = true;
 
+  c.Options.BBAddrMap = config->ltoBBAddrMap;
+
   // Check if basic block sections must be used.
   // Allowed values for --lto-basic-block-sections are "all", "labels",
   // "<file name specifying basic block ids>", or none.  This is the equivalent
diff --git a/lld/ELF/Options.td b/lld/ELF/Options.td
index c2c9cabc92a4da..c10a73e2d9c36f 100644
--- a/lld/ELF/Options.td
+++ b/lld/ELF/Options.td
@@ -637,6 +637,9 @@ def save_temps_eq: JJ<"save-temps=">, HelpText<"Save select intermediate LTO com
   Values<"resolution,preopt,promote,internalize,import,opt,precodegen,prelink,combinedindex">;
 def lto_basic_block_sections: JJ<"lto-basic-block-sections=">,
   HelpText<"Enable basic block sections for LTO">;
+defm lto_basic_block_address_map: BB<"lto-basic-block-address-map",
+  "Emit basic block address map for LTO",
+  "Do not emit basic block address map for LTO (default)">;
 defm lto_unique_basic_block_section_names: BB<"lto-unique-basic-block-section-names",
     "Give unique names to every basic block section for LTO",
     "Do not give unique names to every basic block section for LTO (default)">;
diff --git a/lld/test/ELF/lto/basic-block-address-map.ll b/lld/test/ELF/lto/basic-block-address-map.ll
new file mode 100644
index 00000000000000..b96e7a8401f851
--- /dev/null
+++ b/lld/test/ELF/lto/basic-block-address-map.ll
@@ -0,0 +1,28 @@
+; REQUIRES: x86
+; RUN: llvm-as %s -o %t.o
+; RUN: ld.lld %t.o -o %t --lto-basic-block-address-map --lto-O0 --save-temps
+; RUN: llvm-readobj --sections %t.lto.o | FileCheck --check-prefix=SECNAMES %s
+
+; SECNAMES: Type: SHT_LLVM_BB_ADDR_MAP
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: nounwind uwtable
+define dso_local void @foo(i32 %b) local_unnamed_addr {
+entry:
+  %tobool.not = icmp eq i32 %b, 0
+  br i1 %tobool.not, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  tail call void @foo(i32 0)
+  br label %if.end
+
+if.end:                                           ; preds = %entry, %if.then
+  ret void
+}
+
+define void @_start() {
+  call void @foo(i32 1)
+  ret void
+}
diff --git a/llvm/include/llvm/CodeGen/CommandFlags.h b/llvm/include/llvm/CodeGen/CommandFlags.h
index 6407dde5bcd6c7..73c9c7ce9a5e1f 100644
--- a/llvm/include/llvm/CodeGen/CommandFlags.h
+++ b/llvm/include/llvm/CodeGen/CommandFlags.h
@@ -156,6 +156,8 @@ struct RegisterCodeGenFlags {
   RegisterCodeGenFlags();
 };
 
+bool getEnableBBAddrMap();
+
 llvm::BasicBlockSection getBBSectionsMode(llvm::TargetOptions &Options);
 
 /// Common utility function tightly tied to the options listed here. Initializes
diff --git a/llvm/include/llvm/Object/ELFTypes.h b/llvm/include/llvm/Object/ELFTypes.h
index d3351a2d1650ed..9c201ac46f7e67 100644
--- a/llvm/include/llvm/Object/ELFTypes.h
+++ b/llvm/include/llvm/Object/ELFTypes.h
@@ -796,6 +796,44 @@ template <class ELFT> struct Elf_Mips_ABIFlags {
 
 // Struct representing the BBAddrMap for one function.
 struct BBAddrMap {
+
+  /// Bitfield of optional features to include in the PGO extended map.
+  struct Features {
+    bool FuncEntryCount : 1;
+    bool BBFreq : 1;
+    bool BrProb : 1;
+    bool MultiBBRange : 1;
+
+    bool hasPGOAnalysis() const { return FuncEntryCount || BBFreq || BrProb; }
+
+    // Encodes to minimum bit width representation.
+    uint8_t encode() const {
+      return (static_cast<uint8_t>(FuncEntryCount) << 0) |
+             (static_cast<uint8_t>(BBFreq) << 1) |
+             (static_cast<uint8_t>(BrProb) << 2) |
+             (static_cast<uint8_t>(MultiBBRange) << 3);
+    }
+
+    // Decodes from minimum bit width representation and validates no
+    // unnecessary bits are used.
+    static Expected<Features> decode(uint8_t Val) {
+      Features Feat{
+          static_cast<bool>(Val & (1 << 0)), static_cast<bool>(Val & (1 << 1)),
+          static_cast<bool>(Val & (1 << 2)), static_cast<bool>(Val & (1 << 3))};
+      if (Feat.encode() != Val)
+        return createStringError(
+            std::error_code(), "invalid encoding for BBAddrMap::Features: 0x%x",
+            Val);
+      return Feat;
+    }
+
+    bool operator==(const Features &Other) const {
+      return std::tie(FuncEntryCount, BBFreq, BrProb, MultiBBRange) ==
+             std::tie(Other.FuncEntryCount, Other.BBFreq, Other.BrProb,
+                      Other.MultiBBRange);
+    }
+  };
+
   // Struct representing the BBAddrMap information for one basic block.
   struct BBEntry {
     struct Metadata {
@@ -839,7 +877,7 @@ struct BBAddrMap {
     };
 
     uint32_t ID;     // Unique ID of this basic block.
-    uint32_t Offset; // Offset of basic block relative to function start.
+    uint32_t Offset; // Offset of basic block relative to the base address.
     uint32_t Size;   // Size of the basic block.
     Metadata MD;     // Metdata for this basic block.
 
@@ -858,59 +896,41 @@ struct BBAddrMap {
     bool hasIndirectBranch() const { return MD.HasIndirectBranch; }
   };
 
-  BBAddrMap(uint64_t Addr, std::vector<BBEntry> 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.
+    std::vector<BBEntry> BBEntries; // Basic block entries for this range.
+
+    // Equality operator for unit testing.
+    bool operator==(const BBRangeEntry &Other) const {
+      return BaseAddress == Other.BaseAddress &&
+             std::equal(BBEntries.begin(), BBEntries.end(),
+                        Other.BBEntries.begin());
+    }
+  };
 
-  // Returns the address of the corresponding function.
-  uint64_t getFunctionAddress() const { return Addr; }
+  // All ranges for this function. The first range always corresponds to the
+  // function entry.
+  std::vector<BBRangeEntry> BBRanges;
 
-  // Returns the basic block entries for this function.
-  const std::vector<BBEntry> &getBBEntries() const { return BBEntries; }
+  // Returns the function address associated with this BBAddrMap, which is
+  // stored as the `BaseAddress` of its first BBRangeEntry. Returns 0 if
+  // BBRanges is empty.
+  uint64_t getFunctionAddress() const {
+    if (BBRanges.empty())
+      return 0;
+    return BBRanges.front().BaseAddress;
+  }
 
   // Equality operator for unit testing.
   bool operator==(const BBAddrMap &Other) const {
-    return Addr == Other.Addr && std::equal(BBEntries.begin(), BBEntries.end(),
-                                            Other.BBEntries.begin());
+    return std::equal(BBRanges.begin(), BBRanges.end(), Other.BBRanges.begin());
   }
-
-  uint64_t Addr;                  // Function address
-  std::vector<BBEntry> BBEntries; // Basic block entries for this function.
 };
 
 /// A feature extension of BBAddrMap that holds information relevant to PGO.
 struct PGOAnalysisMap {
-  /// Bitfield of optional features to include in the PGO extended map.
-  struct Features {
-    bool FuncEntryCount : 1;
-    bool BBFreq : 1;
-    bool BrProb : 1;
-
-    // Encodes to minimum bit width representation.
-    uint8_t encode() const {
-      return (static_cast<uint8_t>(FuncEntryCount) << 0) |
-             (static_cast<uint8_t>(BBFreq) << 1) |
-             (static_cast<uint8_t>(BrProb) << 2);
-    }
-
-    // Decodes from minimum bit width representation and validates no
-    // unnecessary bits are used.
-    static Expected<Features> decode(uint8_t Val) {
-      Features Feat{static_cast<bool>(Val & (1 << 0)),
-                    static_cast<bool>(Val & (1 << 1)),
-                    static_cast<bool>(Val & (1 << 2))};
-      if (Feat.encode() != Val)
-        return createStringError(
-            std::error_code(),
-            "invalid encoding for PGOAnalysisMap::Features: 0x%x", Val);
-      return Feat;
-    }
-
-    bool operator==(const Features &Other) const {
-      return std::tie(FuncEntryCount, BBFreq, BrProb) ==
-             std::tie(Other.FuncEntryCount, Other.BBFreq, Other.BrProb);
-    }
-  };
-
   /// Extra basic block data with fields for block frequency and branch
   /// probability.
   struct PGOBBEntry {
@@ -942,7 +962,7 @@ struct PGOAnalysisMap {
   std::vector<PGOBBEntry> BBEntries; // Extended basic block entries
 
   // Flags to indicate if each PGO related info was enabled in this function
-  Features FeatEnable;
+  BBAddrMap::Features FeatEnable;
 
   bool operator==(const PGOAnalysisMap &Other) const {
     return std::tie(FuncEntryCount, BBEntries, FeatEnable) ==
diff --git a/llvm/include/llvm/ObjectYAML/ELFYAML.h b/llvm/include/llvm/ObjectYAML/ELFYAML.h
index 12b47c271da2cd..8f045d6383623b 100644
--- a/llvm/include/llvm/ObjectYAML/ELFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/ELFYAML.h
@@ -165,9 +165,21 @@ struct BBAddrMapEntry {
   };
   uint8_t Version;
   llvm::yaml::Hex8 Feature;
-  llvm::yaml::Hex64 Address;
-  std::optional<uint64_t> NumBlocks;
-  std::optional<std::vector<BBEntry>> BBEntries;
+
+  struct BBRangeEntry {
+    llvm::yaml::Hex64 BaseAddress;
+    std::optional<uint64_t> NumBlocks;
+    std::optional<std::vector<BBEntry>> BBEntries;
+  };
+
+  std::optional<uint64_t> NumBBRanges;
+  std::optional<std::vector<BBRangeEntry>> BBRanges;
+
+  llvm::yaml::Hex64 getFunctionAddress() const {
+    if (!BBRanges || BBRanges->empty())
+      return 0;
+    return BBRanges->front().BaseAddress;
+  }
 };
 
 struct PGOAnalysisMapEntry {
@@ -751,6 +763,7 @@ bool shouldAllocateFileSpace(ArrayRef<ProgramHeader> Phdrs,
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::StackSizeEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry::BBEntry)
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry::BBRangeEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::PGOAnalysisMapEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::PGOAnalysisMapEntry::PGOBBEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(
@@ -916,11 +929,15 @@ template <> struct MappingTraits<ELFYAML::StackSizeEntry> {
 };
 
 template <> struct MappingTraits<ELFYAML::BBAddrMapEntry> {
-  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry &Rel);
+  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry &E);
+};
+
+template <> struct MappingTraits<ELFYAML::BBAddrMapEntry::BBRangeEntry> {
+  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry::BBRangeEntry &E);
 };
 
 template <> struct MappingTraits<ELFYAML::BBAddrMapEntry::BBEntry> {
-  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry::BBEntry &Rel);
+  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry::BBEntry &E);
 };
 
 template <> struct MappingTraits<ELFYAML::PGOAnalysisMapEntry> {
diff --git a/llvm/include/llvm/Target/TargetOptions.h b/llvm/include/llvm/Target/TargetOptions.h
index 4df897c047a38a..4a39c539e70a94 100644
--- a/llvm/include/llvm/Target/TargetOptions.h
+++ b/llvm/include/llvm/Target/TargetOptions.h
@@ -147,7 +147,7 @@ namespace llvm {
           TrapUnreachable(false), NoTrapAfterNoreturn(false), TLSSize(0),
           EmulatedTLS(false), EnableIPRA(false), EmitStackSizeSection(false),
           EnableMachineOutliner(false), EnableMachineFunctionSplitter(false),
-          SupportsDefaultOutlining(false), EmitAddrsig(false),
+          SupportsDefaultOutlining(false), EmitAddrsig(false), BBAddrMap(false),
           EmitCallSiteInfo(false), SupportsDebugEntryValues(false),
           EnableDebugEntryValues(false), ValueTrackingVariableLocations(false),
           ForceDwarfFrameSection(false), XRayFunctionIndex(true),
@@ -313,6 +313,10 @@ namespace llvm {
     /// Emit address-significance table.
     unsigned EmitAddrsig : 1;
 
+    // Emit the SHT_LLVM_BB_ADDR_MAP section containing basic block address
+    // which can be used to map virtual addresses to machine basic blocks.
+    unsigned BBAddrMap : 1;
+
     /// Emit basic blocks into separate sections.
     BasicBlockSection BBSections = BasicBlockSection::None;
 
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 7df1c82bf357f6..ca9e5684f93fd6 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -141,10 +141,6 @@ static cl::opt<std::string> BasicBlockProfileDump(
              "performed with -basic-block-sections=labels. Enabling this "
              "flag during in-process ThinLTO is not supported."));
 
-// This is a replication of fields of object::PGOAnalysisMap::Features. It
-// should match the order of the fields so that
-// `object::PGOAnalysisMap::Features::decode(PgoAnalysisMapFeatures.getBits())`
-// succeeds.
 enum class PGOMapFeaturesEnum {
   FuncEntryCount,
   BBFreq,
@@ -158,8 +154,9 @@ static cl::bits<PGOMapFeaturesEnum> PgoAnalysisMapFeatures(
                           "Basic Block Frequency"),
                clEnumValN(PGOMapFeaturesEnum::BrProb, "br-prob",
                           "Branch Probability")),
-    cl::desc("Enable extended information within the BBAddrMap that is "
-             "extracted from PGO related analysis."));
+    cl::desc(
+        "Enable extended information within the SHT_LLVM_BB_ADDR_MAP that is "
+        "extracted from PGO related analysis."));
 
 const char DWARFGroupName[] = "dwarf";
 const char DWARFGroupDescription[] = "DWARF Emission";
@@ -1388,6 +1385,14 @@ static uint32_t getBBAddrMapMetadata(const MachineBasicBlock &MBB) {
       .encode();
 }
 
+static llvm::object::BBAddrMap::Features
+getBBAddrMapFeature(const MachineFunction &MF, int NumMBBSectionRanges) {
+  return {PgoAnalysisMapFeatures.isSet(PGOMapFeaturesEnum::FuncEntryCount),
+          PgoAnalysisMapFeatures.isSet(PGOMapFeaturesEnum::BBFreq),
+          PgoAnalysisMapFeatures.isSet(PGOMapFeaturesEnum::BrProb),
+          MF.hasBBSections() && NumMBBSectionRanges > 1};
+}
+
 void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
   MCSection *BBAddrMapSection =
       getObjFileLowering().getBBAddrMapSection(*MF.getSection());
@@ -1401,17 +1406,48 @@ void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
   uint8_t BBAddrMapVersion = OutStreamer->getContext().getBBAddrMapVersion();
   OutStreamer->emitInt8(BBAddrMapVersion);
   OutStreamer->AddComment("feature");
-  auto FeaturesBits = static_cast<uint8_t>(PgoAnalysisMapFeatures.getBits());
-  OutStreamer->emitInt8(FeaturesBits);
-  OutStreamer->AddComment("function address");
-  OutStreamer->emitSymbolValue(FunctionSymbol, getPointerSize());
-  OutStrea...
[truncated]

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 4, 2024

@llvm/pr-subscribers-llvm-binary-utilities

Author: Rahman Lavaee (rlavaee)

Changes

Today -split-machine-functions and -fbasic-block-sections={all,list} cannot be combined with -basic-block-sections=labels (the labels option will be ignored).
The inconsistency comes from the way basic block address map -- the underlying mechanism for basic block labels -- encodes basic block addresses (https://lists.llvm.org/pipermail/llvm-dev/2020-July/143512.html). Specifically, basic block offsets are computed relative to the function begin symbol. This relies on functions being contiguous which is not the case for MFS and basic block section binaries. This means Propeller cannot use binary profiles collected from these binaries, which limits the applicability of Propeller for iterative optimization.

To make the SHT_LLVM_BB_ADDR_MAP feature work with basic block section binaries, we propose modifying the encoding of this section as follows.

First let us review the current encoding which emits the address of each function and its number of basic blocks, followed by basic block entries for each basic block.

Address of the function Function Address
Number of basic blocks in this function NumBlocks
BB entry 1
BB entry 2
...
BB entry #NumBlocks

To make this work for basic block sections, we treat each basic block section similar to a function, except that basic block sections of the same function must be encapsulated in the same structure so we can map all of them to their single function.

We modify the encoding to first emit the number of basic block sections (BB ranges) in the function. Then we emit the address map of each basic block section section as before: the base address of the section, its number of blocks, and BB entries for its basic block. The first section in the BB address map is always the function entry section.

Number of sections for this function NumBBRanges
Section 1 begin address BaseAddress[1]
Number of basic blocks in section 1 NumBlocks[1]
BB entries for Section 1
..................
Section #NumBBRanges begin address BaseAddress[NumBBRanges]
Number of basic blocks in section #NumBBRanges NumBlocks[NumBBRanges]
BB entries for Section #NumBBRanges

The encoding of basic block entries remains as before with the minor change that each basic block offset is now computed relative to the begin symbol of its containing BB section.

This patch adds a new boolean codegen option -basic-block-address-map. Correspondingly, the front-end flag -fbasic-block-address-map and LLD flag --lto-basic-block-address-map are introduced.
Analogously, we add a new TargetOption field BBAddrMap. This means BB address maps are either generated for all functions in the compiling unit, or for none (depending on TargetOptions::BBAddrMap).

This patch keeps the functionality of the old -fbasic-block-sections=labels option but does not remove it. A subsequent patch will remove the obsolete option.

We refactor the BasicBlockSections pass by separating the BB address map and BB sections handing to their own functions (named handleBBAddrMap and handleBBSections). handleBBSections renumbers basic blocks and places them in their assigned sections. handleBBAddrMap is invoked after handleBBSections (if requested) and only renumbers the blocks.

  • New tests added:
    • Two tests basic-block-address-map-with-basic-block-sections.ll and basic-block-address-map-with-mfs.ll to exercise the combination of -basic-block-address-map with -basic-block-sections=list and '-split-machine-functions`.
    • A driver sanity test for the -fbasic-block-address-map option (basic-block-address-map.c).
    • An LLD test for testing the --lto-basic-block-address-map option. This reuses the LLVM IR from lld/test/ELF/lto/basic-block-sections.ll.
  • Renamed and modified the two existing codegen tests for basic block address map (basic-block-sections-labels-functions-sections.ll and basic-block-sections-labels.ll)

Patch is 125.95 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/74128.diff

38 Files Affected:

  • (modified) clang/include/clang/Basic/CodeGenOptions.def (+1)
  • (modified) clang/include/clang/Driver/Options.td (+4)
  • (modified) clang/lib/CodeGen/BackendUtil.cpp (+1)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+11)
  • (added) clang/test/Driver/basic-block-address-map.c (+8)
  • (modified) lld/ELF/Config.h (+3)
  • (modified) lld/ELF/Driver.cpp (+3)
  • (modified) lld/ELF/LTO.cpp (+2)
  • (modified) lld/ELF/Options.td (+3)
  • (added) lld/test/ELF/lto/basic-block-address-map.ll (+28)
  • (modified) llvm/include/llvm/CodeGen/CommandFlags.h (+2)
  • (modified) llvm/include/llvm/Object/ELFTypes.h (+65-45)
  • (modified) llvm/include/llvm/ObjectYAML/ELFYAML.h (+22-5)
  • (modified) llvm/include/llvm/Target/TargetOptions.h (+5-1)
  • (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+66-31)
  • (modified) llvm/lib/CodeGen/BasicBlockSections.cpp (+31-6)
  • (modified) llvm/lib/CodeGen/CommandFlags.cpp (+7)
  • (modified) llvm/lib/CodeGen/MachineFunction.cpp (+1)
  • (modified) llvm/lib/CodeGen/TargetPassConfig.cpp (+16-12)
  • (modified) llvm/lib/MC/MCSectionELF.cpp (-2)
  • (modified) llvm/lib/Object/ELF.cpp (+93-43)
  • (modified) llvm/lib/ObjectYAML/ELFEmitter.cpp (+38-20)
  • (modified) llvm/lib/ObjectYAML/ELFYAML.cpp (+7-2)
  • (renamed) llvm/test/CodeGen/X86/basic-block-address-map-function-sections.ll (+1)
  • (added) llvm/test/CodeGen/X86/basic-block-address-map-with-basic-block-sections.ll (+71)
  • (added) llvm/test/CodeGen/X86/basic-block-address-map-with-mfs.ll (+89)
  • (renamed) llvm/test/CodeGen/X86/basic-block-address-map.ll (+2)
  • (modified) llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-disassemble-symbolize-operands.yaml (+62-58)
  • (modified) llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml (+12-10)
  • (modified) llvm/test/tools/llvm-readobj/ELF/bb-addr-map-relocatable.test (+63-44)
  • (modified) llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test (+99-173)
  • (modified) llvm/test/tools/obj2yaml/ELF/bb-addr-map.yaml (+73-143)
  • (modified) llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml (+21-18)
  • (modified) llvm/tools/llvm-objdump/llvm-objdump.cpp (+23-21)
  • (modified) llvm/tools/llvm-readobj/ELFDumper.cpp (+25-18)
  • (modified) llvm/tools/obj2yaml/elf2yaml.cpp (+34-12)
  • (modified) llvm/unittests/Object/ELFObjectFileTest.cpp (+242-164)
  • (modified) llvm/unittests/Object/ELFTypesTest.cpp (+20-18)
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 0acb5ae134ea24..1fc960010d0525 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -88,6 +88,7 @@ ENUM_CODEGENOPT(InlineAsmDialect, InlineAsmDialectKind, 1, IAD_ATT)
 CODEGENOPT(ForbidGuardVariables , 1, 0) ///< Issue errors if C++ guard variables
                                         ///< are required.
 CODEGENOPT(FunctionSections  , 1, 0) ///< Set when -ffunction-sections is enabled.
+CODEGENOPT(BBAddrMap  , 1, 0) ///< Set when -fbasic-block-address-map is enabled.
 CODEGENOPT(InstrumentFunctions , 1, 0) ///< Set when -finstrument-functions is
                                        ///< enabled.
 CODEGENOPT(InstrumentFunctionsAfterInlining , 1, 0) ///< Set when
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 2b93ddf033499c..8a12e41280d27c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3952,6 +3952,10 @@ defm function_sections : BoolFOption<"function-sections",
   PosFlag<SetTrue, [], [ClangOption, CC1Option],
           "Place each function in its own section">,
   NegFlag<SetFalse>>;
+defm basic_block_address_map : BoolFOption<"basic-block-address-map",
+  CodeGenOpts<"BBAddrMap">, DefaultFalse,
+  PosFlag<SetTrue, [], [CC1Option], "Emit the basic block address map section.">,
+  NegFlag<SetFalse>>;
 def fbasic_block_sections_EQ : Joined<["-"], "fbasic-block-sections=">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option, CC1AsOption]>,
   HelpText<"Place each function's basic blocks in unique sections (ELF Only)">,
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index a6142d99f3b688..1baff5b4fa7229 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -374,6 +374,7 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
                               LangOptions::FPModeKind::FPM_FastHonorPragmas);
   Options.ApproxFuncFPMath = LangOpts.ApproxFunc;
 
+  Options.BBAddrMap = CodeGenOpts.BBAddrMap;
   Options.BBSections =
       llvm::StringSwitch<llvm::BasicBlockSection>(CodeGenOpts.BBSections)
           .Case("all", llvm::BasicBlockSection::All)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index acfa119805068d..43882d9bb49854 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5884,6 +5884,17 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     CmdArgs.push_back("-ffunction-sections");
   }
 
+  if (Arg *A = Args.getLastArg(options::OPT_fbasic_block_address_map,
+                               options::OPT_fno_basic_block_address_map)) {
+    if (Triple.isX86() && Triple.isOSBinFormatELF()) {
+      if (A->getOption().matches(options::OPT_fbasic_block_address_map))
+        A->render(Args, CmdArgs);
+    } else {
+      D.Diag(diag::err_drv_unsupported_opt_for_target)
+          << A->getAsString(Args) << TripleStr;
+    }
+  }
+
   if (Arg *A = Args.getLastArg(options::OPT_fbasic_block_sections_EQ)) {
     StringRef Val = A->getValue();
     if (Triple.isX86() && Triple.isOSBinFormatELF()) {
diff --git a/clang/test/Driver/basic-block-address-map.c b/clang/test/Driver/basic-block-address-map.c
new file mode 100644
index 00000000000000..022f972b412d6b
--- /dev/null
+++ b/clang/test/Driver/basic-block-address-map.c
@@ -0,0 +1,8 @@
+// RUN: %clang -### -target x86_64 -fbasic-block-address-map %s -S 2>&1 | FileCheck -check-prefix=CHECK-PRESENT %s
+// CHECK-PRESENT: -fbasic-block-address-map
+
+// RUN: %clang -### -target x86_64 -fno-basic-block-address-map %s -S 2>&1 | FileCheck %s --check-prefix=CHECK-ABSENT
+// CHECK-ABSENT-NOT: -fbasic-block-address-map
+
+// RUN: not %clang -c -target x86_64-apple-darwin10 -fbasic-block-address-map %s -S 2>&1 | FileCheck -check-prefix=CHECK-TRIPLE %s
+// CHECK-TRIPLE: error: unsupported option '-fbasic-block-address-map' for target
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index 56229334f9a44a..ffdb6ca9584cd0 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -187,6 +187,7 @@ struct Config {
   llvm::StringRef cmseOutputLib;
   StringRef zBtiReport = "none";
   StringRef zCetReport = "none";
+  bool ltoBBAddrMap;
   llvm::StringRef ltoBasicBlockSections;
   std::pair<llvm::StringRef, llvm::StringRef> thinLTOObjectSuffixReplace;
   llvm::StringRef thinLTOPrefixReplaceOld;
@@ -249,6 +250,8 @@ struct Config {
   bool ltoPGOWarnMismatch;
   bool ltoDebugPassManager;
   bool ltoEmitAsm;
+  bool ltoNewPassManager;
+  bool ltoPseudoProbeForProfiling;
   bool ltoUniqueBasicBlockSectionNames;
   bool ltoValidateAllVtablesHaveTypeInfos;
   bool ltoWholeProgramVisibility;
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 6bef09eeca015a..9ff84048192e10 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1319,6 +1319,9 @@ static void readConfigs(opt::InputArgList &args) {
   config->ltoObjPath = args.getLastArgValue(OPT_lto_obj_path_eq);
   config->ltoPartitions = args::getInteger(args, OPT_lto_partitions, 1);
   config->ltoSampleProfile = args.getLastArgValue(OPT_lto_sample_profile);
+  config->ltoBBAddrMap =
+      args.hasFlag(OPT_lto_basic_block_address_map,
+                   OPT_no_lto_basic_block_address_map, false);
   config->ltoBasicBlockSections =
       args.getLastArgValue(OPT_lto_basic_block_sections);
   config->ltoUniqueBasicBlockSectionNames =
diff --git a/lld/ELF/LTO.cpp b/lld/ELF/LTO.cpp
index 504c12aac6c569..9fb0b18cd0ea67 100644
--- a/lld/ELF/LTO.cpp
+++ b/lld/ELF/LTO.cpp
@@ -59,6 +59,8 @@ static lto::Config createConfig() {
   c.Options.FunctionSections = true;
   c.Options.DataSections = true;
 
+  c.Options.BBAddrMap = config->ltoBBAddrMap;
+
   // Check if basic block sections must be used.
   // Allowed values for --lto-basic-block-sections are "all", "labels",
   // "<file name specifying basic block ids>", or none.  This is the equivalent
diff --git a/lld/ELF/Options.td b/lld/ELF/Options.td
index c2c9cabc92a4da..c10a73e2d9c36f 100644
--- a/lld/ELF/Options.td
+++ b/lld/ELF/Options.td
@@ -637,6 +637,9 @@ def save_temps_eq: JJ<"save-temps=">, HelpText<"Save select intermediate LTO com
   Values<"resolution,preopt,promote,internalize,import,opt,precodegen,prelink,combinedindex">;
 def lto_basic_block_sections: JJ<"lto-basic-block-sections=">,
   HelpText<"Enable basic block sections for LTO">;
+defm lto_basic_block_address_map: BB<"lto-basic-block-address-map",
+  "Emit basic block address map for LTO",
+  "Do not emit basic block address map for LTO (default)">;
 defm lto_unique_basic_block_section_names: BB<"lto-unique-basic-block-section-names",
     "Give unique names to every basic block section for LTO",
     "Do not give unique names to every basic block section for LTO (default)">;
diff --git a/lld/test/ELF/lto/basic-block-address-map.ll b/lld/test/ELF/lto/basic-block-address-map.ll
new file mode 100644
index 00000000000000..b96e7a8401f851
--- /dev/null
+++ b/lld/test/ELF/lto/basic-block-address-map.ll
@@ -0,0 +1,28 @@
+; REQUIRES: x86
+; RUN: llvm-as %s -o %t.o
+; RUN: ld.lld %t.o -o %t --lto-basic-block-address-map --lto-O0 --save-temps
+; RUN: llvm-readobj --sections %t.lto.o | FileCheck --check-prefix=SECNAMES %s
+
+; SECNAMES: Type: SHT_LLVM_BB_ADDR_MAP
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: nounwind uwtable
+define dso_local void @foo(i32 %b) local_unnamed_addr {
+entry:
+  %tobool.not = icmp eq i32 %b, 0
+  br i1 %tobool.not, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  tail call void @foo(i32 0)
+  br label %if.end
+
+if.end:                                           ; preds = %entry, %if.then
+  ret void
+}
+
+define void @_start() {
+  call void @foo(i32 1)
+  ret void
+}
diff --git a/llvm/include/llvm/CodeGen/CommandFlags.h b/llvm/include/llvm/CodeGen/CommandFlags.h
index 6407dde5bcd6c7..73c9c7ce9a5e1f 100644
--- a/llvm/include/llvm/CodeGen/CommandFlags.h
+++ b/llvm/include/llvm/CodeGen/CommandFlags.h
@@ -156,6 +156,8 @@ struct RegisterCodeGenFlags {
   RegisterCodeGenFlags();
 };
 
+bool getEnableBBAddrMap();
+
 llvm::BasicBlockSection getBBSectionsMode(llvm::TargetOptions &Options);
 
 /// Common utility function tightly tied to the options listed here. Initializes
diff --git a/llvm/include/llvm/Object/ELFTypes.h b/llvm/include/llvm/Object/ELFTypes.h
index d3351a2d1650ed..9c201ac46f7e67 100644
--- a/llvm/include/llvm/Object/ELFTypes.h
+++ b/llvm/include/llvm/Object/ELFTypes.h
@@ -796,6 +796,44 @@ template <class ELFT> struct Elf_Mips_ABIFlags {
 
 // Struct representing the BBAddrMap for one function.
 struct BBAddrMap {
+
+  /// Bitfield of optional features to include in the PGO extended map.
+  struct Features {
+    bool FuncEntryCount : 1;
+    bool BBFreq : 1;
+    bool BrProb : 1;
+    bool MultiBBRange : 1;
+
+    bool hasPGOAnalysis() const { return FuncEntryCount || BBFreq || BrProb; }
+
+    // Encodes to minimum bit width representation.
+    uint8_t encode() const {
+      return (static_cast<uint8_t>(FuncEntryCount) << 0) |
+             (static_cast<uint8_t>(BBFreq) << 1) |
+             (static_cast<uint8_t>(BrProb) << 2) |
+             (static_cast<uint8_t>(MultiBBRange) << 3);
+    }
+
+    // Decodes from minimum bit width representation and validates no
+    // unnecessary bits are used.
+    static Expected<Features> decode(uint8_t Val) {
+      Features Feat{
+          static_cast<bool>(Val & (1 << 0)), static_cast<bool>(Val & (1 << 1)),
+          static_cast<bool>(Val & (1 << 2)), static_cast<bool>(Val & (1 << 3))};
+      if (Feat.encode() != Val)
+        return createStringError(
+            std::error_code(), "invalid encoding for BBAddrMap::Features: 0x%x",
+            Val);
+      return Feat;
+    }
+
+    bool operator==(const Features &Other) const {
+      return std::tie(FuncEntryCount, BBFreq, BrProb, MultiBBRange) ==
+             std::tie(Other.FuncEntryCount, Other.BBFreq, Other.BrProb,
+                      Other.MultiBBRange);
+    }
+  };
+
   // Struct representing the BBAddrMap information for one basic block.
   struct BBEntry {
     struct Metadata {
@@ -839,7 +877,7 @@ struct BBAddrMap {
     };
 
     uint32_t ID;     // Unique ID of this basic block.
-    uint32_t Offset; // Offset of basic block relative to function start.
+    uint32_t Offset; // Offset of basic block relative to the base address.
     uint32_t Size;   // Size of the basic block.
     Metadata MD;     // Metdata for this basic block.
 
@@ -858,59 +896,41 @@ struct BBAddrMap {
     bool hasIndirectBranch() const { return MD.HasIndirectBranch; }
   };
 
-  BBAddrMap(uint64_t Addr, std::vector<BBEntry> 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.
+    std::vector<BBEntry> BBEntries; // Basic block entries for this range.
+
+    // Equality operator for unit testing.
+    bool operator==(const BBRangeEntry &Other) const {
+      return BaseAddress == Other.BaseAddress &&
+             std::equal(BBEntries.begin(), BBEntries.end(),
+                        Other.BBEntries.begin());
+    }
+  };
 
-  // Returns the address of the corresponding function.
-  uint64_t getFunctionAddress() const { return Addr; }
+  // All ranges for this function. The first range always corresponds to the
+  // function entry.
+  std::vector<BBRangeEntry> BBRanges;
 
-  // Returns the basic block entries for this function.
-  const std::vector<BBEntry> &getBBEntries() const { return BBEntries; }
+  // Returns the function address associated with this BBAddrMap, which is
+  // stored as the `BaseAddress` of its first BBRangeEntry. Returns 0 if
+  // BBRanges is empty.
+  uint64_t getFunctionAddress() const {
+    if (BBRanges.empty())
+      return 0;
+    return BBRanges.front().BaseAddress;
+  }
 
   // Equality operator for unit testing.
   bool operator==(const BBAddrMap &Other) const {
-    return Addr == Other.Addr && std::equal(BBEntries.begin(), BBEntries.end(),
-                                            Other.BBEntries.begin());
+    return std::equal(BBRanges.begin(), BBRanges.end(), Other.BBRanges.begin());
   }
-
-  uint64_t Addr;                  // Function address
-  std::vector<BBEntry> BBEntries; // Basic block entries for this function.
 };
 
 /// A feature extension of BBAddrMap that holds information relevant to PGO.
 struct PGOAnalysisMap {
-  /// Bitfield of optional features to include in the PGO extended map.
-  struct Features {
-    bool FuncEntryCount : 1;
-    bool BBFreq : 1;
-    bool BrProb : 1;
-
-    // Encodes to minimum bit width representation.
-    uint8_t encode() const {
-      return (static_cast<uint8_t>(FuncEntryCount) << 0) |
-             (static_cast<uint8_t>(BBFreq) << 1) |
-             (static_cast<uint8_t>(BrProb) << 2);
-    }
-
-    // Decodes from minimum bit width representation and validates no
-    // unnecessary bits are used.
-    static Expected<Features> decode(uint8_t Val) {
-      Features Feat{static_cast<bool>(Val & (1 << 0)),
-                    static_cast<bool>(Val & (1 << 1)),
-                    static_cast<bool>(Val & (1 << 2))};
-      if (Feat.encode() != Val)
-        return createStringError(
-            std::error_code(),
-            "invalid encoding for PGOAnalysisMap::Features: 0x%x", Val);
-      return Feat;
-    }
-
-    bool operator==(const Features &Other) const {
-      return std::tie(FuncEntryCount, BBFreq, BrProb) ==
-             std::tie(Other.FuncEntryCount, Other.BBFreq, Other.BrProb);
-    }
-  };
-
   /// Extra basic block data with fields for block frequency and branch
   /// probability.
   struct PGOBBEntry {
@@ -942,7 +962,7 @@ struct PGOAnalysisMap {
   std::vector<PGOBBEntry> BBEntries; // Extended basic block entries
 
   // Flags to indicate if each PGO related info was enabled in this function
-  Features FeatEnable;
+  BBAddrMap::Features FeatEnable;
 
   bool operator==(const PGOAnalysisMap &Other) const {
     return std::tie(FuncEntryCount, BBEntries, FeatEnable) ==
diff --git a/llvm/include/llvm/ObjectYAML/ELFYAML.h b/llvm/include/llvm/ObjectYAML/ELFYAML.h
index 12b47c271da2cd..8f045d6383623b 100644
--- a/llvm/include/llvm/ObjectYAML/ELFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/ELFYAML.h
@@ -165,9 +165,21 @@ struct BBAddrMapEntry {
   };
   uint8_t Version;
   llvm::yaml::Hex8 Feature;
-  llvm::yaml::Hex64 Address;
-  std::optional<uint64_t> NumBlocks;
-  std::optional<std::vector<BBEntry>> BBEntries;
+
+  struct BBRangeEntry {
+    llvm::yaml::Hex64 BaseAddress;
+    std::optional<uint64_t> NumBlocks;
+    std::optional<std::vector<BBEntry>> BBEntries;
+  };
+
+  std::optional<uint64_t> NumBBRanges;
+  std::optional<std::vector<BBRangeEntry>> BBRanges;
+
+  llvm::yaml::Hex64 getFunctionAddress() const {
+    if (!BBRanges || BBRanges->empty())
+      return 0;
+    return BBRanges->front().BaseAddress;
+  }
 };
 
 struct PGOAnalysisMapEntry {
@@ -751,6 +763,7 @@ bool shouldAllocateFileSpace(ArrayRef<ProgramHeader> Phdrs,
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::StackSizeEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry::BBEntry)
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry::BBRangeEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::PGOAnalysisMapEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::PGOAnalysisMapEntry::PGOBBEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(
@@ -916,11 +929,15 @@ template <> struct MappingTraits<ELFYAML::StackSizeEntry> {
 };
 
 template <> struct MappingTraits<ELFYAML::BBAddrMapEntry> {
-  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry &Rel);
+  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry &E);
+};
+
+template <> struct MappingTraits<ELFYAML::BBAddrMapEntry::BBRangeEntry> {
+  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry::BBRangeEntry &E);
 };
 
 template <> struct MappingTraits<ELFYAML::BBAddrMapEntry::BBEntry> {
-  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry::BBEntry &Rel);
+  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry::BBEntry &E);
 };
 
 template <> struct MappingTraits<ELFYAML::PGOAnalysisMapEntry> {
diff --git a/llvm/include/llvm/Target/TargetOptions.h b/llvm/include/llvm/Target/TargetOptions.h
index 4df897c047a38a..4a39c539e70a94 100644
--- a/llvm/include/llvm/Target/TargetOptions.h
+++ b/llvm/include/llvm/Target/TargetOptions.h
@@ -147,7 +147,7 @@ namespace llvm {
           TrapUnreachable(false), NoTrapAfterNoreturn(false), TLSSize(0),
           EmulatedTLS(false), EnableIPRA(false), EmitStackSizeSection(false),
           EnableMachineOutliner(false), EnableMachineFunctionSplitter(false),
-          SupportsDefaultOutlining(false), EmitAddrsig(false),
+          SupportsDefaultOutlining(false), EmitAddrsig(false), BBAddrMap(false),
           EmitCallSiteInfo(false), SupportsDebugEntryValues(false),
           EnableDebugEntryValues(false), ValueTrackingVariableLocations(false),
           ForceDwarfFrameSection(false), XRayFunctionIndex(true),
@@ -313,6 +313,10 @@ namespace llvm {
     /// Emit address-significance table.
     unsigned EmitAddrsig : 1;
 
+    // Emit the SHT_LLVM_BB_ADDR_MAP section containing basic block address
+    // which can be used to map virtual addresses to machine basic blocks.
+    unsigned BBAddrMap : 1;
+
     /// Emit basic blocks into separate sections.
     BasicBlockSection BBSections = BasicBlockSection::None;
 
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 7df1c82bf357f6..ca9e5684f93fd6 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -141,10 +141,6 @@ static cl::opt<std::string> BasicBlockProfileDump(
              "performed with -basic-block-sections=labels. Enabling this "
              "flag during in-process ThinLTO is not supported."));
 
-// This is a replication of fields of object::PGOAnalysisMap::Features. It
-// should match the order of the fields so that
-// `object::PGOAnalysisMap::Features::decode(PgoAnalysisMapFeatures.getBits())`
-// succeeds.
 enum class PGOMapFeaturesEnum {
   FuncEntryCount,
   BBFreq,
@@ -158,8 +154,9 @@ static cl::bits<PGOMapFeaturesEnum> PgoAnalysisMapFeatures(
                           "Basic Block Frequency"),
                clEnumValN(PGOMapFeaturesEnum::BrProb, "br-prob",
                           "Branch Probability")),
-    cl::desc("Enable extended information within the BBAddrMap that is "
-             "extracted from PGO related analysis."));
+    cl::desc(
+        "Enable extended information within the SHT_LLVM_BB_ADDR_MAP that is "
+        "extracted from PGO related analysis."));
 
 const char DWARFGroupName[] = "dwarf";
 const char DWARFGroupDescription[] = "DWARF Emission";
@@ -1388,6 +1385,14 @@ static uint32_t getBBAddrMapMetadata(const MachineBasicBlock &MBB) {
       .encode();
 }
 
+static llvm::object::BBAddrMap::Features
+getBBAddrMapFeature(const MachineFunction &MF, int NumMBBSectionRanges) {
+  return {PgoAnalysisMapFeatures.isSet(PGOMapFeaturesEnum::FuncEntryCount),
+          PgoAnalysisMapFeatures.isSet(PGOMapFeaturesEnum::BBFreq),
+          PgoAnalysisMapFeatures.isSet(PGOMapFeaturesEnum::BrProb),
+          MF.hasBBSections() && NumMBBSectionRanges > 1};
+}
+
 void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
   MCSection *BBAddrMapSection =
       getObjFileLowering().getBBAddrMapSection(*MF.getSection());
@@ -1401,17 +1406,48 @@ void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
   uint8_t BBAddrMapVersion = OutStreamer->getContext().getBBAddrMapVersion();
   OutStreamer->emitInt8(BBAddrMapVersion);
   OutStreamer->AddComment("feature");
-  auto FeaturesBits = static_cast<uint8_t>(PgoAnalysisMapFeatures.getBits());
-  OutStreamer->emitInt8(FeaturesBits);
-  OutStreamer->AddComment("function address");
-  OutStreamer->emitSymbolValue(FunctionSymbol, getPointerSize());
-  OutStrea...
[truncated]

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 4, 2024

@llvm/pr-subscribers-clang-codegen

Author: Rahman Lavaee (rlavaee)

Changes

Today -split-machine-functions and -fbasic-block-sections={all,list} cannot be combined with -basic-block-sections=labels (the labels option will be ignored).
The inconsistency comes from the way basic block address map -- the underlying mechanism for basic block labels -- encodes basic block addresses (https://lists.llvm.org/pipermail/llvm-dev/2020-July/143512.html). Specifically, basic block offsets are computed relative to the function begin symbol. This relies on functions being contiguous which is not the case for MFS and basic block section binaries. This means Propeller cannot use binary profiles collected from these binaries, which limits the applicability of Propeller for iterative optimization.

To make the SHT_LLVM_BB_ADDR_MAP feature work with basic block section binaries, we propose modifying the encoding of this section as follows.

First let us review the current encoding which emits the address of each function and its number of basic blocks, followed by basic block entries for each basic block.

Address of the function Function Address
Number of basic blocks in this function NumBlocks
BB entry 1
BB entry 2
...
BB entry #NumBlocks

To make this work for basic block sections, we treat each basic block section similar to a function, except that basic block sections of the same function must be encapsulated in the same structure so we can map all of them to their single function.

We modify the encoding to first emit the number of basic block sections (BB ranges) in the function. Then we emit the address map of each basic block section section as before: the base address of the section, its number of blocks, and BB entries for its basic block. The first section in the BB address map is always the function entry section.

Number of sections for this function NumBBRanges
Section 1 begin address BaseAddress[1]
Number of basic blocks in section 1 NumBlocks[1]
BB entries for Section 1
..................
Section #NumBBRanges begin address BaseAddress[NumBBRanges]
Number of basic blocks in section #NumBBRanges NumBlocks[NumBBRanges]
BB entries for Section #NumBBRanges

The encoding of basic block entries remains as before with the minor change that each basic block offset is now computed relative to the begin symbol of its containing BB section.

This patch adds a new boolean codegen option -basic-block-address-map. Correspondingly, the front-end flag -fbasic-block-address-map and LLD flag --lto-basic-block-address-map are introduced.
Analogously, we add a new TargetOption field BBAddrMap. This means BB address maps are either generated for all functions in the compiling unit, or for none (depending on TargetOptions::BBAddrMap).

This patch keeps the functionality of the old -fbasic-block-sections=labels option but does not remove it. A subsequent patch will remove the obsolete option.

We refactor the BasicBlockSections pass by separating the BB address map and BB sections handing to their own functions (named handleBBAddrMap and handleBBSections). handleBBSections renumbers basic blocks and places them in their assigned sections. handleBBAddrMap is invoked after handleBBSections (if requested) and only renumbers the blocks.

  • New tests added:
    • Two tests basic-block-address-map-with-basic-block-sections.ll and basic-block-address-map-with-mfs.ll to exercise the combination of -basic-block-address-map with -basic-block-sections=list and '-split-machine-functions`.
    • A driver sanity test for the -fbasic-block-address-map option (basic-block-address-map.c).
    • An LLD test for testing the --lto-basic-block-address-map option. This reuses the LLVM IR from lld/test/ELF/lto/basic-block-sections.ll.
  • Renamed and modified the two existing codegen tests for basic block address map (basic-block-sections-labels-functions-sections.ll and basic-block-sections-labels.ll)

Patch is 125.95 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/74128.diff

38 Files Affected:

  • (modified) clang/include/clang/Basic/CodeGenOptions.def (+1)
  • (modified) clang/include/clang/Driver/Options.td (+4)
  • (modified) clang/lib/CodeGen/BackendUtil.cpp (+1)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+11)
  • (added) clang/test/Driver/basic-block-address-map.c (+8)
  • (modified) lld/ELF/Config.h (+3)
  • (modified) lld/ELF/Driver.cpp (+3)
  • (modified) lld/ELF/LTO.cpp (+2)
  • (modified) lld/ELF/Options.td (+3)
  • (added) lld/test/ELF/lto/basic-block-address-map.ll (+28)
  • (modified) llvm/include/llvm/CodeGen/CommandFlags.h (+2)
  • (modified) llvm/include/llvm/Object/ELFTypes.h (+65-45)
  • (modified) llvm/include/llvm/ObjectYAML/ELFYAML.h (+22-5)
  • (modified) llvm/include/llvm/Target/TargetOptions.h (+5-1)
  • (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+66-31)
  • (modified) llvm/lib/CodeGen/BasicBlockSections.cpp (+31-6)
  • (modified) llvm/lib/CodeGen/CommandFlags.cpp (+7)
  • (modified) llvm/lib/CodeGen/MachineFunction.cpp (+1)
  • (modified) llvm/lib/CodeGen/TargetPassConfig.cpp (+16-12)
  • (modified) llvm/lib/MC/MCSectionELF.cpp (-2)
  • (modified) llvm/lib/Object/ELF.cpp (+93-43)
  • (modified) llvm/lib/ObjectYAML/ELFEmitter.cpp (+38-20)
  • (modified) llvm/lib/ObjectYAML/ELFYAML.cpp (+7-2)
  • (renamed) llvm/test/CodeGen/X86/basic-block-address-map-function-sections.ll (+1)
  • (added) llvm/test/CodeGen/X86/basic-block-address-map-with-basic-block-sections.ll (+71)
  • (added) llvm/test/CodeGen/X86/basic-block-address-map-with-mfs.ll (+89)
  • (renamed) llvm/test/CodeGen/X86/basic-block-address-map.ll (+2)
  • (modified) llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-disassemble-symbolize-operands.yaml (+62-58)
  • (modified) llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml (+12-10)
  • (modified) llvm/test/tools/llvm-readobj/ELF/bb-addr-map-relocatable.test (+63-44)
  • (modified) llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test (+99-173)
  • (modified) llvm/test/tools/obj2yaml/ELF/bb-addr-map.yaml (+73-143)
  • (modified) llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml (+21-18)
  • (modified) llvm/tools/llvm-objdump/llvm-objdump.cpp (+23-21)
  • (modified) llvm/tools/llvm-readobj/ELFDumper.cpp (+25-18)
  • (modified) llvm/tools/obj2yaml/elf2yaml.cpp (+34-12)
  • (modified) llvm/unittests/Object/ELFObjectFileTest.cpp (+242-164)
  • (modified) llvm/unittests/Object/ELFTypesTest.cpp (+20-18)
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 0acb5ae134ea24..1fc960010d0525 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -88,6 +88,7 @@ ENUM_CODEGENOPT(InlineAsmDialect, InlineAsmDialectKind, 1, IAD_ATT)
 CODEGENOPT(ForbidGuardVariables , 1, 0) ///< Issue errors if C++ guard variables
                                         ///< are required.
 CODEGENOPT(FunctionSections  , 1, 0) ///< Set when -ffunction-sections is enabled.
+CODEGENOPT(BBAddrMap  , 1, 0) ///< Set when -fbasic-block-address-map is enabled.
 CODEGENOPT(InstrumentFunctions , 1, 0) ///< Set when -finstrument-functions is
                                        ///< enabled.
 CODEGENOPT(InstrumentFunctionsAfterInlining , 1, 0) ///< Set when
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 2b93ddf033499c..8a12e41280d27c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3952,6 +3952,10 @@ defm function_sections : BoolFOption<"function-sections",
   PosFlag<SetTrue, [], [ClangOption, CC1Option],
           "Place each function in its own section">,
   NegFlag<SetFalse>>;
+defm basic_block_address_map : BoolFOption<"basic-block-address-map",
+  CodeGenOpts<"BBAddrMap">, DefaultFalse,
+  PosFlag<SetTrue, [], [CC1Option], "Emit the basic block address map section.">,
+  NegFlag<SetFalse>>;
 def fbasic_block_sections_EQ : Joined<["-"], "fbasic-block-sections=">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option, CC1AsOption]>,
   HelpText<"Place each function's basic blocks in unique sections (ELF Only)">,
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index a6142d99f3b688..1baff5b4fa7229 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -374,6 +374,7 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
                               LangOptions::FPModeKind::FPM_FastHonorPragmas);
   Options.ApproxFuncFPMath = LangOpts.ApproxFunc;
 
+  Options.BBAddrMap = CodeGenOpts.BBAddrMap;
   Options.BBSections =
       llvm::StringSwitch<llvm::BasicBlockSection>(CodeGenOpts.BBSections)
           .Case("all", llvm::BasicBlockSection::All)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index acfa119805068d..43882d9bb49854 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5884,6 +5884,17 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     CmdArgs.push_back("-ffunction-sections");
   }
 
+  if (Arg *A = Args.getLastArg(options::OPT_fbasic_block_address_map,
+                               options::OPT_fno_basic_block_address_map)) {
+    if (Triple.isX86() && Triple.isOSBinFormatELF()) {
+      if (A->getOption().matches(options::OPT_fbasic_block_address_map))
+        A->render(Args, CmdArgs);
+    } else {
+      D.Diag(diag::err_drv_unsupported_opt_for_target)
+          << A->getAsString(Args) << TripleStr;
+    }
+  }
+
   if (Arg *A = Args.getLastArg(options::OPT_fbasic_block_sections_EQ)) {
     StringRef Val = A->getValue();
     if (Triple.isX86() && Triple.isOSBinFormatELF()) {
diff --git a/clang/test/Driver/basic-block-address-map.c b/clang/test/Driver/basic-block-address-map.c
new file mode 100644
index 00000000000000..022f972b412d6b
--- /dev/null
+++ b/clang/test/Driver/basic-block-address-map.c
@@ -0,0 +1,8 @@
+// RUN: %clang -### -target x86_64 -fbasic-block-address-map %s -S 2>&1 | FileCheck -check-prefix=CHECK-PRESENT %s
+// CHECK-PRESENT: -fbasic-block-address-map
+
+// RUN: %clang -### -target x86_64 -fno-basic-block-address-map %s -S 2>&1 | FileCheck %s --check-prefix=CHECK-ABSENT
+// CHECK-ABSENT-NOT: -fbasic-block-address-map
+
+// RUN: not %clang -c -target x86_64-apple-darwin10 -fbasic-block-address-map %s -S 2>&1 | FileCheck -check-prefix=CHECK-TRIPLE %s
+// CHECK-TRIPLE: error: unsupported option '-fbasic-block-address-map' for target
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index 56229334f9a44a..ffdb6ca9584cd0 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -187,6 +187,7 @@ struct Config {
   llvm::StringRef cmseOutputLib;
   StringRef zBtiReport = "none";
   StringRef zCetReport = "none";
+  bool ltoBBAddrMap;
   llvm::StringRef ltoBasicBlockSections;
   std::pair<llvm::StringRef, llvm::StringRef> thinLTOObjectSuffixReplace;
   llvm::StringRef thinLTOPrefixReplaceOld;
@@ -249,6 +250,8 @@ struct Config {
   bool ltoPGOWarnMismatch;
   bool ltoDebugPassManager;
   bool ltoEmitAsm;
+  bool ltoNewPassManager;
+  bool ltoPseudoProbeForProfiling;
   bool ltoUniqueBasicBlockSectionNames;
   bool ltoValidateAllVtablesHaveTypeInfos;
   bool ltoWholeProgramVisibility;
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 6bef09eeca015a..9ff84048192e10 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1319,6 +1319,9 @@ static void readConfigs(opt::InputArgList &args) {
   config->ltoObjPath = args.getLastArgValue(OPT_lto_obj_path_eq);
   config->ltoPartitions = args::getInteger(args, OPT_lto_partitions, 1);
   config->ltoSampleProfile = args.getLastArgValue(OPT_lto_sample_profile);
+  config->ltoBBAddrMap =
+      args.hasFlag(OPT_lto_basic_block_address_map,
+                   OPT_no_lto_basic_block_address_map, false);
   config->ltoBasicBlockSections =
       args.getLastArgValue(OPT_lto_basic_block_sections);
   config->ltoUniqueBasicBlockSectionNames =
diff --git a/lld/ELF/LTO.cpp b/lld/ELF/LTO.cpp
index 504c12aac6c569..9fb0b18cd0ea67 100644
--- a/lld/ELF/LTO.cpp
+++ b/lld/ELF/LTO.cpp
@@ -59,6 +59,8 @@ static lto::Config createConfig() {
   c.Options.FunctionSections = true;
   c.Options.DataSections = true;
 
+  c.Options.BBAddrMap = config->ltoBBAddrMap;
+
   // Check if basic block sections must be used.
   // Allowed values for --lto-basic-block-sections are "all", "labels",
   // "<file name specifying basic block ids>", or none.  This is the equivalent
diff --git a/lld/ELF/Options.td b/lld/ELF/Options.td
index c2c9cabc92a4da..c10a73e2d9c36f 100644
--- a/lld/ELF/Options.td
+++ b/lld/ELF/Options.td
@@ -637,6 +637,9 @@ def save_temps_eq: JJ<"save-temps=">, HelpText<"Save select intermediate LTO com
   Values<"resolution,preopt,promote,internalize,import,opt,precodegen,prelink,combinedindex">;
 def lto_basic_block_sections: JJ<"lto-basic-block-sections=">,
   HelpText<"Enable basic block sections for LTO">;
+defm lto_basic_block_address_map: BB<"lto-basic-block-address-map",
+  "Emit basic block address map for LTO",
+  "Do not emit basic block address map for LTO (default)">;
 defm lto_unique_basic_block_section_names: BB<"lto-unique-basic-block-section-names",
     "Give unique names to every basic block section for LTO",
     "Do not give unique names to every basic block section for LTO (default)">;
diff --git a/lld/test/ELF/lto/basic-block-address-map.ll b/lld/test/ELF/lto/basic-block-address-map.ll
new file mode 100644
index 00000000000000..b96e7a8401f851
--- /dev/null
+++ b/lld/test/ELF/lto/basic-block-address-map.ll
@@ -0,0 +1,28 @@
+; REQUIRES: x86
+; RUN: llvm-as %s -o %t.o
+; RUN: ld.lld %t.o -o %t --lto-basic-block-address-map --lto-O0 --save-temps
+; RUN: llvm-readobj --sections %t.lto.o | FileCheck --check-prefix=SECNAMES %s
+
+; SECNAMES: Type: SHT_LLVM_BB_ADDR_MAP
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: nounwind uwtable
+define dso_local void @foo(i32 %b) local_unnamed_addr {
+entry:
+  %tobool.not = icmp eq i32 %b, 0
+  br i1 %tobool.not, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  tail call void @foo(i32 0)
+  br label %if.end
+
+if.end:                                           ; preds = %entry, %if.then
+  ret void
+}
+
+define void @_start() {
+  call void @foo(i32 1)
+  ret void
+}
diff --git a/llvm/include/llvm/CodeGen/CommandFlags.h b/llvm/include/llvm/CodeGen/CommandFlags.h
index 6407dde5bcd6c7..73c9c7ce9a5e1f 100644
--- a/llvm/include/llvm/CodeGen/CommandFlags.h
+++ b/llvm/include/llvm/CodeGen/CommandFlags.h
@@ -156,6 +156,8 @@ struct RegisterCodeGenFlags {
   RegisterCodeGenFlags();
 };
 
+bool getEnableBBAddrMap();
+
 llvm::BasicBlockSection getBBSectionsMode(llvm::TargetOptions &Options);
 
 /// Common utility function tightly tied to the options listed here. Initializes
diff --git a/llvm/include/llvm/Object/ELFTypes.h b/llvm/include/llvm/Object/ELFTypes.h
index d3351a2d1650ed..9c201ac46f7e67 100644
--- a/llvm/include/llvm/Object/ELFTypes.h
+++ b/llvm/include/llvm/Object/ELFTypes.h
@@ -796,6 +796,44 @@ template <class ELFT> struct Elf_Mips_ABIFlags {
 
 // Struct representing the BBAddrMap for one function.
 struct BBAddrMap {
+
+  /// Bitfield of optional features to include in the PGO extended map.
+  struct Features {
+    bool FuncEntryCount : 1;
+    bool BBFreq : 1;
+    bool BrProb : 1;
+    bool MultiBBRange : 1;
+
+    bool hasPGOAnalysis() const { return FuncEntryCount || BBFreq || BrProb; }
+
+    // Encodes to minimum bit width representation.
+    uint8_t encode() const {
+      return (static_cast<uint8_t>(FuncEntryCount) << 0) |
+             (static_cast<uint8_t>(BBFreq) << 1) |
+             (static_cast<uint8_t>(BrProb) << 2) |
+             (static_cast<uint8_t>(MultiBBRange) << 3);
+    }
+
+    // Decodes from minimum bit width representation and validates no
+    // unnecessary bits are used.
+    static Expected<Features> decode(uint8_t Val) {
+      Features Feat{
+          static_cast<bool>(Val & (1 << 0)), static_cast<bool>(Val & (1 << 1)),
+          static_cast<bool>(Val & (1 << 2)), static_cast<bool>(Val & (1 << 3))};
+      if (Feat.encode() != Val)
+        return createStringError(
+            std::error_code(), "invalid encoding for BBAddrMap::Features: 0x%x",
+            Val);
+      return Feat;
+    }
+
+    bool operator==(const Features &Other) const {
+      return std::tie(FuncEntryCount, BBFreq, BrProb, MultiBBRange) ==
+             std::tie(Other.FuncEntryCount, Other.BBFreq, Other.BrProb,
+                      Other.MultiBBRange);
+    }
+  };
+
   // Struct representing the BBAddrMap information for one basic block.
   struct BBEntry {
     struct Metadata {
@@ -839,7 +877,7 @@ struct BBAddrMap {
     };
 
     uint32_t ID;     // Unique ID of this basic block.
-    uint32_t Offset; // Offset of basic block relative to function start.
+    uint32_t Offset; // Offset of basic block relative to the base address.
     uint32_t Size;   // Size of the basic block.
     Metadata MD;     // Metdata for this basic block.
 
@@ -858,59 +896,41 @@ struct BBAddrMap {
     bool hasIndirectBranch() const { return MD.HasIndirectBranch; }
   };
 
-  BBAddrMap(uint64_t Addr, std::vector<BBEntry> 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.
+    std::vector<BBEntry> BBEntries; // Basic block entries for this range.
+
+    // Equality operator for unit testing.
+    bool operator==(const BBRangeEntry &Other) const {
+      return BaseAddress == Other.BaseAddress &&
+             std::equal(BBEntries.begin(), BBEntries.end(),
+                        Other.BBEntries.begin());
+    }
+  };
 
-  // Returns the address of the corresponding function.
-  uint64_t getFunctionAddress() const { return Addr; }
+  // All ranges for this function. The first range always corresponds to the
+  // function entry.
+  std::vector<BBRangeEntry> BBRanges;
 
-  // Returns the basic block entries for this function.
-  const std::vector<BBEntry> &getBBEntries() const { return BBEntries; }
+  // Returns the function address associated with this BBAddrMap, which is
+  // stored as the `BaseAddress` of its first BBRangeEntry. Returns 0 if
+  // BBRanges is empty.
+  uint64_t getFunctionAddress() const {
+    if (BBRanges.empty())
+      return 0;
+    return BBRanges.front().BaseAddress;
+  }
 
   // Equality operator for unit testing.
   bool operator==(const BBAddrMap &Other) const {
-    return Addr == Other.Addr && std::equal(BBEntries.begin(), BBEntries.end(),
-                                            Other.BBEntries.begin());
+    return std::equal(BBRanges.begin(), BBRanges.end(), Other.BBRanges.begin());
   }
-
-  uint64_t Addr;                  // Function address
-  std::vector<BBEntry> BBEntries; // Basic block entries for this function.
 };
 
 /// A feature extension of BBAddrMap that holds information relevant to PGO.
 struct PGOAnalysisMap {
-  /// Bitfield of optional features to include in the PGO extended map.
-  struct Features {
-    bool FuncEntryCount : 1;
-    bool BBFreq : 1;
-    bool BrProb : 1;
-
-    // Encodes to minimum bit width representation.
-    uint8_t encode() const {
-      return (static_cast<uint8_t>(FuncEntryCount) << 0) |
-             (static_cast<uint8_t>(BBFreq) << 1) |
-             (static_cast<uint8_t>(BrProb) << 2);
-    }
-
-    // Decodes from minimum bit width representation and validates no
-    // unnecessary bits are used.
-    static Expected<Features> decode(uint8_t Val) {
-      Features Feat{static_cast<bool>(Val & (1 << 0)),
-                    static_cast<bool>(Val & (1 << 1)),
-                    static_cast<bool>(Val & (1 << 2))};
-      if (Feat.encode() != Val)
-        return createStringError(
-            std::error_code(),
-            "invalid encoding for PGOAnalysisMap::Features: 0x%x", Val);
-      return Feat;
-    }
-
-    bool operator==(const Features &Other) const {
-      return std::tie(FuncEntryCount, BBFreq, BrProb) ==
-             std::tie(Other.FuncEntryCount, Other.BBFreq, Other.BrProb);
-    }
-  };
-
   /// Extra basic block data with fields for block frequency and branch
   /// probability.
   struct PGOBBEntry {
@@ -942,7 +962,7 @@ struct PGOAnalysisMap {
   std::vector<PGOBBEntry> BBEntries; // Extended basic block entries
 
   // Flags to indicate if each PGO related info was enabled in this function
-  Features FeatEnable;
+  BBAddrMap::Features FeatEnable;
 
   bool operator==(const PGOAnalysisMap &Other) const {
     return std::tie(FuncEntryCount, BBEntries, FeatEnable) ==
diff --git a/llvm/include/llvm/ObjectYAML/ELFYAML.h b/llvm/include/llvm/ObjectYAML/ELFYAML.h
index 12b47c271da2cd..8f045d6383623b 100644
--- a/llvm/include/llvm/ObjectYAML/ELFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/ELFYAML.h
@@ -165,9 +165,21 @@ struct BBAddrMapEntry {
   };
   uint8_t Version;
   llvm::yaml::Hex8 Feature;
-  llvm::yaml::Hex64 Address;
-  std::optional<uint64_t> NumBlocks;
-  std::optional<std::vector<BBEntry>> BBEntries;
+
+  struct BBRangeEntry {
+    llvm::yaml::Hex64 BaseAddress;
+    std::optional<uint64_t> NumBlocks;
+    std::optional<std::vector<BBEntry>> BBEntries;
+  };
+
+  std::optional<uint64_t> NumBBRanges;
+  std::optional<std::vector<BBRangeEntry>> BBRanges;
+
+  llvm::yaml::Hex64 getFunctionAddress() const {
+    if (!BBRanges || BBRanges->empty())
+      return 0;
+    return BBRanges->front().BaseAddress;
+  }
 };
 
 struct PGOAnalysisMapEntry {
@@ -751,6 +763,7 @@ bool shouldAllocateFileSpace(ArrayRef<ProgramHeader> Phdrs,
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::StackSizeEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry::BBEntry)
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry::BBRangeEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::PGOAnalysisMapEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::PGOAnalysisMapEntry::PGOBBEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(
@@ -916,11 +929,15 @@ template <> struct MappingTraits<ELFYAML::StackSizeEntry> {
 };
 
 template <> struct MappingTraits<ELFYAML::BBAddrMapEntry> {
-  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry &Rel);
+  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry &E);
+};
+
+template <> struct MappingTraits<ELFYAML::BBAddrMapEntry::BBRangeEntry> {
+  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry::BBRangeEntry &E);
 };
 
 template <> struct MappingTraits<ELFYAML::BBAddrMapEntry::BBEntry> {
-  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry::BBEntry &Rel);
+  static void mapping(IO &IO, ELFYAML::BBAddrMapEntry::BBEntry &E);
 };
 
 template <> struct MappingTraits<ELFYAML::PGOAnalysisMapEntry> {
diff --git a/llvm/include/llvm/Target/TargetOptions.h b/llvm/include/llvm/Target/TargetOptions.h
index 4df897c047a38a..4a39c539e70a94 100644
--- a/llvm/include/llvm/Target/TargetOptions.h
+++ b/llvm/include/llvm/Target/TargetOptions.h
@@ -147,7 +147,7 @@ namespace llvm {
           TrapUnreachable(false), NoTrapAfterNoreturn(false), TLSSize(0),
           EmulatedTLS(false), EnableIPRA(false), EmitStackSizeSection(false),
           EnableMachineOutliner(false), EnableMachineFunctionSplitter(false),
-          SupportsDefaultOutlining(false), EmitAddrsig(false),
+          SupportsDefaultOutlining(false), EmitAddrsig(false), BBAddrMap(false),
           EmitCallSiteInfo(false), SupportsDebugEntryValues(false),
           EnableDebugEntryValues(false), ValueTrackingVariableLocations(false),
           ForceDwarfFrameSection(false), XRayFunctionIndex(true),
@@ -313,6 +313,10 @@ namespace llvm {
     /// Emit address-significance table.
     unsigned EmitAddrsig : 1;
 
+    // Emit the SHT_LLVM_BB_ADDR_MAP section containing basic block address
+    // which can be used to map virtual addresses to machine basic blocks.
+    unsigned BBAddrMap : 1;
+
     /// Emit basic blocks into separate sections.
     BasicBlockSection BBSections = BasicBlockSection::None;
 
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 7df1c82bf357f6..ca9e5684f93fd6 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -141,10 +141,6 @@ static cl::opt<std::string> BasicBlockProfileDump(
              "performed with -basic-block-sections=labels. Enabling this "
              "flag during in-process ThinLTO is not supported."));
 
-// This is a replication of fields of object::PGOAnalysisMap::Features. It
-// should match the order of the fields so that
-// `object::PGOAnalysisMap::Features::decode(PgoAnalysisMapFeatures.getBits())`
-// succeeds.
 enum class PGOMapFeaturesEnum {
   FuncEntryCount,
   BBFreq,
@@ -158,8 +154,9 @@ static cl::bits<PGOMapFeaturesEnum> PgoAnalysisMapFeatures(
                           "Basic Block Frequency"),
                clEnumValN(PGOMapFeaturesEnum::BrProb, "br-prob",
                           "Branch Probability")),
-    cl::desc("Enable extended information within the BBAddrMap that is "
-             "extracted from PGO related analysis."));
+    cl::desc(
+        "Enable extended information within the SHT_LLVM_BB_ADDR_MAP that is "
+        "extracted from PGO related analysis."));
 
 const char DWARFGroupName[] = "dwarf";
 const char DWARFGroupDescription[] = "DWARF Emission";
@@ -1388,6 +1385,14 @@ static uint32_t getBBAddrMapMetadata(const MachineBasicBlock &MBB) {
       .encode();
 }
 
+static llvm::object::BBAddrMap::Features
+getBBAddrMapFeature(const MachineFunction &MF, int NumMBBSectionRanges) {
+  return {PgoAnalysisMapFeatures.isSet(PGOMapFeaturesEnum::FuncEntryCount),
+          PgoAnalysisMapFeatures.isSet(PGOMapFeaturesEnum::BBFreq),
+          PgoAnalysisMapFeatures.isSet(PGOMapFeaturesEnum::BrProb),
+          MF.hasBBSections() && NumMBBSectionRanges > 1};
+}
+
 void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
   MCSection *BBAddrMapSection =
       getObjFileLowering().getBBAddrMapSection(*MF.getSection());
@@ -1401,17 +1406,48 @@ void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
   uint8_t BBAddrMapVersion = OutStreamer->getContext().getBBAddrMapVersion();
   OutStreamer->emitInt8(BBAddrMapVersion);
   OutStreamer->AddComment("feature");
-  auto FeaturesBits = static_cast<uint8_t>(PgoAnalysisMapFeatures.getBits());
-  OutStreamer->emitInt8(FeaturesBits);
-  OutStreamer->AddComment("function address");
-  OutStreamer->emitSymbolValue(FunctionSymbol, getPointerSize());
-  OutStrea...
[truncated]

@rlavaee rlavaee force-pushed the bb-addr-map branch 3 times, most recently from 1ea74dc to ccbe21d Compare January 5, 2024 00:02
Copy link
Member

@red1bluelost red1bluelost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to me so far, just two minor things and the merge conflict.

llvm/lib/Object/ELF.cpp Outdated Show resolved Hide resolved
llvm/lib/Object/ELF.cpp Outdated Show resolved Hide resolved
DictScope D(W, "Function");
W.printHex("At", AM.Addr);
DictScope FD(W, "Function");
if (AM.BBRanges.empty())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there scenarios where BBRanges would be completely empty? If so, what are those scenarios?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. Except for tests. I will add one.

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

llvm/lib/ObjectYAML/ELFEmitter.cpp Outdated Show resolved Hide resolved
// First handle the basic block sections.
auto R1 = handleBBSections(MF);
// Handle basic block address map after basic block sections are finalized.
auto R2 = handleBBAddrMap(MF);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If basic-block-sections=labels and basic-block-address-map were used together, doesn't need to run handleBBAddrMap

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Thanks.

@rlavaee rlavaee force-pushed the bb-addr-map branch 2 times, most recently from 1b5fb57 to 4f30dfa Compare January 18, 2024 23:47
@rlavaee rlavaee requested a review from tmsri January 22, 2024 22:23
@rlavaee rlavaee force-pushed the bb-addr-map branch 2 times, most recently from 49407d2 to a37d8bf Compare January 25, 2024 16:36
@rlavaee
Copy link
Contributor Author

rlavaee commented Jan 25, 2024

@boomanaiden154 I appreciate if you could PTAL at llvm-objdump changes.

@rlavaee
Copy link
Contributor Author

rlavaee commented Jan 25, 2024

@tmsri I appreciate if you could please review the codegen and options part.

@rlavaee rlavaee requested a review from jh7370 January 25, 2024 21:08
// Returns the function address associated with this BBAddrMap, which is
// stored as the `BaseAddress` of its first BBRangeEntry. Returns 0 if
// BBRanges is empty.
uint64_t getFunctionAddress() const {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: how about return std::optional<uint64_t>? makes the user check that the result is actually a value ("0" can be used in subsequent numerical operations, std::nullopt can't)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the return type would change the API. So we would need to sequence this. How about an assertion? BBRanges should not be empty (except for tests).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I now enforce (when decoding) that BB ranges must be non-empty. Thanks for the suggestion.

// 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just nits/minor questions from me, code otherwise LGTM.

Sorry for the delay in review. I didn't realize you explicitly wanted me to review portions of the code.

@rlavaee rlavaee force-pushed the bb-addr-map branch 4 times, most recently from b756359 to 886ecf8 Compare February 1, 2024 19:50
@rlavaee
Copy link
Contributor Author

rlavaee commented Feb 1, 2024

Just nits/minor questions from me, code otherwise LGTM.

Sorry for the delay in review. I didn't realize you explicitly wanted me to review portions of the code.

Thank you for reviewing. The change in llvm-objdump happened after your PR handling PGOAnalysis. So I wanted to make sure it gets on your radar.

…together by decoupling the handling of the two features.

Today `-split-machine-functions` (MFS) and `-fbasic-block-sections={all,list}` cannot be combined with `-basic-block-sections=labels` (the labels option will be ignored).
The inconsistency comes from the way basic block address map -- the underlying mechanism for basic block labels -- encodes basic block addresses (https://lists.llvm.org/pipermail/llvm-dev/2020-July/143512.html).  Specifically, basic block offsets are computed relative to the function begin symbol. This relies on functions being contiguous which is not the case for MFS and basic block section binaries. This means Propeller cannot use binary profiles collected from these binaries, which limits the applicability of Propeller for iterative optimization.

To make the BB address map feature work with BB section binaries, we propose modifying the encoding of the BB address map as follows.

The current encoding emits the address of each function and its number of basic blocks, followed by basic block entries for each basic block.

|  Address of the function                           | //Address//      |   -> 8 bytes (pointer size)
|  Number of basic blocks in this function |//NumBlocks// |   -> ULEB128
|  BB entry llvm#1
|  BB entry llvm#2
|   ...
|  BB entry #//NumBlocks//

To make this work for basic block sections, we treat each basic block section similar to a function, except that basic block sections of the same function must be encapsulated in the same structure so we can map all of them to their single function.

We modify the encoding to first emit the number of basic block sections (BB ranges) in the function. Then we emit the address map of each basic block section section as before: the base address of the section, its number of blocks, and BB entries for its basic block. The first section in the BB address map is always the function entry section.

|  Number of sections for this function   | //NumBBRanges//) |     -> ULEB128

| Section //0// begin address                     |//BaseAddress[0]//  |   -> 8 bytes (pointer size)
| Number of basic blocks in section //0// |//NumBlocks[0]//    |   -> ULEB128
| BB entries for Section //0//
.
.
.
| Section //K// begin address                      |//BaseAddress[K]// |   -> 8 bytes (pointer size)
| Number of basic blocks in section //K//  |//NumBlocks[K]//   |   -> ULEB128
| BB entries for Section //K//

The encoding of basic block entries remains as before with the minor change that each basic block offset is now computed relative to the begin symbol of its containing BB section.

This patch adds a new boolean codegen option `-basic-block-address-map`. Correspondingly, the front-end flag `-fbasic-block-address-map` and LLD flag `--lto-basic-block-address-map` are introduced.
Analogously, we add a new TargetOption field `BBAddrMap`. This means BB address maps are either generated for all functions in the compiling unit, or for none (depending on `TargetOptions::BBAddrMap`).

This patch disables the old `-fbasic-block-sections=labels` value option but does not remove it. A subsequent patch will remove the obsolete option.

We refactor the `BasicBlockSections` pass by separating the BB address map and BB sections handing to their own functions (named `handleBBAddrMap` and `handleBBSections`).  `handleBBSections` renumbers basic blocks and places them in their assigned sections. `handleBBAddrMap` is invoked after `handleBBSections` (if requested) and only renumbers the blocks.

- New tests added:
   # A codgen test `basic-block-labels-with-sections.ll` to exercise the combination of `-basic-block-address-map` with `-basic-block-sections=list`.
   # A driver sanity test for the `-fbasic-block-address-map` option.
   # An LLD test for testing the `--lto-basic-block-address-map` option. This reuses the LLVM IR from `lld/test/ELF/lto/basic-block-sections.ll`.
- Renamed and modified the two existing codegen tests for basic block address map (`basic-block-sections-labels-functions-sections.ll` and `basic-block-sections-labels.ll`)

Differential Revision: https://reviews.llvm.org/D97526
@rlavaee
Copy link
Contributor Author

rlavaee commented Feb 1, 2024

Thank everyone for reviewing. I will probably merge this today. I want to save myself and others the trouble of merge conflicts over 38 files.

@rlavaee rlavaee merged commit acec641 into llvm:main Feb 2, 2024
6 of 7 checks passed
boomanaiden154 added a commit to boomanaiden154/gematria that referenced this pull request Feb 2, 2024
A couple additional cases were recently added into the BB Address Map
section in llvm/llvm-project#74128. This broke
llvm-cm as a couple method names changed. This patch provides the quick
fix to get everything working.

Eventually llvm-cm will need some work to support multiple BBRanges
within a single function.
boomanaiden154 added a commit to boomanaiden154/gematria that referenced this pull request Feb 2, 2024
A couple additional cases were recently added into the BB Address Map
section in llvm/llvm-project#74128. This broke
llvm-cm as a couple method names changed. This patch provides the quick
fix to get everything working.

Eventually llvm-cm will need some work to support multiple BBRanges
within a single function.
boomanaiden154 added a commit to google/gematria that referenced this pull request Feb 3, 2024
A couple additional cases were recently added into the BB Address Map
section in llvm/llvm-project#74128. This broke
llvm-cm as a couple method names changed. This patch provides the quick
fix to get everything working.

Eventually llvm-cm will need some work to support multiple BBRanges
within a single function.
agozillon pushed a commit to agozillon/llvm-project that referenced this pull request Feb 5, 2024
…together by decoupling the handling of the two features. (llvm#74128)

Today `-split-machine-functions` and `-fbasic-block-sections={all,list}`
cannot be combined with `-basic-block-sections=labels` (the labels
option will be ignored).
The inconsistency comes from the way basic block address map -- the
underlying mechanism for basic block labels -- encodes basic block
addresses
(https://lists.llvm.org/pipermail/llvm-dev/2020-July/143512.html).
Specifically, basic block offsets are computed relative to the function
begin symbol. This relies on functions being contiguous which is not the
case for MFS and basic block section binaries. This means Propeller
cannot use binary profiles collected from these binaries, which limits
the applicability of Propeller for iterative optimization.
    
To make the `SHT_LLVM_BB_ADDR_MAP` feature work with basic block section
binaries, we propose modifying the encoding of this section as follows.

First let us review the current encoding which emits the address of each
function and its number of basic blocks, followed by basic block entries
for each basic block.

| | |
|--|--|
| Address of the function | Function Address |
|  Number of basic blocks in this function | NumBlocks |
|  BB entry 1
|  BB entry 2
|   ...
|  BB entry #NumBlocks
    
To make this work for basic block sections, we treat each basic block
section similar to a function, except that basic block sections of the
same function must be encapsulated in the same structure so we can map
all of them to their single function.
    
We modify the encoding to first emit the number of basic block sections
(BB ranges) in the function. Then we emit the address map of each basic
block section section as before: the base address of the section, its
number of blocks, and BB entries for its basic block. The first section
in the BB address map is always the function entry section.
| | |
|--|--|
|  Number of sections for this function   | NumBBRanges |
| Section 1 begin address                     | BaseAddress[1]  |
| Number of basic blocks in section 1 | NumBlocks[1]    |
| BB entries for Section 1
|..................|
| Section #NumBBRanges begin address | BaseAddress[NumBBRanges] |
| Number of basic blocks in section #NumBBRanges |
NumBlocks[NumBBRanges] |
| BB entries for Section #NumBBRanges
    
The encoding of basic block entries remains as before with the minor
change that each basic block offset is now computed relative to the
begin symbol of its containing BB section.
    
This patch adds a new boolean codegen option `-basic-block-address-map`.
Correspondingly, the front-end flag `-fbasic-block-address-map` and LLD
flag `--lto-basic-block-address-map` are introduced.
Analogously, we add a new TargetOption field `BBAddrMap`. This means BB
address maps are either generated for all functions in the compiling
unit, or for none (depending on `TargetOptions::BBAddrMap`).
    
This patch keeps the functionality of the old
`-fbasic-block-sections=labels` option but does not remove it. A
subsequent patch will remove the obsolete option.

We refactor the `BasicBlockSections` pass by separating the BB address
map and BB sections handing to their own functions (named
`handleBBAddrMap` and `handleBBSections`). `handleBBSections` renumbers
basic blocks and places them in their assigned sections.
`handleBBAddrMap` is invoked after `handleBBSections` (if requested) and
only renumbers the blocks.
  - New tests added:
- Two tests basic-block-address-map-with-basic-block-sections.ll and
basic-block-address-map-with-mfs.ll to exercise the combination of
`-basic-block-address-map` with `-basic-block-sections=list` and
'-split-machine-functions`.
- A driver sanity test for the `-fbasic-block-address-map` option
(basic-block-address-map.c).
- An LLD test for testing the `--lto-basic-block-address-map` option.
This reuses the LLVM IR from `lld/test/ELF/lto/basic-block-sections.ll`.
- Renamed and modified the two existing codegen tests for basic block
address map (`basic-block-sections-labels-functions-sections.ll` and
`basic-block-sections-labels.ll`)
- Removed `SHT_LLVM_BB_ADDR_MAP_V0` tests. Full deprecation of
`SHT_LLVM_BB_ADDR_MAP_V0` and `SHT_LLVM_BB_ADDR_MAP` version less than 2
will happen in a separate PR in a few months.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 clang:codegen clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category lld:ELF lld llvm:binary-utilities mc Machine (object) code objectyaml
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants