Discuss rfc for removing vulkan SDK

[coldav]

Currently in #655, will update after merging details.

[coldav]

Now merged - see https://github.com/uxlfoundation/oneapi-construction-kit/blob/rfcs/rfc-0003.md.
Now at the discussion phase.

[hvdijk]

It also leads to strange dependency such as the mutex which doesn't really belong in the compiler_pipeline passes.

Although I agree it does not belong there, even with Vulkan we should be able to move that into compiler-base? I don't think this is an argument either way on Vulkan removal.

Remove everything under source/vk and from all documentation that refers to it, as well as any github workflow or docker requirements such as installing the vulkan SDK.

We also have some code outside of source/vk that is only used for Vulkan, we need some way of identifying that code and removing it as well.

Since the Vulkan API will no longer be supported, there will be no backward compatibility in this regard.

This should probably mention that if any interested user wants to use this, they can continue to use the 4.0 release. What will be our policy on bug reports against it?

Rejected Ideas

This block is a bit hard to read and the last sentence suggests that that idea has not been rejected.

[coldav]

I agree we could possibly look at moving it into compiler-base, but I had issues trying to do that (can't recall the details). Regardless, it shows that it has tendrils into the wider codebase that has led to making certain changes less trivial (which was the main reason it wasn't moved during the restructuring of compiler pipeline).

Wrt "Rejected Ideas" I am rejecting it, but I am also offering an alternative if we decided to keep it but have some level of testing. I can make that clearer.

Happy to add about 3.0, and try to make "Rejected ideas" cleaner.

[coldav]

Suggest removing "We could do a very specific test overnight though as a possibility if we want to keep it disabled by default." which seems to add nothing. I think the rest of that paragraph could be left unchanged?

[coldav]

Suggest updating "It also leads to strange dependency such as the mutex which doesn't really belong in the compiler_pipeline passes." to "It can lead to additional requirements on the main part of ock, such as the mutex in the compiler_pipeline passes or descriptor binding lists. Regardless of how hard it would be to remove these, they do add to the general overhead and cleanliness of the codebase.

[hvdijk]

Suggest removing "We could do a very specific test overnight though as a possibility if we want to keep it disabled by default." which seems to add nothing. I think the rest of that paragraph could be left unchanged?

Agreed, it starts as "We could keep it as is but have it disabled by default and only test it in our nightly testing" which looks to me like the same thing, and that is followed by why you are rejecting it.

"It can lead to additional requirements on the main part of ock, such as the mutex in the compiler_pipeline passes [...]. Regardless of how hard it would be to remove these, they do add to the general overhead and cleanliness of the codebase.

Yes, but that is still not an argument in favour of Vulkan removal (nor one against it) in my opinion. It would take some work to clean it up, but that work would be the same regardless of Vulkan. I think we're better off not mentioning this.

or descriptor binding lists

This is a good point though, and applies to anything else that is specific to shaders.

[coldav]

okay, I'll drop the additional requirements part, but say
"It can lead to additional requirements on the main part of ock, such as descriptor binding lists."

[hvdijk]

Thanks

[coldav]

Suggest "Anything that exists purely for the sake of Vulkan should also be
removed, such as the descriptor binding lists. This can be analysed by reviewing
the removed Vulkan code for dependencies." under Specification section.

[coldav]

Suggest "This will no longer be supported beyond the 4.0.0 release and no bugs will be fixed against it wrt to Vulkan." added to Specification.

[hvdijk]

I those changes are probably good in principle, we can discuss the exact wording in the pull request. For 4.0.0, supported but bugs will not be fixed, what does that mean in practice? If someone tries to use it and cannot because of a showstopper bug, what do we do?

[coldav]

I mean we don't support Vulkan SDK fixes. They can fork it if they want and any fixes and fix it themselves.
(I don't believe this is a real problem though).

I could add that "if users want Vulkan SDK fixes they would need to fork and fix it themselves"

[hvdijk]

But that means it won't be supported (by us) even in the 4.0.0 release. Which is probably fine, but the opposite of how I read your wording.

[coldav]

I'll try and come up with wording to that affect of users requiring to support it themselves.

[hvdijk]

We've discussed this and we're happy to go ahead with it, we can mark this as accepted.