Skip to content

Conversation

@AlexeySachkov
Copy link
Contributor

UrArray is a wrapper intended to extend lifetime of mock objects so that native objects referncing mock data can be accessed by SYCL RT without any issues.

This commit removes the wrapper to simplify writing unit-tests: essentially there is now one less custom class to care about, collections of properties and entry points are now handled using regular std::vector and initializer lists.

The cost of this is a slightly more complicated implementation for UrPropertySet and UrImage that now need to replicate functionality of that extra storage from UrArray.

`UrArray` is a wrapper intended to extend lifetime of mock objects so
that native objects referncing mock data can be accessed by SYCL RT
without any issues.

This commit removes the wrapper to simplify writing unit-tests:
essentially there is now one less custom class to care about,
collections of properties and entry points are now handled using regular
`std::vector` and initializer lists.

The cost of this is a slightly more complicated implementation for
`UrPropertySet` and `UrImage` that now need to replicate functionality
of that extra storage from `UrArray`.
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

Overall LGTM!

AlexeySachkov added a commit to AlexeySachkov/llvm that referenced this pull request Oct 4, 2024
This is a first patch in series of detaching unit-test classes from UR
names.

In a previous attempts to do so (intel#14815) there were concerns about what
to do with `UrArray` and this PR is another attempt (see also intel#15014) to
do so.

This PR renames `UrArray` into `LifetimeExtender` to better communicate
its purpose: data structures emitted by the compiler (and therefore used
by the runtime) to describe device image and their properties to not
store data, but only hold pointers to them. Therefore when we mock those
in our unit-tests we need to ensure that their lifetime is long enough
to cover the whole test.

This is a non-functional change by its spirit, but what used to be
`UrArray` is now hidden from writers of unit-tests and the interface is
switched to `std::vector` - that is done to hide an implementaiton
detail and simplify amount of knowledge required to write unit-tests.
AlexeySachkov added a commit that referenced this pull request Oct 7, 2024
This is a first patch in series of detaching unit-test classes from UR
names.

In a previous attempts to do so (#14815) there were concerns about what
to do with `UrArray` and this PR is another attempt (see also #15014) to
do that.

This PR renames `UrArray` into `LifetimeExtender` to better communicate
its purpose: data structures emitted by the compiler (and therefore used
by the runtime) to describe device image and their properties do not
store data, but only hold pointers to them. Therefore when we mock those
in our unit-tests we need to ensure that their lifetime is long enough
to cover the whole test.

This is a non-functional change by its spirit, but what used to be
`UrArray` is now hidden from writers of unit-tests and the interface is
switched to `std::vector` - that is done to hide an implementation
detail and simplify amount of knowledge required to write unit-tests.
@AlexeySachkov
Copy link
Contributor Author

Superseded by #15604

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