Allow Subzero to parse function blocks in parallel.

pydir/run-pnacl-sz.py

@@ -94,6 +94,9 @@ def main():
                            help='Input is textual bitcode (not .ll)')
     argparser.add_argument('--expect-fail', required=False, action='store_true',
                            help='Negate success of run by using LLVM not')
+    argparser.add_argument('--allow-pnacl-reader-error-recovery',
+                           action='store_true',
+                           help='Continue parsing after first error')
     argparser.add_argument('--args', '-a', nargs=argparse.REMAINDER,
                            default=[],
                            help='Remaining arguments are passed to pnacl-sz')
@@ -118,7 +121,9 @@ def main():
       raise RuntimeError("Can't specify both '--tbc' and '--llvm'")
 
     if args.forceasm:
-      if args.filetype == 'asm':
+      if args.expect_fail:
+        args.forceasm = False
+      elif args.filetype == 'asm':
         pass
       elif args.filetype == 'iasm':
         # TODO(sehr) implement forceasm for iasm.
@@ -148,6 +153,8 @@ def main():
       # single-threaded translation because dump output does not get
       # reassembled into order.
       cmd += ['-verbose', 'inst,global_init', '-notranslate', '-threads=0']
+    elif args.allow_pnacl_reader_error_recovery:
+      cmd += ['-allow-pnacl-reader-error-recovery', '-threads=0']
     if not args.llvm_source:
       cmd += ['--bitcode-format=pnacl']
       if not args.no_local_syms:

src/IceClFlags.def

@@ -196,6 +196,9 @@ struct dev_list_flag {};
                    "Low-level integrated assembly ('.s') file"),               \
         clEnumValEnd))                                                         \
                                                                                \
+  X(ParseParallel, bool, dev_opt_flag, "parse-parallel",                       \
+    cl::desc("Parse function blocks in parallel"), cl::init(true))             \
+                                                                               \
   X(RandomizeAndPoolImmediatesOption, Ice::RandomizeAndPoolImmediatesEnum,     \
     dev_opt_flag, "randomize-pool-immediates",                                 \
     cl::desc("Randomize or pooling the representation of immediates"),         \

src/IceCompiler.cpp

@@ -62,8 +62,6 @@ void Compiler::run(const Ice::ClFlags &Flags, GlobalContext &Ctx,
   // allows only --filetype=obj. Check here to avoid cryptic error messages
   // downstream.
   if (!BuildDefs::dump() && Ctx.getFlags().getOutFileType() != FT_Elf) {
-    // TODO(stichnot): Access the actual command-line argument via
-    // llvm::Option.ArgStr and .ValueStr .
     Ctx.getStrError()
         << "Error: only --filetype=obj is supported in this build.\n";
     Ctx.getErrorStatus()->assign(EC_Args);
@@ -89,6 +87,7 @@ void Compiler::run(const Ice::ClFlags &Flags, GlobalContext &Ctx,
       Ctx.getStrError()
           << "non BuildOnRead is not supported w/ PNACL_BROWSER_TRANSLATOR\n";
       Ctx.getErrorStatus()->assign(EC_Args);
+      Ctx.waitForWorkerThreads();
       return;
     }
     // Globals must be kept alive after lowering when converting from LLVM to
@@ -107,6 +106,7 @@ void Compiler::run(const Ice::ClFlags &Flags, GlobalContext &Ctx,
     if (!Mod) {
       Err.print(Flags.getAppName().c_str(), llvm::errs());
       Ctx.getErrorStatus()->assign(EC_Bitcode);
+      Ctx.waitForWorkerThreads();
       return;
     }
 
@@ -117,6 +117,7 @@ void Compiler::run(const Ice::ClFlags &Flags, GlobalContext &Ctx,
     Ctx.getStrError() << "Error: Build doesn't allow LLVM IR, "
                       << "--build-on-read=0 not allowed\n";
     Ctx.getErrorStatus()->assign(EC_Args);
+    Ctx.waitForWorkerThreads();
     return;
   }
 

src/IceGlobalContext.cpp

@@ -213,6 +213,37 @@ public:
   UndefPool Undefs;
 };
 
+void GlobalContext::waitForWorkerThreads() {
+  if (WaitForWorkerThreadsCalled.exchange(true))
+    return;
+  optQueueNotifyEnd();
+  for (std::thread &Worker : TranslationThreads) {
+    Worker.join();
+  }
+  TranslationThreads.clear();
+
+  // Only notify the emit queue to end after all the translation threads have
+  // ended.
+  emitQueueNotifyEnd();
+  for (std::thread &Worker : EmitterThreads) {
+    Worker.join();
+  }
+  EmitterThreads.clear();
+
+  if (BuildDefs::timers()) {
+    auto Timers = getTimers();
+    for (ThreadContext *TLS : AllThreadContexts)
+      Timers->mergeFrom(TLS->Timers);
+  }
+  if (BuildDefs::dump()) {
+    // Do a separate loop over AllThreadContexts to avoid holding two locks at
+    // once.
+    auto Stats = getStatsCumulative();
+    for (ThreadContext *TLS : AllThreadContexts)
+      Stats->add(TLS->StatsCumulative);
+  }
+}
+
 void GlobalContext::CodeStats::dump(const std::string &Name,
                                     GlobalContext *Ctx) {
   if (!BuildDefs::dump())
@@ -252,7 +283,10 @@ GlobalContext::GlobalContext(Ostream *OsDump, Ostream *OsEmit, Ostream *OsError,
     : Strings(new StringPool()), ConstPool(new ConstantPool()), ErrorStatus(),
       StrDump(OsDump), StrEmit(OsEmit), StrError(OsError), IntrinsicsInfo(this),
       ObjectWriter(), OptQ(/*Sequential=*/Flags.isSequential(),
-                           /*MaxSize=*/Flags.getNumTranslationThreads()),
+                           /*MaxSize=*/
+                           (Flags.getParseParallel() && Flags.getBuildOnRead())
+                               ? MaxOptQSize
+                               : Flags.getNumTranslationThreads()),
       // EmitQ is allowed unlimited size.
       EmitQ(/*Sequential=*/Flags.isSequential()),
       DataLowering(TargetDataLowering::createLowering(this)) {
@@ -305,7 +339,8 @@ GlobalContext::GlobalContext(Ostream *OsDump, Ostream *OsEmit, Ostream *OsError,
 
 void GlobalContext::translateFunctions() {
   TimerMarker Timer(TimerStack::TT_translateFunctions, this);
-  while (std::unique_ptr<Cfg> Func = optQueueBlockingPop()) {
+  while (std::unique_ptr<OptWorkItem> OptItem = optQueueBlockingPop()) {
+    auto Func = OptItem->getParsedCfg();
     // Install Func in TLS for Cfg-specific container allocators.
     CfgLocalAllocatorScope _(Func.get());
     // Reset per-function stats being accumulated in TLS.
@@ -878,19 +913,19 @@ void GlobalContext::setTimerName(TimerStackIdT StackID,
 // interface to take and transfer ownership, but they internally store the raw
 // Cfg pointer in the work queue. This allows e.g. future queue optimizations
 // such as the use of atomics to modify queue elements.
-void GlobalContext::optQueueBlockingPush(std::unique_ptr<Cfg> Func) {
-  assert(Func);
+void GlobalContext::optQueueBlockingPush(std::unique_ptr<OptWorkItem> Item) {
+  assert(Item);
   {
     TimerMarker _(TimerStack::TT_qTransPush, this);
-    OptQ.blockingPush(std::move(Func));
+    OptQ.blockingPush(std::move(Item));
   }
   if (getFlags().isSequential())
     translateFunctions();
 }
 
-std::unique_ptr<Cfg> GlobalContext::optQueueBlockingPop() {
+std::unique_ptr<OptWorkItem> GlobalContext::optQueueBlockingPop() {
   TimerMarker _(TimerStack::TT_qTransPop, this);
-  return std::unique_ptr<Cfg>(OptQ.blockingPop());
+  return std::unique_ptr<OptWorkItem>(OptQ.blockingPop());
 }
 
 void GlobalContext::emitQueueBlockingPush(

src/IceGlobalContext.h

@@ -29,6 +29,7 @@
 #include "IceUtils.h"
 
 #include <array>
+#include <atomic>
 #include <cassert>
 #include <functional>
 #include <memory>
@@ -54,6 +55,21 @@ enum class RuntimeHelper {
       H_Num
 };
 
+/// OptWorkItem is a simple wrapper used to pass parse information on a function
+/// block, to a translator thread.
+class OptWorkItem {
+  OptWorkItem(const OptWorkItem &) = delete;
+  OptWorkItem &operator=(const OptWorkItem &) = delete;
+
+public:
+  // Get the Cfg for the funtion to translate.
+  virtual std::unique_ptr<Cfg> getParsedCfg() = 0;
+  virtual ~OptWorkItem() = default;
+
+protected:
+  OptWorkItem() = default;
+};
+
 class GlobalContext {
   GlobalContext() = delete;
   GlobalContext(const GlobalContext &) = delete;
@@ -358,12 +374,12 @@ public:
   /// Notifies any idle workers that a new function is available for
   /// translating. May block if the work queue is too large, in order to control
   /// memory footprint.
-  void optQueueBlockingPush(std::unique_ptr<Cfg> Func);
+  void optQueueBlockingPush(std::unique_ptr<OptWorkItem> Item);
   /// Takes a Cfg from the work queue for translating. May block if the work
   /// queue is currently empty. Returns nullptr if there is no more work - the
   /// queue is empty and either end() has been called or the Sequential flag was
   /// set.
-  std::unique_ptr<Cfg> optQueueBlockingPop();
+  std::unique_ptr<OptWorkItem> optQueueBlockingPop();
   /// Notifies that no more work will be added to the work queue.
   void optQueueNotifyEnd() { OptQ.notifyEnd(); }
 
@@ -405,34 +421,7 @@ public:
     }
   }
 
-  void waitForWorkerThreads() {
-    optQueueNotifyEnd();
-    for (std::thread &Worker : TranslationThreads) {
-      Worker.join();
-    }
-    TranslationThreads.clear();
-
-    // Only notify the emit queue to end after all the translation threads have
-    // ended.
-    emitQueueNotifyEnd();
-    for (std::thread &Worker : EmitterThreads) {
-      Worker.join();
-    }
-    EmitterThreads.clear();
-
-    if (BuildDefs::timers()) {
-      auto Timers = getTimers();
-      for (ThreadContext *TLS : AllThreadContexts)
-        Timers->mergeFrom(TLS->Timers);
-    }
-    if (BuildDefs::dump()) {
-      // Do a separate loop over AllThreadContexts to avoid holding two locks
-      // at once.
-      auto Stats = getStatsCumulative();
-      for (ThreadContext *TLS : AllThreadContexts)
-        Stats->add(TLS->StatsCumulative);
-    }
-  }
+  void waitForWorkerThreads();
 
   /// Translation thread startup routine.
   void translateFunctionsWrapper(ThreadContext *MyTLS) {
@@ -545,12 +534,16 @@ private:
   Ostream *StrEmit;  /// Stream for code emission
   Ostream *StrError; /// Stream for logging errors.
 
+  // True if waitForWorkerThreads() has been called.
+  std::atomic_bool WaitForWorkerThreadsCalled;
+
   ICE_CACHELINE_BOUNDARY;
 
   Intrinsics IntrinsicsInfo;
   // TODO(jpp): move to EmitterContext.
   std::unique_ptr<ELFObjectWriter> ObjectWriter;
-  BoundedProducerConsumerQueue<Cfg> OptQ;
+  static constexpr size_t MaxOptQSize = 1 << 16;
+  BoundedProducerConsumerQueue<OptWorkItem, MaxOptQSize> OptQ;
   BoundedProducerConsumerQueue<EmitterWorkItem> EmitQ;
   // DataLowering is only ever used by a single thread at a time (either in
   // emitItems(), or in IceCompiler::run before the compilation is over.)

src/IceTranslator.cpp

@@ -24,6 +24,20 @@
 
 namespace Ice {
 
+class CfgOptWorkItem final : public OptWorkItem {
+  CfgOptWorkItem() = delete;
+  CfgOptWorkItem(const CfgOptWorkItem &) = delete;
+  CfgOptWorkItem &operator=(const CfgOptWorkItem &) = delete;
+
+public:
+  CfgOptWorkItem(std::unique_ptr<Cfg> Func) : Func(std::move(Func)) {}
+  std::unique_ptr<Cfg> getParsedCfg() override { return std::move(Func); }
+  ~CfgOptWorkItem() override = default;
+
+private:
+  std::unique_ptr<Ice::Cfg> Func;
+};
+
 Translator::Translator(GlobalContext *Ctx)
     : Ctx(Ctx), NextSequenceNumber(GlobalContext::getFirstSequenceNumber()),
       ErrorStatus() {}
@@ -57,7 +71,7 @@ bool Translator::checkIfUnnamedNameSafe(const std::string &Name,
 }
 
 void Translator::translateFcn(std::unique_ptr<Cfg> Func) {
-  Ctx->optQueueBlockingPush(std::move(Func));
+  Ctx->optQueueBlockingPush(makeUnique<CfgOptWorkItem>(std::move(Func)));
 }
 
 void Translator::lowerGlobals(

src/PNaClTranslator.cpp

@@ -221,7 +221,7 @@ private:
 class BlockParserBaseClass;
 
 // Top-level class to read PNaCl bitcode files, and translate to ICE.
-class TopLevelParser : public NaClBitcodeParser {
+class TopLevelParser final : public NaClBitcodeParser {
   TopLevelParser() = delete;
   TopLevelParser(const TopLevelParser &) = delete;
   TopLevelParser &operator=(const TopLevelParser &) = delete;
@@ -237,21 +237,14 @@ public:
 
   Ice::Translator &getTranslator() const { return Translator; }
 
-  void setBlockParser(BlockParserBaseClass *NewBlockParser) {
-    BlockParser = NewBlockParser;
-  }
-
   /// Generates error with given Message, occurring at BitPosition within the
   /// bitcode file. Always returns true.
   bool ErrorAt(naclbitc::ErrorLevel Level, uint64_t BitPosition,
-               const std::string &Message) final;
+               const std::string &Message) override;
 
   /// Generates error message with respect to the current block parser.
   bool blockError(const std::string &Message);
 
-  /// Returns the number of errors found while parsing the bitcode file.
-  unsigned getNumErrors() const { return NumErrors; }
-
   /// Changes the size of the type list to the given size.
   void resizeTypeIDValues(size_t NewSize) { TypeIDValues.resize(NewSize); }
 
@@ -428,10 +421,12 @@ public:
 private:
   // The translator associated with the parser.
   Ice::Translator &Translator;
+
+  // ErrorStatus should only be updated while this lock is locked.
+  Ice::GlobalLockType ErrorReportingLock;
   // The exit status that should be set to true if an error occurs.
   Ice::ErrorCode &ErrorStatus;
-  // The number of errors reported.
-  unsigned NumErrors = 0;
+
   // The types associated with each type ID.
   std::vector<ExtendedType> TypeIDValues;
   // The set of functions (prototype and defined).
@@ -448,8 +443,6 @@ private:
   Ice::ConstantList ValueIDConstants;
   // Error recovery value to use when getFuncSigTypeByID fails.
   Ice::FuncSigType UndefinedFuncSigType;
-  // The block parser currently being applied. Used for error reporting.
-  BlockParserBaseClass *BlockParser = nullptr;
   // Defines if a module block has already been parsed.
   bool ParsedModuleBlock = false;
 
@@ -584,9 +577,11 @@ private:
 
 bool TopLevelParser::ErrorAt(naclbitc::ErrorLevel Level, uint64_t Bit,
                              const std::string &Message) {
-  ErrorStatus.assign(Ice::EC_Bitcode);
-  ++NumErrors;
   Ice::GlobalContext *Context = Translator.getContext();
+  {
+    std::unique_lock<Ice::GlobalLockType> _(ErrorReportingLock);
+    ErrorStatus.assign(Ice::EC_Bitcode);
+  }
   { // Lock while printing out error message.
     Ice::OstreamLocker L(Context);
     raw_ostream &OldErrStream = setErrStream(Context->getStrError());
@@ -665,11 +660,14 @@ class BlockParserBaseClass : public NaClBitcodeParser {
 public:
   // Constructor for the top-level module block parser.
   BlockParserBaseClass(unsigned BlockID, TopLevelParser *Context)
-      : NaClBitcodeParser(BlockID, Context), Context(Context) {
-    Context->setBlockParser(this);
-  }
+      : NaClBitcodeParser(BlockID, Context), Context(Context) {}
 
-  ~BlockParserBaseClass() override { Context->setBlockParser(nullptr); }
+  BlockParserBaseClass(unsigned BlockID, BlockParserBaseClass *EnclosingParser,
+                       NaClBitstreamCursor &Cursor)
+      : NaClBitcodeParser(BlockID, EnclosingParser, Cursor),
+        Context(EnclosingParser->Context) {}
+
+  ~BlockParserBaseClass() override {}
 
   // Returns the printable name of the type of block being parsed.
   virtual const char *getBlockName() const {
@@ -679,11 +677,13 @@ public:
 
   // Generates an error Message with the Bit address prefixed to it.
   bool ErrorAt(naclbitc::ErrorLevel Level, uint64_t Bit,
-               const std::string &Message) final;
+               const std::string &Message) override;
 
 protected:
   // The context parser that contains the decoded state.
   TopLevelParser *Context;
+  // True if ErrorAt has been called in this block.
+  bool BlockHasError = false;
 
   // Constructor for nested block parsers.
   BlockParserBaseClass(unsigned BlockID, BlockParserBaseClass *EnclosingParser)
@@ -752,15 +752,19 @@ private:
 };
 
 bool TopLevelParser::blockError(const std::string &Message) {
-  if (BlockParser)
-    return BlockParser->Error(Message);
-  else
+  // TODO(kschimpf): Remove this method. This method used to redirect
+  // block-level errors to the block we are in, rather than the top-level
+  // block. This gave better bit location for error messages. However, with
+  // parallel parsing, we can't keep a field to redirect (there could be many
+  // and we don't know which block parser applies). Hence, This redirect can't
+  // be applied anymore.
   return Error(Message);
 }
 
 // Generates an error Message with the bit address prefixed to it.
 bool BlockParserBaseClass::ErrorAt(naclbitc::ErrorLevel Level, uint64_t Bit,
                                    const std::string &Message) {
+  BlockHasError = true;
   std::string Buffer;
   raw_string_ostream StrBuf(Buffer);
   // Note: If dump routines have been turned off, the error messages will not
@@ -817,7 +821,7 @@ void BlockParserBaseClass::ProcessRecord() {
 }
 
 // Class to parse a types block.
-class TypesParser : public BlockParserBaseClass {
+class TypesParser final : public BlockParserBaseClass {
   TypesParser() = delete;
   TypesParser(const TypesParser &) = delete;
   TypesParser &operator=(const TypesParser &) = delete;
@@ -1021,7 +1025,7 @@ void TypesParser::ProcessRecord() {
 
 /// Parses the globals block (i.e. global variable declarations and
 /// corresponding initializers).
-class GlobalsParser : public BlockParserBaseClass {
+class GlobalsParser final : public BlockParserBaseClass {
   GlobalsParser() = delete;
   GlobalsParser(const GlobalsParser &) = delete;
   GlobalsParser &operator=(const GlobalsParser &) = delete;
@@ -1037,7 +1041,7 @@ public:
     Context->getGlobalVariablesPool()->willNotBeEmitted(DummyGlobalVar);
   }
 
-  ~GlobalsParser() final = default;
+  ~GlobalsParser() override = default;
 
   const char *getBlockName() const override { return "globals"; }
 
@@ -1341,29 +1345,37 @@ void ValuesymtabParser::ProcessRecord() {
 }
 
 /// Parses function blocks in the bitcode file.
-class FunctionParser : public BlockParserBaseClass {
+class FunctionParser final : public BlockParserBaseClass {
   FunctionParser() = delete;
   FunctionParser(const FunctionParser &) = delete;
   FunctionParser &operator=(const FunctionParser &) = delete;
 
 public:
-  FunctionParser(unsigned BlockID, BlockParserBaseClass *EnclosingParser)
+  FunctionParser(unsigned BlockID, BlockParserBaseClass *EnclosingParser,
+                 NaClBcIndexSize_t FcnId)
       : BlockParserBaseClass(BlockID, EnclosingParser),
         Timer(Ice::TimerStack::TT_parseFunctions, getTranslator().getContext()),
-        Func(nullptr), FcnId(Context->getNextFunctionBlockValueID()),
-        FuncDecl(Context->getFunctionByID(FcnId)),
+        Func(nullptr), FuncDecl(Context->getFunctionByID(FcnId)),
+        CachedNumGlobalValueIDs(Context->getNumGlobalIDs()),
+        NextLocalInstIndex(Context->getNumGlobalIDs()) {}
+
+  FunctionParser(unsigned BlockID, BlockParserBaseClass *EnclosingParser,
+                 NaClBcIndexSize_t FcnId, NaClBitstreamCursor &Cursor)
+      : BlockParserBaseClass(BlockID, EnclosingParser, Cursor),
+        Timer(Ice::TimerStack::TT_parseFunctions, getTranslator().getContext()),
+        Func(nullptr), FuncDecl(Context->getFunctionByID(FcnId)),
         CachedNumGlobalValueIDs(Context->getNumGlobalIDs()),
         NextLocalInstIndex(Context->getNumGlobalIDs()) {}
 
-  bool convertFunction() {
+  std::unique_ptr<Ice::Cfg> parseFunction(uint32_t SeqNumber) {
     bool ParserResult;
+    Ice::GlobalContext *Ctx = getTranslator().getContext();
     {
-      Ice::TimerMarker T(getTranslator().getContext(),
-                         FuncDecl->getName().toStringOrEmpty());
+      Ice::TimerMarker T(Ctx, FuncDecl->getName().toStringOrEmpty());
       // Note: The Cfg is created, even when IR generation is disabled. This is
       // done to install a CfgLocalAllocator for various internal containers.
       Ice::GlobalContext *Ctx = getTranslator().getContext();
-      Func = Ice::Cfg::create(Ctx, getTranslator().getNextSequenceNumber());
+      Func = Ice::Cfg::create(Ctx, SeqNumber);
 
       Ice::CfgLocalAllocatorScope _(Func.get());
 
@@ -1380,23 +1392,15 @@ public:
       }
 
       ParserResult = ParseThisBlock();
-
-      // Note: Once any errors have been found, we turn off all translation of
-      // all remaining functions. This allows successive parsing errors to be
-      // reported, without adding extra checks to the translator for such
-      // parsing errors.
-    }
-    if (Context->getNumErrors() == 0 && Func) {
-      getTranslator().translateFcn(std::move(Func));
-      // The translator now has ownership of Func.
-    } else {
-      Func.reset();
     }
 
-    return ParserResult;
+    if (ParserResult || BlockHasError)
+      Func->setError("Unable to parse function");
+
+    return std::move(Func);
   }
 
-  ~FunctionParser() final = default;
+  ~FunctionParser() override = default;
 
   const char *getBlockName() const override { return "function"; }
 
@@ -1440,8 +1444,6 @@ private:
   NaClBcIndexSize_t DeclaredNumberBbs = 0;
   // The basic block being built.
   Ice::CfgNode *CurrentNode = nullptr;
-  // The ID for the function.
-  NaClBcIndexSize_t FcnId;
   // The corresponding function declaration.
   Ice::FunctionDeclaration *FuncDecl;
   // Holds the dividing point between local and global absolute value indices.
@@ -1459,7 +1461,7 @@ private:
 
   void ProcessRecord() override;
 
-  void EnterBlock(unsigned NumWords) final {
+  void EnterBlock(unsigned NumWords) override {
     // Note: Bitstream defines words as 32-bit values.
     NumBytesDefiningFunction = NumWords * sizeof(uint32_t);
     // We know that all records are minimally defined by a two-bit abreviation.
@@ -2780,7 +2782,7 @@ void FunctionParser::ProcessRecord() {
 }
 
 /// Parses constants within a function block.
-class ConstantsParser : public BlockParserBaseClass {
+class ConstantsParser final : public BlockParserBaseClass {
   ConstantsParser() = delete;
   ConstantsParser(const ConstantsParser &) = delete;
   ConstantsParser &operator=(const ConstantsParser &) = delete;
@@ -2897,7 +2899,7 @@ void ConstantsParser::ProcessRecord() {
 }
 
 // Parses valuesymtab blocks appearing in a function block.
-class FunctionValuesymtabParser : public ValuesymtabParser {
+class FunctionValuesymtabParser final : public ValuesymtabParser {
   FunctionValuesymtabParser() = delete;
   FunctionValuesymtabParser(const FunctionValuesymtabParser &) = delete;
   void operator=(const FunctionValuesymtabParser &) = delete;
@@ -2915,10 +2917,10 @@ private:
     return reinterpret_cast<FunctionParser *>(GetEnclosingParser());
   }
 
-  const char *getTableKind() const final { return "Function"; }
+  const char *getTableKind() const override { return "Function"; }
 
-  void setValueName(NaClBcIndexSize_t Index, StringType &Name) final;
-  void setBbName(NaClBcIndexSize_t Index, StringType &Name) final;
+  void setValueName(NaClBcIndexSize_t Index, StringType &Name) override;
+  void setBbName(NaClBcIndexSize_t Index, StringType &Name) override;
 
   // Reports that the assignment of Name to the value associated with index is
   // not possible, for the given Context.
@@ -2984,7 +2986,7 @@ bool FunctionParser::ParseBlock(unsigned BlockID) {
 }
 
 /// Parses the module block in the bitcode file.
-class ModuleParser : public BlockParserBaseClass {
+class ModuleParser final : public BlockParserBaseClass {
   ModuleParser() = delete;
   ModuleParser(const ModuleParser &) = delete;
   ModuleParser &operator=(const ModuleParser &) = delete;
@@ -2994,10 +2996,9 @@ public:
       : BlockParserBaseClass(BlockID, Context),
         Timer(Ice::TimerStack::TT_parseModule,
               Context->getTranslator().getContext()) {}
-
   ~ModuleParser() override = default;
-
   const char *getBlockName() const override { return "module"; }
+  NaClBitstreamCursor &getCursor() const { return Record.GetCursor(); }
 
 private:
   Ice::TimerMarker Timer;
@@ -3025,7 +3026,10 @@ private:
   }
   bool ParseBlock(unsigned BlockID) override;
 
-  void ExitBlock() override { installGlobalNamesAndGlobalVarInitializers(); }
+  void ExitBlock() override {
+    installGlobalNamesAndGlobalVarInitializers();
+    Context->getTranslator().getContext()->waitForWorkerThreads();
+  }
 
   void ProcessRecord() override;
 };
@@ -3045,9 +3049,9 @@ public:
 
 private:
   Ice::TimerMarker Timer;
-  const char *getTableKind() const final { return "Module"; }
-  void setValueName(NaClBcIndexSize_t Index, StringType &Name) final;
-  void setBbName(NaClBcIndexSize_t Index, StringType &Name) final;
+  const char *getTableKind() const override { return "Module"; }
+  void setValueName(NaClBcIndexSize_t Index, StringType &Name) override;
+  void setBbName(NaClBcIndexSize_t Index, StringType &Name) override;
 };
 
 void ModuleValuesymtabParser::setValueName(NaClBcIndexSize_t Index,
@@ -3075,6 +3079,44 @@ void ModuleValuesymtabParser::setBbName(NaClBcIndexSize_t Index,
   reportUnableToAssign("Basic block", Index, Name);
 }
 
+class CfgParserWorkItem final : public Ice::OptWorkItem {
+  CfgParserWorkItem() = delete;
+  CfgParserWorkItem(const CfgParserWorkItem &) = delete;
+  CfgParserWorkItem &operator=(const CfgParserWorkItem &) = delete;
+
+public:
+  CfgParserWorkItem(unsigned BlockID, NaClBcIndexSize_t FcnId,
+                    ModuleParser *ModParser, std::unique_ptr<uint8_t[]> Buffer,
+                    uintptr_t BufferSize, uint64_t StartBit, uint32_t SeqNumber)
+      : BlockID(BlockID), FcnId(FcnId), ModParser(ModParser),
+        Buffer(std::move(Buffer)), BufferSize(BufferSize), StartBit(StartBit),
+        SeqNumber(SeqNumber) {}
+  std::unique_ptr<Ice::Cfg> getParsedCfg() override;
+  ~CfgParserWorkItem() override = default;
+
+private:
+  const unsigned BlockID;
+  const NaClBcIndexSize_t FcnId;
+  // Note: ModParser can't be const because the function parser needs to
+  // access non-const member functions (of ModuleParser and TopLevelParser).
+  // TODO(kschimpf): Fix this issue.
+  ModuleParser *ModParser;
+  const std::unique_ptr<uint8_t[]> Buffer;
+  const uintptr_t BufferSize;
+  const uint64_t StartBit;
+  const uint32_t SeqNumber;
+};
+
+std::unique_ptr<Ice::Cfg> CfgParserWorkItem::getParsedCfg() {
+  NaClBitstreamCursor &OldCursor(ModParser->getCursor());
+  llvm::NaClBitstreamReader Reader(Buffer.get(), Buffer.get() + BufferSize,
+                                   OldCursor.getBitStreamReader());
+  NaClBitstreamCursor NewCursor(Reader);
+  NewCursor.JumpToBit(NewCursor.getWordBitNo(StartBit));
+  FunctionParser Parser(BlockID, ModParser, FcnId, NewCursor);
+  return Parser.parseFunction(SeqNumber);
+}
+
 bool ModuleParser::ParseBlock(unsigned BlockID) {
   switch (BlockID) {
   case naclbitc::BLOCKINFO_BLOCK_ID:
@@ -3103,8 +3145,37 @@ bool ModuleParser::ParseBlock(unsigned BlockID) {
   }
   case naclbitc::FUNCTION_BLOCK_ID: {
     installGlobalNamesAndGlobalVarInitializers();
-    FunctionParser Parser(BlockID, this);
-    return Parser.convertFunction();
+    Ice::GlobalContext *Ctx = Context->getTranslator().getContext();
+    uint32_t SeqNumber = Context->getTranslator().getNextSequenceNumber();
+    NaClBcIndexSize_t FcnId = Context->getNextFunctionBlockValueID();
+    if (Ctx->getFlags().getParseParallel()) {
+      // Skip the block and copy into a buffer. Note: We copy into a buffer
+      // using the top-level parser to make sure that the underlying
+      // buffer reading from the data streamer is not thread safe.
+      NaClBitstreamCursor &Cursor = Record.GetCursor();
+      uint64_t StartBit = Cursor.GetCurrentBitNo();
+      if (SkipBlock())
+        return true;
+      const uint64_t EndBit = Cursor.GetCurrentBitNo();
+      const uintptr_t StartByte = Cursor.getStartWordByteForBit(StartBit);
+      const uintptr_t EndByte = Cursor.getEndWordByteForBit(EndBit);
+      const uintptr_t BufferSize = EndByte - StartByte;
+      std::unique_ptr<uint8_t[]> Buffer((uint8_t *)(new uint8_t[BufferSize]));
+      for (size_t i = Cursor.fillBuffer(Buffer.get(), BufferSize, StartByte);
+           i < BufferSize; ++i) {
+        Buffer[i] = 0;
+      }
+      Ctx->optQueueBlockingPush(Ice::makeUnique<CfgParserWorkItem>(
+          BlockID, FcnId, this, std::move(Buffer), BufferSize, StartBit,
+          SeqNumber));
+      return false;
+    } else {
+      FunctionParser Parser(BlockID, this, FcnId);
+      std::unique_ptr<Ice::Cfg> Func = Parser.parseFunction(SeqNumber);
+      bool Failed = Func->hasError();
+      getTranslator().translateFcn(std::move(Func));
+      return Failed && !getTranslator().getFlags().getAllowErrorRecovery();
+    }
   }
   default:
     return BlockParserBaseClass::ParseBlock(BlockID);

tests_lit/parse_errs/insertextract-err.ll

 ; Tests malformed insertelement and extractelement vector instructions.
 
-; RUN: %if --need=allow_dump --command llvm-as < %s \
-; RUN:   | %if --need=allow_dump --command pnacl-freeze \
-; RUN:   | %if --need=allow_dump --command not %pnacl_sz -notranslate \
-; RUN:     -build-on-read -allow-pnacl-reader-error-recovery \
-; RUN:     -filetype=obj -o /dev/null \
+; RUN: %if --need=allow_dump --command \
+; RUN:   %p2i --expect-fail -i %s --allow-pnacl-reader-error-recovery \
+; RUN:   --filetype=obj -o /dev/null --args -notranslate \
 ; RUN:   | %if --need=allow_dump --command FileCheck %s
 
-; RUN: %if --need=no_dump --command llvm-as < %s \
-; RUN:   | %if --need=no_dump --command pnacl-freeze \
-; RUN:   | %if --need=no_dump --command not %pnacl_sz -notranslate \
-; RUN:     -build-on-read -allow-pnacl-reader-error-recovery \
-; RUN:     -filetype=obj -o /dev/null \
+; RUN: %if --need=no_dump --command \
+; RUN:   %p2i --expect-fail -i %s --allow-pnacl-reader-error-recovery \
+; RUN:   --filetype=obj -o /dev/null --args -notranslate \
 ; RUN:   | %if --need=no_dump --command FileCheck %s --check-prefix=MIN
 
 define void @ExtractV4xi1(<4 x i1> %v, i32 %i) {

unittest/BitcodeMunge.cpp

@@ -25,7 +25,8 @@ void IceTest::SubzeroBitcodeMunger::resetMungeFlags() {
   Flags.setOptLevel(Ice::Opt_m1);
   Flags.setOutFileType(Ice::FT_Iasm);
   Flags.setTargetArch(Ice::Target_X8632);
-  Flags.setVerbose(Ice::IceV_Instructions);
+  Flags.setNumTranslationThreads(0);
+  Flags.setParseParallel(false);
 }
 
 bool IceTest::SubzeroBitcodeMunger::runTest(const uint64_t Munges[],
@@ -34,10 +35,12 @@ bool IceTest::SubzeroBitcodeMunger::runTest(const uint64_t Munges[],
   const bool AddHeader = true;
   setupTest(Munges, MungeSize, AddHeader);
   Ice::GlobalContext Ctx(DumpStream, DumpStream, DumpStream, nullptr);
+  Ctx.startWorkerThreads();
   Ice::PNaClTranslator Translator(&Ctx);
   const char *BufferName = "Test";
   Flags.setDisableTranslation(DisableTranslation);
   Translator.translateBuffer(BufferName, MungedInput.get());
+  Ctx.waitForWorkerThreads();
 
   cleanupTest();
   return Translator.getErrorStatus().value() == 0;