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

@propagate_inbounds not working through copyto! #57003

Open
vustef opened this issue Jan 9, 2025 · 3 comments
Open

@propagate_inbounds not working through copyto! #57003

vustef opened this issue Jan 9, 2025 · 3 comments

Comments

@vustef
Copy link

vustef commented Jan 9, 2025

copyto!(dest::Array, doffs::Integer, src::Array, soffs::Integer, n::Integer) = _copyto_impl!(dest, doffs, src, soffs, n)
invokes another function, even if I have my own function that propagates inbounds and invokes copyto!, boundscheck will actually trigger.
It seems like an oversight, but not sure if it was intentional.
I think I read somewhere that @propagate_inbounds is intended for one level of function calls, but given that, was it intentional that it invokes another function and therefore doesn't actually propagate inbounds to where it matters?

cc @NHDaly

@NHDaly
Copy link
Member

NHDaly commented Jan 9, 2025

As an example from our source code that @vustef is referring to, we have something like:

@inbounds begin
        resize!(result, len)
        copyto!(left.res_join_cols.data, 1, left.join_cols, 1, len)
        copyto!(left.res_non_join_cols.data, 1, left.non_join_cols, 1, len)
        # ...
end

and we were surprised to find that copyto!() is still performing boundschecks even though it's marked @inbounds.

Is this on-purpose?

@gbaraldi
Copy link
Member

gbaraldi commented Jan 9, 2025

It's mostly intended since the boundscheck there is very cheap (It's one check for the whole operation).

@oscardssmith
Copy link
Member

We could put a @propagate_inbounds on the copyto! and _copyto_impl! definitions to remove the check which would result in boundscheck removal.

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

No branches or pull requests

4 participants