Revert "Bitcode: Collect all MDString records into a single blob"

This reverts commit r264409 since it failed to bootstrap:
http://lab.llvm.org:8080/green/job/clang-stage2-configure-Rlto_build/8302/

llvm-svn: 264410

5 files changed, 34 insertions, 129 deletions

diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index b849db4427a..bb479dbcedc 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -2363,7 +2363,7 @@ std::error_code BitcodeReader::parseMetadata(bool ModuleLevel) {
           NextMetadataNo++);
       break;
     }
-    case bitc::METADATA_STRING_OLD: {
+    case bitc::METADATA_STRING: {
       std::string String(Record.begin(), Record.end());
 
       // Test for upgrading !llvm.loop.
@@ -2373,38 +2373,6 @@ std::error_code BitcodeReader::parseMetadata(bool ModuleLevel) {
       MetadataList.assignValue(MD, NextMetadataNo++);
       break;
     }
-    case bitc::METADATA_BULK_STRING_SIZES: {
-      // This is a pair of records for an MDString block: SIZES, which is a
-      // list of string lengths; and DATA, which is a blob with all the strings
-      // concatenated together.
-      //
-      // Note: since this record type was introduced after the upgrade for
-      // !llvm.loop, we don't need to change HasSeenOldLoopTags.
-      if (Record.empty())
-        return error("Invalid record: missing bulk metadata string sizes");
-
-      StringRef Blob;
-      SmallVector<uint64_t, 1> BlobRecord;
-      Code = Stream.ReadCode();
-      unsigned BlobCode = Stream.readRecord(Code, BlobRecord, &Blob);
-      if (BlobCode != bitc::METADATA_BULK_STRING_DATA)
-        return error("Invalid record: missing bulk metadata string data");
-      if (!BlobRecord.empty())
-        return error("Invalid record: unexpected bulk metadata arguments");
-
-      for (uint64_t Size : Record) {
-        if (Blob.size() < Size)
-          return error("Invalid record: not enough bulk metadata string bytes");
-
-        // Extract the current string.
-        MetadataList.assignValue(MDString::get(Context, Blob.slice(0, Size)),
-                                 NextMetadataNo++);
-        Blob = Blob.drop_front(Size);
-      }
-      if (!Blob.empty())
-        return error("Invalid record: too many bulk metadata string bytes");
-      break;
-    }
     case bitc::METADATA_KIND: {
       // Support older bitcode files that had METADATA_KIND records in a
       // block with METADATA_BLOCK_ID.
diff --git a/llvm/lib/Bitcode/Reader/BitstreamReader.cpp b/llvm/lib/Bitcode/Reader/BitstreamReader.cpp
index 43dae0c35a7..db9e0cd63ac 100644
--- a/llvm/lib/Bitcode/Reader/BitstreamReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitstreamReader.cpp
@@ -261,10 +261,6 @@ unsigned BitstreamCursor::readRecord(unsigned AbbrevID,
     }
 
     // Otherwise, inform the streamer that we need these bytes in memory.
-    // Skip over tail padding first.  We can't do it later if this is a
-    // streaming memory object, since that could reallocate the storage that
-    // the blob pointer references.
-    JumpToBit(NewEnd);
     const char *Ptr = (const char*)
       BitStream->getBitcodeBytes().getPointer(CurBitPos/8, NumElts);
 
@@ -276,6 +272,8 @@ unsigned BitstreamCursor::readRecord(unsigned AbbrevID,
       for (; NumElts; --NumElts)
         Vals.push_back((unsigned char)*Ptr++);
     }
+    // Skip over tail padding.
+    JumpToBit(NewEnd);
   }
 
   return Code;
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 0108667c9e2..5d051649699 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -1347,78 +1347,31 @@ static void writeNamedMetadata(const Module &M, const ValueEnumerator &VE,
   }
 }
 
-static unsigned createMDStringDataAbbrev(BitstreamWriter &Stream) {
-  BitCodeAbbrev *Abbv = new BitCodeAbbrev();
-  Abbv->Add(BitCodeAbbrevOp(bitc::METADATA_BULK_STRING_DATA));
-  Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
-  return Stream.EmitAbbrev(Abbv);
-}
-
-static void emitMDStringBlob(unsigned DataAbbrev,
-                             ArrayRef<const Metadata *> Strings,
-                             BitstreamWriter &Stream,
-                             SmallVectorImpl<uint64_t> &Record,
-                             SmallString<4096> &Blob) {
-  for (const Metadata *MD : Strings) {
-    StringRef S = cast<MDString>(MD)->getString();
-    Record.push_back(S.size());
-    Blob.append(S.begin(), S.end());
-  }
-
-  Stream.EmitRecord(bitc::METADATA_BULK_STRING_SIZES, Record);
-  Record.clear();
-
-  Record.push_back(bitc::METADATA_BULK_STRING_DATA);
-  Stream.EmitRecordWithBlob(DataAbbrev, Record, Blob);
-  Record.clear();
-}
-
-/// Write out a section of records for MDString.
-///
-/// All the MDString elements in a metadata block are emitted in bulk.  They're
-/// grouped into blocks, and each block is emitted with pair of records:
-///
-///   - SIZES: a list of the sizes of the strings in the block.
-///   - DATA: the blob itself.
-static void writeMetadataStrings(ArrayRef<const Metadata *> Strings,
-                                 BitstreamWriter &Stream,
-                                 SmallVectorImpl<uint64_t> &Record) {
-  if (Strings.empty())
-    return;
-
-  // Emit strings in large blocks to reduce record overhead.  Somewhat
-  // arbitrarily, limit this to 512 strings per blob:
-  //   - big enough to eliminate overhead;
-  //   - small enough that the reader's SIZES record will stay within a page.
-  const size_t NumStringsPerBlob = 512;
-  Record.reserve(std::min(NumStringsPerBlob, Strings.size()));
-
-  unsigned DataAbbrev = createMDStringDataAbbrev(Stream);
-  SmallString<4096> Blob;
-  while (Strings.size() > NumStringsPerBlob) {
-    emitMDStringBlob(DataAbbrev, Strings.slice(0, NumStringsPerBlob), Stream,
-                     Record, Blob);
-    Strings = Strings.slice(NumStringsPerBlob);
-  }
-  if (!Strings.empty())
-    emitMDStringBlob(DataAbbrev, Strings, Stream, Record, Blob);
-}
-
 static void WriteModuleMetadata(const Module &M,
                                 const ValueEnumerator &VE,
                                 BitstreamWriter &Stream) {
-  if (VE.getMDs().empty() && M.named_metadata_empty())
+  const auto &MDs = VE.getMDs();
+  if (MDs.empty() && M.named_metadata_empty())
     return;
 
   Stream.EnterSubblock(bitc::METADATA_BLOCK_ID, 3);
 
+  unsigned MDSAbbrev = 0;
+  if (VE.hasMDString()) {
+    // Abbrev for METADATA_STRING.
+    BitCodeAbbrev *Abbv = new BitCodeAbbrev();
+    Abbv->Add(BitCodeAbbrevOp(bitc::METADATA_STRING));
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 8));
+    MDSAbbrev = Stream.EmitAbbrev(Abbv);
+  }
+
   // Initialize MDNode abbreviations.
 #define HANDLE_MDNODE_LEAF(CLASS) unsigned CLASS##Abbrev = 0;
 #include "llvm/IR/Metadata.def"
 
   SmallVector<uint64_t, 64> Record;
-  writeMetadataStrings(VE.getMDStrings(), Stream, Record);
-  for (const Metadata *MD : VE.getNonMDStrings()) {
+  for (const Metadata *MD : MDs) {
     if (const MDNode *N = dyn_cast<MDNode>(MD)) {
       assert(N->isResolved() && "Expected forward references to be resolved");
 
@@ -1432,7 +1385,17 @@ static void WriteModuleMetadata(const Module &M,
 #include "llvm/IR/Metadata.def"
       }
     }
-    WriteValueAsMetadata(cast<ConstantAsMetadata>(MD), VE, Stream, Record);
+    if (const auto *MDC = dyn_cast<ConstantAsMetadata>(MD)) {
+      WriteValueAsMetadata(MDC, VE, Stream, Record);
+      continue;
+    }
+    const MDString *MDS = cast<MDString>(MD);
+    // Code: [strchar x N]
+    Record.append(MDS->bytes_begin(), MDS->bytes_end());
+
+    // Emit the finished record.
+    Stream.EmitRecord(bitc::METADATA_STRING, Record, MDSAbbrev);
+    Record.clear();
   }
 
   writeNamedMetadata(M, VE, Stream, Record);
diff --git a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
index 69cafb7eeee..08b5e45703a 100644
--- a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
+++ b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
@@ -280,7 +280,8 @@ static bool isIntOrIntVectorValue(const std::pair<const Value*, unsigned> &V) {
 
 ValueEnumerator::ValueEnumerator(const Module &M,
                                  bool ShouldPreserveUseListOrder)
-    : ShouldPreserveUseListOrder(ShouldPreserveUseListOrder) {
+    : HasMDString(false),
+      ShouldPreserveUseListOrder(ShouldPreserveUseListOrder) {
   if (ShouldPreserveUseListOrder)
     UseListOrders = predictUseListOrder(M);
 
@@ -374,9 +375,6 @@ ValueEnumerator::ValueEnumerator(const Module &M,
 
   // Optimize constant ordering.
   OptimizeConstants(FirstConstant, Values.size());
-
-  // Organize metadata ordering.
-  organizeMetadata();
 }
 
 unsigned ValueEnumerator::getInstructionID(const Instruction *Inst) const {
@@ -532,8 +530,8 @@ void ValueEnumerator::EnumerateMetadata(const Metadata *MD) {
     EnumerateMDNodeOperands(N);
   else if (auto *C = dyn_cast<ConstantAsMetadata>(MD))
     EnumerateValue(C->getValue());
-  else
-    ++NumMDStrings;
+
+  HasMDString |= isa<MDString>(MD);
 
   // Replace the dummy ID inserted above with the correct one.  MetadataMap may
   // have changed by inserting operands, so we need a fresh lookup here.
@@ -559,19 +557,6 @@ void ValueEnumerator::EnumerateFunctionLocalMetadata(
   FunctionLocalMDs.push_back(Local);
 }
 
-void ValueEnumerator::organizeMetadata() {
-  if (!NumMDStrings)
-    return;
-
-  // Put the strings first.
-  std::stable_partition(MDs.begin(), MDs.end(),
-                        [](const Metadata *MD) { return isa<MDString>(MD); });
-
-  // Renumber.
-  for (unsigned I = 0, E = MDs.size(); I != E; ++I)
-    MetadataMap[MDs[I]] = I + 1;
-}
-
 void ValueEnumerator::EnumerateValue(const Value *V) {
   assert(!V->getType()->isVoidTy() && "Can't insert void values!");
   assert(!isa<MetadataAsValue>(V) && "EnumerateValue doesn't handle Metadata!");
diff --git a/llvm/lib/Bitcode/Writer/ValueEnumerator.h b/llvm/lib/Bitcode/Writer/ValueEnumerator.h
index fd09a695a99..7665210d014 100644
--- a/llvm/lib/Bitcode/Writer/ValueEnumerator.h
+++ b/llvm/lib/Bitcode/Writer/ValueEnumerator.h
@@ -66,7 +66,7 @@ private:
   SmallVector<const LocalAsMetadata *, 8> FunctionLocalMDs;
   typedef DenseMap<const Metadata *, unsigned> MetadataMapType;
   MetadataMapType MetadataMap;
-  unsigned NumMDStrings = 0;
+  bool HasMDString;
   bool ShouldPreserveUseListOrder;
 
   typedef DenseMap<AttributeSet, unsigned> AttributeGroupMapType;
@@ -121,6 +121,8 @@ public:
   }
   unsigned numMDs() const { return MDs.size(); }
 
+  bool hasMDString() const { return HasMDString; }
+
   bool shouldPreserveUseListOrder() const { return ShouldPreserveUseListOrder; }
 
   unsigned getTypeID(Type *T) const {
@@ -155,16 +157,9 @@ public:
 
   const ValueList &getValues() const { return Values; }
   const std::vector<const Metadata *> &getMDs() const { return MDs; }
-  ArrayRef<const Metadata *> getMDStrings() const {
-    return makeArrayRef(MDs).slice(0, NumMDStrings);
-  }
-  ArrayRef<const Metadata *> getNonMDStrings() const {
-    return makeArrayRef(MDs).slice(NumMDStrings);
-  }
   const SmallVectorImpl<const LocalAsMetadata *> &getFunctionLocalMDs() const {
     return FunctionLocalMDs;
   }
-
   const TypeList &getTypes() const { return Types; }
   const std::vector<const BasicBlock*> &getBasicBlocks() const {
     return BasicBlocks;
@@ -194,10 +189,6 @@ public:
 private:
   void OptimizeConstants(unsigned CstStart, unsigned CstEnd);
 
-  // Reorder the reachable metadata.  This is not just an optimization, but is
-  // mandatory for emitting MDString correctly.
-  void organizeMetadata();
-
   void EnumerateMDNodeOperands(const MDNode *N);
   void EnumerateMetadata(const Metadata *MD);
   void EnumerateFunctionLocalMetadata(const LocalAsMetadata *Local);