[SHT_LLVM_BB_ADDR_MAP] Change the callsite feature to emit end of callsites. (#155041)

This PR simply moves the callsite anchors from the beginning of
callsites to their end.

Emitting the end of callsites is more sensible as it allows breaking the
basic block into subblocks which end with control transfer instructions.

Author: rlavaee

llvm/docs/Extensions.rst

@@ -410,8 +410,8 @@ two years old). Each function entry starts with a version byte which specifies
 the encoding version to use. This is followed by a feature byte which specifies
 the features specific to this particular entry. The function base address is
 stored as a full address. Other addresses in the entry (block begin and end
-addresses and callsite addresses) are stored in a running-offset fashion, as
-offsets relative to prior addresses.
+addresses and callsite end addresses) are stored in a running-offset fashion,
+as offsets relative to prior addresses.
 
 The following versioning schemes are currently supported (newer versions support
 features of the older versions).
@@ -438,8 +438,8 @@ Example:
    .byte     1                            # BB_1 ID
    .uleb128  .LBB0_1-.LBB_END0_0          # BB_1 offset relative to the end of last block (BB_0).
    .byte     2                            # number of callsites in this block
-   .uleb128  .LBB0_1_CS0-.LBB0_1          # offset of callsite relative to the previous offset (.LBB0_1)
-   .uleb128  .LBB0_1_CS1-.LBB0_1_CS0      # offset of callsite relative to the previous offset (.LBB0_1_CS0)
+   .uleb128  .LBB0_1_CS0-.LBB0_1          # offset of callsite end relative to the previous offset (.LBB0_1)
+   .uleb128  .LBB0_1_CS1-.LBB0_1_CS0      # offset of callsite end relative to the previous offset (.LBB0_1_CS0)
    .uleb128  .LBB_END0_1-.LBB0_1_CS1      # BB_1 size offset (Offset of the block end relative to the previous offset).
    .byte     y                            # BB_1 metadata
 

llvm/include/llvm/CodeGen/AsmPrinter.h

@@ -135,12 +135,12 @@ class LLVM_ABI AsmPrinter : public MachineFunctionPass {
   /// default, this is equal to CurrentFnSym.
   MCSymbol *CurrentFnSymForSize = nullptr;
 
-  /// Vector of symbols marking the position of callsites in the current
+  /// Vector of symbols marking the end of the callsites in the current
   /// function, keyed by their containing basic block.
   /// The callsite symbols of each block are stored in the order they appear
   /// in that block.
   DenseMap<const MachineBasicBlock *, SmallVector<MCSymbol *, 1>>
-      CurrentFnCallsiteSymbols;
+      CurrentFnCallsiteEndSymbols;
 
   /// Provides the profile information for constants.
   const StaticDataProfileInfo *SDPI = nullptr;
@@ -332,9 +332,9 @@ class LLVM_ABI AsmPrinter : public MachineFunctionPass {
   /// to emit them as well, return the whole set.
   ArrayRef<MCSymbol *> getAddrLabelSymbolToEmit(const BasicBlock *BB);
 
-  /// Creates a new symbol to be used for the beginning of a callsite at the
-  /// specified basic block.
-  MCSymbol *createCallsiteSymbol(const MachineBasicBlock &MBB);
+  /// Creates a new symbol to be used for the end of a callsite at the specified
+  /// basic block.
+  MCSymbol *createCallsiteEndSymbol(const MachineBasicBlock &MBB);
 
   /// If the specified function has had any references to address-taken blocks
   /// generated, but the block got deleted, return the symbol now so we can

llvm/include/llvm/Object/ELFTypes.h

@@ -832,7 +832,7 @@ struct BBAddrMap {
     bool BrProb : 1;
     bool MultiBBRange : 1;
     bool OmitBBEntries : 1;
-    bool CallsiteOffsets : 1;
+    bool CallsiteEndOffsets : 1;
 
     bool hasPGOAnalysis() const { return FuncEntryCount || BBFreq || BrProb; }
 
@@ -845,7 +845,7 @@ struct BBAddrMap {
              (static_cast<uint8_t>(BrProb) << 2) |
              (static_cast<uint8_t>(MultiBBRange) << 3) |
              (static_cast<uint8_t>(OmitBBEntries) << 4) |
-             (static_cast<uint8_t>(CallsiteOffsets) << 5);
+             (static_cast<uint8_t>(CallsiteEndOffsets) << 5);
     }
 
     // Decodes from minimum bit width representation and validates no
@@ -864,10 +864,10 @@ struct BBAddrMap {
 
     bool operator==(const Features &Other) const {
       return std::tie(FuncEntryCount, BBFreq, BrProb, MultiBBRange,
-                      OmitBBEntries, CallsiteOffsets) ==
+                      OmitBBEntries, CallsiteEndOffsets) ==
              std::tie(Other.FuncEntryCount, Other.BBFreq, Other.BrProb,
                       Other.MultiBBRange, Other.OmitBBEntries,
-                      Other.CallsiteOffsets);
+                      Other.CallsiteEndOffsets);
     }
   };
 
@@ -918,20 +918,19 @@ struct BBAddrMap {
     uint32_t Size = 0;   // Size of the basic block.
     Metadata MD = {false, false, false, false,
                    false}; // Metdata for this basic block.
-    // Offsets of callsites (beginning of call instructions), relative to the
-    // basic block start.
-    SmallVector<uint32_t, 1> CallsiteOffsets;
+    // Offsets of end of call instructions, relative to the basic block start.
+    SmallVector<uint32_t, 1> CallsiteEndOffsets;
 
     BBEntry(uint32_t ID, uint32_t Offset, uint32_t Size, Metadata MD,
-            SmallVector<uint32_t, 1> CallsiteOffsets)
+            SmallVector<uint32_t, 1> CallsiteEndOffsets)
         : ID(ID), Offset(Offset), Size(Size), MD(MD),
-          CallsiteOffsets(std::move(CallsiteOffsets)) {}
+          CallsiteEndOffsets(std::move(CallsiteEndOffsets)) {}
 
     UniqueBBID getID() const { return {ID, 0}; }
 
     bool operator==(const BBEntry &Other) const {
       return ID == Other.ID && Offset == Other.Offset && Size == Other.Size &&
-             MD == Other.MD && CallsiteOffsets == Other.CallsiteOffsets;
+             MD == Other.MD && CallsiteEndOffsets == Other.CallsiteEndOffsets;
     }
 
     bool hasReturn() const { return MD.HasReturn; }

llvm/include/llvm/ObjectYAML/ELFYAML.h

@@ -162,7 +162,7 @@ struct BBAddrMapEntry {
     llvm::yaml::Hex64 AddressOffset;
     llvm::yaml::Hex64 Size;
     llvm::yaml::Hex64 Metadata;
-    std::optional<std::vector<llvm::yaml::Hex64>> CallsiteOffsets;
+    std::optional<std::vector<llvm::yaml::Hex64>> CallsiteEndOffsets;
   };
   uint8_t Version;
   llvm::yaml::Hex8 Feature;
@@ -183,14 +183,14 @@ struct BBAddrMapEntry {
   }
 
   // Returns if any BB entries have non-empty callsite offsets.
-  bool hasAnyCallsiteOffsets() const {
+  bool hasAnyCallsiteEndOffsets() const {
     if (!BBRanges)
       return false;
     for (const ELFYAML::BBAddrMapEntry::BBRangeEntry &BBR : *BBRanges) {
       if (!BBR.BBEntries)
         continue;
       for (const ELFYAML::BBAddrMapEntry::BBEntry &BBE : *BBR.BBEntries)
-        if (BBE.CallsiteOffsets && !BBE.CallsiteOffsets->empty())
+        if (BBE.CallsiteEndOffsets && !BBE.CallsiteEndOffsets->empty())
           return true;
     }
     return false;

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

@@ -1432,7 +1432,7 @@ void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
   MCSection *BBAddrMapSection =
       getObjFileLowering().getBBAddrMapSection(*MF.getSection());
   assert(BBAddrMapSection && ".llvm_bb_addr_map section is not initialized.");
-  bool HasCalls = !CurrentFnCallsiteSymbols.empty();
+  bool HasCalls = !CurrentFnCallsiteEndSymbols.empty();
 
   const MCSymbol *FunctionSymbol = getFunctionBegin();
 
@@ -1497,13 +1497,13 @@ void AsmPrinter::emitBBAddrMapSection(const MachineFunction &MF) {
       emitLabelDifferenceAsULEB128(MBBSymbol, PrevMBBEndSymbol);
       const MCSymbol *CurrentLabel = MBBSymbol;
       if (HasCalls) {
-        auto CallsiteSymbols = CurrentFnCallsiteSymbols.lookup(&MBB);
+        auto CallsiteEndSymbols = CurrentFnCallsiteEndSymbols.lookup(&MBB);
         OutStreamer->AddComment("number of callsites");
-        OutStreamer->emitULEB128IntValue(CallsiteSymbols.size());
-        for (const MCSymbol *CallsiteSymbol : CallsiteSymbols) {
+        OutStreamer->emitULEB128IntValue(CallsiteEndSymbols.size());
+        for (const MCSymbol *CallsiteEndSymbol : CallsiteEndSymbols) {
           // Emit the callsite offset.
-          emitLabelDifferenceAsULEB128(CallsiteSymbol, CurrentLabel);
-          CurrentLabel = CallsiteSymbol;
+          emitLabelDifferenceAsULEB128(CallsiteEndSymbol, CurrentLabel);
+          CurrentLabel = CallsiteEndSymbol;
         }
       }
       // Emit the offset to the end of the block, which can be used to compute
@@ -1941,8 +1941,6 @@ void AsmPrinter::emitFunctionBody() {
           !MI.isDebugInstr()) {
         HasAnyRealCode = true;
       }
-      if (MI.isCall() && MF->getTarget().Options.BBAddrMap)
-        OutStreamer->emitLabel(createCallsiteSymbol(MBB));
 
       // If there is a pre-instruction symbol, emit a label for it here.
       if (MCSymbol *S = MI.getPreInstrSymbol())
@@ -2064,6 +2062,9 @@ void AsmPrinter::emitFunctionBody() {
         break;
       }
 
+      if (MI.isCall() && MF->getTarget().Options.BBAddrMap)
+        OutStreamer->emitLabel(createCallsiteEndSymbol(MBB));
+
       if (TM.Options.EmitCallGraphSection && MI.isCall())
         emitIndirectCalleeLabels(FuncInfo, CallSitesInfoMap, MI);
 
@@ -2897,11 +2898,11 @@ MCSymbol *AsmPrinter::getMBBExceptionSym(const MachineBasicBlock &MBB) {
   return Res.first->second;
 }
 
-MCSymbol *AsmPrinter::createCallsiteSymbol(const MachineBasicBlock &MBB) {
+MCSymbol *AsmPrinter::createCallsiteEndSymbol(const MachineBasicBlock &MBB) {
   MCContext &Ctx = MF->getContext();
   MCSymbol *Sym = Ctx.createTempSymbol("BB" + Twine(MF->getFunctionNumber()) +
                                        "_" + Twine(MBB.getNumber()) + "_CS");
-  CurrentFnCallsiteSymbols[&MBB].push_back(Sym);
+  CurrentFnCallsiteEndSymbols[&MBB].push_back(Sym);
   return Sym;
 }
 
@@ -2939,7 +2940,7 @@ void AsmPrinter::SetupMachineFunction(MachineFunction &MF) {
   CurrentFnBegin = nullptr;
   CurrentFnBeginLocal = nullptr;
   CurrentSectionBeginSym = nullptr;
-  CurrentFnCallsiteSymbols.clear();
+  CurrentFnCallsiteEndSymbols.clear();
   MBBSectionRanges.clear();
   MBBSectionExceptionSyms.clear();
   bool NeedsLocalForSize = MAI->needsLocalForSize();

llvm/lib/Object/ELF.cpp

@@ -847,7 +847,7 @@ decodeBBAddrMapImpl(const ELFFile<ELFT> &EF,
     if (!FeatEnableOrErr)
       return FeatEnableOrErr.takeError();
     FeatEnable = *FeatEnableOrErr;
-    if (FeatEnable.CallsiteOffsets && Version < 3)
+    if (FeatEnable.CallsiteEndOffsets && Version < 3)
       return createError("version should be >= 3 for SHT_LLVM_BB_ADDR_MAP when "
                          "callsite offsets feature is enabled: version = " +
                          Twine(static_cast<int>(Version)) +
@@ -890,22 +890,22 @@ decodeBBAddrMapImpl(const ELFFile<ELFT> &EF,
           uint32_t ID = readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr);
           uint32_t Offset = readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr);
           // Read the callsite offsets.
-          uint32_t LastCallsiteOffset = 0;
-          SmallVector<uint32_t, 1> CallsiteOffsets;
-          if (FeatEnable.CallsiteOffsets) {
+          uint32_t LastCallsiteEndOffset = 0;
+          SmallVector<uint32_t, 1> CallsiteEndOffsets;
+          if (FeatEnable.CallsiteEndOffsets) {
             uint32_t NumCallsites =
                 readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr);
-            CallsiteOffsets.reserve(NumCallsites);
+            CallsiteEndOffsets.reserve(NumCallsites);
             for (uint32_t CallsiteIndex = 0;
                  !ULEBSizeErr && Cur && (CallsiteIndex < NumCallsites);
                  ++CallsiteIndex) {
-              LastCallsiteOffset +=
+              LastCallsiteEndOffset +=
                   readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr);
-              CallsiteOffsets.push_back(LastCallsiteOffset);
+              CallsiteEndOffsets.push_back(LastCallsiteEndOffset);
             }
           }
           uint32_t Size = readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr) +
-                          LastCallsiteOffset;
+                          LastCallsiteEndOffset;
           uint32_t MD = readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr);
           Expected<BBAddrMap::BBEntry::Metadata> MetadataOrErr =
               BBAddrMap::BBEntry::Metadata::decode(MD);
@@ -914,7 +914,7 @@ decodeBBAddrMapImpl(const ELFFile<ELFT> &EF,
             break;
           }
           BBEntries.push_back({ID, Offset + PrevBBEndOffset, Size,
-                               *MetadataOrErr, CallsiteOffsets});
+                               *MetadataOrErr, CallsiteEndOffsets});
           PrevBBEndOffset += Offset + Size;
         }
         TotalNumBlocks += BBEntries.size();

llvm/lib/ObjectYAML/ELFEmitter.cpp

@@ -1487,8 +1487,8 @@ void ELFState<ELFT>::writeSectionContent(
     if (!E.BBRanges)
       continue;
     uint64_t TotalNumBlocks = 0;
-    bool EmitCallsiteOffsets =
-        FeatureOrErr->CallsiteOffsets || E.hasAnyCallsiteOffsets();
+    bool EmitCallsiteEndOffsets =
+        FeatureOrErr->CallsiteEndOffsets || E.hasAnyCallsiteEndOffsets();
     for (const ELFYAML::BBAddrMapEntry::BBRangeEntry &BBR : *E.BBRanges) {
       // Write the base address of the range.
       CBA.write<uintX_t>(BBR.BaseAddress, ELFT::Endianness);
@@ -1506,12 +1506,12 @@ void ELFState<ELFT>::writeSectionContent(
         if (Section.Type == llvm::ELF::SHT_LLVM_BB_ADDR_MAP && E.Version > 1)
           SHeader.sh_size += CBA.writeULEB128(BBE.ID);
         SHeader.sh_size += CBA.writeULEB128(BBE.AddressOffset);
-        if (EmitCallsiteOffsets) {
-          size_t NumCallsiteOffsets =
-              BBE.CallsiteOffsets ? BBE.CallsiteOffsets->size() : 0;
-          SHeader.sh_size += CBA.writeULEB128(NumCallsiteOffsets);
-          if (BBE.CallsiteOffsets) {
-            for (uint32_t Offset : *BBE.CallsiteOffsets)
+        if (EmitCallsiteEndOffsets) {
+          size_t NumCallsiteEndOffsets =
+              BBE.CallsiteEndOffsets ? BBE.CallsiteEndOffsets->size() : 0;
+          SHeader.sh_size += CBA.writeULEB128(NumCallsiteEndOffsets);
+          if (BBE.CallsiteEndOffsets) {
+            for (uint32_t Offset : *BBE.CallsiteEndOffsets)
               SHeader.sh_size += CBA.writeULEB128(Offset);
           }
         }

llvm/lib/ObjectYAML/ELFYAML.cpp

@@ -1884,7 +1884,7 @@ void MappingTraits<ELFYAML::BBAddrMapEntry::BBEntry>::mapping(
   IO.mapRequired("AddressOffset", E.AddressOffset);
   IO.mapRequired("Size", E.Size);
   IO.mapRequired("Metadata", E.Metadata);
-  IO.mapOptional("CallsiteOffsets", E.CallsiteOffsets);
+  IO.mapOptional("CallsiteEndOffsets", E.CallsiteEndOffsets);
 }
 
 void MappingTraits<ELFYAML::PGOAnalysisMapEntry>::mapping(

llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test

@@ -49,7 +49,7 @@
 # CHECK-NEXT:           {
 # CHECK-NEXT:             ID: 2
 # CHECK-NEXT:             Offset: 0x3
-# CHECK-NEXT:             Callsite Offsets: [1, 3]
+# CHECK-NEXT:             Callsite End Offsets: [1, 3]
 # CHECK-NEXT:             Size: 0x7
 # CHECK-NEXT:             HasReturn: Yes
 # CHECK-NEXT:             HasTailCall: No
@@ -159,7 +159,7 @@ Sections:
                 AddressOffset: 0x3
                 Size:          0x4
                 Metadata:      0x15
-                CallsiteOffsets: [ 0x1 , 0x2 ]
+                CallsiteEndOffsets: [ 0x1 , 0x2 ]
       - Version: 2
         BBRanges:
           - BaseAddress: 0x22222

llvm/test/tools/obj2yaml/ELF/bb-addr-map.yaml

@@ -39,7 +39,7 @@
 # VALID-NEXT:                 AddressOffset:   0xA
 # VALID-NEXT:                 Size:            0xB
 # VALID-NEXT:                 Metadata:        0xC
-# VALID-NEXT:                 CallsiteOffsets: [ 0x1, 0x2 ]
+# VALID-NEXT:                 CallsiteEndOffsets: [ 0x1, 0x2 ]
 
 --- !ELF
 FileHeader:
@@ -79,7 +79,7 @@ Sections:
                AddressOffset:   0xA
                Size:            0xB
                Metadata:        0xC
-               CallsiteOffsets: [ 0x1, 0x2 ]
+               CallsiteEndOffsets: [ 0x1, 0x2 ]
 
 ## Check obj2yaml can dump empty .llvm_bb_addr_map sections.