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.

Sign up for GitHub

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

Jump to bottom

[CPU]JIRA 93714 fix CPU plugin warning after remove wd4309 wd4018 #15961

Merged
merged 15 commits into from
Mar 8, 2023
Merged

[CPU]JIRA 93714 fix CPU plugin warning after remove wd4309 wd4018 #15961

merged 15 commits into from
Mar 8, 2023

Conversation

xipingyan
Copy link
Contributor

Details:

  • Remove intel_cpu/CMakeLists.txt ie_add_compiler_flags(/wd4018) and ie_add_compiler_flags(/wd4309)
  • Fix all the corresponding warning of Windows.
  • I also fix some Linux sign-compare issues, but I find there are too many linux issues, new PR will be used to processing left Linux sign-compare issues.

Tickets:

  • 104792

@xipingyan xipingyan requested review from a team as code owners February 27, 2023 01:47
@github-actions github-actions bot added category: build OpenVINO cmake script / infra category: Core OpenVINO Core (aka ngraph) labels Feb 27, 2023
@ilya-lavrenov ilya-lavrenov added this to the 2023.0 milestone Feb 27, 2023
@xipingyan xipingyan requested review from a team as code owners February 27, 2023 08:36
@github-actions github-actions bot added the category: IE Tests OpenVINO Test: plugins and common label Feb 27, 2023
@xipingyan xipingyan force-pushed the xp/jira_93714_fix_warning_after_rm_wd4309_wd4018 branch from 1c35ee6 to bbe7888 Compare February 28, 2023 02:23
xipingyan added 7 commits March 2, 2023 14:31
@xipingyan
Remove warning suppression: wd4018, wd4309
fe0ab8c 
Signed-off-by: Yan, Xiping <[email protected]>
@xipingyan
Remove linux warning suppression no-sign-compare
0c4b89d 
Signed-off-by: Yan, Xiping <[email protected]>
@xipingyan
ov::intel_cpu::VectorDims base value type is size_t;
315bd43 
dnnl::memory::dims base value type is int64_t;

All compare data up to int64_t can fix warning and there is potential issue.

Signed-off-by: Yan, Xiping <[email protected]>
@xipingyan
channelAxis maybe == -1; means: no exist any more.
40360f4 
Signed-off-by: Yan, Xiping <[email protected]>
@xipingyan
Fix recursive macro: "one_of", "everyone_is" sign-compare warning.
c1641cb 
Must pass same value type.

Signed-off-by: Yan, Xiping <[email protected]>
@xipingyan
Fix Windows sign unsign compare warning
32a7b95 
Signed-off-by: Yan, Xiping <[email protected]>
@xipingyan
There are 2 instances:
c8eaa77 
using ov::Dimension::value_type = int64_t
using ov::intel_cpu::StaticDimension::value_type = size_t

All up to int64.

Signed-off-by: Yan, Xiping <[email protected]>
xipingyan added 7 commits March 2, 2023 14:31
@xipingyan
linux have too many sign-compare issue.
974400d 
Complete windows sign-compare firstly.

Signed-off-by: Yan, Xiping <[email protected]>
@xipingyan
Fix clang issues.
d878f3e 
Signed-off-by: Yan, Xiping <[email protected]>
@xipingyan
Fix warning.
f8af4d9 
Because instantiate T1=unsigned int, T2=int

Signed-off-by: Yan, Xiping <[email protected]>
@xipingyan
Fix warning for tests unit reorder_node_test.cpp
fc8aa3f 
Signed-off-by: Yan, Xiping <[email protected]>
@xipingyan
Fix warning : ASSERT_GE(step, 1u);
9ddd941 
Signed-off-by: Yan, Xiping <[email protected]>
@xipingyan
Fix tests: warning C4018
a371446 
Signed-off-by: Yan, Xiping <[email protected]>
@xipingyan
Remove auto, using int64_t is more reasonable.
7af1009 
Signed-off-by: Yan, Xiping <[email protected]>
@xipingyan xipingyan force-pushed the xp/jira_93714_fix_warning_after_rm_wd4309_wd4018 branch from f429d86 to 7af1009 Compare March 2, 2023 06:32
@xipingyan xipingyan requested a review from zhangYiIntel March 2, 2023 06:33
zhangYiIntel
zhangYiIntel approved these changes Mar 2, 2023
View reviewed changes
Copy link
Contributor

@zhangYiIntel zhangYiIntel left a comment

Choose a reason for hiding this comment

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

LGTM Thx!

dmitry-gorokhov
dmitry-gorokhov approved these changes Mar 2, 2023
View reviewed changes
Copy link
Contributor

@dmitry-gorokhov dmitry-gorokhov left a comment

Choose a reason for hiding this comment

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

CPU part LGTM

@dmitry-gorokhov dmitry-gorokhov self-assigned this Mar 3, 2023
@xipingyan
Copy link
Contributor Author

Hi @ilya-lavrenov , Do you have some comments about this PR? If not, coudl you help merge it, or forward to others, because Linux sign-compare issue needs this PR as base.

@ilya-lavrenov ilya-lavrenov merged commit 68b8d41 into openvinotoolkit:master Mar 8, 2023
@xipingyan xipingyan deleted the xp/jira_93714_fix_warning_after_rm_wd4309_wd4018 branch March 10, 2023 00:47
andrei-cv pushed a commit to andrei-cv/openvino that referenced this pull request Mar 21, 2023
@xipingyan @andrei-cv
[CPU]JIRA 93714 fix CPU plugin warning after remove wd4309 wd4018 (op…
1e2062c 
…envinotoolkit#15961)

* Remove warning suppression: wd4018, wd4309

Signed-off-by: Yan, Xiping <[email protected]>

* Remove linux warning suppression no-sign-compare

Signed-off-by: Yan, Xiping <[email protected]>

* ov::intel_cpu::VectorDims base value type is size_t;
dnnl::memory::dims base value type is int64_t;

All compare data up to int64_t can fix warning and there is potential issue.

Signed-off-by: Yan, Xiping <[email protected]>

* channelAxis maybe == -1; means: no exist any more.

Signed-off-by: Yan, Xiping <[email protected]>

* Fix recursive macro: "one_of", "everyone_is" sign-compare warning.
Must pass same value type.

Signed-off-by: Yan, Xiping <[email protected]>

* Fix Windows sign unsign compare warning

Signed-off-by: Yan, Xiping <[email protected]>

* There are 2 instances:

using ov::Dimension::value_type = int64_t
using ov::intel_cpu::StaticDimension::value_type = size_t

All up to int64.

Signed-off-by: Yan, Xiping <[email protected]>

* linux have too many sign-compare issue.
Complete windows sign-compare firstly.

Signed-off-by: Yan, Xiping <[email protected]>

* Fix clang issues.

Signed-off-by: Yan, Xiping <[email protected]>

* Fix warning.
Because instantiate T1=unsigned int, T2=int

Signed-off-by: Yan, Xiping <[email protected]>

* Fix warning for tests unit reorder_node_test.cpp

Signed-off-by: Yan, Xiping <[email protected]>

* Fix warning : ASSERT_GE(step, 1u);

Signed-off-by: Yan, Xiping <[email protected]>

* Fix tests: warning C4018

Signed-off-by: Yan, Xiping <[email protected]>

* Remove auto, using int64_t is more reasonable.

Signed-off-by: Yan, Xiping <[email protected]>

---------

Signed-off-by: Yan, Xiping <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhangYiIntel zhangYiIntel zhangYiIntel approved these changes

@dmitry-gorokhov dmitry-gorokhov dmitry-gorokhov approved these changes

@ilya-lavrenov ilya-lavrenov ilya-lavrenov approved these changes

@peterchen-intel peterchen-intel Awaiting requested review from peterchen-intel

@usstq usstq Awaiting requested review from usstq

@luo-cheng2021 luo-cheng2021 Awaiting requested review from luo-cheng2021

@chenhu-wang chenhu-wang Awaiting requested review from chenhu-wang

Assignees

@dmitry-gorokhov dmitry-gorokhov

Labels
category: build OpenVINO cmake script / infra category: Core OpenVINO Core (aka ngraph) category: CPU OpenVINO CPU plugin category: IE Tests OpenVINO Test: plugins and common
Projects
None yet
Milestone
2023.0
Development

Successfully merging this pull request may close these issues.
4 participants
@xipingyan @ilya-lavrenov @dmitry-gorokhov @zhangYiIntel