[llvm] r261624 - [PM] Add a unittest for the CGSCC pass manager in the new pass manager
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 23 15:46:26 PST 2016
Thanks for the review. I just wanted to mention I saw it and am working on
it. This is mostly cleanup, so it may not be at the top of the queue, but I
really don't want to forget it either...
On Tue, Feb 23, 2016 at 8:33 AM David Blaikie <dblaikie at gmail.com> wrote:
> On Tue, Feb 23, 2016 at 2:02 AM, Chandler Carruth via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: chandlerc
>> Date: Tue Feb 23 04:02:02 2016
>> New Revision: 261624
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=261624&view=rev
>> Log:
>> [PM] Add a unittest for the CGSCC pass manager in the new pass manager
>> system.
>>
>> Previously, this was only being tested with larger integration tests.
>> That makes it hard to isolated specific issues with it, and makes the
>> APIs themselves less well tested. Add a unittest based around the same
>> patterns used for testing the general pass manager.
>>
>
> *thumbs up* for unit testing this sort of stuff - thanks for adding it!
>
>
>>
>> Added:
>> llvm/trunk/unittests/Analysis/CGSCCPassManagerTest.cpp
>> Modified:
>> llvm/trunk/unittests/Analysis/CMakeLists.txt
>>
>> Added: llvm/trunk/unittests/Analysis/CGSCCPassManagerTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Analysis/CGSCCPassManagerTest.cpp?rev=261624&view=auto
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/Analysis/CGSCCPassManagerTest.cpp (added)
>> +++ llvm/trunk/unittests/Analysis/CGSCCPassManagerTest.cpp Tue Feb 23
>> 04:02:02 2016
>> @@ -0,0 +1,287 @@
>> +//===- CGSCCPassManagerTest.cpp
>> -------------------------------------------===//
>> +//
>> +// The LLVM Compiler Infrastructure
>> +//
>> +// This file is distributed under the University of Illinois Open Source
>> +// License. See LICENSE.TXT for details.
>> +//
>>
>> +//===----------------------------------------------------------------------===//
>> +
>> +#include "llvm/Analysis/CGSCCPassManager.h"
>> +#include "llvm/Analysis/LazyCallGraph.h"
>> +#include "llvm/AsmParser/Parser.h"
>> +#include "llvm/IR/Function.h"
>> +#include "llvm/IR/InstIterator.h"
>> +#include "llvm/IR/LLVMContext.h"
>> +#include "llvm/IR/Module.h"
>> +#include "llvm/IR/PassManager.h"
>> +#include "llvm/Support/SourceMgr.h"
>> +#include "gtest/gtest.h"
>> +
>> +using namespace llvm;
>> +
>> +namespace {
>> +
>> +class TestModuleAnalysis {
>> +public:
>> + struct Result {
>> + Result(int Count) : FunctionCount(Count) {}
>> + int FunctionCount;
>> + };
>> +
>> + static void *ID() { return (void *)&PassID; }
>>
>
> Just a thought - could we remove the burden for passes to provide their
> own ID by using a template helper? (template<typename Pass> struct
> PassIdentifier { static const char ID; }; sort of thing) or is it helpful
> to have implementations be able to differ here? (or just to have simpler
> API/interaction?)
>
>
>> + static StringRef name() { return "TestModuleAnalysis"; }
>> +
>> + TestModuleAnalysis(int &Runs) : Runs(Runs) {}
>> +
>> + Result run(Module &M, ModuleAnalysisManager *AM) {
>> + ++Runs;
>> + return Result(M.size());
>> + }
>> +
>> +private:
>> + static char PassID;
>> +
>> + int &Runs;
>> +};
>> +
>> +char TestModuleAnalysis::PassID;
>> +
>> +class TestSCCAnalysis {
>> +public:
>> + struct Result {
>> + Result(int Count) : FunctionCount(Count) {}
>> + int FunctionCount;
>> + };
>> +
>> + static void *ID() { return (void *)&PassID; }
>> + static StringRef name() { return "TestSCCAnalysis"; }
>> +
>> + TestSCCAnalysis(int &Runs) : Runs(Runs) {}
>> +
>> + Result run(LazyCallGraph::SCC &C, CGSCCAnalysisManager *AM) {
>> + ++Runs;
>> + return Result(C.size());
>> + }
>> +
>> +private:
>> + static char PassID;
>> +
>> + int &Runs;
>> +};
>> +
>> +char TestSCCAnalysis::PassID;
>> +
>> +class TestFunctionAnalysis {
>> +public:
>> + struct Result {
>> + Result(int Count) : InstructionCount(Count) {}
>>
>
> I think most, if not all, the ctors in here could be omitted & {} init
> used instead. At least for these Result structs that seems OK? Maybe even
> for the passes - which just init their members in turn anyway (though I'm
> not sure off the top of my head how {} init would interact with reference
> members)
>
>
>> + int InstructionCount;
>> + };
>> +
>> + static void *ID() { return (void *)&PassID; }
>> + static StringRef name() { return "TestFunctionAnalysis"; }
>> +
>> + TestFunctionAnalysis(int &Runs) : Runs(Runs) {}
>> +
>> + Result run(Function &F, FunctionAnalysisManager *AM) {
>> + ++Runs;
>> + int Count = 0;
>> + for (Instruction &I : instructions(F)) {
>> + (void)I;
>> + ++Count;
>> + }
>> + return Result(Count);
>> + }
>> +
>> +private:
>> + static char PassID;
>> +
>> + int &Runs;
>> +};
>> +
>> +char TestFunctionAnalysis::PassID;
>> +
>> +struct TestModulePass {
>> + TestModulePass(int &RunCount) : RunCount(RunCount) {}
>> +
>> + PreservedAnalyses run(Module &M, ModuleAnalysisManager *AM) {
>> + ++RunCount;
>> + (void)AM->getResult<TestModuleAnalysis>(M);
>> + return PreservedAnalyses::all();
>> + }
>> +
>> + static StringRef name() { return "TestModulePass"; }
>> +
>> + int &RunCount;
>> +};
>> +
>> +struct TestSCCPass {
>> + TestSCCPass(int &RunCount, int &AnalyzedInstrCount,
>> + int &AnalyzedSCCFunctionCount,
>> + int &AnalyzedModuleFunctionCount,
>> + bool OnlyUseCachedResults = false)
>> + : RunCount(RunCount), AnalyzedInstrCount(AnalyzedInstrCount),
>> + AnalyzedSCCFunctionCount(AnalyzedSCCFunctionCount),
>> + AnalyzedModuleFunctionCount(AnalyzedModuleFunctionCount),
>> + OnlyUseCachedResults(OnlyUseCachedResults) {}
>> +
>> + PreservedAnalyses run(LazyCallGraph::SCC &C, CGSCCAnalysisManager *AM)
>> {
>> + ++RunCount;
>> +
>> + const ModuleAnalysisManager &MAM =
>> + AM->getResult<ModuleAnalysisManagerCGSCCProxy>(C).getManager();
>> + FunctionAnalysisManager &FAM =
>> + AM->getResult<FunctionAnalysisManagerCGSCCProxy>(C).getManager();
>> + if (TestModuleAnalysis::Result *TMA =
>> + MAM.getCachedResult<TestModuleAnalysis>(
>> + *C.begin()->getFunction().getParent()))
>> + AnalyzedModuleFunctionCount += TMA->FunctionCount;
>> +
>> + if (OnlyUseCachedResults) {
>> + // Hack to force the use of the cached interface.
>> + if (TestSCCAnalysis::Result *AR =
>> + AM->getCachedResult<TestSCCAnalysis>(C))
>> + AnalyzedSCCFunctionCount += AR->FunctionCount;
>> + for (LazyCallGraph::Node &N : C)
>> + if (TestFunctionAnalysis::Result *FAR =
>> +
>> FAM.getCachedResult<TestFunctionAnalysis>(N.getFunction()))
>> + AnalyzedInstrCount += FAR->InstructionCount;
>> + } else {
>> + // Typical path just runs the analysis as needed.
>> + TestSCCAnalysis::Result &AR = AM->getResult<TestSCCAnalysis>(C);
>> + AnalyzedSCCFunctionCount += AR.FunctionCount;
>> + for (LazyCallGraph::Node &N : C) {
>> + TestFunctionAnalysis::Result &FAR =
>> + FAM.getResult<TestFunctionAnalysis>(N.getFunction());
>> + AnalyzedInstrCount += FAR.InstructionCount;
>> + }
>> + }
>> +
>> + return PreservedAnalyses::all();
>> + }
>> +
>> + static StringRef name() { return "TestSCCPass"; }
>> +
>> + int &RunCount;
>> + int &AnalyzedInstrCount;
>> + int &AnalyzedSCCFunctionCount;
>> + int &AnalyzedModuleFunctionCount;
>> + bool OnlyUseCachedResults;
>> +};
>> +
>> +struct TestFunctionPass {
>> + TestFunctionPass(int &RunCount) : RunCount(RunCount) {}
>> +
>> + PreservedAnalyses run(Function &M) {
>> + ++RunCount;
>> + return PreservedAnalyses::none();
>> + }
>> +
>> + static StringRef name() { return "TestFunctionPass"; }
>> +
>> + int &RunCount;
>> +};
>> +
>> +std::unique_ptr<Module> parseIR(const char *IR) {
>> + LLVMContext &C = getGlobalContext();
>> + SMDiagnostic Err;
>> + return parseAssemblyString(IR, Err, C);
>> +}
>> +
>> +class CGSCCPassManagerTest : public ::testing::Test {
>> +protected:
>> + std::unique_ptr<Module> M;
>> +
>> +public:
>> + CGSCCPassManagerTest()
>> + : M(parseIR("define void @f() {\n"
>>
>
> Can/should we use the fancy C++11 string literals here?
>
>
>> + "entry:\n"
>> + " call void @g()\n"
>> + " call void @h1()\n"
>> + " ret void\n"
>> + "}\n"
>> + "define void @g() {\n"
>> + "entry:\n"
>> + " call void @g()\n"
>> + " call void @x()\n"
>> + " ret void\n"
>> + "}\n"
>> + "define void @h1() {\n"
>> + "entry:\n"
>> + " call void @h2()\n"
>> + " ret void\n"
>> + "}\n"
>> + "define void @h2() {\n"
>> + "entry:\n"
>> + " call void @h3()\n"
>> + " call void @x()\n"
>> + " ret void\n"
>> + "}\n"
>> + "define void @h3() {\n"
>> + "entry:\n"
>> + " call void @h1()\n"
>> + " ret void\n"
>> + "}\n"
>> + "define void @x() {\n"
>> + "entry:\n"
>> + " ret void\n"
>> + "}\n"
>> + )) {}
>> +};
>> +
>> +TEST_F(CGSCCPassManagerTest, Basic) {
>> + FunctionAnalysisManager FAM(/*DebugLogging*/ true);
>> + int FunctionAnalysisRuns = 0;
>> + FAM.registerPass([&] { return
>> TestFunctionAnalysis(FunctionAnalysisRuns); });
>> +
>> + CGSCCAnalysisManager CGAM(/*DebugLogging*/ true);
>> + int SCCAnalysisRuns = 0;
>> + CGAM.registerPass([&] { return TestSCCAnalysis(SCCAnalysisRuns); });
>> +
>> + ModuleAnalysisManager MAM(/*DebugLogging*/ true);
>> + int ModuleAnalysisRuns = 0;
>> + MAM.registerPass([&] { return LazyCallGraphAnalysis(); });
>> + MAM.registerPass([&] { return TestModuleAnalysis(ModuleAnalysisRuns);
>> });
>> +
>> + MAM.registerPass([&] { return FunctionAnalysisManagerModuleProxy(FAM);
>> });
>> + MAM.registerPass([&] { return CGSCCAnalysisManagerModuleProxy(CGAM);
>> });
>> + CGAM.registerPass([&] { return FunctionAnalysisManagerCGSCCProxy(FAM);
>> });
>> + CGAM.registerPass([&] { return ModuleAnalysisManagerCGSCCProxy(MAM);
>> });
>> + FAM.registerPass([&] { return CGSCCAnalysisManagerFunctionProxy(CGAM);
>> });
>> + FAM.registerPass([&] { return ModuleAnalysisManagerFunctionProxy(MAM);
>> });
>> +
>> + ModulePassManager MPM(/*DebugLogging*/ true);
>> + int ModulePassRunCount1 = 0;
>> + MPM.addPass(TestModulePass(ModulePassRunCount1));
>> +
>> + CGSCCPassManager CGPM1(/*DebugLogging*/ true);
>> + int SCCPassRunCount1 = 0;
>> + int AnalyzedInstrCount1 = 0;
>> + int AnalyzedSCCFunctionCount1 = 0;
>> + int AnalyzedModuleFunctionCount1 = 0;
>> + CGPM1.addPass(TestSCCPass(SCCPassRunCount1, AnalyzedInstrCount1,
>> + AnalyzedSCCFunctionCount1,
>> + AnalyzedModuleFunctionCount1));
>> +
>> + FunctionPassManager FPM1(/*DebugLogging*/ true);
>> + int FunctionPassRunCount1 = 0;
>> + FPM1.addPass(TestFunctionPass(FunctionPassRunCount1));
>> + CGPM1.addPass(createCGSCCToFunctionPassAdaptor(std::move(FPM1)));
>> + MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM1)));
>> +
>> + MPM.run(*M, &MAM);
>> +
>> + EXPECT_EQ(1, ModulePassRunCount1);
>>
>
> Perhaps it's obvious to readers in this domain, but it's not clear to me
> why these numbers are the right ones - perhaps a comment explaining the
> test scenario would be helpful (maybe at the top by the module IR, or down
> here by the EXPECTs, not sure)? Or isolating the tests a bit more
> (refactoring so as not to have to repeat too much boilerplate setup) so
> each one is more obvious?
>
>
>> +
>> + EXPECT_EQ(1, ModuleAnalysisRuns);
>> + EXPECT_EQ(4, SCCAnalysisRuns);
>> + EXPECT_EQ(6, FunctionAnalysisRuns);
>> +
>> + EXPECT_EQ(4, SCCPassRunCount1);
>> + EXPECT_EQ(14, AnalyzedInstrCount1);
>> + EXPECT_EQ(6, AnalyzedSCCFunctionCount1);
>> + EXPECT_EQ(4 * 6, AnalyzedModuleFunctionCount1);
>> +}
>> +
>> +}
>>
>> Modified: llvm/trunk/unittests/Analysis/CMakeLists.txt
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Analysis/CMakeLists.txt?rev=261624&r1=261623&r2=261624&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/Analysis/CMakeLists.txt (original)
>> +++ llvm/trunk/unittests/Analysis/CMakeLists.txt Tue Feb 23 04:02:02 2016
>> @@ -9,6 +9,7 @@ add_llvm_unittest(AnalysisTests
>> AliasAnalysisTest.cpp
>> CallGraphTest.cpp
>> CFGTest.cpp
>> + CGSCCPassManagerTest.cpp
>> LazyCallGraphTest.cpp
>> ScalarEvolutionTest.cpp
>> MixedTBAATest.cpp
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160223/0a620ada/attachment.html>
More information about the llvm-commits
mailing list