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.

Sign up for GitHub

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

Jump to bottom

Avoid capturing AbstractArrays in BoundsError #2314

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Avoid capturing AbstractArrays in BoundsError #2314

merged 1 commit into from
Apr 10, 2024

Conversation

lcw
Copy link
Contributor

@lcw lcw commented Apr 3, 2024

This quirk allows bounds checking of MArrays on device in more cases. This is a workaround to address #2313.

@lcw lcw mentioned this pull request Apr 3, 2024
Closed
lcw added a commit to HorribleSanity/Raven.jl that referenced this pull request Apr 3, 2024
@lcw
Avoid capturing MArrays in BoundsError
8c16db2 
This quirk allows bounds checking of `MArray`s on device in more cases.
This is a workaround to address
<JuliaGPU/CUDA.jl#2313>.

This change was proposed upstream here
   <JuliaGPU/CUDA.jl#2314>
and is no longer needed if accepted upstream.
vchuravy
vchuravy requested changes Apr 5, 2024
View reviewed changes
Copy link
Member

@vchuravy vchuravy left a comment

Choose a reason for hiding this comment

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

Could you instead change

CUDA.jl/src/device/quirks.jl

Line 40 in 7f725c0

@device_override @noinline Base.throw_boundserror(A, I) =

to be @inline and move the error report to a second function?

@lcw
Copy link
Contributor Author

lcw commented Apr 5, 2024

Could you instead change

CUDA.jl/src/device/quirks.jl

Line 40 in 7f725c0

@device_override @noinline Base.throw_boundserror(A, I) =

to be @inline and move the error report to a second function?

Is this what you are thinking? Feel free to tweek it.

@vchuravy
Copy link
Member

vchuravy commented Apr 5, 2024

@maleadt are you okay with that extra call frame?

And should we add the Static array test case?

@maleadt
Copy link
Member

maleadt commented Apr 5, 2024

@maleadt are you okay with that extra call frame?

Sure. I would just call it throw_boundserror though, the printing is a bit of an implementation detail.

And should we add the Static array test case?

I don't think so, because I hope we can get it working under --check-bounds=auto (and the tests shouldn't require --check-bounds=yes to pass, as I occasionally execute them outside of Pkg).

@lcw
Inline Base.throw_boundserror quirk
ea9331c 
This avoid capturing `MArray`s in `BoundsError` to allow bounds checking
of `MArray`s on device in more cases. This is a workaround to address
<JuliaGPU#2313>.
@vchuravy vchuravy changed the title Avoid capturing MArrays in BoundsError Avoid capturing AbstractArrays in BoundsError Apr 10, 2024
@vchuravy vchuravy merged commit 5da4d1d into JuliaGPU:master Apr 10, 2024
1 check passed
lcw added a commit to HorribleSanity/Raven.jl that referenced this pull request Apr 19, 2024
@lcw
Avoid capturing MArrays in BoundsError
80935eb 
This quirk allows bounds checking of `MArray`s on device in more cases.
This is a workaround to address
<JuliaGPU/CUDA.jl#2313>.

This change was proposed upstream here
   <JuliaGPU/CUDA.jl#2314>
and is no longer needed if accepted upstream.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vchuravy vchuravy vchuravy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lcw @vchuravy @maleadt