Bitcode: use blob for string storage in the IR: trade a bit of space for faster reading

[mehdi_amini]

Summary

The bitcode store string in a compressed fashion. This is nice for space, but
it is expensive to encode/decode. I experimented using the "blob" type in the
bitcode, and obtained non-negligible speedup.
The following patch provides 10% speedup on the execution of getLazyIRModule()
when linking "opt" with ThinLTO. The on-disk size is also increased by ~10%.

I left it under a flag in the writer for experiment purpose, but I'm seeking
opinion on the subject.

[filcab]

include/llvm/Support/StreamingMemoryObject.h

34

What happens if there's two BLOBs in the stream? Wouldn't you overwrite one with the other?

[mehdi_amini]

include/llvm/Support/StreamingMemoryObject.h

34

Yes the validity of the pointer returned last only till the next read from the stream.
The model is that there will be a copy made by the client anyway. But with blob won't "unpack" 6 bits elements to an array of unsigned, and then decode the 6 bits encoding to char, and then do the copy.

Note also that I haven't find another place than llvm-dis that uses this code-path.

[filcab]

Have you tried running clang with this patch? If possible, with ASan on.

include/llvm/Support/StreamingMemoryObject.h

34

Have you tried clang too?

Wouldn't this code (in clang) break (I added comments)?

case SM_SLOC_BUFFER_ENTRY: {
  const char *Name = Blob.data();           // <- Getting a ref to the current blob
  unsigned Offset = Record[0];
  SrcMgr::CharacteristicKind
    FileCharacter = (SrcMgr::CharacteristicKind)Record[2];
  SourceLocation IncludeLoc = ReadSourceLocation(*F, Record[1]);
  if (IncludeLoc.isInvalid() &&
      (F->Kind == MK_ImplicitModule || F->Kind == MK_ExplicitModule)) {
    IncludeLoc = getImportLocation(F);
  }
  unsigned Code = SLocEntryCursor.ReadCode();
  Record.clear();
  unsigned RecCode
    = SLocEntryCursor.readRecord(Code, Record, &Blob);     // <- That old blob reference is now invalid

  if (RecCode != SM_SLOC_BUFFER_BLOB) {
    Error("AST record has invalid code");
    return true;
  }

  std::unique_ptr<llvm::MemoryBuffer> Buffer =
      llvm::MemoryBuffer::getMemBuffer(Blob.drop_back(1), Name);    // <- Use ref to first blob
  SourceMgr.createFileID(std::move(Buffer), FileCharacter, ID,
                         BaseOffset + Offset, IncludeLoc);
  break;
}

"Nothing" in "llvm only" uses Blobs, basically (llvm-bcanalyzer does get one and dump it :-) ). But clang uses them a lot.

[tejohnson]

Was the improvement in speed measured on top of your DecodeChar6 change? If not, do you still see a nice improvement?

include/llvm/Support/StreamingMemoryObject.h

34

The old streaming getPointer took a fatal error, so presumably the clang code wasn't invoking it during streaming, so wouldn't break with Mehdi's change. Or is the concern that other code like this may creep in for StreamingMemoryObject?

lib/Bitcode/Reader/BitcodeReader.cpp

1732

There's a lot of code duplication here with the other recordValue(). Looks like the other recordValue() could get the Name string and invoke this one. Or just keep this one and if Name is empty here, invoke convertToString.

2675

Looks like these two cases could be collapsed as they were before. Is it clearer with them separated?

lib/Bitcode/Writer/BitcodeWriter.cpp

1514

This should be CST_CODE_STRING. Also, change comment to distinguish from original CST_CODE_STRING case above?

1531

Change comment to distinguish from original CST_CODE_CSTRING case above?

1643

Move up and make other cases "else if"

2300

Needs comment.

2337

Move this up and make the other case "else if"

2360

Move this up and make the other cases "else if"

2619

Needs comment.

[filcab]

include/llvm/Support/StreamingMemoryObject.h

34

True, the Streaming version is not being used in clang for sure. And if it's used by someone else (PNaCl?), it's not a problem now, so it can't be a problem after Mehdi's patch.

Mehdi: Instead of keeping the assert like I was saying (which is basically there to block code paths), let's just do your change. But please add some documentation stating that the interface to getPointer only guarantees that it's valid until the next call to it, and that the caller needs to either use it before then, or copy it. That way it's explicitly stated in the comment.

Thank you.

[mehdi_amini]

Thanks for the comments, the measurements were made after the decodeChar6 changes.

lib/Bitcode/Reader/BitcodeReader.cpp

1732

I like you second suggestion! Thanks.

2675

The CST_CODE_CSTRING has to add a 0 at the end of the string. This was hidden in ConstantDataArray::getString at the price of an extra copy.
This is why I splitted it to save the SmallString copy in the non-blob case for String.
I just notice now that I still have a Smallstring for the STRING case, I think I can remove this.

lib/Bitcode/Writer/BitcodeWriter.cpp

1643

The OptSpeed flag is hacked a bit everywhere, I'll clean it but I wasn't sure about the general feeling with this patch?

[tejohnson]

In D16277#330142, @joker.eph wrote:

Thanks for the comments, the measurements were made after the decodeChar6 changes.

Do you know where the current hotspot is? Do the strings have such higher overhead than blobs because of the copying required in the BitstreamReader when decoding? I wouldn't think that your new version of decodeChar6 would be very expensive. If it is due to the copying, what about an alternate approach, where the string is returned as is without copying/decoding (like a blob), but the decoding is done in the BitcodeReader or whatever it calls that copies the blob (e.g. ConstantDataArray::getString for constants, and does Value::setName do the copying for VST entries in the blob case?).

That's more invasive, but 10% is a pretty big on-disk size increase.

Another question - do you know the breakdown between the different record types? This should be pretty easily extracted from the llvm-bcanalyzer record histograms. A couple thoughts:

• If it is dominated by the VST FNENTRY records, then I would imagine that reading in the combined function index file in the ThinLTO compile would also suffer from the same problem. Do we need to do something there too?
• If it is module-level constant strings and VST ENTRY records (for declarations), I wonder if there is some kind of lazy reading/decoding we could do for these (they presumably aren't all needed by imported functions.
• If it is function level BBENTRY and ENTRY records (presumably for the imported functions since we wouldn't parse the others), then I wonder if we can get some savings (both time and space) by using a string table. For a simple example I looked at, I had two functions each containing a call to printf that each had a BBENTRY record with the string "entry" and ENTRY record with the string "call". I would imagine the former in particular is duplicated quite frequently. I saw BBENTRY records in another simple function I looked at that had other very common sounding strings such as "return", "if.else", "if.then", "retval", etc. Ditto for ENTRY records for local values/parameters which might have the same name in multiple functions.

[mehdi_amini]

In D16277#331129, @tejohnson wrote:

In D16277#330142, @joker.eph wrote:

Thanks for the comments, the measurements were made after the decodeChar6 changes.

Do you know where the current hotspot is? Do the strings have such higher overhead than blobs because of the copying required in the BitstreamReader when decoding? I wouldn't think that your new version of decodeChar6 would be very expensive. If it is due to the copying, what about an alternate approach, where the string is returned as is without copying/decoding (like a blob), but the decoding is done in the BitcodeReader or whatever it calls that copies the blob (e.g. ConstantDataArray::getString for constants, and does Value::setName do the copying for VST entries in the blob case?).

The problem is that the bitcode is not byte aligned. Are you suggesting that we store the strings as "blob", correctly aligned, but potentially "char6 encoded"?

Another question - do you know the breakdown between the different record types? This should be pretty easily extracted from the llvm-bcanalyzer record histograms. A couple thoughts:

Haven't looked, I have shifted to debug info in the meantime ;)

• If it is dominated by the VST FNENTRY records, then I would imagine that reading in the combined function index file in the ThinLTO compile would also suffer from the same problem. Do we need to do something there too?

In my use case, the combine function index is only built in memory and never stored on disk.

• If it is module-level constant strings and VST ENTRY records (for declarations), I wonder if there is some kind of lazy reading/decoding we could do for these (they presumably aren't all needed by imported functions.

Yes we're not lazy enough in many places, but it is not trivial to implement though. :(

• If it is function level BBENTRY and ENTRY records (presumably for the imported functions since we wouldn't parse the others), then I wonder if we can get some savings (both time and space) by using a string table. For a simple example I looked at, I had two functions each containing a call to printf that each had a BBENTRY record with the string "entry" and ENTRY record with the string "call". I would imagine the former in particular is duplicated quite frequently. I saw BBENTRY records in another simple function I looked at that had other very common sounding strings such as "return", "if.else", "if.then", "retval", etc. Ditto for ENTRY records for local values/parameters which might have the same name in multiple functions.

Bitcode is lazy loaded, so Function were not parsed. I suspect the same problem would show up anywhere. Reading elements per elements out of the bitcode stream is far too expensive. The BitstreamCursor has to keep track of the bit shifting state and co.

I experimented storing all the global metadata in single blob record, that is serialized with a FlatBuffer (https://github.com/google/flatbuffers): my ThinLTO importing gets ~10% faster.

[mehdi_amini]

Note: I leave this patch here as a straw man, but I don't actually plan to include it as is. I think the way forward it to completely break down the bitcode encoding decoupling serialization and compression.

[mehdi_amini]

Obsolete per r264551

[mehdi_amini]

My comment was wrong: this is not obsolete per r264551, the current one is about every non-metadata string while r264551 is about MDString

include/llvm/Support/StreamingMemoryObject.h

 //===- StreamingMemoryObject.h - Streamable data interface -----*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_STREAMINGMEMORYOBJECT_H
 #define LLVM_SUPPORT_STREAMINGMEMORYOBJECT_H
 
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/DataStream.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MemoryObject.h"
 #include <memory>
 #include <vector>
 
 namespace llvm {
 
 /// Interface to data which is actually streamed from a DataStreamer. In
 /// addition to inherited members, it has the dropLeadingBytes and
 /// setKnownObjectSize methods which are not applicable to non-streamed objects.
 class StreamingMemoryObject : public MemoryObject {
+  /// Buffer for temporary storage if getPointer() is used
+  mutable SmallVector<uint8_t, 128> Buffer;
 public:
   StreamingMemoryObject(std::unique_ptr<DataStreamer> Streamer);
   uint64_t getExtent() const override;
   uint64_t readBytes(uint8_t *Buf, uint64_t Size,
                      uint64_t Address) const override;
-  const uint8_t *getPointer(uint64_t address, uint64_t size) const override {
-    // FIXME: This could be fixed by ensuring the bytes are fetched and
-    // making a copy, requiring that the bitcode size be known, or
-    // otherwise ensuring that the memory doesn't go away/get reallocated,
-    // but it's not currently necessary. Users that need the pointer (any
-    // that need Blobs) don't stream.
-    report_fatal_error("getPointer in streaming memory objects not allowed");
-    return nullptr;
+  const uint8_t *getPointer(uint64_t Address, uint64_t Size) const override {
+    Buffer.resize(Size);
+    readBytes(&Buffer[0], Size, Address);
+    return &Buffer[0];
   }
   bool isValidAddress(uint64_t address) const override;
 
   /// Drop s bytes from the front of the stream, pushing the positions of the
   /// remaining bytes down by s. This is used to skip past the bitcode header,
   /// since we don't know a priori if it's present, and we can't put bytes
   /// back into the stream once we've read them.
   bool dropLeadingBytes(size_t s);

lib/Bitcode/Reader/BitcodeReader.cpp

private:
   std::error_code parseAttributeBlock();
   std::error_code parseAttributeGroupBlock();
   std::error_code parseTypeTable();
   std::error_code parseTypeTableBody();
   std::error_code parseOperandBundleTags();
 
   ErrorOr<Value *> recordValue(SmallVectorImpl<uint64_t> &Record,
                                unsigned NameIndex, Triple &TT);
+  ErrorOr<Value *> recordValue(SmallVectorImpl<uint64_t> &Record, StringRef Name,
+                               unsigned NameIndex, Triple &TT);
   std::error_code parseValueSymbolTable(uint64_t Offset = 0);
   std::error_code parseConstants();
   std::error_code rememberAndSkipFunctionBodies();
   std::error_code rememberAndSkipFunctionBody();
   /// Save the positions of the Metadata blocks and skip parsing the blocks.
   std::error_code rememberAndSkipMetadata();
   std::error_code parseFunctionBody(Function *F);
   std::error_code globalCleanup();
if (GO->getComdat() == reinterpret_cast<Comdat *>(1)) {
         GO->setComdat(nullptr);
       else
         GO->setComdat(TheModule->getOrInsertComdat(V->getName()));
     }
   }
   return V;
 }
 
+ErrorOr<Value *> BitcodeReader::recordValue(SmallVectorImpl<uint64_t> &Record, StringRef Name,
+                             unsigned NameIndex, Triple &TT) {
+  if(Name.empty()) {
+    return recordValue(Record, NameIndex, TT);
+  }
+  unsigned ValueID = Record[0];
+  if (ValueID >= ValueList.size() || !ValueList[ValueID])
+    return error("Invalid record");
+  Value *V = ValueList[ValueID];
+
+  if (Name.find_first_of(0) != StringRef::npos)
+    return error("Invalid value name");
+  V->setName(Name);
+  auto *GO = dyn_cast<GlobalObject>(V);
+  if (GO) {
+    if (GO->getComdat() == reinterpret_cast<Comdat *>(1)) {
+      if (TT.isOSBinFormatMachO())
+        GO->setComdat(nullptr);
+      else
+        GO->setComdat(TheModule->getOrInsertComdat(V->getName()));
+    }
+  }
+  return V;
+}
+
+
 /// Parse the value symbol table at either the current parsing location or
 /// at the given bit offset if provided.
 std::error_code BitcodeReader::parseValueSymbolTable(uint64_t Offset) {
   uint64_t CurrentBit;
   // Pass in the Offset to distinguish between calling for the module-level
   // VST (where we want to jump to the VST offset) and the function-level
   // VST (where we don't).
   if (Offset > 0) {
case BitstreamEntry::EndBlock:
       return std::error_code();
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read a record.
     Record.clear();
-    switch (Stream.readRecord(Entry.ID, Record)) {
+    StringRef Name;
+    switch (Stream.readRecord(Entry.ID, Record, &Name)) {
     default:  // Default behavior: unknown type.
       break;
     case bitc::VST_CODE_ENTRY: {  // VST_ENTRY: [valueid, namechar x N]
-      ErrorOr<Value *> ValOrErr = recordValue(Record, 1, TT);
+      ErrorOr<Value *> ValOrErr = recordValue(Record, Name, 1, TT);
       if (std::error_code EC = ValOrErr.getError())
         return EC;
       ValOrErr.get();
       break;
     }
     case bitc::VST_CODE_FNENTRY: {
       // VST_FNENTRY: [valueid, offset, namechar x N]
-      ErrorOr<Value *> ValOrErr = recordValue(Record, 2, TT);
+      ErrorOr<Value *> ValOrErr = recordValue(Record, Name, 2, TT);
       if (std::error_code EC = ValOrErr.getError())
         return EC;
       Value *V = ValOrErr.get();
 
       auto *GO = dyn_cast<GlobalObject>(V);
       if (!GO) {
         // If this is an alias, need to get the actual Function object
         // it aliases, in order to set up the DeferredFunctionInfo entry below.
case bitc::VST_CODE_FNENTRY: {
       // Set the LastFunctionBlockBit to point to the last function block.
       // Later when parsing is resumed after function materialization,
       // we can simply skip that last function block.
       if (FuncBitOffset > LastFunctionBlockBit)
         LastFunctionBlockBit = FuncBitOffset;
       break;
     }
     case bitc::VST_CODE_BBENTRY: {
-      if (convertToString(Record, 1, ValueName))
+      if (Name.empty()) {
+        if(convertToString(Record, 1, ValueName))
           return error("Invalid record");
+        Name = ValueName;
+      }
       BasicBlock *BB = getBasicBlock(Record[0]);
       if (!BB)
         return error("Invalid record");
 
-      BB->setName(StringRef(ValueName.data(), ValueName.size()));
+      BB->setName(Name);
       ValueName.clear();
       break;
     }
     }
   }
 }
 
 /// Parse a single METADATA_KIND record, inserting result in MDKindMap.
while (1) {
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     // Read a record.
     Record.clear();
     Value *V = nullptr;
-    unsigned BitCode = Stream.readRecord(Entry.ID, Record);
+    StringRef Blob;
+    unsigned BitCode = Stream.readRecord(Entry.ID, Record, &Blob);
     switch (BitCode) {
     default:  // Default behavior: unknown constant
     case bitc::CST_CODE_UNDEF:     // UNDEF
       V = UndefValue::get(CurTy);
       break;
     case bitc::CST_CODE_SETTYPE:   // SETTYPE: [typeid]
       if (Record.empty())
         return error("Invalid record");
case bitc::CST_CODE_AGGREGATE: {// AGGREGATE: [n x value number]
         for (unsigned i = 0; i != Size; ++i)
           Elts.push_back(ValueList.getConstantFwdRef(Record[i], EltTy));
         V = ConstantVector::get(Elts);
       } else {
         V = UndefValue::get(CurTy);
       }
       break;
     }
-    case bitc::CST_CODE_STRING:    // STRING: [values]
+    case bitc::CST_CODE_STRING:  {  // STRING: [values]
+      if (!Blob.empty()) {
+        V = ConstantDataArray::getString(Context, Blob, false);
+        break;
+      }
+      if (Record.empty())
+        return error("Invalid record");
+      SmallString<16> Elts(Record.begin(), Record.end());
+      V = ConstantDataArray::getString(Context, Elts, false);
+      break;
+    }
     case bitc::CST_CODE_CSTRING: { // CSTRING: [values]
+      if (!Blob.empty()) {
+        V = ConstantDataArray::getString(Context, Blob, true);
+        break;
+      }
       if (Record.empty())
         return error("Invalid record");
 
       SmallString<16> Elts(Record.begin(), Record.end());
-      V = ConstantDataArray::getString(Context, Elts,
-                                       BitCode == bitc::CST_CODE_CSTRING);
+      Elts.push_back('\0');
+      V = ConstantDataArray::getString(Context, Elts, false);
       break;
     }
     case bitc::CST_CODE_DATA: {// DATA: [n x value]
       if (Record.empty())
         return error("Invalid record");
 
       Type *EltTy = cast<SequentialType>(CurTy)->getElementType();
       if (EltTy->isIntegerTy(8)) {

lib/Bitcode/Writer/BitcodeWriter.cpp

 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cctype>
 #include <map>
 using namespace llvm;
 
+static cl::opt<bool> OptSpeed("bc-optspeed", cl::init(false),
+                             cl::desc("Optimize bitcode for speed."));
+
 /// These are manifest constants used by the bitcode writer. They do not need to
 /// be kept in sync with the reader, but need to be consistent within this file.
 enum {
   // VALUE_SYMTAB_BLOCK abbrev id's.
   VST_ENTRY_8_ABBREV = bitc::FIRST_APPLICATION_ABBREV,
   VST_ENTRY_7_ABBREV,
   VST_ENTRY_6_ABBREV,
+  VST_ENTRY_BLOB_ABBREV,
   VST_BBENTRY_6_ABBREV,
+  VST_BBENTRY_BLOB_ABBREV,
 
   // CONSTANTS_BLOCK abbrev id's.
   CONSTANTS_SETTYPE_ABBREV = bitc::FIRST_APPLICATION_ABBREV,
   CONSTANTS_INTEGER_ABBREV,
   CONSTANTS_CE_CAST_Abbrev,
   CONSTANTS_NULL_Abbrev,
 
   // FUNCTION_BLOCK abbrev id's.
static void WriteConstants(unsigned FirstVal, unsigned LastVal,
                            const ValueEnumerator &VE,
                            BitstreamWriter &Stream, bool isGlobal) {
   if (FirstVal == LastVal) return;
 
   Stream.EnterSubblock(bitc::CONSTANTS_BLOCK_ID, 4);
 
   unsigned AggregateAbbrev = 0;
   unsigned String8Abbrev = 0;
+  unsigned StringBlobAbbrev = 0;
   unsigned CString7Abbrev = 0;
   unsigned CString6Abbrev = 0;
+  unsigned CStringBlobAbbrev = 0;
   // If this is a constant pool for the module, emit module-specific abbrevs.
   if (isGlobal) {
     // Abbrev for CST_CODE_AGGREGATE.
     BitCodeAbbrev *Abbv = new BitCodeAbbrev();
     Abbv->Add(BitCodeAbbrevOp(bitc::CST_CODE_AGGREGATE));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, Log2_32_Ceil(LastVal+1)));
     AggregateAbbrev = Stream.EmitAbbrev(Abbv);
 
     // Abbrev for CST_CODE_STRING.
     Abbv = new BitCodeAbbrev();
     Abbv->Add(BitCodeAbbrevOp(bitc::CST_CODE_STRING));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 8));
     String8Abbrev = Stream.EmitAbbrev(Abbv);
     // Abbrev for CST_CODE_CSTRING.
     Abbv = new BitCodeAbbrev();
+    Abbv->Add(BitCodeAbbrevOp(bitc::CST_CODE_STRING));
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
+    StringBlobAbbrev = Stream.EmitAbbrev(Abbv);
+    // Abbrev for CST_CODE_CSTRING.
+    Abbv = new BitCodeAbbrev();
     Abbv->Add(BitCodeAbbrevOp(bitc::CST_CODE_CSTRING));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 7));
     CString7Abbrev = Stream.EmitAbbrev(Abbv);
     // Abbrev for CST_CODE_CSTRING.
     Abbv = new BitCodeAbbrev();
     Abbv->Add(BitCodeAbbrevOp(bitc::CST_CODE_CSTRING));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Char6));
     CString6Abbrev = Stream.EmitAbbrev(Abbv);
+    // Abbrev for CST_CODE_CSTRING.
+    Abbv = new BitCodeAbbrev();
+    Abbv->Add(BitCodeAbbrevOp(bitc::CST_CODE_CSTRING));
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
+    CStringBlobAbbrev = Stream.EmitAbbrev(Abbv);
   }
 
   SmallVector<uint64_t, 64> Record;
 
   const ValueEnumerator::ValueList &Vals = VE.getValues();
   Type *LastTy = nullptr;
   for (unsigned i = FirstVal; i != LastVal; ++i) {
     const Value *V = Vals[i].first;
if (C->isNullValue()) {
         if (isCStrChar6)
           isCStrChar6 = BitCodeAbbrevOp::isChar6(V);
       }
 
       if (isCStrChar6)
         AbbrevToUse = CString6Abbrev;
       else if (isCStr7)
         AbbrevToUse = CString7Abbrev;
+      if (OptSpeed) {
+        if (Code == bitc::CST_CODE_STRING)
+          AbbrevToUse = StringBlobAbbrev;
+        else
+          AbbrevToUse = CStringBlobAbbrev;
+      }
     } else if (const ConstantDataSequential *CDS =
                   dyn_cast<ConstantDataSequential>(C)) {
       Code = bitc::CST_CODE_DATA;
       Type *EltTy = CDS->getType()->getElementType();
       if (isa<IntegerType>(EltTy)) {
         for (unsigned i = 0, e = CDS->getNumElements(); i != e; ++i)
           Record.push_back(CDS->getElementAsInteger(i));
       } else {
static void WriteValueSymbolTable(
 
   Stream.EnterSubblock(bitc::VALUE_SYMTAB_BLOCK_ID, 4);
 
   // For the module-level VST, add abbrev Ids for the VST_CODE_FNENTRY
   // records, which are not used in the per-function VSTs.
   unsigned FnEntry8BitAbbrev;
   unsigned FnEntry7BitAbbrev;
   unsigned FnEntry6BitAbbrev;
+  unsigned FnEntryBlobAbbrev;
   if (VSTOffsetPlaceholder > 0) {
     // 8-bit fixed-width VST_FNENTRY function strings.
     BitCodeAbbrev *Abbv = new BitCodeAbbrev();
     Abbv->Add(BitCodeAbbrevOp(bitc::VST_CODE_FNENTRY));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8)); // value id
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8)); // funcoffset
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 8));
if (VSTOffsetPlaceholder > 0) {
     // 6-bit char6 VST_FNENTRY function strings.
     Abbv = new BitCodeAbbrev();
     Abbv->Add(BitCodeAbbrevOp(bitc::VST_CODE_FNENTRY));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8)); // value id
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8)); // funcoffset
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Char6));
     FnEntry6BitAbbrev = Stream.EmitAbbrev(Abbv);
+
+    Abbv = new BitCodeAbbrev();
+    Abbv->Add(BitCodeAbbrevOp(bitc::VST_CODE_FNENTRY));
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8)); // value id
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8)); // funcoffset
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
+    FnEntryBlobAbbrev = Stream.EmitAbbrev(Abbv);
   }
 
   // FIXME: Set up the abbrev, we know how many values there are!
   // FIXME: We know if the type names can use 7-bit ascii.
   SmallVector<unsigned, 64> NameVals;
 
   for (const ValueName &Name : VST) {
     // Figure out the encoding to use for the name.
for (const ValueName &Name : VST) {
     // VST_ENTRY:   [valueid, namechar x N]
     // VST_FNENTRY: [valueid, funcoffset, namechar x N]
     // VST_BBENTRY: [bbid, namechar x N]
     unsigned Code;
     if (isa<BasicBlock>(Name.getValue())) {
       Code = bitc::VST_CODE_BBENTRY;
       if (Bits == SE_Char6)
         AbbrevToUse = VST_BBENTRY_6_ABBREV;
+      if (OptSpeed)
+        AbbrevToUse = VST_BBENTRY_BLOB_ABBREV;
     } else if (F && !F->isDeclaration()) {
       // Must be the module-level VST, where we pass in the Index and
       // have a VSTOffsetPlaceholder. The function-level VST should not
       // contain any Function symbols.
       assert(FunctionIndex);
       assert(VSTOffsetPlaceholder > 0);
 
       // Save the word offset of the function (from the start of the
       // actual bitcode written to the stream).
       assert(FunctionIndex->count(F) == 1);
       uint64_t BitcodeIndex =
           (*FunctionIndex)[F]->bitcodeIndex() - BitcodeStartBit;
       assert((BitcodeIndex & 31) == 0 && "function block not 32-bit aligned");
       NameVals.push_back(BitcodeIndex / 32);
 
       Code = bitc::VST_CODE_FNENTRY;
       AbbrevToUse = FnEntry8BitAbbrev;
       if (Bits == SE_Char6)
         AbbrevToUse = FnEntry6BitAbbrev;
       else if (Bits == SE_Fixed7)
         AbbrevToUse = FnEntry7BitAbbrev;
+      if (OptSpeed)
+        AbbrevToUse = FnEntryBlobAbbrev;
     } else {
       Code = bitc::VST_CODE_ENTRY;
-      if (Bits == SE_Char6)
+      if (OptSpeed) {
+        AbbrevToUse = VST_ENTRY_BLOB_ABBREV;
+      } else if (Bits == SE_Char6)
         AbbrevToUse = VST_ENTRY_6_ABBREV;
       else if (Bits == SE_Fixed7)
         AbbrevToUse = VST_ENTRY_7_ABBREV;
     }
 
     for (const auto P : Name.getKey())
       NameVals.push_back((unsigned char)P);
 
Stream.EnterBlockInfoBlock(2);
     Abbv->Add(BitCodeAbbrevOp(bitc::VST_CODE_ENTRY));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Char6));
     if (Stream.EmitBlockInfoAbbrev(bitc::VALUE_SYMTAB_BLOCK_ID,
                                    Abbv) != VST_ENTRY_6_ABBREV)
       llvm_unreachable("Unexpected abbrev ordering!");
   }
+  {
+    BitCodeAbbrev *Abbv = new BitCodeAbbrev();
+    Abbv->Add(BitCodeAbbrevOp(bitc::VST_CODE_ENTRY));
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
+    if (Stream.EmitBlockInfoAbbrev(bitc::VALUE_SYMTAB_BLOCK_ID,
+                                   Abbv) != VST_ENTRY_BLOB_ABBREV)
+      llvm_unreachable("Unexpected abbrev ordering!");
+  }
   { // 6-bit char6 VST_BBENTRY strings.
     BitCodeAbbrev *Abbv = new BitCodeAbbrev();
     Abbv->Add(BitCodeAbbrevOp(bitc::VST_CODE_BBENTRY));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Char6));
     if (Stream.EmitBlockInfoAbbrev(bitc::VALUE_SYMTAB_BLOCK_ID,
                                    Abbv) != VST_BBENTRY_6_ABBREV)
       llvm_unreachable("Unexpected abbrev ordering!");
   }
-
-
+  { // Blob VST_BBENTRY strings.
+    BitCodeAbbrev *Abbv = new BitCodeAbbrev();
+    Abbv->Add(BitCodeAbbrevOp(bitc::VST_CODE_BBENTRY));
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
+    if (Stream.EmitBlockInfoAbbrev(bitc::VALUE_SYMTAB_BLOCK_ID,
+                                   Abbv) != VST_BBENTRY_BLOB_ABBREV)
+      llvm_unreachable("Unexpected abbrev ordering!");
+  }
 
   { // SETTYPE abbrev for CONSTANTS_BLOCK.
     BitCodeAbbrev *Abbv = new BitCodeAbbrev();
     Abbv->Add(BitCodeAbbrevOp(bitc::CST_CODE_SETTYPE));
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed,
                               VE.computeBitsRequiredForTypeIndicies()));
     if (Stream.EmitBlockInfoAbbrev(bitc::CONSTANTS_BLOCK_ID,
                                    Abbv) != CONSTANTS_SETTYPE_ABBREV)

test/Bitcode/invalid.test

 
 INVALID-CODELENWIDTH: Malformed block
 
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-function-argument-type.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=INVALID-ARGUMENT-TYPE %s
 
 INVALID-ARGUMENT-TYPE: Invalid function argument type
 
-RUN: not llvm-dis -disable-output %p/Inputs/invalid-fixme-streaming-blob.bc 2>&1 | \
-RUN:   FileCheck --check-prefix=STREAMING-BLOB %s
-
-STREAMING-BLOB: getPointer in streaming memory objects not allowed
-
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-function-comdat-id.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=INVALID-FCOMDAT-ID %s
 
 INVALID-FCOMDAT-ID: Invalid function comdat ID
 
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-global-var-comdat-id.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=INVALID-GVCOMDAT-ID %s