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

audit inbounds for arrays #46911

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

audit inbounds for arrays #46911

wants to merge 1 commit into from

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Sep 26, 2022

Remove some unnecessary and incorrect inbounds annotations.

Fix #46879

@nanosoldier runbenchmarks(!"scalar" && !"union", vs=":master")

Remove some unnecessary and incorrect inbounds annotations.

Fix #46879
@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@vtjnash vtjnash added the triage This should be discussed on a triage call label Sep 29, 2022
@vtjnash vtjnash added DO NOT MERGE Do not merge this PR! and removed triage This should be discussed on a triage call labels Sep 29, 2022
while true
y = iterate(itr, st)
y === nothing && break
el, st = y
@assert len == length(dest)
if el isa T
@inbounds dest[i] = el
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

@aviatesk could we make this @inbounds conditional on iterate(itr, st) being pure and inlined (e.g. a simple load or computation)? That is the only case where this might SIMD, and thus the main case where we might care to eliminate the check here.

(We would like to check also if the user miscomputed length(itr) also and so this ran too many or too few iterations, but that seems harder)

@DilumAluthge DilumAluthge marked this pull request as draft June 24, 2023 04:06
@ViralBShah
Copy link
Member

This seems like a good PR to get merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE Do not merge this PR!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simple segfault on 1.8, 1.9, and master
5 participants