Feature/pass manager #11440
Feature/pass manager #11440
Conversation
Superjomn
commented
Jun 13, 2018
•
Superjomn
commented
- add class Argument to unify the data passed across all the Passes and PassManagers
- add PassManager test
- simplify CMAKE
… feature/pass-manager
| auto program = Load(&executor, &scope, model_dir); | ||
| return *program->Proto(); | ||
| std::string msg; | ||
| std::string net_file = FLAGS_inference_model_dir + "/__model__"; |
There was a problem hiding this comment.
Duplicated code to load content from a file with https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/fluid/inference/io.cc#L47
There was a problem hiding this comment.
yes, this is copied from there. But I found it depends on a lot of fluid core libraries, and those are really heavy to compile. The analysis module just analyze the graph and no need to depend on any runtime modules, so just copied a simple method here, just several lines.
| }; | ||
|
|
||
| #define ANALYSIS_ARGUMENT_CHECK_FIELD(field__) \ | ||
| if (!(field__)) { \ |
There was a problem hiding this comment.
Because most enforce conditions would evaluate to true, we can use __builtin_expect to instruct the C++ compiler to generate code that always forces branch prediction of true.
https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/fluid/platform/enforce.h#L108
| default: | ||
| PADDLE_THROW("unsupported Node type %d", static_cast<int>(node.type())); | ||
| } | ||
| dot.AddNode(node.repr(), node.dot_attrs()); |
There was a problem hiding this comment.
DotString is not a good name.
Maybe DotGraphgString or String is better?
There was a problem hiding this comment.
there is a Graph word in DataFlowGraph::DotString, I think DotString is more clear.
| LOG(INFO) << "draw dot to " << GenDotPath(); | ||
| } | ||
|
|
||
| std::string DFG_GraphvizDrawPass::Draw(DataFlowGraph *graph) { |
There was a problem hiding this comment.
Const DataFlowGraph&
https://google.github.io/styleguide/cppguide.html#Reference_Arguments
There was a problem hiding this comment.
For simplicity, each pass takes a DataFlowGraph* as input, I will enhance this and protect the read operation in another PR latter.
| bool Initialize() override; | ||
| bool Initialize(const framework::proto::ProgramDesc &desc) override; | ||
| FluidToDataFlowGraphPass() = default; | ||
| // bool Initialize(const framework::proto::ProgramDesc &desc) override; |
| @@ -109,6 +110,11 @@ class OrderedRegistry { | |||
| std::vector<std::unique_ptr<T>> data_; | |||
| }; | |||
|
|
|||
| template <typename T, typename... Args> | |||
| std::unique_ptr<T> make_unique(Args &&... args) { | |||
|
|
||
| void SetName(const std::string &name) { name_ = name; } | ||
| const std::string &name() const { return name_; } | ||
|
|
||
| void SetType(Type type) { type_ = type; } | ||
| Type type() const { return type_; } | ||
|
|
||
| void *extra_info() const { return extra_info_; } | ||
| void SetExtraInfo(void *extra_info) { extra_info_ = extra_info; } | ||
|
|
||
| // Input links. | ||
| std::vector<Node *> inlinks; |
There was a problem hiding this comment.
Class members should end with _!
There was a problem hiding this comment.
this is a public member, better not be exposed with _.
| // Virtual method overridden by subclasses to do only necessary initialization | ||
| // before any pass is run. | ||
| virtual bool Initialize() { return false; } | ||
| // virtual bool Initialize() { return false; } |
There was a problem hiding this comment.
This region should be cleaned up!
|
|
||
| framework::proto::ProgramDesc desc; | ||
| Argument argument; |
There was a problem hiding this comment.
This is a public argument that will be used by all the other UTs.
| @@ -149,7 +151,7 @@ class Node { | |||
| // Mark this node is deleted by some pass. | |||
| bool deleted_{false}; | |||
|
|
|||
| void *extra_info_; | |||
| // void *extra_info_; | |||
