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

Performance tips recommend invalid use of @inbounds #54218

Closed
LilithHafner opened this issue Apr 23, 2024 · 0 comments · Fixed by #54221
Closed

Performance tips recommend invalid use of @inbounds #54218

LilithHafner opened this issue Apr 23, 2024 · 0 comments · Fixed by #54221
Labels
bug Indicates an unexpected problem or unintended behavior docs This change adds or pertains to documentation

Comments

@LilithHafner
Copy link
Member

https://docs.julialang.org/en/v1/manual/performance-tips/#man-performance-annotations (still broken on master)

includes

@noinline function inner(x, y)
    s = zero(eltype(x))
    for i=eachindex(x)
        @inbounds s += x[i]*y[i]
    end
    return s
end

which is UB because eachindex(x) may be out of bounds in y and @inbounds applies to all possible inputs to inner.

It should be eachindex(x, y). Making an issue instead of a PR to avoid conflict with #54211

@LilithHafner LilithHafner added bug Indicates an unexpected problem or unintended behavior docs This change adds or pertains to documentation labels Apr 23, 2024
fatteneder pushed a commit that referenced this issue Apr 23, 2024
Remove some UB from examples in performance tips

Fixes #54218

I also changed the iteration specification to better align with
elsewhere in the document.

A separate PR may want to delete or revise this section if automatic
simd has improved. Nevertheless, I think this PR should be
unobjectionable as is.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant