Skip to content

Make target for @overload have 'generic' as default. - #8554

Merged
sklam merged 2 commits into
numba:mainfrom
stuartarchibald:wip/overload_target_generic_default_v2
Nov 4, 2022
Merged

sklam merged 2 commits into
numba:mainfrom
stuartarchibald:wip/overload_target_generic_default_v2

Conversation

@stuartarchibald

Copy link
Copy Markdown
Contributor

This makes the default target for @overload the 'generic' target, as, in effect, very few @overload implementations are genuinely target specific. Further, the use of the standard numba.jit wrapper (with no target set) for @overload implementations leads to strange cases where some function cannot be resolved on e.g. the CUDA target, but because there's no target set in the jit wrapper the error message cannot tell you why, it just materialises as a lowering error (i.e. this should fix #8530).

The first commit in this PR (a3ecb1f) is from @gmarkall's Numba fork on branch overload-target-generic, the corresponding commit is: 38b5379, authorship is preserved, many thanks @gmarkall!

gmarkall and others added 2 commits October 31, 2022 13:45
As title. This preserves the historical behaviour of `@overload`
having the kwarg default of `nopython=True`. Without this,
`@overload`s that end up with `literally` calls in their path can
refuse to resolve as `object-mode` fallback occurs and there's no
compiler pass for handling `literally` calls in that mode.
@gmarkall

Copy link
Copy Markdown
Member

gpuci run tests

@stuartarchibald

Copy link
Copy Markdown
Contributor Author

RE patch approval. I approve a3ecb1f which was authored by @gmarkall. I had made identical changes locally before realising that @gmarkall already had a patch for it so am happy with what is present!

@stuartarchibald stuartarchibald added this to the Numba 0.57 RC milestone Nov 4, 2022

@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 looks good to me, though I agree with what was said OOB about a buildfarm run being a good idea first.

@gmarkall gmarkall added Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm 4 - Waiting on CI Review etc done, waiting for CI to finish and removed 3 - Ready for Review labels Nov 4, 2022
@esc

esc commented Nov 4, 2022

Copy link
Copy Markdown
Member

Build numba_smoketest_cuda_yaml_169 has started

@esc

esc commented Nov 4, 2022

Copy link
Copy Markdown
Member

Build numba_smoketest_cuda_yaml_169 has started

This was fine.

@gmarkall gmarkall added BuildFarm Passed For PRs that have been through the buildfarm and passed 4 - Waiting on reviewer Waiting for reviewer to respond to author 5 - Ready to merge Review and testing done, is ready to merge and removed Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm 4 - Waiting on CI Review etc done, waiting for CI to finish 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Nov 4, 2022
@gmarkall

gmarkall commented Nov 4, 2022

Copy link
Copy Markdown
Member

@esc Many thanks!

@sklam
sklam merged commit 4535cb6 into numba:main Nov 4, 2022
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.

Cross target use of target-less @overload leads to lowering errors.

4 participants