[SYCL] Remove esimd_emulator from filters #12428
Conversation
uditagarwal97
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
This PR separates out the removal of esimd_emulator from PR##12384 |
sycl/source/detail/config.cpp
Outdated
| {{"host", backend::host}, | ||
| {"opencl", backend::opencl}, | ||
| {"level_zero", backend::ext_oneapi_level_zero}, | ||
| {"cuda", backend::ext_oneapi_cuda}, | ||
| {"hip", backend::ext_oneapi_hip}, | ||
| {"esimd_emulator", backend::ext_intel_esimd_emulator}, |
There was a problem hiding this comment.
I think this might be an ABI break and thus unacceptable.
There was a problem hiding this comment.
esimd_emulator is not functional starting from 2024.0 - we have removed all the code for emulator, maybe except few places like this one.
It is not ABI break IMO, if it was we would have fails in those special tests that verify presence of global symbols.
There was a problem hiding this comment.
If return type was part of the mangling, the symbol name would have changed (and that would definitely be an ABI break). I'm not sure what would be the implications of a code compiled expecting 8-element std::array and only looking into a 7-element array.
There was a problem hiding this comment.
@aelovikov-intel I went through all the code usages of getSyclBeMap() in DPC++ but did not found any usage that implicitly requires the SYCL backend std::array to have 8-elements. In all the cases, the returned array is traversed using iterators with no hard-coded requirement on its size.
There was a problem hiding this comment.
Now that I rethink again, I see your point. Some pre-compiled binaries using libsycl might expect the return value of getSyclBeMap to have 8 elements. So, @aelovikov-intel what do you suggest, should we table this change until the next ABI breaking window?
There was a problem hiding this comment.
It depends on the usage of this array. Maybe we can just keep this line and apply the rest of the PR. Still, it's better to discuss that with @v-klochkov and @turinevgeny
There was a problem hiding this comment.
This array is not ESIMD specific. If usage of it creates problems, then we can wait till the next ABI breaking window.
I don't see any urgency in removing the enum now.
uditagarwal97
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
uditagarwal97
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
uditagarwal97
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
@v-klochkov I've reverted the ABI-breaking change for the time being. If rest of the changes looks good, could you please approve this? |
This PR removes esimd_emulator from device filters and filter selectors. Esimd_emulator is already depreciated.