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

Add missing affinity domains #132

Closed
wants to merge 1 commit into from

Conversation

FranklandJack
Copy link
Contributor

@FranklandJack FranklandJack commented Dec 13, 2022

Add missing cache affinity domains to device affinity domains
enumeration. Fixes #115.

Add missing cache affinity domains to device affinity domains
enumeration.
Copy link
Contributor

@veselypeta veselypeta left a comment

Choose a reason for hiding this comment

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

LGTM

@FranklandJack
Copy link
Contributor Author

This might be worth reconsidering.

Looking at the usage of PI_DEVICE_AFFINITY_DOMAIN_L(1|2|3|4)_CACHE none of the adaptors actually handle these enumeration values, so although they would be required replace the usage in the DPC++ codebase as is, perhaps they aren't really needed by the DPC++ codebase to begin with unless there are out of tree adapters using them?

@kbenzie
Copy link
Contributor

kbenzie commented Dec 13, 2022

... perhaps they aren't really needed by the DPC++ codebase to begin with unless there are out of tree adapters using them?

Does make me wonder if we should hold off making this change then.

@FranklandJack
Copy link
Contributor Author

... perhaps they aren't really needed by the DPC++ codebase to begin with unless there are out of tree adapters using them?

Does make me wonder if we should hold off making this change then.

Yeah I think so, this may be true of other missing PI features. But it would be useful to get confirmation from engineers who are familiar with that codebase.

@FranklandJack
Copy link
Contributor Author

I'm closing this since there is a question of if it is actually required.

@kbenzie kbenzie deleted the add_missing_affinity_domains branch March 9, 2023 12:22
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.

Add Missing ur_device_affinity_domain_flag_t Enumerations
3 participants