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

[GPU] Address clang-tidy complaints #2431

Merged
merged 8 commits into from
Jan 26, 2025

Conversation

atkassen
Copy link
Contributor

@atkassen atkassen commented Jan 16, 2025

Addresses most of the clang-tidy hits in MFDNN-13007. Missing are fixes for macro warnings and emulation.hpp class name styling.

  • Do all unit and benchdnn tests (make test and make test_benchdnn_*) pass locally for each commit?
  • Have you formatted the code using clang-format?

@atkassen atkassen self-assigned this Jan 16, 2025
@atkassen atkassen requested review from a team as code owners January 16, 2025 22:57
@github-actions github-actions bot added platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel platform:gpu-generic Codeowner: @oneapi-src/onednn-gpu-generic labels Jan 16, 2025
src/gpu/intel/jit/ir/blocking.hpp Outdated Show resolved Hide resolved
src/gpu/intel/jit/ir/config.hpp Outdated Show resolved Hide resolved
src/gpu/intel/jit/ir/gemm_schedule.hpp Show resolved Hide resolved
@@ -553,7 +553,8 @@ class layout_t {
b_str.append(1, '*');
}
}
ret = b_str + ret;
b_str += ret;
std::swap(ret, b_str);
Copy link
Contributor

Choose a reason for hiding this comment

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

Was it a bug? The new sequence doesn't seem to do what it was doing before...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

b_str is destroyed immediately after this, so we don't care what happens to it. The effect on ret should be the same. I could replace the swap with ret = std::move(b_str) if it is clearer? Visually that change looks like it would be better as the original, so the swap is there to help prevent someone coming along later and trying to undo the change.

src/gpu/intel/jit/utils/utils.hpp Show resolved Hide resolved
@atkassen atkassen force-pushed the akassen/clang-tidy-gpu branch from 60e94cb to c377734 Compare January 17, 2025 00:55
@atkassen atkassen force-pushed the akassen/clang-tidy-gpu branch from c377734 to e1d1ebe Compare January 17, 2025 21:54
@atkassen atkassen force-pushed the akassen/clang-tidy-gpu branch from e1d1ebe to bece593 Compare January 23, 2025 18:48
@atkassen
Copy link
Contributor Author

make test
disable test_device_cpu
disable build_cpu_runtime_omp
disable build_cpu_runtime_sycl
disable build_cpu_runtime_tbb

@atkassen atkassen force-pushed the akassen/clang-tidy-gpu branch from bece593 to 5d22ea7 Compare January 25, 2025 05:02
@atkassen atkassen merged commit 6b90d64 into oneapi-src:main Jan 26, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform:gpu-generic Codeowner: @oneapi-src/onednn-gpu-generic platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants