Return a std::unique_ptr from parseInputFile and propagate. NFC.

The memory management in BugPoint is fairly convoluted, so this just unwraps
one layer by changing the return type of functions that always return
owned Modules.

llvm-svn: 216464

llvm/tools/bugpoint/BugDriver.cpp

@@ -82,14 +82,10 @@ BugDriver::~BugDriver() {
   delete gcc;
 }
 
-
+std::unique_ptr<Module> llvm::parseInputFile(StringRef Filename,
-/// ParseInputFile - Given a bitcode or assembly input filename, parse and
+                                             LLVMContext &Ctxt) {
-/// return it, or return null if not possible.
-///
-Module *llvm::ParseInputFile(const std::string &Filename,
-                             LLVMContext& Ctxt) {
   SMDiagnostic Err;
-  Module *Result = ParseIRFile(Filename, Err, Ctxt);
+  std::unique_ptr<Module> Result (ParseIRFile(Filename, Err, Ctxt));
   if (!Result)
     Err.print("bugpoint", errs());
 
@@ -120,13 +116,13 @@ bool BugDriver::addSources(const std::vector<std::string> &Filenames) {
   assert(!Filenames.empty() && "Must specify at least on input filename!");
 
   // Load the first input file.
-  Program = ParseInputFile(Filenames[0], Context);
+  Program = parseInputFile(Filenames[0], Context).release();
   if (!Program) return true;
 
   outs() << "Read input file      : '" << Filenames[0] << "'\n";
 
   for (unsigned i = 1, e = Filenames.size(); i != e; ++i) {
-    std::unique_ptr<Module> M(ParseInputFile(Filenames[i], Context));
+    std::unique_ptr<Module> M = parseInputFile(Filenames[i], Context);
     if (!M.get()) return true;
 
     outs() << "Linking in input file: '" << Filenames[i] << "'\n";

llvm/tools/bugpoint/BugDriver.h

@@ -18,6 +18,7 @@
 
 #include "llvm/IR/ValueMap.h"
 #include "llvm/Transforms/Utils/ValueMapper.h"
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -210,41 +211,46 @@ public:
   void EmitProgressBitcode(const Module *M, const std::string &ID,
                            bool NoFlyer = false) const;
 
-  /// deleteInstructionFromProgram - This method clones the current Program and
+  /// This method clones the current Program and deletes the specified
-  /// deletes the specified instruction from the cloned module.  It then runs a
+  /// instruction from the cloned module.  It then runs a series of cleanup
-  /// series of cleanup passes (ADCE and SimplifyCFG) to eliminate any code
+  /// passes (ADCE and SimplifyCFG) to eliminate any code which depends on the
-  /// which depends on the value.  The modified module is then returned.
+  /// value. The modified module is then returned.
   ///
-  Module *deleteInstructionFromProgram(const Instruction *I, unsigned Simp);
+  std::unique_ptr<Module> deleteInstructionFromProgram(const Instruction *I,
+                                                       unsigned Simp);
 
-  /// performFinalCleanups - This method clones the current Program and performs
+  /// This method clones the current Program and performs a series of cleanups
-  /// a series of cleanups intended to get rid of extra cruft on the module.  If
+  /// intended to get rid of extra cruft on the module. If the
-  /// the MayModifySemantics argument is true, then the cleanups is allowed to
+  /// MayModifySemantics argument is true, then the cleanups is allowed to
   /// modify how the code behaves.
   ///
-  Module *performFinalCleanups(Module *M, bool MayModifySemantics = false);
+  std::unique_ptr<Module> performFinalCleanups(Module *M,
+                                               bool MayModifySemantics = false);
 
-  /// ExtractLoop - Given a module, extract up to one loop from it into a new
+  /// Given a module, extract up to one loop from it into a new function. This
-  /// function.  This returns null if there are no extractable loops in the
+  /// returns null if there are no extractable loops in the program or if the
-  /// program or if the loop extractor crashes.
+  /// loop extractor crashes.
-  Module *ExtractLoop(Module *M);
+  std::unique_ptr<Module> extractLoop(Module *M);
 
-  /// ExtractMappedBlocksFromModule - Extract all but the specified basic blocks
+  /// Extract all but the specified basic blocks into their own functions. The
-  /// into their own functions.  The only detail is that M is actually a module
+  /// only detail is that M is actually a module cloned from the one the BBs are
-  /// cloned from the one the BBs are in, so some mapping needs to be performed.
+  /// in, so some mapping needs to be performed. If this operation fails for
-  /// If this operation fails for some reason (ie the implementation is buggy),
+  /// some reason (ie the implementation is buggy), this function should return
-  /// this function should return null, otherwise it returns a new Module.
+  /// null, otherwise it returns a new Module.
-  Module *ExtractMappedBlocksFromModule(const std::vector<BasicBlock*> &BBs,
+  std::unique_ptr<Module>
-                                        Module *M);
+  extractMappedBlocksFromModule(const std::vector<BasicBlock *> &BBs,
+                                Module *M);
 
-  /// runPassesOn - Carefully run the specified set of pass on the specified
+  /// Carefully run the specified set of pass on the specified/ module,
-  /// module, returning the transformed module on success, or a null pointer on
+  /// returning the transformed module on success, or a null pointer on failure.
-  /// failure.  If AutoDebugCrashes is set to true, then bugpoint will
+  /// If AutoDebugCrashes is set to true, then bugpoint will automatically
-  /// automatically attempt to track down a crashing pass if one exists, and
+  /// attempt to track down a crashing pass if one exists, and this method will
-  /// this method will never return null.
+  /// never return null.
-  Module *runPassesOn(Module *M, const std::vector<std::string> &Passes,
+  std::unique_ptr<Module> runPassesOn(Module *M,
-                      bool AutoDebugCrashes = false, unsigned NumExtraArgs = 0,
+                                      const std::vector<std::string> &Passes,
-                      const char * const *ExtraArgs = nullptr);
+                                      bool AutoDebugCrashes = false,
+                                      unsigned NumExtraArgs = 0,
+                                      const char *const *ExtraArgs = nullptr);
 
   /// runPasses - Run the specified passes on Program, outputting a bitcode
   /// file and writting the filename into OutputFile if successful.  If the
@@ -296,12 +302,11 @@ private:
   bool initializeExecutionEnvironment();
 };
 
-/// ParseInputFile - Given a bitcode or assembly input filename, parse and
+///  Given a bitcode or assembly input filename, parse and return it, or return
-/// return it, or return null if not possible.
+///  null if not possible.
 ///
-Module *ParseInputFile(const std::string &InputFilename,
+std::unique_ptr<Module> parseInputFile(StringRef InputFilename,
-                       LLVMContext& ctxt);
+                                       LLVMContext &ctxt);
-
 
 /// getPassesString - Turn a list of passes into a string which indicates the
 /// command line options that must be passed to add the passes.

llvm/tools/bugpoint/CrashDebugger.cpp

@@ -72,7 +72,7 @@ ReducePassList::doTest(std::vector<std::string> &Prefix,
 
     OrigProgram = BD.Program;
 
-    BD.Program = ParseInputFile(PrefixOutput, BD.getContext());
+    BD.Program = parseInputFile(PrefixOutput, BD.getContext()).release();
     if (BD.Program == nullptr) {
       errs() << BD.getToolName() << ": Error reading bitcode file '"
              << PrefixOutput << "'!\n";
@@ -320,13 +320,13 @@ bool ReduceCrashingBlocks::TestBlocks(std::vector<const BasicBlock*> &BBs) {
   std::vector<std::string> Passes;
   Passes.push_back("simplifycfg");
   Passes.push_back("verify");
-  Module *New = BD.runPassesOn(M, Passes);
+  std::unique_ptr<Module> New = BD.runPassesOn(M, Passes);
   delete M;
   if (!New) {
     errs() << "simplifycfg failed!\n";
     exit(1);
   }
-  M = New;
+  M = New.release();
 
   // Try running on the hacked up program...
   if (TestFn(BD, M)) {
@@ -576,20 +576,17 @@ static bool DebugACrash(BugDriver &BD,
                 continue;
 
               outs() << "Checking instruction: " << *I;
-              Module *M = BD.deleteInstructionFromProgram(I, Simplification);
+              std::unique_ptr<Module> M =
+                  BD.deleteInstructionFromProgram(I, Simplification);
 
               // Find out if the pass still crashes on this pass...
-              if (TestFn(BD, M)) {
+              if (TestFn(BD, M.get())) {
                 // Yup, it does, we delete the old module, and continue trying
                 // to reduce the testcase...
-                BD.setNewProgram(M);
+                BD.setNewProgram(M.release());
                 InstructionsToSkipBeforeDeleting = CurInstructionNum;
                 goto TryAgain;  // I wish I had a multi-level break here!
               }
-
-              // This pass didn't crash without this instruction, try the next
-              // one.
-              delete M;
             }
           }
 
@@ -605,7 +602,7 @@ ExitLoops:
   if (!BugpointIsInterrupted) {
     outs() << "\n*** Attempting to perform final cleanups: ";
     Module *M = CloneModule(BD.getProgram());
-    M = BD.performFinalCleanups(M, true);
+    M = BD.performFinalCleanups(M, true).release();
 
     // Find out if the pass still crashes on the cleaned up program...
     if (TestFn(BD, M)) {

llvm/tools/bugpoint/ExtractFunction.cpp

@@ -82,13 +82,9 @@ namespace {
   }
 }  // end anonymous namespace
 
-/// deleteInstructionFromProgram - This method clones the current Program and
+std::unique_ptr<Module>
-/// deletes the specified instruction from the cloned module.  It then runs a
+BugDriver::deleteInstructionFromProgram(const Instruction *I,
-/// series of cleanup passes (ADCE and SimplifyCFG) to eliminate any code which
+                                        unsigned Simplification) {
-/// depends on the value.  The modified module is then returned.
-///
-Module *BugDriver::deleteInstructionFromProgram(const Instruction *I,
-                                                unsigned Simplification) {
   // FIXME, use vmap?
   Module *Clone = CloneModule(Program);
 
@@ -123,7 +119,7 @@ Module *BugDriver::deleteInstructionFromProgram(const Instruction *I,
     Passes.push_back("simplifycfg");      // Delete dead control flow
 
   Passes.push_back("verify");
-  Module *New = runPassesOn(Clone, Passes);
+  std::unique_ptr<Module> New = runPassesOn(Clone, Passes);
   delete Clone;
   if (!New) {
     errs() << "Instruction removal failed.  Sorry. :(  Please report a bug!\n";
@@ -132,11 +128,8 @@ Module *BugDriver::deleteInstructionFromProgram(const Instruction *I,
   return New;
 }
 
-/// performFinalCleanups - This method clones the current Program and performs
+std::unique_ptr<Module>
-/// a series of cleanups intended to get rid of extra cruft on the module
+BugDriver::performFinalCleanups(Module *M, bool MayModifySemantics) {
-/// before handing it to the user.
-///
-Module *BugDriver::performFinalCleanups(Module *M, bool MayModifySemantics) {
   // Make all functions external, so GlobalDCE doesn't delete them...
   for (Module::iterator I = M->begin(), E = M->end(); I != E; ++I)
     I->setLinkage(GlobalValue::ExternalLinkage);
@@ -149,24 +142,20 @@ Module *BugDriver::performFinalCleanups(Module *M, bool MayModifySemantics) {
   else
     CleanupPasses.push_back("deadargelim");
 
-  Module *New = runPassesOn(M, CleanupPasses);
+  std::unique_ptr<Module> New = runPassesOn(M, CleanupPasses);
   if (!New) {
     errs() << "Final cleanups failed.  Sorry. :(  Please report a bug!\n";
-    return M;
+    return nullptr;
   }
   delete M;
   return New;
 }
 
-
+std::unique_ptr<Module> BugDriver::extractLoop(Module *M) {
-/// ExtractLoop - Given a module, extract up to one loop from it into a new
-/// function.  This returns null if there are no extractable loops in the
-/// program or if the loop extractor crashes.
-Module *BugDriver::ExtractLoop(Module *M) {
   std::vector<std::string> LoopExtractPasses;
   LoopExtractPasses.push_back("loop-extract-single");
 
-  Module *NewM = runPassesOn(M, LoopExtractPasses);
+  std::unique_ptr<Module> NewM = runPassesOn(M, LoopExtractPasses);
   if (!NewM) {
     outs() << "*** Loop extraction failed: ";
     EmitProgressBitcode(M, "loopextraction", true);
@@ -179,7 +168,6 @@ Module *BugDriver::ExtractLoop(Module *M) {
   // to avoid taking forever.
   static unsigned NumExtracted = 32;
   if (M->size() == NewM->size() || --NumExtracted == 0) {
-    delete NewM;
     return nullptr;
   } else {
     assert(M->size() < NewM->size() && "Loop extract removed functions?");
@@ -356,14 +344,9 @@ llvm::SplitFunctionsOutOfModule(Module *M,
 // Basic Block Extraction Code
 //===----------------------------------------------------------------------===//
 
-/// ExtractMappedBlocksFromModule - Extract all but the specified basic blocks
+std::unique_ptr<Module>
-/// into their own functions.  The only detail is that M is actually a module
+BugDriver::extractMappedBlocksFromModule(const std::vector<BasicBlock *> &BBs,
-/// cloned from the one the BBs are in, so some mapping needs to be performed.
+                                         Module *M) {
-/// If this operation fails for some reason (ie the implementation is buggy),
-/// this function should return null, otherwise it returns a new Module.
-Module *BugDriver::ExtractMappedBlocksFromModule(const
-                                                 std::vector<BasicBlock*> &BBs,
-                                                 Module *M) {
   SmallString<128> Filename;
   int FD;
   std::error_code EC = sys::fs::createUniqueFile(
@@ -401,7 +384,7 @@ Module *BugDriver::ExtractMappedBlocksFromModule(const
 
   std::vector<std::string> PI;
   PI.push_back("extract-blocks");
-  Module *Ret = runPassesOn(M, PI, false, 1, &ExtraArg);
+  std::unique_ptr<Module> Ret = runPassesOn(M, PI, false, 1, &ExtraArg);
 
   sys::fs::remove(Filename.c_str());
 

llvm/tools/bugpoint/Miscompilation.cpp

@@ -128,8 +128,8 @@ ReduceMiscompilingPasses::doTest(std::vector<std::string> &Prefix,
   // Ok, so now we know that the prefix passes work, try running the suffix
   // passes on the result of the prefix passes.
   //
-  std::unique_ptr<Module> PrefixOutput(
+  std::unique_ptr<Module> PrefixOutput =
-      ParseInputFile(BitcodeResult, BD.getContext()));
+      parseInputFile(BitcodeResult, BD.getContext());
   if (!PrefixOutput) {
     errs() << BD.getToolName() << ": Error reading bitcode file '"
            << BitcodeResult << "'!\n";
@@ -316,7 +316,7 @@ static bool ExtractLoops(BugDriver &BD,
     Module *ToOptimize = SplitFunctionsOutOfModule(ToNotOptimize,
                                                    MiscompiledFunctions,
                                                    VMap);
-    Module *ToOptimizeLoopExtracted = BD.ExtractLoop(ToOptimize);
+    Module *ToOptimizeLoopExtracted = BD.extractLoop(ToOptimize).release();
     if (!ToOptimizeLoopExtracted) {
       // If the loop extractor crashed or if there were no extractible loops,
       // then this chapter of our odyssey is over with.
@@ -334,8 +334,8 @@ static bool ExtractLoops(BugDriver &BD,
     // extraction.
     AbstractInterpreter *AI = BD.switchToSafeInterpreter();
     bool Failure;
-    Module *New = TestMergedProgram(BD, ToOptimizeLoopExtracted, ToNotOptimize,
+    Module *New = TestMergedProgram(BD, ToOptimizeLoopExtracted,
-                                    false, Error, Failure);
+                                    ToNotOptimize, false, Error, Failure);
     if (!New)
       return false;
 
@@ -364,7 +364,6 @@ static bool ExtractLoops(BugDriver &BD,
              << OutputPrefix << "-loop-extract-fail-*.bc files.\n";
       delete ToOptimize;
       delete ToNotOptimize;
-      delete ToOptimizeLoopExtracted;
       return MadeChange;
     }
     delete ToOptimize;
@@ -533,11 +532,12 @@ bool ReduceMiscompiledBlocks::TestFuncs(const std::vector<BasicBlock*> &BBs,
 
   // Try the extraction.  If it doesn't work, then the block extractor crashed
   // or something, in which case bugpoint can't chase down this possibility.
-  if (Module *New = BD.ExtractMappedBlocksFromModule(BBsOnClone, ToOptimize)) {
+  if (std::unique_ptr<Module> New =
+          BD.extractMappedBlocksFromModule(BBsOnClone, ToOptimize)) {
     delete ToOptimize;
     // Run the predicate,
     // note that the predicate will delete both input modules.
-    bool Ret = TestFn(BD, New, ToNotOptimize, Error);
+    bool Ret = TestFn(BD, New.get(), ToNotOptimize, Error);
     delete BD.swapProgramIn(Orig);
     return Ret;
   }
@@ -591,7 +591,8 @@ static bool ExtractBlocks(BugDriver &BD,
   Module *ToExtract = SplitFunctionsOutOfModule(ProgClone,
                                                 MiscompiledFunctions,
                                                 VMap);
-  Module *Extracted = BD.ExtractMappedBlocksFromModule(Blocks, ToExtract);
+  std::unique_ptr<Module> Extracted =
+      BD.extractMappedBlocksFromModule(Blocks, ToExtract);
   if (!Extracted) {
     // Weird, extraction should have worked.
     errs() << "Nondeterministic problem extracting blocks??\n";
@@ -612,13 +613,12 @@ static bool ExtractBlocks(BugDriver &BD,
                                                 I->getFunctionType()));
 
   std::string ErrorMsg;
-  if (Linker::LinkModules(ProgClone, Extracted, Linker::DestroySource, 
+  if (Linker::LinkModules(ProgClone, Extracted.get(), Linker::DestroySource, 
                           &ErrorMsg)) {
     errs() << BD.getToolName() << ": Error linking modules together:"
            << ErrorMsg << '\n';
     exit(1);
   }
-  delete Extracted;
 
   // Set the new program and delete the old one.
   BD.setNewProgram(ProgClone);
@@ -730,14 +730,15 @@ static bool TestOptimizer(BugDriver &BD, Module *Test, Module *Safe,
   // Run the optimization passes on ToOptimize, producing a transformed version
   // of the functions being tested.
   outs() << "  Optimizing functions being tested: ";
-  Module *Optimized = BD.runPassesOn(Test, BD.getPassesToRun(),
+  std::unique_ptr<Module> Optimized = BD.runPassesOn(Test, BD.getPassesToRun(),
-                                     /*AutoDebugCrashes*/true);
+                                                     /*AutoDebugCrashes*/ true);
   outs() << "done.\n";
   delete Test;
 
   outs() << "  Checking to see if the merged program executes correctly: ";
   bool Broken;
-  Module *New = TestMergedProgram(BD, Optimized, Safe, true, Error, Broken);
+  Module *New =
+      TestMergedProgram(BD, Optimized.get(), Safe, true, Error, Broken);
   if (New) {
     outs() << (Broken ? " nope.\n" : " yup.\n");
     // Delete the original and set the new program.
@@ -796,7 +797,7 @@ void BugDriver::debugMiscompilation(std::string *Error) {
 static void CleanupAndPrepareModules(BugDriver &BD, Module *&Test,
                                      Module *Safe) {
   // Clean up the modules, removing extra cruft that we don't need anymore...
-  Test = BD.performFinalCleanups(Test);
+  Test = BD.performFinalCleanups(Test).release();
 
   // If we are executing the JIT, we have several nasty issues to take care of.
   if (!BD.isExecutingJIT()) return;

llvm/tools/bugpoint/OptimizerDriver.cpp

@@ -247,13 +247,10 @@ bool BugDriver::runPasses(Module *Program,
 }
 
 
-/// runPassesOn - Carefully run the specified set of pass on the specified
+std::unique_ptr<Module>
-/// module, returning the transformed module on success, or a null pointer on
+BugDriver::runPassesOn(Module *M, const std::vector<std::string> &Passes,
-/// failure.
+                       bool AutoDebugCrashes, unsigned NumExtraArgs,
-Module *BugDriver::runPassesOn(Module *M,
+                       const char *const *ExtraArgs) {
-                               const std::vector<std::string> &Passes,
-                               bool AutoDebugCrashes, unsigned NumExtraArgs,
-                               const char * const *ExtraArgs) {
   std::string BitcodeResult;
   if (runPasses(M, Passes, BitcodeResult, false/*delete*/, true/*quiet*/,
                 NumExtraArgs, ExtraArgs)) {
@@ -267,7 +264,7 @@ Module *BugDriver::runPassesOn(Module *M,
     return nullptr;
   }
 
-  Module *Ret = ParseInputFile(BitcodeResult, Context);
+  std::unique_ptr<Module> Ret = parseInputFile(BitcodeResult, Context);
   if (!Ret) {
     errs() << getToolName() << ": Error reading bitcode file '"
            << BitcodeResult << "'!\n";