[MC] Move MCAssembler::DataRegions to MachObjectWriter

and make some cleanup.

Author: MaskRay

llvm/include/llvm/MC/MCAssembler.h

@@ -53,16 +53,6 @@ class MCObjectWriter;
 class MCSection;
 class MCValue;
 
-// FIXME: Ditto this. Purely so the Streamer and the ObjectWriter can talk
-// to one another.
-struct DataRegionData {
-  // This enum should be kept in sync w/ the mach-o definition in
-  // llvm/Object/MachOFormat.h.
-  enum KindTy { Data = 1, JumpTable8, JumpTable16, JumpTable32 } Kind;
-  MCSymbol *Start;
-  MCSymbol *End;
-};
-
 class MCAssembler {
 public:
   using SectionListType = std::vector<MCSection *>;
@@ -78,10 +68,6 @@ class MCAssembler {
   using symbol_range = iterator_range<symbol_iterator>;
   using const_symbol_range = iterator_range<const_symbol_iterator>;
 
-  using const_data_region_iterator =
-      std::vector<DataRegionData>::const_iterator;
-  using data_region_iterator = std::vector<DataRegionData>::iterator;
-
   /// MachO specific deployment target version info.
   // A Major version of 0 indicates that no version information was supplied
   // and so the corresponding load command should not be emitted.
@@ -114,8 +100,6 @@ class MCAssembler {
 
   SymbolDataListType Symbols;
 
-  std::vector<DataRegionData> DataRegions;
-
   /// The list of linker options to propagate into the object file.
   std::vector<std::vector<std::string>> LinkerOptions;
 
@@ -384,31 +368,6 @@ class MCAssembler {
     return LinkerOptions;
   }
 
-  /// @}
-  /// \name Data Region List Access
-  /// @{
-
-  // FIXME: This is a total hack, this should not be here. Once things are
-  // factored so that the streamer has direct access to the .o writer, it can
-  // disappear.
-  std::vector<DataRegionData> &getDataRegions() { return DataRegions; }
-
-  data_region_iterator data_region_begin() { return DataRegions.begin(); }
-  const_data_region_iterator data_region_begin() const {
-    return DataRegions.begin();
-  }
-
-  data_region_iterator data_region_end() { return DataRegions.end(); }
-  const_data_region_iterator data_region_end() const {
-    return DataRegions.end();
-  }
-
-  size_t data_region_size() const { return DataRegions.size(); }
-
-  /// @}
-  /// \name Data Region List Access
-  /// @{
-
   // FIXME: This is a total hack, this should not be here. Once things are
   // factored so that the streamer has direct access to the .o writer, it can
   // disappear.

llvm/include/llvm/MC/MCMachObjectWriter.h

@@ -81,6 +81,14 @@ class MCMachObjectTargetWriter : public MCObjectTargetWriter {
 };
 
 class MachObjectWriter : public MCObjectWriter {
+public:
+  struct DataRegionData {
+    MachO::DataRegionType Kind;
+    MCSymbol *Start;
+    MCSymbol *End;
+  };
+
+private:
   /// Helper struct for containing some precomputed information on symbols.
   struct MachSymbolData {
     const MCSymbol *Symbol;
@@ -113,6 +121,8 @@ class MachObjectWriter : public MCObjectWriter {
   std::vector<IndirectSymbolData> IndirectSymbols;
   DenseMap<const MCSection *, unsigned> IndirectSymBase;
 
+  std::vector<DataRegionData> DataRegions;
+
   SectionAddrMap SectionAddress;
 
   // List of sections in layout order. Virtual sections are after non-virtual
@@ -162,6 +172,7 @@ class MachObjectWriter : public MCObjectWriter {
   std::vector<IndirectSymbolData> &getIndirectSymbols() {
     return IndirectSymbols;
   }
+  std::vector<DataRegionData> &getDataRegions() { return DataRegions; }
   const llvm::SmallVectorImpl<MCSection *> &getSectionOrder() const {
     return SectionOrder;
   }

llvm/lib/MC/MCAssembler.cpp

@@ -97,7 +97,6 @@ void MCAssembler::reset() {
   IncrementalLinkerCompatible = false;
   Sections.clear();
   Symbols.clear();
-  DataRegions.clear();
   LinkerOptions.clear();
   FileNames.clear();
   ThumbFuncs.clear();

llvm/lib/MC/MCMachOStreamer.cpp

@@ -10,6 +10,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/BinaryFormat/MachO.h"
 #include "llvm/MC/MCAsmBackend.h"
 #include "llvm/MC/MCAssembler.h"
 #include "llvm/MC/MCCodeEmitter.h"
@@ -63,7 +64,7 @@ class MCMachOStreamer : public MCObjectStreamer {
 
   void emitInstToData(const MCInst &Inst, const MCSubtargetInfo &STI) override;
 
-  void emitDataRegion(DataRegionData::KindTy Kind);
+  void emitDataRegion(MachO::DataRegionType Kind);
   void emitDataRegionEnd();
 
 public:
@@ -83,6 +84,10 @@ class MCMachOStreamer : public MCObjectStreamer {
     MCObjectStreamer::reset();
   }
 
+  MachObjectWriter &getWriter() {
+    return static_cast<MachObjectWriter &>(getAssembler().getWriter());
+  }
+
   /// @name MCStreamer Interface
   /// @{
 
@@ -227,20 +232,18 @@ void MCMachOStreamer::emitAssignment(MCSymbol *Symbol, const MCExpr *Value) {
   MCObjectStreamer::emitAssignment(Symbol, Value);
 }
 
-void MCMachOStreamer::emitDataRegion(DataRegionData::KindTy Kind) {
+void MCMachOStreamer::emitDataRegion(MachO::DataRegionType Kind) {
   // Create a temporary label to mark the start of the data region.
   MCSymbol *Start = getContext().createTempSymbol();
   emitLabel(Start);
   // Record the region for the object writer to use.
-  DataRegionData Data = { Kind, Start, nullptr };
-  std::vector<DataRegionData> &Regions = getAssembler().getDataRegions();
-  Regions.push_back(Data);
+  getWriter().getDataRegions().push_back({Kind, Start, nullptr});
 }
 
 void MCMachOStreamer::emitDataRegionEnd() {
-  std::vector<DataRegionData> &Regions = getAssembler().getDataRegions();
+  auto &Regions = getWriter().getDataRegions();
   assert(!Regions.empty() && "Mismatched .end_data_region!");
-  DataRegionData &Data = Regions.back();
+  auto &Data = Regions.back();
   assert(!Data.End && "Mismatched .end_data_region!");
   // Create a temporary label to mark the end of the data region.
   Data.End = getContext().createTempSymbol();
@@ -269,16 +272,16 @@ void MCMachOStreamer::emitLinkerOptions(ArrayRef<std::string> Options) {
 void MCMachOStreamer::emitDataRegion(MCDataRegionType Kind) {
   switch (Kind) {
   case MCDR_DataRegion:
-    emitDataRegion(DataRegionData::Data);
+    emitDataRegion(MachO::DataRegionType::DICE_KIND_DATA);
     return;
   case MCDR_DataRegionJT8:
-    emitDataRegion(DataRegionData::JumpTable8);
+    emitDataRegion(MachO::DataRegionType::DICE_KIND_JUMP_TABLE8);
     return;
   case MCDR_DataRegionJT16:
-    emitDataRegion(DataRegionData::JumpTable16);
+    emitDataRegion(MachO::DataRegionType::DICE_KIND_JUMP_TABLE16);
     return;
   case MCDR_DataRegionJT32:
-    emitDataRegion(DataRegionData::JumpTable32);
+    emitDataRegion(MachO::DataRegionType::DICE_KIND_JUMP_TABLE32);
     return;
   case MCDR_DataRegionEnd:
     emitDataRegionEnd();
@@ -322,9 +325,8 @@ bool MCMachOStreamer::emitSymbolAttribute(MCSymbol *Sym,
   if (Attribute == MCSA_IndirectSymbol) {
     // Note that we intentionally cannot use the symbol data here; this is
     // important for matching the string table that 'as' generates.
-    static_cast<MachObjectWriter &>(getAssembler().getWriter())
-        .getIndirectSymbols()
-        .push_back({Symbol, getCurrentSectionOnly()});
+    getWriter().getIndirectSymbols().push_back(
+        {Symbol, getCurrentSectionOnly()});
     return true;
   }
 

llvm/lib/MC/MachObjectWriter.cpp

@@ -48,6 +48,7 @@ void MachObjectWriter::reset() {
   Relocations.clear();
   IndirectSymBase.clear();
   IndirectSymbols.clear();
+  DataRegions.clear();
   SectionAddress.clear();
   SectionOrder.clear();
   StringTable.clear();
@@ -828,7 +829,7 @@ uint64_t MachObjectWriter::writeObject(MCAssembler &Asm) {
   }
 
   // Add the data-in-code load command size, if used.
-  unsigned NumDataRegions = Asm.getDataRegions().size();
+  unsigned NumDataRegions = DataRegions.size();
   if (NumDataRegions) {
     ++NumLoadCommands;
     LoadCommandsSize += sizeof(MachO::linkedit_data_command);
@@ -1025,25 +1026,21 @@ uint64_t MachObjectWriter::writeObject(MCAssembler &Asm) {
   }
 
   // Write out the data-in-code region payload, if there is one.
-  for (MCAssembler::const_data_region_iterator
-         it = Asm.data_region_begin(), ie = Asm.data_region_end();
-         it != ie; ++it) {
-    const DataRegionData *Data = &(*it);
-    uint64_t Start = getSymbolAddress(*Data->Start, Asm);
+  for (DataRegionData Data : DataRegions) {
+    uint64_t Start = getSymbolAddress(*Data.Start, Asm);
     uint64_t End;
-    if (Data->End)
-      End = getSymbolAddress(*Data->End, Asm);
+    if (Data.End)
+      End = getSymbolAddress(*Data.End, Asm);
     else
       report_fatal_error("Data region not terminated");
 
-    LLVM_DEBUG(dbgs() << "data in code region-- kind: " << Data->Kind
-                      << "  start: " << Start << "(" << Data->Start->getName()
-                      << ")"
-                      << "  end: " << End << "(" << Data->End->getName() << ")"
-                      << "  size: " << End - Start << "\n");
+    LLVM_DEBUG(dbgs() << "data in code region-- kind: " << Data.Kind
+                      << "  start: " << Start << "(" << Data.Start->getName()
+                      << ")" << "  end: " << End << "(" << Data.End->getName()
+                      << ")" << "  size: " << End - Start << "\n");
     W.write<uint32_t>(Start);
     W.write<uint16_t>(End - Start);
-    W.write<uint16_t>(Data->Kind);
+    W.write<uint16_t>(Data.Kind);
   }
 
   // Write out the loh commands, if there is one.