Bitcode reader: Inline readAbbreviatedField in readRecord and move the enclosing loop in each case (NFC)

[mehdi_amini]

Summary

This make readRecord 20% faster, measured on an LTO build

[tejohnson]

Nice improvement. Do you know how much this improves the overall parse time or compile time?

lib/Bitcode/Reader/BitstreamReader.cpp

135

Similar loop unswitching optimization could be done here, but presumably not as hot. Consider implementing here too for consistency?

209

Move these into the default case in your new switch below. Otherwise essentially checking for the error case multiple times.

[mehdi_amini]

Take Teresa's comment into account.

To answer the question on the impact on the total time: my profile shows readRecord for ~3% for an LTO link and close to %10 for a ThinLTO link.

[tejohnson]

LGTM with fix noted below.

lib/Bitcode/Reader/BitstreamReader.cpp

147

ReadVBR64?

[mehdi_amini]

ReadVBR64(), good catch!

[mehdi_amini]

r262811

lib/Bitcode/Reader/BitstreamReader.cpp

if (Op.getEncoding() == BitCodeAbbrevOp::Array) {
       unsigned NumElts = ReadVBR(6);
 
       // Get the element encoding.
       assert(i+2 == e && "array op not second to last?");
       const BitCodeAbbrevOp &EltEnc = Abbv->getOperandInfo(++i);
 
       // Read all the elements.
       for (; NumElts; --NumElts)
         skipAbbreviatedField(*this, EltEnc);
       continue;
     }
 
     assert(Op.getEncoding() == BitCodeAbbrevOp::Blob);
     // Blob case.  Read the number of bytes as a vbr6.
     unsigned NumElts = ReadVBR(6);
     SkipToFourByteBoundary();  // 32-bit alignment
 
     // Figure out where the end of this blob will be including tail padding.
     size_t NewEnd = GetCurrentBitNo()+((NumElts+3)&~3)*8;
 
     // If this would read off the end of the bitcode file, just set the
     // record to empty and return.
     if (!canSkipToPos(NewEnd/8)) {
       NextChar = BitStream->getBitcodeBytes().getExtent();
       break;
     }
 
     // Skip over the blob.
     JumpToBit(NewEnd);
if (Op.getEncoding() == BitCodeAbbrevOp::Array) {
 
       // Get the element encoding.
       if (i + 2 != e)
         report_fatal_error("Array op not second to last");
       const BitCodeAbbrevOp &EltEnc = Abbv->getOperandInfo(++i);
       if (!EltEnc.isEncoding())
         report_fatal_error(
             "Array element type has to be an encoding of a type");
       if (EltEnc.getEncoding() == BitCodeAbbrevOp::Array ||
           EltEnc.getEncoding() == BitCodeAbbrevOp::Blob)
         report_fatal_error("Array element type can't be an Array or a Blob");
 
       // Read all the elements.
+      switch (EltEnc.getEncoding()) {
+        default:
+          llvm_unreachable("Unexpected case");
+        case BitCodeAbbrevOp::Fixed:
+          for (; NumElts; --NumElts)
+            Vals.push_back(Read((unsigned)EltEnc.getEncodingData()));
+          break;
+        case BitCodeAbbrevOp::VBR:
           for (; NumElts; --NumElts)
-        Vals.push_back(readAbbreviatedField(*this, EltEnc));
+            Vals.push_back(ReadVBR64((unsigned)EltEnc.getEncodingData()));
+          break;
+        case BitCodeAbbrevOp::Char6:
+          for (; NumElts; --NumElts)
+            Vals.push_back(BitCodeAbbrevOp::DecodeChar6(Read(6)));
+      }
       continue;
     }
 
     assert(Op.getEncoding() == BitCodeAbbrevOp::Blob);
     // Blob case.  Read the number of bytes as a vbr6.
     unsigned NumElts = ReadVBR(6);
     SkipToFourByteBoundary();  // 32-bit alignment