[SYCL][Bindless][ABI-Break] Remove deprecated functions and structs

[cppchedy]

• remove the following deprecated functions and their overloads:

  • read_image
  • read_mipmap
  • map_external_memory_array
  • free_mipmap_mem
  • alloc_mipmap_mem
  • free_image_mem

• remove deprecated structures in bindless_images_interop.hpp

• remove piextMemImportOpaqueFD and piextImportExternalSemaphoreOpaqueFD

[cppchedy]

friendly ping. @intel/dpcpp-nativecpu-pi-reviewers, @intel/llvm-reviewers-cuda, @intel/llvm-reviewers-runtime, @MartinWehking and @steffenlarsen. Thanks

[PietroGhg]

Native CPU lgtm, thank you

[kbenzie]

UR LGTM

[cppchedy]

friendly ping @intel/llvm-reviewers-cuda, @intel/llvm-reviewers-cuda, @MartinWehking and @steffenlarsen. thanks

[MartinWehking]

LGTM

[cppchedy]

friendly ping @intel/llvm-reviewers-runtime and @steffenlarsen. thanks

[steffenlarsen]

Sorry, this one flew under my radar! Due to this touching PI interfaces, I suspect it will conflict strongly with #14145. Since the ABI breaks related to this is in experimental interfaces, it is not strictly restricted by the ABI-breaking window, so I would prefer that we prioritize #14145 getting merged.

[ProGTX]

Sorry, this one flew under my radar! Due to this touching PI interfaces, I suspect it will conflict strongly with #14145. Since the ABI breaks related to this is in experimental interfaces, it is not strictly restricted by the ABI-breaking window, so I would prefer that we prioritize #14145 getting merged.

I'm not sure we're able to skirt the ABI restrictions, though. Because if we remove symbols from the SYCL library, that breaks ABI for all users, not just ones using Bindless Images. So that means if we don't get the patches in this window, we need to wait until the next major release.

We realize #14145 is very important, however all of the Bindless Images patches have been thoroughly reviewed and are basically ready to merge. One option that we could do is to collate all of our patches into one big PR, that should reduce potential conflicts with #14145 , though I'm a bit skeptical about collecting approvals for everything again. Do you think it's worth doing that?

[steffenlarsen]

I'm not sure we're able to skirt the ABI restrictions, though. Because if we remove symbols from the SYCL library, that breaks ABI for all users, not just ones using Bindless Images. So that means if we don't get the patches in this window, we need to wait until the next major release.

Feel free to prove me wrong, but I do not believe this is correct. As long as an application does not use these symbols, they should not be affected by these symbols disappearing. Keep in mind, it is not just my opinion that experimental ABI-breaking changes are allowed outside ABI-breaking windows, it is mentioned in https://github.com/intel/llvm/blob/sycl/sycl/doc/developer/ABIPolicyGuide.md#changing-abi.

Of course, this does not mean we should make them all the time, but it means for changes like these we are not as heavily restricted.

We realize #14145 is very important, however all of the Bindless Images patches have been thoroughly reviewed and are basically ready to merge. One option that we could do is to collate all of our patches into one big PR, that should reduce potential conflicts with #14145 , though I'm a bit skeptical about collecting approvals for everything again. Do you think it's worth doing that?

I understand that, but based on what I mention above, #14145 is fighting time more than this patch is.

I do not believe that combining them into a single PR will do much to aid the conflict issue, apart from maybe making it fewer iterations for the maintainers in #14145. I suspect rebasing these will only really require re-review from runtime reviewers, so I will happily prioritize doing these re-reviews if you ping me on them or send me a list of PRs.

[steffenlarsen]

Tag @aarongreig & @kbenzie - If you are okay with these proceeding, we can proceed, but there will be more conflicts to resolve in #14145 if we do. Same applies to #14444 and #14140.

[steffenlarsen]

Spoke to @kbenzie and it sounds like #14140 and this can go in and we can do the rebasing. However, merging #14140 has caused conflicts in this.

[cppchedy]

However, merging #14140 has caused conflicts in this.

@steffenlarsen thanks for pointing that out. It should be resolved now.

[cppchedy]

@intel/llvm-gatekeepers This can be meged. Thanks.

[seifsg]

@cppchedy LGTM