<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 23, 2016 at 2:02 AM, Chandler Carruth via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: chandlerc<br>
Date: Tue Feb 23 04:02:02 2016<br>
New Revision: 261624<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=261624&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=261624&view=rev</a><br>
Log:<br>
[PM] Add a unittest for the CGSCC pass manager in the new pass manager<br>
system.<br>
<br>
Previously, this was only being tested with larger integration tests.<br>
That makes it hard to isolated specific issues with it, and makes the<br>
APIs themselves less well tested. Add a unittest based around the same<br>
patterns used for testing the general pass manager.<br></blockquote><div><br>*thumbs up* for unit testing this sort of stuff - thanks for adding it!<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Added:<br>
llvm/trunk/unittests/Analysis/CGSCCPassManagerTest.cpp<br>
Modified:<br>
llvm/trunk/unittests/Analysis/CMakeLists.txt<br>
<br>
Added: llvm/trunk/unittests/Analysis/CGSCCPassManagerTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Analysis/CGSCCPassManagerTest.cpp?rev=261624&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Analysis/CGSCCPassManagerTest.cpp?rev=261624&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/Analysis/CGSCCPassManagerTest.cpp (added)<br>
+++ llvm/trunk/unittests/Analysis/CGSCCPassManagerTest.cpp Tue Feb 23 04:02:02 2016<br>
@@ -0,0 +1,287 @@<br>
+//===- CGSCCPassManagerTest.cpp -------------------------------------------===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#include "llvm/Analysis/CGSCCPassManager.h"<br>
+#include "llvm/Analysis/LazyCallGraph.h"<br>
+#include "llvm/AsmParser/Parser.h"<br>
+#include "llvm/IR/Function.h"<br>
+#include "llvm/IR/InstIterator.h"<br>
+#include "llvm/IR/LLVMContext.h"<br>
+#include "llvm/IR/Module.h"<br>
+#include "llvm/IR/PassManager.h"<br>
+#include "llvm/Support/SourceMgr.h"<br>
+#include "gtest/gtest.h"<br>
+<br>
+using namespace llvm;<br>
+<br>
+namespace {<br>
+<br>
+class TestModuleAnalysis {<br>
+public:<br>
+ struct Result {<br>
+ Result(int Count) : FunctionCount(Count) {}<br>
+ int FunctionCount;<br>
+ };<br>
+<br>
+ static void *ID() { return (void *)&PassID; }<br></blockquote><div><br></div><div>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?)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ static StringRef name() { return "TestModuleAnalysis"; }<br>
+<br>
+ TestModuleAnalysis(int &Runs) : Runs(Runs) {}<br>
+<br>
+ Result run(Module &M, ModuleAnalysisManager *AM) {<br>
+ ++Runs;<br>
+ return Result(M.size());<br>
+ }<br>
+<br>
+private:<br>
+ static char PassID;<br>
+<br>
+ int &Runs;<br>
+};<br>
+<br>
+char TestModuleAnalysis::PassID;<br>
+<br>
+class TestSCCAnalysis {<br>
+public:<br>
+ struct Result {<br>
+ Result(int Count) : FunctionCount(Count) {}<br>
+ int FunctionCount;<br>
+ };<br>
+<br>
+ static void *ID() { return (void *)&PassID; }<br>
+ static StringRef name() { return "TestSCCAnalysis"; }<br>
+<br>
+ TestSCCAnalysis(int &Runs) : Runs(Runs) {}<br>
+<br>
+ Result run(LazyCallGraph::SCC &C, CGSCCAnalysisManager *AM) {<br>
+ ++Runs;<br>
+ return Result(C.size());<br>
+ }<br>
+<br>
+private:<br>
+ static char PassID;<br>
+<br>
+ int &Runs;<br>
+};<br>
+<br>
+char TestSCCAnalysis::PassID;<br>
+<br>
+class TestFunctionAnalysis {<br>
+public:<br>
+ struct Result {<br>
+ Result(int Count) : InstructionCount(Count) {}<br></blockquote><div><br></div><div>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)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ int InstructionCount;<br>
+ };<br>
+<br>
+ static void *ID() { return (void *)&PassID; }<br>
+ static StringRef name() { return "TestFunctionAnalysis"; }<br>
+<br>
+ TestFunctionAnalysis(int &Runs) : Runs(Runs) {}<br>
+<br>
+ Result run(Function &F, FunctionAnalysisManager *AM) {<br>
+ ++Runs;<br>
+ int Count = 0;<br>
+ for (Instruction &I : instructions(F)) {<br>
+ (void)I;<br>
+ ++Count;<br>
+ }<br>
+ return Result(Count);<br>
+ }<br>
+<br>
+private:<br>
+ static char PassID;<br>
+<br>
+ int &Runs;<br>
+};<br>
+<br>
+char TestFunctionAnalysis::PassID;<br>
+<br>
+struct TestModulePass {<br>
+ TestModulePass(int &RunCount) : RunCount(RunCount) {}<br>
+<br>
+ PreservedAnalyses run(Module &M, ModuleAnalysisManager *AM) {<br>
+ ++RunCount;<br>
+ (void)AM->getResult<TestModuleAnalysis>(M);<br>
+ return PreservedAnalyses::all();<br>
+ }<br>
+<br>
+ static StringRef name() { return "TestModulePass"; }<br>
+<br>
+ int &RunCount;<br>
+};<br>
+<br>
+struct TestSCCPass {<br>
+ TestSCCPass(int &RunCount, int &AnalyzedInstrCount,<br>
+ int &AnalyzedSCCFunctionCount,<br>
+ int &AnalyzedModuleFunctionCount,<br>
+ bool OnlyUseCachedResults = false)<br>
+ : RunCount(RunCount), AnalyzedInstrCount(AnalyzedInstrCount),<br>
+ AnalyzedSCCFunctionCount(AnalyzedSCCFunctionCount),<br>
+ AnalyzedModuleFunctionCount(AnalyzedModuleFunctionCount),<br>
+ OnlyUseCachedResults(OnlyUseCachedResults) {}<br>
+<br>
+ PreservedAnalyses run(LazyCallGraph::SCC &C, CGSCCAnalysisManager *AM) {<br>
+ ++RunCount;<br>
+<br>
+ const ModuleAnalysisManager &MAM =<br>
+ AM->getResult<ModuleAnalysisManagerCGSCCProxy>(C).getManager();<br>
+ FunctionAnalysisManager &FAM =<br>
+ AM->getResult<FunctionAnalysisManagerCGSCCProxy>(C).getManager();<br>
+ if (TestModuleAnalysis::Result *TMA =<br>
+ MAM.getCachedResult<TestModuleAnalysis>(<br>
+ *C.begin()->getFunction().getParent()))<br>
+ AnalyzedModuleFunctionCount += TMA->FunctionCount;<br>
+<br>
+ if (OnlyUseCachedResults) {<br>
+ // Hack to force the use of the cached interface.<br>
+ if (TestSCCAnalysis::Result *AR =<br>
+ AM->getCachedResult<TestSCCAnalysis>(C))<br>
+ AnalyzedSCCFunctionCount += AR->FunctionCount;<br>
+ for (LazyCallGraph::Node &N : C)<br>
+ if (TestFunctionAnalysis::Result *FAR =<br>
+ FAM.getCachedResult<TestFunctionAnalysis>(N.getFunction()))<br>
+ AnalyzedInstrCount += FAR->InstructionCount;<br>
+ } else {<br>
+ // Typical path just runs the analysis as needed.<br>
+ TestSCCAnalysis::Result &AR = AM->getResult<TestSCCAnalysis>(C);<br>
+ AnalyzedSCCFunctionCount += AR.FunctionCount;<br>
+ for (LazyCallGraph::Node &N : C) {<br>
+ TestFunctionAnalysis::Result &FAR =<br>
+ FAM.getResult<TestFunctionAnalysis>(N.getFunction());<br>
+ AnalyzedInstrCount += FAR.InstructionCount;<br>
+ }<br>
+ }<br>
+<br>
+ return PreservedAnalyses::all();<br>
+ }<br>
+<br>
+ static StringRef name() { return "TestSCCPass"; }<br>
+<br>
+ int &RunCount;<br>
+ int &AnalyzedInstrCount;<br>
+ int &AnalyzedSCCFunctionCount;<br>
+ int &AnalyzedModuleFunctionCount;<br>
+ bool OnlyUseCachedResults;<br>
+};<br>
+<br>
+struct TestFunctionPass {<br>
+ TestFunctionPass(int &RunCount) : RunCount(RunCount) {}<br>
+<br>
+ PreservedAnalyses run(Function &M) {<br>
+ ++RunCount;<br>
+ return PreservedAnalyses::none();<br>
+ }<br>
+<br>
+ static StringRef name() { return "TestFunctionPass"; }<br>
+<br>
+ int &RunCount;<br>
+};<br>
+<br>
+std::unique_ptr<Module> parseIR(const char *IR) {<br>
+ LLVMContext &C = getGlobalContext();<br>
+ SMDiagnostic Err;<br>
+ return parseAssemblyString(IR, Err, C);<br>
+}<br>
+<br>
+class CGSCCPassManagerTest : public ::testing::Test {<br>
+protected:<br>
+ std::unique_ptr<Module> M;<br>
+<br>
+public:<br>
+ CGSCCPassManagerTest()<br>
+ : M(parseIR("define void @f() {\n"<br></blockquote><div><br></div><div>Can/should we use the fancy C++11 string literals here?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ "entry:\n"<br>
+ " call void @g()\n"<br>
+ " call void @h1()\n"<br>
+ " ret void\n"<br>
+ "}\n"<br>
+ "define void @g() {\n"<br>
+ "entry:\n"<br>
+ " call void @g()\n"<br>
+ " call void @x()\n"<br>
+ " ret void\n"<br>
+ "}\n"<br>
+ "define void @h1() {\n"<br>
+ "entry:\n"<br>
+ " call void @h2()\n"<br>
+ " ret void\n"<br>
+ "}\n"<br>
+ "define void @h2() {\n"<br>
+ "entry:\n"<br>
+ " call void @h3()\n"<br>
+ " call void @x()\n"<br>
+ " ret void\n"<br>
+ "}\n"<br>
+ "define void @h3() {\n"<br>
+ "entry:\n"<br>
+ " call void @h1()\n"<br>
+ " ret void\n"<br>
+ "}\n"<br>
+ "define void @x() {\n"<br>
+ "entry:\n"<br>
+ " ret void\n"<br>
+ "}\n"<br>
+ )) {}<br>
+};<br>
+<br>
+TEST_F(CGSCCPassManagerTest, Basic) {<br>
+ FunctionAnalysisManager FAM(/*DebugLogging*/ true);<br>
+ int FunctionAnalysisRuns = 0;<br>
+ FAM.registerPass([&] { return TestFunctionAnalysis(FunctionAnalysisRuns); });<br>
+<br>
+ CGSCCAnalysisManager CGAM(/*DebugLogging*/ true);<br>
+ int SCCAnalysisRuns = 0;<br>
+ CGAM.registerPass([&] { return TestSCCAnalysis(SCCAnalysisRuns); });<br>
+<br>
+ ModuleAnalysisManager MAM(/*DebugLogging*/ true);<br>
+ int ModuleAnalysisRuns = 0;<br>
+ MAM.registerPass([&] { return LazyCallGraphAnalysis(); });<br>
+ MAM.registerPass([&] { return TestModuleAnalysis(ModuleAnalysisRuns); });<br>
+<br>
+ MAM.registerPass([&] { return FunctionAnalysisManagerModuleProxy(FAM); });<br>
+ MAM.registerPass([&] { return CGSCCAnalysisManagerModuleProxy(CGAM); });<br>
+ CGAM.registerPass([&] { return FunctionAnalysisManagerCGSCCProxy(FAM); });<br>
+ CGAM.registerPass([&] { return ModuleAnalysisManagerCGSCCProxy(MAM); });<br>
+ FAM.registerPass([&] { return CGSCCAnalysisManagerFunctionProxy(CGAM); });<br>
+ FAM.registerPass([&] { return ModuleAnalysisManagerFunctionProxy(MAM); });<br>
+<br>
+ ModulePassManager MPM(/*DebugLogging*/ true);<br>
+ int ModulePassRunCount1 = 0;<br>
+ MPM.addPass(TestModulePass(ModulePassRunCount1));<br>
+<br>
+ CGSCCPassManager CGPM1(/*DebugLogging*/ true);<br>
+ int SCCPassRunCount1 = 0;<br>
+ int AnalyzedInstrCount1 = 0;<br>
+ int AnalyzedSCCFunctionCount1 = 0;<br>
+ int AnalyzedModuleFunctionCount1 = 0;<br>
+ CGPM1.addPass(TestSCCPass(SCCPassRunCount1, AnalyzedInstrCount1,<br>
+ AnalyzedSCCFunctionCount1,<br>
+ AnalyzedModuleFunctionCount1));<br>
+<br>
+ FunctionPassManager FPM1(/*DebugLogging*/ true);<br>
+ int FunctionPassRunCount1 = 0;<br>
+ FPM1.addPass(TestFunctionPass(FunctionPassRunCount1));<br>
+ CGPM1.addPass(createCGSCCToFunctionPassAdaptor(std::move(FPM1)));<br>
+ MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM1)));<br>
+<br>
+ MPM.run(*M, &MAM);<br>
+<br>
+ EXPECT_EQ(1, ModulePassRunCount1);<br></blockquote><div><br></div><div>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?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+ EXPECT_EQ(1, ModuleAnalysisRuns);<br>
+ EXPECT_EQ(4, SCCAnalysisRuns);<br>
+ EXPECT_EQ(6, FunctionAnalysisRuns);<br>
+<br>
+ EXPECT_EQ(4, SCCPassRunCount1);<br>
+ EXPECT_EQ(14, AnalyzedInstrCount1);<br>
+ EXPECT_EQ(6, AnalyzedSCCFunctionCount1);<br>
+ EXPECT_EQ(4 * 6, AnalyzedModuleFunctionCount1);<br>
+}<br>
+<br>
+}<br>
<br>
Modified: llvm/trunk/unittests/Analysis/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Analysis/CMakeLists.txt?rev=261624&r1=261623&r2=261624&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Analysis/CMakeLists.txt?rev=261624&r1=261623&r2=261624&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/Analysis/CMakeLists.txt (original)<br>
+++ llvm/trunk/unittests/Analysis/CMakeLists.txt Tue Feb 23 04:02:02 2016<br>
@@ -9,6 +9,7 @@ add_llvm_unittest(AnalysisTests<br>
AliasAnalysisTest.cpp<br>
CallGraphTest.cpp<br>
CFGTest.cpp<br>
+ CGSCCPassManagerTest.cpp<br>
LazyCallGraphTest.cpp<br>
ScalarEvolutionTest.cpp<br>
MixedTBAATest.cpp<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>