Made a bunch of cleanups, as per Chris' recommendations:

* Removed unused global and member variables
* Fixed comments (CodeGeneratorBug.cpp)
* Check for possibly failing GCC::create() and CBE::create()
* Remove generated files after diffing the output (e.g., shared object)
* Instead of using std::for_each, use explicit loops as std::for_each may
  duplicate the functor, and ours carries state
* Changed member var from cl::opt<std::string> to just std::string
* Fixed doxygen comments
* Fixed string comparisons to use [ str.empty() ] instead of [ str == "" ]
* Cache instances of CBE and GCC in BugDriver across compilations and executions
  while testing tools.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@7302 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 61 insertions, 44 deletions

diff --git a/tools/bugpoint/BugDriver.cpp b/tools/bugpoint/BugDriver.cpp
index 53a8395aaf..f69feabfff 100644
--- a/tools/bugpoint/BugDriver.cpp
+++ b/tools/bugpoint/BugDriver.cpp
@@ -66,7 +66,7 @@ void DeleteFunctionBody(Function *F) {
 
 BugDriver::BugDriver(const char *toolname)
   : ToolName(toolname), ReferenceOutputFile(OutputFile),
-    Program(0), Interpreter(0) {}
+    Program(0), Interpreter(0), cbe(0), gcc(0) {}
 
 
 /// ParseInputFile - Given a bytecode or assembly input filename, parse and
diff --git a/tools/bugpoint/BugDriver.h b/tools/bugpoint/BugDriver.h
index 0c461d1e17..0eb6188bee 100644
--- a/tools/bugpoint/BugDriver.h
+++ b/tools/bugpoint/BugDriver.h
@@ -9,7 +9,6 @@
 #ifndef BUGDRIVER_H
 #define BUGDRIVER_H
 
-#include "Support/CommandLine.h"
 #include <vector>
 #include <string>
 
@@ -25,12 +24,17 @@ class ReduceMiscompilingFunctions;
 class ReduceCrashingFunctions;
 class ReduceCrashingBlocks;
 
+class CBE;
+class GCC;
+
 class BugDriver {
   const std::string ToolName;  // Name of bugpoint
-  cl::opt<std::string> ReferenceOutputFile; // Name of `good' output file
+  std::string ReferenceOutputFile; // Name of `good' output file
   Module *Program;             // The raw program, linked together
   std::vector<const PassInfo*> PassesToRun;
   AbstractInterpreter *Interpreter;   // How to run the program
+  CBE *cbe;
+  GCC *gcc;
 
   // FIXME: sort out public/private distinctions...
   friend class DebugCrashes;
@@ -128,6 +132,7 @@ private:
   }
 
   /// PrintFunctionList - prints out list of problematic functions
+  ///
   static void PrintFunctionList(const std::vector<Function*> &Funcs);
 
   /// deleteInstructionFromProgram - This method clones the current Program and
diff --git a/tools/bugpoint/CodeGeneratorBug.cpp b/tools/bugpoint/CodeGeneratorBug.cpp
index 772b4fdf75..93ee24ea08 100644
--- a/tools/bugpoint/CodeGeneratorBug.cpp
+++ b/tools/bugpoint/CodeGeneratorBug.cpp
@@ -17,9 +17,6 @@
 #include <algorithm>
 #include <set>
 
-// Passed as a command-line argument to Bugpoint
-extern cl::opt<std::string> Output;
-
 class ReduceMisCodegenFunctions : public ListReducer<Function*> {
   BugDriver &BD;
 public:
@@ -59,8 +56,7 @@ bool ReduceMisCodegenFunctions::TestFuncs(const std::vector<Function*> &Funcs)
   for (Module::giterator I=TestModule->gbegin(),E = TestModule->gend();I!=E;++I)
     I->setInitializer(0);  // Delete the initializer to make it external
 
-  // Remove the Test functions from the Safe module, and
-  // all of the global variables.
+  // Remove the Test functions from the Safe module
   for (unsigned i = 0, e = Funcs.size(); i != e; ++i) {
     Function *TNOF = SafeModule->getFunction(Funcs[i]->getName(),
                                              Funcs[i]->getFunctionType());
@@ -100,25 +96,25 @@ bool ReduceMisCodegenFunctions::TestFuncs(const std::vector<Function*> &Funcs)
 
   // Run the code generator on the `Test' code, loading the shared library.
   // The function returns whether or not the new output differs from reference.
-  return BD.diffProgram(TestModuleBC, SharedObject, false);
+  int Result =  BD.diffProgram(TestModuleBC, SharedObject, false);
+  removeFile(SharedObject);
+  return Result;
 }
 
 namespace {
-  struct Disambiguator /*: public unary_function<GlobalValue&, void>*/ {
+  struct Disambiguator {
     std::set<std::string> SymbolNames;
-    std::set<Value*> Symbols;
     uint64_t uniqueCounter;
     bool externalOnly;
-
+  public:
     Disambiguator() : uniqueCounter(0), externalOnly(true) {}
     void setExternalOnly(bool value) { externalOnly = value; }
-    void operator() (GlobalValue &V) {
+    void add(GlobalValue &V) {
       if (externalOnly && !V.isExternal()) return;
 
       if (SymbolNames.count(V.getName()) == 0) {
         DEBUG(std::cerr << "Disambiguator: adding " << V.getName() 
                         << ", no conflicts.\n");
-        Symbols.insert(&V);
         SymbolNames.insert(V.getName());
       } else { 
         // Mangle name before adding
@@ -133,7 +129,6 @@ namespace {
                         << ", adding: " << newName << "\n");
         V.setName(newName);
         SymbolNames.insert(newName);
-        Symbols.insert(&V);
       }
     }
   };
@@ -142,14 +137,15 @@ namespace {
 void ReduceMisCodegenFunctions::DisambiguateGlobalSymbols(Module *M) {
   // First, try not to cause collisions by minimizing chances of renaming an
   // already-external symbol, so take in external globals and functions as-is.
-  Disambiguator D = std::for_each(M->gbegin(), M->gend(), Disambiguator());
-  std::for_each(M->begin(), M->end(), D);
+  Disambiguator D;
+  for (Module::giterator I = M->gbegin(), E = M->gend(); I != E; ++I) D.add(*I);
+  for (Module::iterator  I = M->begin(),  E = M->end();  I != E; ++I) D.add(*I);
 
   // Now just rename functions and globals as necessary, keeping what's already
   // in the set unique.
   D.setExternalOnly(false);
-  std::for_each(M->gbegin(), M->gend(), D);
-  std::for_each(M->begin(), M->end(), D);
+  for (Module::giterator I = M->gbegin(), E = M->gend(); I != E; ++I) D.add(*I);
+  for (Module::iterator  I = M->begin(),  E = M->end();  I != E; ++I) D.add(*I);
 }
 
 
@@ -169,7 +165,7 @@ bool BugDriver::debugCodeGenerator() {
   std::cout << "\n";
 
   // Output a bunch of bytecode files for the user...
-  ReduceMisCodegenFunctions(*this).TestFuncs(MisCodegenFunctions);
+  // ReduceMisCodegenFunctions(*this).TestFuncs(MisCodegenFunctions);
 
   return false;
 }
diff --git a/tools/bugpoint/ExecutionDriver.cpp b/tools/bugpoint/ExecutionDriver.cpp
index 307f408606..b391b0492f 100644
--- a/tools/bugpoint/ExecutionDriver.cpp
+++ b/tools/bugpoint/ExecutionDriver.cpp
@@ -38,6 +38,8 @@ namespace {
   cl::opt<std::string>
   InputFile("input", cl::init("/dev/null"),
             cl::desc("Filename to pipe in as stdin (default: /dev/null)"));
+
+  enum FileType { AsmFile, CFile };
 }
 
 /// AbstractInterpreter Class - Subclasses of this class are used to execute
@@ -84,7 +86,7 @@ public:
 int LLI::ExecuteProgram(const std::string &Bytecode,
                         const std::string &OutputFile,
                         const std::string &SharedLib) {
-  if (SharedLib != "") {
+  if (!SharedLib.empty()) {
     std::cerr << "LLI currently does not support loading shared libraries.\n"
               << "Exiting.\n";
     exit(1);
@@ -109,10 +111,11 @@ int LLI::ExecuteProgram(const std::string &Bytecode,
 // This is not a *real* AbstractInterpreter as it does not accept bytecode
 // files, but only input acceptable to GCC, i.e. C, C++, and assembly files
 //
-class GCC : public AbstractInterpreter {
+class GCC {
   std::string GCCPath;          // The path to the gcc executable
 public:
   GCC(const std::string &gccPath) : GCCPath(gccPath) { }
+  virtual ~GCC() {}
 
   // GCC create method - Try to find the `gcc' executable
   static GCC *create(BugDriver *BD, std::string &Message) {
@@ -127,16 +130,19 @@ public:
   }
 
   virtual int ExecuteProgram(const std::string &ProgramFile,
+                             FileType fileType,
                              const std::string &OutputFile,
                              const std::string &SharedLib = "");
 
   int MakeSharedObject(const std::string &InputFile,
+                       FileType fileType,
                        std::string &OutputFile);
   
   void ProcessFailure(const char **Args);
 };
 
 int GCC::ExecuteProgram(const std::string &ProgramFile,
+                        FileType fileType,
                         const std::string &OutputFile,
                         const std::string &SharedLib) {
   std::string OutputBinary = "bugpoint.gcc.exe";
@@ -144,6 +150,7 @@ int GCC::ExecuteProgram(const std::string &ProgramFile,
 
   const char *ArgsWithoutSO[] = {
     GCCPath.c_str(),
+    "-x", (fileType == AsmFile) ? "assembler" : "c",
     ProgramFile.c_str(),         // Specify the input filename...
     "-o", OutputBinary.c_str(),  // Output to the right filename...
     "-lm",                       // Hard-code the math library...
@@ -152,6 +159,7 @@ int GCC::ExecuteProgram(const std::string &ProgramFile,
   };
   const char *ArgsWithSO[] = {
     GCCPath.c_str(),
+    "-x", (fileType == AsmFile) ? "assembler" : "c",
     ProgramFile.c_str(),         // Specify the input filename...
     SharedLib.c_str(),           // Specify the shared library to link in...
     "-o", OutputBinary.c_str(),  // Output to the right filename...
@@ -160,12 +168,12 @@ int GCC::ExecuteProgram(const std::string &ProgramFile,
     0
   };
 
-  GCCArgs = (SharedLib == "") ? ArgsWithoutSO : ArgsWithSO;
+  GCCArgs = (SharedLib.empty()) ? ArgsWithoutSO : ArgsWithSO;
   std::cout << "<gcc>";
-  if (RunProgramWithTimeout(GCCPath, GCCArgs, "/dev/null",
-                            "/dev/null", "/dev/null")) {
+  if (RunProgramWithTimeout(GCCPath, GCCArgs, "/dev/null", "/dev/null",
+                            "/dev/null")) {
     ProcessFailure(GCCArgs);
-    exit(1);  // Leave stuff around for the user to inspect or debug the CBE
+    exit(1);
   }
 
   const char *ProgramArgs[] = {
@@ -184,11 +192,13 @@ int GCC::ExecuteProgram(const std::string &ProgramFile,
 }
 
 int GCC::MakeSharedObject(const std::string &InputFile,
+                          FileType fileType,
                           std::string &OutputFile) {
   OutputFile = "./bugpoint.so";
   // Compile the C/asm file into a shared object
   const char* GCCArgs[] = {
     GCCPath.c_str(),
+    "-x", (fileType == AsmFile) ? "assembler" : "c",
     InputFile.c_str(),           // Specify the input filename...
 #if defined(sparc) || defined(__sparc__) || defined(__sparcv9)
     "-G",                        // Compile a shared library, `-G' for Sparc
@@ -254,6 +264,10 @@ public:
 
     Message = "Found llc: " + LLCPath + "\n";
     GCC *gcc = GCC::create(BD, Message);
+    if (!gcc) {
+      std::cerr << Message << "\n";
+      exit(1);
+    }
     return new LLC(LLCPath, gcc);
   }
 
@@ -300,7 +314,7 @@ int LLC::ExecuteProgram(const std::string &Bytecode,
   }
 
   // Assuming LLC worked, compile the result with GCC and run it.
-  int Result = gcc->ExecuteProgram(OutputAsmFile, OutputFile, SharedLib);
+  int Result = gcc->ExecuteProgram(OutputAsmFile,AsmFile,OutputFile,SharedLib);
   removeFile(OutputAsmFile);
   return Result;
 }
@@ -333,18 +347,20 @@ public:
 int JIT::ExecuteProgram(const std::string &Bytecode,
                         const std::string &OutputFile,
                         const std::string &SharedLib) {
-  if (SharedLib == "") {
+  if (SharedLib.empty()) {
     const char* Args[] = {
-      LLIPath.c_str(), "-quiet", "-force-interpreter=false", Bytecode.c_str(),
+      LLIPath.c_str(),
+      "-quiet",
+      "-force-interpreter=false",
+      Bytecode.c_str(),
       0
     };
     return RunProgramWithTimeout(LLIPath, Args,
                                  InputFile, OutputFile, OutputFile);
   } else {
-    std::string SharedLibOpt = "-load=" + SharedLib;
     const char* Args[] = {
       LLIPath.c_str(), "-quiet", "-force-interpreter=false", 
-      SharedLibOpt.c_str(),
+      "-load", SharedLib.c_str(),
       Bytecode.c_str(),
       0
     };
@@ -374,6 +390,10 @@ public:
     Message = "Found dis: " + DISPath + "\n";
 
     GCC *gcc = GCC::create(BD, Message);
+    if (!gcc) {
+      std::cerr << Message << "\n";
+      exit(1);
+    }
     return new CBE(DISPath, gcc);
   }
 
@@ -421,7 +441,7 @@ int CBE::ExecuteProgram(const std::string &Bytecode,
     exit(1);
   }
 
-  int Result = gcc->ExecuteProgram(OutputCFile, OutputFile, SharedLib);
+  int Result = gcc->ExecuteProgram(OutputCFile, CFile, OutputFile, SharedLib);
   removeFile(OutputCFile);
 
   return Result;
@@ -456,6 +476,12 @@ bool BugDriver::initializeExecutionEnvironment() {
 
   std::cout << Message;
 
+  // Initialize auxiliary tools for debugging
+  cbe = CBE::create(this, Message);
+  if (!cbe) { std::cout << Message << "\nExiting.\n"; exit(1); }
+  gcc = GCC::create(this, Message);
+  if (!gcc) { std::cout << Message << "\nExiting.\n"; exit(1); }
+
   // If there was an error creating the selected interpreter, quit with error.
   return Interpreter == 0;
 }
@@ -503,11 +529,7 @@ std::string BugDriver::executeProgram(std::string OutputFile,
 std::string BugDriver::executeProgramWithCBE(std::string OutputFile,
                                              std::string BytecodeFile,
                                              std::string SharedObject) {
-  std::string Output;
-  CBE *cbe = CBE::create(this, Output);
-  Output = executeProgram(OutputFile, BytecodeFile, SharedObject, cbe);
-  delete cbe;
-  return Output;
+  return executeProgram(OutputFile, BytecodeFile, SharedObject, cbe);
 }
 
 int BugDriver::compileSharedObject(const std::string &BytecodeFile,
@@ -516,7 +538,6 @@ int BugDriver::compileSharedObject(const std::string &BytecodeFile,
   std::string Message, OutputCFile;
 
   // Using CBE
-  CBE *cbe = CBE::create(this, Message);
   cbe->OutputC(BytecodeFile, OutputCFile);
 
 #if 0 /* This is an alternative, as yet unimplemented */
@@ -528,16 +549,11 @@ int BugDriver::compileSharedObject(const std::string &BytecodeFile,
   }
 #endif
 
-  GCC *gcc = GCC::create(this, Message);
-  gcc->MakeSharedObject(OutputCFile, SharedObject);
+  gcc->MakeSharedObject(OutputCFile, CFile, SharedObject);
 
   // Remove the intermediate C file
   removeFile(OutputCFile);
 
-  // We are done with the CBE & GCC
-  delete cbe;
-  delete gcc;
-
   return 0;
 }