Skip to content

[SYCL][NFC] Detach unit-tests naming from UR #14815

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

Conversation

AlexeySachkov
Copy link
Contributor

This is a follow-up from #14145. Our unit tests have mock classes corresponding to device images and other entries our compiler generates. They are all named using Ur prefix, but they have nothing to do with Unified Runtime, they are part of our compiler interface.

This patch renames associated files and classes so they do not refer to Ur anymore.
This is a mechanical find-and-replace change.

@AlexeySachkov
Copy link
Contributor Author

AlexeySachkov commented Jul 28, 2024

This PR will conflict with #14816. I have no strict preference about which one goes first, but I think that if we merge this one first, then I will have less conflicts to resolve.

@aelovikov-intel
Copy link
Contributor

then I will have left conflicts to resolve.

Is that s/left/less/ ?

@AlexeySachkov
Copy link
Contributor Author

then I will have left conflicts to resolve.

Is that s/left/less/ ?

Yes

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 AlexeySachkov marked this pull request as draft October 10, 2024 11:17
@AlexeySachkov
Copy link
Contributor Author

3eafbcb - a very small change which I missed earlier because virtual functions unit-tests were disabled at the time.

@AlexeySachkov
Copy link
Contributor Author

I've verified locally that build and unit-tests still pass after merge with sycl branch, so I will proceed with this PR.

@AlexeySachkov AlexeySachkov merged commit eded703 into intel:sycl Nov 5, 2024
13 checks passed
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