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

[common] fixed some compilation warnings #2071

Merged
merged 1 commit into from
Oct 4, 2024
Merged

[common] fixed some compilation warnings #2071

merged 1 commit into from
Oct 4, 2024

Conversation

lslusarczyk
Copy link
Contributor

@lslusarczyk lslusarczyk commented Sep 9, 2024 •
edited
Loading

  • excessive semicolons
  • unused variables
  • use python3.6 friendly "universal_newlines" alias instead of "text" param

@lslusarczyk lslusarczyk requested review from a team as code owners September 9, 2024 09:54
@github-actions github-actions bot added loader Loader related feature/bug common Changes or additions to common utilities conformance Conformance test suite issues. level-zero L0 adapter specific issues sanitizer Sanitizer layer issues/changes/specification labels Sep 9, 2024
@pbalcer
Copy link
Contributor

pbalcer commented Sep 9, 2024

You need to format your code changes. make cppformat.

@lslusarczyk lslusarczyk force-pushed the compilation_fixes_for_sles branch 2 times, most recently from e88e2b4 to 947951c Compare September 23, 2024 09:22
@lukaszstolarczuk lukaszstolarczuk mentioned this pull request Oct 1, 2024
Closed
@lslusarczyk lslusarczyk mentioned this pull request Oct 2, 2024
Draft
@lukaszstolarczuk
Copy link
Contributor

👍

@lslusarczyk
Copy link
Contributor Author

lslusarczyk commented Oct 3, 2024 •
edited
Loading

@pbalcer , can you take a look again and if all is OK, then merge this?
Only OpenCL checks fail (as always). Llvm tests pass with this change: intel/llvm#15574

pbalcer
pbalcer reviewed Oct 3, 2024
View reviewed changes
source/loader/layers/sanitizer/asan_buffer.cpp Outdated
@@ -164,7 +164,8 @@ ur_result_t MemBuffer::getHandle(ur_device_handle_t Device, char *&Handle) {
}

ur_result_t MemBuffer::free() {
for (const auto &[_, Ptr] : Allocations) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIR compiling on jf cluster with g++7 being newest compiler avail there reports warning about "_" as unused variable. These were the only fixes needed to make UR compilable there.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should not worry about warnings in an old compiler. Just don't set -Werror.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, reverted

source/loader/ur_lib.cpp Outdated
@@ -50,8 +50,8 @@ void context_t::parseEnvEnabledLayers() {
}

void context_t::initLayers() {
for (auto &[layer, _] : layers) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@lslusarczyk
[common] Fix some compilation warnings
97a22aa 
Remove excessive semicolons.
Use python3.6 friendly alias instead of text param.
@lslusarczyk
Copy link
Contributor Author

@pbalcer , if all is OK please merge

@pbalcer pbalcer merged commit 7b4bc76 into oneapi-src:main Oct 4, 2024
78 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukaszstolarczuk lukaszstolarczuk lukaszstolarczuk left review comments

@RossBrunton RossBrunton RossBrunton approved these changes

@pbalcer pbalcer pbalcer approved these changes

Assignees
No one assigned
Labels
common Changes or additions to common utilities conformance Conformance test suite issues. level-zero L0 adapter specific issues loader Loader related feature/bug sanitizer Sanitizer layer issues/changes/specification
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lslusarczyk @pbalcer @lukaszstolarczuk @RossBrunton