Make abbreviations explicit in pnacl-freeze/thaw.

[1] Explicitly enumerate all abbreviation values, including the
maximum abbreviation for each type of block.

[2] Make "enter subblock" calculate number of bits needed by passing
in maximum abbreviation (associated with block) rather than requiring
the developer to compute this value every time a subblock is entered.

*NOTE* This code changes encoding sizes to be based on
the maximum allowed value, rather than requiring the
developer to calculate out the number of bits needed. This
change doesn't make  the PNaCL bitcode files
incompatable with LLVM bitcode files, since it does
not effect the bitcode reader.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3405
R=jvoung@chromium.org

Review URL: https://codereview.chromium.org/14813032

4 files changed, 199 insertions, 92 deletions

diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
index 5317e11a27..df93c1b4de 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
@@ -8,6 +8,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+#define DEBUG_TYPE "NaClBitcodeReader"
+
 #include "llvm/Bitcode/NaCl/NaClReaderWriter.h"
 #include "NaClBitcodeReader.h"
 #include "llvm/ADT/SmallString.h"
@@ -20,6 +22,7 @@
 #include "llvm/IR/Module.h"
 #include "llvm/IR/OperandTraits.h"
 #include "llvm/IR/Operator.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/DataStream.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -450,6 +453,7 @@ static void decodeLLVMAttributesForBitcode(AttrBuilder &B,
 }
 
 bool NaClBitcodeReader::ParseAttributeBlock() {
+  DEBUG(dbgs() << "-> ParseAttributeBlock\n");
   if (Stream.EnterSubBlock(naclbitc::PARAMATTR_BLOCK_ID))
     return Error("Malformed block record");
 
@@ -469,6 +473,7 @@ bool NaClBitcodeReader::ParseAttributeBlock() {
     case NaClBitstreamEntry::Error:
       return Error("Error at end of PARAMATTR block");
     case NaClBitstreamEntry::EndBlock:
+      DEBUG(dbgs() << "<- ParseAttributeBlock\n");
       return false;
     case NaClBitstreamEntry::Record:
       // The interesting case.
@@ -508,6 +513,7 @@ bool NaClBitcodeReader::ParseAttributeBlock() {
 }
 
 bool NaClBitcodeReader::ParseAttributeGroupBlock() {
+  DEBUG(dbgs() << "-> ParseAttributeGroupBlock\n");
   if (Stream.EnterSubBlock(naclbitc::PARAMATTR_GROUP_BLOCK_ID))
     return Error("Malformed block record");
 
@@ -525,6 +531,7 @@ bool NaClBitcodeReader::ParseAttributeGroupBlock() {
     case NaClBitstreamEntry::Error:
       return Error("Error at end of PARAMATTR_GROUP block");
     case NaClBitstreamEntry::EndBlock:
+      DEBUG(dbgs() << "<- ParseAttributeGroupBlock\n");
       return false;
     case NaClBitstreamEntry::Record:
       // The interesting case.
@@ -584,10 +591,14 @@ bool NaClBitcodeReader::ParseAttributeGroupBlock() {
 }
 
 bool NaClBitcodeReader::ParseTypeTable() {
+  DEBUG(dbgs() << "-> ParseTypeTable\n");
   if (Stream.EnterSubBlock(naclbitc::TYPE_BLOCK_ID_NEW))
     return Error("Malformed block record");
 
-  return ParseTypeTableBody();
+  bool result = ParseTypeTableBody();
+  if (!result)
+    DEBUG(dbgs() << "<- ParseTypeTable\n");
+  return result;
 }
 
 bool NaClBitcodeReader::ParseTypeTableBody() {
@@ -810,6 +821,7 @@ bool NaClBitcodeReader::ParseTypeTableBody() {
 }
 
 bool NaClBitcodeReader::ParseValueSymbolTable() {
+  DEBUG(dbgs() << "-> ParseValueSymbolTable\n");
   if (Stream.EnterSubBlock(naclbitc::VALUE_SYMTAB_BLOCK_ID))
     return Error("Malformed block record");
 
@@ -825,6 +837,7 @@ bool NaClBitcodeReader::ParseValueSymbolTable() {
     case NaClBitstreamEntry::Error:
       return Error("malformed value symbol table block");
     case NaClBitstreamEntry::EndBlock:
+      DEBUG(dbgs() << "<- ParseValueSymbolTable\n");
       return false;
     case NaClBitstreamEntry::Record:
       // The interesting case.
@@ -866,6 +879,7 @@ bool NaClBitcodeReader::ParseValueSymbolTable() {
 bool NaClBitcodeReader::ParseMetadata() {
   unsigned NextMDValueNo = MDValueList.size();
 
+  DEBUG(dbgs() << "-> ParseMetadata\n");
   if (Stream.EnterSubBlock(naclbitc::METADATA_BLOCK_ID))
     return Error("Malformed block record");
 
@@ -881,6 +895,7 @@ bool NaClBitcodeReader::ParseMetadata() {
       Error("malformed metadata block");
       return true;
     case NaClBitstreamEntry::EndBlock:
+      DEBUG(dbgs() << "<- ParseMetadata\n");
       return false;
     case NaClBitstreamEntry::Record:
       // The interesting case.
@@ -961,17 +976,6 @@ bool NaClBitcodeReader::ParseMetadata() {
   }
 }
 
-/// decodeSignRotatedValue - Decode a signed value stored with the sign bit in
-/// the LSB for dense VBR encoding.
-uint64_t NaClBitcodeReader::decodeSignRotatedValue(uint64_t V) {
-  if ((V & 1) == 0)
-    return V >> 1;
-  if (V != 1)
-    return -(V >> 1);
-  // There is no such thing as -0 with integers.  "-0" really means MININT.
-  return 1ULL << 63;
-}
-
 /// ResolveGlobalAndAliasInits - Resolve all of the initializers for global
 /// values and aliases that we can.
 bool NaClBitcodeReader::ResolveGlobalAndAliasInits() {
@@ -1013,12 +1017,13 @@ bool NaClBitcodeReader::ResolveGlobalAndAliasInits() {
 static APInt ReadWideAPInt(ArrayRef<uint64_t> Vals, unsigned TypeBits) {
   SmallVector<uint64_t, 8> Words(Vals.size());
   std::transform(Vals.begin(), Vals.end(), Words.begin(),
-                 NaClBitcodeReader::decodeSignRotatedValue);
+                 NaClDecodeSignRotatedValue);
 
   return APInt(TypeBits, Words);
 }
 
 bool NaClBitcodeReader::ParseConstants() {
+  DEBUG(dbgs() << "-> ParseConstants\n");
   if (Stream.EnterSubBlock(naclbitc::CONSTANTS_BLOCK_ID))
     return Error("Malformed block record");
 
@@ -1041,6 +1046,7 @@ bool NaClBitcodeReader::ParseConstants() {
       // Once all the constants have been read, go through and resolve forward
       // references.
       ValueList.ResolveConstantForwardRefs();
+      DEBUG(dbgs() << "<- ParseConstants\n");
       return false;
     case NaClBitstreamEntry::Record:
       // The interesting case.
@@ -1069,7 +1075,7 @@ bool NaClBitcodeReader::ParseConstants() {
     case naclbitc::CST_CODE_INTEGER:   // INTEGER: [intval]
       if (!CurTy->isIntegerTy() || Record.empty())
         return Error("Invalid CST_INTEGER record");
-      V = ConstantInt::get(CurTy, decodeSignRotatedValue(Record[0]));
+      V = ConstantInt::get(CurTy, NaClDecodeSignRotatedValue(Record[0]));
       break;
     case naclbitc::CST_CODE_WIDE_INTEGER: {// WIDE_INTEGER: [n x intval]
       if (!CurTy->isIntegerTy() || Record.empty())
@@ -1415,6 +1421,7 @@ bool NaClBitcodeReader::ParseConstants() {
 }
 
 bool NaClBitcodeReader::ParseUseLists() {
+  DEBUG(dbgs() << "-> ParseUseLists\n");
   if (Stream.EnterSubBlock(naclbitc::USELIST_BLOCK_ID))
     return Error("Malformed block record");
 
@@ -1429,6 +1436,7 @@ bool NaClBitcodeReader::ParseUseLists() {
     case NaClBitstreamEntry::Error:
       return Error("malformed use list block");
     case NaClBitstreamEntry::EndBlock:
+      DEBUG(dbgs() << "<- ParseUseLists\n");
       return false;
     case NaClBitstreamEntry::Record:
       // The interesting case.
@@ -1455,6 +1463,7 @@ bool NaClBitcodeReader::ParseUseLists() {
 /// remember where it is and then skip it.  This lets us lazily deserialize the
 /// functions.
 bool NaClBitcodeReader::RememberAndSkipFunctionBody() {
+  DEBUG(dbgs() << "-> RememberAndSkipFunctionBody\n");
   // Get the function we are talking about.
   if (FunctionsWithBodies.empty())
     return Error("Insufficient function protos");
@@ -1469,6 +1478,7 @@ bool NaClBitcodeReader::RememberAndSkipFunctionBody() {
   // Skip over the function block for now.
   if (Stream.SkipBlock())
     return Error("Malformed block record");
+  DEBUG(dbgs() << "<- RememberAndSkipFunctionBody\n");
   return false;
 }
 
@@ -1499,6 +1509,7 @@ bool NaClBitcodeReader::GlobalCleanup() {
 }
 
 bool NaClBitcodeReader::ParseModule(bool Resume) {
+  DEBUG(dbgs() << "-> ParseModule\n");
   if (Resume)
     Stream.JumpToBit(NextUnreadBit);
   else if (Stream.EnterSubBlock(naclbitc::MODULE_BLOCK_ID))
@@ -1517,11 +1528,13 @@ bool NaClBitcodeReader::ParseModule(bool Resume) {
       Error("malformed module block");
       return true;
     case NaClBitstreamEntry::EndBlock:
+      DEBUG(dbgs() << "<- ParseModule\n");
       return GlobalCleanup();
 
     case NaClBitstreamEntry::SubBlock:
       switch (Entry.ID) {
       default:  // Skip unknown content.
+        DEBUG(dbgs() << "Skip unknown context\n");
         if (Stream.SkipBlock())
           return Error("Malformed block record");
         break;
@@ -1566,6 +1579,7 @@ bool NaClBitcodeReader::ParseModule(bool Resume) {
 
         if (RememberAndSkipFunctionBody())
           return true;
+
         // For streaming bitcode, suspend parsing when we reach the function
         // bodies. Subsequent materialization calls will resume it when
         // necessary. For streaming, the function bodies must be at the end of
@@ -1574,6 +1588,7 @@ bool NaClBitcodeReader::ParseModule(bool Resume) {
         // just finish the parse now.
         if (LazyStreamer && SeenValueSymbolTable) {
           NextUnreadBit = Stream.GetCurrentBitNo();
+          DEBUG(dbgs() << "<- ParseModule\n");
           return false;
         }
         break;
@@ -1861,6 +1876,7 @@ bool NaClBitcodeReader::ParseBitcodeInto(Module *M) {
 }
 
 bool NaClBitcodeReader::ParseModuleTriple(std::string &Triple) {
+  DEBUG(dbgs() << "-> ParseModuleTriple\n");
   if (Stream.EnterSubBlock(naclbitc::MODULE_BLOCK_ID))
     return Error("Malformed block record");
 
@@ -1875,6 +1891,7 @@ bool NaClBitcodeReader::ParseModuleTriple(std::string &Triple) {
     case NaClBitstreamEntry::Error:
       return Error("malformed module block");
     case NaClBitstreamEntry::EndBlock:
+      DEBUG(dbgs() << "<- ParseModuleTriple\n");
       return false;
     case NaClBitstreamEntry::Record:
       // The interesting case.
@@ -1940,6 +1957,7 @@ bool NaClBitcodeReader::ParseTriple(std::string &Triple) {
 
 /// ParseMetadataAttachment - Parse metadata attachments.
 bool NaClBitcodeReader::ParseMetadataAttachment() {
+  DEBUG(dbgs() << "-> ParseMetadataAttachment\n");
   if (Stream.EnterSubBlock(naclbitc::METADATA_ATTACHMENT_ID))
     return Error("Malformed block record");
 
@@ -1952,6 +1970,7 @@ bool NaClBitcodeReader::ParseMetadataAttachment() {
     case NaClBitstreamEntry::Error:
       return Error("malformed metadata block");
     case NaClBitstreamEntry::EndBlock:
+      DEBUG(dbgs() << "<- ParseMetadataAttachment\n");
       return false;
     case NaClBitstreamEntry::Record:
       // The interesting case.
@@ -1985,6 +2004,7 @@ bool NaClBitcodeReader::ParseMetadataAttachment() {
 
 /// ParseFunctionBody - Lazily parse the specified function body block.
 bool NaClBitcodeReader::ParseFunctionBody(Function *F) {
+  DEBUG(dbgs() << "-> ParseFunctionBody\n");
   if (Stream.EnterSubBlock(naclbitc::FUNCTION_BLOCK_ID))
     return Error("Malformed block record");
 
@@ -2016,21 +2036,26 @@ bool NaClBitcodeReader::ParseFunctionBody(Function *F) {
     case NaClBitstreamEntry::SubBlock:
       switch (Entry.ID) {
       default:  // Skip unknown content.
+        dbgs() << "default skip block\n";
         if (Stream.SkipBlock())
           return Error("Malformed block record");
         break;
       case naclbitc::CONSTANTS_BLOCK_ID:
-        if (ParseConstants()) return true;
+        if (ParseConstants())
+          return true;
         NextValueNo = ValueList.size();
         break;
       case naclbitc::VALUE_SYMTAB_BLOCK_ID:
-        if (ParseValueSymbolTable()) return true;
+        if (ParseValueSymbolTable())
+          return true;
         break;
       case naclbitc::METADATA_ATTACHMENT_ID:
-        if (ParseMetadataAttachment()) return true;
+        if (ParseMetadataAttachment())
+          return true;
         break;
       case naclbitc::METADATA_BLOCK_ID:
-        if (ParseMetadata()) return true;
+        if (ParseMetadata())
+          return true;
         break;
       }
       continue;
@@ -2862,6 +2887,7 @@ OutOfRecordLoop:
   ValueList.shrinkTo(ModuleValueListSize);
   MDValueList.shrinkTo(ModuleMDValueListSize);
   std::vector<BasicBlock*>().swap(FunctionBBs);
+  DEBUG(dbgs() << "-> ParseFunctionBody\n");
   return false;
 }
 
diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h
index f6e3e94841..f3d36862c3 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h
+++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.h
@@ -237,8 +237,6 @@ public:
   /// @returns true if an error occurred.
   bool ParseTriple(std::string &Triple);
 
-  static uint64_t decodeSignRotatedValue(uint64_t V);
-
 private:
   Type *getTypeByID(unsigned ID);
   Value *getFnValueByID(unsigned ID, Type *Ty) {
@@ -315,7 +313,7 @@ private:
   Value *getValueSigned(SmallVector<uint64_t, 64> &Record, unsigned Slot,
                         unsigned InstNum, Type *Ty) {
     if (Slot == Record.size()) return 0;
-    unsigned ValNo = (unsigned)decodeSignRotatedValue(Record[Slot]);
+    unsigned ValNo = (unsigned) NaClDecodeSignRotatedValue(Record[Slot]);
     // Adjust the ValNo, if it was encoded relative to the InstNum.
     if (UseRelativeIDs)
       ValNo = InstNum - ValNo;
diff --git a/lib/Bitcode/NaCl/Reader/NaClBitstreamReader.cpp b/lib/Bitcode/NaCl/Reader/NaClBitstreamReader.cpp
index 677539071b..d75c42ca9b 100644
--- a/lib/Bitcode/NaCl/Reader/NaClBitstreamReader.cpp
+++ b/lib/Bitcode/NaCl/Reader/NaClBitstreamReader.cpp
@@ -71,13 +71,14 @@ bool NaClBitstreamCursor::EnterSubBlock(unsigned BlockID, unsigned *NumWordsP) {
   }
 
   // Get the codesize of this block.
-  CurCodeSize = ReadVBR(naclbitc::CodeLenWidth);
+  CurCodeSize.IsFixed = true;
+  CurCodeSize.NumBits = ReadVBR(naclbitc::CodeLenWidth);
   SkipToFourByteBoundary();
   unsigned NumWords = Read(naclbitc::BlockSizeWidth);
   if (NumWordsP) *NumWordsP = NumWords;
 
   // Validate that this block is sane.
-  if (CurCodeSize == 0 || AtEndOfStream())
+  if (CurCodeSize.NumBits == 0 || AtEndOfStream())
     return true;
 
   return false;
diff --git a/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp b/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
index 2e318020ad..d77a79a2c9 100644
--- a/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
+++ b/lib/Bitcode/NaCl/Writer/NaClBitcodeWriter.cpp
@@ -11,6 +11,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+#define DEBUG_TYPE "NaClBitcodeWriter"
+
 #include "llvm/Bitcode/NaCl/NaClReaderWriter.h"
 #include "NaClValueEnumerator.h"
 #include "llvm/ADT/Triple.h"
@@ -23,6 +25,7 @@
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/ValueSymbolTable.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
@@ -32,20 +35,39 @@
 #include <map>
 using namespace llvm;
 
-/// 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.
+/// 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.
+///
+/// Note that for each block type GROUP, the last entry should be of
+/// the form:
+///
+///    GROUP_MAX_ABBREV = GROUP_LAST_ABBREV,
+///
+/// where GROUP_LAST_ABBREV is the last defined abbreviation. See
+/// include file "llvm/Bitcode/NaCl/NaClBitCodes.h" for more
+/// information on how groups should be defined.
 enum {
   // VALUE_SYMTAB_BLOCK abbrev id's.
   VST_ENTRY_8_ABBREV = naclbitc::FIRST_APPLICATION_ABBREV,
   VST_ENTRY_7_ABBREV,
   VST_ENTRY_6_ABBREV,
   VST_BBENTRY_6_ABBREV,
+  VST_MAX_ABBREV = VST_BBENTRY_6_ABBREV,
 
   // CONSTANTS_BLOCK abbrev id's.
   CONSTANTS_SETTYPE_ABBREV = naclbitc::FIRST_APPLICATION_ABBREV,
   CONSTANTS_INTEGER_ABBREV,
   CONSTANTS_CE_CAST_Abbrev,
   CONSTANTS_NULL_Abbrev,
+  CONSTANTS_MAX_ABBREV = CONSTANTS_NULL_Abbrev,
+
+  // CONSTANTS_BLOCK abbrev id's when global (extends list above).
+  CST_CONSTANTS_AGGREGATE_ABBREV = CONSTANTS_MAX_ABBREV+1,
+  CST_CONSTANTS_STRING_ABBREV,
+  CST_CONSTANTS_CSTRING_7_ABBREV,
+  CST_CONSTANTS_CSTRING_6_ABBREV,
+  CST_CONSTANTS_MAX_ABBREV = CST_CONSTANTS_CSTRING_6_ABBREV,
 
   // FUNCTION_BLOCK abbrev id's.
   FUNCTION_INST_LOAD_ABBREV = naclbitc::FIRST_APPLICATION_ABBREV,
@@ -55,6 +77,24 @@ enum {
   FUNCTION_INST_RET_VOID_ABBREV,
   FUNCTION_INST_RET_VAL_ABBREV,
   FUNCTION_INST_UNREACHABLE_ABBREV,
+  FUNCTION_INST_MAX_ABBREV = FUNCTION_INST_UNREACHABLE_ABBREV,
+
+  // TYPE_BLOCK_ID_NEW abbrev id's.
+  TYPE_POINTER_ABBREV = naclbitc::FIRST_APPLICATION_ABBREV,
+  TYPE_FUNCTION_ABBREV,
+  TYPE_STRUCT_ANON_ABBREV,
+  TYPE_STRUCT_NAME_ABBREV,
+  TYPE_STRUCT_NAMED_ABBREV,
+  TYPE_ARRAY_ABBREV,
+  TYPE_MAX_ABBREV = TYPE_ARRAY_ABBREV,
+
+  // META_DATA_BLOCK abbrev id's.
+  METADATA_STRING_ABBREV = naclbitc::FIRST_APPLICATION_ABBREV,
+  METADATA_MAX_ABBREV = METADATA_STRING_ABBREV,
+
+  // MODULE_BLOCK abbrev id's.
+  MODULE_GLOBALVAR_ABBREV = naclbitc::FIRST_APPLICATION_ABBREV,
+  MODULE_MAX_ABBREV = MODULE_GLOBALVAR_ABBREV,
 
   // SwitchInst Magic
   SWITCH_INST_MAGIC = 0x4B5 // May 2012 => 1205 => Hex
@@ -160,8 +200,9 @@ static void WriteAttributeGroupTable(const NaClValueEnumerator &VE,
                                      NaClBitstreamWriter &Stream) {
   const std::vector<AttributeSet> &AttrGrps = VE.getAttributeGroups();
   if (AttrGrps.empty()) return;
+  DEBUG(dbgs() << "-> WriteAbbributeGroupTable\n");
 
-  Stream.EnterSubblock(naclbitc::PARAMATTR_GROUP_BLOCK_ID, 3);
+  Stream.EnterSubblock(naclbitc::PARAMATTR_GROUP_BLOCK_ID);
 
   SmallVector<uint64_t, 64> Record;
   for (unsigned i = 0, e = AttrGrps.size(); i != e; ++i) {
@@ -202,14 +243,16 @@ static void WriteAttributeGroupTable(const NaClValueEnumerator &VE,
   }
 
   Stream.ExitBlock();
+  DEBUG(dbgs() << "<- WriteAbbributeGroupTable\n");
 }
 
 static void WriteAttributeTable(const NaClValueEnumerator &VE,
                                 NaClBitstreamWriter &Stream) {
   const std::vector<AttributeSet> &Attrs = VE.getAttributes();
   if (Attrs.empty()) return;
+  DEBUG(dbgs() << "-> WriteAttributeTable\n");
 
-  Stream.EnterSubblock(naclbitc::PARAMATTR_BLOCK_ID, 3);
+  Stream.EnterSubblock(naclbitc::PARAMATTR_BLOCK_ID);
 
   SmallVector<uint64_t, 64> Record;
   for (unsigned i = 0, e = Attrs.size(); i != e; ++i) {
@@ -222,23 +265,26 @@ static void WriteAttributeTable(const NaClValueEnumerator &VE,
   }
 
   Stream.ExitBlock();
+  DEBUG(dbgs() << "<- WriteAttributeTable\n");
 }
 
 /// WriteTypeTable - Write out the type table for a module.
 static void WriteTypeTable(const NaClValueEnumerator &VE,
                            NaClBitstreamWriter &Stream) {
+  DEBUG(dbgs() << "-> WriteTypeTable\n");
   const NaClValueEnumerator::TypeList &TypeList = VE.getTypes();
 
-  Stream.EnterSubblock(naclbitc::TYPE_BLOCK_ID_NEW,
-                       4 /*count from # abbrevs */);
+  Stream.EnterSubblock(naclbitc::TYPE_BLOCK_ID_NEW, TYPE_MAX_ABBREV);
+
   SmallVector<uint64_t, 64> TypeVals;
 
+
   // Note: modify to use maximum number of bits if under cutoff. Otherwise,
   // use VBR to take advantage that frequently referenced types have
   // small IDs.
   //
   // Note: Cutoff chosen based on experiments on pnacl-translate.pexe.
-  uint64_t NumBits = Log2_32_Ceil(VE.getTypes().size()+1);
+  uint64_t NumBits = NaClBitsNeededForValue(VE.getTypes().size());
   static const uint64_t TypeVBRCutoff = 6;
   uint64_t TypeIdNumBits = (NumBits <= TypeVBRCutoff ? NumBits : TypeVBRCutoff);
   NaClBitCodeAbbrevOp::Encoding TypeIdEncoding =
@@ -250,49 +296,51 @@ static void WriteTypeTable(const NaClValueEnumerator &VE,
   Abbv->Add(NaClBitCodeAbbrevOp(naclbitc::TYPE_CODE_POINTER));
   Abbv->Add(NaClBitCodeAbbrevOp(TypeIdEncoding, TypeIdNumBits));
   Abbv->Add(NaClBitCodeAbbrevOp(0));  // Addrspace = 0
-  unsigned PtrAbbrev = Stream.EmitAbbrev(Abbv);
+  if (TYPE_POINTER_ABBREV != Stream.EmitAbbrev(Abbv))
+    llvm_unreachable("Unexpected abbrev ordering!");
 
   // Abbrev for TYPE_CODE_FUNCTION.
   Abbv = new NaClBitCodeAbbrev();
   Abbv->Add(NaClBitCodeAbbrevOp(naclbitc::TYPE_CODE_FUNCTION));
   Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed, 1));  // isvararg
   Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Array));
-  Abbv->Add(NaClBitCodeAbbrevOp(TypeIdEncoding, TypeIdNumBits));
-
-  unsigned FunctionAbbrev = Stream.EmitAbbrev(Abbv);
+  Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed, NumBits));
+  if (TYPE_FUNCTION_ABBREV != Stream.EmitAbbrev(Abbv))
+    llvm_unreachable("Unexpected abbrev ordering!");
 
   // Abbrev for TYPE_CODE_STRUCT_ANON.
   Abbv = new NaClBitCodeAbbrev();
   Abbv->Add(NaClBitCodeAbbrevOp(naclbitc::TYPE_CODE_STRUCT_ANON));
   Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed, 1));  // ispacked
   Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Array));
-  Abbv->Add(NaClBitCodeAbbrevOp(TypeIdEncoding, TypeIdNumBits));
-
-  unsigned StructAnonAbbrev = Stream.EmitAbbrev(Abbv);
+  Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed, NumBits));
+  if (TYPE_STRUCT_ANON_ABBREV != Stream.EmitAbbrev(Abbv))
+    llvm_unreachable("Unexpected abbrev ordering!");
 
   // Abbrev for TYPE_CODE_STRUCT_NAME.
   Abbv = new NaClBitCodeAbbrev();
   Abbv->Add(NaClBitCodeAbbrevOp(naclbitc::TYPE_CODE_STRUCT_NAME));
   Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Array));
   Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Char6));
-  unsigned StructNameAbbrev = Stream.EmitAbbrev(Abbv);
+  if (TYPE_STRUCT_NAME_ABBREV != Stream.EmitAbbrev(Abbv))
+    llvm_unreachable("Unexpected abbrev ordering!");
 
   // Abbrev for TYPE_CODE_STRUCT_NAMED.
   Abbv = new NaClBitCodeAbbrev();
   Abbv->Add(NaClBitCodeAbbrevOp(naclbitc::TYPE_CODE_STRUCT_NAMED));
   Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed, 1));  // ispacked
   Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Array));
-  Abbv->Add(NaClBitCodeAbbrevOp(TypeIdEncoding, TypeIdNumBits));
-
-  unsigned StructNamedAbbrev = Stream.EmitAbbrev(Abbv);
+  Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed, NumBits));
+  if (TYPE_STRUCT_NAMED_ABBREV != Stream.EmitAbbrev(Abbv))
+    llvm_unreachable("Unexpected abbrev ordering!");
 
   // Abbrev for TYPE_CODE_ARRAY.
   Abbv = new NaClBitCodeAbbrev();
   Abbv->Add(NaClBitCodeAbbrevOp(naclbitc::TYPE_CODE_ARRAY));
   Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::VBR, 8));   // size
-  Abbv->Add(NaClBitCodeAbbrevOp(TypeIdEncoding, TypeIdNumBits));
-
-  unsigned ArrayAbbrev = Stream.EmitAbbrev(Abbv);
+  Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed, NumBits));
+  if (TYPE_ARRAY_ABBREV != Stream.EmitAbbrev(Abbv))
+    llvm_unreachable("Unexpected abbrev ordering!");
 
   // Emit an entry count so the reader can reserve space.
   TypeVals.push_back(TypeList.size());
@@ -329,7 +377,7 @@ static void WriteTypeTable(const NaClValueEnumerator &VE,
       TypeVals.push_back(VE.getTypeID(PTy->getElementType()));
       unsigned AddressSpace = PTy->getAddressSpace();
       TypeVals.push_back(AddressSpace);
-      if (AddressSpace == 0) AbbrevToUse = PtrAbbrev;
+      if (AddressSpace == 0) AbbrevToUse = TYPE_POINTER_ABBREV;
       break;
     }
     case Type::FunctionTyID: {
@@ -340,7 +388,7 @@ static void WriteTypeTable(const NaClValueEnumerator &VE,
       TypeVals.push_back(VE.getTypeID(FT->getReturnType()));
       for (unsigned i = 0, e = FT->getNumParams(); i != e; ++i)
         TypeVals.push_back(VE.getTypeID(FT->getParamType(i)));
-      AbbrevToUse = FunctionAbbrev;
+      AbbrevToUse = TYPE_FUNCTION_ABBREV;
       break;
     }
     case Type::StructTyID: {
@@ -354,19 +402,19 @@ static void WriteTypeTable(const NaClValueEnumerator &VE,
 
       if (ST->isLiteral()) {
         Code = naclbitc::TYPE_CODE_STRUCT_ANON;
-        AbbrevToUse = StructAnonAbbrev;
+        AbbrevToUse = TYPE_STRUCT_ANON_ABBREV;
       } else {
         if (ST->isOpaque()) {
           Code = naclbitc::TYPE_CODE_OPAQUE;
         } else {
           Code = naclbitc::TYPE_CODE_STRUCT_NAMED;
-          AbbrevToUse = StructNamedAbbrev;
+          AbbrevToUse = TYPE_STRUCT_NAMED_ABBREV;
         }
 
         // Emit the name if it is present.
         if (!ST->getName().empty())
           WriteStringRecord(naclbitc::TYPE_CODE_STRUCT_NAME, ST->getName(),
-                            StructNameAbbrev, Stream);
+                            TYPE_STRUCT_NAME_ABBREV, Stream);
       }
       break;
     }
@@ -376,7 +424,7 @@ static void WriteTypeTable(const NaClValueEnumerator &VE,
       Code = naclbitc::TYPE_CODE_ARRAY;
       TypeVals.push_back(AT->getNumElements());
       TypeVals.push_back(VE.getTypeID(AT->getElementType()));
-      AbbrevToUse = ArrayAbbrev;
+      AbbrevToUse = TYPE_ARRAY_ABBREV;
       break;
     }
     case Type::VectorTyID: {
@@ -395,6 +443,7 @@ static void WriteTypeTable(const NaClValueEnumerator &VE,
   }
 
   Stream.ExitBlock();
+  DEBUG(dbgs() << "<- WriteTypeTable\n");
 }
 
 static unsigned getEncodedLinkage(const GlobalValue *GV) {
@@ -443,6 +492,7 @@ static unsigned getEncodedThreadLocalMode(const GlobalVariable *GV) {
 // descriptors for global variables, and function prototype info.
 static void WriteModuleInfo(const Module *M, const NaClValueEnumerator &VE,
                             NaClBitstreamWriter &Stream) {
+  DEBUG(dbgs() << "-> WriteModuleInfo\n");
   // Emit various pieces of data attached to a module.
   if (!M->getTargetTriple().empty())
     WriteStringRecord(naclbitc::MODULE_CODE_TRIPLE, M->getTargetTriple(),
@@ -497,13 +547,13 @@ static void WriteModuleInfo(const Module *M, const NaClValueEnumerator &VE,
   }
 
   // Emit abbrev for globals, now that we know # sections and max alignment.
-  unsigned SimpleGVarAbbrev = 0;
-  if (!M->global_empty()) {
-    // Add an abbrev for common globals with no visibility or thread localness.
+  // Add an abbrev for common globals with no visibility or thread localness.
+  // Don't bother emitting vis + thread local abbreviation.
+  {
     NaClBitCodeAbbrev *Abbv = new NaClBitCodeAbbrev();
     Abbv->Add(NaClBitCodeAbbrevOp(naclbitc::MODULE_CODE_GLOBALVAR));
-    Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed,
-                              Log2_32_Ceil(MaxGlobalType+1)));
+    Abbv->Add(NaClBitCodeAbbrevOp(
+        NaClBitCodeAbbrevOp::Fixed, NaClBitsNeededForValue(MaxGlobalType)));
     Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed, 1)); // Constant.
     Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::VBR, 6));   // Initializer.
     Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed, 4)); // Linkage.
@@ -512,15 +562,15 @@ static void WriteModuleInfo(const Module *M, const NaClValueEnumerator &VE,
     else {
       unsigned MaxEncAlignment = Log2_32(MaxAlignment)+1;
       Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed,
-                               Log2_32_Ceil(MaxEncAlignment+1)));
+                                    NaClBitsNeededForValue(MaxEncAlignment)));
     }
     if (SectionMap.empty())                                    // Section.
       Abbv->Add(NaClBitCodeAbbrevOp(0));
     else
       Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed,
-                               Log2_32_Ceil(SectionMap.size()+1)));
-    // Don't bother emitting vis + thread local.
-    SimpleGVarAbbrev = Stream.EmitAbbrev(Abbv);
+                                    NaClBitsNeededForValue(SectionMap.size())));
+    if (MODULE_GLOBALVAR_ABBREV != Stream.EmitAbbrev(Abbv))
+      llvm_unreachable("Unexpected abbrev ordering!");
   }
 
   // Emit the global variable information.
@@ -547,7 +597,7 @@ static void WriteModuleInfo(const Module *M, const NaClValueEnumerator &VE,
       Vals.push_back(GV->hasUnnamedAddr());
       Vals.push_back(GV->isExternallyInitialized());
     } else {
-      AbbrevToUse = SimpleGVarAbbrev;
+      AbbrevToUse = MODULE_GLOBALVAR_ABBREV;
     }
 
     Stream.EmitRecord(naclbitc::MODULE_CODE_GLOBALVAR, Vals, AbbrevToUse);
@@ -586,6 +636,7 @@ static void WriteModuleInfo(const Module *M, const NaClValueEnumerator &VE,
     Stream.EmitRecord(naclbitc::MODULE_CODE_ALIAS, Vals, AbbrevToUse);
     Vals.clear();
   }
+  DEBUG(dbgs() << "<- WriteModuleInfo\n");
 }
 
 static uint64_t GetOptimizationFlags(const Value *V) {
@@ -640,30 +691,23 @@ static void WriteMDNode(const MDNode *N,
 static void WriteModuleMetadata(const Module *M,
                                 const NaClValueEnumerator &VE,
                                 NaClBitstreamWriter &Stream) {
+  DEBUG(dbgs() << "-> WriteModuleMetadata\n");
   const NaClValueEnumerator::ValueList &Vals = VE.getMDValues();
   bool StartedMetadataBlock = false;
-  unsigned MDSAbbrev = 0;
   SmallVector<uint64_t, 64> Record;
   for (unsigned i = 0, e = Vals.size(); i != e; ++i) {
 
     if (const MDNode *N = dyn_cast<MDNode>(Vals[i].first)) {
       if (!N->isFunctionLocal() || !N->getFunction()) {
         if (!StartedMetadataBlock) {
-          Stream.EnterSubblock(naclbitc::METADATA_BLOCK_ID, 3);
+          Stream.EnterSubblock(naclbitc::METADATA_BLOCK_ID);
           StartedMetadataBlock = true;
         }
         WriteMDNode(N, VE, Stream, Record);
       }
     } else if (const MDString *MDS = dyn_cast<MDString>(Vals[i].first)) {
       if (!StartedMetadataBlock)  {
-        Stream.EnterSubblock(naclbitc::METADATA_BLOCK_ID, 3);
-
-        // Abbrev for METADATA_STRING.
-        NaClBitCodeAbbrev *Abbv = new NaClBitCodeAbbrev();
-        Abbv->Add(NaClBitCodeAbbrevOp(naclbitc::METADATA_STRING));
-        Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Array));
-        Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed, 8));
-        MDSAbbrev = Stream.EmitAbbrev(Abbv);
+        Stream.EnterSubblock(naclbitc::METADATA_BLOCK_ID);
         StartedMetadataBlock = true;
       }
 
@@ -671,7 +715,8 @@ static void WriteModuleMetadata(const Module *M,
       Record.append(MDS->begin(), MDS->end());
 
       // Emit the finished record.
-      Stream.EmitRecord(naclbitc::METADATA_STRING, Record, MDSAbbrev);
+      Stream.EmitRecord(naclbitc::METADATA_STRING, Record,
+                        METADATA_STRING_ABBREV);
       Record.clear();
     }
   }
@@ -681,7 +726,7 @@ static void WriteModuleMetadata(const Module *M,
        E = M->named_metadata_end(); I != E; ++I) {
     const NamedMDNode *NMD = I;
     if (!StartedMetadataBlock)  {
-      Stream.EnterSubblock(naclbitc::METADATA_BLOCK_ID, 3);
+      Stream.EnterSubblock(naclbitc::METADATA_BLOCK_ID);
       StartedMetadataBlock = true;
     }
 
@@ -701,11 +746,14 @@ static void WriteModuleMetadata(const Module *M,
 
   if (StartedMetadataBlock)
     Stream.ExitBlock();
+
+  DEBUG(dbgs() << "<- WriteModuleMetadata\n");
 }
 
 static void WriteFunctionLocalMetadata(const Function &F,
                                        const NaClValueEnumerator &VE,
                                        NaClBitstreamWriter &Stream) {
+  DEBUG(dbgs() << "-> WriteFunctionLocalMetadata\n");
   bool StartedMetadataBlock = false;
   SmallVector<uint64_t, 64> Record;
   const SmallVector<const MDNode *, 8> &Vals = VE.getFunctionLocalMDValues();
@@ -713,7 +761,7 @@ static void WriteFunctionLocalMetadata(const Function &F,
     if (const MDNode *N = Vals[i])
       if (N->isFunctionLocal() && N->getFunction() == &F) {
         if (!StartedMetadataBlock) {
-          Stream.EnterSubblock(naclbitc::METADATA_BLOCK_ID, 3);
+          Stream.EnterSubblock(naclbitc::METADATA_BLOCK_ID);
           StartedMetadataBlock = true;
         }
         WriteMDNode(N, VE, Stream, Record);
@@ -721,12 +769,13 @@ static void WriteFunctionLocalMetadata(const Function &F,
 
   if (StartedMetadataBlock)
     Stream.ExitBlock();
+  DEBUG(dbgs() << "<- WriteFunctionLocalMetadata\n");
 }
 
 static void WriteMetadataAttachment(const Function &F,
                                     const NaClValueEnumerator &VE,
                                     NaClBitstreamWriter &Stream) {
-  Stream.EnterSubblock(naclbitc::METADATA_ATTACHMENT_ID, 3);
+  Stream.EnterSubblock(naclbitc::METADATA_ATTACHMENT_ID);
 
   SmallVector<uint64_t, 64> Record;
 
@@ -767,7 +816,7 @@ static void WriteModuleMetadataStore(const Module *M,
 
   if (Names.empty()) return;
 
-  Stream.EnterSubblock(naclbitc::METADATA_BLOCK_ID, 3);
+  Stream.EnterSubblock(naclbitc::METADATA_BLOCK_ID);
 
   for (unsigned MDKindID = 0, e = Names.size(); MDKindID != e; ++MDKindID) {
     Record.push_back(MDKindID);
@@ -782,10 +831,7 @@ static void WriteModuleMetadataStore(const Module *M,
 }
 
 static void emitSignedInt64(SmallVectorImpl<uint64_t> &Vals, uint64_t V) {
-  if ((int64_t)V >= 0)
-    Vals.push_back(V << 1);
-  else
-    Vals.push_back((-V << 1) | 1);
+  Vals.push_back(NaClEncodeSignRotatedValue((int64_t)V));
 }
 
 static void EmitAPInt(SmallVectorImpl<uint64_t> &Vals,
@@ -821,21 +867,29 @@ static void WriteConstants(unsigned FirstVal, unsigned LastVal,
                            NaClBitstreamWriter &Stream, bool isGlobal) {
   if (FirstVal == LastVal) return;
 
-  Stream.EnterSubblock(naclbitc::CONSTANTS_BLOCK_ID, 4);
+  Stream.EnterSubblock(naclbitc::CONSTANTS_BLOCK_ID,
+                       (isGlobal
+                        ? CST_CONSTANTS_MAX_ABBREV
+                        : CONSTANTS_MAX_ABBREV));
 
   unsigned AggregateAbbrev = 0;
   unsigned String8Abbrev = 0;
   unsigned CString7Abbrev = 0;
   unsigned CString6Abbrev = 0;
   // If this is a constant pool for the module, emit module-specific abbrevs.
+  // Note: These abbreviations are size specific (to LastVal), and hence,
+  // can be more efficient if LastVal is known (rather then generating
+  // up-front for all constant sections).
   if (isGlobal) {
     // Abbrev for CST_CODE_AGGREGATE.
     NaClBitCodeAbbrev *Abbv = new NaClBitCodeAbbrev();
     Abbv->Add(NaClBitCodeAbbrevOp(naclbitc::CST_CODE_AGGREGATE));
     Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Array));
     Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed,
-                                  Log2_32_Ceil(LastVal+1)));
+                                  NaClBitsNeededForValue(LastVal)));
     AggregateAbbrev = Stream.EmitAbbrev(Abbv);
+    if (CST_CONSTANTS_AGGREGATE_ABBREV != AggregateAbbrev)
+      llvm_unreachable("Unexpected abbrev ordering!");
 
     // Abbrev for CST_CODE_STRING.
     Abbv = new NaClBitCodeAbbrev();
@@ -843,20 +897,31 @@ static void WriteConstants(unsigned FirstVal, unsigned LastVal,
     Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Array));
     Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed, 8));
     String8Abbrev = Stream.EmitAbbrev(Abbv);
+    if (CST_CONSTANTS_STRING_ABBREV != String8Abbrev)
+      llvm_unreachable("Unexpected abbrev ordering!");
+
     // Abbrev for CST_CODE_CSTRING.
     Abbv = new NaClBitCodeAbbrev();
     Abbv->Add(NaClBitCodeAbbrevOp(naclbitc::CST_CODE_CSTRING));
     Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Array));
     Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Fixed, 7));
     CString7Abbrev = Stream.EmitAbbrev(Abbv);
+    if (CST_CONSTANTS_CSTRING_7_ABBREV != CString7Abbrev)
+      llvm_unreachable("Unexpected abbrev ordering!");
+
     // Abbrev for CST_CODE_CSTRING.
     Abbv = new NaClBitCodeAbbrev();
     Abbv->Add(NaClBitCodeAbbrevOp(naclbitc::CST_CODE_CSTRING));
     Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Array));
     Abbv->Add(NaClBitCodeAbbrevOp(NaClBitCodeAbbrevOp::Char6));
     CString6Abbrev = Stream.EmitAbbrev(Abbv);
+    if (CST_CONSTANTS_CSTRING_6_ABBREV != CString6Abbrev)
+      llvm_unreachable("Unexpected abbrev ordering!");
+
+    DEBUG(dbgs() << "-- emitted abbreviations\n");
   }
 
+
   SmallVector<uint64_t, 64> Record;
 
   const NaClValueEnumerator::ValueList &Vals = VE.getValues();
@@ -1062,6 +1127,7 @@ static void WriteConstants(unsigned FirstVal, unsigned LastVal,
   }
 
   Stream.ExitBlock();
+  DEBUG(dbgs() << "<- WriteConstants\n");
 }
 
 static void WriteModuleConstants(const NaClValueEnumerator &VE,
@@ -1486,7 +1552,7 @@ static void WriteValueSymbolTable(const ValueSymbolTable &VST,
                                   const NaClValueEnumerator &VE,
                                   NaClBitstreamWriter &Stream) {
   if (VST.empty()) return;
-  Stream.EnterSubblock(naclbitc::VALUE_SYMTAB_BLOCK_ID, 4);
+  Stream.EnterSubblock(naclbitc::VALUE_SYMTAB_BLOCK_ID);
 
   // FIXME: Set up the abbrev, we know how many values there are!
   // FIXME: We know if the type names can use 7-bit ascii.
@@ -1542,7 +1608,7 @@