Skip to content

ABC the target descriptor and make consistent throughout. - #6941

Merged
sklam merged 1 commit into
numba:masterfrom
stuartarchibald:wip/abc_target_descriptor
Apr 21, 2021
Merged

ABC the target descriptor and make consistent throughout.#6941
sklam merged 1 commit into
numba:masterfrom
stuartarchibald:wip/abc_target_descriptor

Conversation

@stuartarchibald

@stuartarchibald stuartarchibald commented Apr 19, 2021

Copy link
Copy Markdown
Contributor

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

@stuartarchibald
ABC the target descriptor and make consistent throughout.
a4879c6 
As title. Needed to enforce the interface for `@intrinsic` template
`generic` methods.
@stuartarchibald stuartarchibald added this to the Numba 0.54 RC milestone Apr 19, 2021
gmarkall
gmarkall approved these changes Apr 19, 2021
View reviewed changes

@gmarkall gmarkall left a comment

Copy link
Copy Markdown
Member

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

stuartarchibald commented Apr 19, 2021

Copy link
Copy Markdown
Contributor Author

Buildfarm ID: numba_smoketest_cuda_yaml_48.

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

stuartarchibald commented Apr 19, 2021

Copy link
Copy Markdown
Contributor Author

Buildfarm ID: numba_smoketest_cuda_yaml_48.

This passed.

@stuartarchibald

stuartarchibald commented Apr 21, 2021

Copy link
Copy Markdown
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
sklam
sklam approved these changes Apr 21, 2021
View reviewed changes

@sklam sklam left a comment

Copy link
Copy Markdown
Member

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

Reviewers

@gmarkall gmarkall gmarkall approved these changes
@sklam sklam sklam approved these changes
@esc esc Awaiting requested review from esc

Assignees

No one assigned

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

Milestone

Numba 0.54 RC

Development

Successfully merging this pull request may close these issues.

3 participants

@stuartarchibald @gmarkall @sklam