Convert Subzero's bitcode reader to generate ICE types.

src/PNaClTranslator.cpp

Index: src/PNaClTranslator.cpp
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index af78e2f3e802e3f29af026d6a9f3aa69d2762c20..beccf142fb44f94e09f78ca4f46117a7dc722f31 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -49,6 +49,132 @@ static cl::opt<bool> AllowErrorRecovery(
     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 undefined, a (simple) type, or a function type
+// signature. Note that an extended type is undefined on construction.
+// Use methods setAsSimpleType and setAsFuncSigType to define
+// the extended type.
+class ExtendedType {
+
+public:
+  /// Discriminator for LLVM-style RTTI.
+  enum TypeKind { Undefined, Simple, FuncSig };
+
+  explicit ExtendedType() : Kind(Undefined) {}

[Jim Stichnoth, 2014/10/06 22:48:52]

I don't think you need "explicit" on a zero-arg ctor?

[Karl, 2014/10/07 20:15:58]

Done.

+
+  explicit ExtendedType(const ExtendedType &Ty)
+      : Signature(Ty.Signature), Kind(Ty.Kind) {}
+
+  ExtendedType &operator=(const ExtendedType &Ty) {

[Jim Stichnoth, 2014/10/06 22:48:52]

This is a shallow copy, so can you just use the default assignment operator
implementation?

Also, disable the default copy ctor?

[Karl, 2014/10/07 20:15:59]

Removed assignment, not needed. Replaced explicit copy constructor with default
constructor.

+    Kind = Ty.Kind;
+    Signature = Ty.Signature;
+    return *this;
+  }
+
+  virtual ~ExtendedType() {}
+
+  ExtendedType::TypeKind getKind() const { return Kind; }
+  void Dump(Ice::Ostream &Stream) const;
+
+  /// Changes the extended type to a simple type with the given
+  /// value.
+  void setAsSimpleType(Ice::Type Ty) {
+    assert(Kind == Undefined);
+    Kind = Simple;
+    Signature.setReturnType(Ty);
+  }
+
+  /// Changes the extended type to an (empty) function signature type.
+  void setAsFunctionType() {
+    assert(Kind == Undefined);
+    Kind = FuncSig;
+  }
+
+  /// Changes type back to undefined type.
+  void resetToUndefined() {
+    if (Kind != Undefined) {
+      Kind = Undefined;
+      Signature.reset();
+    }
+  }
+
+protected:
+  // Note: For simple types, the return type of the signature  will

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

extra space between sig and will

[Karl, 2014/10/07 20:15:59]

Done.

+  // be used to hold the simple type.
+  Ice::FuncSigType Signature;
+
+private:
+  ExtendedType::TypeKind Kind;
+};
+
+Ice::Ostream &operator<<(Ice::Ostream &Stream, const ExtendedType &Ty) {
+  Ty.Dump(Stream);
+  return Stream;
+}
+
+Ice::Ostream &operator<<(Ice::Ostream &Stream, ExtendedType::TypeKind Kind) {
+  Stream << "ExtendedType::";
+  switch (Kind) {
+  case ExtendedType::Undefined:
+    Stream << "Undefined";
+    break;
+  case ExtendedType::Simple:
+    Stream << "Simple";
+    break;
+  case ExtendedType::FuncSig:
+    Stream << "FuncSig";
+    break;
+  default:
+    Stream << "??";
+    break;
+  }
+  return Stream;
+}
+
+// Models an ICE type as an extended type.
+class SimpleExtendedType : public ExtendedType {
+  SimpleExtendedType(const SimpleExtendedType &) = delete;
+  SimpleExtendedType &operator=(const SimpleExtendedType &) = delete;
+
+public:
+  Ice::Type getType() const { return Signature.getReturnType(); }
+
+  static bool classof(const ExtendedType *Ty) {
+    return Ty->getKind() == Simple;
+  }
+};
+
+// Models a function signature as an extended type.
+class FuncSigExtendedType : public ExtendedType {
+  FuncSigExtendedType(const FuncSigExtendedType &) = delete;
+  FuncSigExtendedType &operator=(const FuncSigExtendedType &) = delete;
+
+public:
+  const Ice::FuncSigType &getSignature() const { return Signature; }
+  void setReturnType(Ice::Type ReturnType) {
+    Signature.setReturnType(ReturnType);
+  }
+  void appendArgType(Ice::Type ArgType) { Signature.appendArgType(ArgType); }
+  static bool classof(const ExtendedType *Ty) {
+    return Ty->getKind() == FuncSig;
+  }
+};
+
+void ExtendedType::Dump(Ice::Ostream &Stream) const {
+  Stream << Kind;
+  switch (Kind) {
+  case Simple: {
+    Stream << " " << Signature.getReturnType();
+    break;
+  }
+  case FuncSig: {
+    Stream << " " << Signature;
+  }
+  default:
+    break;
+  }
+}
+
 // Top-level class to read PNaCl bitcode files, and translate to ICE.
 class TopLevelParser : public NaClBitcodeParser {
   TopLevelParser(const TopLevelParser &) = delete;
@@ -100,23 +226,31 @@ public:
   /// Changes the size of the type list to the given size.
   void resizeTypeIDValues(unsigned NewSize) { TypeIDValues.resize(NewSize); }
 
+  /// Returns the undefined type associated with type ID.
+  ExtendedType *getUndefinedTypeByID(unsigned ID) const {
+    ExtendedType *Ty = const_cast<::ExtendedType *>(
+        getTypeByIDAsKind(ID, ExtendedType::Undefined));
+    if (Ty == nullptr)
+      Ty = const_cast<TopLevelParser *>(this)->getDummyUndefinedExtendedType();

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

In this case, how does the result of getDummyUndefinedExtendedType get
associated w/ the ID? (It's not a pointer to an entry in the vector)?

If it's not associated with the ID, then would that affect later calls to
getTypeByIDAsKind?

[Karl, 2014/10/07 20:15:58]

This method is called only once for each ID, to get the extended type to define.
If it isn't defined, it is because the count record of the types block was
wrong, and the call to getTypeByIDAsKind will generate the error message. The
call to getDummyUndefinedExtendedType will "reset" the current definition, so
that the previous record will be cleaned. This code is only for error recovery
during the parsing of the types block.

I will try to clarify this in two ways:

1) I will rename this method to clarify it isn't used for general lookup (only
for defining).

2) Rather than reusing, I will go ahead and just extend the types vector, and
use the corresponding vector position. This will guarantee that no corruption
will occur.

+    return Ty;
+  }
+
   /// Returns the type associated with the given index.
-  Type *getTypeByID(unsigned ID) {
-    // 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);
+  Ice::Type getSimpleTypeByID(unsigned ID) const {
+    const ExtendedType *Ty = getTypeByIDAsKind(ID, ExtendedType::Simple);
+    if (Ty == nullptr)
+      // Return error recovery value.
+      return Ice::IceType_void;
+    return cast<SimpleExtendedType>(Ty)->getType();
   }
 
-  /// Defines type for ID.
-  void setTypeID(unsigned ID, Type *Ty) {
-    if (ID < TypeIDValues.size() && TypeIDValues[ID] == NULL) {
-      TypeIDValues[ID] = Ty;
-      return;
-    }
-    reportBadSetTypeID(ID, Ty);
+  /// Returns the type signature associated with the given index.
+  const Ice::FuncSigType &getFuncSigTypeByID(unsigned ID) const {
+    const ExtendedType *Ty = getTypeByIDAsKind(ID, ExtendedType::FuncSig);
+    if (Ty == nullptr)
+      // Return error recovery value.
+      return UndefinedFuncSigType;
+    return cast<FuncSigExtendedType>(Ty)->getSignature();
   }
 
   /// Sets the next function ID to the given LLVM function.
@@ -144,7 +278,7 @@ public:
   /// 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];
   }
 
@@ -154,11 +288,11 @@ public:
     // 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));
@@ -188,7 +322,7 @@ public:
   /// 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,
@@ -204,7 +338,7 @@ public:
     if (ID < NumFunctionIds || ID >= ValueIDValues.size())
       return false;
     WeakVH &OldV = ValueIDValues[ID];
-    if (OldV == NULL) {
+    if (OldV == nullptr) {
       ValueIDValues[ID] = GV;
       return true;
     }
@@ -233,18 +367,6 @@ public:
     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.
-  Type *getLLVMIntegerType(unsigned Bits) const {
-    return TypeConverter.getLLVMIntegerType(Bits);
-  }
-
-  /// Returns the LLVM vector with the given Size and Ty. If not a
-  /// valid PNaCl vector type, returns NULL.
-  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();
@@ -266,7 +388,7 @@ private:
   // 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.
@@ -281,43 +403,59 @@ private:
   // Cached global variable placeholder type. Used for all forward
   // references to global variable addresses.
   Type *GlobalVarPlaceHolderType;
+  // Models an undefined function type signature.
+  Ice::FuncSigType UndefinedFuncSigType;
+  // Models an badly indexed undefined extended type.

[Jim Stichnoth, 2014/10/06 22:48:52]

a badly

[Karl, 2014/10/07 20:15:59]

Removed the need for this field, so this has been removed.

+  ExtendedType DummyUndefinedExtendedType;
 
   bool ParseBlock(unsigned BlockID) override;
 
-  /// Reports that type ID is undefined, and then returns
-  /// the void type.
-  Type *reportTypeIDAsUndefined(unsigned ID);
+  // Gets extended type associated with the given index, assuming the
+  // extended type is of the WantedKind. Generates error message if
+  // corresponding extended type of WantedKind can't be found, and
+  // returns nullptr.
+  const ExtendedType *
+  getTypeByIDAsKind(unsigned ID, ExtendedType::TypeKind WantedKind) const {
+    const ExtendedType *Ty = nullptr;
+    if (ID < TypeIDValues.size()) {
+      Ty = &TypeIDValues[ID];
+      if (Ty->getKind() == WantedKind)
+        return Ty;
+    }
+    // Note: We turn off constness so that we can generate an error message.

[Jim Stichnoth, 2014/10/06 22:48:51]

"... so that we can set ErrorStatus"

[Karl, 2014/10/07 20:15:59]

Done.

+    const_cast<TopLevelParser *>(this)->reportBadTypeIDAs(ID, Ty, WantedKind);
+    return nullptr;
+  }
+
+  // Reports that type ID is undefined.
+  void reportBadTypeIDAs(unsigned ID, const ExtendedType *Ty,
+                         ExtendedType::TypeKind WantedType);
 
-  /// Reports error about bad call to setTypeID.
-  void reportBadSetTypeID(unsigned ID, Type *Ty);
+  // Returns dummy undefined extended type. Used to guarantee
+  // that getUndefinedByTypeID always returns a valid extended type.

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

getUndefinedTypeByID?

[Karl, 2014/10/07 20:15:58]

This method has been removed, so nothing to be fixed.

+  ExtendedType *getDummyUndefinedExtendedType();
 
   // 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) {
-  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;
+ExtendedType *TopLevelParser::getDummyUndefinedExtendedType() {
+  ExtendedType *Ty = &DummyUndefinedExtendedType;
+  // Note: Reset to make sure we wipe out any changes (accidentally)

[Jim Stichnoth, 2014/10/06 22:48:52]

As we discussed in person, try to get rid of const_cast.

[Karl, 2014/10/07 20:15:59]

Done.

+  // made to it.
+  Ty->resetToUndefined();
   return Ty;
 }
 
-void TopLevelParser::reportBadSetTypeID(unsigned ID, Type *Ty) {
+void TopLevelParser::reportBadTypeIDAs(unsigned ID, const ExtendedType *Ty,
+                                       ExtendedType::TypeKind WantedType) {
   std::string Buffer;
   raw_string_ostream StrBuf(Buffer);
-  if (ID >= TypeIDValues.size()) {
-    StrBuf << "Type index " << ID << " out of range: can't install.";
+  if (Ty == nullptr) {
+    StrBuf << "Can't find extend type for type id: " << ID;
   } else {
-    // Must be case that index already defined.
-    StrBuf << "Type index " << ID << " defined as " << *TypeIDValues[ID]
-           << " and " << *Ty << ".";
+    StrBuf << "Type id " << ID << " not " << WantedType << ". Found: " << *Ty;
   }
   Error(StrBuf.str());
 }
@@ -380,7 +518,7 @@ protected:
     const NaClBitcodeRecord::RecordVector &Values = Record.GetValues();
     if (Values.size() == Size)
       return true;
-    ReportRecordSizeError(Size, RecordName, NULL);
+    ReportRecordSizeError(Size, RecordName, nullptr);
     return false;
   }
 
@@ -418,7 +556,7 @@ protected:
 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,
@@ -474,10 +612,13 @@ private:
   unsigned NextTypeId;
 
   void ProcessRecord() override;
+
+  void setNextTypeIDAsSimpleType(Ice::Type Ty) {
+    Context->getUndefinedTypeByID(NextTypeId++)->setAsSimpleType(Ty);
+  }
 };
 
 void TypesParser::ProcessRecord() {
-  Type *Ty = NULL;
   const NaClBitcodeRecord::RecordVector &Values = Record.GetValues();
   switch (Record.GetCode()) {
   case naclbitc::TYPE_CODE_NUMENTRY:
@@ -490,71 +631,141 @@ void TypesParser::ProcessRecord() {
     // VOID
     if (!isValidRecordSize(0, "Type void"))
       return;
-    Ty = Context->convertToLLVMType(Ice::IceType_void);
-    break;
+    setNextTypeIDAsSimpleType(Ice::IceType_void);
+    return;
   case naclbitc::TYPE_CODE_FLOAT:
     // FLOAT
     if (!isValidRecordSize(0, "Type float"))
       return;
-    Ty = Context->convertToLLVMType(Ice::IceType_f32);
-    break;
+    setNextTypeIDAsSimpleType(Ice::IceType_f32);
+    return;
   case naclbitc::TYPE_CODE_DOUBLE:
     // DOUBLE
     if (!isValidRecordSize(0, "Type double"))
       return;
-    Ty = Context->convertToLLVMType(Ice::IceType_f64);
-    break;
+    setNextTypeIDAsSimpleType(Ice::IceType_f64);
+    return;
   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:
+      setNextTypeIDAsSimpleType(Ice::IceType_i1);
+      return;
+    case 8:
+      setNextTypeIDAsSimpleType(Ice::IceType_i8);
+      return;
+    case 16:
+      setNextTypeIDAsSimpleType(Ice::IceType_i16);
+      return;
+    case 32:
+      setNextTypeIDAsSimpleType(Ice::IceType_i32);
+      return;
+    case 64:
+      setNextTypeIDAsSimpleType(Ice::IceType_i64);
+      return;
+    default:
+      break;
+    }
+    {
       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;
+    return;
   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],
-                                    Context->convertToIceType(BaseTy));
-    if (Ty == NULL) {
+    Ice::Type BaseTy = Context->getSimpleTypeByID(Values[1]);
+    Ice::SizeT Size = Values[0];
+    switch (BaseTy) {
+    case Ice::IceType_i1:
+      switch (Size) {
+      case 4:
+        setNextTypeIDAsSimpleType(Ice::IceType_v4i1);
+        return;
+      case 8:
+        setNextTypeIDAsSimpleType(Ice::IceType_v8i1);
+        return;
+      case 16:
+        setNextTypeIDAsSimpleType(Ice::IceType_v16i1);
+        return;
+      default:
+        break;
+      }
+      break;
+    case Ice::IceType_i8:
+      if (Size == 16) {
+        setNextTypeIDAsSimpleType(Ice::IceType_v16i8);
+        return;
+      }
+      break;
+    case Ice::IceType_i16:
+      if (Size == 8) {
+        setNextTypeIDAsSimpleType(Ice::IceType_v8i16);
+        return;
+      }
+      break;
+    case Ice::IceType_i32:
+      if (Size == 4) {
+        setNextTypeIDAsSimpleType(Ice::IceType_v4i32);
+        return;
+      }
+      break;
+    case Ice::IceType_f32:
+      if (Size == 4) {
+        setNextTypeIDAsSimpleType(Ice::IceType_v4f32);
+        return;
+      }
+      break;
+    default:
+      break;
+    }
+    {
       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;
+    return;
   }
   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");
+    ExtendedType *Ty = Context->getUndefinedTypeByID(NextTypeId++);
+    Ty->setAsFunctionType();
+    FuncSigExtendedType *FcnTy = cast<FuncSigExtendedType>(Ty);
+    FcnTy->setReturnType(Context->getSimpleTypeByID(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->getSimpleTypeByID(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]);
-    break;
+    return;
   }
   default:
     BlockParserBaseClass::ProcessRecord();
     return;
   }
-  // If Ty not defined, assume error. Use void as filler.
-  if (Ty == NULL)
-    Ty = Context->convertToLLVMType(Ice::IceType_void);
-  Context->setTypeID(NextTypeId++, Ty);
+  // We shoudn't reach here!
+  assert(false);
 }
 
 /// Parses the globals block (i.e. global variables).
@@ -636,7 +847,7 @@ private:
   // 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");
@@ -740,7 +951,7 @@ void GlobalsParser::ProcessRecord() {
     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];
@@ -956,7 +1167,7 @@ private:
     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;
@@ -1007,7 +1218,7 @@ private:
       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!";
@@ -1033,7 +1244,7 @@ private:
 
     // If element not defined, set it.
     Ice::Operand *OldOp = LocalOperands[LocalIndex];
-    if (OldOp == NULL) {
+    if (OldOp == nullptr) {
       LocalOperands[LocalIndex] = Op;
       return;
     }
@@ -1473,7 +1684,7 @@ void FunctionParser::ProcessRecord() {
     if (!isValidRecordSize(3, "function block cast"))
       return;
     Ice::Operand *Src = getRelativeOperand(Values[0], BaseIndex);
-    Type *CastType = Context->getTypeByID(Values[1]);
+    Ice::Type CastType = Context->getSimpleTypeByID(Values[1]);
     Instruction::CastOps LLVMCastOp;
     Ice::InstCast::OpKind CastKind;
     if (!naclbitc::DecodeCastOpcode(Values[2], LLVMCastOp) ||
@@ -1484,18 +1695,18 @@ void FunctionParser::ProcessRecord() {
       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: {
@@ -1678,7 +1889,7 @@ void FunctionParser::ProcessRecord() {
     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 {
@@ -1696,7 +1907,7 @@ void FunctionParser::ProcessRecord() {
       }
       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));
@@ -1715,8 +1926,7 @@ void FunctionParser::ProcessRecord() {
     // 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->getSimpleTypeByID(Values[0]);
     if (!Ice::isScalarIntegerType(CondTy)) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
@@ -1784,7 +1994,7 @@ void FunctionParser::ProcessRecord() {
       Error(StrBuf.str());
       return;
     }
-    Ice::Type Ty = Context->convertToIceType(Context->getTypeByID(Values[0]));
+    Ice::Type Ty = Context->getSimpleTypeByID(Values[0]);
     if (Ty == Ice::IceType_void) {
       Error("Phi record using type void not allowed");
       return;
@@ -1835,7 +2045,7 @@ void FunctionParser::ProcessRecord() {
       return;
     unsigned Alignment;
     extractAlignment("Load", Values[1], Alignment);
-    Ice::Type Ty = Context->convertToIceType(Context->getTypeByID(Values[2]));
+    Ice::Type Ty = Context->getSimpleTypeByID(Values[2]);
     if (!isValidLoadStoreAlignment(Alignment, Ty, "Load"))
       return;
     CurrentNode->appendInst(
@@ -1897,11 +2107,11 @@ void FunctionParser::ProcessRecord() {
     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;
@@ -1928,14 +2138,15 @@ void FunctionParser::ProcessRecord() {
         }
       }
     } else {
-      ReturnType = Context->convertToIceType(Context->getTypeByID(Values[2]));
+      ReturnType = Context->getSimpleTypeByID(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,
@@ -1999,8 +2210,7 @@ void FunctionParser::ProcessRecord() {
     // 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->getSimpleTypeByID(Values[1])));
     break;
   }
   default:
@@ -2049,8 +2259,7 @@ void ConstantsParser::ProcessRecord() {
     // SETTYPE: [typeid]
     if (!isValidRecordSize(1, "constants block set type"))
       return;
-    NextConstantType =
-        Context->convertToIceType(Context->getTypeByID(Values[0]));
+    NextConstantType = Context->getSimpleTypeByID(Values[0]);
     if (NextConstantType == Ice::IceType_void)
       Error("constants block set type not allowed for void type");
     return;
@@ -2255,7 +2464,7 @@ private:
 
 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;
@@ -2319,15 +2528,7 @@ void ModuleParser::ProcessRecord() {
     // 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::FuncSigType &Ty = Context->getFuncSigTypeByID(Values[0]);
     CallingConv::ID CallingConv;
     if (!naclbitc::DecodeCallingConv(Values[1], CallingConv)) {
       std::string Buffer;
@@ -2345,7 +2546,14 @@ void ModuleParser::ProcessRecord() {
       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();