From a22bfe1edb68b616eaa9cebfcdaf773414f65bde Mon Sep 17 00:00:00 2001 From: =?utf8?q?=D0=9F=D0=B0=D0=B2=D0=B5=D0=BB=20=D0=98=D0=BB=D1=8C=D1=8E?= =?utf8?q?=D1=82=D1=87=D0=B5=D0=BD=D0=BA=D0=BE/AI=20Tools=20Lab=20/SRR/Ass?= =?utf8?q?istant=20Engineer/=EC=82=BC=EC=84=B1=EC=A0=84=EC=9E=90?= Date: Tue, 23 Oct 2018 21:24:20 +0300 Subject: [PATCH] [nnc] Eliminate singleton from passes and passManager (#1844) Eliminating singletons in frontend and backend passes with creating vector of unique_ptr of Passes in PassManager which contains in Driver which now non static. All passes will be deleted via PassManger destructor. Signed-off-by: Pavel Iliutchenko p.iliutchenk@partner.samsung.com --- contrib/nnc/driver/Driver.cpp | 79 +++++++++------------- contrib/nnc/driver/Driver.h | 10 ++- contrib/nnc/driver/main.cpp | 3 +- contrib/nnc/include/pass/Pass.h | 9 +-- contrib/nnc/include/pass/PassManager.h | 22 +++--- .../passes/acl_soft_backend/AclCppGenerator.h | 6 +- .../include/passes/caffe_frontend/CaffeFrontend.h | 8 +-- .../include/passes/interpreter/InterpreterPass.h | 11 ++- .../include/passes/soft_backend/BaseGenerator.h | 6 +- .../nnc/include/passes/soft_backend/CPPGenerator.h | 33 ++++----- .../passes/tflite_frontend/TfliteFrontend.h | 20 ++---- contrib/nnc/pass/PassManager.cpp | 14 ++-- .../passes/acl_soft_backend/AclCppGenerator.cpp | 7 -- .../nnc/passes/caffe_frontend/caffe_frontend.cpp | 8 +-- .../nnc/passes/interpreter/interpreter_pass.cpp | 5 -- contrib/nnc/passes/soft_backend/CPPGenerator.cpp | 6 -- .../nnc/passes/tflite_frontend/tflite_frontend.cpp | 18 +---- contrib/nnc/tests/soft_backend/CompileCPP.cpp | 3 +- contrib/nnc/unittests/soft_backend/Generator.cpp | 7 +- 19 files changed, 92 insertions(+), 183 deletions(-) diff --git a/contrib/nnc/driver/Driver.cpp b/contrib/nnc/driver/Driver.cpp index 162c98e..f3180fd 100644 --- a/contrib/nnc/driver/Driver.cpp +++ b/contrib/nnc/driver/Driver.cpp @@ -14,7 +14,6 @@ * limitations under the License. */ -#include "pass/PassManager.h" #include "pass/PassData.h" #include "passes/caffe_frontend/CaffeFrontend.h" @@ -29,21 +28,19 @@ #include "Driver.h" -namespace nnc -{ +namespace nnc { /** * @brief run all registered passes * @throw PassException, if errors occured */ -static void runPasses() -{ - auto registeredPasses = PassManager::getPassManager()->getPasses(); - PassData passData(nullptr); - - for ( auto &pass : registeredPasses ) - { - passData = pass->run(passData); +void Driver::runPasses() { + + const auto& registered_passes = _passManager.getPasses(); + PassData pass_data(nullptr); + + for (auto &pass : registered_passes) { + pass_data = pass->run(pass_data); } } // runPasses @@ -52,43 +49,36 @@ static void runPasses() * @brief Register frontend pass * @throw DriverException if errors occurred */ -static void registerFrontendPass() -{ - Pass *pass; +void Driver::registerFrontendPass() { - if ( cli::caffeFrontend.isDisabled() && cli::tflFrontend.isDisabled() ) - { + std::unique_ptr pass; + + if (cli::caffeFrontend.isDisabled() && cli::tflFrontend.isDisabled()) { throw DriverException("frontends are not available"); } - if ( cli::caffeFrontend && cli::tflFrontend ) - { + if (cli::caffeFrontend && cli::tflFrontend) { throw DriverException("only one of the following options are allowed" " to be set in the same time: '" + cli::caffeFrontend.getNames()[0] + "', '" + cli::tflFrontend.getNames()[0] + "'"); } - if ( cli::caffeFrontend ) - { + if (cli::caffeFrontend) { #ifdef NNC_FRONTEND_CAFFE_ENABLED - pass = &CaffeFrontend::getInstance(); + pass = std::move(std::unique_ptr(new CaffeFrontend())); #endif // NNC_FRONTEND_CAFFE_ENABLED - } - else if ( cli::tflFrontend ) - { + } else if (cli::tflFrontend) { #ifdef NNC_FRONTEND_TFLITE_ENABLED - pass = &tflite::TFLiteFrontend::getInstance(); + pass = std::move(std::unique_ptr(new TFLiteFrontend())); #endif // NNC_FRONTEND_TFLITE_ENABLED - } - else - { + } else { throw DriverException("one of the following options must be defined: '" + cli::caffeFrontend.getNames()[0] + "', '" + cli::tflFrontend.getNames()[0] + "'"); } - PassManager::getPassManager()->registerPass(pass); + _passManager.registerPass(std::move(pass)); } // registerFrontendPass @@ -96,33 +86,26 @@ static void registerFrontendPass() * @brief Register backend pass * @throw DriverException if errors occurred */ -static void registerBackendPass() -{ - Pass *pass; +void Driver::registerBackendPass() { - if ( cli::target == NNC_TARGET_X86_CPP ) - { - pass = &CPPCodeGenerator::getInstance(); - } - else if (cli::target == NNC_TARGET_ARM_GPU_CPP ) - { - pass = &AclCppCodeGenerator::getInstance(); - } - else if ( cli::target == NNC_TARGET_INTERPRETER ) - { - pass = &InterpreterPass::getInstance(); + std::unique_ptr pass; - } else - { + if (cli::target == NNC_TARGET_X86_CPP) { + pass = std::move(std::unique_ptr(new CPPCodeGenerator())); + } else if (cli::target == NNC_TARGET_ARM_GPU_CPP) { + pass = std::move(std::unique_ptr(new AclCppCodeGenerator())); + } else if (cli::target == NNC_TARGET_INTERPRETER) { + pass = std::move(std::unique_ptr(new InterpreterPass())); + } else { assert(false && "invalid option value"); } - PassManager::getPassManager()->registerPass(pass); + _passManager.registerPass(std::move(pass)); } // registerBackendPass -void Driver::runDriver() -{ +void Driver::runDriver() { + // register passes registerFrontendPass(); registerBackendPass(); diff --git a/contrib/nnc/driver/Driver.h b/contrib/nnc/driver/Driver.h index 181afbd..06059a6 100644 --- a/contrib/nnc/driver/Driver.h +++ b/contrib/nnc/driver/Driver.h @@ -20,6 +20,8 @@ #include #include +#include "pass/PassManager.h" + namespace nnc { @@ -50,8 +52,14 @@ public: * @throw DriverException if errors occurred in driver * PassException if errors occurred in passes */ - static void runDriver(); + void runDriver(); + +private: + void registerFrontendPass(); + void registerBackendPass(); + void runPasses(); + PassManager _passManager; }; } // namespace nnc diff --git a/contrib/nnc/driver/main.cpp b/contrib/nnc/driver/main.cpp index 4574205..1e19bd7 100644 --- a/contrib/nnc/driver/main.cpp +++ b/contrib/nnc/driver/main.cpp @@ -38,7 +38,8 @@ int main(int argc, const char *argv[]) // for_each(all_passes): // run pass // - Driver::runDriver(); + Driver driver; + driver.runDriver(); // errors didn't happen exit_code = EXIT_SUCCESS; diff --git a/contrib/nnc/include/pass/Pass.h b/contrib/nnc/include/pass/Pass.h index 005e6bb..170de75 100644 --- a/contrib/nnc/include/pass/Pass.h +++ b/contrib/nnc/include/pass/Pass.h @@ -27,15 +27,8 @@ namespace nnc /** * @brief this class represent an interface for all compiler passes like that frontend, backend etc */ -class Pass -{ +class Pass { public: - Pass() = default; - - // to prevent copy of object - Pass &operator=(const Pass &) = delete; - Pass(const Pass &) = delete; - /** * @brief run compiler pass * @param data - data that pass is taken diff --git a/contrib/nnc/include/pass/PassManager.h b/contrib/nnc/include/pass/PassManager.h index 6050ac1..09a5a73 100644 --- a/contrib/nnc/include/pass/PassManager.h +++ b/contrib/nnc/include/pass/PassManager.h @@ -18,9 +18,9 @@ #define __PASS_MANAGER_H__ #include +#include -namespace nnc -{ +namespace nnc { // forward declaration class Pass; @@ -28,30 +28,24 @@ class Pass; /** * @brief pass manager class. This class manages running of passes */ -class PassManager -{ +class PassManager { public: - /** - * @brief singleton method to get PassManager instance - */ - static PassManager *getPassManager(); + PassManager(); + ~PassManager(); /** * @brief register pass in pass manager * @param pass - registered pass */ - void registerPass(Pass *pass); + void registerPass(std::unique_ptr pass); /** * @brief get all registered passes in order in which they were registered */ - using Passes = std::vector; - Passes getPasses() const { return _passes; } + using Passes = std::vector>; + const Passes& getPasses() const { return _passes; } private: - PassManager() = default; - ~PassManager() = default; - // data Passes _passes; // registered passes }; diff --git a/contrib/nnc/include/passes/acl_soft_backend/AclCppGenerator.h b/contrib/nnc/include/passes/acl_soft_backend/AclCppGenerator.h index 795e0e2..736c7e4 100644 --- a/contrib/nnc/include/passes/acl_soft_backend/AclCppGenerator.h +++ b/contrib/nnc/include/passes/acl_soft_backend/AclCppGenerator.h @@ -19,6 +19,7 @@ #include "pass/Pass.h" + namespace nnc { /** @@ -27,10 +28,6 @@ namespace nnc { */ class AclCppCodeGenerator: public Pass { public: - static Pass& getInstance(); - -public: - AclCppCodeGenerator(); /** * @brief Method represents the generation sequence: analysis, serialization, * header/code generation, etc @@ -38,6 +35,7 @@ public: * @return returns empty PassData object */ PassData run(PassData data) override; + }; } // namespace nnc diff --git a/contrib/nnc/include/passes/caffe_frontend/CaffeFrontend.h b/contrib/nnc/include/passes/caffe_frontend/CaffeFrontend.h index 13e6c54..c35e28b 100644 --- a/contrib/nnc/include/passes/caffe_frontend/CaffeFrontend.h +++ b/contrib/nnc/include/passes/caffe_frontend/CaffeFrontend.h @@ -28,14 +28,8 @@ namespace nnc { */ class CaffeFrontend : public Pass { public: - CaffeFrontend &operator=(const CaffeFrontend &) = delete; - CaffeFrontend(const CaffeFrontend &) = delete; + PassData run(PassData) override; - CaffeFrontend() = default; - ~CaffeFrontend() override = default; - - static Pass &getInstance(); - PassData run(PassData data) override; }; } // namespace nnc diff --git a/contrib/nnc/include/passes/interpreter/InterpreterPass.h b/contrib/nnc/include/passes/interpreter/InterpreterPass.h index 2734343..4a56b77 100644 --- a/contrib/nnc/include/passes/interpreter/InterpreterPass.h +++ b/contrib/nnc/include/passes/interpreter/InterpreterPass.h @@ -23,17 +23,14 @@ #include "pass/Pass.h" #include "pass/PassData.h" -namespace nnc -{ +namespace nnc { -class InterpreterPass : public Pass -{ +class InterpreterPass : public Pass { public: - static Pass &getInstance(); - PassData run(PassData data) override; + ~InterpreterPass() override; - virtual ~InterpreterPass(); + PassData run(PassData data) override; private: nnc::mir::TensorVariant loadInput(const nnc::mir::Shape &); diff --git a/contrib/nnc/include/passes/soft_backend/BaseGenerator.h b/contrib/nnc/include/passes/soft_backend/BaseGenerator.h index 36084f1..452d238 100644 --- a/contrib/nnc/include/passes/soft_backend/BaseGenerator.h +++ b/contrib/nnc/include/passes/soft_backend/BaseGenerator.h @@ -25,8 +25,7 @@ #include -namespace nnc -{ +namespace nnc { class ModelAnalyzer; @@ -41,8 +40,7 @@ class Serializer; * + code file generation function: materuializeCode * + header file generation to expose artifact inerface to user */ -class BaseCodeGenerator : public Pass -{ +class BaseCodeGenerator : public Pass { public: /** * @brief Method represents base generation sequence: analysis, serialization, header/code generation, etc diff --git a/contrib/nnc/include/passes/soft_backend/CPPGenerator.h b/contrib/nnc/include/passes/soft_backend/CPPGenerator.h index 5920cd5..f704937 100644 --- a/contrib/nnc/include/passes/soft_backend/CPPGenerator.h +++ b/contrib/nnc/include/passes/soft_backend/CPPGenerator.h @@ -20,8 +20,7 @@ #include "passes/soft_backend/BaseGenerator.h" #include "pass/Pass.h" -namespace nnc -{ +namespace nnc { class TensorDescription; @@ -29,14 +28,10 @@ class TensorDescription; * @brief CPPCodeGenerator implements interfaces that provides BaseCodeGenerator for C++ language * This includes header file generation, code file generation and variable renaming according to C++ naming requirements */ -class CPPCodeGenerator: public BaseCodeGenerator -{ -public: - static Pass &getInstance(); - +class CPPCodeGenerator: public BaseCodeGenerator { protected: - void formatTensorNames(const ModelAnalyzer &ma) override; - void materializeHeader(std::ostream &out, const ModelAnalyzer &ma) override; + void formatTensorNames(const ModelAnalyzer& ma) override; + void materializeHeader(std::ostream& out, const ModelAnalyzer& ma) override; /** * @brief Form list of function call arguments @@ -44,9 +39,9 @@ protected: * @param argIds List of argument variable ids * @param args Result list of arguments transformed in form of strings */ - void gatherOperationArguments(const ModelAnalyzer &ma, - const std::vector &argIds, - std::vector &args); + void gatherOperationArguments(const ModelAnalyzer& ma, + const std::vector& argIds, + std::vector& args); /** * @brief Prints setter of artifact * @param out Output stream @@ -54,8 +49,8 @@ protected: * @param setterName Name of setter function * @param varId id of variable that setter fills */ - void printSetter(std::ostream &out, const std::string &className, - const std::string &setterName, const TensorDescription &td); + void printSetter(std::ostream& out, const std::string& className, + const std::string& setterName, const TensorDescription& td); /** * @brief Prints getters of artifact * @param out Output stream @@ -63,18 +58,16 @@ protected: * @param setterName Name of setter function * @param varId id of variable that getter returns */ - void printGetter(std::ostream &out, const std::string &className, - const std::string &setterName, const TensorDescription &td); + void printGetter(std::ostream& out, const std::string& className, + const std::string& setterName, const TensorDescription& td); /** * @brief Prints inference sequence placed in doInference method of artifact * @param out Output stream * @param ma Intermediate model representation */ - void materializeInferenceSequence(std::ostream &out, const ModelAnalyzer &ma); - void materializeCode(std::ostream &out, const ModelAnalyzer &ma, const Serializer &s) override; + void materializeInferenceSequence(std::ostream& out, const ModelAnalyzer& ma); + void materializeCode(std::ostream& out, const ModelAnalyzer& ma, const Serializer& s) override; -private: - CPPCodeGenerator(): BaseCodeGenerator() {} }; } // namespace nnc diff --git a/contrib/nnc/include/passes/tflite_frontend/TfliteFrontend.h b/contrib/nnc/include/passes/tflite_frontend/TfliteFrontend.h index 97549ad..894fa00 100644 --- a/contrib/nnc/include/passes/tflite_frontend/TfliteFrontend.h +++ b/contrib/nnc/include/passes/tflite_frontend/TfliteFrontend.h @@ -21,29 +21,17 @@ #include "pass/PassData.h" -namespace nnc -{ -namespace tflite -{ +namespace nnc { /** - * @brief class represent frontend of tensor flow lite NN framework + * @brief class represent frontend of tensorflow lite NN framework */ -class TFLiteFrontend : public Pass -{ +class TFLiteFrontend : public Pass { public: - TFLiteFrontend &operator=(const TFLiteFrontend &) = delete; - TFLiteFrontend(const TFLiteFrontend &) = delete; + PassData run(PassData) override; - static Pass &getInstance(); - PassData run(PassData data) override; - -private: - TFLiteFrontend() = default; - ~TFLiteFrontend() override = default; }; -} // namespace tflite } // namespace nnc #endif //NNCC_TFLITEFRONTEND_H diff --git a/contrib/nnc/pass/PassManager.cpp b/contrib/nnc/pass/PassManager.cpp index d85f89b..677e67c 100644 --- a/contrib/nnc/pass/PassManager.cpp +++ b/contrib/nnc/pass/PassManager.cpp @@ -15,22 +15,18 @@ */ #include "pass/PassManager.h" +#include "pass/Pass.h" namespace nnc { -PassManager *PassManager::getPassManager() -{ - static PassManager passManager; - - return &passManager; - -} // getPassManager +PassManager::PassManager() {} +PassManager::~PassManager() {} -void PassManager::registerPass(Pass *pass) +void PassManager::registerPass(std::unique_ptr pass) { - _passes.push_back(pass); + _passes.push_back(std::move(pass)); } // registerPass diff --git a/contrib/nnc/passes/acl_soft_backend/AclCppGenerator.cpp b/contrib/nnc/passes/acl_soft_backend/AclCppGenerator.cpp index c83a703..d07c227 100644 --- a/contrib/nnc/passes/acl_soft_backend/AclCppGenerator.cpp +++ b/contrib/nnc/passes/acl_soft_backend/AclCppGenerator.cpp @@ -30,8 +30,6 @@ namespace nnc { using namespace std; -AclCppCodeGenerator::AclCppCodeGenerator() {} - PassData AclCppCodeGenerator::run(PassData data) { mir::Graph* g = data; assert(g); @@ -79,9 +77,4 @@ PassData AclCppCodeGenerator::run(PassData data) { return nullptr; } -Pass& AclCppCodeGenerator::getInstance() { - static AclCppCodeGenerator acl_cpp_code_generator; - return acl_cpp_code_generator; -} - } // namespace nnc diff --git a/contrib/nnc/passes/caffe_frontend/caffe_frontend.cpp b/contrib/nnc/passes/caffe_frontend/caffe_frontend.cpp index f511301..6291900 100644 --- a/contrib/nnc/passes/caffe_frontend/caffe_frontend.cpp +++ b/contrib/nnc/passes/caffe_frontend/caffe_frontend.cpp @@ -27,13 +27,7 @@ namespace nnc { -Pass &CaffeFrontend::getInstance() { - static CaffeFrontend instance; - return instance; -} - -PassData CaffeFrontend::run(PassData data) { - (void)data; +PassData CaffeFrontend::run(PassData) { nnc::CaffeImporter importer{cli::inputFile}; importer.import(); diff --git a/contrib/nnc/passes/interpreter/interpreter_pass.cpp b/contrib/nnc/passes/interpreter/interpreter_pass.cpp index 358c112..7f9d71d 100644 --- a/contrib/nnc/passes/interpreter/interpreter_pass.cpp +++ b/contrib/nnc/passes/interpreter/interpreter_pass.cpp @@ -48,11 +48,6 @@ namespace nnc using namespace mir; -Pass &InterpreterPass::getInstance() { - static InterpreterPass instance; - return instance; -} - #ifdef NNC_HDF5_SUPPORTED /** * @brief save tensor in file in '.hdf5' format diff --git a/contrib/nnc/passes/soft_backend/CPPGenerator.cpp b/contrib/nnc/passes/soft_backend/CPPGenerator.cpp index 211f10e..b70c855 100644 --- a/contrib/nnc/passes/soft_backend/CPPGenerator.cpp +++ b/contrib/nnc/passes/soft_backend/CPPGenerator.cpp @@ -338,10 +338,4 @@ void CPPCodeGenerator::materializeCode(ostream &out, const ModelAnalyzer &ma, co out << "}"; } -Pass &CPPCodeGenerator::getInstance() -{ - static CPPCodeGenerator cppCodeGenerator; - return cppCodeGenerator; -} - } // namespace nnc diff --git a/contrib/nnc/passes/tflite_frontend/tflite_frontend.cpp b/contrib/nnc/passes/tflite_frontend/tflite_frontend.cpp index a66a33d..b0c1c21 100644 --- a/contrib/nnc/passes/tflite_frontend/tflite_frontend.cpp +++ b/contrib/nnc/passes/tflite_frontend/tflite_frontend.cpp @@ -18,7 +18,6 @@ #include #include -#include "pass/Pass.h" #include "pass/PassException.h" #include "passes/tflite_frontend/TfliteFrontend.h" #include "option/Options.h" @@ -26,25 +25,14 @@ #include "tflite_importer.h" -namespace nnc -{ -namespace tflite -{ +namespace nnc { -Pass &TFLiteFrontend::getInstance() -{ - static TFLiteFrontend instance; - return instance; -} - -PassData TFLiteFrontend::run(PassData data) -{ +PassData TFLiteFrontend::run(PassData) { nnc::tflite::v_dev::TfliteImporter importer{cli::inputFile}; importer.import(); - return reinterpret_cast(importer.createIR()); + return reinterpret_cast(importer.createIR()); } -} // namespace tflite } // namespace nnc diff --git a/contrib/nnc/tests/soft_backend/CompileCPP.cpp b/contrib/nnc/tests/soft_backend/CompileCPP.cpp index e91950a..1c1ba68 100644 --- a/contrib/nnc/tests/soft_backend/CompileCPP.cpp +++ b/contrib/nnc/tests/soft_backend/CompileCPP.cpp @@ -94,7 +94,8 @@ int main(int argc, const char *argv[]) Graph g; fillGraph(g); - nnc::CPPCodeGenerator::getInstance().run(&g); + nnc::CPPCodeGenerator cppCodeGenerator; + cppCodeGenerator.run(&g); string basePath = outputDir + "/" + artifactName; diff --git a/contrib/nnc/unittests/soft_backend/Generator.cpp b/contrib/nnc/unittests/soft_backend/Generator.cpp index b821363..6189920 100644 --- a/contrib/nnc/unittests/soft_backend/Generator.cpp +++ b/contrib/nnc/unittests/soft_backend/Generator.cpp @@ -98,14 +98,15 @@ TEST(Generator, check_generator_call) deleteDir(TEST_DIR); } assert(!isFileExists(TEST_DIR) && "remove output dir"); - CPPCodeGenerator::getInstance().run(&g); + CPPCodeGenerator cpp_code_generator; + cpp_code_generator.run(&g); checkOutputExists(BASE_NAME); // test that generator creates output files in existing empty dir deleteFile(BASE_NAME ".h"); deleteFile(BASE_NAME ".cpp"); deleteFile(BASE_NAME ".params"); - CPPCodeGenerator::getInstance().run(&g); + cpp_code_generator.run(&g); checkOutputExists(BASE_NAME); // test that generator rewrites existing files @@ -115,7 +116,7 @@ TEST(Generator, check_generator_call) assert(res == 0); (void)res; assert(sBefore.st_size == 0); - CPPCodeGenerator::getInstance().run(&g); + cpp_code_generator.run(&g); res = stat(BASE_NAME ".h", &sAfter); assert(res == 0); -- 2.7.4