Skip to content

[GPU] address coverity hits #4103

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
Merged
merged 10 commits into from
Oct 15, 2025
Merged

[GPU] address coverity hits #4103

merged 10 commits into from
Oct 15, 2025

Conversation

atkassen
Copy link
Contributor

@atkassen atkassen commented Oct 9, 2025

Addresses MFDNN-14280.

@atkassen atkassen self-assigned this Oct 9, 2025
@atkassen atkassen requested a review from a team as a code owner October 9, 2025 18:46
@github-actions github-actions bot added platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel component:tests Codeowner: @oneapi-src/onednn-arch third_party labels Oct 9, 2025
@atkassen
Copy link
Contributor Author

atkassen commented Oct 9, 2025

make test
disable build_cpu_runtime_omp
disable build_cpu_runtime_sycl
disable build_cpu_runtime_tbb
disable test_device_cpu
set test_scope=NIGHTLY

@atkassen atkassen force-pushed the akassen/coverity branch 2 times, most recently from 0519ddc to f90f8ce Compare October 10, 2025 17:51
@atkassen
Copy link
Contributor Author

make test
disable build_cpu_runtime_omp
disable build_cpu_runtime_sycl
disable build_cpu_runtime_tbb
disable test_device_cpu
set test_scope=NIGHTLY

hidefromkgb
hidefromkgb reviewed Oct 11, 2025
View reviewed changes
src/gpu/intel/jit/ir/v2/reqs.cpp
for (req_kind_t op : {req_kind_t::_or_eq, req_kind_t::eq,
req_kind_t::ge, req_kind_t::le}) {
auto s_op = to_string(op);
const auto &s_op = to_string(op);
Copy link
Contributor

@hidefromkgb hidefromkgb Oct 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This one might be dangerous.

to_string() creates a prvalue which is then bound to a constant reference. From C++17 onwards this is called temporary materialization and is legal — but are we sure that the usage of C++17 is mandated when building oneDNN?

Copy link
Contributor

Choose a reason for hiding this comment

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

…I stand corrected.
I found the implementation (here and here) and it does return a precomputed value instead of creating it ad-hoc. Amazing!
Sorry for the noise.

@atkassen atkassen mentioned this pull request Oct 13, 2025
Merged
@atkassen
Copy link
Contributor Author

make test
disable build_cpu_runtime_omp
disable build_cpu_runtime_sycl
disable build_cpu_runtime_tbb
disable test_device_cpu
set test_scope=NIGHTLY

@atkassen atkassen merged commit a4874e4 into uxlfoundation:main Oct 15, 2025
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hidefromkgb hidefromkgb hidefromkgb approved these changes

@kealan-barbieri kealan-barbieri kealan-barbieri approved these changes

Assignees

@atkassen atkassen

Labels

component:tests Codeowner: @oneapi-src/onednn-arch platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel third_party

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@atkassen @hidefromkgb @kealan-barbieri