Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PT FE] Fix coverity issues #20743

Merged
merged 1 commit into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/frontends/pytorch/src/input_model.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Place;
class TorchDecoder;

struct PlaceDesc {
PlaceDesc(std::shared_ptr<Node> value) : m_value(value) {}
PlaceDesc(const std::shared_ptr<Node>& value) : m_value(value) {}
std::shared_ptr<Node> m_value;
};

Expand Down
3 changes: 2 additions & 1 deletion src/frontends/pytorch/src/op/avg_poolnd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ OutputVector translate_avg_poolnd(const NodeContext& context) {
auto pads_len = context.mark_node(v0::Constant::create(element::i32, Shape{}, {pads.size()}));
auto pads_diff = context.mark_node(std::make_shared<v1::Subtract>(rank, pads_len));
auto pads_remaining = context.mark_node(std::make_shared<v3::Broadcast>(zero_i32, pads_diff));
auto padding = context.mark_node(std::make_shared<v0::Concat>(OutputVector{pads_remaining, pad_values}, 0));
auto padding = context.mark_node(
std::make_shared<v0::Concat>(OutputVector{std::move(pads_remaining), std::move(pad_values)}, 0));
input = context.mark_node(std::make_shared<v1::Pad>(input, padding, padding, zero, ov::op::PadMode::CONSTANT));
pads = Shape(pads.size(), 0);
}
Expand Down
2 changes: 1 addition & 1 deletion src/frontends/pytorch/src/op/max_poolnd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ OutputVector translate_max_poolnd(const NodeContext& context) {
if (context.get_output_size() == 2) {
auto out1 = res->output(0);
auto out2 = res->output(1);
return {out1, out2};
return {std::move(out1), std::move(out2)};
} else {
return {res};
}
Expand Down
3 changes: 2 additions & 1 deletion src/frontends/pytorch/src/op/pixel_shuffle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ OutputVector translate_channel_shuffle(const NodeContext& context) {
auto k = context.mark_node(std::make_shared<v1::Divide>(c, groups, true));
auto g = context.mark_node(std::make_shared<v0::Unsqueeze>(groups, zero));
// 1. Reshape input [N, G, K=C/G, -1]
auto reshape_indices = context.mark_node(std::make_shared<v0::Concat>(OutputVector{n, g, k, neg_1}, 0));
auto reshape_indices = context.mark_node(
std::make_shared<v0::Concat>(OutputVector{std::move(n), std::move(g), std::move(k), std::move(neg_1)}, 0));
x = context.mark_node(std::make_shared<v1::Reshape>(x, reshape_indices, false));
// 2. Transpose to [N, K, G, -1]
auto permute_indices = context.mark_node(v0::Constant::create(element::i32, Shape{4}, {0, 2, 1, 3}));
Expand Down
2 changes: 1 addition & 1 deletion src/frontends/pytorch/src/op/pythonop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ OutputVector translate_pythonop(const NodeContext& context) {
}

OutputVector outputs{};
for (auto result : body->get_results()) {
for (auto& result : body->get_results()) {
auto output = result->get_input_source_output(0);
outputs.push_back(context.mark_output(output));
}
Expand Down
12 changes: 8 additions & 4 deletions src/frontends/pytorch/src/op/scatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ namespace op {
using namespace ov::op;

namespace {
Output<Node> prepare_source(const NodeContext& context, Output<Node> src, Output<Node> index, Output<Node> input) {
Output<Node> prepare_source(const NodeContext& context,
const Output<Node>& src,
const Output<Node>& index,
const Output<Node>& input) {
auto src_partial_shape = src.get_partial_shape();
auto index_shape_rank = get_shape_rank(context, index);
auto index_shape = std::get<0>(index_shape_rank);
Expand All @@ -28,8 +31,9 @@ Output<Node> prepare_source(const NodeContext& context, Output<Node> src, Output
// into shape of indices.
// TODO: Figure out way to dynamically broadcast scalar src only, without affecting Tensor src. Current
// implementation will fail if Scalar source would have dynamic rank.
auto _src = std::move(src);
if (src_partial_shape.rank().is_static() && src_partial_shape.rank().get_length() == 0) {
src = context.mark_node(std::make_shared<v3::Broadcast>(src, index_shape));
_src = context.mark_node(std::make_shared<v3::Broadcast>(_src, index_shape));
}

auto const_0 = context.mark_node(v0::Constant::create(element::i32, Shape{}, {0}));
Expand All @@ -38,13 +42,13 @@ Output<Node> prepare_source(const NodeContext& context, Output<Node> src, Output
auto ones = context.mark_node(std::make_shared<v3::Broadcast>(const_1, index_rank));
// In torch indices can be of different shape than source tensor. Create slice to trim source tensor to shape of
// indices.
auto src_pruned = context.mark_node(std::make_shared<v8::Slice>(src, zeros, index_shape, ones));
auto src_pruned = context.mark_node(std::make_shared<v8::Slice>(_src, zeros, index_shape, ones));

auto src_input_dtype = context.mark_node(std::make_shared<v1::ConvertLike>(src_pruned, input));
return src_input_dtype;
};

const v12::ScatterElementsUpdate::Reduction get_reduction_mode(std::string pt_reduce_mode) {
const v12::ScatterElementsUpdate::Reduction get_reduction_mode(const std::string& pt_reduce_mode) {
static const std::unordered_map<std::string, v12::ScatterElementsUpdate::Reduction> TORCH_REDUCTION_TO_OV{
{"add", v12::ScatterElementsUpdate::Reduction::SUM},
{"multiply", v12::ScatterElementsUpdate::Reduction::PROD},
Expand Down
2 changes: 1 addition & 1 deletion src/frontends/pytorch/src/transforms.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ class DecomposeListResults : public pass::ModelPass {
// Replace a single result with 6 results, per each input of parent list_pack

auto inputs = list_pack->inputs();
for (auto input : inputs) {
for (auto& input : inputs) {
model->add_results({make_shared<opset10::Result>(input.get_source_output())});
// TODO: Keep tracking between original and new Results
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ AppendListUnpackReplacer::AppendListUnpackReplacer() {
auto split = std::make_shared<v1::Split>(inputs[index], axis_0, list_unpack->get_output_size());
NodeVector to_copy_rt{axis_0, split};
OutputVector res;
for (auto output : split->outputs()) {
for (auto& output : split->outputs()) {
auto squeeze = std::make_shared<v0::Squeeze>(output, axis_0);
to_copy_rt.push_back(squeeze);
res.push_back(squeeze);
Expand Down
4 changes: 2 additions & 2 deletions src/frontends/pytorch/src/transforms/aten_cat_replacer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ AtenCatToConcat::AtenCatToConcat() {
auto body = loop->get_function();
auto output_index = cat->input(0).get_source_output().get_index();
int64_t body_result_index = -1;
for (auto out_desc : loop->get_output_descriptions()) {
for (auto& out_desc : loop->get_output_descriptions()) {
if (out_desc->m_output_index == output_index) {
body_result_index = static_cast<int64_t>(out_desc->m_body_value_index);
break;
Expand All @@ -99,7 +99,7 @@ AtenCatToConcat::AtenCatToConcat() {
auto body_param_index = body->get_parameter_index(param);
FRONT_END_GENERAL_CHECK(body_param_index >= 0, "Couldn't find parameter in body parameters.");
int64_t input_index = -1;
for (auto in_desc : loop->get_input_descriptions()) {
for (auto& in_desc : loop->get_input_descriptions()) {
if (in_desc->m_body_parameter_index == static_cast<size_t>(body_param_index)) {
input_index = static_cast<int64_t>(in_desc->m_input_index);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ IndexLoopGetitemReplacer::IndexLoopGetitemReplacer() {

auto body = loop_op->get_function();
std::shared_ptr<Node> chunk_param;
for (auto input_desc : loop_op->get_input_descriptions()) {
for (auto& input_desc : loop_op->get_input_descriptions()) {
if (input_desc->m_input_index == chunk_idx) {
chunk_param = body->get_parameters().at(input_desc->m_body_parameter_index);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,14 +181,14 @@ PrimListUnpackReplacer::PrimListUnpackReplacer() {
return false;
}
Output<Node> final_shape_t = opset10::Constant::create(element::i32, Shape{}, {0});
for (auto input : tensors->inputs()) {
for (auto& input : tensors->inputs()) {
auto tensor_shape = rg.make<opset10::ShapeOf>(input.get_source_output(), element::i32);
final_shape_t =
rg.make<opset10::Broadcast>(final_shape_t, tensor_shape, ov::op::BroadcastType::BIDIRECTIONAL);
}
auto final_shape = rg.make<opset10::ShapeOf>(final_shape_t, element::i32);
OutputVector outputs;
for (auto input : tensors->inputs()) {
for (auto& input : tensors->inputs()) {
outputs.push_back(rg.make<opset10::Broadcast>(input.get_source_output(), final_shape));
}
copy_runtime_info_and_name(list_unpack, rg.get(), {input_node});
Expand All @@ -202,7 +202,7 @@ PrimListUnpackReplacer::PrimListUnpackReplacer() {
const auto num_splits = list_unpack->get_output_size();
auto split = rg.make<opset10::Split>(input, axis, num_splits);
OutputVector outputs;
for (auto output : split->outputs()) {
for (auto& output : split->outputs()) {
const auto squeeze = rg.make<opset10::Squeeze>(output, axis);
outputs.push_back(squeeze);
}
Expand All @@ -218,7 +218,7 @@ PrimListUnpackReplacer::PrimListUnpackReplacer() {
const auto num_splits = list_unpack->get_output_size();
auto split = rg.make<opset10::Split>(non_zero, axis, num_splits);
OutputVector outputs;
for (auto output : split->outputs()) {
for (auto& output : split->outputs()) {
const auto squeeze = rg.make<opset10::Squeeze>(output, axis);
outputs.push_back(squeeze);
}
Expand All @@ -234,7 +234,7 @@ PrimListUnpackReplacer::PrimListUnpackReplacer() {
const auto num_splits = list_unpack->get_output_size();
auto split = rg.make<opset10::Split>(non_zero, axis, num_splits);
OutputVector outputs;
for (auto output : split->outputs()) {
for (auto& output : split->outputs()) {
const auto squeeze = rg.make<opset10::Squeeze>(output, axis);
outputs.push_back(squeeze);
}
Expand Down Expand Up @@ -310,7 +310,7 @@ PrimListUnpackReplacer::PrimListUnpackReplacer() {
auto split = rg.make<opset10::Split>(shape_of, axis_0, list_unpack->get_output_size());

OutputVector res;
for (auto output : split->outputs()) {
for (auto& output : split->outputs()) {
auto squeeze = rg.make<opset10::Squeeze>(output, axis_0);
res.push_back(squeeze);
}
Expand All @@ -328,7 +328,7 @@ PrimListUnpackReplacer::PrimListUnpackReplacer() {
auto split = rg.make<opset10::Split>(slice, axis_0, list_unpack->get_output_size());

OutputVector res;
for (auto output : split->outputs()) {
for (auto& output : split->outputs()) {
auto squeeze = rg.make<opset10::Squeeze>(output, axis_0);
res.push_back(squeeze);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ bool DecomposeTupleParameters::run_on_model(const std::shared_ptr<Model>& model)

auto new_parameter = std::make_shared<ov::op::v0::Parameter>(et, ps);

for (auto input : inputs) {
for (auto& input : inputs) {
auto names = input.get_tensor().get_names();
input.replace_source_output(new_parameter->output(0));
new_parameter->output(0).add_names(names);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ RFFTNComplexReplacer::RFFTNComplexReplacer() {
auto normalized_rfftn_splitted = std::make_shared<v1::Split>(normalized_rfftn, const_neg_1, 2);
auto rfftn_outs = rfftn_op->get_users();
bool rval = false;
for (auto out : rfftn_outs) {
for (auto& out : rfftn_outs) {
if (auto real_op = cast_fw_node(out, "aten::real")) {
auto squeezed = std::make_shared<v0::Squeeze>(normalized_rfftn_splitted->output(0), const_neg_1);
copy_runtime_info({rfftn_op, real_op}, squeezed);
Expand Down
10 changes: 5 additions & 5 deletions src/frontends/pytorch/src/transforms/tuple_unpack_replacer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ PrimTupleUnpackReplacer::PrimTupleUnpackReplacer() {

bool TupleUnpackInBodyReplacer::run_on_model(const std::shared_ptr<Model>& model) {
bool result = false;
for (auto op : model->get_ordered_ops()) {
for (auto& op : model->get_ordered_ops()) {
const auto if_op = as_type_ptr<v8::If>(op);
if (if_op) {
for (size_t i = 1; i < if_op->get_input_size(); i++) {
Expand All @@ -61,7 +61,7 @@ bool TupleUnpackInBodyReplacer::run_on_model(const std::shared_ptr<Model>& model
int else_body_idx = -1;
auto then_descs = if_op->get_input_descriptions(v8::If::THEN_BODY_INDEX);
auto else_descs = if_op->get_input_descriptions(v8::If::ELSE_BODY_INDEX);
for (auto inp_desc : then_descs) {
for (auto& inp_desc : then_descs) {
if (inp_desc->m_input_index == i) {
if (then_body_idx != -1) {
add_exception_to_fw_node(
Expand All @@ -72,7 +72,7 @@ bool TupleUnpackInBodyReplacer::run_on_model(const std::shared_ptr<Model>& model
}
}
}
for (auto inp_desc : else_descs) {
for (auto& inp_desc : else_descs) {
if (inp_desc->m_input_index == i) {
if (else_body_idx != -1) {
add_exception_to_fw_node(
Expand Down Expand Up @@ -130,10 +130,10 @@ bool TupleUnpackInBodyReplacer::run_on_model(const std::shared_ptr<Model>& model

// create new If inputs
std::vector<std::pair<int, int>> inputs_mapping(if_op->get_input_size(), {-1, -1});
for (auto inp_desc : then_descs) {
for (auto& inp_desc : then_descs) {
inputs_mapping[inp_desc->m_input_index].first = static_cast<int>(inp_desc->m_body_parameter_index);
}
for (auto inp_desc : else_descs) {
for (auto& inp_desc : else_descs) {
inputs_mapping[inp_desc->m_input_index].second = static_cast<int>(inp_desc->m_body_parameter_index);
}
for (size_t j = 0; j < inputs_mapping.size(); j++) {
Expand Down
3 changes: 2 additions & 1 deletion src/frontends/pytorch/src/transforms/u4_block_repack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ U4BlockRepack::U4BlockRepack() {
}
}

copy_runtime_info(NodeVector{constant, reshape1, transpose, reshape2}, new_const);
copy_runtime_info({std::move(constant), std::move(reshape1), std::move(transpose), std::move(reshape2)},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaks GPTQ int4 models. Should be reverted. @andrei-kochin

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rkazants FYI

new_const);
replace_node(reshape2, new_const);

return true;
Expand Down
6 changes: 3 additions & 3 deletions src/frontends/pytorch/src/translate_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ std::shared_ptr<Model> TranslateSession::convert_pytorch_model(
if (input_model) {
// When we have input model we should use its inputs order to create Parameters
// We use m_inputs instead of get_inputs() because latter doesn't have "self" input
for (auto input_p : input_model->m_inputs) {
for (auto& input_p : input_model->m_inputs) {
auto pytorch_place = std::dynamic_pointer_cast<pytorch::Place>(input_p);
FRONT_END_GENERAL_CHECK(pytorch_place, "Only place produced by PyTorch Frontend is supported.");
auto tensor_id = pytorch_place->get_tensor_index();
Expand All @@ -108,7 +108,7 @@ std::shared_ptr<Model> TranslateSession::convert_pytorch_model(
(*tensor_map)[tensor_id] = parameter;
}
// Add all tensors that were frozen
for (auto desc : input_model->m_descriptors) {
for (auto& desc : input_model->m_descriptors) {
(*tensor_map)[desc.first] = desc.second.m_value;
}
} else {
Expand Down Expand Up @@ -225,7 +225,7 @@ std::shared_ptr<Model> TranslateSession::convert_pytorch_model(
ResultVector results;
if (input_model) {
// For the case when we have InputModel we need to have same order as its outputs
for (auto output_p : input_model->get_outputs()) {
for (auto& output_p : input_model->get_outputs()) {
auto pytorch_place = std::dynamic_pointer_cast<pytorch::Place>(output_p);
FRONT_END_GENERAL_CHECK(pytorch_place, "Only place produced by PyTorch Frontend is supported.");
auto tensor_id = pytorch_place->get_tensor_index();
Expand Down