Start processing function blocks in Subzero.

src/PNaClTranslator.cpp

Index: src/PNaClTranslator.cpp
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index 7f9c514542a73cbebec36d9cb3a2bbd9ad15515b..cc36340368b9ff5d21634d17338104c0b7495ea7 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -14,6 +14,13 @@
 
 #include "PNaClTranslator.h"

[Jim Stichnoth, 2014/07/17 23:00:33]

Alphabetize this after Ice*.h if possible.

[Karl, 2014/07/18 20:27:43]

I intentionally placed this header file first, following the style convention of
always including the corresponding .h file first to verify it has all needed
include files.

 #include "IceCfg.h"
+#include "IceCfgNode.h"
+#include "IceClFlags.h"
+#include "IceDefs.h"
+#include "IceInst.h"
+#include "IceOperand.h"
+#include "IceTypeConverter.h"
+#include "llvm/Analysis/NaCl/PNaClABITypeChecker.h"
 #include "llvm/Bitcode/NaCl/NaClBitcodeDecoders.h"
 #include "llvm/Bitcode/NaCl/NaClBitcodeHeader.h"
 #include "llvm/Bitcode/NaCl/NaClBitcodeParser.h"
@@ -39,18 +46,26 @@ class TopLevelParser : public NaClBitcodeParser {
   TopLevelParser &operator=(const TopLevelParser &) LLVM_DELETED_FUNCTION;
 
 public:
-  TopLevelParser(const std::string &InputName, NaClBitcodeHeader &Header,
+  TopLevelParser(Ice::Translator &Translator,
+                 const std::string &InputName, NaClBitcodeHeader &Header,
                  NaClBitstreamCursor &Cursor, bool &ErrorStatus)
       : NaClBitcodeParser(Cursor),
+        Translator(Translator),
         Mod(new Module(InputName, getGlobalContext())), Header(Header),
         ErrorStatus(ErrorStatus), NumErrors(0), NumFunctionIds(0),
-        GlobalVarPlaceHolderType(Type::getInt8Ty(getLLVMContext())) {
+        NumFunctionBlocks(0),
+        GlobalVarPlaceHolderType(Type::getInt8Ty(getLLVMContext())),
+        TypeConverter(getLLVMContext()) {
     Mod->setDataLayout(PNaClDataLayout);
   }
 
   virtual ~TopLevelParser() {}
   LLVM_OVERRIDE;
 
+  Ice::Translator &getTranslator() {
+    return Translator;
+  }
+
   virtual bool Error(const std::string &Message) LLVM_OVERRIDE {
     ErrorStatus = true;
     ++NumErrors;
@@ -104,6 +119,15 @@ public:
     DefiningFunctionsList.push_back(ValueIDValues.size());
   }
 
+  /// Returns the value id that should be associated with the the

[jvoung (off chromium), 2014/07/17 23:57:56]

the the -> the

[Karl, 2014/07/18 20:27:43]

Done.

+  /// current function block. Increments internal counters during call
+  /// so that it will be in correct position for next function block.
+  unsigned getNextFunctionBlockValueID() {
+    if (DefiningFunctionsList.size() <= NumFunctionBlocks)
+      report_fatal_error("More function blocks than defined function addresses");

[Jim Stichnoth, 2014/07/17 23:00:33]

80-char

[Karl, 2014/07/18 20:27:43]

Done.

+    return DefiningFunctionsList[NumFunctionBlocks++];
+  }
+
   /// Returns the LLVM IR value associatd with the global value ID.
   Value *getGlobalValueByID(unsigned ID) const {
     if (ID >= ValueIDValues.size())
@@ -162,7 +186,35 @@ public:
     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.
+  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);
+  }
+
 private:
+  // The translator associated with the parser.
+  Ice::Translator &Translator;
   // The parsed module.
   OwningPtr<Module> Mod;
   // The bitcode header.
@@ -177,12 +229,16 @@ private:
   std::vector<WeakVH> ValueIDValues;
   // 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;
+  // Converter between LLVM and ICE types.
+  Ice::TypeConverter TypeConverter;
 
   virtual bool ParseBlock(unsigned BlockID) LLVM_OVERRIDE;
 
@@ -192,6 +248,10 @@ private:
 
   /// Reports error about bad call to setTypeID.
   void reportBadSetTypeID(unsigned ID, Type *Ty);
+
+  // Reports that there is no corresponding ICE type for LlvmTy.
+  // returns ICE::IceType_void.
+  Ice::Type convertToIceTypeError(Type *LlvmTy);
 };
 
 Type *TopLevelParser::reportTypeIDAsUndefined(unsigned ID) {
@@ -219,6 +279,14 @@ void TopLevelParser::reportBadSetTypeID(unsigned ID, Type *Ty) {
   Error(StrBuf.str());
 }
 
+Ice::Type TopLevelParser::convertToIceTypeError(Type *LlvmTy) {
+  std::string Buffer;
+  raw_string_ostream StrBuf(Buffer);
+  StrBuf << "Invalid LLVM type: " << *LlvmTy;
+  Error(StrBuf.str());
+  return Ice::IceType_void;
+}
+
 // Base class for parsing blocks within the bitcode file.  Note:
 // Because this is the base class of block parsers, we generate error
 // messages if ParseBlock or ParseRecord is not overridden in derived
@@ -240,6 +308,11 @@ protected:
       : NaClBitcodeParser(BlockID, EnclosingParser),
         Context(EnclosingParser->Context) {}
 
+  // Gets the translator associated with the bitcode parser.
+  Ice::Translator &getTranslator() {
+    return Context->getTranslator();
+  }
+
   // Generates an error Message with the bit address prefixed to it.
   virtual bool Error(const std::string &Message) LLVM_OVERRIDE {
     uint64_t Bit = Record.GetStartBit() + Context->getHeaderSize() * 8;
@@ -367,33 +440,51 @@ void TypesParser::ProcessRecord() {
     // VOID
     if (checkRecordSize(0, "Type void"))
       break;
-    Ty = Type::getVoidTy(Context->getLLVMContext());
+    Ty = Context->convertToLlvmType(Ice::IceType_void);

[jvoung (off chromium), 2014/07/17 23:57:56]

Is this mostly because it's cheaper to check the converter table than to use the
LLVMContext?

Or that it'll be easier to write this as a plain "Ice::IceType_void" later?

Are all the uses of "Type::foo(...getLLVMContext())" meant to be changed? 
Otherwise, there are other cases where that's not rewritten.

[Karl, 2014/07/18 20:27:43]

What I was trying to do is, whenever possible, always describe the types in
terms of ICE types, for consistency. While the use of LLVM type creators may be
relatively fast (via the LLVM context), there are no guarantees. On the other
hand, when using the TypeConverter, we know it is nothing more than indexing a
vector.

I've gone ahead and fixed all other cases (that I could) find, where there is a
corresponding Ice::Type to use.

There are two cases where I can't do this:

(1) Function signatures, since Ice::Type doesn't implement this.

(2) Global variable initialization, which uses a byte array.

     break;
   case naclbitc::TYPE_CODE_FLOAT:
     // FLOAT
     if (checkRecordSize(0, "Type float"))
       break;
-    Ty = Type::getFloatTy(Context->getLLVMContext());
+    Ty = Context->convertToLlvmType(Ice::IceType_f32);
     break;
   case naclbitc::TYPE_CODE_DOUBLE:
     // DOUBLE
     if (checkRecordSize(0, "Type double"))
       break;
-    Ty = Type::getDoubleTy(Context->getLLVMContext());
+    Ty = Context->convertToLlvmType(Ice::IceType_f64);
     break;
   case naclbitc::TYPE_CODE_INTEGER:
     // INTEGER: [width]
     if (checkRecordSize(1, "Type integer"))
       break;
-    Ty = IntegerType::get(Context->getLLVMContext(), Values[0]);
-    // TODO(kschimpf) Check if size is legal.
+    Ty = Context->getLlvmIntegerType(Values[0]);
+    if (Ty == NULL) {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << "Type integer record with invalid bitsize: " << Values[0];
+      Error(StrBuf.str());
+      // Fix type so that we can continue.
+      Ty = Context->convertToLlvmType(Ice::IceType_i32);
+    }
     break;
-  case naclbitc::TYPE_CODE_VECTOR:
+  case naclbitc::TYPE_CODE_VECTOR: {
     // VECTOR: [numelts, eltty]
     if (checkRecordSize(2, "Type vector"))
       break;
-    Ty = VectorType::get(Context->getTypeByID(Values[1]), Values[0]);
+    Type *BaseTy = Context->getTypeByID(Values[1]);
+    Ty = Context->getLlvmVectorType(Values[0],
+                                    Context->convertToIceType(BaseTy));
+    if (Ty == NULL) {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      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 (checkRecordSizeAtLeast(2, "Type signature"))
@@ -685,6 +776,387 @@ void ValuesymtabParser::ProcessRecord() {
   return;
 }
 
+/// Parses function blocks in the bitcode file.
+class FunctionParser : public BlockParserBaseClass {
+public:
+  FunctionParser(unsigned BlockID, BlockParserBaseClass *EnclosingParser)
+      : BlockParserBaseClass(BlockID, EnclosingParser),
+        Func(new Ice::Cfg(getTranslator().getContext())),
+        CurrentBbIndex(0),
+        FcnId(Context->getNextFunctionBlockValueID()),
+        LlvmFunc(cast<Function>(Context->getGlobalValueByID(FcnId))),
+        CachedNumGlobalValueIDs(Context->getNumGlobalValueIDs()),
+        InstIsTerminating(false)
+  {
+    Func->setFunctionName(LlvmFunc->getName());
+    Func->setReturnType(Context->convertToIceType(LlvmFunc->getReturnType()));
+    Func->setInternal(LlvmFunc->hasInternalLinkage());
+    CurrentNode = InstallNextBasicBlock();
+    for (Function::const_arg_iterator ArgI = LlvmFunc->arg_begin(),
+                                      ArgE = LlvmFunc->arg_end();
+         ArgI != ArgE; ++ArgI) {
+      Func->addArg(NextInstVar(Context->convertToIceType(ArgI->getType())));
+    }
+  }
+
+  ~FunctionParser() LLVM_OVERRIDE;
+
+private:
+  // Timer for reading function bitcode and converting to ICE.
+  Ice::Timer TConvert;
+  // The corresponding ICE function defined by the function block.
+  Ice::Cfg *Func;
+  // The index to the current basic block being built.
+  uint32_t CurrentBbIndex;
+  // The basic block being built.
+  Ice::CfgNode *CurrentNode;
+  // The ID for the function.
+  unsigned FcnId;
+  // The corresponding llvm function.
+  Function *LlvmFunc;
+  // Holds variables local to the function block.
+  std::vector<Ice::Operand *> LocalVars;
+  // Holds the list of basic blocks defined for the function.
+  std::vector<Ice::CfgNode*> Nodes;
+  // Holds the dividing point between local and global absolute value indices.
+  uint32_t CachedNumGlobalValueIDs;
+  // True if the last processed instruction was a terminating
+  // instruction.
+  bool InstIsTerminating;
+
+  virtual void ProcessRecord() LLVM_OVERRIDE;
+
+  // Creates and appends a new basic block to the list of basic blocks.
+  Ice::CfgNode *InstallNextBasicBlock() {
+    std::string Buffer;
+    raw_string_ostream StrBuf(Buffer);
+    // TODO(kschimpf): Make (basic block) nodes make up the name if needed.
+    StrBuf << "bb" << Nodes.size();
+    Ice::CfgNode *Node = Func->makeNode(StrBuf.str());
+    Nodes.push_back(Node);
+    return Node;
+  }
+
+  // Returns the Index-th basic block in the list of basic blocks.
+  Ice::CfgNode *GetBasicBlock(uint32_t Index) {
+    if (Index >= Nodes.size()) {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << "Reference to basic block " << Index
+             << " not found. Must be less than " << Nodes.size();
+      Error(StrBuf.str());
+      Index = 0;
+    }
+    return Nodes[Index];
+  }
+
+  // Generates the next available local variable using the given
+  // type.  Note: if Ty is void, this function returns NULL.
+  Ice::Variable *NextInstVar(Ice::Type Ty) {
+    if (Ty == Ice::IceType_void)
+      return NULL;
+    Ice::Variable *Var = Func->makeVariable(Ty, CurrentNode);
+    LocalVars.push_back(Var);
+    return Var;
+  }
+
+  // Converts a relative index (to the next instruction to be read) to
+  // an absolute value index.
+  uint32_t convertRelativeToAbsIndex(int32_t Id) {
+    int32_t AbsNextId = CachedNumGlobalValueIDs + LocalVars.size();
+    if (Id > 0 && AbsNextId < static_cast<uint32_t>(Id)) {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << "Invalid relative value id: " << Id
+             << " (must be <= " << AbsNextId << ")";
+      Error(StrBuf.str());
+      // Fix so that we can go on.

[jvoung (off chromium), 2014/07/17 23:57:56]

Hmm, while this is still a bit early, what should the error model be?

(a) abort immediately
(b) perform these fixups and continue a bit more, then abort at some point

How much less friendly is (a), if friendliness is the advantage of (b) over (a)?
What are the other advantages of (b)? Batched testing of errors?

What is the consequence of forgetting to do a fixup and then continuing? Using
uninitialized memory?

[Karl, 2014/07/18 20:27:43]

The goal of error recovery is to make it easier to incrementally grow. I don't
need (necessarily) tweak examples down to the subset that is currently
implemented if error recovery occurs.

To deal with this, I'm adding a command line flag to control if error recovery
is allowed. When false (the default), method Error will call report_fatal_error.
When true, it will report the error and return.

+      return 0;
+    }
+    return AbsNextId - Id;
+  }
+
+  // Returns the value referenced by the given value Index.
+  Ice::Operand *getOperand(uint32_t Index) {
+    if (Index < CachedNumGlobalValueIDs) {
+      // TODO(kschimpf): Define implementation.
+      report_fatal_error("getOperand of global addresses not implemented");
+    }
+    uint32_t LocalIndex = Index - CachedNumGlobalValueIDs;
+    if (LocalIndex >= LocalVars.size()) {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << "Value index " << Index << " out of range. Must be less than "
+             << (LocalVars.size() + CachedNumGlobalValueIDs);
+      // TODO(kschimpf): Fix to recover.
+      report_fatal_error(StrBuf.str());
+    }
+    return LocalVars[LocalIndex];
+  }
+
+  // Returns true if LLVM float/double type.
+  bool isFloatingType(Type *Ty) {
+    return Ty->isFloatTy() || Ty->isDoubleTy();
+  }
+
+  // Checks if integer arithmetic Op, for type OpTy, is valid.
+  // Returns false if valid. Otherwise generates error message and
+  // returns true.
+  bool checkIfIntArithmeticOp(Ice::InstArithmetic::OpKind Op, Type *OpTy) {

[jvoung (off chromium), 2014/07/17 23:57:55]

Some of these functions take LLVM types. At some point these should work without
the LLVM type right?

[Karl, 2014/07/18 20:27:43]

That would be ideal. However, I'm not sure how we can do this since the code is
also used by the PNaCl ABI verifier for LLVM code.

I do agree that we should switch the top-level calls to use ICE types. So I
don't forget, I'm doing that switch now.

+    if (!PNaClABITypeChecker::isValidIntArithmeticType(OpTy)) {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << Ice::InstArithmetic::getOpName(Op)
+             << ": Invalid integer arithmetic type: " << *OpTy;
+      return Error(StrBuf.str());
+    }
+    return false;
+  }
+
+  // Checks if integer (or vector of integers) arithmetic Op, for type
+  // OpTy, is valid.  Returns false if valid. Otherwise generates
+  // error message and returns true.
+  bool checkIfIntegerOrVectorOp(Ice::InstArithmetic::OpKind Op, Type *OpTy) {
+    Type *BaseTy = OpTy;
+    if (OpTy->isVectorTy()) {
+      if (!PNaClABITypeChecker::isValidVectorType(OpTy)) {
+        std::string Buffer;
+        raw_string_ostream StrBuf(Buffer);
+        StrBuf << Ice::InstArithmetic::getOpName(Op)
+               << ": invalid vector type: " << *OpTy;
+        return Error(StrBuf.str());
+      }
+      BaseTy = OpTy->getVectorElementType();
+    }
+    if (BaseTy->isIntegerTy()) {
+      if (PNaClABITypeChecker::isValidScalarType(BaseTy)) return false;
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << Ice::InstArithmetic::getOpName(Op)
+             << ": Invalid integer type: " << *OpTy;
+      return Error(StrBuf.str());
+    } else {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << Ice::InstArithmetic::getOpName(Op)
+             << ": Expects integer type. Found: " << *OpTy;
+      return Error(StrBuf.str());
+    }
+    return false;
+  }
+
+  // Checks if floating arithmetic Op, for type OpTy, is valid.
+  // Returns false if valid. Otherwise generates an error message and
+  // returns true.
+  bool checkIfFloatingOrVectorOp(Ice::InstArithmetic::OpKind Op, Type *OpTy) {
+    Type *BaseTy = OpTy;
+    if (OpTy->isVectorTy()) {
+      if (!PNaClABITypeChecker::isValidVectorType(OpTy)) {
+        std::string Buffer;
+        raw_string_ostream StrBuf(Buffer);
+        StrBuf << Ice::InstArithmetic::getOpName(Op)
+               << ": invalid vector type: " << *OpTy;
+        return Error(StrBuf.str());
+      }
+      BaseTy = OpTy->getVectorElementType();
+    }
+    if (!isFloatingType(BaseTy)) {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << Ice::InstArithmetic::getOpName(Op)
+             << ": Expects floating point. Found " << *OpTy;
+      return Error(StrBuf.str());
+    }
+    if (!PNaClABITypeChecker::isValidScalarType(BaseTy)) {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << Ice::InstArithmetic::getOpName(Op)
+             << ": type not allowed: " << *OpTy;
+      return Error(StrBuf.str());
+    }
+    return false;
+  }
+
+  /// Takes the PNaCl bitcode binary operator Opcode, and the opcode
+  /// type Ty, and sets Op to the corresponding ICE binary
+  /// opcode. Returns false if able to convert, true otherwise.
+  bool convertBinopOpcode(unsigned Opcode,
+                          Ice::Type Ty,
+                          Ice::InstArithmetic::OpKind &Op) {
+    Instruction::BinaryOps LLVMOpcode;
+    Type *LlvmTy = Context->convertToLlvmType(Ty);
+    if (!naclbitc::DecodeBinaryOpcode(Opcode, LlvmTy, LLVMOpcode)) {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << "Binary opcode not understood: " << Opcode;
+      Error(StrBuf.str());
+      Op = Ice::InstArithmetic::Add;
+      return true;
+    }
+    Op = Ice::InstArithmetic::Add;
+    switch (LLVMOpcode) {
+    default: {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << "Binary opcode not understood: " << Opcode;
+      Error(StrBuf.str());
+      return true;
+    }
+    case Instruction::Add:
+      Op = Ice::InstArithmetic::Add;
+      return checkIfIntArithmeticOp(Op, LlvmTy);
+    case Instruction::FAdd:
+      Op = Ice::InstArithmetic::Fadd;
+      return checkIfFloatingOrVectorOp(Op, LlvmTy);
+    case Instruction::Sub:
+      Op = Ice::InstArithmetic::Sub;
+      return checkIfIntArithmeticOp(Op, LlvmTy);
+    case Instruction::FSub:
+      Op = Ice::InstArithmetic::Fsub;
+      return checkIfFloatingOrVectorOp(Op, LlvmTy);
+    case Instruction::Mul:
+      Op = Ice::InstArithmetic::Mul;
+      return checkIfIntArithmeticOp(Op, LlvmTy);
+    case Instruction::FMul:
+      Op = Ice::InstArithmetic::Fmul;
+      return checkIfFloatingOrVectorOp(Op, LlvmTy);
+    case Instruction::UDiv:
+      Op = Ice::InstArithmetic::Udiv;
+      return checkIfIntArithmeticOp(Op, LlvmTy);
+    case Instruction::SDiv:
+      Op = Ice::InstArithmetic::Sdiv;
+      return checkIfIntArithmeticOp(Op, LlvmTy);
+    case Instruction::FDiv:
+      Op = Ice::InstArithmetic::Fdiv;
+      return checkIfFloatingOrVectorOp(Op, LlvmTy);
+    case Instruction::URem:
+      Op = Ice::InstArithmetic::Urem;
+      return checkIfIntArithmeticOp(Op, LlvmTy);
+    case Instruction::SRem:
+      Op = Ice::InstArithmetic::Srem;
+      return checkIfIntArithmeticOp(Op, LlvmTy);
+    case Instruction::FRem:
+      Op = Ice::InstArithmetic::Frem;
+      return checkIfFloatingOrVectorOp(Op, LlvmTy);
+    case Instruction::Shl:
+      Op = Ice::InstArithmetic::Shl;
+      return checkIfIntArithmeticOp(Op, LlvmTy);
+    case Instruction::LShr:
+      Op = Ice::InstArithmetic::Lshr;
+      return checkIfIntArithmeticOp(Op, LlvmTy);
+    case Instruction::AShr:
+      Op = Ice::InstArithmetic::Ashr;
+      return checkIfIntArithmeticOp(Op, LlvmTy);
+    case Instruction::And:
+      Op = Ice::InstArithmetic::And;
+      return checkIfIntegerOrVectorOp(Op, LlvmTy);
+    case Instruction::Or:
+      Op = Ice::InstArithmetic::Or;
+      return checkIfIntegerOrVectorOp(Op, LlvmTy);
+    case Instruction::Xor:
+      Op = Ice::InstArithmetic::Or;

[jvoung (off chromium), 2014/07/17 23:57:56]

Xor

[Karl, 2014/07/18 20:27:43]

Done.

+      return checkIfIntegerOrVectorOp(Op, LlvmTy);
+    }
+  }
+};
+
+FunctionParser::~FunctionParser() {
+  if (getTranslator().getFlags().SubzeroTimingEnabled) {
+    errs() << "[Subzero timing] Convert function "
+           << Func->getFunctionName() << ": " << TConvert.getElapsedSec()
+           << " sec\n";
+  }
+  // Before translating, check for blocks without instructions, and
+  // insert unreachable. This shouldn't happen, but be safe.
+  unsigned Index = 0;
+  for (std::vector<Ice::CfgNode*>::iterator
+           Iter = Nodes.begin(), IterEnd = Nodes.end();
+       Iter != IterEnd; ++Iter, ++Index) {
+    Ice::CfgNode *Node = *Iter;
+    if (Node->getInsts().size() == 0) {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << "Basic block " << Index << " contains no instructions";
+      Error(StrBuf.str());
+      Node->appendInst(Ice::InstUnreachable::create(Func));
+    }
+  }
+  getTranslator().translateFcn(Func);
+}
+
+void FunctionParser::ProcessRecord() {
+  const NaClBitcodeRecord::RecordVector &Values = Record.GetValues();
+  if (InstIsTerminating) {
+    InstIsTerminating = false;
+    CurrentNode = GetBasicBlock(++CurrentBbIndex);
+  }
+  Ice::Inst *Inst = NULL;
+  switch (Record.GetCode()) {
+  case naclbitc::FUNC_CODE_DECLAREBLOCKS: {
+    // DECLAREBLOCKS: [n]
+    if (checkRecordSize(1, "function block count"))
+      break;
+    if (Nodes.size() != 1) {
+      Error("Duplicate function block count record");
+      return;
+    }
+    uint32_t NumBbs = Values[0];
+    if (NumBbs == 0) {
+      Error("Functions must contain at least one basic block.");
+      NumBbs = 1;
+    }
+    // Install the basic blocks, skipping bb0 which was created in the
+    // constructor.
+    for (size_t i = 1; i < NumBbs; ++i)
+      InstallNextBasicBlock();
+    break;
+  }
+  case naclbitc::FUNC_CODE_INST_BINOP: {
+    // BINOP: [opval, opval, opcode]
+    if (checkRecordSize(3, "function block binop"))
+      break;
+    Ice::Operand *Op1 = getOperand(convertRelativeToAbsIndex(Values[0]));
+    Ice::Operand *Op2 = getOperand(convertRelativeToAbsIndex(Values[1]));
+    Ice::Type Type1 = Op1->getType();
+    Ice::Type Type2 = Op2->getType();
+    if (Type1 != Type2) {
+      std::string Buffer;
+      raw_string_ostream StrBuf(Buffer);
+      StrBuf << "Binop argument types differ: " << Type1 << " and " << Type2;
+      Error(StrBuf.str());
+      // Fix so that we can continue.
+      Op2 = Op1;
+    }
+
+    Ice::InstArithmetic::OpKind Opcode;
+    if (convertBinopOpcode(Values[2], Type1, Opcode))
+      break;
+    Ice::Variable *Dest = NextInstVar(Type1);
+    Inst = Ice::InstArithmetic::create(Func, Opcode, Dest, Op1, Op2);
+    break;
+  }
+  case naclbitc::FUNC_CODE_INST_RET: {
+    // RET: [opval?]
+    InstIsTerminating = true;
+    if (checkRecordSizeInRange(0, 1, "function block ret"))
+      break;
+    if (Values.size() == 0) {
+      Inst = Ice::InstRet::create(Func);
+    } else {
+      Inst = Ice::InstRet::create(
+          Func, getOperand(convertRelativeToAbsIndex(Values[0])));
+    }
+    break;
+  }
+  default:
+    // Generate error message!
+    BlockParserBaseClass::ProcessRecord();
+    break;
+  }
+  if (Inst)
+    CurrentNode->appendInst(Inst);
+}
+
 /// Parses the module block in the bitcode file.
 class ModuleParser : public BlockParserBaseClass {
 public:
@@ -716,9 +1188,8 @@ bool ModuleParser::ParseBlock(unsigned BlockID) LLVM_OVERRIDE {
     return Parser.ParseThisBlock();
   }
   case naclbitc::FUNCTION_BLOCK_ID: {
-    Error("Function block parser not yet implemented, skipping");
-    SkipBlock();
-    return false;
+    FunctionParser Parser(BlockID, this);
+    return Parser.ParseThisBlock();
   }
   default:
     return BlockParserBaseClass::ParseBlock(BlockID);
@@ -788,13 +1259,7 @@ void ModuleParser::ProcessRecord() {
 bool TopLevelParser::ParseBlock(unsigned BlockID) {
   if (BlockID == naclbitc::MODULE_BLOCK_ID) {
     ModuleParser Parser(BlockID, this);
-    bool ReturnValue = Parser.ParseThisBlock();
-    // TODO(kschimpf): Remove once translating function blocks.
-    errs() << "Global addresses:\n";
-    for (size_t i = 0; i < ValueIDValues.size(); ++i) {
-      errs() << "[" << i << "]: " << *ValueIDValues[i] << "\n";
-    }
-    return ReturnValue;
+    return Parser.ParseThisBlock();
   }
   // Generate error message by using default block implementation.
   BlockParserBaseClass Parser(BlockID, this);
@@ -836,8 +1301,8 @@ void PNaClTranslator::translate(const std::string &IRFilename) {
   NaClBitstreamReader InputStreamFile(BufPtr, EndBufPtr);
   NaClBitstreamCursor InputStream(InputStreamFile);
 
-  TopLevelParser Parser(MemBuf->getBufferIdentifier(), Header, InputStream,
-                        ErrorStatus);
+  TopLevelParser Parser(*this, MemBuf->getBufferIdentifier(), Header,
+                        InputStream, ErrorStatus);
   int TopLevelBlocks = 0;
   while (!InputStream.AtEndOfStream()) {
     if (Parser.Parse()) {