Unverified Commit e28c393b authored by maksfb's avatar maksfb Committed by GitHub
Browse files

[BOLT] Reduce the number of emitted symbols. NFCI. (#70175) 

We emit a symbol before an instruction for a number of reasons, e.g. for
tracking LocSyms, debug line, or if the instruction has a label
annotation. Currently, we may emit multiple symbols per instruction.

Reuse the same label instead of creating and emitting new ones when
possible. I'm planning to refactor EH labels as well in a separate diff.

Change getLabel() to return a pointer instead of std::optional<> since
an empty label should be treated identically to no label.
parent 565e21b3

bolt/include/bolt/Core/MCPlusBuilder.h

+1 −1
Original line number Diff line number Diff line
@@ -1181,7 +1181,7 @@ public: 
  bool clearOffset(MCInst &Inst);



  /// Return the label of \p Inst, if available.

  std::optional<MCSymbol *> getLabel(const MCInst &Inst) const;

  MCSymbol *getLabel(const MCInst &Inst) const;



  /// Set the label of \p Inst. This label will be emitted right before \p Inst

  /// is emitted to MCStreamer.

bolt/lib/Core/BinaryContext.cpp

+2 −2
Original line number Diff line number Diff line
@@ -1901,8 +1901,8 @@ void BinaryContext::printInstruction(raw_ostream &OS, const MCInst &Instruction, 
  }

  if (std::optional<uint32_t> Offset = MIB->getOffset(Instruction))

    OS << " # Offset: " << *Offset;

  if (auto Label = MIB->getLabel(Instruction))

    OS << " # Label: " << **Label;

  if (MCSymbol *Label = MIB->getLabel(Instruction))

    OS << " # Label: " << *Label;



  MIB->printAnnotations(Instruction, OS);

bolt/lib/Core/BinaryEmitter.cpp

+34 −20
Original line number Diff line number Diff line
@@ -161,9 +161,17 @@ private: 
  /// \p FirstInstr indicates if \p NewLoc represents the first instruction

  /// in a sequence, such as a function fragment.

  ///

  /// If \p NewLoc location matches \p PrevLoc, no new line number entry will be

  /// created and the function will return \p PrevLoc while \p InstrLabel will

  /// be ignored. Otherwise, the caller should use \p InstrLabel to mark the

  /// corresponding instruction by emitting \p InstrLabel before it.

  /// If \p InstrLabel is set by the caller, its value will be used with \p

  /// \p NewLoc. If it was nullptr on entry, it will be populated with a pointer

  /// to a new temp symbol used with \p NewLoc.

  ///

  /// Return new current location which is either \p NewLoc or \p PrevLoc.

  SMLoc emitLineInfo(const BinaryFunction &BF, SMLoc NewLoc, SMLoc PrevLoc,

                     bool FirstInstr);

                     bool FirstInstr, MCSymbol *&InstrLabel);



  /// Use \p FunctionEndSymbol to mark the end of the line info sequence.

  /// Note that it does not automatically result in the insertion of the EOS
@@ -483,23 +491,28 @@ void BinaryEmitter::emitFunctionBody(BinaryFunction &BF, FunctionFragment &FF, 
        // are relaxable, we should be safe.

      }



      if (!EmitCodeOnly && opts::UpdateDebugSections && BF.getDWARFUnit()) {

        LastLocSeen = emitLineInfo(BF, Instr.getLoc(), LastLocSeen, FirstInstr);

      if (!EmitCodeOnly) {

        // A symbol to be emitted before the instruction to mark its location.

        MCSymbol *InstrLabel = BC.MIB->getLabel(Instr);



        if (opts::UpdateDebugSections && BF.getDWARFUnit()) {

          LastLocSeen = emitLineInfo(BF, Instr.getLoc(), LastLocSeen,

                                     FirstInstr, InstrLabel);

          FirstInstr = false;

        }



        // Prepare to tag this location with a label if we need to keep track of

        // the location of calls/returns for BOLT address translation maps

      if (!EmitCodeOnly && BF.requiresAddressTranslation() &&

          BC.MIB->getOffset(Instr)) {

        if (BF.requiresAddressTranslation() && BC.MIB->getOffset(Instr)) {

          const uint32_t Offset = *BC.MIB->getOffset(Instr);

        MCSymbol *LocSym = BC.Ctx->createTempSymbol();

        Streamer.emitLabel(LocSym);

        BB->getLocSyms().emplace_back(Offset, LocSym);

          if (!InstrLabel)

            InstrLabel = BC.Ctx->createTempSymbol();

          BB->getLocSyms().emplace_back(Offset, InstrLabel);

        }



      if (auto Label = BC.MIB->getLabel(Instr))

        Streamer.emitLabel(*Label);

        if (InstrLabel)

          Streamer.emitLabel(InstrLabel);

      }



      Streamer.emitInstruction(Instr, *BC.STI);

      LastIsPrefix = BC.MIB->isPrefix(Instr);
@@ -661,7 +674,8 @@ void BinaryEmitter::emitConstantIslands(BinaryFunction &BF, bool EmitColdPart, 
}



SMLoc BinaryEmitter::emitLineInfo(const BinaryFunction &BF, SMLoc NewLoc,

                                  SMLoc PrevLoc, bool FirstInstr) {

                                  SMLoc PrevLoc, bool FirstInstr,

                                  MCSymbol *&InstrLabel) {

  DWARFUnit *FunctionCU = BF.getDWARFUnit();

  const DWARFDebugLine::LineTable *FunctionLineTable = BF.getDWARFLineTable();

  assert(FunctionCU && "cannot emit line info for function without CU");
@@ -711,12 +725,12 @@ SMLoc BinaryEmitter::emitLineInfo(const BinaryFunction &BF, SMLoc NewLoc, 
  const MCDwarfLoc &DwarfLoc = BC.Ctx->getCurrentDwarfLoc();

  BC.Ctx->clearDwarfLocSeen();



  MCSymbol *LineSym = BC.Ctx->createTempSymbol();

  Streamer.emitLabel(LineSym);

  if (!InstrLabel)

    InstrLabel = BC.Ctx->createTempSymbol();



  BC.getDwarfLineTable(FunctionUnitIndex)

      .getMCLineSections()

      .addLineEntry(MCDwarfLineEntry(LineSym, DwarfLoc),

      .addLineEntry(MCDwarfLineEntry(InstrLabel, DwarfLoc),

                    Streamer.getCurrentSectionOnly());



  return NewLoc;

bolt/lib/Core/MCPlusBuilder.cpp

+2 −2
Original line number Diff line number Diff line
@@ -268,10 +268,10 @@ bool MCPlusBuilder::clearOffset(MCInst &Inst) { 
  return true;

}



std::optional<MCSymbol *> MCPlusBuilder::getLabel(const MCInst &Inst) const {

MCSymbol *MCPlusBuilder::getLabel(const MCInst &Inst) const {

  if (auto Label = tryGetAnnotationAs<MCSymbol *>(Inst, MCAnnotation::kLabel))

    return *Label;

  return std::nullopt;

  return nullptr;

}



bool MCPlusBuilder::setLabel(MCInst &Inst, MCSymbol *Label,

bolt/lib/Passes/BinaryPasses.cpp

+4 −4
Original line number Diff line number Diff line
@@ -610,8 +610,8 @@ void LowerAnnotations::runOnFunctions(BinaryContext &BC) { 
          if (BF.requiresAddressTranslation() && BC.MIB->getOffset(*II))

            PreservedOffsetAnnotations.emplace_back(&(*II),

                                                    *BC.MIB->getOffset(*II));

          if (auto Label = BC.MIB->getLabel(*II))

            PreservedLabelAnnotations.emplace_back(&*II, *Label);

          if (MCSymbol *Label = BC.MIB->getLabel(*II))

            PreservedLabelAnnotations.emplace_back(&*II, Label);

          BC.MIB->stripAnnotations(*II);

        }

      }
@@ -620,8 +620,8 @@ void LowerAnnotations::runOnFunctions(BinaryContext &BC) { 
  for (BinaryFunction *BF : BC.getInjectedBinaryFunctions())

    for (BinaryBasicBlock &BB : *BF)

      for (MCInst &Instruction : BB) {

        if (auto Label = BC.MIB->getLabel(Instruction))

          PreservedLabelAnnotations.emplace_back(&Instruction, *Label);

        if (MCSymbol *Label = BC.MIB->getLabel(Instruction))

          PreservedLabelAnnotations.emplace_back(&Instruction, Label);

        BC.MIB->stripAnnotations(Instruction);

      }