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

[spec][usm-p2p] p2p info query return int instead of uint32_t, fix l0 impl #2246

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

JackAKirk
Copy link
Contributor

@JackAKirk JackAKirk commented Oct 25, 2024

The level zero p2p impl currently returns boolean values instead of an integer. This PR fixes that to match the spec requirement.
I've also changed the spec requirement slightly so that all ur_exp_peer_info_t types are int instead of uint32_t

The switch to int practically makes no real difference right now but I think is an improvement.
I think that I made a mistake choosing uint32_t originally since in theory the cuda/hip backends could make use of negative values in the future since they use int type; such that by restricting to an unsigned type we are technically restricting these backends.
Also by not specifying a 32 bit type we allow for any unexpected issues where a runtime platform could theoretically use e.g. 16 bit integers as a query return value.

Fixes #2079

@JackAKirk
Copy link
Contributor Author

Corresponding DPC++ change here: intel/llvm#15873

@github-actions github-actions bot added specification Changes or additions to the specification experimental Experimental feature additions/changes/specification level-zero L0 adapter specific issues labels Oct 25, 2024
@JackAKirk
Copy link
Contributor Author

L0 Failures are completely unrelated and also occur e.g. here: https://github.com/oneapi-src/unified-runtime/actions/runs/11527149243/job/32093062503?pr=2242

@JackAKirk JackAKirk added the ready to merge Added to PR's which are ready to merge label Oct 28, 2024
@pbalcer pbalcer merged commit e3d127d into oneapi-src:main Oct 29, 2024
76 of 77 checks passed
sommerlukas pushed a commit to intel/llvm that referenced this pull request Oct 31, 2024
There has been some confusion I think originating in the fact that L0
backend returned bool type instead of int for P2P queries. This issue is
fixed here oneapi-src/unified-runtime#2246

This PR correspondingly updates the dpc++ runtime.

Fixes #15841

---------

Signed-off-by: JackAKirk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Experimental feature additions/changes/specification level-zero L0 adapter specific issues ready to merge Added to PR's which are ready to merge specification Changes or additions to the specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type mismatch in p2p peer info enum
2 participants