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

Add more tests to api validation testing #447

Merged
merged 6 commits into from
Oct 18, 2024
Merged

Add more tests to api validation testing #447

merged 6 commits into from
Oct 18, 2024

Conversation

christiangnrd
Copy link
Contributor

Not worth running a test we know will fail with only one apple silicon runner.

@christiangnrd
Copy link
Contributor Author

christiangnrd commented Oct 3, 2024

Right this is still broken on Ventura. I'll split up the PR into what will pass CI now and what can wait.

@christiangnrd christiangnrd changed the title Validate more code in CI and pause nightly CI Add more tests to api validation testing Oct 4, 2024
@christiangnrd christiangnrd marked this pull request as draft October 4, 2024 02:55
@christiangnrd christiangnrd marked this pull request as ready for review October 17, 2024 19:37
@christiangnrd
Copy link
Contributor Author

All tests can now be run with the shader validation enabled except for device_code_agx for which I've opened #463!

@maleadt
Copy link
Member

maleadt commented Oct 18, 2024

At some point we should device and audit whether to check for non-zero length as part of each API call (e.g., unsafe_fill! here), or only from the high-level caller. It's a bit messy to have these N > 0 calls all over the place. For fill!, the high-level caller already does so:

Metal.jl/src/array.jl

Lines 534 to 540 in 711758d

function Base.fill!(A::MtlArray{T}, val) where T <: Union{UInt8,Int8}
if length(A) > 0
B = convert(T, val)
unsafe_fill!(device(A), pointer(A), B, length(A))
end
A
end

@maleadt maleadt merged commit 35189d9 into main Oct 18, 2024
2 checks passed
@maleadt maleadt deleted the apivalidation branch October 18, 2024 06:17
@christiangnrd
Copy link
Contributor Author

At some point we should device and audit whether to check for non-zero length as part of each API call (e.g., unsafe_fill! here), or only from the high-level caller. It's a bit messy to have these N > 0 calls all over the place. For fill!, the high-level caller already does so:

Metal.jl/src/array.jl

Lines 534 to 540 in 711758d

function Base.fill!(A::MtlArray{T}, val) where T <: Union{UInt8,Int8}
if length(A) > 0
B = convert(T, val)
unsafe_fill!(device(A), pointer(A), B, length(A))
end
A
end

That's from a recent PR where I tried to address some validation errors. I can remove the high-level change since it wasn't enough and leave the low-level one in #464

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

Successfully merging this pull request may close these issues.

2 participants