Repository navigation
Conversation
JLArray accepted element types like `Union{Int, Nothing}`, but stored
no selector bytes and wrapped its memory as a plain `Array`, so reading
such an array back returned wrong values and kernels failed.
Store the elements and selector bytes like `Array` does: elements are
padded to `aligned_sizeof(T)`, and one selector byte per element
follows the data. Arrays track the offset of their first selector
byte, which derived arrays advance by element, so that views of unions
of singletons, which have no data, still find their own selectors.
Copies to, from and between JLArrays copy the selector bytes too, and
the device array loads and stores elements according to them. The
host algorithms that ran on a wrapped `Array` use the device array
for these element types instead.
Every back-end now supports arrays of isbits unions, but the testsuite only covered them in passing, and only where the back-end supported them. Add tests for them, based on the bugs that came up implementing them in the back-ends: views whose selector bytes were taken from their parent, views of unions of singletons (which have no data), elements whose size is not a multiple of their alignment, `fill!` fast paths that skip the selector bytes, and `resize!` of views.
The `reinterpret` and `any`/`all` tests of isbits-union arrays only ran when the back-end could construct one. Run them unconditionally now that the testsuite requires support for such arrays.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Arrays with an isbits-union element type (
Union{Int32, Nothing},Union{Missing, Bool},Union{Nothing, Missing}, ...) store one selector byte per element after the data, likeArraydoes. The back-ends either didn't support this or got it wrong in subtle ways: views read their parent's selector bytes, views of unions of singletons (which have no data, so no byte offset) did the same,fill!used memset and left stale selectors,resize!of a view or of a zero-size union broke. Without union support,Array(GPUArray(Union{Int,Nothing}[1, nothing]))silently returns wrong values. The testsuite only touched such arrays in passing, and only after checking at run time whether the back-end could construct one.This PR makes union support part of the testsuite's contract. The new
unionstests cover round trips and partial copies between host and device for unions with members of different sizes and alignments, elements padded beyond theirsizeof(Union{Int64, NTuple{3, Int32}}, 12 bytes with a 16-byte stride), unions of singletons and nested unions; views with irregular data, checking that writes through them leave the rest of the parent alone; KernelAbstractions kernels on such arrays and on structs with a union field;fill!with unions memset could otherwise fill; broadcasting,any/all,mapreduce,findall, scalar indexing andresize!. The existingreinterpretandany/alltests for unions now run unconditionally. Unions stick toInt32,Float32and smaller members, so Metal can run them.JLArrays didn't support unions either: it stored no selector bytes and wrapped its memory as an
Array. The first commit adds support with the same layout, tracking where an array's selector bytes start so that derived arrays advance them by element.This needs the back-end PRs to land first:
Results of the
unions,findallandbasetests:fill!,resize!of viewsThe full JLArrays and
Arraysuite passes on Julia 1.13.