Clean up exit status and globals procecessing in llvm2ice.

Makes IceTranslator.ExitStatus a boolean (rather than int), and changes
code to check flag when done. Fixes bug introduced in
https://codereview.chromium.org/387023002.

Also cleans up the (Ice) Converter class to handle globals processing,
rathe than doing it in llvm2ice.cpp.

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

Review URL: https://codereview.chromium.org/387023002
diff --git a/src/IceConverter.cpp b/src/IceConverter.cpp
index 5a85034..9ba1fea 100644
--- a/src/IceConverter.cpp
+++ b/src/IceConverter.cpp
@@ -20,6 +20,7 @@
 #include "IceGlobalContext.h"
 #include "IceInst.h"
 #include "IceOperand.h"
+#include "IceTargetLowering.h"
 #include "IceTypes.h"
 
 #include "llvm/IR/Constant.h"
@@ -657,14 +658,63 @@
 
 } // end of anonymous namespace.
 
-int Ice::Converter::convertToIce(llvm::Module *Mod) {
+namespace Ice {
+
+void Converter::convertToIce(Module *Mod) {
+  convertGlobals(Mod);
+  convertFunctions(Mod);
+}
+
+void Converter::convertGlobals(Module *Mod) {
+  OwningPtr<TargetGlobalInitLowering> GlobalLowering(
+      TargetGlobalInitLowering::createLowering(Ctx->getTargetArch(), Ctx));
+  for (Module::const_global_iterator I = Mod->global_begin(),
+                                     E = Mod->global_end();
+       I != E; ++I) {
+    if (!I->hasInitializer())
+      continue;
+    const llvm::Constant *Initializer = I->getInitializer();
+    IceString Name = I->getName();
+    unsigned Align = I->getAlignment();
+    uint64_t NumElements = 0;
+    const char *Data = NULL;
+    bool IsInternal = I->hasInternalLinkage();
+    bool IsConst = I->isConstant();
+    bool IsZeroInitializer = false;
+
+    if (const ConstantDataArray *CDA =
+            dyn_cast<ConstantDataArray>(Initializer)) {
+      NumElements = CDA->getNumElements();
+      assert(isa<IntegerType>(CDA->getElementType()) &&
+             cast<IntegerType>(CDA->getElementType())->getBitWidth() == 8);
+      Data = CDA->getRawDataValues().data();
+    } else if (isa<ConstantAggregateZero>(Initializer)) {
+      if (const ArrayType *AT = dyn_cast<ArrayType>(Initializer->getType())) {
+        assert(isa<IntegerType>(AT->getElementType()) &&
+               cast<IntegerType>(AT->getElementType())->getBitWidth() == 8);
+        NumElements = AT->getNumElements();
+        IsZeroInitializer = true;
+      } else {
+        llvm_unreachable("Unhandled constant aggregate zero type");
+      }
+    } else {
+      llvm_unreachable("Unhandled global initializer");
+    }
+
+    GlobalLowering->lower(Name, Align, IsInternal, IsConst, IsZeroInitializer,
+                          NumElements, Data, Flags.DisableTranslation);
+  }
+  GlobalLowering.reset();
+}
+
+void Converter::convertFunctions(Module *Mod) {
   for (Module::const_iterator I = Mod->begin(), E = Mod->end(); I != E; ++I) {
     if (I->empty())
       continue;
     LLVM2ICEConverter FunctionConverter(Ctx);
 
-    Ice::Timer TConvert;
-    Ice::Cfg *Fcn = FunctionConverter.convertFunction(I);
+    Timer TConvert;
+    Cfg *Fcn = FunctionConverter.convertFunction(I);
     if (Flags.SubzeroTimingEnabled) {
       std::cerr << "[Subzero timing] Convert function "
                 << Fcn->getFunctionName() << ": " << TConvert.getElapsedSec()
@@ -674,5 +724,6 @@
   }
 
   emitConstants();
-  return ExitStatus;
 }
+
+} // end of Ice namespace.
diff --git a/src/IceConverter.h b/src/IceConverter.h
index dc18e7a..30d3b60 100644
--- a/src/IceConverter.h
+++ b/src/IceConverter.h
@@ -25,11 +25,15 @@
 class Converter : public Translator {
 public:
   Converter(GlobalContext *Ctx, Ice::ClFlags &Flags) : Translator(Ctx, Flags) {}
-  /// Converts the LLVM Module to ICE. Returns exit status 0 if successful,
-  /// Nonzero otherwise.
-  int convertToIce(llvm::Module *Mod);
+  /// Converts the LLVM Module to ICE. Sets exit status to false if successful,
+  /// true otherwise.
+  void convertToIce(llvm::Module *Mod);
 
 private:
+  // Converts globals to ICE, and then machine code.
+  void convertGlobals(llvm::Module *Mod);
+  // Converts functions to ICE, and then machine code.
+  void convertFunctions(llvm::Module *Mod);
   Converter(const Converter &) LLVM_DELETED_FUNCTION;
   Converter &operator=(const Converter &) LLVM_DELETED_FUNCTION;
 };
diff --git a/src/IceTranslator.cpp b/src/IceTranslator.cpp
index 7eb0fc0..4b84688 100644
--- a/src/IceTranslator.cpp
+++ b/src/IceTranslator.cpp
@@ -40,7 +40,7 @@
     }
     if (Func->hasError()) {
       std::cerr << "ICE translation error: " << Func->getError() << "\n";
-      ExitStatus = 1;
+      ErrorStatus = true;
     }
 
     Ice::Timer TEmit;
diff --git a/src/IceTranslator.h b/src/IceTranslator.h
index 648faea..1189951 100644
--- a/src/IceTranslator.h
+++ b/src/IceTranslator.h
@@ -30,17 +30,17 @@
 class Translator {
 public:
   Translator(GlobalContext *Ctx, ClFlags &Flags)
-      : Ctx(Ctx), Flags(Flags), ExitStatus(0) {}
+      : Ctx(Ctx), Flags(Flags), ErrorStatus(0) {}
 
   ~Translator();
-  int getExitStatus() const { return ExitStatus; }
+  bool getErrorStatus() const { return ErrorStatus; }
 
 protected:
   GlobalContext *Ctx;
   ClFlags &Flags;
-  // The exit status of the translation. 0 is successful. Nonzero
+  // The exit status of the translation. False is successful. True
   // otherwise.
-  int ExitStatus;
+  bool ErrorStatus;
   // Ideally, Func would be inside the methods that converts IR to
   // functions.  However, emitting the constant pool requires a valid
   // Cfg object, so we need to defer deleting the last non-empty Cfg
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index 82ef5ac..7f9c514 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -40,10 +40,10 @@
 
 public:
   TopLevelParser(const std::string &InputName, NaClBitcodeHeader &Header,
-                 NaClBitstreamCursor &Cursor, int &ExitStatusFlag)
+                 NaClBitstreamCursor &Cursor, bool &ErrorStatus)
       : NaClBitcodeParser(Cursor),
         Mod(new Module(InputName, getGlobalContext())), Header(Header),
-        ExitStatusFlag(ExitStatusFlag), NumErrors(0), NumFunctionIds(0),
+        ErrorStatus(ErrorStatus), NumErrors(0), NumFunctionIds(0),
         GlobalVarPlaceHolderType(Type::getInt8Ty(getLLVMContext())) {
     Mod->setDataLayout(PNaClDataLayout);
   }
@@ -52,7 +52,7 @@
   LLVM_OVERRIDE;
 
   virtual bool Error(const std::string &Message) LLVM_OVERRIDE {
-    ExitStatusFlag = 1;
+    ErrorStatus = true;
     ++NumErrors;
     return NaClBitcodeParser::Error(Message);
   }
@@ -167,8 +167,8 @@
   OwningPtr<Module> Mod;
   // The bitcode header.
   NaClBitcodeHeader &Header;
-  // The exit status flag that should be set to 1 if an error occurs.
-  int &ExitStatusFlag;
+  // 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.
@@ -810,14 +810,14 @@
   if (error_code ec =
           MemoryBuffer::getFileOrSTDIN(IRFilename.c_str(), MemBuf)) {
     errs() << "Error reading '" << IRFilename << "': " << ec.message() << "\n";
-    ExitStatus = 1;
+    ErrorStatus = true;
     return;
   }
 
   if (MemBuf->getBufferSize() % 4 != 0) {
     errs() << IRFilename
            << ": Bitcode stream should be a multiple of 4 bytes in length.\n";
-    ExitStatus = 1;
+    ErrorStatus = true;
     return;
   }
 
@@ -828,7 +828,7 @@
   NaClBitcodeHeader Header;
   if (Header.Read(BufPtr, EndBufPtr) || !Header.IsSupported()) {
     errs() << "Invalid PNaCl bitcode header.\n";
-    ExitStatus = 1;
+    ErrorStatus = true;
     return;
   }
 
@@ -837,11 +837,11 @@
   NaClBitstreamCursor InputStream(InputStreamFile);
 
   TopLevelParser Parser(MemBuf->getBufferIdentifier(), Header, InputStream,
-                        ExitStatus);
+                        ErrorStatus);
   int TopLevelBlocks = 0;
   while (!InputStream.AtEndOfStream()) {
     if (Parser.Parse()) {
-      ExitStatus = 1;
+      ErrorStatus = true;
       return;
     }
     ++TopLevelBlocks;
@@ -851,7 +851,7 @@
     errs() << IRFilename
            << ": Contains more than one module. Found: " << TopLevelBlocks
            << "\n";
-    ExitStatus = 1;
+    ErrorStatus = true;
   }
   return;
 }
diff --git a/src/PNaClTranslator.h b/src/PNaClTranslator.h
index a0316d7..a8ca06a 100644
--- a/src/PNaClTranslator.h
+++ b/src/PNaClTranslator.h
@@ -25,7 +25,7 @@
   PNaClTranslator(GlobalContext *Ctx, ClFlags &Flags)
       : Translator(Ctx, Flags) {}
   // Reads the PNaCl bitcode file and translates to ICE, which is then
-  // converted to machine code. Sets ExitStatus to non-zero if any
+  // converted to machine code. Sets ErrorStatus to true if any
   // errors occurred.
   void translate(const std::string &IRFilename);
 
diff --git a/src/llvm2ice.cpp b/src/llvm2ice.cpp
index 822a0ce..bf7de31 100644
--- a/src/llvm2ice.cpp
+++ b/src/llvm2ice.cpp
@@ -16,14 +16,9 @@
 #include "IceCfg.h"
 #include "IceClFlags.h"
 #include "IceConverter.h"
-#include "IceDefs.h"
-#include "IceTargetLowering.h"
-#include "IceTypes.h"
 #include "PNaClTranslator.h"
 
-#include "llvm/IR/Constants.h"
 #include "llvm/IR/LLVMContext.h"
-#include "llvm/IR/Module.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/raw_os_ostream.h"
@@ -137,7 +132,7 @@
   if (BuildOnRead) {
     Ice::PNaClTranslator Translator(&Ctx, Flags);
     Translator.translate(IRFilename);
-    return Translator.getExitStatus();
+    return Translator.getErrorStatus();
   } else {
     // Parse the input LLVM IR file into a module.
     SMDiagnostic Err;
@@ -155,48 +150,8 @@
       return 1;
     }
 
-    // TODO(stichnot): Move this into IceConverter.cpp.
-    OwningPtr<Ice::TargetGlobalInitLowering> GlobalLowering(
-        Ice::TargetGlobalInitLowering::createLowering(TargetArch, &Ctx));
-    for (Module::const_global_iterator I = Mod->global_begin(),
-                                       E = Mod->global_end();
-         I != E; ++I) {
-      if (!I->hasInitializer())
-        continue;
-      const Constant *Initializer = I->getInitializer();
-      Ice::IceString Name = I->getName();
-      unsigned Align = I->getAlignment();
-      uint64_t NumElements = 0;
-      const char *Data = NULL;
-      bool IsInternal = I->hasInternalLinkage();
-      bool IsConst = I->isConstant();
-      bool IsZeroInitializer = false;
-
-      if (const ConstantDataArray *CDA =
-              dyn_cast<ConstantDataArray>(Initializer)) {
-        NumElements = CDA->getNumElements();
-        assert(isa<IntegerType>(CDA->getElementType()) &&
-               cast<IntegerType>(CDA->getElementType())->getBitWidth() == 8);
-        Data = CDA->getRawDataValues().data();
-      } else if (isa<ConstantAggregateZero>(Initializer)) {
-        if (const ArrayType *AT = dyn_cast<ArrayType>(Initializer->getType())) {
-          assert(isa<IntegerType>(AT->getElementType()) &&
-                 cast<IntegerType>(AT->getElementType())->getBitWidth() == 8);
-          NumElements = AT->getNumElements();
-          IsZeroInitializer = true;
-        } else {
-          llvm_unreachable("Unhandled constant aggregate zero type");
-        }
-      } else {
-        llvm_unreachable("Unhandled global initializer");
-      }
-
-      GlobalLowering->lower(Name, Align, IsInternal, IsConst, IsZeroInitializer,
-                            NumElements, Data, DisableTranslation);
-    }
-    GlobalLowering.reset();
-
     Ice::Converter Converter(&Ctx, Flags);
-    return Converter.convertToIce(Mod);
+    Converter.convertToIce(Mod);
+    return Converter.getErrorStatus();
   }
 }