[WIP] [Bitstream] take Expected<T> off critical reading paths

[sammccall]

Summary

BitstreamReader can encounter many types of error and returns Expected<T> to
signal them. This is rich & typesafe, though adds some overhead.

Unfortunately this technique is used for functions as low-level as "read an
n-bit integer" which are called frequently, and the overhead adds up.

This patch tries to avoid this:

• next to low-level functions that return Expected (e.g. Read), adds a lower-level ReadE which returns the value, and signals errors out of band.
• ReadE sets a "last error" object on the stream which needs to be checked
• it turns out there are cases where we can safely elide/combine some checks, simplifying control flow
• Read is still available with the same signature, but implemented on ReadE.
• ReadRecord is converted to use ReadE and friends
• to avoid inlining a bunch of "makeStringError" formatting into hot functions, we convert the errors used to a custom error type which is cheaper.

A motivating use case: clangd builds a clang PCH ones and then reuses it for
many reparses of the main file. This patch speeds up those reparses by ~7%
(tested with clang/lib/Sema/SemaOverload.cpp and clangd/AST.cpp).

[sammccall]

This is very much a prototype, hopefully the idea is clear.

Would appreciate some feedback on:

• is this a reasonable/comprehensible error-handling scheme, does it introduce too much tech debt for the performance win etc
• whether to keep the E versions of the functions private, whether to make both versions public, or whether to completely replace the old versions
• better naming

Profile before

Profile after

The workload is clangd --check=AST.cpp, modified to rebuild the AST 1000 times, it's roughly 2/3rds buildAST.
There's about 4% savings clearly visible, and 4% divided by 2/3rs is close to the 7% speedup I saw in the logs.

I would expect this to generalize to other PCH/modules use.

llvm/include/llvm/Bitstream/BitCodes.h

public:
   }
 
   static char DecodeChar6(unsigned V) {
     assert((V & ~63) == 0 && "Not a Char6 encoded character!");
     return "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789._"
         [V];
   }
 
+  static char DecodeChar6Tolerant(unsigned V) {
+    return DecodeChar6(V & ~unsigned(63));
+  }
 };
 
 /// BitCodeAbbrev - This class represents an abbreviation record.  An
 /// abbreviation allows a complex record that has redundancy to be stored in a
 /// specialized format instead of the fully-general, fully-vbr, format.
 class BitCodeAbbrev {
   SmallVector<BitCodeAbbrevOp, 32> OperandList;
 

llvm/include/llvm/Bitstream/BitstreamReader.h

BlockInfo &getOrCreateBlockInfo(unsigned BlockID) {
 
     // Otherwise, add a new record.
     BlockInfoRecords.emplace_back();
     BlockInfoRecords.back().BlockID = BlockID;
     return BlockInfoRecords.back();
   }
 };
 
+class BitstreamError : public ErrorInfo<BitstreamError> {
+public:
+  enum ErrKind : unsigned {
+    UnexpectedEofBits,
+    UnexpectedEofBytes,
+    SkipBlockAtEof,
+    BadSkip,
+    UnterminatedVBR,
+    BadAbbrev,
+  };
+  static char ID;
+
+  BitstreamError() = default;
+  BitstreamError(ErrKind Kind, uint64_t Arg1 = 0, uint64_t Arg2 = 0)
+      : Kind(Kind){};
+
+  void log(raw_ostream &OS) const override;
+  std::error_code convertToErrorCode() const override;
+
+private:
+  ErrKind Kind;
+  size_t Arg1, Arg2;
+};
+
 /// This represents a position within a bitstream. There may be multiple
 /// independent cursors reading within one bitstream, each maintaining their
 /// own local state.
 class SimpleBitstreamCursor {
   ArrayRef<uint8_t> BitcodeBytes;
   size_t NextChar = 0;
 
 public:
   /// This is the current data we have pulled from the stream but have not
   /// returned to the client. This is specifically and intentionally defined to
   /// follow the word size of the host machine for efficiency. We use word_t in
   /// places that are aware of this to make it perfectly explicit what is going
   /// on.
   using word_t = size_t;
 
 private:
   word_t CurWord = 0;
 
   /// This is the number of bits in CurWord that are valid. This is always from
   /// [0...bits_of(size_t)-1] inclusive.
   unsigned BitsInCurWord = 0;
 
+protected:
+  llvm::Optional<BitstreamError> Err;
+  llvm::Error err() {
+    assert(Err);
+    auto E = make_error<BitstreamError>(*Err);
+    Err.reset();
+    return E;
+  }
+
 public:
   SimpleBitstreamCursor() = default;
   explicit SimpleBitstreamCursor(ArrayRef<uint8_t> BitcodeBytes)
       : BitcodeBytes(BitcodeBytes) {}
   explicit SimpleBitstreamCursor(StringRef BitcodeBytes)
       : BitcodeBytes(arrayRefFromStringRef(BitcodeBytes)) {}
   explicit SimpleBitstreamCursor(MemoryBufferRef BitcodeBytes)
       : SimpleBitstreamCursor(BitcodeBytes.getBuffer()) {}
public:
 
   // Return the byte # of the current bit.
   uint64_t getCurrentByteNo() const { return GetCurrentBitNo() / 8; }
 
   ArrayRef<uint8_t> getBitcodeBytes() const { return BitcodeBytes; }
 
   /// Reset the stream to the specified bit number.
   Error JumpToBit(uint64_t BitNo) {
+    JumpToBitE(BitNo);
+    if (Err)
+      return err();
+    return Error::success();
+  }
+  void JumpToBitE(uint64_t BitNo) {
     size_t ByteNo = size_t(BitNo/8) & ~(sizeof(word_t)-1);
     unsigned WordBitNo = unsigned(BitNo & (sizeof(word_t)*8-1));
     assert(canSkipToPos(ByteNo) && "Invalid location");
 
     // Move the cursor to the right word.
     NextChar = ByteNo;
     BitsInCurWord = 0;
 
     // Skip over any bits that are already consumed.
-    if (WordBitNo) {
-      if (Expected<word_t> Res = Read(WordBitNo))
-        return Error::success();
-      else
-        return Res.takeError();
-    }
-
-    return Error::success();
+    if (WordBitNo)
+      ReadE(WordBitNo);
   }
 
   /// Get a pointer into the bitstream at the specified byte offset.
   const uint8_t *getPointerToByte(uint64_t ByteNo, uint64_t NumBytes) {
     return BitcodeBytes.data() + ByteNo;
   }
 
   /// Get a pointer into the bitstream at the specified bit offset.
   ///
   /// The bit offset must be on a byte boundary.
   const uint8_t *getPointerToBit(uint64_t BitNo, uint64_t NumBytes) {
     assert(!(BitNo % 8) && "Expected bit on byte boundary");
     return getPointerToByte(BitNo / 8, NumBytes);
   }
 
-  Error fillCurWord() {
-    if (NextChar >= BitcodeBytes.size())
-      return createStringError(std::errc::io_error,
-                               "Unexpected end of file reading %u of %u bytes",
-                               NextChar, BitcodeBytes.size());
+  void fillCurWordE() {
+    if (NextChar >= BitcodeBytes.size()) {
+      Err.emplace(BitstreamError::UnexpectedEofBytes, NextChar,
+                  BitcodeBytes.size());
+      return;
+    }
 
     // Read the next word from the stream.
     const uint8_t *NextCharPtr = BitcodeBytes.data() + NextChar;
     unsigned BytesRead;
     if (BitcodeBytes.size() >= NextChar + sizeof(word_t)) {
       BytesRead = sizeof(word_t);
       CurWord =
           support::endian::read<word_t, support::little, support::unaligned>(
               NextCharPtr);
     } else {
       // Short read.
       BytesRead = BitcodeBytes.size() - NextChar;
       CurWord = 0;
       for (unsigned B = 0; B != BytesRead; ++B)
         CurWord |= uint64_t(NextCharPtr[B]) << (B * 8);
     }
     NextChar += BytesRead;
     BitsInCurWord = BytesRead * 8;
-    return Error::success();
   }
 
   Expected<word_t> Read(unsigned NumBits) {
+    auto Ret = ReadE(NumBits);
+    if (Err)
+      return err();
+    return Ret;
+  }
+  word_t ReadE(unsigned NumBits) {
     static const unsigned BitsInWord = sizeof(word_t) * 8;
 
     assert(NumBits && NumBits <= BitsInWord &&
            "Cannot return zero or more than BitsInWord bits!");
 
     static const unsigned Mask = sizeof(word_t) > 4 ? 0x3f : 0x1f;
 
     // If the field is fully contained by CurWord, return it quickly.
     if (BitsInCurWord >= NumBits) {
       word_t R = CurWord & (~word_t(0) >> (BitsInWord - NumBits));
 
       // Use a mask to avoid undefined behavior.
       CurWord >>= (NumBits & Mask);
 
       BitsInCurWord -= NumBits;
       return R;
     }
 
     word_t R = BitsInCurWord ? CurWord : 0;
     unsigned BitsLeft = NumBits - BitsInCurWord;
 
-    if (Error fillResult = fillCurWord())
-      return std::move(fillResult);
-
-    // If we run out of data, abort.
-    if (BitsLeft > BitsInCurWord)
-      return createStringError(std::errc::io_error,
-                               "Unexpected end of file reading %u of %u bits",
-                               BitsInCurWord, BitsLeft);
+    fillCurWordE();
+    if (!Err && BitsLeft > BitsInCurWord)
+      Err.emplace(BitstreamError::UnexpectedEofBits, BitsInCurWord, BitsLeft);
+    // Don't bother branching for errors, just return garbage with Err set.
 
     word_t R2 = CurWord & (~word_t(0) >> (BitsInWord - BitsLeft));
 
     // Use a mask to avoid undefined behavior.
     CurWord >>= (BitsLeft & Mask);
 
     BitsInCurWord -= BitsLeft;
 
     R |= R2 << (NumBits - BitsLeft);
 
     return R;
   }
 
   Expected<uint32_t> ReadVBR(const unsigned NumBits) {
-    Expected<unsigned> MaybeRead = Read(NumBits);
-    if (!MaybeRead)
-      return MaybeRead;
-    uint32_t Piece = MaybeRead.get();
+    auto Ret = ReadVBRE(NumBits);
+    if (Err)
+      return err();
+    return Ret;
+  }
+  uint32_t ReadVBRE(const unsigned NumBits) {
+    uint32_t Piece = ReadE(NumBits);
+    // Error checked in loop below.
 
     assert(NumBits <= 32 && NumBits >= 1 && "Invalid NumBits value");
     const uint32_t MaskBitOrder = (NumBits - 1);
     const uint32_t Mask = 1UL << MaskBitOrder;
 
     if ((Piece & Mask) == 0)
       return Piece;
 
     uint32_t Result = 0;
     unsigned NextBit = 0;
-    while (true) {
+    while (!Err) {
       Result |= (Piece & (Mask - 1)) << NextBit;
 
       if ((Piece & Mask) == 0)
         return Result;
 
       NextBit += NumBits-1;
       if (NextBit >= 32)
-        return createStringError(std::errc::illegal_byte_sequence,
-                                 "Unterminated VBR");
-
-      MaybeRead = Read(NumBits);
-      if (!MaybeRead)
-        return MaybeRead;
-      Piece = MaybeRead.get();
+        Err.emplace(BitstreamError::UnterminatedVBR);
+      else
+        Piece = ReadE(NumBits);
     }
+    return 0; // error
   }
 
   // Read a VBR that may have a value up to 64-bits in size. The chunk size of
   // the VBR must still be <= 32 bits though.
   Expected<uint64_t> ReadVBR64(const unsigned NumBits) {
-    Expected<uint64_t> MaybeRead = Read(NumBits);
-    if (!MaybeRead)
-      return MaybeRead;
-    uint32_t Piece = MaybeRead.get();
+    auto Ret = ReadVBR64E(NumBits);
+    if (Err)
+      return err();
+    return Ret;
+  }
+  uint64_t ReadVBR64E(const unsigned NumBits) {
+    uint64_t Piece = ReadE(NumBits);
     assert(NumBits <= 32 && NumBits >= 1 && "Invalid NumBits value");
     const uint32_t MaskBitOrder = (NumBits - 1);
     const uint32_t Mask = 1UL << MaskBitOrder;
 
     if ((Piece & Mask) == 0)
       return uint64_t(Piece);
 
     uint64_t Result = 0;
     unsigned NextBit = 0;
-    while (true) {
+    while (!Err) {
       Result |= uint64_t(Piece & (Mask - 1)) << NextBit;
 
       if ((Piece & Mask) == 0)
         return Result;
 
       NextBit += NumBits-1;
       if (NextBit >= 64)
-        return createStringError(std::errc::illegal_byte_sequence,
-                                 "Unterminated VBR");
-
-      MaybeRead = Read(NumBits);
-      if (!MaybeRead)
-        return MaybeRead;
-      Piece = MaybeRead.get();
+        Err.emplace(BitstreamError::UnterminatedVBR);
+      else
+        Piece = ReadE(NumBits);
     }
+    return 0; // error
   }
 
   void SkipToFourByteBoundary() {
     // If word_t is 64-bits and if we've read less than 32 bits, just dump
     // the bits we have up to the next 32-bit boundary.
     if (sizeof(word_t) > 4 &&
         BitsInCurWord >= 32) {
       CurWord >>= BitsInCurWord-32;
explicit BitstreamCursor(ArrayRef<uint8_t> BitcodeBytes)
       : SimpleBitstreamCursor(BitcodeBytes) {}
   explicit BitstreamCursor(StringRef BitcodeBytes)
       : SimpleBitstreamCursor(BitcodeBytes) {}
   explicit BitstreamCursor(MemoryBufferRef BitcodeBytes)
       : SimpleBitstreamCursor(BitcodeBytes) {}
 
   using SimpleBitstreamCursor::AtEndOfStream;
   using SimpleBitstreamCursor::canSkipToPos;
-  using SimpleBitstreamCursor::fillCurWord;
   using SimpleBitstreamCursor::getBitcodeBytes;
   using SimpleBitstreamCursor::GetCurrentBitNo;
   using SimpleBitstreamCursor::getCurrentByteNo;
   using SimpleBitstreamCursor::getPointerToByte;
   using SimpleBitstreamCursor::JumpToBit;
   using SimpleBitstreamCursor::Read;
   using SimpleBitstreamCursor::ReadVBR;
   using SimpleBitstreamCursor::ReadVBR64;
public:
 
   /// Having read the ENTER_SUBBLOCK code, read the BlockID for the block.
   Expected<unsigned> ReadSubBlockID() { return ReadVBR(bitc::BlockIDWidth); }
 
   /// Having read the ENTER_SUBBLOCK abbrevid and a BlockID, skip over the body
   /// of this block.
   Error SkipBlock() {
     // Read and ignore the codelen value.
-    if (Expected<uint32_t> Res = ReadVBR(bitc::CodeLenWidth))
-      ; // Since we are skipping this block, we don't care what code widths are
-        // used inside of it.
-    else
-      return Res.takeError();
+    ReadVBRE(bitc::CodeLenWidth);
+    // Since we are skipping this block, we don't care what code widths are
+    if (Err)
+      return err();
 
     SkipToFourByteBoundary();
-    Expected<unsigned> MaybeNum = Read(bitc::BlockSizeWidth);
-    if (!MaybeNum)
-      return MaybeNum.takeError();
-    size_t NumFourBytes = MaybeNum.get();
+    size_t NumFourBytes = ReadE(bitc::BlockSizeWidth);
+    if (Err)
+      return err();
 
     // Check that the block wasn't partially defined, and that the offset isn't
     // bogus.
     size_t SkipTo = GetCurrentBitNo() + NumFourBytes * 4 * 8;
     if (AtEndOfStream())
-      return createStringError(std::errc::illegal_byte_sequence,
-                               "can't skip block: already at end of stream");
+      return make_error<BitstreamError>(BitstreamError::SkipBlockAtEof);
     if (!canSkipToPos(SkipTo / 8))
-      return createStringError(std::errc::illegal_byte_sequence,
-                               "can't skip to bit %zu from %" PRIu64, SkipTo,
+      return make_error<BitstreamError>(BitstreamError::BadSkip, SkipTo,
                                         GetCurrentBitNo());
 
-    if (Error Res = JumpToBit(SkipTo))
-      return Res;
+    JumpToBitE(SkipTo);
+    if (Err)
+      return err();
 
     return Error::success();
   }
 
   /// Having read the ENTER_SUBBLOCK abbrevid, and enter the block.
   Error EnterSubBlock(unsigned BlockID, unsigned *NumWordsP = nullptr);
 
   bool ReadBlockEnd() {

 private:
   void popBlockScope() {
     CurCodeSize = BlockScope.back().PrevCodeSize;
 
     CurAbbrevs = std::move(BlockScope.back().PrevAbbrevs);
     BlockScope.pop_back();
   }
 
+  uint64_t readAbbreviatedFieldE(const BitCodeAbbrevOp &Op);
   //===--------------------------------------------------------------------===//
   // Record Processing
   //===--------------------------------------------------------------------===//
 
 public:
   /// Return the abbreviation for the specified AbbrevId.
   Expected<const BitCodeAbbrev *> getAbbrev(unsigned AbbrevID) {
     unsigned AbbrevNo = AbbrevID - bitc::FIRST_APPLICATION_ABBREV;
     if (AbbrevNo >= CurAbbrevs.size())
-      return createStringError(
-          std::errc::illegal_byte_sequence, "Invalid abbrev number");
+      return make_error<BitstreamError>(BitstreamError::BadAbbrev);
     return CurAbbrevs[AbbrevNo].get();
   }
 
   /// Read the current record and discard it, returning the code for the record.
   Expected<unsigned> skipRecord(unsigned AbbrevID);
 
   Expected<unsigned> readRecord(unsigned AbbrevID,
                                 SmallVectorImpl<uint64_t> &Vals,

llvm/lib/Bitstream/Reader/BitstreamReader.cpp

Error BitstreamCursor::EnterSubBlock(unsigned BlockID, unsigned *NumWordsP) {
   if (BlockInfo) {
     if (const BitstreamBlockInfo::BlockInfo *Info =
             BlockInfo->getBlockInfo(BlockID)) {
       llvm::append_range(CurAbbrevs, Info->Abbrevs);
     }
   }
 
   // Get the codesize of this block.
-  Expected<uint32_t> MaybeVBR = ReadVBR(bitc::CodeLenWidth);
-  if (!MaybeVBR)
-    return MaybeVBR.takeError();
-  CurCodeSize = MaybeVBR.get();
-
+  CurCodeSize = ReadVBRE(bitc::CodeLenWidth);
+  if (Err)
+    return err();
   if (CurCodeSize > MaxChunkSize)
     return llvm::createStringError(
         std::errc::illegal_byte_sequence,
         "can't read more than %zu at a time, trying to read %u", +MaxChunkSize,
         CurCodeSize);
 
   SkipToFourByteBoundary();
-  Expected<word_t> MaybeNum = Read(bitc::BlockSizeWidth);
-  if (!MaybeNum)
-    return MaybeNum.takeError();
-  word_t NumWords = MaybeNum.get();
+  word_t NumWords = ReadE(bitc::BlockSizeWidth);
   if (NumWordsP)
     *NumWordsP = NumWords;
 
+  if (Err)
+    return err();
   if (CurCodeSize == 0)
     return llvm::createStringError(
         std::errc::illegal_byte_sequence,
         "can't enter sub-block: current code size is 0");
   if (AtEndOfStream())
     return llvm::createStringError(
         std::errc::illegal_byte_sequence,
         "can't enter sub block: already at end of stream");
 
   return Error::success();
 }
 
-static Expected<uint64_t> readAbbreviatedField(BitstreamCursor &Cursor,
-                                               const BitCodeAbbrevOp &Op) {
+uint64_t BitstreamCursor::readAbbreviatedFieldE(const BitCodeAbbrevOp &Op) {
   assert(!Op.isLiteral() && "Not to be used with literals!");
 
   // Decode the value as we are commanded.
   switch (Op.getEncoding()) {
   case BitCodeAbbrevOp::Array:
   case BitCodeAbbrevOp::Blob:
     llvm_unreachable("Should not reach here");
   case BitCodeAbbrevOp::Fixed:
-    assert((unsigned)Op.getEncodingData() <= Cursor.MaxChunkSize);
-    return Cursor.Read((unsigned)Op.getEncodingData());
+    assert((unsigned)Op.getEncodingData() <= MaxChunkSize);
+    return ReadE((unsigned)Op.getEncodingData());
   case BitCodeAbbrevOp::VBR:
-    assert((unsigned)Op.getEncodingData() <= Cursor.MaxChunkSize);
-    return Cursor.ReadVBR64((unsigned)Op.getEncodingData());
+    assert((unsigned)Op.getEncodingData() <= MaxChunkSize);
+    return ReadVBR64E((unsigned)Op.getEncodingData());
   case BitCodeAbbrevOp::Char6:
-    if (Expected<unsigned> Res = Cursor.Read(6))
-      return BitCodeAbbrevOp::DecodeChar6(Res.get());
-    else
-      return Res.takeError();
+    return BitCodeAbbrevOp::DecodeChar6Tolerant(ReadE(6));
   }
   llvm_unreachable("invalid abbreviation encoding");
 }
 
 /// skipRecord - Read the current record and discard it.
 Expected<unsigned> BitstreamCursor::skipRecord(unsigned AbbrevID) {
   // Skip unabbreviated records by reading past their entries.
   if (AbbrevID == bitc::UNABBREV_RECORD) {
-    Expected<uint32_t> MaybeCode = ReadVBR(6);
-    if (!MaybeCode)
-      return MaybeCode.takeError();
-    unsigned Code = MaybeCode.get();
-    Expected<uint32_t> MaybeVBR = ReadVBR(6);
-    if (!MaybeVBR)
-      return MaybeVBR.takeError();
-    unsigned NumElts = MaybeVBR.get();
-    for (unsigned i = 0; i != NumElts; ++i)
-      if (Expected<uint64_t> Res = ReadVBR64(6))
-        ; // Skip!
-      else
-        return Res.takeError();
+    unsigned Code = ReadVBRE(6);
+    unsigned NumElts = ReadVBRE(6);
+    for (unsigned i = 0; i != NumElts && !Err; ++i)
+      ReadVBR64E(6);
+    if (Err)
+      return err();
     return Code;
   }
 
   Expected<const BitCodeAbbrev *> MaybeAbbv = getAbbrev(AbbrevID);
   if (!MaybeAbbv)
     return MaybeAbbv.takeError();
 
   const BitCodeAbbrev *Abbv = MaybeAbbv.get();
   const BitCodeAbbrevOp &CodeOp = Abbv->getOperandInfo(0);
   unsigned Code;
   if (CodeOp.isLiteral())
     Code = CodeOp.getLiteralValue();
   else {
     if (CodeOp.getEncoding() == BitCodeAbbrevOp::Array ||
         CodeOp.getEncoding() == BitCodeAbbrevOp::Blob)
       return llvm::createStringError(
           std::errc::illegal_byte_sequence,
           "Abbreviation starts with an Array or a Blob");
-    Expected<uint64_t> MaybeCode = readAbbreviatedField(*this, CodeOp);
-    if (!MaybeCode)
-      return MaybeCode.takeError();
-    Code = MaybeCode.get();
+    Code = readAbbreviatedFieldE(CodeOp);
+    if (Err)
+      return err();
   }
 
-  for (unsigned i = 1, e = Abbv->getNumOperandInfos(); i < e; ++i) {
+  for (unsigned i = 1, e = Abbv->getNumOperandInfos(); i < e && !Err; ++i) {
     const BitCodeAbbrevOp &Op = Abbv->getOperandInfo(i);
     if (Op.isLiteral())
       continue;
 
     if (Op.getEncoding() != BitCodeAbbrevOp::Array &&
         Op.getEncoding() != BitCodeAbbrevOp::Blob) {
-      if (Expected<uint64_t> MaybeField = readAbbreviatedField(*this, Op))
-        continue;
-      else
-        return MaybeField.takeError();
+      readAbbreviatedFieldE(Op);
     }
 
     if (Op.getEncoding() == BitCodeAbbrevOp::Array) {
       // Array case.  Read the number of elements as a vbr6.
-      Expected<uint32_t> MaybeNum = ReadVBR(6);
-      if (!MaybeNum)
-        return MaybeNum.takeError();
-      unsigned NumElts = MaybeNum.get();
+      unsigned NumElts = ReadVBRE(6);
+      if (Err)
+        return err();
 
       // Get the element encoding.
       assert(i+2 == e && "array op not second to last?");
       const BitCodeAbbrevOp &EltEnc = Abbv->getOperandInfo(++i);
 
       // Read all the elements.
       // Decode the value as we are commanded.
       switch (EltEnc.getEncoding()) {
       default:
         return error("Array element type can't be an Array or a Blob");
       case BitCodeAbbrevOp::Fixed:
         assert((unsigned)EltEnc.getEncodingData() <= MaxChunkSize);
-        if (Error Err =
-                JumpToBit(GetCurrentBitNo() + static_cast<uint64_t>(NumElts) *
-                                                  EltEnc.getEncodingData()))
-          return std::move(Err);
+        JumpToBitE(GetCurrentBitNo() +
+                   static_cast<uint64_t>(NumElts) * EltEnc.getEncodingData());
         break;
       case BitCodeAbbrevOp::VBR:
         assert((unsigned)EltEnc.getEncodingData() <= MaxChunkSize);
-        for (; NumElts; --NumElts)
-          if (Expected<uint64_t> Res =
-                  ReadVBR64((unsigned)EltEnc.getEncodingData()))
-            ; // Skip!
-          else
-            return Res.takeError();
+        for (; NumElts && !Err; --NumElts)
+          ReadVBR64E((unsigned)EltEnc.getEncodingData()); // Skip!
         break;
       case BitCodeAbbrevOp::Char6:
-        if (Error Err = JumpToBit(GetCurrentBitNo() + NumElts * 6))
-          return std::move(Err);
+        JumpToBitE(GetCurrentBitNo() + NumElts * 6);
         break;
       }
       continue;
     }
 
     assert(Op.getEncoding() == BitCodeAbbrevOp::Blob);
     // Blob case.  Read the number of bytes as a vbr6.
-    Expected<uint32_t> MaybeNum = ReadVBR(6);
-    if (!MaybeNum)
-      return MaybeNum.takeError();
-    unsigned NumElts = MaybeNum.get();
+    unsigned NumElts = ReadVBRE(6);
+    if (Err)
+      return err();
     SkipToFourByteBoundary();  // 32-bit alignment
 
     // Figure out where the end of this blob will be including tail padding.
     const size_t NewEnd = GetCurrentBitNo() + alignTo(NumElts, 4) * 8;
 
     // If this would read off the end of the bitcode file, just set the
     // record to empty and return.
     if (!canSkipToPos(NewEnd/8)) {
       skipToEnd();
       break;
     }
 
     // Skip over the blob.
-    if (Error Err = JumpToBit(NewEnd))
-      return std::move(Err);
+    JumpToBitE(NewEnd);
   }
+  if (Err)
+    return err();
   return Code;
 }
 
 Expected<unsigned> BitstreamCursor::readRecord(unsigned AbbrevID,
                                                SmallVectorImpl<uint64_t> &Vals,
                                                StringRef *Blob) {
   if (AbbrevID == bitc::UNABBREV_RECORD) {
-    Expected<uint32_t> MaybeCode = ReadVBR(6);
-    if (!MaybeCode)
-      return MaybeCode.takeError();
-    uint32_t Code = MaybeCode.get();
-    Expected<uint32_t> MaybeNumElts = ReadVBR(6);
-    if (!MaybeNumElts)
-      return error(
-          ("Failed to read size: " + toString(MaybeNumElts.takeError()))
-              .c_str());
-    uint32_t NumElts = MaybeNumElts.get();
+    uint32_t Code = ReadVBRE(6);
+    if (Err)
+      return err();
+    uint32_t NumElts = ReadVBRE(6);
+    if (Err)
+      return error(("Failed to read size: " + toString(err())).c_str());
     if (!isSizePlausible(NumElts))
       return error("Size is not plausible");
     Vals.reserve(Vals.size() + NumElts);
 
-    for (unsigned i = 0; i != NumElts; ++i)
-      if (Expected<uint64_t> MaybeVal = ReadVBR64(6))
-        Vals.push_back(MaybeVal.get());
-      else
-        return MaybeVal.takeError();
+    for (unsigned i = 0; i != NumElts; ++i) {
+      Vals.push_back(ReadVBR64E(6));
+      if (Err)
+        return err();
+    }
     return Code;
   }
 
   Expected<const BitCodeAbbrev *> MaybeAbbv = getAbbrev(AbbrevID);
   if (!MaybeAbbv)
     return MaybeAbbv.takeError();
   const BitCodeAbbrev *Abbv = MaybeAbbv.get();
 
   // Read the record code first.
   assert(Abbv->getNumOperandInfos() != 0 && "no record code in abbreviation?");
   const BitCodeAbbrevOp &CodeOp = Abbv->getOperandInfo(0);
   unsigned Code;
   if (CodeOp.isLiteral())
     Code = CodeOp.getLiteralValue();
   else {
     if (CodeOp.getEncoding() == BitCodeAbbrevOp::Array ||
         CodeOp.getEncoding() == BitCodeAbbrevOp::Blob)
       return error("Abbreviation starts with an Array or a Blob");
-    if (Expected<uint64_t> MaybeCode = readAbbreviatedField(*this, CodeOp))
-      Code = MaybeCode.get();
-    else
-      return MaybeCode.takeError();
+    Code = readAbbreviatedFieldE(CodeOp);
+    if (Err)
+      return err();
   }
 
   for (unsigned i = 1, e = Abbv->getNumOperandInfos(); i != e; ++i) {
     const BitCodeAbbrevOp &Op = Abbv->getOperandInfo(i);
     if (Op.isLiteral()) {
       Vals.push_back(Op.getLiteralValue());
       continue;
     }
 
     if (Op.getEncoding() != BitCodeAbbrevOp::Array &&
         Op.getEncoding() != BitCodeAbbrevOp::Blob) {
-      if (Expected<uint64_t> MaybeVal = readAbbreviatedField(*this, Op))
-        Vals.push_back(MaybeVal.get());
-      else
-        return MaybeVal.takeError();
+      Vals.push_back(readAbbreviatedFieldE(Op));
       continue;
     }
 
     if (Op.getEncoding() == BitCodeAbbrevOp::Array) {
       // Array case.  Read the number of elements as a vbr6.
-      Expected<uint32_t> MaybeNumElts = ReadVBR(6);
-      if (!MaybeNumElts)
-        return error(
-            ("Failed to read size: " + toString(MaybeNumElts.takeError()))
-                .c_str());
-      uint32_t NumElts = MaybeNumElts.get();
+      uint32_t NumElts = ReadVBRE(6);
+      if (Err)
+        return error(("Failed to read size: " + toString(err())).c_str());
       if (!isSizePlausible(NumElts))
         return error("Size is not plausible");
       Vals.reserve(Vals.size() + NumElts);
 
       // Get the element encoding.
       if (i + 2 != e)
         return error("Array op not second to last");
       const BitCodeAbbrevOp &EltEnc = Abbv->getOperandInfo(++i);
       if (!EltEnc.isEncoding())
         return error(
             "Array element type has to be an encoding of a type");
 
       // Read all the elements.
       switch (EltEnc.getEncoding()) {
       default:
         return error("Array element type can't be an Array or a Blob");
       case BitCodeAbbrevOp::Fixed:
-        for (; NumElts; --NumElts)
-          if (Expected<SimpleBitstreamCursor::word_t> MaybeVal =
-                  Read((unsigned)EltEnc.getEncodingData()))
-            Vals.push_back(MaybeVal.get());
-          else
-            return MaybeVal.takeError();
+        for (; NumElts && !Err; --NumElts)
+          Vals.push_back(ReadE((unsigned)EltEnc.getEncodingData()));
         break;
       case BitCodeAbbrevOp::VBR:
-        for (; NumElts; --NumElts)
-          if (Expected<uint64_t> MaybeVal =
-                  ReadVBR64((unsigned)EltEnc.getEncodingData()))
-            Vals.push_back(MaybeVal.get());
-          else
-            return MaybeVal.takeError();
+        for (; NumElts && !Err; --NumElts)
+          Vals.push_back(ReadVBR64E((unsigned)EltEnc.getEncodingData()));
         break;
       case BitCodeAbbrevOp::Char6:
-        for (; NumElts; --NumElts)
-          if (Expected<SimpleBitstreamCursor::word_t> MaybeVal = Read(6))
-            Vals.push_back(BitCodeAbbrevOp::DecodeChar6(MaybeVal.get()));
-          else
-            return MaybeVal.takeError();
+        for (; NumElts && !Err; --NumElts)
+          Vals.push_back(BitCodeAbbrevOp::DecodeChar6Tolerant(ReadE(6)));
       }
       continue;
     }
 
     assert(Op.getEncoding() == BitCodeAbbrevOp::Blob);
     // Blob case.  Read the number of bytes as a vbr6.
-    Expected<uint32_t> MaybeNumElts = ReadVBR(6);
-    if (!MaybeNumElts)
-      return MaybeNumElts.takeError();
-    uint32_t NumElts = MaybeNumElts.get();
+    uint32_t NumElts = ReadVBRE(6);
+    if (Err)
+      return err();
     SkipToFourByteBoundary();  // 32-bit alignment
 
     // Figure out where the end of this blob will be including tail padding.
     size_t CurBitPos = GetCurrentBitNo();
     const size_t NewEnd = CurBitPos + alignTo(NumElts, 4) * 8;
 
     // Make sure the bitstream is large enough to contain the blob.
     if (!canSkipToPos(NewEnd/8))
       return error("Blob ends too soon");
 
     // Otherwise, inform the streamer that we need these bytes in memory.  Skip
     // over tail padding first, in case jumping to NewEnd invalidates the Blob
     // pointer.
-    if (Error Err = JumpToBit(NewEnd))
-      return std::move(Err);
+    JumpToBitE(NewEnd);
+    if (Err)
+      return err();
     const char *Ptr = (const char *)getPointerToBit(CurBitPos, NumElts);
 
     // If we can return a reference to the data, do so to avoid copying it.
     if (Blob) {
       *Blob = StringRef(Ptr, NumElts);
     } else {
       // Otherwise, unpack into Vals with zero extension.
       auto *UPtr = reinterpret_cast<const unsigned char *>(Ptr);
       Vals.append(UPtr, UPtr + NumElts);
     }
   }
+  if (Err)
+    return err();
 
   return Code;
 }
 
 Error BitstreamCursor::ReadAbbrevRecord() {
   auto Abbv = std::make_shared<BitCodeAbbrev>();
-  Expected<uint32_t> MaybeNumOpInfo = ReadVBR(5);
-  if (!MaybeNumOpInfo)
-    return MaybeNumOpInfo.takeError();
-  unsigned NumOpInfo = MaybeNumOpInfo.get();
-  for (unsigned i = 0; i != NumOpInfo; ++i) {
-    Expected<word_t> MaybeIsLiteral = Read(1);
-    if (!MaybeIsLiteral)
-      return MaybeIsLiteral.takeError();
-    bool IsLiteral = MaybeIsLiteral.get();
+  uint32_t NumOpInfo = ReadVBRE(5);
+  for (unsigned i = 0; i != NumOpInfo && !Err; ++i) {
+    bool IsLiteral = ReadE(1);
+    if (Err)
+      break;
     if (IsLiteral) {
-      Expected<uint64_t> MaybeOp = ReadVBR64(8);
-      if (!MaybeOp)
-        return MaybeOp.takeError();
-      Abbv->Add(BitCodeAbbrevOp(MaybeOp.get()));
+      Abbv->Add(BitCodeAbbrevOp(ReadVBR64E(8)));
       continue;
     }
 
-    Expected<word_t> MaybeEncoding = Read(3);
-    if (!MaybeEncoding)
-      return MaybeEncoding.takeError();
-    if (!BitCodeAbbrevOp::isValidEncoding(MaybeEncoding.get()))
+    word_t MaybeEncoding = ReadE(3);
+    if (Err)
+      return err();
+    if (!BitCodeAbbrevOp::isValidEncoding(MaybeEncoding))
       return error("Invalid encoding");
 
     BitCodeAbbrevOp::Encoding E =
-        (BitCodeAbbrevOp::Encoding)MaybeEncoding.get();
+        (BitCodeAbbrevOp::Encoding)MaybeEncoding;
     if (BitCodeAbbrevOp::hasEncodingData(E)) {
-      Expected<uint64_t> MaybeData = ReadVBR64(5);
-      if (!MaybeData)
-        return MaybeData.takeError();
-      uint64_t Data = MaybeData.get();
+      uint64_t Data = ReadVBR64E(5);
 
       // As a special case, handle fixed(0) (i.e., a fixed field with zero bits)
       // and vbr(0) as a literal zero.  This is decoded the same way, and avoids
       // a slow path in Read() to have to handle reading zero bits.
       if ((E == BitCodeAbbrevOp::Fixed || E == BitCodeAbbrevOp::VBR) &&
           Data == 0) {
         Abbv->Add(BitCodeAbbrevOp(0));
         continue;
       }
 
       if ((E == BitCodeAbbrevOp::Fixed || E == BitCodeAbbrevOp::VBR) &&
-          Data > MaxChunkSize)
+          Data > MaxChunkSize && !Err)
         return error("Fixed or VBR abbrev record with size > MaxChunkData");
 
       Abbv->Add(BitCodeAbbrevOp(E, Data));
     } else
       Abbv->Add(BitCodeAbbrevOp(E));
   }
+  if (Err)
+    return err();
 
   if (Abbv->getNumOperandInfos() == 0)
     return error("Abbrev record with no operands");
   CurAbbrevs.push_back(std::move(Abbv));
 
   return Error::success();
 }
 
case bitc::BLOCKINFO_CODE_BLOCKNAME: {
           break; // Ignore name.
         CurBlockInfo->RecordNames.emplace_back(
             (unsigned)Record[0], std::string(Record.begin() + 1, Record.end()));
         break;
       }
       }
   }
 }
+
+void llvm::BitstreamError::log(raw_ostream &OS) const {
+  switch (Kind) {
+  case UnexpectedEofBits:
+  case UnexpectedEofBytes:
+      OS << "Unexpected end of file reading " << Arg1 << " of " << Arg2
+         << (Kind == UnexpectedEofBits ? " bits" : " bytes");
+      break;
+  case SkipBlockAtEof:
+      OS << "can't skip block: already at end of stream";
+      break;
+  case BadSkip:
+      OS << "can't skip to bit " << Arg1 << " from " << Arg2;
+      break;
+  case UnterminatedVBR:
+      OS << "Unterminated VBR";
+      break;
+  case BadAbbrev:
+      OS << "Invalid abbrev number";
+      break;
+  }
+}
+
+std::error_code llvm::BitstreamError::convertToErrorCode() const {
+  switch (Kind) {
+  case UnexpectedEofBits:
+  case UnexpectedEofBytes:
+      return std::make_error_code(std::errc::io_error);
+  case SkipBlockAtEof:
+  case BadAbbrev:
+  case BadSkip:
+  case UnterminatedVBR:
+      return std::make_error_code(std::errc::illegal_byte_sequence);
+  }
+}
+
+char llvm::BitstreamError::ID;