From bd6b2138b94a47a9226ec1e69bf3be55a47fd1ef Mon Sep 17 00:00:00 2001 From: Fedor Sergeev Date: Wed, 17 Oct 2018 10:36:23 +0000 Subject: [PATCH] [NewPM] teach -passes= to emit meaningful error messages All the PassBuilder::parse interfaces now return descriptive StringError instead of a plain bool. It allows to make -passes/aa-pipeline parsing errors context-specific and thus less confusing. TODO: ideally we should also make suggestions for misspelled pass names, but that requires some extensions to PassBuilder. Reviewed By: philip.pfaffe, chandlerc Differential Revision: https://reviews.llvm.org/D53246 llvm-svn: 344685 --- lld/test/ELF/lto/ltopasses-custom.ll | 4 +- llvm/include/llvm/Passes/PassBuilder.h | 51 ++-- llvm/lib/LTO/LTOBackend.cpp | 14 +- llvm/lib/Passes/PassBuilder.cpp | 366 ++++++++++++++---------- llvm/test/Other/pass-pipeline-parsing.ll | 83 ++++-- llvm/test/tools/llvm-lto2/X86/pipeline.ll | 4 +- llvm/test/tools/llvm-opt-fuzzer/command-line.ll | 2 +- llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp | 11 +- llvm/tools/opt/NewPMDriver.cpp | 90 +++--- llvm/unittests/IR/CMakeLists.txt | 2 + llvm/unittests/IR/PassBuilderCallbacksTest.cpp | 37 +-- llvm/unittests/Passes/CMakeLists.txt | 1 + llvm/unittests/Passes/PluginsTest.cpp | 5 +- 13 files changed, 400 insertions(+), 270 deletions(-) diff --git a/lld/test/ELF/lto/ltopasses-custom.ll b/lld/test/ELF/lto/ltopasses-custom.ll index a75000d..23f1564 100644 --- a/lld/test/ELF/lto/ltopasses-custom.ll +++ b/lld/test/ELF/lto/ltopasses-custom.ll @@ -27,11 +27,11 @@ define void @barrier() { ; RUN: not ld.lld -m elf_x86_64 %t.o -o %t2.so \ ; RUN: --lto-newpm-passes=iamnotapass -shared 2>&1 | \ ; RUN: FileCheck %s --check-prefix=INVALID -; INVALID: unable to parse pass pipeline description: iamnotapass +; INVALID: unable to parse pass pipeline description 'iamnotapass': unknown pass name 'iamnotapass' ; Check that invalid AA pipelines are rejected gracefully. ; RUN: not ld.lld -m elf_x86_64 %t.o -o %t2.so \ ; RUN: --lto-newpm-passes=globaldce --lto-aa-pipeline=patatino \ ; RUN: -shared 2>&1 | \ ; RUN: FileCheck %s --check-prefix=INVALIDAA -; INVALIDAA: unable to parse AA pipeline description: patatino +; INVALIDAA: unknown alias analysis name 'patatino' diff --git a/llvm/include/llvm/Passes/PassBuilder.h b/llvm/include/llvm/Passes/PassBuilder.h index 9131443..22e5eb0 100644 --- a/llvm/include/llvm/Passes/PassBuilder.h +++ b/llvm/include/llvm/Passes/PassBuilder.h @@ -19,6 +19,7 @@ #include "llvm/ADT/Optional.h" #include "llvm/Analysis/CGSCCPassManager.h" #include "llvm/IR/PassManager.h" +#include "llvm/Support/Error.h" #include "llvm/Transforms/Instrumentation.h" #include "llvm/Transforms/Scalar/LoopPassManager.h" #include @@ -384,8 +385,9 @@ public: /// If the sequence of passes aren't all the exact same kind of pass, it will /// be an error. You cannot mix different levels implicitly, you must /// explicitly form a pass manager in which to nest passes. - bool parsePassPipeline(ModulePassManager &MPM, StringRef PipelineText, - bool VerifyEachPass = true, bool DebugLogging = false); + Error parsePassPipeline(ModulePassManager &MPM, StringRef PipelineText, + bool VerifyEachPass = true, + bool DebugLogging = false); /// {{@ Parse a textual pass pipeline description into a specific PassManager /// @@ -394,12 +396,15 @@ public: /// this is the valid pipeline text: /// /// function(lpass) - bool parsePassPipeline(CGSCCPassManager &CGPM, StringRef PipelineText, - bool VerifyEachPass = true, bool DebugLogging = false); - bool parsePassPipeline(FunctionPassManager &FPM, StringRef PipelineText, - bool VerifyEachPass = true, bool DebugLogging = false); - bool parsePassPipeline(LoopPassManager &LPM, StringRef PipelineText, - bool VerifyEachPass = true, bool DebugLogging = false); + Error parsePassPipeline(CGSCCPassManager &CGPM, StringRef PipelineText, + bool VerifyEachPass = true, + bool DebugLogging = false); + Error parsePassPipeline(FunctionPassManager &FPM, StringRef PipelineText, + bool VerifyEachPass = true, + bool DebugLogging = false); + Error parsePassPipeline(LoopPassManager &LPM, StringRef PipelineText, + bool VerifyEachPass = true, + bool DebugLogging = false); /// @}} /// Parse a textual alias analysis pipeline into the provided AA manager. @@ -417,7 +422,7 @@ public: /// Returns false if the text cannot be parsed cleanly. The specific state of /// the \p AA manager is unspecified if such an error is encountered and this /// returns false. - bool parseAAPipeline(AAManager &AA, StringRef PipelineText); + Error parseAAPipeline(AAManager &AA, StringRef PipelineText); /// Register a callback for a default optimizer pipeline extension /// point @@ -565,28 +570,28 @@ private: static Optional> parsePipelineText(StringRef Text); - bool parseModulePass(ModulePassManager &MPM, const PipelineElement &E, + Error parseModulePass(ModulePassManager &MPM, const PipelineElement &E, + bool VerifyEachPass, bool DebugLogging); + Error parseCGSCCPass(CGSCCPassManager &CGPM, const PipelineElement &E, bool VerifyEachPass, bool DebugLogging); - bool parseCGSCCPass(CGSCCPassManager &CGPM, const PipelineElement &E, + Error parseFunctionPass(FunctionPassManager &FPM, const PipelineElement &E, + bool VerifyEachPass, bool DebugLogging); + Error parseLoopPass(LoopPassManager &LPM, const PipelineElement &E, bool VerifyEachPass, bool DebugLogging); - bool parseFunctionPass(FunctionPassManager &FPM, const PipelineElement &E, - bool VerifyEachPass, bool DebugLogging); - bool parseLoopPass(LoopPassManager &LPM, const PipelineElement &E, - bool VerifyEachPass, bool DebugLogging); bool parseAAPassName(AAManager &AA, StringRef Name); - bool parseLoopPassPipeline(LoopPassManager &LPM, - ArrayRef Pipeline, - bool VerifyEachPass, bool DebugLogging); - bool parseFunctionPassPipeline(FunctionPassManager &FPM, - ArrayRef Pipeline, - bool VerifyEachPass, bool DebugLogging); - bool parseCGSCCPassPipeline(CGSCCPassManager &CGPM, + Error parseLoopPassPipeline(LoopPassManager &LPM, ArrayRef Pipeline, bool VerifyEachPass, bool DebugLogging); - bool parseModulePassPipeline(ModulePassManager &MPM, + Error parseFunctionPassPipeline(FunctionPassManager &FPM, + ArrayRef Pipeline, + bool VerifyEachPass, bool DebugLogging); + Error parseCGSCCPassPipeline(CGSCCPassManager &CGPM, ArrayRef Pipeline, bool VerifyEachPass, bool DebugLogging); + Error parseModulePassPipeline(ModulePassManager &MPM, + ArrayRef Pipeline, + bool VerifyEachPass, bool DebugLogging); void addPGOInstrPasses(ModulePassManager &MPM, bool DebugLogging, OptimizationLevel Level, bool RunProfileGen, diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp index 20fc40d..1f9d60a 100644 --- a/llvm/lib/LTO/LTOBackend.cpp +++ b/llvm/lib/LTO/LTOBackend.cpp @@ -162,7 +162,7 @@ static void runNewPMPasses(Config &Conf, Module &Mod, TargetMachine *TM, AAManager AA; // Parse a custom AA pipeline if asked to. - if (!PB.parseAAPipeline(AA, "default")) + if (auto Err = PB.parseAAPipeline(AA, "default")) report_fatal_error("Error parsing default AA pipeline"); LoopAnalysisManager LAM(Conf.DebugPassManager); @@ -221,9 +221,9 @@ static void runNewPMCustomPasses(Module &Mod, TargetMachine *TM, // Parse a custom AA pipeline if asked to. if (!AAPipelineDesc.empty()) - if (!PB.parseAAPipeline(AA, AAPipelineDesc)) - report_fatal_error("unable to parse AA pipeline description: " + - AAPipelineDesc); + if (auto Err = PB.parseAAPipeline(AA, AAPipelineDesc)) + report_fatal_error("unable to parse AA pipeline description '" + + AAPipelineDesc + "': " + toString(std::move(Err))); LoopAnalysisManager LAM; FunctionAnalysisManager FAM; @@ -246,9 +246,9 @@ static void runNewPMCustomPasses(Module &Mod, TargetMachine *TM, MPM.addPass(VerifierPass()); // Now, add all the passes we've been requested to. - if (!PB.parsePassPipeline(MPM, PipelineDesc)) - report_fatal_error("unable to parse pass pipeline description: " + - PipelineDesc); + if (auto Err = PB.parsePassPipeline(MPM, PipelineDesc)) + report_fatal_error("unable to parse pass pipeline description '" + + PipelineDesc + "': " + toString(std::move(Err))); if (!DisableVerify) MPM.addPass(VerifierPass()); diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp index 09758dc..f6313d2 100644 --- a/llvm/lib/Passes/PassBuilder.cpp +++ b/llvm/lib/Passes/PassBuilder.cpp @@ -58,6 +58,7 @@ #include "llvm/IR/PassManager.h" #include "llvm/IR/Verifier.h" #include "llvm/Support/Debug.h" +#include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Regex.h" #include "llvm/Target/TargetMachine.h" #include "llvm/Transforms/AggressiveInstCombine/AggressiveInstCombine.h" @@ -1402,9 +1403,9 @@ PassBuilder::parsePipelineText(StringRef Text) { return {std::move(ResultPipeline)}; } -bool PassBuilder::parseModulePass(ModulePassManager &MPM, - const PipelineElement &E, bool VerifyEachPass, - bool DebugLogging) { +Error PassBuilder::parseModulePass(ModulePassManager &MPM, + const PipelineElement &E, + bool VerifyEachPass, bool DebugLogging) { auto &Name = E.Name; auto &InnerPipeline = E.InnerPipeline; @@ -1412,50 +1413,56 @@ bool PassBuilder::parseModulePass(ModulePassManager &MPM, if (!InnerPipeline.empty()) { if (Name == "module") { ModulePassManager NestedMPM(DebugLogging); - if (!parseModulePassPipeline(NestedMPM, InnerPipeline, VerifyEachPass, - DebugLogging)) - return false; + if (auto Err = parseModulePassPipeline(NestedMPM, InnerPipeline, + VerifyEachPass, DebugLogging)) + return Err; MPM.addPass(std::move(NestedMPM)); - return true; + return Error::success(); } if (Name == "cgscc") { CGSCCPassManager CGPM(DebugLogging); - if (!parseCGSCCPassPipeline(CGPM, InnerPipeline, VerifyEachPass, - DebugLogging)) - return false; + if (auto Err = parseCGSCCPassPipeline(CGPM, InnerPipeline, VerifyEachPass, + DebugLogging)) + return Err; MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM))); - return true; + return Error::success(); } if (Name == "function") { FunctionPassManager FPM(DebugLogging); - if (!parseFunctionPassPipeline(FPM, InnerPipeline, VerifyEachPass, - DebugLogging)) - return false; + if (auto Err = parseFunctionPassPipeline(FPM, InnerPipeline, + VerifyEachPass, DebugLogging)) + return Err; MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM))); - return true; + return Error::success(); } if (auto Count = parseRepeatPassName(Name)) { ModulePassManager NestedMPM(DebugLogging); - if (!parseModulePassPipeline(NestedMPM, InnerPipeline, VerifyEachPass, - DebugLogging)) - return false; + if (auto Err = parseModulePassPipeline(NestedMPM, InnerPipeline, + VerifyEachPass, DebugLogging)) + return Err; MPM.addPass(createRepeatedPass(*Count, std::move(NestedMPM))); - return true; + return Error::success(); } for (auto &C : ModulePipelineParsingCallbacks) if (C(Name, MPM, InnerPipeline)) - return true; + return Error::success(); // Normal passes can't have pipelines. - return false; + return make_error( + formatv("invalid use of '{0}' pass as module pipeline", Name).str(), + inconvertibleErrorCode()); + ; } // Manually handle aliases for pre-configured pipeline fragments. if (startsWithDefaultPipelineAliasPrefix(Name)) { SmallVector Matches; if (!DefaultAliasRegex.match(Name, &Matches)) - return false; + return make_error( + formatv("unknown default pipeline alias '{0}'", Name).str(), + inconvertibleErrorCode()); + assert(Matches.size() == 3 && "Must capture two matched strings!"); OptimizationLevel L = StringSwitch(Matches[2]) @@ -1467,7 +1474,7 @@ bool PassBuilder::parseModulePass(ModulePassManager &MPM, .Case("Oz", Oz); if (L == O0) // At O0 we do nothing at all! - return true; + return Error::success(); if (Matches[1] == "default") { MPM.addPass(buildPerModuleDefaultPipeline(L, DebugLogging)); @@ -1481,38 +1488,40 @@ bool PassBuilder::parseModulePass(ModulePassManager &MPM, assert(Matches[1] == "lto" && "Not one of the matched options!"); MPM.addPass(buildLTODefaultPipeline(L, DebugLogging, nullptr)); } - return true; + return Error::success(); } // Finally expand the basic registered passes from the .inc file. #define MODULE_PASS(NAME, CREATE_PASS) \ if (Name == NAME) { \ MPM.addPass(CREATE_PASS); \ - return true; \ + return Error::success(); \ } #define MODULE_ANALYSIS(NAME, CREATE_PASS) \ if (Name == "require<" NAME ">") { \ MPM.addPass( \ RequireAnalysisPass< \ std::remove_reference::type, Module>()); \ - return true; \ + return Error::success(); \ } \ if (Name == "invalidate<" NAME ">") { \ MPM.addPass(InvalidateAnalysisPass< \ std::remove_reference::type>()); \ - return true; \ + return Error::success(); \ } #include "PassRegistry.def" for (auto &C : ModulePipelineParsingCallbacks) if (C(Name, MPM, InnerPipeline)) - return true; - return false; + return Error::success(); + return make_error( + formatv("unknown module pass '{0}'", Name).str(), + inconvertibleErrorCode()); } -bool PassBuilder::parseCGSCCPass(CGSCCPassManager &CGPM, - const PipelineElement &E, bool VerifyEachPass, - bool DebugLogging) { +Error PassBuilder::parseCGSCCPass(CGSCCPassManager &CGPM, + const PipelineElement &E, bool VerifyEachPass, + bool DebugLogging) { auto &Name = E.Name; auto &InnerPipeline = E.InnerPipeline; @@ -1520,53 +1529,55 @@ bool PassBuilder::parseCGSCCPass(CGSCCPassManager &CGPM, if (!InnerPipeline.empty()) { if (Name == "cgscc") { CGSCCPassManager NestedCGPM(DebugLogging); - if (!parseCGSCCPassPipeline(NestedCGPM, InnerPipeline, VerifyEachPass, - DebugLogging)) - return false; + if (auto Err = parseCGSCCPassPipeline(NestedCGPM, InnerPipeline, + VerifyEachPass, DebugLogging)) + return Err; // Add the nested pass manager with the appropriate adaptor. CGPM.addPass(std::move(NestedCGPM)); - return true; + return Error::success(); } if (Name == "function") { FunctionPassManager FPM(DebugLogging); - if (!parseFunctionPassPipeline(FPM, InnerPipeline, VerifyEachPass, - DebugLogging)) - return false; + if (auto Err = parseFunctionPassPipeline(FPM, InnerPipeline, + VerifyEachPass, DebugLogging)) + return Err; // Add the nested pass manager with the appropriate adaptor. CGPM.addPass(createCGSCCToFunctionPassAdaptor(std::move(FPM))); - return true; + return Error::success(); } if (auto Count = parseRepeatPassName(Name)) { CGSCCPassManager NestedCGPM(DebugLogging); - if (!parseCGSCCPassPipeline(NestedCGPM, InnerPipeline, VerifyEachPass, - DebugLogging)) - return false; + if (auto Err = parseCGSCCPassPipeline(NestedCGPM, InnerPipeline, + VerifyEachPass, DebugLogging)) + return Err; CGPM.addPass(createRepeatedPass(*Count, std::move(NestedCGPM))); - return true; + return Error::success(); } if (auto MaxRepetitions = parseDevirtPassName(Name)) { CGSCCPassManager NestedCGPM(DebugLogging); - if (!parseCGSCCPassPipeline(NestedCGPM, InnerPipeline, VerifyEachPass, - DebugLogging)) - return false; + if (auto Err = parseCGSCCPassPipeline(NestedCGPM, InnerPipeline, + VerifyEachPass, DebugLogging)) + return Err; CGPM.addPass( createDevirtSCCRepeatedPass(std::move(NestedCGPM), *MaxRepetitions)); - return true; + return Error::success(); } for (auto &C : CGSCCPipelineParsingCallbacks) if (C(Name, CGPM, InnerPipeline)) - return true; + return Error::success(); // Normal passes can't have pipelines. - return false; + return make_error( + formatv("invalid use of '{0}' pass as cgscc pipeline", Name).str(), + inconvertibleErrorCode()); } // Now expand the basic registered passes from the .inc file. #define CGSCC_PASS(NAME, CREATE_PASS) \ if (Name == NAME) { \ CGPM.addPass(CREATE_PASS); \ - return true; \ + return Error::success(); \ } #define CGSCC_ANALYSIS(NAME, CREATE_PASS) \ if (Name == "require<" NAME ">") { \ @@ -1574,24 +1585,26 @@ bool PassBuilder::parseCGSCCPass(CGSCCPassManager &CGPM, std::remove_reference::type, \ LazyCallGraph::SCC, CGSCCAnalysisManager, LazyCallGraph &, \ CGSCCUpdateResult &>()); \ - return true; \ + return Error::success(); \ } \ if (Name == "invalidate<" NAME ">") { \ CGPM.addPass(InvalidateAnalysisPass< \ std::remove_reference::type>()); \ - return true; \ + return Error::success(); \ } #include "PassRegistry.def" for (auto &C : CGSCCPipelineParsingCallbacks) if (C(Name, CGPM, InnerPipeline)) - return true; - return false; + return Error::success(); + return make_error( + formatv("unknown cgscc pass '{0}'", Name).str(), + inconvertibleErrorCode()); } -bool PassBuilder::parseFunctionPass(FunctionPassManager &FPM, - const PipelineElement &E, - bool VerifyEachPass, bool DebugLogging) { +Error PassBuilder::parseFunctionPass(FunctionPassManager &FPM, + const PipelineElement &E, + bool VerifyEachPass, bool DebugLogging) { auto &Name = E.Name; auto &InnerPipeline = E.InnerPipeline; @@ -1599,68 +1612,72 @@ bool PassBuilder::parseFunctionPass(FunctionPassManager &FPM, if (!InnerPipeline.empty()) { if (Name == "function") { FunctionPassManager NestedFPM(DebugLogging); - if (!parseFunctionPassPipeline(NestedFPM, InnerPipeline, VerifyEachPass, - DebugLogging)) - return false; + if (auto Err = parseFunctionPassPipeline(NestedFPM, InnerPipeline, + VerifyEachPass, DebugLogging)) + return Err; // Add the nested pass manager with the appropriate adaptor. FPM.addPass(std::move(NestedFPM)); - return true; + return Error::success(); } if (Name == "loop") { LoopPassManager LPM(DebugLogging); - if (!parseLoopPassPipeline(LPM, InnerPipeline, VerifyEachPass, - DebugLogging)) - return false; + if (auto Err = parseLoopPassPipeline(LPM, InnerPipeline, VerifyEachPass, + DebugLogging)) + return Err; // Add the nested pass manager with the appropriate adaptor. FPM.addPass( createFunctionToLoopPassAdaptor(std::move(LPM), DebugLogging)); - return true; + return Error::success(); } if (auto Count = parseRepeatPassName(Name)) { FunctionPassManager NestedFPM(DebugLogging); - if (!parseFunctionPassPipeline(NestedFPM, InnerPipeline, VerifyEachPass, - DebugLogging)) - return false; + if (auto Err = parseFunctionPassPipeline(NestedFPM, InnerPipeline, + VerifyEachPass, DebugLogging)) + return Err; FPM.addPass(createRepeatedPass(*Count, std::move(NestedFPM))); - return true; + return Error::success(); } for (auto &C : FunctionPipelineParsingCallbacks) if (C(Name, FPM, InnerPipeline)) - return true; + return Error::success(); // Normal passes can't have pipelines. - return false; + return make_error( + formatv("invalid use of '{0}' pass as function pipeline", Name).str(), + inconvertibleErrorCode()); } // Now expand the basic registered passes from the .inc file. #define FUNCTION_PASS(NAME, CREATE_PASS) \ if (Name == NAME) { \ FPM.addPass(CREATE_PASS); \ - return true; \ + return Error::success(); \ } #define FUNCTION_ANALYSIS(NAME, CREATE_PASS) \ if (Name == "require<" NAME ">") { \ FPM.addPass( \ RequireAnalysisPass< \ std::remove_reference::type, Function>()); \ - return true; \ + return Error::success(); \ } \ if (Name == "invalidate<" NAME ">") { \ FPM.addPass(InvalidateAnalysisPass< \ std::remove_reference::type>()); \ - return true; \ + return Error::success(); \ } #include "PassRegistry.def" for (auto &C : FunctionPipelineParsingCallbacks) if (C(Name, FPM, InnerPipeline)) - return true; - return false; + return Error::success(); + return make_error( + formatv("unknown function pass '{0}'", Name).str(), + inconvertibleErrorCode()); } -bool PassBuilder::parseLoopPass(LoopPassManager &LPM, const PipelineElement &E, - bool VerifyEachPass, bool DebugLogging) { +Error PassBuilder::parseLoopPass(LoopPassManager &LPM, const PipelineElement &E, + bool VerifyEachPass, bool DebugLogging) { StringRef Name = E.Name; auto &InnerPipeline = E.InnerPipeline; @@ -1668,35 +1685,37 @@ bool PassBuilder::parseLoopPass(LoopPassManager &LPM, const PipelineElement &E, if (!InnerPipeline.empty()) { if (Name == "loop") { LoopPassManager NestedLPM(DebugLogging); - if (!parseLoopPassPipeline(NestedLPM, InnerPipeline, VerifyEachPass, - DebugLogging)) - return false; + if (auto Err = parseLoopPassPipeline(NestedLPM, InnerPipeline, + VerifyEachPass, DebugLogging)) + return Err; // Add the nested pass manager with the appropriate adaptor. LPM.addPass(std::move(NestedLPM)); - return true; + return Error::success(); } if (auto Count = parseRepeatPassName(Name)) { LoopPassManager NestedLPM(DebugLogging); - if (!parseLoopPassPipeline(NestedLPM, InnerPipeline, VerifyEachPass, - DebugLogging)) - return false; + if (auto Err = parseLoopPassPipeline(NestedLPM, InnerPipeline, + VerifyEachPass, DebugLogging)) + return Err; LPM.addPass(createRepeatedPass(*Count, std::move(NestedLPM))); - return true; + return Error::success(); } for (auto &C : LoopPipelineParsingCallbacks) if (C(Name, LPM, InnerPipeline)) - return true; + return Error::success(); // Normal passes can't have pipelines. - return false; + return make_error( + formatv("invalid use of '{0}' pass as loop pipeline", Name).str(), + inconvertibleErrorCode()); } // Now expand the basic registered passes from the .inc file. #define LOOP_PASS(NAME, CREATE_PASS) \ if (Name == NAME) { \ LPM.addPass(CREATE_PASS); \ - return true; \ + return Error::success(); \ } #define LOOP_ANALYSIS(NAME, CREATE_PASS) \ if (Name == "require<" NAME ">") { \ @@ -1704,19 +1723,20 @@ bool PassBuilder::parseLoopPass(LoopPassManager &LPM, const PipelineElement &E, std::remove_reference::type, Loop, \ LoopAnalysisManager, LoopStandardAnalysisResults &, \ LPMUpdater &>()); \ - return true; \ + return Error::success(); \ } \ if (Name == "invalidate<" NAME ">") { \ LPM.addPass(InvalidateAnalysisPass< \ std::remove_reference::type>()); \ - return true; \ + return Error::success(); \ } #include "PassRegistry.def" for (auto &C : LoopPipelineParsingCallbacks) if (C(Name, LPM, InnerPipeline)) - return true; - return false; + return Error::success(); + return make_error(formatv("unknown loop pass '{0}'", Name).str(), + inconvertibleErrorCode()); } bool PassBuilder::parseAAPassName(AAManager &AA, StringRef Name) { @@ -1740,41 +1760,42 @@ bool PassBuilder::parseAAPassName(AAManager &AA, StringRef Name) { return false; } -bool PassBuilder::parseLoopPassPipeline(LoopPassManager &LPM, - ArrayRef Pipeline, - bool VerifyEachPass, - bool DebugLogging) { +Error PassBuilder::parseLoopPassPipeline(LoopPassManager &LPM, + ArrayRef Pipeline, + bool VerifyEachPass, + bool DebugLogging) { for (const auto &Element : Pipeline) { - if (!parseLoopPass(LPM, Element, VerifyEachPass, DebugLogging)) - return false; + if (auto Err = parseLoopPass(LPM, Element, VerifyEachPass, DebugLogging)) + return Err; // FIXME: No verifier support for Loop passes! } - return true; + return Error::success(); } -bool PassBuilder::parseFunctionPassPipeline(FunctionPassManager &FPM, - ArrayRef Pipeline, - bool VerifyEachPass, - bool DebugLogging) { +Error PassBuilder::parseFunctionPassPipeline(FunctionPassManager &FPM, + ArrayRef Pipeline, + bool VerifyEachPass, + bool DebugLogging) { for (const auto &Element : Pipeline) { - if (!parseFunctionPass(FPM, Element, VerifyEachPass, DebugLogging)) - return false; + if (auto Err = + parseFunctionPass(FPM, Element, VerifyEachPass, DebugLogging)) + return Err; if (VerifyEachPass) FPM.addPass(VerifierPass()); } - return true; + return Error::success(); } -bool PassBuilder::parseCGSCCPassPipeline(CGSCCPassManager &CGPM, - ArrayRef Pipeline, - bool VerifyEachPass, - bool DebugLogging) { +Error PassBuilder::parseCGSCCPassPipeline(CGSCCPassManager &CGPM, + ArrayRef Pipeline, + bool VerifyEachPass, + bool DebugLogging) { for (const auto &Element : Pipeline) { - if (!parseCGSCCPass(CGPM, Element, VerifyEachPass, DebugLogging)) - return false; + if (auto Err = parseCGSCCPass(CGPM, Element, VerifyEachPass, DebugLogging)) + return Err; // FIXME: No verifier support for CGSCC passes! } - return true; + return Error::success(); } void PassBuilder::crossRegisterProxies(LoopAnalysisManager &LAM, @@ -1790,28 +1811,30 @@ void PassBuilder::crossRegisterProxies(LoopAnalysisManager &LAM, LAM.registerPass([&] { return FunctionAnalysisManagerLoopProxy(FAM); }); } -bool PassBuilder::parseModulePassPipeline(ModulePassManager &MPM, - ArrayRef Pipeline, - bool VerifyEachPass, - bool DebugLogging) { +Error PassBuilder::parseModulePassPipeline(ModulePassManager &MPM, + ArrayRef Pipeline, + bool VerifyEachPass, + bool DebugLogging) { for (const auto &Element : Pipeline) { - if (!parseModulePass(MPM, Element, VerifyEachPass, DebugLogging)) - return false; + if (auto Err = parseModulePass(MPM, Element, VerifyEachPass, DebugLogging)) + return Err; if (VerifyEachPass) MPM.addPass(VerifierPass()); } - return true; + return Error::success(); } // Primary pass pipeline description parsing routine for a \c ModulePassManager // FIXME: Should this routine accept a TargetMachine or require the caller to // pre-populate the analysis managers with target-specific stuff? -bool PassBuilder::parsePassPipeline(ModulePassManager &MPM, - StringRef PipelineText, bool VerifyEachPass, - bool DebugLogging) { +Error PassBuilder::parsePassPipeline(ModulePassManager &MPM, + StringRef PipelineText, + bool VerifyEachPass, bool DebugLogging) { auto Pipeline = parsePipelineText(PipelineText); if (!Pipeline || Pipeline->empty()) - return false; + return make_error( + formatv("invalid pipeline '{0}'", PipelineText).str(), + inconvertibleErrorCode()); // If the first name isn't at the module layer, wrap the pipeline up // automatically. @@ -1828,73 +1851,106 @@ bool PassBuilder::parsePassPipeline(ModulePassManager &MPM, } else { for (auto &C : TopLevelPipelineParsingCallbacks) if (C(MPM, *Pipeline, VerifyEachPass, DebugLogging)) - return true; - - // Unknown pass name! - return false; + return Error::success(); + + // Unknown pass or pipeline name! + auto &InnerPipeline = Pipeline->front().InnerPipeline; + return make_error( + formatv("unknown {0} name '{1}'", + (InnerPipeline.empty() ? "pass" : "pipeline"), FirstName) + .str(), + inconvertibleErrorCode()); } } - return parseModulePassPipeline(MPM, *Pipeline, VerifyEachPass, DebugLogging); + if (auto Err = + parseModulePassPipeline(MPM, *Pipeline, VerifyEachPass, DebugLogging)) + return Err; + return Error::success(); } // Primary pass pipeline description parsing routine for a \c CGSCCPassManager -bool PassBuilder::parsePassPipeline(CGSCCPassManager &CGPM, - StringRef PipelineText, bool VerifyEachPass, - bool DebugLogging) { +Error PassBuilder::parsePassPipeline(CGSCCPassManager &CGPM, + StringRef PipelineText, + bool VerifyEachPass, bool DebugLogging) { auto Pipeline = parsePipelineText(PipelineText); if (!Pipeline || Pipeline->empty()) - return false; + return make_error( + formatv("invalid pipeline '{0}'", PipelineText).str(), + inconvertibleErrorCode()); StringRef FirstName = Pipeline->front().Name; if (!isCGSCCPassName(FirstName, CGSCCPipelineParsingCallbacks)) - return false; - - return parseCGSCCPassPipeline(CGPM, *Pipeline, VerifyEachPass, DebugLogging); + return make_error( + formatv("unknown cgscc pass '{0}' in pipeline '{1}'", FirstName, + PipelineText) + .str(), + inconvertibleErrorCode()); + + if (auto Err = + parseCGSCCPassPipeline(CGPM, *Pipeline, VerifyEachPass, DebugLogging)) + return Err; + return Error::success(); } // Primary pass pipeline description parsing routine for a \c // FunctionPassManager -bool PassBuilder::parsePassPipeline(FunctionPassManager &FPM, - StringRef PipelineText, bool VerifyEachPass, - bool DebugLogging) { +Error PassBuilder::parsePassPipeline(FunctionPassManager &FPM, + StringRef PipelineText, + bool VerifyEachPass, bool DebugLogging) { auto Pipeline = parsePipelineText(PipelineText); if (!Pipeline || Pipeline->empty()) - return false; + return make_error( + formatv("invalid pipeline '{0}'", PipelineText).str(), + inconvertibleErrorCode()); StringRef FirstName = Pipeline->front().Name; if (!isFunctionPassName(FirstName, FunctionPipelineParsingCallbacks)) - return false; - - return parseFunctionPassPipeline(FPM, *Pipeline, VerifyEachPass, - DebugLogging); + return make_error( + formatv("unknown function pass '{0}' in pipeline '{1}'", FirstName, + PipelineText) + .str(), + inconvertibleErrorCode()); + + if (auto Err = parseFunctionPassPipeline(FPM, *Pipeline, VerifyEachPass, + DebugLogging)) + return Err; + return Error::success(); } // Primary pass pipeline description parsing routine for a \c LoopPassManager -bool PassBuilder::parsePassPipeline(LoopPassManager &CGPM, - StringRef PipelineText, bool VerifyEachPass, - bool DebugLogging) { +Error PassBuilder::parsePassPipeline(LoopPassManager &CGPM, + StringRef PipelineText, + bool VerifyEachPass, bool DebugLogging) { auto Pipeline = parsePipelineText(PipelineText); if (!Pipeline || Pipeline->empty()) - return false; + return make_error( + formatv("invalid pipeline '{0}'", PipelineText).str(), + inconvertibleErrorCode()); - return parseLoopPassPipeline(CGPM, *Pipeline, VerifyEachPass, DebugLogging); + if (auto Err = + parseLoopPassPipeline(CGPM, *Pipeline, VerifyEachPass, DebugLogging)) + return Err; + + return Error::success(); } -bool PassBuilder::parseAAPipeline(AAManager &AA, StringRef PipelineText) { +Error PassBuilder::parseAAPipeline(AAManager &AA, StringRef PipelineText) { // If the pipeline just consists of the word 'default' just replace the AA // manager with our default one. if (PipelineText == "default") { AA = buildDefaultAAPipeline(); - return true; + return Error::success(); } while (!PipelineText.empty()) { StringRef Name; std::tie(Name, PipelineText) = PipelineText.split(','); if (!parseAAPassName(AA, Name)) - return false; + return make_error( + formatv("unknown alias analysis name '{0}'", Name).str(), + inconvertibleErrorCode()); } - return true; + return Error::success(); } diff --git a/llvm/test/Other/pass-pipeline-parsing.ll b/llvm/test/Other/pass-pipeline-parsing.ll index b303318..d26d000 100644 --- a/llvm/test/Other/pass-pipeline-parsing.ll +++ b/llvm/test/Other/pass-pipeline-parsing.ll @@ -54,52 +54,52 @@ ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='no-op-module)' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-UNBALANCED1 -; CHECK-UNBALANCED1: unable to parse pass pipeline description +; CHECK-UNBALANCED1: invalid pipeline 'no-op-module)' ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='module(no-op-module))' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-UNBALANCED2 -; CHECK-UNBALANCED2: unable to parse pass pipeline description +; CHECK-UNBALANCED2: invalid pipeline 'module(no-op-module))' ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='module(no-op-module' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-UNBALANCED3 -; CHECK-UNBALANCED3: unable to parse pass pipeline description +; CHECK-UNBALANCED3: invalid pipeline 'module(no-op-module' ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='no-op-function)' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-UNBALANCED4 -; CHECK-UNBALANCED4: unable to parse pass pipeline description +; CHECK-UNBALANCED4: invalid pipeline 'no-op-function)' ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='function(no-op-function))' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-UNBALANCED5 -; CHECK-UNBALANCED5: unable to parse pass pipeline description +; CHECK-UNBALANCED5: invalid pipeline 'function(no-op-function))' ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='function(function(no-op-function)))' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-UNBALANCED6 -; CHECK-UNBALANCED6: unable to parse pass pipeline description +; CHECK-UNBALANCED6: invalid pipeline 'function(function(no-op-function)))' ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='function(no-op-function' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-UNBALANCED7 -; CHECK-UNBALANCED7: unable to parse pass pipeline description +; CHECK-UNBALANCED7: invalid pipeline 'function(no-op-function' ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='function(function(no-op-function)' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-UNBALANCED8 -; CHECK-UNBALANCED8: unable to parse pass pipeline description +; CHECK-UNBALANCED8: invalid pipeline 'function(function(no-op-function)' ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='no-op-module,)' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-UNBALANCED9 -; CHECK-UNBALANCED9: unable to parse pass pipeline description +; CHECK-UNBALANCED9: invalid pipeline 'no-op-module,)' ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='no-op-function,)' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-UNBALANCED10 -; CHECK-UNBALANCED10: unable to parse pass pipeline description +; CHECK-UNBALANCED10: invalid pipeline 'no-op-function,)' ; RUN: opt -disable-output -debug-pass-manager \ ; RUN: -passes=no-op-cgscc,no-op-cgscc %s 2>&1 \ @@ -176,37 +176,86 @@ ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='function(no-op-function)function(no-op-function)' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-MISSING-COMMA1 -; CHECK-MISSING-COMMA1: unable to parse pass pipeline description +; CHECK-MISSING-COMMA1: invalid pipeline 'function(no-op-function)function(no-op-function)' ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='function()' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-EMPTY-INNER-PIPELINE -; CHECK-EMPTY-INNER-PIPELINE: unable to parse pass pipeline description +; CHECK-EMPTY-INNER-PIPELINE: unknown function pass '' ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='no-op-module(no-op-module,whatever)' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-PIPELINE-ON-MODULE-PASS -; CHECK-PIPELINE-ON-MODULE-PASS: unable to parse pass pipeline description +; CHECK-PIPELINE-ON-MODULE-PASS: invalid use of 'no-op-module' pass as module pipeline ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='no-op-cgscc(no-op-cgscc,whatever)' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-PIPELINE-ON-CGSCC-PASS -; CHECK-PIPELINE-ON-CGSCC-PASS: unable to parse pass pipeline description +; CHECK-PIPELINE-ON-CGSCC-PASS: invalid use of 'no-op-cgscc' pass as cgscc pipeline ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='no-op-function(no-op-function,whatever)' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-PIPELINE-ON-FUNCTION-PASS -; CHECK-PIPELINE-ON-FUNCTION-PASS: unable to parse pass pipeline description +; CHECK-PIPELINE-ON-FUNCTION-PASS: invalid use of 'no-op-function' pass as function pipeline ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='no-op-loop(no-op-loop,whatever)' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-PIPELINE-ON-LOOP-PASS -; CHECK-PIPELINE-ON-LOOP-PASS: unable to parse pass pipeline description +; CHECK-PIPELINE-ON-LOOP-PASS: invalid use of 'no-op-loop' pass as loop pipeline ; RUN: not opt -disable-output -debug-pass-manager \ ; RUN: -passes='no-op-function()' %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-EMPTY-PIPELINE-ON-PASS -; CHECK-EMPTY-PIPELINE-ON-PASS: unable to parse pass pipeline description +; CHECK-EMPTY-PIPELINE-ON-PASS: invalid use of 'no-op-function' pass as function pipeline + +; RUN: not opt -passes='no-op-module,bad' \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-UNKNOWN-MODULE +; CHECK-UNKNOWN-MODULE: opt: unknown module pass 'bad' + +; RUN: not opt -passes='no-op-loop,bad' \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-UNKNOWN-LOOP +; CHECK-UNKNOWN-LOOP: opt: unknown loop pass 'bad' + +; RUN: not opt -passes='no-op-cgscc,bad' \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-UNKNOWN-CGSCC +; CHECK-UNKNOWN-CGSCC: opt: unknown cgscc pass 'bad' + +; RUN: not opt -passes='no-op-function,bad' \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-UNKNOWN-FUNCTION +; RUN: not opt -passes='function(bad,pipeline,text)' \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-UNKNOWN-FUNCTION +; RUN: not opt -passes='module(no-op-module,function(bad,pipeline,text))' \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-UNKNOWN-FUNCTION +; RUN: not opt -passes='no-op-module,function(bad,pipeline,text)' \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-UNKNOWN-FUNCTION +; RUN: not opt -passes='module(cgscc(function(bad,pipeline,text)))' \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-UNKNOWN-FUNCTION +; CHECK-UNKNOWN-FUNCTION: opt: unknown function pass 'bad' + +; RUN: not opt -aa-pipeline=bad -passes=no-op-function \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=AA-PIPELINE-ERR +; AA-PIPELINE-ERR: unknown alias analysis name 'bad' +; RUN: opt -passes-ep-peephole=bad -passes=no-op-function \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=PASSES-EP-PEEPHOLE-ERR +; PASSES-EP-PEEPHOLE-ERR: Could not parse -passes-ep-peephole pipeline: unknown function pass 'bad' +; RUN: opt -passes-ep-late-loop-optimizations=bad -passes=no-op-function \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=PASSES-EP-LATELOOPOPT-ERR +; PASSES-EP-LATELOOPOPT-ERR: Could not parse -passes-ep-late-loop-optimizations pipeline: unknown loop pass 'bad' +; RUN: opt -passes-ep-loop-optimizer-end=bad -passes=no-op-function \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=PASSES-EP-LOOPOPTEND-ERR +; PASSES-EP-LOOPOPTEND-ERR: Could not parse -passes-ep-loop-optimizer-end pipeline: unknown loop pass 'bad' +; RUN: opt -passes-ep-scalar-optimizer-late=bad -passes=no-op-function \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=PASSES-EP-SCALAROPTLATE-ERR +; PASSES-EP-SCALAROPTLATE-ERR: Could not parse -passes-ep-scalar-optimizer-late pipeline: unknown function pass 'bad' +; RUN: opt -passes-ep-cgscc-optimizer-late=bad -passes=no-op-function \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=PASSES-EP-CGSCCOPTLATE-ERR +; PASSES-EP-CGSCCOPTLATE-ERR: Could not parse -passes-ep-cgscc-optimizer-late pipeline: unknown cgscc pass 'bad' +; RUN: opt -passes-ep-vectorizer-start=bad -passes=no-op-function \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=PASSES-EP-VECTORIZERSTART-ERR +; PASSES-EP-VECTORIZERSTART-ERR: Could not parse -passes-ep-vectorizer-start pipeline: unknown function pass 'bad' +; RUN: opt -passes-ep-pipeline-start=bad -passes=no-op-function \ +; RUN: /dev/null -disable-output 2>&1 | FileCheck %s -check-prefix=PASSES-EP-PIPELINESTART-ERR +; PASSES-EP-PIPELINESTART-ERR: Could not parse -passes-ep-pipeline-start pipeline: unknown pass name 'bad' define void @f() { entry: diff --git a/llvm/test/tools/llvm-lto2/X86/pipeline.ll b/llvm/test/tools/llvm-lto2/X86/pipeline.ll index 29276d8..9ab81ac 100644 --- a/llvm/test/tools/llvm-lto2/X86/pipeline.ll +++ b/llvm/test/tools/llvm-lto2/X86/pipeline.ll @@ -32,11 +32,11 @@ define void @patatino() { ; RUN: -r %t1.bc,patatino,px -opt-pipeline foogoo 2>&1 | \ ; RUN: FileCheck %s --check-prefix=ERR -; ERR: LLVM ERROR: unable to parse pass pipeline description: foogoo +; ERR: LLVM ERROR: unable to parse pass pipeline description 'foogoo': unknown pass name 'foogoo' ; RUN: not llvm-lto2 run %t1.bc -o %t.o \ ; RUN: -r %t1.bc,patatino,px -aa-pipeline patatino \ ; RUN: -opt-pipeline loweratomic 2>&1 | \ ; RUN: FileCheck %s --check-prefix=AAERR -; AAERR: LLVM ERROR: unable to parse AA pipeline description: patatino +; AAERR: LLVM ERROR: unable to parse AA pipeline description 'patatino': unknown alias analysis name 'patatino' diff --git a/llvm/test/tools/llvm-opt-fuzzer/command-line.ll b/llvm/test/tools/llvm-opt-fuzzer/command-line.ll index f747bba..8c3f6b6 100644 --- a/llvm/test/tools/llvm-opt-fuzzer/command-line.ll +++ b/llvm/test/tools/llvm-opt-fuzzer/command-line.ll @@ -13,7 +13,7 @@ ; Don't start with incorrect passes specified ; RUN: not llvm-opt-fuzzer %t -ignore_remaining_args=1 -mtriple x86_64 -passes no-pass 2>&1 | FileCheck -check-prefix=PIPELINE %s -; PIPELINE: can't parse pass pipeline +; PIPELINE: unknown pass name 'no-pass' ; Correct command line ; RUN: llvm-opt-fuzzer %t -ignore_remaining_args=1 -mtriple x86_64 -passes instcombine 2>&1 | FileCheck -check-prefix=CORRECT %s diff --git a/llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp b/llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp index 98d5428..57e75b1 100644 --- a/llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp +++ b/llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp @@ -144,9 +144,10 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { PB.registerLoopAnalyses(LAM); PB.crossRegisterProxies(LAM, FAM, CGAM, MAM); - bool Ok = PB.parsePassPipeline(MPM, PassPipeline, false, false); - assert(Ok && "Should have been checked during fuzzer initialization"); - (void)Ok; // silence unused variable warning on release builds + auto Err = PB.parsePassPipeline(MPM, PassPipeline, false, false); + assert(!Err && "Should have been checked during fuzzer initialization"); + // Only fail with assert above, otherwise ignore the parsing error. + consumeError(std::move(Err)); // Run passes which we need to test // @@ -235,8 +236,8 @@ extern "C" LLVM_ATTRIBUTE_USED int LLVMFuzzerInitialize( PassBuilder PB(TM.get()); ModulePassManager MPM; - if (!PB.parsePassPipeline(MPM, PassPipeline, false, false)) { - errs() << *argv[0] << ": can't parse pass pipeline\n"; + if (auto Err = PB.parsePassPipeline(MPM, PassPipeline, false, false)) { + errs() << *argv[0] << ": " << toString(std::move(Err)) << "\n"; exit(1); } diff --git a/llvm/tools/opt/NewPMDriver.cpp b/llvm/tools/opt/NewPMDriver.cpp index e63547a..e2f9a06 100644 --- a/llvm/tools/opt/NewPMDriver.cpp +++ b/llvm/tools/opt/NewPMDriver.cpp @@ -118,18 +118,20 @@ static cl::opt DebugInfoForProfiling( /// @}} template -bool tryParsePipelineText(PassBuilder &PB, StringRef PipelineText) { - if (PipelineText.empty()) +bool tryParsePipelineText(PassBuilder &PB, + const cl::opt &PipelineOpt) { + if (PipelineOpt.empty()) return false; // Verify the pipeline is parseable: PassManagerT PM; - if (PB.parsePassPipeline(PM, PipelineText)) - return true; - - errs() << "Could not parse pipeline '" << PipelineText - << "'. I'm going to igore it.\n"; - return false; + if (auto Err = PB.parsePassPipeline(PM, PipelineOpt)) { + errs() << "Could not parse -" << PipelineOpt.ArgStr + << " pipeline: " << toString(std::move(Err)) + << "... I'm going to ignore it.\n"; + return false; + } + return true; } /// If one of the EPPipeline command line options was given, register callbacks @@ -137,50 +139,61 @@ bool tryParsePipelineText(PassBuilder &PB, StringRef PipelineText) { static void registerEPCallbacks(PassBuilder &PB, bool VerifyEachPass, bool DebugLogging) { if (tryParsePipelineText(PB, PeepholeEPPipeline)) - PB.registerPeepholeEPCallback([&PB, VerifyEachPass, DebugLogging]( - FunctionPassManager &PM, PassBuilder::OptimizationLevel Level) { - PB.parsePassPipeline(PM, PeepholeEPPipeline, VerifyEachPass, - DebugLogging); - }); + PB.registerPeepholeEPCallback( + [&PB, VerifyEachPass, DebugLogging]( + FunctionPassManager &PM, PassBuilder::OptimizationLevel Level) { + ExitOnError Err("Unable to parse PeepholeEP pipeline: "); + Err(PB.parsePassPipeline(PM, PeepholeEPPipeline, VerifyEachPass, + DebugLogging)); + }); if (tryParsePipelineText(PB, LateLoopOptimizationsEPPipeline)) PB.registerLateLoopOptimizationsEPCallback( [&PB, VerifyEachPass, DebugLogging]( LoopPassManager &PM, PassBuilder::OptimizationLevel Level) { - PB.parsePassPipeline(PM, LateLoopOptimizationsEPPipeline, - VerifyEachPass, DebugLogging); + ExitOnError Err("Unable to parse LateLoopOptimizationsEP pipeline: "); + Err(PB.parsePassPipeline(PM, LateLoopOptimizationsEPPipeline, + VerifyEachPass, DebugLogging)); }); if (tryParsePipelineText(PB, LoopOptimizerEndEPPipeline)) - PB.registerLoopOptimizerEndEPCallback([&PB, VerifyEachPass, DebugLogging]( - LoopPassManager &PM, PassBuilder::OptimizationLevel Level) { - PB.parsePassPipeline(PM, LoopOptimizerEndEPPipeline, VerifyEachPass, - DebugLogging); - }); + PB.registerLoopOptimizerEndEPCallback( + [&PB, VerifyEachPass, DebugLogging]( + LoopPassManager &PM, PassBuilder::OptimizationLevel Level) { + ExitOnError Err("Unable to parse LoopOptimizerEndEP pipeline: "); + Err(PB.parsePassPipeline(PM, LoopOptimizerEndEPPipeline, + VerifyEachPass, DebugLogging)); + }); if (tryParsePipelineText(PB, ScalarOptimizerLateEPPipeline)) PB.registerScalarOptimizerLateEPCallback( [&PB, VerifyEachPass, DebugLogging]( FunctionPassManager &PM, PassBuilder::OptimizationLevel Level) { - PB.parsePassPipeline(PM, ScalarOptimizerLateEPPipeline, - VerifyEachPass, DebugLogging); + ExitOnError Err("Unable to parse ScalarOptimizerLateEP pipeline: "); + Err(PB.parsePassPipeline(PM, ScalarOptimizerLateEPPipeline, + VerifyEachPass, DebugLogging)); }); if (tryParsePipelineText(PB, CGSCCOptimizerLateEPPipeline)) - PB.registerCGSCCOptimizerLateEPCallback([&PB, VerifyEachPass, DebugLogging]( - CGSCCPassManager &PM, PassBuilder::OptimizationLevel Level) { - PB.parsePassPipeline(PM, CGSCCOptimizerLateEPPipeline, VerifyEachPass, - DebugLogging); - }); + PB.registerCGSCCOptimizerLateEPCallback( + [&PB, VerifyEachPass, DebugLogging]( + CGSCCPassManager &PM, PassBuilder::OptimizationLevel Level) { + ExitOnError Err("Unable to parse CGSCCOptimizerLateEP pipeline: "); + Err(PB.parsePassPipeline(PM, CGSCCOptimizerLateEPPipeline, + VerifyEachPass, DebugLogging)); + }); if (tryParsePipelineText(PB, VectorizerStartEPPipeline)) - PB.registerVectorizerStartEPCallback([&PB, VerifyEachPass, DebugLogging]( - FunctionPassManager &PM, PassBuilder::OptimizationLevel Level) { - PB.parsePassPipeline(PM, VectorizerStartEPPipeline, VerifyEachPass, - DebugLogging); - }); + PB.registerVectorizerStartEPCallback( + [&PB, VerifyEachPass, DebugLogging]( + FunctionPassManager &PM, PassBuilder::OptimizationLevel Level) { + ExitOnError Err("Unable to parse VectorizerStartEP pipeline: "); + Err(PB.parsePassPipeline(PM, VectorizerStartEPPipeline, + VerifyEachPass, DebugLogging)); + }); if (tryParsePipelineText(PB, PipelineStartEPPipeline)) PB.registerPipelineStartEPCallback( [&PB, VerifyEachPass, DebugLogging](ModulePassManager &PM) { - PB.parsePassPipeline(PM, PipelineStartEPPipeline, VerifyEachPass, - DebugLogging); + ExitOnError Err("Unable to parse PipelineStartEP pipeline: "); + Err(PB.parsePassPipeline(PM, PipelineStartEPPipeline, VerifyEachPass, + DebugLogging)); }); } @@ -258,8 +271,8 @@ bool llvm::runPassPipeline(StringRef Arg0, Module &M, TargetMachine *TM, // Specially handle the alias analysis manager so that we can register // a custom pipeline of AA passes with it. AAManager AA; - if (!PB.parseAAPipeline(AA, AAPipeline)) { - errs() << Arg0 << ": unable to parse AA pipeline description.\n"; + if (auto Err = PB.parseAAPipeline(AA, AAPipeline)) { + errs() << Arg0 << ": " << toString(std::move(Err)) << "\n"; return false; } @@ -284,8 +297,9 @@ bool llvm::runPassPipeline(StringRef Arg0, Module &M, TargetMachine *TM, if (EnableDebugify) MPM.addPass(NewPMDebugifyPass()); - if (!PB.parsePassPipeline(MPM, PassPipeline, VerifyEachPass, DebugPM)) { - errs() << Arg0 << ": unable to parse pass pipeline description.\n"; + if (auto Err = + PB.parsePassPipeline(MPM, PassPipeline, VerifyEachPass, DebugPM)) { + errs() << Arg0 << ": " << toString(std::move(Err)) << "\n"; return false; } diff --git a/llvm/unittests/IR/CMakeLists.txt b/llvm/unittests/IR/CMakeLists.txt index 211ab10..7498983 100644 --- a/llvm/unittests/IR/CMakeLists.txt +++ b/llvm/unittests/IR/CMakeLists.txt @@ -40,3 +40,5 @@ add_llvm_unittest(IRTests VerifierTest.cpp WaymarkTest.cpp ) + +target_link_libraries(IRTests PRIVATE LLVMTestingSupport) diff --git a/llvm/unittests/IR/PassBuilderCallbacksTest.cpp b/llvm/unittests/IR/PassBuilderCallbacksTest.cpp index 97bbb81..20c47b0 100644 --- a/llvm/unittests/IR/PassBuilderCallbacksTest.cpp +++ b/llvm/unittests/IR/PassBuilderCallbacksTest.cpp @@ -7,6 +7,7 @@ // //===----------------------------------------------------------------------===// +#include "llvm/Testing/Support/Error.h" #include #include #include @@ -460,7 +461,7 @@ TEST_F(ModuleCallbacksTest, Passes) { .WillOnce(Invoke(getAnalysisResult)); StringRef PipelineText = "test-transform"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); @@ -494,7 +495,7 @@ TEST_F(ModuleCallbacksTest, InstrumentedPasses) { .InSequence(PISequence); StringRef PipelineText = "test-transform"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); @@ -525,7 +526,7 @@ TEST_F(ModuleCallbacksTest, InstrumentedSkippedPasses) { .Times(0); StringRef PipelineText = "test-transform"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); @@ -537,7 +538,7 @@ TEST_F(FunctionCallbacksTest, Passes) { .WillOnce(Invoke(getAnalysisResult)); StringRef PipelineText = "test-transform"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); } @@ -571,7 +572,7 @@ TEST_F(FunctionCallbacksTest, InstrumentedPasses) { .InSequence(PISequence); StringRef PipelineText = "test-transform"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); } @@ -604,7 +605,7 @@ TEST_F(FunctionCallbacksTest, InstrumentedSkippedPasses) { .Times(0); StringRef PipelineText = "test-transform"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); } @@ -615,7 +616,7 @@ TEST_F(LoopCallbacksTest, Passes) { .WillOnce(WithArgs<0, 1, 2>(Invoke(getAnalysisResult))); StringRef PipelineText = "test-transform"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); } @@ -650,7 +651,7 @@ TEST_F(LoopCallbacksTest, InstrumentedPasses) { .InSequence(PISequence); StringRef PipelineText = "test-transform"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); } @@ -682,7 +683,7 @@ TEST_F(LoopCallbacksTest, InstrumentedSkippedPasses) { .Times(0); StringRef PipelineText = "test-transform"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); } @@ -693,7 +694,7 @@ TEST_F(CGSCCCallbacksTest, Passes) { .WillOnce(WithArgs<0, 1, 2>(Invoke(getAnalysisResult))); StringRef PipelineText = "test-transform"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); } @@ -727,7 +728,7 @@ TEST_F(CGSCCCallbacksTest, InstrumentedPasses) { .InSequence(PISequence); StringRef PipelineText = "test-transform"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); } @@ -759,7 +760,7 @@ TEST_F(CGSCCCallbacksTest, InstrumentedSkippedPasses) { .Times(0); StringRef PipelineText = "test-transform"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); } @@ -774,7 +775,7 @@ TEST_F(ModuleCallbacksTest, AnalysisUtilities) { EXPECT_CALL(AnalysisHandle, invalidate(HasName(""), _, _)); StringRef PipelineText = "require,invalidate"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); } @@ -784,7 +785,7 @@ TEST_F(CGSCCCallbacksTest, PassUtilities) { EXPECT_CALL(AnalysisHandle, invalidate(HasName("(foo)"), _, _)); StringRef PipelineText = "require,invalidate"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); } @@ -794,7 +795,7 @@ TEST_F(FunctionCallbacksTest, AnalysisUtilities) { EXPECT_CALL(AnalysisHandle, invalidate(HasName("foo"), _, _)); StringRef PipelineText = "require,invalidate"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); } @@ -805,7 +806,7 @@ TEST_F(LoopCallbacksTest, PassUtilities) { StringRef PipelineText = "require,invalidate"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); } @@ -845,13 +846,13 @@ TEST_F(ModuleCallbacksTest, ParseTopLevelPipeline) { StringRef PipelineText = "another-pipeline(test-transform,invalidate)"; - ASSERT_TRUE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; PM.run(*M, AM); /// Test the negative case PipelineText = "another-pipeline(instcombine)"; - ASSERT_FALSE(PB.parsePassPipeline(PM, PipelineText, true)) + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Failed()) << "Pipeline was: " << PipelineText; } } // end anonymous namespace diff --git a/llvm/unittests/Passes/CMakeLists.txt b/llvm/unittests/Passes/CMakeLists.txt index d90df20..415f3a7 100644 --- a/llvm/unittests/Passes/CMakeLists.txt +++ b/llvm/unittests/Passes/CMakeLists.txt @@ -12,6 +12,7 @@ add_llvm_unittest(PluginsTests PluginsTest.cpp ) export_executable_symbols(PluginsTests) +target_link_libraries(PluginsTests PRIVATE LLVMTestingSupport) set(LLVM_LINK_COMPONENTS) add_llvm_loadable_module(TestPlugin diff --git a/llvm/unittests/Passes/PluginsTest.cpp b/llvm/unittests/Passes/PluginsTest.cpp index 7269787..abb7b57 100644 --- a/llvm/unittests/Passes/PluginsTest.cpp +++ b/llvm/unittests/Passes/PluginsTest.cpp @@ -15,6 +15,7 @@ #include "llvm/Support/FileSystem.h" #include "llvm/Support/ManagedStatic.h" #include "llvm/Support/Path.h" +#include "llvm/Testing/Support/Error.h" #include "llvm/Transforms/Scalar/LoopPassManager.h" #include "gtest/gtest.h" @@ -54,8 +55,8 @@ TEST(PluginsTests, LoadPlugin) { PassBuilder PB; ModulePassManager PM; - ASSERT_FALSE(PB.parsePassPipeline(PM, "plugin-pass")); + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, "plugin-pass"), Failed()); Plugin->registerPassBuilderCallbacks(PB); - ASSERT_TRUE(PB.parsePassPipeline(PM, "plugin-pass")); + ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, "plugin-pass"), Succeeded()); } -- 2.7.4