fix code check problem

update code review comments
This commit is contained in:
Zichun Ye 2022-03-17 14:21:55 +08:00
parent a25c5b46aa
commit b81d6bc68f
11 changed files with 79 additions and 69 deletions

View File

@ -20,12 +20,14 @@ namespace mindspore::graphkernel::inner {
namespace {
class OpRegister {
public:
OpRegister(const std::string &name, const CreatorFunc &func) { OpRegistry::Instance().Register(name, func); }
OpRegister(const std::string &name, const CreatorFunc &func) : name_(name) {
OpRegistry::Instance().Register(name, func);
}
~OpRegister() = default;
protected:
// for pclint-plus
bool rev_{false};
std::string name_;
};
#define JOIN(x, y) x##y

View File

@ -30,7 +30,7 @@ class OpRegistry {
static OpRegistry instance{};
return instance;
}
void Register(const std::string &op_name, const CreatorFunc &func) { creators.insert({op_name, func}); }
void Register(const std::string &op_name, const CreatorFunc &func) { (void)creators.emplace(op_name, func); }
PrimOpPtr NewOp(const std::string &op) {
// "OpaqueOp" is registered by default.

View File

@ -107,7 +107,7 @@ void KeepExecOrder(const FuncGraphPtr &func_graph, const AnfNodePtr &getitem, co
load_node->set_abstract(assign_to_node->abstract());
func_graph->AddNode(load_node);
mng->Replace(getitem, load_node);
(void)mng->Replace(getitem, load_node);
}
int64_t GetitemIndex(const AnfNodePtr &getitem) {
@ -131,7 +131,7 @@ void UpdateUsersOfGraphKernel(const FuncGraphPtr &func_graph, const AnfNodePtr &
// 2. If the `cnode` has another path to the getitem_user, it's unnecessary to add update_state and load node to
// keep exec_order.
if (HasPathToParamUser(cnode, getitem_user, getitem)) {
mng->Replace(getitem, assign_to);
(void)mng->Replace(getitem, assign_to);
continue;
}
KeepExecOrder(func_graph, getitem, assign_to, mng);
@ -180,7 +180,7 @@ bool ReplaceAssignByInplaceAssignInGraphkernel(const FuncGraphPtr &func_graph) {
std::vector<TypeId> output_types = {input_types.back()};
auto graph_sel_info = BuildSelectKernelBuildInfo(input_formats, input_types, output_formats, output_types, cnode);
AnfAlgo::SetSelectKernelBuildInfo(graph_sel_info, new_cnode.get());
mng->Replace(cnode, new_cnode);
(void)mng->Replace(cnode, new_cnode);
}
return changed;
}

View File

@ -49,8 +49,8 @@ std::tuple<std::vector<DimInfoPtr>, int, FusionInfoPtr> ParallelCostModel::CalFu
const AnfNodePtrList &nodes) const {
nlohmann::json json_desc;
std::vector<AnfNodePtrList> graphs;
std::transform(nodes.begin(), nodes.end(), std::back_inserter(graphs),
[](const AnfNodePtr &node) -> AnfNodePtrList { return {node}; });
(void)std::transform(nodes.begin(), nodes.end(), std::back_inserter(graphs),
[](const AnfNodePtr &node) -> AnfNodePtrList { return {node}; });
DumpOption dump_option;
if (!AnfToJsonDesc(graphs, dump_option, &json_desc)) {
MS_LOG(EXCEPTION) << "Collect json desc failed.";

View File

@ -107,7 +107,7 @@ using ParallelCostModelPtr = std::shared_ptr<ParallelCostModel>;
class ParellelCostModelWarehouse {
public:
static ParellelCostModelWarehouse &Instance() {
static ParellelCostModelWarehouse instance;
static ParellelCostModelWarehouse instance = ParellelCostModelWarehouse();
return instance;
}
ParallelCostModelPtr GetParallelCostModel(const std::string &target) const;

View File

@ -40,7 +40,7 @@ bool IsOneOf(const AnfNodePtr &node, const std::vector<PrimitivePtr> &ops_prim)
}
void ProcessThroughPassCNode(const std::function<bool(const AnfNodePtr &)> &pass_fn,
OrderedMap<AnfNodePtr, NodeRelation> *const node_rels) {
OrderedMap<AnfNodePtr, NodeRelation> *node_rels) {
std::set<AnfNodePtr> latter_to_be_erased;
for (const auto &[node, node_rel] : (*node_rels)) {
if (!pass_fn(node) || latter_to_be_erased.count(node) != 0) {
@ -62,35 +62,35 @@ void ProcessThroughPassCNode(const std::function<bool(const AnfNodePtr &)> &pass
continue;
}
latter_to_be_erased.insert(cur_node);
(void)latter_to_be_erased.insert(cur_node);
auto predecessors = (*node_rels)[cur_node].pres;
if (predecessors.empty()) {
continue;
}
for (const auto &pre_node : predecessors) {
(*node_rels)[cur_node].pres.erase(pre_node);
(*node_rels)[pre_node].nexts.erase(cur_node);
(void)(*node_rels)[cur_node].pres.erase(pre_node);
(void)(*node_rels)[pre_node].nexts.erase(cur_node);
node_que.push(pre_node);
}
}
// Modify the relation: delete node <-> next_node, add pre node <-> next_node.
for (const auto &next_node : nexts) {
(*node_rels)[next_node].pres.erase(node);
(void)(*node_rels)[next_node].pres.erase(node);
for (const auto &cur_node : pre_nodes) {
(*node_rels)[next_node].pres.insert(cur_node);
(*node_rels)[cur_node].nexts.insert(next_node);
(void)(*node_rels)[next_node].pres.insert(cur_node);
(void)(*node_rels)[cur_node].nexts.insert(next_node);
}
}
}
for (const auto &node : latter_to_be_erased) {
node_rels->erase(node);
(void)node_rels->erase(node);
}
}
void ProcessTailMakeTupleCNode(OrderedMap<AnfNodePtr, NodeRelation> *const node_rels) {
void ProcessTailMakeTupleCNode(OrderedMap<AnfNodePtr, NodeRelation> *node_rels) {
AnfNodePtrList latter_to_be_erased;
for (auto &[node, node_rel] : (*node_rels)) {
if (!IsPrimitiveCNode(node, prim::kPrimMakeTuple)) {
@ -123,15 +123,15 @@ void ProcessTailMakeTupleCNode(OrderedMap<AnfNodePtr, NodeRelation> *const node_
// Delete Tail MakeTuple(including its getitem nodes).
for (const auto &node : latter_to_be_erased) {
for (auto &pre : (*node_rels)[node].pres) {
(*node_rels)[pre].nexts.erase(node);
(void)(*node_rels)[pre].nexts.erase(node);
}
// Tail MakeTuple is just be consumed by nothing or invalid getitem node.
for (auto &getitem : (*node_rels)[node].nexts) {
node_rels->erase(getitem);
(void)node_rels->erase(getitem);
}
node_rels->erase(node);
(void)node_rels->erase(node);
}
}
@ -177,8 +177,8 @@ bool IsNoOutputsNode(const OrderedMap<AnfNodePtr, NodeRelation> &node_rels, cons
return false;
}
void ProcessLocalStructure(OrderedMap<AnfNodePtr, NodeRelation> *node_rels,
std::set<AnfNodePtr> *const virtual_noout_nodes, std::set<AnfNodePtr> *ignore_noin_nodes) {
void ProcessLocalStructure(OrderedMap<AnfNodePtr, NodeRelation> *node_rels, std::set<AnfNodePtr> *virtual_noout_nodes,
std::set<AnfNodePtr> *ignore_noin_nodes) {
// 1. Local relation
// Graph as following left part, relation D->B and D->E(D is a no input node)
// will make B and E to be multiply inputs node.
@ -218,16 +218,16 @@ void ProcessLocalStructure(OrderedMap<AnfNodePtr, NodeRelation> *node_rels,
serial_tail = cur_node;
cur_node = *((*node_rels)[cur_node].nexts.begin());
}
latter_delete.emplace_back(serial_tail, cur_node);
(void)latter_delete.emplace_back(serial_tail, cur_node);
}
}
// Delete relation.
for (const auto &[serial_tail, cur_node] : latter_delete) {
virtual_noout_nodes->insert(serial_tail);
ignore_noin_nodes->insert(cur_node);
(*node_rels)[serial_tail].nexts.erase(cur_node);
(*node_rels)[cur_node].pres.erase(serial_tail);
(void)virtual_noout_nodes->insert(serial_tail);
(void)ignore_noin_nodes->insert(cur_node);
(void)(*node_rels)[serial_tail].nexts.erase(cur_node);
(void)(*node_rels)[cur_node].pres.erase(serial_tail);
MS_LOG(INFO) << "Process local relation delete relation: " << serial_tail->fullname_with_scope() << " -> "
<< cur_node->fullname_with_scope();
}
@ -302,7 +302,7 @@ bool Parallelizable(const AnfNodePtr &node) { return WhiteOpsFilter(node) && !Un
std::vector<AnfNodePtrList> SearchFromNodes(const AnfNodePtrList &nodes,
const std::function<bool(const AnfNodePtr &)> &filter_func,
const OrderedMap<AnfNodePtr, NodeRelation> &node_rels, bool is_backward,
std::set<AnfNodePtr> *const seen) {
std::set<AnfNodePtr> *seen) {
// Start from multi-inputs node, stop on seen node or multi-inputs or multi-outputs nodes.
// For backward search, the other multi-inputs node can be contained in.
// For forward search, the other multi-outputs node can be contained in.
@ -319,12 +319,12 @@ std::vector<AnfNodePtrList> SearchFromNodes(const AnfNodePtrList &nodes,
iter = node_rels.find(n)) {
if (filter_func(n)) {
stream.push_back(n);
seen->insert(n);
(void)seen->insert(n);
}
if (get_contain_node_set(iter->second).size() != 1) {
break;
}
n = *(get_contain_node_set(iter->second).begin());
n = *(get_contain_node_set(iter->second).cbegin());
}
if (stream.size() > 0) {
group.push_back(stream);
@ -333,7 +333,7 @@ std::vector<AnfNodePtrList> SearchFromNodes(const AnfNodePtrList &nodes,
if (group.size() == 1) {
for (const auto &drop : group[0]) {
seen->erase(drop);
(void)seen->erase(drop);
}
group.clear();
}
@ -343,8 +343,7 @@ std::vector<AnfNodePtrList> SearchFromNodes(const AnfNodePtrList &nodes,
void SearchStreamFromMultiRelationNode(const AnfNodePtrList &multi_nodes,
const OrderedMap<AnfNodePtr, NodeRelation> &node_rels, bool is_backward,
std::vector<std::vector<AnfNodePtrList>> *groups,
std::set<AnfNodePtr> *const seen) {
std::vector<std::vector<AnfNodePtrList>> *groups, std::set<AnfNodePtr> *seen) {
auto get_related_nodes = is_backward ? [](const NodeRelation &info) { return info.pres; }
: [](const NodeRelation &info) { return info.nexts; };
for (const auto &node : multi_nodes) {
@ -367,8 +366,7 @@ void SearchStreamFromMultiRelationNode(const AnfNodePtrList &multi_nodes,
void SearchStreamFromUnidirectionalNode(const AnfNodePtrList &ud_nodes,
const OrderedMap<AnfNodePtr, NodeRelation> &node_rels, bool is_backward,
std::vector<std::vector<AnfNodePtrList>> *groups,
std::set<AnfNodePtr> *const seen) {
std::vector<std::vector<AnfNodePtrList>> *groups, std::set<AnfNodePtr> *seen) {
groups->push_back(SearchFromNodes(ud_nodes, Parallelizable, node_rels, is_backward, seen));
// Erase empty groups.
@ -426,18 +424,15 @@ inline bool ParameterLimit(const AnfNodePtrList &nodes) {
}
bool res = true;
switch (AnfAlgo::GetProcessor(nodes[0])) {
case kernel::Processor::CUDA: {
// The number of inputs and outputs for a valid kernel should be less than cuda's limit.
size_t para_count = 0;
for (const auto &node : nodes) {
para_count += common::AnfAlgo::GetInputTensorNum(node);
para_count += common::AnfAlgo::GetOutputTensorNum(node);
}
res = para_count <= CUDA_PARA_LIMIT;
} break;
default:
break;
auto processor_type = AnfAlgo::GetProcessor(nodes[0]);
if (processor_type == kernel::Processor::CUDA) {
// The number of inputs and outputs for a valid kernel should be less than cuda's limit.
size_t para_count = 0;
for (const auto &node : nodes) {
para_count += common::AnfAlgo::GetInputTensorNum(node);
para_count += common::AnfAlgo::GetOutputTensorNum(node);
}
res = para_count <= CUDA_PARA_LIMIT;
}
return res;
@ -467,8 +462,8 @@ OrderedMap<AnfNodePtr, NodeRelation> ParallelOpFusion::GenAnalysisGraph(const An
continue;
}
auto behind_node = get_info(input);
prior_node->pres.insert(input);
behind_node->nexts.insert(node);
(void)prior_node->pres.insert(input);
(void)behind_node->nexts.insert(node);
}
}
@ -550,8 +545,8 @@ std::tuple<std::vector<bool>, std::vector<ParallelInfo>> ParallelOpFusion::DoSea
std::vector<ParallelInfo> parallel_infos;
std::vector<bool> origin_candidates_used(origin_size, false);
std::vector<bool> sorted_candidates_used(candidates.size(), false);
for (size_t i = 0; i < candidates.size(); ++i) {
size_t i = 0;
while (i < candidates.size()) {
if (sorted_candidates_used[i]) {
continue;
}
@ -572,7 +567,9 @@ std::tuple<std::vector<bool>, std::vector<ParallelInfo>> ParallelOpFusion::DoSea
while (begin <= end) {
size_t mid = (begin + end) / 2;
std::vector<size_t> tc(mid);
std::iota(tc.begin(), tc.end(), 1);
for (size_t idx = 0; idx < mid; idx++) {
tc[idx] = idx + 1;
}
AnfNodePtrList other_candidates;
std::tie(other_candidates, std::ignore) =
GetAvaliableNodesByOffset(SizeToInt(i), tc, sorted_candidates_used, candidates, std::set<int>());
@ -589,7 +586,9 @@ std::tuple<std::vector<bool>, std::vector<ParallelInfo>> ParallelOpFusion::DoSea
if (begin > 1) {
std::vector<size_t> tc(begin - 1);
std::iota(tc.begin(), tc.end(), 1);
for (size_t idx = 0; idx < begin - 1; idx++) {
tc[idx] = idx + 1;
}
AnfNodePtrList other_candidates;
std::tie(other_candidates, std::ignore) =
GetAvaliableNodesByOffset(SizeToInt(i), tc, sorted_candidates_used, candidates, std::set<int>());
@ -610,6 +609,7 @@ std::tuple<std::vector<bool>, std::vector<ParallelInfo>> ParallelOpFusion::DoSea
origin_candidates_used[IntToSize(get_index(origin_indices, node))] = true;
}
}
i++;
}
// Current nodes is not suitable to fuse, so pop first node to try other fusion possibility.
@ -626,7 +626,7 @@ std::tuple<std::vector<bool>, std::vector<ParallelInfo>> ParallelOpFusion::Searc
std::vector<size_t> indices;
for (size_t i = 0; i < cs.size(); ++i) {
if (cs[i]) {
(void)origin_indices.emplace(cs[i], i);
origin_indices[cs[i]] = i;
indices.push_back(i);
}
}
@ -646,7 +646,7 @@ std::tuple<std::vector<bool>, std::vector<ParallelInfo>> ParallelOpFusion::Searc
std::map<AnfNodePtr, int> sorted_indices;
for (size_t i = 0; i < candidates.size(); ++i) {
(void)sorted_indices.emplace(candidates[i], i);
sorted_indices[candidates[i]] = i;
}
return DoSearchInSortedCandidates(cs.size(), candidates, &origin_indices, &sorted_indices);
@ -685,8 +685,8 @@ void ParallelOpFusion::SearchFuseNodesInParallelGroup(const std::vector<AnfNodeP
auto candidates = get_candidates();
while (valid_candidate_num(candidates) > 1) {
auto [used, fnds] = SearchFuseNodesInCandidates(candidates);
std::transform(fnds.cbegin(), fnds.cend(), std::back_insert_iterator(*parallel_infos),
[](const ParallelInfo &pi) { return pi; });
(void)std::transform(fnds.cbegin(), fnds.cend(), std::back_insert_iterator(*parallel_infos),
[](const ParallelInfo &pi) { return pi; });
update_tails(used);
candidates = get_candidates();
}

View File

@ -128,7 +128,7 @@ void Area::FuseInput(const AreaPtr &input_area) {
if (iter == inputs_with_relation_.end()) {
MS_LOG(EXCEPTION) << "The area " << input_area->ToString() << " should be the input of area " << this->ToString();
}
auto input_idx = iter - inputs_with_relation_.begin();
auto input_idx = IntToSize(iter - inputs_with_relation_.begin());
if (input_area->is_output_) {
is_output_ = true;

View File

@ -26,6 +26,7 @@ class CircleChecker {
public:
// whether it will form a circle if the two areas are fused.
virtual bool HasCircle(const AreaPtr &a, const AreaPtr &b) const = 0;
virtual ~CircleChecker() = default;
};
using CircleCheckerPtr = std::shared_ptr<CircleChecker>;
@ -50,7 +51,7 @@ class FusePattern {
std::string name() const { return name_; }
FuseDirection direction() const { return direction_; }
std::vector<AreaPtr> &fused_areas() { return fused_areas_; }
std::vector<AreaPtr> fused_areas_;
protected:
void Reset() { fused_areas_.clear(); }
@ -65,7 +66,6 @@ class FusePattern {
}
std::string name_;
std::vector<AreaPtr> fused_areas_;
FuseDirection direction_{FuseDirection::FORWARD};
CircleCheckerPtr circle_checker_{nullptr};
};

View File

@ -59,7 +59,7 @@ void ReachTable::FuseArea(size_t target, size_t other) {
}
}
// discard other_node.
alive_.erase(other);
(void)alive_.erase(other);
}
bool ReachTable::HasCircle(const AreaPtr &a, const AreaPtr &b) const {
@ -115,7 +115,7 @@ void SplitModel::AlignShape(const LiteGraphPtr &litegraph) {
}
if (cur_shape_size > op->shape.size()) {
auto num = cur_shape_size - op->shape.size();
op->shape.insert(op->shape.begin(), num, 1LL);
(void)op->shape.insert(op->shape.begin(), num, 1LL);
}
}
}
@ -150,7 +150,9 @@ void SplitModel::AddPattern(const std::shared_ptr<FusePattern> &pn, bool enable)
void SplitModel::LimitAreaSize(const AreaPtr &dom, std::vector<AreaPtr> *areas, size_t max_size) {
auto dom_size = dom->size();
std::for_each(areas->begin(), areas->end(), [&dom_size](const AreaPtr &a) { dom_size += a->size(); });
for (auto a = areas->begin(); a != areas->end(); ++a) {
dom_size += (*a)->size();
}
if (dom_size <= max_size) {
return;
}
@ -161,7 +163,7 @@ void SplitModel::LimitAreaSize(const AreaPtr &dom, std::vector<AreaPtr> *areas,
cur_size += a->size();
return cur_size > max_size;
});
areas->erase(iter, areas->end());
(void)areas->erase(iter, areas->end());
}
void SplitModel::FuseAreas(const AreaPtr &dom, const std::vector<AreaPtr> &areas, FuseDirection direction) {
@ -195,9 +197,9 @@ bool SplitModel::RunOnePattern(const FusePatternPtr &pattern) {
}
if (pattern->Run(area)) {
MS_LOG(DEBUG) << "Area " << area->ToString() << " matches " << pattern->ToString();
LimitAreaSize(area, &pattern->fused_areas());
if (!pattern->fused_areas().empty()) {
FuseAreas(area, pattern->fused_areas(), pattern->direction());
LimitAreaSize(area, &pattern->fused_areas_);
if (!pattern->fused_areas_.empty()) {
FuseAreas(area, pattern->fused_areas_, pattern->direction());
changed = true;
continue;
}

View File

@ -29,7 +29,7 @@ namespace mindspore::graphkernel::inner {
class ReachTable : public CircleChecker {
public:
explicit ReachTable(size_t size);
~ReachTable() = default;
virtual ~ReachTable() = default;
bool HasCircle(const AreaPtr &a, const AreaPtr &b) const override;
// Link area from `from` to `to`.
@ -51,6 +51,8 @@ class SplitModel {
public:
void Run(const LiteGraphPtr &litegraph);
const std::list<AreaPtr> &areas() const { return areas_; }
SplitModel() = default;
virtual ~SplitModel() = default;
protected:
// transform the litegraph to areas, and initialize inner tables.

View File

@ -20,6 +20,10 @@
namespace mindspore::graphkernel::inner {
class SplitModelCpu : public SplitModel {
public:
SplitModelCpu() = default;
virtual ~SplitModelCpu() = default;
protected:
AreaMode GetDefaultAreaMode(const PrimOpPtr &) const override;
void InitFusePatterns() override;
};