[CodeView Type Merging] Try hard not to deserialize records when re-writing type indices

[zturner]

Summary

A profile shows the majority of time doing type merging is spent deserializing records from sequences of bytes into friendly C++ structures that we can easily access members of in order to find the type indices to re-write.

Records are prefixed with their length, however, and most records have type indices that appear at fixed offsets in the record. For these records, we can save some cycles by just looking at the right place in the byte sequence and re-writing the value, then skipping the record in the type stream. This saves us from the costly deserialization of examining every field, including potentially null terminated strings which are the slowest, even though it was unnecessary to begin with.

In addition, we apply another optimization. Previously, after deserializing a record and re-writing its type indices, we would unconditionally re-serialize it in order to compute the hash of the re-written record. This would result in an alloc and memcpy for every record. If no type indices were re-written, however, this was an unnecessary allocation. In this patch re-writing is made two phase. The first phase discovers the indices that need to be rewritten and their new values. This information is passed through to the de-duplication code, which only copies and re-writes type indices in the serialized byte sequence if at least one type index is different.

Some records have type indices which only appear after variable length strings, or which have lists of type indices, or various other situations that can make it tricky to make this optimization. While I'm not giving up on optimizing these cases as well, for now we can get the easy cases out of the way and lay the groundwork for more complicated cases later.

This patch yields another 50% speedup on top of the already large speedups submitted over the past 2 days. In two tests I have run, I went from 9 seconds to 3 seconds, and from 16 seconds to 8 seconds.

[zturner]

Minor improvement. Store the temporary storage in the type hasher so that we don't re-allocate on every record, but only when a record is larger than any other record we've seen.

[rnk]

llvm/lib/DebugInfo/CodeView/TypeSerializer.cpp

71 ↗

(On Diff #100039)

There's no point in a type hasher that shouldn't hash. If we're not going to hash, let's just skip creation of TypeHasher. We can move SeenRecords back up to TypeSerializer. I only put it on TypeHasher so I wouldn't have to return if the value was inserted or not, but we might as well do that now.

[zturner]

Raise no-hash logic up out of the hasher.

[rnk]

lgtm

llvm/trunk/include/llvm/DebugInfo/CodeView/CVRecord.h

 //===- RecordIterator.h -----------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_DEBUGINFO_CODEVIEW_RECORDITERATOR_H
 #define LLVM_DEBUGINFO_CODEVIEW_RECORDITERATOR_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/DebugInfo/CodeView/CodeViewError.h"
 #include "llvm/DebugInfo/CodeView/RecordSerialization.h"
+#include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/Support/BinaryStreamReader.h"
 #include "llvm/Support/BinaryStreamRef.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include <cstdint>
 
 namespace llvm {
 
public:
 
   void setHash(uint32_t Value) { Hash = Value; }
 
   Kind Type;
   ArrayRef<uint8_t> RecordData;
   Optional<uint32_t> Hash;
 };
 
+template <typename Kind> struct RemappedRecord {
+  explicit RemappedRecord(const CVRecord<Kind> &R) : OriginalRecord(R) {}
+
+  CVRecord<Kind> OriginalRecord;
+  SmallVector<std::pair<uint32_t, TypeIndex>, 8> Mappings;
+};
+
 } // end namespace codeview
 
 template <typename Kind>
 struct VarStreamArrayExtractor<codeview::CVRecord<Kind>> {
   typedef void ContextType;
 
   static Error extract(BinaryStreamRef Stream, uint32_t &Len,
                        codeview::CVRecord<Kind> &Item) {

llvm/trunk/include/llvm/DebugInfo/CodeView/CVTypeVisitor.h

Error visitMemberRecord(CVMemberRecord Record, TypeVisitorCallbacks &Callbacks,
                         VisitorDataSource Source = VDS_BytesPresent);
 Error visitMemberRecord(TypeLeafKind Kind, ArrayRef<uint8_t> Record,
                         TypeVisitorCallbacks &Callbacks);
 
 Error visitMemberRecordStream(ArrayRef<uint8_t> FieldList,
                               TypeVisitorCallbacks &Callbacks);
 
 Error visitTypeStream(const CVTypeArray &Types, TypeVisitorCallbacks &Callbacks,
+                      VisitorDataSource Source = VDS_BytesPresent,
                       TypeServerHandler *TS = nullptr);
 Error visitTypeStream(CVTypeRange Types, TypeVisitorCallbacks &Callbacks,
                       TypeServerHandler *TS = nullptr);
 Error visitTypeStream(TypeCollection &Types, TypeVisitorCallbacks &Callbacks,
                       TypeServerHandler *TS = nullptr);
 
 } // end namespace codeview
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_CODEVIEW_CVTYPEVISITOR_H

llvm/trunk/include/llvm/DebugInfo/CodeView/TypeDeserializer.h

struct MappingInfo {
     BinaryByteStream Stream;
     BinaryStreamReader Reader;
     TypeRecordMapping Mapping;
   };
 
 public:
   TypeDeserializer() = default;
 
+  template <typename T> static Error deserializeAs(CVType &CVT, T &Record) {
+    MappingInfo I(CVT.content());
+    if (auto EC = I.Mapping.visitTypeBegin(CVT))
+      return EC;
+    if (auto EC = I.Mapping.visitKnownRecord(CVT, Record))
+      return EC;
+    if (auto EC = I.Mapping.visitTypeEnd(CVT))
+      return EC;
+    return Error::success();
+  }
+
   Error visitTypeBegin(CVType &Record) override {
     assert(!Mapping && "Already in a type mapping!");
     Mapping = llvm::make_unique<MappingInfo>(Record.content());
     return Mapping->Mapping.visitTypeBegin(Record);
   }
 
   Error visitTypeBegin(CVType &Record, TypeIndex Index) override {
     return visitTypeBegin(Record);

llvm/trunk/include/llvm/DebugInfo/CodeView/TypeRecord.h

 
 namespace codeview {
 
 using support::little32_t;
 using support::ulittle16_t;
 using support::ulittle32_t;
 
 typedef CVRecord<TypeLeafKind> CVType;
+typedef RemappedRecord<TypeLeafKind> RemappedType;
 
 struct CVMemberRecord {
   TypeLeafKind Kind;
   ArrayRef<uint8_t> Data;
 };
 typedef VarStreamArray<CVType> CVTypeArray;
 typedef iterator_range<CVTypeArray::Iterator> CVTypeRange;
 

llvm/trunk/include/llvm/DebugInfo/CodeView/TypeSerializer.h

class TypeSerializer : public TypeVisitorCallbacks {
   std::vector<uint8_t> RecordBuffer;
   MutableBinaryByteStream Stream;
   BinaryStreamWriter Writer;
   TypeRecordMapping Mapping;
 
   /// Private type record hashing implementation details are handled here.
   std::unique_ptr<TypeHasher> Hasher;
 
+  /// Contains a list of all records indexed by TypeIndex.toArrayIndex().
+  SmallVector<ArrayRef<uint8_t>, 2> SeenRecords;
+
+  /// Temporary storage that we use to copy a record's data while re-writing
+  /// its type indices.
+  SmallVector<uint8_t, 256> RemapStorage;
+
+  TypeIndex nextTypeIndex() const;
+
   bool isInFieldList() const;
   MutableArrayRef<uint8_t> getCurrentSubRecordData();
   MutableArrayRef<uint8_t> getCurrentRecordData();
   Error writeRecordPrefix(TypeLeafKind Kind);
 
   Expected<MutableArrayRef<uint8_t>>
   addPadding(MutableArrayRef<uint8_t> Record);
 
 public:
-  explicit TypeSerializer(BumpPtrAllocator &Storage);
+  explicit TypeSerializer(BumpPtrAllocator &Storage, bool Hash = true);
   ~TypeSerializer();
 
   ArrayRef<ArrayRef<uint8_t>> records() const;
-  TypeIndex insertRecordBytes(ArrayRef<uint8_t> Record);
+  TypeIndex insertRecordBytes(ArrayRef<uint8_t> &Record);
+  TypeIndex insertRecord(const RemappedType &Record);
   Expected<TypeIndex> visitTypeEndGetIndex(CVType &Record);
 
   Error visitTypeBegin(CVType &Record) override;
   Error visitTypeEnd(CVType &Record) override;
   Error visitMemberBegin(CVMemberRecord &Record) override;
   Error visitMemberEnd(CVMemberRecord &Record) override;
 
 #define TYPE_RECORD(EnumName, EnumVal, Name)                                   \

llvm/trunk/include/llvm/DebugInfo/CodeView/TypeTableBuilder.h

template <typename T> TypeIndex writeKnownType(T &Record) {
 
     return *ExpectedIndex;
   }
 
   TypeIndex writeSerializedRecord(ArrayRef<uint8_t> Record) {
     return Serializer.insertRecordBytes(Record);
   }
 
+  TypeIndex writeSerializedRecord(const RemappedType &Record) {
+    return Serializer.insertRecord(Record);
+  }
+
   template <typename TFunc> void ForEachRecord(TFunc Func) {
     uint32_t Index = TypeIndex::FirstNonSimpleIndex;
 
     for (auto Record : Serializer.records()) {
       Func(TypeIndex(Index), Record);
       ++Index;
     }
   }
 
   ArrayRef<ArrayRef<uint8_t>> records() const { return Serializer.records(); }
 };
 
 class FieldListRecordBuilder {
   TypeTableBuilder &TypeTable;
   BumpPtrAllocator Allocator;
   TypeSerializer TempSerializer;
   CVType Type;
 
 public:
   explicit FieldListRecordBuilder(TypeTableBuilder &TypeTable)
-      : TypeTable(TypeTable), TempSerializer(Allocator) {
+      : TypeTable(TypeTable), TempSerializer(Allocator, false) {
     Type.Type = TypeLeafKind::LF_FIELDLIST;
   }
 
   void begin() {
     if (auto EC = TempSerializer.visitTypeBegin(Type))
       consumeError(std::move(EC));
   }
 

llvm/trunk/lib/DebugInfo/CodeView/CVTypeVisitor.cpp

static Error visitKnownMember(CVMemberRecord &Record,
   TypeRecordKind RK = static_cast<TypeRecordKind>(Record.Kind);
   T KnownRecord(RK);
   if (auto EC = Callbacks.visitKnownMember(Record, KnownRecord))
     return EC;
   return Error::success();
 }
 
 static Expected<TypeServer2Record> deserializeTypeServerRecord(CVType &Record) {
-  class StealTypeServerVisitor : public TypeVisitorCallbacks {
-  public:
-    explicit StealTypeServerVisitor(TypeServer2Record &TR) : TR(TR) {}
-
-    Error visitKnownRecord(CVType &CVR, TypeServer2Record &Record) override {
-      TR = Record;
-      return Error::success();
-    }
-
-  private:
-    TypeServer2Record &TR;
-  };
-
   TypeServer2Record R(TypeRecordKind::TypeServer2);
-  StealTypeServerVisitor Thief(R);
-  if (auto EC = visitTypeRecord(Record, Thief))
+  if (auto EC = TypeDeserializer::deserializeAs(Record, R))
     return std::move(EC);
-
   return R;
 }
 
 static Error visitMemberRecord(CVMemberRecord &Record,
                                TypeVisitorCallbacks &Callbacks) {
   if (auto EC = Callbacks.visitMemberBegin(Record))
     return EC;
 
Error llvm::codeview::visitTypeRecord(CVType &Record,
   VisitHelper V(Callbacks, Source);
   if (TS)
     V.Visitor.addTypeServerHandler(*TS);
   return V.Visitor.visitTypeRecord(Record);
 }
 
 Error llvm::codeview::visitTypeStream(const CVTypeArray &Types,
                                       TypeVisitorCallbacks &Callbacks,
+                                      VisitorDataSource Source,
                                       TypeServerHandler *TS) {
-  VisitHelper V(Callbacks, VDS_BytesPresent);
+  VisitHelper V(Callbacks, Source);
   if (TS)
     V.Visitor.addTypeServerHandler(*TS);
   return V.Visitor.visitTypeStream(Types);
 }
 
 Error llvm::codeview::visitTypeStream(CVTypeRange Types,
                                       TypeVisitorCallbacks &Callbacks,
                                       TypeServerHandler *TS) {

llvm/trunk/lib/DebugInfo/CodeView/TypeSerializer.cpp

private:
 
   /// Hash table. We really want a DenseMap<ArrayRef<uint8_t>, TypeIndex> here,
   /// but DenseMap is inefficient when the keys are long (like type records)
   /// because it recomputes the hash value of every key when it grows. This
   /// value type stores the hash out of line in KeyStorage, so that table
   /// entries are small and easy to rehash.
   DenseSet<HashedTypePtr> HashedRecords;
 
-  SmallVector<ArrayRef<uint8_t>, 2> SeenRecords;
-
-  TypeIndex NextTypeIndex = TypeIndex(TypeIndex::FirstNonSimpleIndex);
-
 public:
   TypeHasher(BumpPtrAllocator &RecordStorage) : RecordStorage(RecordStorage) {}
 
-  ArrayRef<ArrayRef<uint8_t>> records() const { return SeenRecords; }
-
   /// Takes the bytes of type record, inserts them into the hash table, saves
   /// them, and returns a pointer to an identical stable type record along with
   /// its type index in the destination stream.
-  TypeIndex getOrCreateRecord(ArrayRef<uint8_t> &Record);
+  TypeIndex getOrCreateRecord(ArrayRef<uint8_t> &Record, TypeIndex TI);
 };
 
-TypeIndex TypeHasher::getOrCreateRecord(ArrayRef<uint8_t> &Record) {
+TypeIndex TypeHasher::getOrCreateRecord(ArrayRef<uint8_t> &Record,
+                                        TypeIndex TI) {
   assert(Record.size() < UINT32_MAX && "Record too big");
   assert(Record.size() % 4 == 0 && "Record is not aligned to 4 bytes!");
 
   // Compute the hash up front so we can store it in the key.
   HashedType TempHashedType = {hash_value(Record), Record.data(),
-                               unsigned(Record.size()), NextTypeIndex};
-
+                               unsigned(Record.size()), TI};
   auto Result = HashedRecords.insert(HashedTypePtr(&TempHashedType));
   HashedType *&Hashed = Result.first->Ptr;
 
   if (Result.second) {
     // This was a new type record. We need stable storage for both the key and
     // the record. The record should outlive the hashing operation.
     Hashed = KeyStorage.Allocate<HashedType>();
     *Hashed = TempHashedType;
 
     uint8_t *Stable = RecordStorage.Allocate<uint8_t>(Record.size());
     memcpy(Stable, Record.data(), Record.size());
     Hashed->Data = Stable;
     assert(Hashed->Size == Record.size());
-
-    // This was a new record, so increment our next type index.
-    ++NextTypeIndex;
   }
 
   // Update the caller's copy of Record to point a stable copy.
   Record = ArrayRef<uint8_t>(Hashed->Data, Hashed->Size);
-
-  if (Result.second) {
-    // FIXME: Can we record these in a more efficient way?
-    SeenRecords.push_back(Record);
-  }
+  return Hashed->Index;
 
-  return TypeIndex(Hashed->Index);
+TypeIndex TypeSerializer::nextTypeIndex() const {
+  return TypeIndex::fromArrayIndex(SeenRecords.size());
 }
 
 bool TypeSerializer::isInFieldList() const {
   return TypeKind.hasValue() && *TypeKind == TypeLeafKind::LF_FIELDLIST;
 }
 
 MutableArrayRef<uint8_t> TypeSerializer::getCurrentSubRecordData() {
   assert(isInFieldList());
while (PaddingBytes > 0) {
     uint8_t Pad = static_cast<uint8_t>(LF_PAD0 + PaddingBytes);
     if (auto EC = Writer.writeInteger(Pad))
       return std::move(EC);
     --PaddingBytes;
   }
   return MutableArrayRef<uint8_t>(Record.data(), Record.size() + N);
 }
 
-TypeSerializer::TypeSerializer(BumpPtrAllocator &Storage)
+TypeSerializer::TypeSerializer(BumpPtrAllocator &Storage, bool Hash)
     : RecordStorage(Storage), RecordBuffer(MaxRecordLength * 2),
       Stream(RecordBuffer, llvm::support::little), Writer(Stream),
-      Mapping(Writer), Hasher(make_unique<TypeHasher>(Storage)) {
+      Mapping(Writer) {
   // RecordBuffer needs to be able to hold enough data so that if we are 1
   // byte short of MaxRecordLen, and then we try to write MaxRecordLen bytes,
   // we won't overflow.
+  if (Hash)
+    Hasher = make_unique<TypeHasher>(Storage);
 }
 
 TypeSerializer::~TypeSerializer() = default;
 
 ArrayRef<ArrayRef<uint8_t>> TypeSerializer::records() const {
-  return Hasher->records();
+  return SeenRecords;
+}
+
+TypeIndex TypeSerializer::insertRecordBytes(ArrayRef<uint8_t> &Record) {
+  assert(!TypeKind.hasValue() && "Already in a type mapping!");
+  assert(Writer.getOffset() == 0 && "Stream has data already!");
+
+  if (Hasher) {
+    TypeIndex ActualTI = Hasher->getOrCreateRecord(Record, nextTypeIndex());
+    if (nextTypeIndex() == ActualTI)
+      SeenRecords.push_back(Record);
+    return ActualTI;
   }
 
-TypeIndex TypeSerializer::insertRecordBytes(ArrayRef<uint8_t> Record) {
+  TypeIndex NewTI = nextTypeIndex();
+  uint8_t *Stable = RecordStorage.Allocate<uint8_t>(Record.size());
+  memcpy(Stable, Record.data(), Record.size());
+  Record = ArrayRef<uint8_t>(Stable, Record.size());
+  SeenRecords.push_back(Record);
+  return NewTI;
+}
+
+TypeIndex TypeSerializer::insertRecord(const RemappedType &Record) {
   assert(!TypeKind.hasValue() && "Already in a type mapping!");
   assert(Writer.getOffset() == 0 && "Stream has data already!");
 
-  return Hasher->getOrCreateRecord(Record);
+  TypeIndex TI;
+  ArrayRef<uint8_t> OriginalData = Record.OriginalRecord.RecordData;
+  if (Record.Mappings.empty()) {
+    // This record did not remap any type indices.  Just write it.
+    return insertRecordBytes(OriginalData);
+  }
+
+  // At least one type index was remapped.  Before we can hash it we have to
+  // copy the full record bytes, re-write each type index, then hash the copy.
+  // We do this in temporary storage since only the DenseMap can decide whether
+  // this record already exists, and if it does we don't want the memory to
+  // stick around.
+  RemapStorage.resize(OriginalData.size());
+  ::memcpy(&RemapStorage[0], OriginalData.data(), OriginalData.size());
+  uint8_t *ContentBegin = RemapStorage.data() + sizeof(RecordPrefix);
+  for (const auto &M : Record.Mappings) {
+    // First 4 bytes of every record are the record prefix, but the mapping
+    // offset is relative to the content which starts after.
+    *(TypeIndex *)(ContentBegin + M.first) = M.second;
+  }
+  auto RemapRef = makeArrayRef(RemapStorage);
+  return insertRecordBytes(RemapRef);
 }
 
 Error TypeSerializer::visitTypeBegin(CVType &Record) {
   assert(!TypeKind.hasValue() && "Already in a type mapping!");
   assert(Writer.getOffset() == 0 && "Stream has data already!");
 
   if (auto EC = writeRecordPrefix(Record.kind()))
     return EC;
Expected<TypeIndex> TypeSerializer::visitTypeEndGetIndex(CVType &Record) {
   ThisRecordData = *ExpectedData;
 
   RecordPrefix *Prefix =
       reinterpret_cast<RecordPrefix *>(ThisRecordData.data());
   Prefix->RecordLen = ThisRecordData.size() - sizeof(uint16_t);
 
   Record.Type = *TypeKind;
   Record.RecordData = ThisRecordData;
-  TypeIndex InsertedTypeIndex = Hasher->getOrCreateRecord(Record.RecordData);
+
+  // insertRecordBytes assumes we're not in a mapping, so do this first.
+  TypeKind.reset();
+  Writer.setOffset(0);
+
+  TypeIndex InsertedTypeIndex = insertRecordBytes(Record.RecordData);
 
   // Write out each additional segment in reverse order, and update each
   // record's continuation index to point to the previous one.
   for (auto X : reverse(FieldListSegments)) {
     auto CIBytes = X.take_back(sizeof(uint32_t));
     support::ulittle32_t *CI =
         reinterpret_cast<support::ulittle32_t *>(CIBytes.data());
     assert(*CI == 0xB0C0B0C0 && "Invalid TypeIndex placeholder");
     *CI = InsertedTypeIndex.getIndex();
-    InsertedTypeIndex = Hasher->getOrCreateRecord(X);
+    InsertedTypeIndex = insertRecordBytes(X);
   }
 
-  TypeKind.reset();
-  Writer.setOffset(0);
   FieldListSegments.clear();
   CurrentSegment.SubRecords.clear();
 
   return InsertedTypeIndex;
 }
 
 Error TypeSerializer::visitTypeEnd(CVType &Record) {
   auto ExpectedIndex = visitTypeEndGetIndex(Record);

llvm/trunk/lib/DebugInfo/CodeView/TypeStreamMerger.cpp

 //===-- TypeStreamMerger.cpp ------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/CodeView/TypeStreamMerger.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
+#include "llvm/DebugInfo/CodeView/TypeDeserializer.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeTableBuilder.h"
 #include "llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 using namespace llvm;

 private:
   Error doit(const CVTypeArray &Types);
 
   void addMapping(TypeIndex Idx);
 
   bool remapTypeIndex(TypeIndex &Idx);
   bool remapItemIndex(TypeIndex &Idx);
 
+  bool remapIndices(RemappedType &Record, ArrayRef<uint32_t> TidOffs,
+                    ArrayRef<uint32_t> IidOffs) {
+    auto OriginalData = Record.OriginalRecord.content();
+    bool Success = true;
+    for (auto Off : TidOffs) {
+      ArrayRef<uint8_t> Bytes = OriginalData.slice(Off, sizeof(TypeIndex));
+      TypeIndex OldTI(
+          *reinterpret_cast<const support::ulittle32_t *>(Bytes.data()));
+      TypeIndex NewTI = OldTI;
+      bool ThisSuccess = remapTypeIndex(NewTI);
+      if (ThisSuccess && NewTI != OldTI)
+        Record.Mappings.emplace_back(Off, NewTI);
+      Success &= ThisSuccess;
+    }
+    for (auto Off : IidOffs) {
+      ArrayRef<uint8_t> Bytes = OriginalData.slice(Off, sizeof(TypeIndex));
+      TypeIndex OldTI(
+          *reinterpret_cast<const support::ulittle32_t *>(Bytes.data()));
+      TypeIndex NewTI = OldTI;
+      bool ThisSuccess = remapItemIndex(NewTI);
+      if (ThisSuccess && NewTI != OldTI)
+        Record.Mappings.emplace_back(Off, NewTI);
+      Success &= ThisSuccess;
+    }
+    return Success;
+  }
+
   bool remapIndex(TypeIndex &Idx, ArrayRef<TypeIndex> Map);
 
   size_t slotForIndex(TypeIndex Idx) const {
     assert(!Idx.isSimple() && "simple type indices have no slots");
     return Idx.getIndex() - TypeIndex::FirstNonSimpleIndex;
   }
 
   Error errorCorruptRecord() const {
     return llvm::make_error<CodeViewError>(cv_error_code::corrupt_record);
   }
 
   template <typename RecordType>
-  Error writeRecord(RecordType &R, bool RemapSuccess) {
+  Error writeKnownRecord(TypeTableBuilder &Dest, RecordType &R,
+                         bool RemapSuccess) {
     TypeIndex DestIdx = Untranslated;
     if (RemapSuccess)
-      DestIdx = DestTypeStream->writeKnownType(R);
+      DestIdx = Dest.writeKnownType(R);
     addMapping(DestIdx);
     return Error::success();
   }
 
   template <typename RecordType>
-  Error writeIdRecord(RecordType &R, bool RemapSuccess) {
+  Error writeKnownTypeRecord(RecordType &R, bool RemapSuccess) {
+    return writeKnownRecord(*DestTypeStream, R, RemapSuccess);
+  }
+
+  template <typename RecordType>
+  Error writeKnownIdRecord(RecordType &R, bool RemapSuccess) {
+    return writeKnownRecord(*DestIdStream, R, RemapSuccess);
+  }
+
+  Error writeRecord(TypeTableBuilder &Dest, const RemappedType &Record,
+                    bool RemapSuccess) {
     TypeIndex DestIdx = Untranslated;
     if (RemapSuccess)
-      DestIdx = DestIdStream->writeKnownType(R);
+      DestIdx = Dest.writeSerializedRecord(Record);
+    addMapping(DestIdx);
+    return Error::success();
+  }
+
+  Error writeTypeRecord(const CVType &Record) {
+    TypeIndex DestIdx =
+        DestTypeStream->writeSerializedRecord(Record.RecordData);
     addMapping(DestIdx);
     return Error::success();
   }
 
+  Error writeTypeRecord(const RemappedType &Record, bool RemapSuccess) {
+    return writeRecord(*DestTypeStream, Record, RemapSuccess);
+  }
+
+  Error writeIdRecord(const RemappedType &Record, bool RemapSuccess) {
+    return writeRecord(*DestIdStream, Record, RemapSuccess);
+  }
+
   template <typename RecordType>
   Error writeMember(RecordType &R, bool RemapSuccess) {
     if (RemapSuccess)
       FieldListBuilder->writeMemberType(R);
     else
       HadUntranslatedMember = true;
     return Error::success();
   }
private:
   /// type index minus 0x1000.
   SmallVectorImpl<TypeIndex> &IndexMap;
 };
 
 } // end anonymous namespace
 
 const TypeIndex TypeStreamMerger::Untranslated(SimpleTypeKind::NotTranslated);
 
-Error TypeStreamMerger::visitTypeBegin(CVRecord<TypeLeafKind> &Rec) {
-  return Error::success();
-}
+Error TypeStreamMerger::visitTypeBegin(CVType &Rec) { return Error::success(); }
 
-Error TypeStreamMerger::visitTypeEnd(CVRecord<TypeLeafKind> &Rec) {
-  CurIndex = TypeIndex(CurIndex.getIndex() + 1);
+Error TypeStreamMerger::visitTypeEnd(CVType &Rec) {
+  ++CurIndex;
   if (!IsSecondPass)
     assert(IndexMap.size() == slotForIndex(CurIndex) &&
            "visitKnownRecord should add one index map entry");
   return Error::success();
 }
 
 Error TypeStreamMerger::visitMemberEnd(CVMemberRecord &Rec) {
   return Error::success();
bool TypeStreamMerger::remapItemIndex(TypeIndex &Idx) {
   assert(DestIdStream);
   return remapIndex(Idx, IndexMap);
 }
 
 //----------------------------------------------------------------------------//
 // Item records
 //----------------------------------------------------------------------------//
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, FuncIdRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, FuncIdRecord &R) {
   assert(DestIdStream);
-  bool Success = true;
-  Success &= remapItemIndex(R.ParentScope);
-  Success &= remapTypeIndex(R.FunctionType);
-  return writeIdRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeIdRecord(RR, remapIndices(RR, {4}, {0}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, MemberFuncIdRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, MemberFuncIdRecord &R) {
   assert(DestIdStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.ClassType);
-  Success &= remapTypeIndex(R.FunctionType);
-  return writeIdRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeIdRecord(RR, remapIndices(RR, {0, 4}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, StringIdRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, StringIdRecord &R) {
   assert(DestIdStream);
-  return writeIdRecord(R, remapItemIndex(R.Id));
+
+  RemappedType RR(CVR);
+  return writeIdRecord(RR, remapIndices(RR, {}, {0}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, StringListRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, StringListRecord &R) {
   assert(DestIdStream);
+
+  if (auto EC = TypeDeserializer::deserializeAs<StringListRecord>(CVR, R))
+    return EC;
   bool Success = true;
-  for (TypeIndex &Str : R.StringIndices)
-    Success &= remapItemIndex(Str);
-  return writeIdRecord(R, Success);
+
+  for (TypeIndex &Id : R.StringIndices)
+    Success &= remapItemIndex(Id);
+  return writeKnownIdRecord(R, Success);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, BuildInfoRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, BuildInfoRecord &R) {
   assert(DestIdStream);
+
+  if (auto EC = TypeDeserializer::deserializeAs(CVR, R))
+    return EC;
+
   bool Success = true;
-  for (TypeIndex &Arg : R.ArgIndices)
-    Success &= remapItemIndex(Arg);
-  return writeIdRecord(R, Success);
+  for (TypeIndex &Str : R.ArgIndices)
+    Success &= remapItemIndex(Str);
+  return writeKnownIdRecord(R, Success);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, UdtSourceLineRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, UdtSourceLineRecord &R) {
   assert(DestIdStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.UDT);
-  Success &= remapItemIndex(R.SourceFile);
+
+  RemappedType RR(CVR);
+
   // FIXME: Translate UdtSourceLineRecord into UdtModSourceLineRecords in the
   // IPI stream.
-  return writeIdRecord(R, Success);
+  return writeIdRecord(RR, remapIndices(RR, {0}, {4}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, UdtModSourceLineRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR,
+                                         UdtModSourceLineRecord &R) {
   assert(DestIdStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.UDT);
-  // UdtModSourceLine Source File Ids are offsets into the global string table.
+
+  RemappedType RR(CVR);
+
+  // UdtModSourceLine Source File Ids are offsets into the global string table,
+  // not type indices.
   // FIXME: We need to merge string table records for this to be valid.
-  // Success &= remapItemIndex(R.SourceFile);
-  return writeIdRecord(R, Success);
+  return writeIdRecord(RR, remapIndices(RR, {0}, {}));
 }
 
 //----------------------------------------------------------------------------//
 // Type records
 //----------------------------------------------------------------------------//
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, ModifierRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, ModifierRecord &R) {
   assert(DestTypeStream);
-  return writeRecord(R, remapTypeIndex(R.ModifiedType));
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {0}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, ProcedureRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, ProcedureRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.ReturnType);
-  Success &= remapTypeIndex(R.ArgumentList);
-  return writeRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {0, 8}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, MemberFunctionRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, MemberFunctionRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.ReturnType);
-  Success &= remapTypeIndex(R.ClassType);
-  Success &= remapTypeIndex(R.ThisType);
-  Success &= remapTypeIndex(R.ArgumentList);
-  return writeRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {0, 4, 8, 16}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &Type, ArgListRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, ArgListRecord &R) {
   assert(DestTypeStream);
+
+  if (auto EC = TypeDeserializer::deserializeAs(CVR, R))
+    return EC;
+
   bool Success = true;
   for (TypeIndex &Arg : R.ArgIndices)
     Success &= remapTypeIndex(Arg);
-  if (auto EC = writeRecord(R, Success))
-    return EC;
-  return Error::success();
+
+  return writeKnownTypeRecord(R, Success);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, PointerRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, PointerRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.ReferentType);
+
+  // Pointer records have a different number of TypeIndex mappings depending
+  // on whether or not it is a pointer to member.
+  if (auto EC = TypeDeserializer::deserializeAs(CVR, R))
+    return EC;
+
+  bool Success = remapTypeIndex(R.ReferentType);
   if (R.isPointerToMember())
     Success &= remapTypeIndex(R.MemberInfo->ContainingType);
-  return writeRecord(R, Success);
+  return writeKnownTypeRecord(R, Success);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, ArrayRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, ArrayRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.ElementType);
-  Success &= remapTypeIndex(R.IndexType);
-  return writeRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {0, 4}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, ClassRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, ClassRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.FieldList);
-  Success &= remapTypeIndex(R.DerivationList);
-  Success &= remapTypeIndex(R.VTableShape);
-  return writeRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {4, 8, 12}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, UnionRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, UnionRecord &R) {
   assert(DestTypeStream);
-  return writeRecord(R, remapTypeIndex(R.FieldList));
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {4}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, EnumRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, EnumRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.FieldList);
-  Success &= remapTypeIndex(R.UnderlyingType);
-  return writeRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {4, 8}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, BitFieldRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, BitFieldRecord &R) {
   assert(DestTypeStream);
-  return writeRecord(R, remapTypeIndex(R.Type));
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {0}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, VFTableShapeRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, VFTableShapeRecord &R) {
   assert(DestTypeStream);
-  return writeRecord(R, true);
+
+  return writeTypeRecord(CVR);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, TypeServer2Record &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, TypeServer2Record &R) {
   assert(DestTypeStream);
-  return writeRecord(R, true);
+
+  return writeTypeRecord(CVR);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, LabelRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, LabelRecord &R) {
   assert(DestTypeStream);
-  return writeRecord(R, true);
+
+  return writeTypeRecord(CVR);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, VFTableRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, VFTableRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.CompleteClass);
-  Success &= remapTypeIndex(R.OverriddenVFTable);
-  return writeRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {0, 4}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &,
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR,
                                          MethodOverloadListRecord &R) {
   assert(DestTypeStream);
+
+  if (auto EC = TypeDeserializer::deserializeAs(CVR, R))
+    return EC;
+
   bool Success = true;
   for (OneMethodRecord &Meth : R.Methods)
     Success &= remapTypeIndex(Meth.Type);
-  return writeRecord(R, Success);
+  return writeKnownTypeRecord(R, Success);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, FieldListRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, FieldListRecord &R) {
   assert(DestTypeStream);
   // Visit the members inside the field list.
   HadUntranslatedMember = false;
   FieldListBuilder = llvm::make_unique<FieldListRecordBuilder>(*DestTypeStream);
 
   FieldListBuilder->begin();
-  if (auto EC = codeview::visitMemberRecordStream(R.Data, *this))
+  if (auto EC = codeview::visitMemberRecordStream(CVR.content(), *this))
     return EC;
 
   // Write the record if we translated all field list members.
   TypeIndex DestIdx = Untranslated;
   if (!HadUntranslatedMember)
     DestIdx = FieldListBuilder->end();
   else
     FieldListBuilder->reset();
Error TypeStreamMerger::mergeTypesAndIds(TypeTableBuilder &DestIds,
   DestTypeStream = &DestTypes;
 
   return doit(IdsAndTypes);
 }
 
 Error TypeStreamMerger::doit(const CVTypeArray &Types) {
   LastError = Error::success();
 
-  if (auto EC = codeview::visitTypeStream(Types, *this, Handler))
+  // We don't want to deserialize records.  I guess this flag is poorly named,
+  // but it really means "Don't deserialize records before switching on the
+  // concrete type.
+  if (auto EC =
+          codeview::visitTypeStream(Types, *this, VDS_BytesExternal, Handler))
     return EC;
 
   // If we found bad indices but no other errors, try doing another pass and see
   // if we can resolve the indices that weren't in the map on the first pass.
   // This may require multiple passes, but we should always make progress. MASM
   // is the only known CodeView producer that makes type streams that aren't
   // topologically sorted. The standard library contains MASM-produced objects,
   // so this is important to handle correctly, but we don't have to be too
   // efficient. MASM type streams are usually very small.
   while (!*LastError && NumBadIndices > 0) {
     unsigned BadIndicesRemaining = NumBadIndices;
     IsSecondPass = true;
     NumBadIndices = 0;
     CurIndex = TypeIndex(TypeIndex::FirstNonSimpleIndex);
 
-    if (auto EC = codeview::visitTypeStream(Types, *this, Handler))
+    if (auto EC =
+            codeview::visitTypeStream(Types, *this, VDS_BytesExternal, Handler))
       return EC;
 
     assert(NumBadIndices <= BadIndicesRemaining &&
            "second pass found more bad indices");
     if (!*LastError && NumBadIndices == BadIndicesRemaining) {
       return llvm::make_error<CodeViewError>(
           cv_error_code::corrupt_record, "input type graph contains cycles");
     }

llvm/trunk/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp

 
 Expected<bool>
 PDBTypeServerHandler::handleInternal(PDBFile &File,
                                      TypeVisitorCallbacks &Callbacks) {
   auto ExpectedTpi = File.getPDBTpiStream();
   if (!ExpectedTpi)
     return ExpectedTpi.takeError();
 
-  if (auto EC = codeview::visitTypeStream(ExpectedTpi->typeArray(), Callbacks))
+  // For handling a type server, we should be using whatever the callback array
+  // was
+  // that is being used for the original file.  We shouldn't allow the visitor
+  // to
+  // arbitrarily stick a deserializer in there.
+  if (auto EC = codeview::visitTypeStream(ExpectedTpi->typeArray(), Callbacks,
+                                          VDS_BytesExternal))
     return std::move(EC);
 
   return true;
 }
 
 Expected<bool> PDBTypeServerHandler::handle(TypeServer2Record &TS,
                                             TypeVisitorCallbacks &Callbacks) {
   if (Session) {