misc cleanup of Compiler::run

src/IceCompiler.cpp

 //===- subzero/src/IceCompiler.cpp - Driver for bitcode translation -------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// \brief Defines a driver for translating PNaCl bitcode into native code.
 ///
 /// The driver can either directly parse the binary bitcode file, or use LLVM
 /// routines to parse a textual bitcode file into LLVM IR and then convert LLVM
 /// IR into ICE. In either case, the high-level ICE is then compiled down to
 /// native code, as either an ELF object file or a textual asm file.
 ///
 //===----------------------------------------------------------------------===//
 
+#include <regex>
+
 #include "IceCompiler.h"
 
 #include "IceBuildDefs.h"
 #include "IceCfg.h"
 #include "IceClFlags.h"
 #include "IceClFlagsExtra.h"
 #include "IceConverter.h"
 #include "IceELFObjectWriter.h"
 #include "PNaClTranslator.h"
 
@@ skipping 35 matching lines @@
        ++i) {
     const auto &A = ConditionalBuildAttributes[i];
     *Stream << Prefix[A.FlagValue] << "_" << A.FlagName << "\n";
   }
 }
 
 } // end of anonymous namespace
 
 void Compiler::run(const Ice::ClFlagsExtra &ExtraFlags, GlobalContext &Ctx,
                    std::unique_ptr<llvm::DataStreamer> &&InputStream) {
-  dumpBuildAttributes(ExtraFlags.getGenerateBuildAtts() ? &Ctx.getStrDump()
-                                                        : nullptr);
-  if (ExtraFlags.getGenerateBuildAtts())
-    return Ctx.getErrorStatus()->assign(EC_None);
-
+  if (ExtraFlags.getGenerateBuildAtts()) {
+    dumpBuildAttributes(&Ctx.getStrDump());
+    Ctx.getErrorStatus()->assign(EC_None);
+    return;
+  }
   // The Minimal build (specifically, when dump()/emit() are not implemented)
   // 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";
-    return Ctx.getErrorStatus()->assign(EC_Args);
+    Ctx.getErrorStatus()->assign(EC_Args);
+    return;
   }
 
-  // Force -build-on-read=0 for .ll files.
-  const std::string LLSuffix = ".ll";
-  const IceString &IRFilename = ExtraFlags.getIRFilename();
-  bool BuildOnRead = ExtraFlags.getBuildOnRead();
-  if (BuildDefs::llvmIrAsInput() && IRFilename.length() >= LLSuffix.length() &&
-      IRFilename.compare(IRFilename.length() - LLSuffix.length(),
-                         LLSuffix.length(), LLSuffix) == 0)
-    BuildOnRead = false;
-
   TimerMarker T(Ice::TimerStack::TT_szmain, &Ctx);
 
   Ctx.emitFileHeader();
   Ctx.startWorkerThreads();
 
   std::unique_ptr<Translator> Translator;
-  if (BuildOnRead) {
+  const IceString &IRFilename = ExtraFlags.getIRFilename();
+  // Force -build-on-read=0 for .ll files.
+  if (!std::regex_match(IRFilename, std::regex(".*\\.ll")) &&

[rkotlerimgtec, 2015/12/22 23:21:55]

I would also be okay to just create a function where IceString is declared that
examines the last part of the string. I used regex here just because I could do
all of this on one line.

I prefer to think of what is on the end of the string as a low level operation
and not in the same place where more high level considerations are at play.

[Jim Stichnoth, 2015/12/23 15:54:15]

The regex use is nice.  However, I would like it to not be used in a MINIMAL
build, because the regex code increases the translator size and textual .ll file
parsing is disabled in MINIMAL mode.  Your change removed the
BuildDefs::llvmIrAsInput() check which had accomplished this.

This led me down a path of trying to make sense of llvmIr() and llvmIrAsInput()
and the --build-on-read argument.  I think these could be simplified.  The logic
could be: if some BuildDefs flag is defined, and the input file name ends in
".ll", then use the LLVM parser, else use the Subzero parser.

That is probably best left to a separate CL, but in the meantime it would be
best to retain the BuildDefs::llvmIrAsInput() check.

+      ExtraFlags.getBuildOnRead()) {
     std::unique_ptr<PNaClTranslator> PTranslator(new PNaClTranslator(&Ctx));
     std::unique_ptr<llvm::StreamingMemoryObject> MemObj(
         new llvm::StreamingMemoryObjectImpl(InputStream.release()));
     PTranslator->translate(IRFilename, std::move(MemObj));
     Translator.reset(PTranslator.release());
   } else if (BuildDefs::llvmIr()) {
     if (BuildDefs::browser()) {
       Ctx.getStrError()
           << "non BuildOnRead is not supported w/ PNACL_BROWSER_TRANSLATOR\n";
-      return Ctx.getErrorStatus()->assign(EC_Args);
+      Ctx.getErrorStatus()->assign(EC_Args);
+      return;
     }
     // Parse the input LLVM IR file into a module.
     llvm::SMDiagnostic Err;
     TimerMarker T1(Ice::TimerStack::TT_parse, &Ctx);
     llvm::DiagnosticHandlerFunction DiagnosticHandler =
         ExtraFlags.getLLVMVerboseErrors()
             ? redirectNaClDiagnosticToStream(llvm::errs())
             : nullptr;
     std::unique_ptr<llvm::Module> Mod =
         NaClParseIRFile(IRFilename, ExtraFlags.getInputFileFormat(), Err,
                         llvm::getGlobalContext(), DiagnosticHandler);
     if (!Mod) {
       Err.print(ExtraFlags.getAppName().c_str(), llvm::errs());
-      return Ctx.getErrorStatus()->assign(EC_Bitcode);
+      Ctx.getErrorStatus()->assign(EC_Bitcode);
+      return;
     }
 
     std::unique_ptr<Converter> Converter(new class Converter(Mod.get(), &Ctx));
     Converter->convertToIce();
     Translator.reset(Converter.release());
   } else {
     Ctx.getStrError() << "Error: Build doesn't allow LLVM IR, "
                       << "--build-on-read=0 not allowed\n";
-    return Ctx.getErrorStatus()->assign(EC_Args);
+    Ctx.getErrorStatus()->assign(EC_Args);
+    return;
   }
 
   Ctx.waitForWorkerThreads();
   if (Translator->getErrorStatus()) {
     Ctx.getErrorStatus()->assign(Translator->getErrorStatus().value());
   } else {
     Ctx.lowerGlobals("last");
     Ctx.lowerProfileData();
     Ctx.lowerConstants();
     Ctx.lowerJumpTables();
@@ skipping 10 matching lines @@
 
   if (Ctx.getFlags().getTimeEachFunction()) {
     constexpr bool DumpCumulative = false;
     Ctx.dumpTimers(GlobalContext::TSK_Funcs, DumpCumulative);
   }
   constexpr bool FinalStats = true;
   Ctx.dumpStats("_FINAL_", FinalStats);
 }
 
 } // end of namespace Ice