From 491f3054b8a116b3f4e112d4b9233107e5f7e711 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Sun, 14 Mar 2004 20:50:42 +0000 Subject: [PATCH] Refactor and clean up a bunch more code. No major functionality changes. * Make several methods of bugdriver global functions (ParseInputFile, PrintFunctionList) * Make PrintFunctionList truncate the output after 10 entries, like the crash debugger did. This allows code sharing. * Add a couple of methods to BugDriver that allows us to eliminate some friends * Improve comments in ExtractFunction.cpp * Make classes that used to be friends up bugdriver now live in anon namespaces * Rip a bunch of functionality in the miscompilation tester into a new TestMergedProgram function for future code sharing. * Fix a bug in the miscompilation tester induced in my last checkin llvm-svn: 12393 --- tools/bugpoint/BugDriver.cpp | 19 ++++--- tools/bugpoint/BugDriver.h | 47 ++++++++------- tools/bugpoint/CodeGeneratorBug.cpp | 2 +- tools/bugpoint/CrashDebugger.cpp | 25 ++++---- tools/bugpoint/ExtractFunction.cpp | 6 +- tools/bugpoint/Miscompilation.cpp | 88 +++++++++++++++-------------- 6 files changed, 98 insertions(+), 89 deletions(-) diff --git a/tools/bugpoint/BugDriver.cpp b/tools/bugpoint/BugDriver.cpp index 2c5cf24890f..aca03fc56a1 100644 --- a/tools/bugpoint/BugDriver.cpp +++ b/tools/bugpoint/BugDriver.cpp @@ -68,16 +68,16 @@ BugDriver::BugDriver(const char *toolname) /// ParseInputFile - Given a bytecode or assembly input filename, parse and /// return it, or return null if not possible. /// -Module *BugDriver::ParseInputFile(const std::string &InputFilename) const { +Module *llvm::ParseInputFile(const std::string &InputFilename) { Module *Result = 0; try { Result = ParseBytecodeFile(InputFilename); if (!Result && !(Result = ParseAssemblyFile(InputFilename))){ - std::cerr << ToolName << ": could not read input file '" + std::cerr << "bugpoint: could not read input file '" << InputFilename << "'!\n"; } } catch (const ParseException &E) { - std::cerr << ToolName << ": " << E.getMessage() << "\n"; + std::cerr << "bugpoint: " << E.getMessage() << "\n"; Result = 0; } return Result; @@ -199,11 +199,12 @@ bool BugDriver::run() { } } -void BugDriver::PrintFunctionList(const std::vector &Funcs) { - for (unsigned i = 0, e = Funcs.size(); i != e; ++i) { - if (i) std::cout << ", "; - std::cout << Funcs[i]->getName(); - } +void llvm::PrintFunctionList(const std::vector &Funcs) { + unsigned NumPrint = Funcs.size(); + if (NumPrint > 10) NumPrint = 10; + for (unsigned i = 0; i != NumPrint; ++i) + std::cout << " " << Funcs[i]->getName(); + if (NumPrint < Funcs.size()) + std::cout << "... <" << Funcs.size() << " total>"; std::cout << std::flush; } - diff --git a/tools/bugpoint/BugDriver.h b/tools/bugpoint/BugDriver.h index eb48eb7d9c3..a4428cffea9 100644 --- a/tools/bugpoint/BugDriver.h +++ b/tools/bugpoint/BugDriver.h @@ -28,8 +28,6 @@ class AbstractInterpreter; class Instruction; class DebugCrashes; -class ReduceMiscompilingPasses; -class ReduceMiscompilingFunctions; class CBE; class GCC; @@ -47,8 +45,6 @@ class BugDriver { // FIXME: sort out public/private distinctions... friend class ReducePassList; - friend class ReduceMiscompilingPasses; - friend class ReduceMiscompilingFunctions; friend class ReduceMisCodegenFunctions; public: @@ -65,6 +61,9 @@ public: void setPassesToRun(const std::vector &PTR) { PassesToRun = PTR; } + const std::vector &getPassesToRun() const { + return PassesToRun; + } /// run - The top level method that is invoked after all of the instance /// variables are set up from command line arguments. @@ -120,7 +119,15 @@ public: return Result; } - const Module *getProgram() const { return Program; } + Module *getProgram() const { return Program; } + + /// swapProgramIn - Set the current module to the specified module, returning + /// the old one. + Module *swapProgramIn(Module *M) { + Module *OldProgram = Program; + Program = M; + return OldProgram; + } /// setNewProgram - If we reduce or update the program somehow, call this /// method to update bugdriver with it. This deletes the old module and sets @@ -183,17 +190,6 @@ public: /// program or if the loop extractor crashes. Module *ExtractLoop(Module *M); -private: - /// ParseInputFile - Given a bytecode or assembly input filename, parse and - /// return it, or return null if not possible. - /// - Module *ParseInputFile(const std::string &InputFilename) const; - - /// writeProgramToFile - This writes the current "Program" to the named - /// bytecode file. If an error occurs, true is returned. - /// - bool writeProgramToFile(const std::string &Filename, Module *M = 0) const; - /// runPasses - Run the specified passes on Program, outputting a bytecode /// file and writting the filename into OutputFile if successful. If the /// optimizations fail for some reason (optimizer crashes), return true, @@ -205,6 +201,11 @@ private: bool runPasses(const std::vector &PassesToRun, std::string &OutputFilename, bool DeleteOutput = false, bool Quiet = false) const; +private: + /// writeProgramToFile - This writes the current "Program" to the named + /// bytecode file. If an error occurs, true is returned. + /// + bool writeProgramToFile(const std::string &Filename, Module *M = 0) const; /// runPasses - Just like the method above, but this just returns true or /// false indicating whether or not the optimizer crashed on the specified @@ -216,21 +217,27 @@ private: return runPasses(PassesToRun, Filename, DeleteOutput); } - /// PrintFunctionList - prints out list of problematic functions - /// - static void PrintFunctionList(const std::vector &Funcs); - /// initializeExecutionEnvironment - This method is used to set up the /// environment for executing LLVM programs. /// bool initializeExecutionEnvironment(); }; +/// ParseInputFile - Given a bytecode or assembly input filename, parse and +/// return it, or return null if not possible. +/// +Module *ParseInputFile(const std::string &InputFilename); + + /// getPassesString - Turn a list of passes into a string which indicates the /// command line options that must be passed to add the passes. /// std::string getPassesString(const std::vector &Passes); +/// PrintFunctionList - prints out list of problematic functions +/// +void PrintFunctionList(const std::vector &Funcs); + // DeleteFunctionBody - "Remove" the function by deleting all of it's basic // blocks, making it external. // diff --git a/tools/bugpoint/CodeGeneratorBug.cpp b/tools/bugpoint/CodeGeneratorBug.cpp index 449aadd62f2..8569bb9337d 100644 --- a/tools/bugpoint/CodeGeneratorBug.cpp +++ b/tools/bugpoint/CodeGeneratorBug.cpp @@ -59,7 +59,7 @@ namespace llvm { bool ReduceMisCodegenFunctions::TestFuncs(const std::vector &Funcs, bool KeepFiles) { std::cout << "Testing functions: "; - BD.PrintFunctionList(Funcs); + PrintFunctionList(Funcs); std::cout << "\t"; // Clone the module for the two halves of the program we want. diff --git a/tools/bugpoint/CrashDebugger.cpp b/tools/bugpoint/CrashDebugger.cpp index 04f494a0b62..2be8cd0244b 100644 --- a/tools/bugpoint/CrashDebugger.cpp +++ b/tools/bugpoint/CrashDebugger.cpp @@ -59,7 +59,7 @@ ReducePassList::doTest(std::vector &Prefix, OrigProgram = BD.Program; - BD.Program = BD.ParseInputFile(PrefixOutput); + BD.Program = ParseInputFile(PrefixOutput); if (BD.Program == 0) { std::cerr << BD.getToolName() << ": Error reading bytecode file '" << PrefixOutput << "'!\n"; @@ -85,7 +85,7 @@ ReducePassList::doTest(std::vector &Prefix, } namespace llvm { - class ReduceCrashingFunctions : public ListReducer { + class ReduceCrashingFunctions : public ListReducer { BugDriver &BD; bool (*TestFn)(BugDriver &, Module *); public: @@ -93,8 +93,8 @@ namespace llvm { bool (*testFn)(BugDriver &, Module *)) : BD(bd), TestFn(testFn) {} - virtual TestResult doTest(std::vector &Prefix, - std::vector &Kept) { + virtual TestResult doTest(std::vector &Prefix, + std::vector &Kept) { if (!Kept.empty() && TestFuncs(Kept)) return KeepSuffix; if (!Prefix.empty() && TestFuncs(Prefix)) @@ -102,11 +102,11 @@ namespace llvm { return NoFailure; } - bool TestFuncs(std::vector &Prefix); + bool TestFuncs(std::vector &Prefix); }; } -bool ReduceCrashingFunctions::TestFuncs(std::vector &Funcs) { +bool ReduceCrashingFunctions::TestFuncs(std::vector &Funcs) { // Clone the program to try hacking it apart... Module *M = CloneModule(BD.getProgram()); @@ -119,13 +119,8 @@ bool ReduceCrashingFunctions::TestFuncs(std::vector &Funcs) { Functions.insert(CMF); } - std::cout << "Checking for crash with only these functions:"; - unsigned NumPrint = Funcs.size(); - if (NumPrint > 10) NumPrint = 10; - for (unsigned i = 0; i != NumPrint; ++i) - std::cout << " " << Funcs[i]->getName(); - if (NumPrint < Funcs.size()) - std::cout << "... <" << Funcs.size() << " total>"; + std::cout << "Checking for crash with only these functions: "; + PrintFunctionList(Funcs); std::cout << ": "; // Loop over and delete any functions which we aren't supposed to be playing @@ -295,8 +290,8 @@ static bool DebugACrash(BugDriver &BD, bool (*TestFn)(BugDriver &, Module *)) { } // Now try to reduce the number of functions in the module to something small. - std::vector Functions; - for (Module::const_iterator I = BD.getProgram()->begin(), + std::vector Functions; + for (Module::iterator I = BD.getProgram()->begin(), E = BD.getProgram()->end(); I != E; ++I) if (!I->isExternal()) Functions.push_back(I); diff --git a/tools/bugpoint/ExtractFunction.cpp b/tools/bugpoint/ExtractFunction.cpp index 8e392613c93..22bd1738952 100644 --- a/tools/bugpoint/ExtractFunction.cpp +++ b/tools/bugpoint/ExtractFunction.cpp @@ -7,8 +7,8 @@ // //===----------------------------------------------------------------------===// // -// This file implements a method that extracts a function from program, cleans -// it up, and returns it as a new module. +// This file implements several methods that are used to extract functions, +// loops, or portions of a module from the rest of the module. // //===----------------------------------------------------------------------===// @@ -76,6 +76,8 @@ Module *BugDriver::deleteInstructionFromProgram(const Instruction *I, // Make sure that the appropriate target data is always used... Passes.add(new TargetData("bugpoint", Result)); + /// FIXME: If this used runPasses() like the methods below, we could get rid + /// of the -disable-* options! if (Simplification > 1 && !NoDCE) Passes.add(createDeadCodeEliminationPass()); if (Simplification && !DisableSimplifyCFG) diff --git a/tools/bugpoint/Miscompilation.cpp b/tools/bugpoint/Miscompilation.cpp index 29274c9df01..830f26f5146 100644 --- a/tools/bugpoint/Miscompilation.cpp +++ b/tools/bugpoint/Miscompilation.cpp @@ -20,8 +20,7 @@ #include "Support/FileUtilities.h" using namespace llvm; -namespace llvm { - +namespace { class ReduceMiscompilingPasses : public ListReducer { BugDriver &BD; public: @@ -88,7 +87,7 @@ 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. // - Module *PrefixOutput = BD.ParseInputFile(BytecodeResult); + Module *PrefixOutput = ParseInputFile(BytecodeResult); if (PrefixOutput == 0) { std::cerr << BD.getToolName() << ": Error reading bytecode file '" << BytecodeResult << "'!\n"; @@ -100,8 +99,7 @@ ReduceMiscompilingPasses::doTest(std::vector &Prefix, << "' passes compile correctly after the '" << getPassesString(Prefix) << "' passes: "; - Module *OriginalInput = BD.Program; - BD.Program = PrefixOutput; + Module *OriginalInput = BD.swapProgramIn(PrefixOutput); if (BD.runPasses(Suffix, BytecodeResult, false/*delete*/, true/*quiet*/)) { std::cerr << " Error running this sequence of passes" << " on the input program!\n"; @@ -119,12 +117,11 @@ ReduceMiscompilingPasses::doTest(std::vector &Prefix, // Otherwise, we must not be running the bad pass anymore. std::cout << "yup.\n"; // No miscompilation! - BD.Program = OriginalInput; // Restore original program - delete PrefixOutput; // Free experiment + delete BD.swapProgramIn(OriginalInput); // Restore orig program & free test return NoFailure; } -namespace llvm { +namespace { class ReduceMiscompilingFunctions : public ListReducer { BugDriver &BD; public: @@ -143,28 +140,52 @@ namespace llvm { }; } +/// TestMergedProgram - Given two modules, link them together and run the +/// program, checking to see if the program matches the diff. If the diff +/// matches, return false, otherwise return true. In either case, we delete +/// both input modules before we return. +static bool TestMergedProgram(BugDriver &BD, Module *M1, Module *M2) { + // Link the two portions of the program back to together. + std::string ErrorMsg; + if (LinkModules(M1, M2, &ErrorMsg)) { + std::cerr << BD.getToolName() << ": Error linking modules together:" + << ErrorMsg << "\n"; + exit(1); + } + delete M2; // We are done with this module... + + Module *OldProgram = BD.swapProgramIn(M1); + + // Execute the program. If it does not match the expected output, we must + // return true. + bool Broken = BD.diffProgram(); + + // Delete the linked module & restore the original + delete BD.swapProgramIn(OldProgram); + return Broken; +} + bool ReduceMiscompilingFunctions::TestFuncs(const std::vector&Funcs){ // Test to see if the function is misoptimized if we ONLY run it on the // functions listed in Funcs. std::cout << "Checking to see if the program is misoptimized when " << (Funcs.size()==1 ? "this function is" : "these functions are") << " run through the pass" - << (BD.PassesToRun.size() == 1 ? "" : "es") << ": "; - BD.PrintFunctionList(Funcs); + << (BD.getPassesToRun().size() == 1 ? "" : "es") << ": "; + PrintFunctionList(Funcs); std::cout << "\n"; // Split the module into the two halves of the program we want. - Module *ToOptimize = CloneModule(BD.getProgram()); - Module *ToNotOptimize = SplitFunctionsOutOfModule(ToOptimize, Funcs); + Module *ToNotOptimize = CloneModule(BD.getProgram()); + Module *ToOptimize = SplitFunctionsOutOfModule(ToNotOptimize, Funcs); // Run the optimization passes on ToOptimize, producing a transformed version // of the functions being tested. - Module *OldProgram = BD.Program; - BD.Program = ToOptimize; + Module *OldProgram = BD.swapProgramIn(ToOptimize); std::cout << " Optimizing functions being tested: "; std::string BytecodeResult; - if (BD.runPasses(BD.PassesToRun, BytecodeResult, false/*delete*/, + if (BD.runPasses(BD.getPassesToRun(), BytecodeResult, false/*delete*/, true/*quiet*/)) { std::cerr << " Error running this sequence of passes" << " on the input program!\n"; @@ -174,35 +195,18 @@ bool ReduceMiscompilingFunctions::TestFuncs(const std::vector&Funcs){ std::cout << "done.\n"; - delete BD.getProgram(); // Delete the old "ToOptimize" module - BD.Program = BD.ParseInputFile(BytecodeResult); - - if (BD.Program == 0) { + // Delete the old "ToOptimize" module + delete BD.swapProgramIn(OldProgram); + Module *Optimized = ParseInputFile(BytecodeResult); + if (Optimized == 0) { std::cerr << BD.getToolName() << ": Error reading bytecode file '" << BytecodeResult << "'!\n"; exit(1); } removeFile(BytecodeResult); // No longer need the file on disk - // Seventh step: Link the optimized part of the program back to the - // unoptimized part of the program. - // - if (LinkModules(BD.Program, ToNotOptimize, &BytecodeResult)) { - std::cerr << BD.getToolName() << ": Error linking modules together:" - << BytecodeResult << "\n"; - exit(1); - } - delete ToNotOptimize; // We are done with this module... - std::cout << " Checking to see if the merged program executes correctly: "; - - // Eighth step: Execute the program. If it does not match the expected - // output, then 'Funcs' are being misoptimized! - bool Broken = BD.diffProgram(); - - delete BD.Program; // Delete the hacked up program - BD.Program = OldProgram; // Restore the original - + bool Broken = TestMergedProgram(BD, Optimized, ToNotOptimize); std::cout << (Broken ? " nope.\n" : " yup.\n"); return Broken; } @@ -220,8 +224,8 @@ bool BugDriver::debugMiscompilation() { } std::cout << "\n*** Found miscompiling pass" - << (PassesToRun.size() == 1 ? "" : "es") << ": " - << getPassesString(PassesToRun) << "\n"; + << (getPassesToRun().size() == 1 ? "" : "es") << ": " + << getPassesString(getPassesToRun()) << "\n"; EmitProgressBytecode("passinput"); // Okay, now that we have reduced the list of passes which are causing the @@ -244,9 +248,9 @@ bool BugDriver::debugMiscompilation() { // Output a bunch of bytecode files for the user... std::cout << "Outputting reduced bytecode files which expose the problem:\n"; - Module *ToOptimize = CloneModule(getProgram()); - Module *ToNotOptimize = SplitFunctionsOutOfModule(ToOptimize, - MiscompiledFunctions); + Module *ToNotOptimize = CloneModule(getProgram()); + Module *ToOptimize = SplitFunctionsOutOfModule(ToNotOptimize, + MiscompiledFunctions); std::cout << " Non-optimized portion: "; std::swap(Program, ToNotOptimize);