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

Update params implementation #901

Merged
merged 2 commits into from
Nov 20, 2024

Conversation

vinser52
Copy link
Contributor

@vinser52 vinser52 commented Nov 15, 2024

Description

Ref. #844

Checklist

  • Code compiles without errors locally
  • All tests pass locally
  • CI workflows execute properly

@vinser52 vinser52 requested a review from a team as a code owner November 15, 2024 13:02
@vinser52 vinser52 marked this pull request as draft November 15, 2024 13:03
@vinser52 vinser52 force-pushed the svinogra_config_params branch from 79b90ba to 67570d5 Compare November 15, 2024 13:54
@vinser52 vinser52 force-pushed the svinogra_config_params branch 3 times, most recently from c67f102 to a3050d4 Compare November 15, 2024 16:52
@vinser52 vinser52 force-pushed the svinogra_config_params branch from a3050d4 to 05a44a0 Compare November 18, 2024 13:43
@igchor
Copy link
Member

igchor commented Nov 18, 2024

Disjoint pool changes look good (but you should probably change umfDisjointPoolParamsSetName to use deep copy as marked in the comment.)

include/umf/pools/pool_disjoint.h Outdated Show resolved Hide resolved
src/pool/pool_disjoint.cpp Show resolved Hide resolved
@vinser52 vinser52 force-pushed the svinogra_config_params branch from 05a44a0 to ab46aaf Compare November 19, 2024 11:55
include/umf/pools/pool_jemalloc.h Outdated Show resolved Hide resolved
src/pool/pool_jemalloc.c Outdated Show resolved Hide resolved
@vinser52 vinser52 force-pushed the svinogra_config_params branch from ab46aaf to 477c341 Compare November 19, 2024 14:19
@vinser52
Copy link
Contributor Author

Disjoint pool changes look good (but you should probably change umfDisjointPoolParamsSetName to use deep copy as marked in the comment.)

@igchor

Done

@vinser52 vinser52 marked this pull request as ready for review November 19, 2024 14:20
@bratpiorka bratpiorka requested review from lplewa and igchor November 19, 2024 14:30
include/umf/pools/pool_disjoint.h Show resolved Hide resolved
include/umf/pools/pool_jemalloc.h Outdated Show resolved Hide resolved
src/pool/pool_disjoint.cpp Show resolved Hide resolved
test/pools/jemalloc_pool.cpp Show resolved Hide resolved
@vinser52 vinser52 force-pushed the svinogra_config_params branch from 477c341 to 056e493 Compare November 19, 2024 14:50
@vinser52 vinser52 force-pushed the svinogra_config_params branch from 056e493 to 9d74c17 Compare November 19, 2024 15:13
@bratpiorka bratpiorka mentioned this pull request Nov 19, 2024
@vinser52
Copy link
Contributor Author

@pbalcer @igchor I just tested this version of UMF with the 2025.0 compiler. Seems it works. Looks like this change does not break ABI.

Of course, I run just a single test app, but it works. I understand that UR needs to be updated to use the new API, but I hope having ABI compatibility decreases the probability that we will break compiler CI with this transition.

@lukaszstolarczuk lukaszstolarczuk merged commit 249433e into oneapi-src:main Nov 20, 2024
78 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants