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

ABC the target descriptor and make consistent throughout. #6941

Merged
merged 1 commit into from Apr 21, 2021

Conversation

stuartarchibald
Copy link
Contributor

As title. Needed to enforce the interface for @intrinsic template
generic methods.

As title. Needed to enforce the interface for `@intrinsic` template
`generic` methods.
Copy link
Member

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

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

This passes tests locally for me with hardware and the changes seem to make sense.

We may still encounter issues with the typing and target contexts existing per-dispatcher in the CUDA target (which is presently needed in CUDA because things like the fast math flag are attached to the context) but being class members in the CPU target (as I understood from an earlier OOB conversation with @stuartarchibald ), but this is a move in the right direction.

@stuartarchibald
Copy link
Contributor Author

Buildfarm ID: numba_smoketest_cuda_yaml_48.

@stuartarchibald stuartarchibald added the Effort - short Short size effort needed label Apr 19, 2021
@stuartarchibald
Copy link
Contributor Author

Buildfarm ID: numba_smoketest_cuda_yaml_48.

This passed.

@stuartarchibald
Copy link
Contributor Author

This passes tests locally for me with hardware and the changes seem to make sense.

Thanks for the review @gmarkall

We may still encounter issues with the typing and target contexts existing per-dispatcher in the CUDA target (which is presently needed in CUDA because things like the fast math flag are attached to the context) but being class members in the CPU target (as I understood from an earlier OOB conversation with @stuartarchibald ), but this is a move in the right direction.

Thanks for noting this, definitely something to investigate as part of the next refactor.

@sklam sklam added the BuildFarm Passed For PRs that have been through the buildfarm and passed label Apr 21, 2021
Copy link
Member

@sklam sklam left a comment

Choose a reason for hiding this comment

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

LGTM

@sklam sklam added 5 - Ready to merge Review and testing done, is ready to merge and removed 3 - Ready for Review labels Apr 21, 2021
@sklam sklam merged commit d7b1965 into numba:master Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to merge Review and testing done, is ready to merge BuildFarm Passed For PRs that have been through the buildfarm and passed Effort - short Short size effort needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants