From 2d74092f5f420bbf10b526835b36d9a521746c2a Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Thu, 13 Mar 2014 10:42:18 +0000 Subject: [PATCH] [PM] As was pointed out in review, I need to define a custom swap in order to use the single assignment. That's probably worth doing for a lot of these types anyways as they may have non-trivial moves and so getting copy elision in more places seems worthwhile. I've tried to add some tests that actually catch this mistake, and one of the types is now well tested but the others' tests still fail to catch this. I'll keep working on tests, but this gets the core pattern right. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@203780 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/PassManager.h | 56 +++++++++++++++++++++++++------- unittests/IR/PassManagerTest.cpp | 47 +++++++++++++++++++++++++-- 2 files changed, 88 insertions(+), 15 deletions(-) diff --git a/include/llvm/IR/PassManager.h b/include/llvm/IR/PassManager.h index 5e5e6537f8a..c6c530cce5e 100644 --- a/include/llvm/IR/PassManager.h +++ b/include/llvm/IR/PassManager.h @@ -71,8 +71,12 @@ public: : PreservedPassIDs(Arg.PreservedPassIDs) {} PreservedAnalyses(PreservedAnalyses &&Arg) : PreservedPassIDs(std::move(Arg.PreservedPassIDs)) {} + friend void swap(PreservedAnalyses &LHS, PreservedAnalyses &RHS) { + using std::swap; + swap(LHS.PreservedPassIDs, RHS.PreservedPassIDs); + } PreservedAnalyses &operator=(PreservedAnalyses RHS) { - std::swap(*this, RHS); + swap(*this, RHS); return *this; } @@ -212,8 +216,12 @@ struct PassModel // refuses to generate them. PassModel(const PassModel &Arg) : Pass(Arg.Pass) {} PassModel(PassModel &&Arg) : Pass(std::move(Arg.Pass)) {} + friend void swap(PassModel &LHS, PassModel &RHS) { + using std::swap; + swap(LHS.Pass, RHS.Pass); + } PassModel &operator=(PassModel RHS) { - std::swap(*this, RHS); + swap(*this, RHS); return *this; } @@ -234,8 +242,12 @@ struct PassModel // refuses to generate them. PassModel(const PassModel &Arg) : Pass(Arg.Pass) {} PassModel(PassModel &&Arg) : Pass(std::move(Arg.Pass)) {} + friend void swap(PassModel &LHS, PassModel &RHS) { + using std::swap; + swap(LHS.Pass, RHS.Pass); + } PassModel &operator=(PassModel RHS) { - std::swap(*this, RHS); + swap(*this, RHS); return *this; } @@ -306,8 +318,12 @@ struct AnalysisResultModel AnalysisResultModel(const AnalysisResultModel &Arg) : Result(Arg.Result) {} AnalysisResultModel(AnalysisResultModel &&Arg) : Result(std::move(Arg.Result)) {} + friend void swap(AnalysisResultModel &LHS, AnalysisResultModel &RHS) { + using std::swap; + swap(LHS.Result, RHS.Result); + } AnalysisResultModel &operator=(AnalysisResultModel RHS) { - std::swap(*this, RHS); + swap(*this, RHS); return *this; } @@ -334,8 +350,12 @@ struct AnalysisResultModel AnalysisResultModel(const AnalysisResultModel &Arg) : Result(Arg.Result) {} AnalysisResultModel(AnalysisResultModel &&Arg) : Result(std::move(Arg.Result)) {} + friend void swap(AnalysisResultModel &LHS, AnalysisResultModel &RHS) { + using std::swap; + swap(LHS.Result, RHS.Result); + } AnalysisResultModel &operator=(AnalysisResultModel RHS) { - std::swap(*this, RHS); + swap(*this, RHS); return *this; } @@ -382,8 +402,12 @@ struct AnalysisPassModel // refuses to generate them. AnalysisPassModel(const AnalysisPassModel &Arg) : Pass(Arg.Pass) {} AnalysisPassModel(AnalysisPassModel &&Arg) : Pass(std::move(Arg.Pass)) {} + friend void swap(AnalysisPassModel &LHS, AnalysisPassModel &RHS) { + using std::swap; + swap(LHS.Pass, RHS.Pass); + } AnalysisPassModel &operator=(AnalysisPassModel RHS) { - std::swap(*this, RHS); + swap(*this, RHS); return *this; } @@ -412,8 +436,12 @@ struct AnalysisPassModel // refuses to generate them. AnalysisPassModel(const AnalysisPassModel &Arg) : Pass(Arg.Pass) {} AnalysisPassModel(AnalysisPassModel &&Arg) : Pass(std::move(Arg.Pass)) {} + friend void swap(AnalysisPassModel &LHS, AnalysisPassModel &RHS) { + using std::swap; + swap(LHS.Pass, RHS.Pass); + } AnalysisPassModel &operator=(AnalysisPassModel RHS) { - std::swap(*this, RHS); + swap(*this, RHS); return *this; } @@ -809,7 +837,7 @@ public: : FAM(std::move(Arg.FAM)) {} FunctionAnalysisManagerModuleProxy & operator=(FunctionAnalysisManagerModuleProxy RHS) { - std::swap(*this, RHS); + std::swap(FAM, RHS.FAM); return *this; } @@ -842,7 +870,7 @@ public: Result(const Result &Arg) : FAM(Arg.FAM) {} Result(Result &&Arg) : FAM(std::move(Arg.FAM)) {} Result &operator=(Result RHS) { - std::swap(*this, RHS); + std::swap(FAM, RHS.FAM); return *this; } ~Result(); @@ -889,7 +917,7 @@ public: Result(const Result &Arg) : MAM(Arg.MAM) {} Result(Result &&Arg) : MAM(std::move(Arg.MAM)) {} Result &operator=(Result RHS) { - std::swap(*this, RHS); + std::swap(MAM, RHS.MAM); return *this; } @@ -915,7 +943,7 @@ public: : MAM(std::move(Arg.MAM)) {} ModuleAnalysisManagerFunctionProxy & operator=(ModuleAnalysisManagerFunctionProxy RHS) { - std::swap(*this, RHS); + std::swap(MAM, RHS.MAM); return *this; } @@ -948,8 +976,12 @@ public: : Pass(Arg.Pass) {} ModuleToFunctionPassAdaptor(ModuleToFunctionPassAdaptor &&Arg) : Pass(std::move(Arg.Pass)) {} + friend void swap(ModuleToFunctionPassAdaptor &LHS, ModuleToFunctionPassAdaptor &RHS) { + using std::swap; + swap(LHS.Pass, RHS.Pass); + } ModuleToFunctionPassAdaptor &operator=(ModuleToFunctionPassAdaptor RHS) { - std::swap(*this, RHS); + swap(*this, RHS); return *this; } diff --git a/unittests/IR/PassManagerTest.cpp b/unittests/IR/PassManagerTest.cpp index a2d59d31a40..310e48fc3b8 100644 --- a/unittests/IR/PassManagerTest.cpp +++ b/unittests/IR/PassManagerTest.cpp @@ -191,6 +191,39 @@ public: "}\n")) {} }; +TEST_F(PassManagerTest, BasicPreservedAnalyses) { + PreservedAnalyses PA1 = PreservedAnalyses(); + EXPECT_FALSE(PA1.preserved()); + EXPECT_FALSE(PA1.preserved()); + PreservedAnalyses PA2 = PreservedAnalyses::none(); + EXPECT_FALSE(PA2.preserved()); + EXPECT_FALSE(PA2.preserved()); + PreservedAnalyses PA3 = PreservedAnalyses::all(); + EXPECT_TRUE(PA3.preserved()); + EXPECT_TRUE(PA3.preserved()); + PreservedAnalyses PA4 = PA1; + EXPECT_FALSE(PA4.preserved()); + EXPECT_FALSE(PA4.preserved()); + PA4 = PA3; + EXPECT_TRUE(PA4.preserved()); + EXPECT_TRUE(PA4.preserved()); + PA4 = std::move(PA2); + EXPECT_FALSE(PA4.preserved()); + EXPECT_FALSE(PA4.preserved()); + PA4.preserve(); + EXPECT_TRUE(PA4.preserved()); + EXPECT_FALSE(PA4.preserved()); + PA1.preserve(); + EXPECT_FALSE(PA1.preserved()); + EXPECT_TRUE(PA1.preserved()); + PA1.preserve(); + EXPECT_TRUE(PA1.preserved()); + EXPECT_TRUE(PA1.preserved()); + PA1.intersect(PA4); + EXPECT_TRUE(PA1.preserved()); + EXPECT_FALSE(PA1.preserved()); +} + TEST_F(PassManagerTest, Basic) { FunctionAnalysisManager FAM; int FunctionAnalysisRuns = 0; @@ -209,10 +242,18 @@ TEST_F(PassManagerTest, Basic) { int AnalyzedInstrCount1 = 0; int AnalyzedFunctionCount1 = 0; { + // Pointless scoped copy to test move assignment. + ModulePassManager NestedMPM; FunctionPassManager FPM; - FPM.addPass(TestFunctionPass(FunctionPassRunCount1, AnalyzedInstrCount1, - AnalyzedFunctionCount1)); - MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM))); + { + // Pointless scope to test move assignment. + FunctionPassManager NestedFPM; + NestedFPM.addPass(TestFunctionPass(FunctionPassRunCount1, AnalyzedInstrCount1, + AnalyzedFunctionCount1)); + FPM = std::move(NestedFPM); + } + NestedMPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM))); + MPM = std::move(NestedMPM); } // Count the runs over a module. -- 2.34.1