Convert Subzero's bitcode reader to generate ICE types.

src/PNaClTranslator.cpp

 //===- subzero/src/PNaClTranslator.cpp - ICE from bitcode -----------------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the PNaCl bitcode file to Ice, to machine code
@@ skipping 31 matching lines @@
 using namespace llvm;
 
 namespace {
 
 // TODO(kschimpf) Remove error recovery once implementation complete.
 static cl::opt<bool> AllowErrorRecovery(
     "allow-pnacl-reader-error-recovery",
     cl::desc("Allow error recovery when reading PNaCl bitcode."),
     cl::init(false));
 
+// Models elements in the list of types defined in the types block.
+// These elements can be a (simple) type, or a function type
+// signature.
+class ExtendedType {
+  ExtendedType(const ExtendedType &) = delete;
+  ExtendedType &operator=(const ExtendedType &) = delete;
+
+public:
+  // Discriminator for LLVM-style RTTI.
+  enum ExtendedTypeKind { SimpleExtendedKind, FcnSigExtendedKind };
+  virtual ~ExtendedType() {}
+  ExtendedTypeKind getKind() const { return Kind; }
+  virtual void Print(Ice::Ostream &Stream) const = 0;
+
+protected:
+  explicit ExtendedType(ExtendedTypeKind Kind) : Kind(Kind) {}
+
+private:
+  ExtendedTypeKind Kind;
+};
+
+Ice::Ostream &operator<<(Ice::Ostream &Stream, const ExtendedType &Ty) {
+  Ty.Print(Stream);
+  return Stream;
+}
+
+// Models an ICE type as an extended type.
+class SimpleExtendedType : public ExtendedType {
+  SimpleExtendedType(const SimpleExtendedType &) = delete;
+  SimpleExtendedType &operator=(const SimpleExtendedType &) = delete;
+
+public:
+  explicit SimpleExtendedType(Ice::Type SimpleType)
+      : ExtendedType(SimpleExtendedKind), SimpleType(SimpleType) {}
+
+  ~SimpleExtendedType() final {}
+
+  Ice::Type getType() const { return SimpleType; }
+
+  void Print(Ice::Ostream &Stream) const final { Stream << SimpleType; }
+
+  static bool classof(const ExtendedType *Ty) {
+    return Ty->getKind() == SimpleExtendedKind;
+  }
+
+private:
+  Ice::Type SimpleType;
+};
+
+// Models a function signature as an extended type.
+class FcnSigExtendedType : public ExtendedType {
+  FcnSigExtendedType(const FcnSigExtendedType &) = delete;
+  FcnSigExtendedType &operator=(const FcnSigExtendedType &) = delete;
+
+public:
+  explicit FcnSigExtendedType(Ice::Type ReturnType)
+      : ExtendedType(FcnSigExtendedKind), FcnSig(ReturnType) {}
+  const Ice::FcnSigType &getFcnSig() const { return FcnSig; }
+  void appendArgType(Ice::Type ArgType) { FcnSig.appendArgType(ArgType); }
+  void Print(Ice::Ostream &Stream) const final { Stream << FcnSig; }
+  static bool classof(const ExtendedType *Ty) {
+    return Ty->getKind() == FcnSigExtendedKind;
+  }
+
+private:
+  Ice::FcnSigType FcnSig;
+};
+
 // Top-level class to read PNaCl bitcode files, and translate to ICE.
 class TopLevelParser : public NaClBitcodeParser {
   TopLevelParser(const TopLevelParser &) = delete;
   TopLevelParser &operator=(const TopLevelParser &) = delete;
 
 public:
   TopLevelParser(Ice::Translator &Translator, const std::string &InputName,
                  NaClBitcodeHeader &Header, NaClBitstreamCursor &Cursor,
                  bool &ErrorStatus)
       : NaClBitcodeParser(Cursor), Translator(Translator),
         Mod(new Module(InputName, getGlobalContext())), DL(PNaClDataLayout),
         Header(Header), TypeConverter(getLLVMContext()),
         ErrorStatus(ErrorStatus), NumErrors(0), NumFunctionIds(0),
         NumFunctionBlocks(0),
-        GlobalVarPlaceHolderType(convertToLLVMType(Ice::IceType_i8)) {
+        GlobalVarPlaceHolderType(convertToLLVMType(Ice::IceType_i8)),
+        UndefinedFcnSigType(Ice::IceType_void) {
     Mod->setDataLayout(PNaClDataLayout);
     setErrStream(Translator.getContext()->getStrDump());
   }
 
-  ~TopLevelParser() override {}
+  ~TopLevelParser() override { DeleteContainerPointers(TypeIDValues); }

[Jim Stichnoth, 2014/10/05 15:35:47]

I think that if possible, the memory allocation strategy here should follow the
arena-based style that Cfg uses for Inst allocation.  This is as long as the
objects being allocated can be made POD, particularly with trivial destructors
which also means no container fields.  (The Inst class deals with this by using
flat arrays and specifying the array size in the constructor.)

If this isn't practical now, at least leave a TODO.

[Karl, 2014/10/06 21:15:54]

Reorganized to no longer allocate extended types. Rather, there is a single
representation (that handles both simple and function signature extended types),
and cast is only used to add accessors. This should solve this problem.

 
   Ice::Translator &getTranslator() { return Translator; }
 
   // Generates error with given Message. Always returns true.
   bool Error(const std::string &Message) override {
     ErrorStatus = true;
     ++NumErrors;
     NaClBitcodeParser::Error(Message);
     if (!AllowErrorRecovery)
       report_fatal_error("Unable to continue");
@@ skipping 12 matching lines @@
   /// Returns the number of bytes in the bitcode header.
   size_t getHeaderSize() const { return Header.getHeaderSize(); }
 
   /// Returns the llvm context to use.
   LLVMContext &getLLVMContext() const { return Mod->getContext(); }
 
   /// Changes the size of the type list to the given size.
   void resizeTypeIDValues(unsigned NewSize) { TypeIDValues.resize(NewSize); }
 
   /// Returns the type associated with the given index.
-  Type *getTypeByID(unsigned ID) {
+  Ice::Type getTypeByID(unsigned ID) {

[Jim Stichnoth, 2014/10/05 15:35:47]

Can this be const?

I guess not because of setting errors?

I think that these get() methods should either be const, or they should document
why they are not const.

[jvoung (off chromium), 2014/10/06 14:32:17]

I wonder if this should be called "getSimpleTypeByID" to be more specific.

Otherwise it looks close to setTypeID, but that handles both types.

[Karl, 2014/10/06 21:15:54]

Done. Added constness and added qualifier to accessor names.

     // Note: method resizeTypeIDValues expands TypeIDValues
-    // to the specified size, and fills elements with NULL.
-    Type *Ty = ID < TypeIDValues.size() ? TypeIDValues[ID] : NULL;
-    if (Ty)
-      return Ty;
-    return reportTypeIDAsUndefined(ID);
+    // to the specified size, and fills elements with nullptr.
+    ExtendedType *Ty = ID < TypeIDValues.size() ? TypeIDValues[ID] : nullptr;
+    if (Ty == nullptr) {
+      reportTypeIDAsUndefined(ID);
+    } else if (auto SimpleTy = dyn_cast<SimpleExtendedType>(Ty)) {
+      return SimpleTy->getType();
+    } else {
+      reportTypeIDNotSimple(ID, Ty);
+    }
+    // TODO(kschimpf) Remove error recovery once implementation complete.
+    return Ice::IceType_void;
+  }
+
+  /// Returns the type signature associated with the given index.
+  const Ice::FcnSigType &getFcnSigTypeByID(unsigned ID) {
+    // Note: method resizeTypeIDValues expands TypeIDValues
+    // to the specified size, and fills elements with nullptr.
+    ExtendedType *Ty = ID < TypeIDValues.size() ? TypeIDValues[ID] : nullptr;
+    if (Ty == nullptr) {
+      reportFcnSigTypeIDAsUndefined(ID);
+    } else if (auto SigTy = dyn_cast<FcnSigExtendedType>(Ty)) {
+      return SigTy->getFcnSig();
+    } else {
+      reportFcnSigTypeIDNotFcnSig(ID, Ty);
+    }
+    // TODO(kschimpf) Remove error recovery once implementation complete.
+    return UndefinedFcnSigType;
   }
 
   /// Defines type for ID.
-  void setTypeID(unsigned ID, Type *Ty) {
-    if (ID < TypeIDValues.size() && TypeIDValues[ID] == NULL) {
+  void setTypeID(unsigned ID, ExtendedType *Ty) {
+    if (ID < TypeIDValues.size() && TypeIDValues[ID] == nullptr) {
       TypeIDValues[ID] = Ty;
       return;
     }
     reportBadSetTypeID(ID, Ty);
   }
 
   /// Sets the next function ID to the given LLVM function.
   void setNextFunctionID(Function *Fcn) {
     ++NumFunctionIds;
     ValueIDValues.push_back(Fcn);
@@ skipping 11 matching lines @@
   unsigned getNextFunctionBlockValueID() {
     if (NumFunctionBlocks >= DefiningFunctionsList.size())
       report_fatal_error(
           "More function blocks than defined function addresses");
     return DefiningFunctionsList[NumFunctionBlocks++];
   }
 
   /// Returns the LLVM IR value associatd with the global value ID.
   Value *getGlobalValueByID(unsigned ID) const {
     if (ID >= ValueIDValues.size())
-      return NULL;
+      return nullptr;
     return ValueIDValues[ID];
   }
 
   /// Returns the corresponding constant associated with a global value
   /// (i.e. relocatable).
   Ice::Constant *getOrCreateGlobalConstantByID(unsigned ID) {
     // TODO(kschimpf): Can this be built when creating global initializers?
     if (ID >= ValueIDConstants.size()) {
       if (ID >= ValueIDValues.size())
-        return NULL;
+        return nullptr;
       ValueIDConstants.resize(ValueIDValues.size());
     }
     Ice::Constant *C = ValueIDConstants[ID];
-    if (C != NULL)
+    if (C != nullptr)
       return C;
     Value *V = ValueIDValues[ID];
     assert(isa<GlobalValue>(V));
     C = getTranslator().getContext()->getConstantSym(getIcePointerType(), 0,
                                                      V->getName());
     ValueIDConstants[ID] = C;
     return C;
   }
 
   /// Returns the number of function addresses (i.e. ID's) defined in
   /// the bitcode file.
   unsigned getNumFunctionIDs() const { return NumFunctionIds; }
 
   /// Returns the number of global values defined in the bitcode
   /// file.
   unsigned getNumGlobalValueIDs() const { return ValueIDValues.size(); }
 
   /// Resizes the list of value IDs to include Count global variable
   /// IDs.
   void resizeValueIDsForGlobalVarCount(unsigned Count) {
     ValueIDValues.resize(ValueIDValues.size() + Count);
   }
 
   /// Returns the global variable address associated with the given
   /// value ID. If the ID refers to a global variable address not yet
   /// defined, a placeholder is created so that we can fix it up
   /// later.
   Constant *getOrCreateGlobalVarRef(unsigned ID) {
     if (ID >= ValueIDValues.size())
-      return NULL;
+      return nullptr;
     if (Value *C = ValueIDValues[ID])
       return dyn_cast<Constant>(C);
     Constant *C = new GlobalVariable(*Mod, GlobalVarPlaceHolderType, false,
                                      GlobalValue::ExternalLinkage, 0);
     ValueIDValues[ID] = C;
     return C;
   }
 
   /// Assigns the given global variable (address) to the given value
   /// ID.  Returns true if ID is a valid global variable ID. Otherwise
   /// returns false.
   bool assignGlobalVariable(GlobalVariable *GV, unsigned ID) {
     if (ID < NumFunctionIds || ID >= ValueIDValues.size())
       return false;
     WeakVH &OldV = ValueIDValues[ID];
-    if (OldV == NULL) {
+    if (OldV == nullptr) {
       ValueIDValues[ID] = GV;
       return true;
     }
 
     // If reached, there was a forward reference to this value. Replace it.
     Value *PrevVal = OldV;
     GlobalVariable *Placeholder = cast<GlobalVariable>(PrevVal);
     Placeholder->replaceAllUsesWith(
         ConstantExpr::getBitCast(GV, Placeholder->getType()));
     Placeholder->eraseFromParent();
     ValueIDValues[ID] = GV;
     return true;
   }
 
   /// Returns the corresponding ICE type for LLVMTy.
   Ice::Type convertToIceType(Type *LLVMTy) {
     Ice::Type IceTy = TypeConverter.convertToIceType(LLVMTy);
     if (IceTy >= Ice::IceType_NUM) {
       return convertToIceTypeError(LLVMTy);
     }
     return IceTy;
   }
 
   /// Returns the corresponding LLVM type for IceTy.
   Type *convertToLLVMType(Ice::Type IceTy) const {
     return TypeConverter.convertToLLVMType(IceTy);
   }
 
   /// Returns the LLVM integer type with the given number of Bits.  If
-  /// Bits is not a valid PNaCl type, returns NULL.
+  /// Bits is not a valid PNaCl type, returns nullptr.
   Type *getLLVMIntegerType(unsigned Bits) const {

[jvoung (off chromium), 2014/10/06 14:32:17]

can this be removed now?

[Karl, 2014/10/06 21:15:54]

Removed getLLVMIntegerType and getLLVMVectorType.

[jvoung (off chromium), 2014/10/07 15:54:41]

Can it be removed from TypeConverter now too?

     return TypeConverter.getLLVMIntegerType(Bits);
   }
 
   /// Returns the LLVM vector with the given Size and Ty. If not a
-  /// valid PNaCl vector type, returns NULL.
+  /// valid PNaCl vector type, returns nullptr.
   Type *getLLVMVectorType(unsigned Size, Ice::Type Ty) const {
     return TypeConverter.getLLVMVectorType(Size, Ty);
   }
 
   /// Returns the model for pointer types in ICE.
   Ice::Type getIcePointerType() const {
     return TypeConverter.getIcePointerType();
   }
 
 private:
   // The translator associated with the parser.
   Ice::Translator &Translator;
   // The parsed module.
   std::unique_ptr<Module> Mod;
   // The data layout to use.
   DataLayout DL;
   // The bitcode header.
   NaClBitcodeHeader &Header;
   // Converter between LLVM and ICE types.
   Ice::TypeConverter TypeConverter;
   // The exit status that should be set to true if an error occurs.
   bool &ErrorStatus;
   // The number of errors reported.
   unsigned NumErrors;
   // The types associated with each type ID.
-  std::vector<Type *> TypeIDValues;
+  std::vector<ExtendedType *> TypeIDValues;
   // The (global) value IDs.
   std::vector<WeakVH> ValueIDValues;
   // Relocatable constants associated with ValueIDValues.
   std::vector<Ice::Constant *> ValueIDConstants;
   // The number of function IDs.
   unsigned NumFunctionIds;
   // The number of function blocks (processed so far).
   unsigned NumFunctionBlocks;
   // The list of value IDs (in the order found) of defining function
   // addresses.
   std::vector<unsigned> DefiningFunctionsList;
   // Cached global variable placeholder type. Used for all forward
   // references to global variable addresses.
   Type *GlobalVarPlaceHolderType;
+  // Models an undefined function type signature.
+  Ice::FcnSigType UndefinedFcnSigType;
 
   bool ParseBlock(unsigned BlockID) override;
 
-  /// Reports that type ID is undefined, and then returns
-  /// the void type.
-  Type *reportTypeIDAsUndefined(unsigned ID);
+  /// Reports that type ID is undefined.

[jvoung (off chromium), 2014/10/06 14:32:17]

double vs triple slash consistency

Clarify this is for simple types? Or you could share reportTypeIDAsUndefined()
between the two, but have some parameter that can be used to clarify the error
message.

[Karl, 2014/10/06 21:15:54]

Simplified all type id look ups to go through one lookup function
getTypeByIDAsKind, and one error report function reportBadTypeIDAs.

+  void reportTypeIDAsUndefined(unsigned ID);
+
+  // Reports that type ID as not a simple type.
+  void reportTypeIDNotSimple(unsigned ID, const ExtendedType *Ty);
+
+  // Reports that type ID is undefined.

[jvoung (off chromium), 2014/10/06 14:32:17]

Clarify this is for function signature types?

[Karl, 2014/10/06 21:15:54]

Removed. No longer needed.

+  void reportFcnSigTypeIDAsUndefined(unsigned ID);
+
+  // Reports that type ID is not a function signature.
+  void reportFcnSigTypeIDNotFcnSig(unsigned ID, const ExtendedType *Ty);
 
   /// Reports error about bad call to setTypeID.
-  void reportBadSetTypeID(unsigned ID, Type *Ty);
+  void reportBadSetTypeID(unsigned ID, ExtendedType *Ty);

[Jim Stichnoth, 2014/10/05 15:35:48]

const ExtendedType * ?

[Karl, 2014/10/06 21:15:54]

Added to reportBadTypeIDAs.

 
   // Reports that there is no corresponding ICE type for LLVMTy, and
   // returns ICE::IceType_void.
   Ice::Type convertToIceTypeError(Type *LLVMTy);
 };
 
-Type *TopLevelParser::reportTypeIDAsUndefined(unsigned ID) {
+void TopLevelParser::reportTypeIDAsUndefined(unsigned ID) {
   std::string Buffer;
   raw_string_ostream StrBuf(Buffer);
   StrBuf << "Can't find type for type id: " << ID;
   Error(StrBuf.str());
-  // TODO(kschimpf) Remove error recovery once implementation complete.
-  Type *Ty = TypeConverter.convertToLLVMType(Ice::IceType_void);
-  // To reduce error messages, update type list if possible.
-  if (ID < TypeIDValues.size())
-    TypeIDValues[ID] = Ty;
-  return Ty;
 }
 
-void TopLevelParser::reportBadSetTypeID(unsigned ID, Type *Ty) {
+void TopLevelParser::reportTypeIDNotSimple(unsigned ID,
+                                           const ExtendedType *Ty) {
+  std::string Buffer;
+  raw_string_ostream StrBuf(Buffer);
+  StrBuf << "Type id " << ID << " not simple type. Found: " << *Ty;
+  Error(StrBuf.str());
+};
+
+void TopLevelParser::reportFcnSigTypeIDAsUndefined(unsigned ID) {
+  std::string Buffer;
+  raw_string_ostream StrBuf(Buffer);
+  StrBuf << "Can't find function signature type for type id: " << ID;
+  Error(StrBuf.str());
+}
+
+void TopLevelParser::reportFcnSigTypeIDNotFcnSig(unsigned ID,
+                                                 const ExtendedType *Ty) {
+  std::string Buffer;
+  raw_string_ostream StrBuf(Buffer);
+  StrBuf << "Type id " << ID << " not function signature type. Found:" << *Ty;
+  Error(StrBuf.str());
+}
+
+void TopLevelParser::reportBadSetTypeID(unsigned ID, ExtendedType *Ty) {
   std::string Buffer;
   raw_string_ostream StrBuf(Buffer);
   if (ID >= TypeIDValues.size()) {
-    StrBuf << "Type index " << ID << " out of range: can't install.";
+    StrBuf << "Type index " << ID
+           << " out of range. Can't install type: " << Ty;
   } else {
     // Must be case that index already defined.
     StrBuf << "Type index " << ID << " defined as " << *TypeIDValues[ID]
            << " and " << *Ty << ".";
   }
   Error(StrBuf.str());
 }
 
 Ice::Type TopLevelParser::convertToIceTypeError(Type *LLVMTy) {
   std::string Buffer;
@@ skipping 46 matching lines @@
   // Default implementation. Reports that the record is not
   // understood.
   void ProcessRecord() override;
 
   // Checks if the size of the record is Size.  Return true if valid.
   // Otherwise generates an error and returns false.
   bool isValidRecordSize(unsigned Size, const char *RecordName) {
     const NaClBitcodeRecord::RecordVector &Values = Record.GetValues();
     if (Values.size() == Size)
       return true;
-    ReportRecordSizeError(Size, RecordName, NULL);
+    ReportRecordSizeError(Size, RecordName, nullptr);
     return false;
   }
 
   // Checks if the size of the record is at least as large as the
   // LowerLimit. Returns true if valid.  Otherwise generates an error
   // and returns false.
   bool isValidRecordSizeAtLeast(unsigned LowerLimit, const char *RecordName) {
     const NaClBitcodeRecord::RecordVector &Values = Record.GetValues();
     if (Values.size() >= LowerLimit)
       return true;
@@ skipping 17 matching lines @@
   // valid.  Otherwise generates an error and returns false.
   bool isValidRecordSizeInRange(unsigned LowerLimit, unsigned UpperLimit,
                                 const char *RecordName) {
     return isValidRecordSizeAtLeast(LowerLimit, RecordName) ||
            isValidRecordSizeAtMost(UpperLimit, RecordName);
   }
 
 private:
   /// Generates a record size error. ExpectedSize is the number
   /// of elements expected. RecordName is the name of the kind of
-  /// record that has incorrect size. ContextMessage (if not NULL)
+  /// record that has incorrect size. ContextMessage (if not nullptr)
   /// is appended to "record expects" to describe how ExpectedSize
   /// should be interpreted.
   void ReportRecordSizeError(unsigned ExpectedSize, const char *RecordName,
                              const char *ContextMessage);
 };
 
 void BlockParserBaseClass::ReportRecordSizeError(unsigned ExpectedSize,
                                                  const char *RecordName,
                                                  const char *ContextMessage) {
   std::string Buffer;
@@ skipping 38 matching lines @@
 
 private:
   // The type ID that will be associated with the next type defining
   // record in the types block.
   unsigned NextTypeId;
 
   void ProcessRecord() override;
 };
 
 void TypesParser::ProcessRecord() {
-  Type *Ty = NULL;
+  ExtendedType *Ty = nullptr;
   const NaClBitcodeRecord::RecordVector &Values = Record.GetValues();
   switch (Record.GetCode()) {
   case naclbitc::TYPE_CODE_NUMENTRY:
     // NUMENTRY: [numentries]
     if (!isValidRecordSize(1, "Type count"))
       return;
     Context->resizeTypeIDValues(Values[0]);
     return;
   case naclbitc::TYPE_CODE_VOID:
     // VOID
     if (!isValidRecordSize(0, "Type void"))
       return;
-    Ty = Context->convertToLLVMType(Ice::IceType_void);
+    Ty = new SimpleExtendedType(Ice::IceType_void);
     break;
   case naclbitc::TYPE_CODE_FLOAT:
     // FLOAT
     if (!isValidRecordSize(0, "Type float"))
       return;
-    Ty = Context->convertToLLVMType(Ice::IceType_f32);
+    Ty = new SimpleExtendedType(Ice::IceType_f32);
     break;
   case naclbitc::TYPE_CODE_DOUBLE:
     // DOUBLE
     if (!isValidRecordSize(0, "Type double"))
       return;
-    Ty = Context->convertToLLVMType(Ice::IceType_f64);
+    Ty = new SimpleExtendedType(Ice::IceType_f64);
     break;
   case naclbitc::TYPE_CODE_INTEGER:
     // INTEGER: [width]
     if (!isValidRecordSize(1, "Type integer"))
       return;
-    Ty = Context->getLLVMIntegerType(Values[0]);
-    if (Ty == NULL) {
+    switch (Values[0]) {
+    case 1:
+      Ty = new SimpleExtendedType(Ice::IceType_i1);
+      break;
+    case 8:
+      Ty = new SimpleExtendedType(Ice::IceType_i8);
+      break;
+    case 16:
+      Ty = new SimpleExtendedType(Ice::IceType_i16);
+      break;
+    case 32:
+      Ty = new SimpleExtendedType(Ice::IceType_i32);
+      break;
+    case 64:
+      Ty = new SimpleExtendedType(Ice::IceType_i64);
+      break;
+    default:
+      break;
+    }
+    if (Ty == nullptr) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "Type integer record with invalid bitsize: " << Values[0];
       Error(StrBuf.str());
-      // TODO(kschimpf) Remove error recovery once implementation complete.
-      // Fix type so that we can continue.
-      Ty = Context->convertToLLVMType(Ice::IceType_i32);
     }
     break;
   case naclbitc::TYPE_CODE_VECTOR: {
     // VECTOR: [numelts, eltty]
     if (!isValidRecordSize(2, "Type vector"))
       return;
-    Type *BaseTy = Context->getTypeByID(Values[1]);
-    Ty = Context->getLLVMVectorType(Values[0],

[jvoung (off chromium), 2014/10/06 14:32:17]

can getLLVMVectorType be removed now?

[Karl, 2014/10/06 21:15:54]

Done.

-                                    Context->convertToIceType(BaseTy));
-    if (Ty == NULL) {
+    Ice::Type BaseTy = Context->getTypeByID(Values[1]);
+    Ice::SizeT Size = Values[0];
+    switch (BaseTy) {
+    case Ice::IceType_i1:
+      switch (Size) {
+      case 4:
+        Ty = new SimpleExtendedType(Ice::IceType_v4i1);
+        break;
+      case 8:
+        Ty = new SimpleExtendedType(Ice::IceType_v8i1);
+        break;
+      case 16:
+        Ty = new SimpleExtendedType(Ice::IceType_v16i1);
+        break;
+      default:
+        break;
+      }
+      break;
+    case Ice::IceType_i8:
+      if (Size == 16)
+        Ty = new SimpleExtendedType(Ice::IceType_v16i8);
+      break;
+    case Ice::IceType_i16:
+      if (Size == 8)
+        Ty = new SimpleExtendedType(Ice::IceType_v8i16);
+      break;
+    case Ice::IceType_i32:
+      if (Size == 4)
+        Ty = new SimpleExtendedType(Ice::IceType_v4i32);
+      break;
+    case Ice::IceType_f32:
+      if (Size == 4)
+        Ty = new SimpleExtendedType(Ice::IceType_v4f32);
+      break;
+    default:
+      break;
+    }
+
+    if (Ty == nullptr) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
-      StrBuf << "Invalid type vector record: <" << Values[0] << " x " << *BaseTy
+      StrBuf << "Invalid type vector record: <" << Values[0] << " x " << BaseTy
              << ">";
       Error(StrBuf.str());
-      Ty = Context->convertToLLVMType(Ice::IceType_void);
     }
     break;
   }
   case naclbitc::TYPE_CODE_FUNCTION: {
     // FUNCTION: [vararg, retty, paramty x N]
     if (!isValidRecordSizeAtLeast(2, "Type signature"))
       return;
-    SmallVector<Type *, 8> ArgTys;
+    if (Values[0])
+      Error("Function type can't define varargs");
+    FcnSigExtendedType *FcnTy =
+        new FcnSigExtendedType(Context->getTypeByID(Values[1]));
     for (unsigned i = 2, e = Values.size(); i != e; ++i) {
-      ArgTys.push_back(Context->getTypeByID(Values[i]));
+      // Check that type void not used as argument type.
+      // Note: PNaCl restrictions can't be checked until we
+      // know the name, because we have to check for intrinsic signatures.
+      Ice::Type ArgTy = Context->getTypeByID(Values[i]);
+      if (ArgTy == Ice::IceType_void) {
+        std::string Buffer;
+        raw_string_ostream StrBuf(Buffer);
+        StrBuf << "Type for parameter " << (i - 1)
+               << " not valid. Found: " << ArgTy;
+        // TODO(kschimpf) Remove error recovery once implementation complete.
+        ArgTy = Ice::IceType_i32;
+      }
+      FcnTy->appendArgType(ArgTy);
     }
-    Ty = FunctionType::get(Context->getTypeByID(Values[1]), ArgTys, Values[0]);
+    Ty = FcnTy;
     break;
   }
   default:
     BlockParserBaseClass::ProcessRecord();
     return;
   }
   // If Ty not defined, assume error. Use void as filler.
-  if (Ty == NULL)
-    Ty = Context->convertToLLVMType(Ice::IceType_void);
+  // TODO(kschimpf) Remove error recovery once implementation complete.
+  // Fix type so that we can continue.
+  if (Ty == nullptr)
+    Ty = new SimpleExtendedType(Ice::IceType_void);
   Context->setTypeID(NextTypeId++, Ty);
 }
 
 /// Parses the globals block (i.e. global variables).
 class GlobalsParser : public BlockParserBaseClass {
 public:
   GlobalsParser(unsigned BlockID, BlockParserBaseClass *EnclosingParser)
       : BlockParserBaseClass(BlockID, EnclosingParser), InitializersNeeded(0),
         Alignment(1), IsConstant(false) {
     NextGlobalID = Context->getNumFunctionIDs();
@@ skipping 62 matching lines @@
     if (InitializersNeeded <= Initializers.size()) {
       Error(std::string(RecordName) +
             " record: Too many initializers, ignoring.");
     }
   }
 
   // Takes the initializers (and other parser state values) and
   // installs a global variable (with the initializers) into the list
   // of ValueIDs.
   void installGlobalVar() {
-    Constant *Init = NULL;
+    Constant *Init = nullptr;
     switch (Initializers.size()) {
     case 0:
       Error("No initializer for global variable in global vars block");
       return;
     case 1:
       Init = Initializers[0];
       break;
     default:
       Init = ConstantStruct::getAnon(Context->getLLVMContext(), Initializers,
                                      true);
@@ skipping 83 matching lines @@
     Constant *Init = ConstantDataArray::get(
         Context->getLLVMContext(), ArrayRef<uint8_t>(Buf.data(), Buf.size()));
     Initializers.push_back(Init);
     break;
   }
   case naclbitc::GLOBALVAR_RELOC: {
     // RELOC: [val, [addend]]
     if (!isValidRecordSizeInRange(1, 2, "Globals reloc"))
       return;
     Constant *BaseVal = Context->getOrCreateGlobalVarRef(Values[0]);
-    if (BaseVal == NULL) {
+    if (BaseVal == nullptr) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "Can't find global relocation value: " << Values[0];
       Error(StrBuf.str());
       return;
     }
     Type *IntPtrType = Context->convertToLLVMType(Context->getIcePointerType());
     Constant *Val = ConstantExpr::getPtrToInt(BaseVal, IntPtrType);
     if (Values.size() == 2) {
       Val = ConstantExpr::getAdd(Val, ConstantInt::get(IntPtrType, Values[1]));
@@ skipping 195 matching lines @@
     return Func->makeVariable(Ty);
   }
 
   // Generates the next available local variable using the given type.
   Ice::Variable *getNextInstVar(Ice::Type Ty) {
     assert(NextLocalInstIndex >= CachedNumGlobalValueIDs);
     // Before creating one, see if a forwardtyperef has already defined it.
     uint32_t LocalIndex = NextLocalInstIndex - CachedNumGlobalValueIDs;
     if (LocalIndex < LocalOperands.size()) {
       Ice::Operand *Op = LocalOperands[LocalIndex];
-      if (Op != NULL) {
+      if (Op != nullptr) {
         if (Ice::Variable *Var = dyn_cast<Ice::Variable>(Op)) {
           if (Var->getType() == Ty) {
             ++NextLocalInstIndex;
             return Var;
           }
         }
         std::string Buffer;
         raw_string_ostream StrBuf(Buffer);
         StrBuf << "Illegal forward referenced instruction ("
                << NextLocalInstIndex << "): " << *Op;
@@ skipping 30 matching lines @@
     }
     uint32_t LocalIndex = Index - CachedNumGlobalValueIDs;
     if (LocalIndex >= LocalOperands.size()) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "Value index " << Index << " not defined!";
       Error(StrBuf.str());
       report_fatal_error("Unable to continue");
     }
     Ice::Operand *Op = LocalOperands[LocalIndex];
-    if (Op == NULL) {
+    if (Op == nullptr) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "Value index " << Index << " not defined!";
       Error(StrBuf.str());
       report_fatal_error("Unable to continue");
     }
     return Op;
   }
 
   // Sets element Index (in the local operands list) to Op.
   void setOperand(uint32_t Index, Ice::Operand *Op) {
     assert(Op);
     // Check if simple push works.
     uint32_t LocalIndex = Index - CachedNumGlobalValueIDs;
     if (LocalIndex == LocalOperands.size()) {
       LocalOperands.push_back(Op);
       return;
     }
 
     // Must be forward reference, expand vector to accommodate.
     if (LocalIndex >= LocalOperands.size())
       LocalOperands.resize(LocalIndex + 1);
 
     // If element not defined, set it.
     Ice::Operand *OldOp = LocalOperands[LocalIndex];
-    if (OldOp == NULL) {
+    if (OldOp == nullptr) {
       LocalOperands[LocalIndex] = Op;
       return;
     }
 
     // See if forward reference matches.
     if (OldOp == Op)
       return;
 
     // Error has occurred.
     std::string Buffer;
@@ skipping 419 matching lines @@
       return;
     CurrentNode->appendInst(Ice::InstArithmetic::create(
         Func, Opcode, getNextInstVar(Type1), Op1, Op2));
     break;
   }
   case naclbitc::FUNC_CODE_INST_CAST: {
     // CAST: [opval, destty, castopc]
     if (!isValidRecordSize(3, "function block cast"))
       return;
     Ice::Operand *Src = getRelativeOperand(Values[0], BaseIndex);
-    Type *CastType = Context->getTypeByID(Values[1]);
+    Ice::Type CastType = Context->getTypeByID(Values[1]);
     Instruction::CastOps LLVMCastOp;
     Ice::InstCast::OpKind CastKind;
     if (!naclbitc::DecodeCastOpcode(Values[2], LLVMCastOp) ||
         !convertLLVMCastOpToIceOp(LLVMCastOp, CastKind)) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "Cast opcode not understood: " << Values[2];
       Error(StrBuf.str());
       return;
     }
-    Type *SrcType = Context->convertToLLVMType(Src->getType());
-    if (!CastInst::castIsValid(LLVMCastOp, SrcType, CastType)) {
+    Ice::Type SrcType = Src->getType();
+    if (!CastInst::castIsValid(LLVMCastOp, Context->convertToLLVMType(SrcType),
+                               Context->convertToLLVMType(CastType))) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "Illegal cast: " << Instruction::getOpcodeName(LLVMCastOp)
-             << " " << *SrcType << " to " << *CastType;
+             << " " << SrcType << " to " << CastType;
       Error(StrBuf.str());
       return;
     }
-    CurrentNode->appendInst(Ice::InstCast::create(
-        Func, CastKind, getNextInstVar(Context->convertToIceType(CastType)),
-        Src));
+    CurrentNode->appendInst(
+        Ice::InstCast::create(Func, CastKind, getNextInstVar(CastType), Src));
     break;
   }
   case naclbitc::FUNC_CODE_INST_VSELECT: {
     // VSELECT: [opval, opval, pred]
     Ice::Operand *ThenVal = getRelativeOperand(Values[0], BaseIndex);
     Ice::Type ThenType = ThenVal->getType();
     Ice::Operand *ElseVal = getRelativeOperand(Values[1], BaseIndex);
     Ice::Type ElseType = ElseVal->getType();
     if (ThenType != ElseType) {
       std::string Buffer;
@@ skipping 162 matching lines @@
       CurrentNode->appendInst(
           Ice::InstRet::create(Func, getRelativeOperand(Values[0], BaseIndex)));
     }
     InstIsTerminating = true;
     break;
   }
   case naclbitc::FUNC_CODE_INST_BR: {
     if (Values.size() == 1) {
       // BR: [bb#]
       Ice::CfgNode *Block = getBranchBasicBlock(Values[0]);
-      if (Block == NULL)
+      if (Block == nullptr)
         return;
       CurrentNode->appendInst(Ice::InstBr::create(Func, Block));
     } else {
       // BR: [bb#, bb#, opval]
       if (!isValidRecordSize(3, "function block branch"))
         return;
       Ice::Operand *Cond = getRelativeOperand(Values[2], BaseIndex);
       if (Cond->getType() != Ice::IceType_i1) {
         std::string Buffer;
         raw_string_ostream StrBuf(Buffer);
         StrBuf << "Branch condition " << *Cond << " not i1. Found: "
                << Cond->getType();
         Error(StrBuf.str());
         return;
       }
       Ice::CfgNode *ThenBlock = getBranchBasicBlock(Values[0]);
       Ice::CfgNode *ElseBlock = getBranchBasicBlock(Values[1]);
-      if (ThenBlock == NULL || ElseBlock == NULL)
+      if (ThenBlock == nullptr || ElseBlock == nullptr)
         return;
       CurrentNode->appendInst(
           Ice::InstBr::create(Func, Cond, ThenBlock, ElseBlock));
     }
     InstIsTerminating = true;
     break;
   }
   case naclbitc::FUNC_CODE_INST_SWITCH: {
     // SWITCH: [Condty, Cond, BbIndex, NumCases Case ...]
     // where Case = [1, 1, Value, BbIndex].
     //
     // Note: Unlike most instructions, we don't infer the type of
     // Cond, but provide it as a separate field. There are also
     // unnecesary data fields (i.e. constants 1).  These were not
     // cleaned up in PNaCl bitcode because the bitcode format was
     // already frozen when the problem was noticed.
     if (!isValidRecordSizeAtLeast(4, "function block switch"))
       return;
-    Ice::Type CondTy =
-        Context->convertToIceType(Context->getTypeByID(Values[0]));
+    Ice::Type CondTy = Context->getTypeByID(Values[0]);
     if (!Ice::isScalarIntegerType(CondTy)) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "Case condition must be non-wide integer. Found: " << CondTy;
       Error(StrBuf.str());
       return;
     }
     Ice::SizeT BitWidth = Ice::getScalarIntBitWidth(CondTy);
     Ice::Operand *Cond = getRelativeOperand(Values[1], BaseIndex);
     if (CondTy != Cond->getType()) {
@@ skipping 47 matching lines @@
     if (!isValidRecordSizeAtLeast(3, "function block phi"))
       return;
     if ((Values.size() & 0x1) == 0) {
       // Not an odd number of values.
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "function block phi record size not valid: " << Values.size();
       Error(StrBuf.str());
       return;
     }
-    Ice::Type Ty = Context->convertToIceType(Context->getTypeByID(Values[0]));
+    Ice::Type Ty = Context->getTypeByID(Values[0]);
     if (Ty == Ice::IceType_void) {
       Error("Phi record using type void not allowed");
       return;
     }
     Ice::Variable *Dest = getNextInstVar(Ty);
     Ice::InstPhi *Phi = Ice::InstPhi::create(Func, Values.size() >> 1, Dest);
     for (unsigned i = 1; i < Values.size(); i += 2) {
       Ice::Operand *Op =
           getRelativeOperand(NaClDecodeSignRotatedValue(Values[i]), BaseIndex);
       if (Op->getType() != Ty) {
@@ skipping 30 matching lines @@
   }
   case naclbitc::FUNC_CODE_INST_LOAD: {
     // LOAD: [address, align, ty]
     if (!isValidRecordSize(3, "function block load"))
       return;
     Ice::Operand *Address = getRelativeOperand(Values[0], BaseIndex);
     if (!isValidPointerType(Address, "Load"))
       return;
     unsigned Alignment;
     extractAlignment("Load", Values[1], Alignment);
-    Ice::Type Ty = Context->convertToIceType(Context->getTypeByID(Values[2]));
+    Ice::Type Ty = Context->getTypeByID(Values[2]);
     if (!isValidLoadStoreAlignment(Alignment, Ty, "Load"))
       return;
     CurrentNode->appendInst(
         Ice::InstLoad::create(Func, getNextInstVar(Ty), Address, Alignment));
     break;
   }
   case naclbitc::FUNC_CODE_INST_STORE: {
     // STORE: [address, value, align]
     if (!isValidRecordSize(3, "function block store"))
       return;
@@ skipping 41 matching lines @@
              << " not understood.";
       Error(StrBuf.str());
       return;
     }
     bool IsTailCall = static_cast<bool>(CCInfo & 1);
 
     // Extract out the called function and its return type.
     uint32_t CalleeIndex = convertRelativeToAbsIndex(Values[1], BaseIndex);
     Ice::Operand *Callee = getOperand(CalleeIndex);
     Ice::Type ReturnType = Ice::IceType_void;
-    const Ice::Intrinsics::FullIntrinsicInfo *IntrinsicInfo = NULL;
+    const Ice::Intrinsics::FullIntrinsicInfo *IntrinsicInfo = nullptr;
     if (Record.GetCode() == naclbitc::FUNC_CODE_INST_CALL) {
       Function *Fcn =
           dyn_cast<Function>(Context->getGlobalValueByID(CalleeIndex));
-      if (Fcn == NULL) {
+      if (Fcn == nullptr) {
         std::string Buffer;
         raw_string_ostream StrBuf(Buffer);
         StrBuf << "Function call to non-function: " << *Callee;
         Error(StrBuf.str());
         return;
       }
 
       FunctionType *FcnTy = Fcn->getFunctionType();
       ReturnType = Context->convertToIceType(FcnTy->getReturnType());
 
       // Check if this direct call is to an Intrinsic (starts with "llvm.")
       static Ice::IceString LLVMPrefix("llvm.");
       Ice::IceString Name = Fcn->getName();
       if (isStringPrefix(Name, LLVMPrefix)) {
         Ice::IceString Suffix = Name.substr(LLVMPrefix.size());
         IntrinsicInfo =
             getTranslator().getContext()->getIntrinsicsInfo().find(Suffix);
         if (!IntrinsicInfo) {
           std::string Buffer;
           raw_string_ostream StrBuf(Buffer);
           StrBuf << "Invalid PNaCl intrinsic call to " << Name;
           Error(StrBuf.str());
           return;
         }
       }
     } else {
-      ReturnType = Context->convertToIceType(Context->getTypeByID(Values[2]));
+      ReturnType = Context->getTypeByID(Values[2]);
     }
 
     // Create the call instruction.
-    Ice::Variable *Dest =
-        (ReturnType == Ice::IceType_void) ? NULL : getNextInstVar(ReturnType);
+    Ice::Variable *Dest = (ReturnType == Ice::IceType_void)
+                              ? nullptr
+                              : getNextInstVar(ReturnType);
     Ice::SizeT NumParams = Values.size() - ParamsStartIndex;
-    Ice::InstCall *Inst = NULL;
+    Ice::InstCall *Inst = nullptr;
     if (IntrinsicInfo) {
       Inst =
           Ice::InstIntrinsicCall::create(Func, NumParams, Dest, Callee,
                                          IntrinsicInfo->Info);
     } else {
       Inst = Ice::InstCall::create(Func, NumParams, Dest, Callee, IsTailCall);
     }
 
     // Add parameters.
     for (Ice::SizeT ParamIndex = 0; ParamIndex < NumParams; ++ParamIndex) {
@@ skipping 43 matching lines @@
       }
     }
 
     CurrentNode->appendInst(Inst);
     return;
   }
   case naclbitc::FUNC_CODE_INST_FORWARDTYPEREF: {
     // FORWARDTYPEREF: [opval, ty]
     if (!isValidRecordSize(2, "function block forward type ref"))
       return;
-    setOperand(Values[0], createInstVar(Context->convertToIceType(
-                              Context->getTypeByID(Values[1]))));
+    setOperand(Values[0], createInstVar(Context->getTypeByID(Values[1])));
     break;
   }
   default:
     // Generate error message!
     BlockParserBaseClass::ProcessRecord();
     break;
   }
 }
 
 /// Parses constants within a function block.
@@ skipping 28 matching lines @@
   }
 };
 
 void ConstantsParser::ProcessRecord() {
   const NaClBitcodeRecord::RecordVector &Values = Record.GetValues();
   switch (Record.GetCode()) {
   case naclbitc::CST_CODE_SETTYPE: {
     // SETTYPE: [typeid]
     if (!isValidRecordSize(1, "constants block set type"))
       return;
-    NextConstantType =
-        Context->convertToIceType(Context->getTypeByID(Values[0]));
+    NextConstantType = Context->getTypeByID(Values[0]);
     if (NextConstantType == Ice::IceType_void)
       Error("constants block set type not allowed for void type");
     return;
   }
   case naclbitc::CST_CODE_UNDEF: {
     // UNDEF
     if (!isValidRecordSize(0, "constants block undef"))
       return;
     if (!isValidNextConstantType())
       return;
@@ skipping 184 matching lines @@
 
   ~ModuleValuesymtabParser() override {}
 
 private:
   void setValueName(uint64_t Index, StringType &Name) override;
   void setBbName(uint64_t Index, StringType &Name) override;
 };
 
 void ModuleValuesymtabParser::setValueName(uint64_t Index, StringType &Name) {
   Value *V = Context->getGlobalValueByID(Index);
-  if (V == NULL) {
+  if (V == nullptr) {
     std::string Buffer;
     raw_string_ostream StrBuf(Buffer);
     StrBuf << "Invalid global address ID in valuesymtab: " << Index;
     Error(StrBuf.str());
     return;
   }
   V->setName(StringRef(Name.data(), Name.size()));
 }
 
 void ModuleValuesymtabParser::setBbName(uint64_t Index, StringType &Name) {
@@ skipping 43 matching lines @@
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "Unknown bitstream version: " << Version;
       Error(StrBuf.str());
     }
     return;
   }
   case naclbitc::MODULE_CODE_FUNCTION: {
     // FUNCTION:  [type, callingconv, isproto, linkage]
     if (!isValidRecordSize(4, "Function heading"))
       return;
-    Type *Ty = Context->getTypeByID(Values[0]);
-    FunctionType *FTy = dyn_cast<FunctionType>(Ty);
-    if (FTy == NULL) {
-      std::string Buffer;
-      raw_string_ostream StrBuf(Buffer);
-      StrBuf << "Function heading expects function type. Found: " << Ty;
-      Error(StrBuf.str());
-      return;
-    }
+    const Ice::FcnSigType &Ty = Context->getFcnSigTypeByID(Values[0]);
     CallingConv::ID CallingConv;
     if (!naclbitc::DecodeCallingConv(Values[1], CallingConv)) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "Function heading has unknown calling convention: "
              << Values[1];
       Error(StrBuf.str());
       return;
     }
     GlobalValue::LinkageTypes Linkage;
     if (!naclbitc::DecodeLinkage(Values[3], Linkage)) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "Function heading has unknown linkage. Found " << Values[3];
       Error(StrBuf.str());
       return;
     }
-    Function *Func = Function::Create(FTy, Linkage, "", Context->getModule());
+    SmallVector<Type *, 8> ArgTys;
+    for (Ice::Type ArgType : Ty.getArgList()) {
+      ArgTys.push_back(Context->convertToLLVMType(ArgType));
+    }
+    Function *Func = Function::Create(
+        FunctionType::get(Context->convertToLLVMType(Ty.getReturnType()),
+                          ArgTys, false),
+        Linkage, "", Context->getModule());
     Func->setCallingConv(CallingConv);
     if (Values[2] == 0)
       Context->setNextValueIDAsImplementedFunction();
     Context->setNextFunctionID(Func);
     // TODO(kschimpf) verify if Func matches PNaCl ABI.
     return;
   }
   default:
     BlockParserBaseClass::ProcessRecord();
     return;
@@ skipping 58 matching lines @@
 
   if (TopLevelBlocks != 1) {
     errs() << IRFilename
            << ": Contains more than one module. Found: " << TopLevelBlocks
            << "\n";
     ErrorStatus = true;
   }
 }
 
 } // end of namespace Ice