From 28b351a56dce9c633e97cdf2762b23b136642c94 Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Tue, 26 Aug 2014 17:19:03 +0000 Subject: [PATCH] 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 | 14 ++--- llvm/tools/bugpoint/BugDriver.h | 71 +++++++++++++------------ llvm/tools/bugpoint/CrashDebugger.cpp | 19 +++---- llvm/tools/bugpoint/ExtractFunction.cpp | 45 +++++----------- llvm/tools/bugpoint/Miscompilation.cpp | 31 +++++------ llvm/tools/bugpoint/OptimizerDriver.cpp | 13 ++--- 6 files changed, 86 insertions(+), 107 deletions(-) diff --git a/llvm/tools/bugpoint/BugDriver.cpp b/llvm/tools/bugpoint/BugDriver.cpp index cecccbe0f0ef..872f1ba13b9c 100644 --- a/llvm/tools/bugpoint/BugDriver.cpp +++ b/llvm/tools/bugpoint/BugDriver.cpp @@ -82,14 +82,10 @@ BugDriver::~BugDriver() { delete gcc; } - -/// ParseInputFile - Given a bitcode or assembly input filename, parse and -/// return it, or return null if not possible. -/// -Module *llvm::ParseInputFile(const std::string &Filename, - LLVMContext& Ctxt) { +std::unique_ptr llvm::parseInputFile(StringRef Filename, + LLVMContext &Ctxt) { SMDiagnostic Err; - Module *Result = ParseIRFile(Filename, Err, Ctxt); + std::unique_ptr Result (ParseIRFile(Filename, Err, Ctxt)); if (!Result) Err.print("bugpoint", errs()); @@ -120,13 +116,13 @@ bool BugDriver::addSources(const std::vector &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 M(ParseInputFile(Filenames[i], Context)); + std::unique_ptr M = parseInputFile(Filenames[i], Context); if (!M.get()) return true; outs() << "Linking in input file: '" << Filenames[i] << "'\n"; diff --git a/llvm/tools/bugpoint/BugDriver.h b/llvm/tools/bugpoint/BugDriver.h index 2ef22f44dbf9..579781246c54 100644 --- a/llvm/tools/bugpoint/BugDriver.h +++ b/llvm/tools/bugpoint/BugDriver.h @@ -18,6 +18,7 @@ #include "llvm/IR/ValueMap.h" #include "llvm/Transforms/Utils/ValueMapper.h" +#include #include #include @@ -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 - /// deletes the specified instruction from the cloned module. It then runs a - /// series of cleanup passes (ADCE and SimplifyCFG) to eliminate any code - /// which depends on the value. The modified module is then returned. + /// This method clones the current Program and deletes the specified + /// instruction from the cloned module. It then runs a series of cleanup + /// passes (ADCE and SimplifyCFG) to eliminate any code which depends on the + /// value. The modified module is then returned. /// - Module *deleteInstructionFromProgram(const Instruction *I, unsigned Simp); + std::unique_ptr deleteInstructionFromProgram(const Instruction *I, + unsigned Simp); - /// performFinalCleanups - This method clones the current Program and performs - /// a series of cleanups intended to get rid of extra cruft on the module. If - /// the MayModifySemantics argument is true, then the cleanups is allowed to + /// This method clones the current Program and performs a series of cleanups + /// intended to get rid of extra cruft on the module. If the + /// 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 performFinalCleanups(Module *M, + bool MayModifySemantics = false); - /// 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 *ExtractLoop(Module *M); + /// 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. + std::unique_ptr extractLoop(Module *M); - /// ExtractMappedBlocksFromModule - Extract all but the specified basic blocks - /// into their own functions. The only detail is that M is actually a module - /// cloned from the one the BBs are in, so some mapping needs to be performed. - /// If this operation fails for some reason (ie the implementation is buggy), - /// this function should return null, otherwise it returns a new Module. - Module *ExtractMappedBlocksFromModule(const std::vector &BBs, - Module *M); + /// Extract all but the specified basic blocks into their own functions. The + /// only detail is that M is actually a module cloned from the one the BBs are + /// in, so some mapping needs to be performed. If this operation fails for + /// some reason (ie the implementation is buggy), this function should return + /// null, otherwise it returns a new Module. + std::unique_ptr + extractMappedBlocksFromModule(const std::vector &BBs, + Module *M); - /// runPassesOn - Carefully run the specified set of pass on the specified - /// module, returning the transformed module on success, or a null pointer on - /// failure. If AutoDebugCrashes is set to true, then bugpoint will - /// automatically attempt to track down a crashing pass if one exists, and - /// this method will never return null. - Module *runPassesOn(Module *M, const std::vector &Passes, - bool AutoDebugCrashes = false, unsigned NumExtraArgs = 0, - const char * const *ExtraArgs = nullptr); + /// Carefully run the specified set of pass on the specified/ module, + /// returning the transformed module on success, or a null pointer on failure. + /// If AutoDebugCrashes is set to true, then bugpoint will automatically + /// attempt to track down a crashing pass if one exists, and this method will + /// never return null. + std::unique_ptr runPassesOn(Module *M, + const std::vector &Passes, + 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 -/// return it, or return null if not possible. +/// Given a bitcode or assembly input filename, parse and return it, or return +/// null if not possible. /// -Module *ParseInputFile(const std::string &InputFilename, - LLVMContext& ctxt); - +std::unique_ptr parseInputFile(StringRef InputFilename, + 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. diff --git a/llvm/tools/bugpoint/CrashDebugger.cpp b/llvm/tools/bugpoint/CrashDebugger.cpp index 60d4123c184b..bac948aaf3ab 100644 --- a/llvm/tools/bugpoint/CrashDebugger.cpp +++ b/llvm/tools/bugpoint/CrashDebugger.cpp @@ -72,7 +72,7 @@ ReducePassList::doTest(std::vector &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 &BBs) { std::vector Passes; Passes.push_back("simplifycfg"); Passes.push_back("verify"); - Module *New = BD.runPassesOn(M, Passes); + std::unique_ptr 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 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)) { diff --git a/llvm/tools/bugpoint/ExtractFunction.cpp b/llvm/tools/bugpoint/ExtractFunction.cpp index 4fb68566ac62..34fe53c08112 100644 --- a/llvm/tools/bugpoint/ExtractFunction.cpp +++ b/llvm/tools/bugpoint/ExtractFunction.cpp @@ -82,13 +82,9 @@ namespace { } } // end anonymous namespace -/// deleteInstructionFromProgram - This method clones the current Program and -/// deletes the specified instruction from the cloned module. It then runs a -/// series of cleanup passes (ADCE and SimplifyCFG) to eliminate any code which -/// depends on the value. The modified module is then returned. -/// -Module *BugDriver::deleteInstructionFromProgram(const Instruction *I, - unsigned Simplification) { +std::unique_ptr +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 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 -/// a series of cleanups intended to get rid of extra cruft on the module -/// before handing it to the user. -/// -Module *BugDriver::performFinalCleanups(Module *M, bool MayModifySemantics) { +std::unique_ptr +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 New = runPassesOn(M, CleanupPasses); if (!New) { errs() << "Final cleanups failed. Sorry. :( Please report a bug!\n"; - return M; + return nullptr; } delete M; return New; } - -/// 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::unique_ptr BugDriver::extractLoop(Module *M) { std::vector LoopExtractPasses; LoopExtractPasses.push_back("loop-extract-single"); - Module *NewM = runPassesOn(M, LoopExtractPasses); + std::unique_ptr 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 -/// into their own functions. The only detail is that M is actually a module -/// cloned from the one the BBs are in, so some mapping needs to be performed. -/// 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 &BBs, - Module *M) { +std::unique_ptr +BugDriver::extractMappedBlocksFromModule(const std::vector &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 PI; PI.push_back("extract-blocks"); - Module *Ret = runPassesOn(M, PI, false, 1, &ExtraArg); + std::unique_ptr Ret = runPassesOn(M, PI, false, 1, &ExtraArg); sys::fs::remove(Filename.c_str()); diff --git a/llvm/tools/bugpoint/Miscompilation.cpp b/llvm/tools/bugpoint/Miscompilation.cpp index 3f1f84ef6244..016242086128 100644 --- a/llvm/tools/bugpoint/Miscompilation.cpp +++ b/llvm/tools/bugpoint/Miscompilation.cpp @@ -128,8 +128,8 @@ ReduceMiscompilingPasses::doTest(std::vector &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 PrefixOutput( - ParseInputFile(BitcodeResult, BD.getContext())); + std::unique_ptr PrefixOutput = + 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, - false, Error, Failure); + Module *New = TestMergedProgram(BD, ToOptimizeLoopExtracted, + 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 &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 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 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(), - /*AutoDebugCrashes*/true); + std::unique_ptr Optimized = BD.runPassesOn(Test, BD.getPassesToRun(), + /*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; diff --git a/llvm/tools/bugpoint/OptimizerDriver.cpp b/llvm/tools/bugpoint/OptimizerDriver.cpp index c75f82f17dc3..3854aa1e5bfb 100644 --- a/llvm/tools/bugpoint/OptimizerDriver.cpp +++ b/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 -/// module, returning the transformed module on success, or a null pointer on -/// failure. -Module *BugDriver::runPassesOn(Module *M, - const std::vector &Passes, - bool AutoDebugCrashes, unsigned NumExtraArgs, - const char * const *ExtraArgs) { +std::unique_ptr +BugDriver::runPassesOn(Module *M, const std::vector &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 Ret = parseInputFile(BitcodeResult, Context); if (!Ret) { errs() << getToolName() << ": Error reading bitcode file '" << BitcodeResult << "'!\n";