[SYCL] Require building with zstd library

[ph0b]

SYCL device image compression requires zstd.
Running clang++ -fsycl --offload-compress with a version of the compiler compiled without zstd leads to an error.
Instead of complicating the handling of device image compression to also make it optional, it's cleaner to avoid compiling without zstd library, and to make it a required dependency for SYCL.
Using LLVM_ENABLE_ZSTD=FORCE_ON prevents from compiling without zstd.

An alternative has been discussed in #17914

[uditagarwal97]

As I mentioned in #17914 (comment),

We can not change to FORCE_ON because internally, not all of our development/testing machines has zstd installed. Setting FORCE_ON would break DPCPP build when zstd is not present.

Proceeding with this change, would break DPCPP build for a lot of folks. Also, our current behavior is aligned with upstream LLVM as well - don't throw if zstd is not available.

[ph0b]

zstd doesn't need to be installed on test machines since it's built statically.

For build machines, sure that's a new required build dependency but that's not the first time a new one is introduced, so how is it a blocker?

If the alternative you propose is to put the burden in documentation and on developers and 3rd party applications to defensively try it and check for errors, that sounds just more blocking.

[uditagarwal97]

zstd doesn't need to be installed on test machines since it's built statically.

True.

For build machines, sure that's a new required build dependency but that's not the first time a new one is introduced, so how is it a blocker?

zstd is not a required build dependency, it's optional, just like in upstream LLVM. The blocker comes from our internal infrastructure, getting hundreds of machines updated to have zstd installed will take time and IMO, it's more cumbersome than you having installed zstd locally when providing custom DPCPP package to Blender.

If the alternative you propose is to put the burden in documentation and on developers and 3rd party applications to defensively try it and check for errors, that sounds just more blocking.

All our opensource and OneAPI official compiler packages builds compiler with zstd. So, 3rd party applications using official DPCPP release packages don't have to do any additional check.
If you are providing a custom DPCPP package to Blender, you should ensure that (1) either the compiler is build with zstd or (2) Blender checks the compiler package to see if --offload-compress is supported.

[sherholz-intel]

@uditagarwal97 maybe I can share my experience from a user perspective of this problem/behavior of the DPCPP compiler, since I encountered this problem and pinged @ph0b.

I tried compiling the latest Blender code, which has --offload-compress using my own build of the latest DPCPP compiler.
I build the compiler on my Ubuntu 24.04 where zstd and libzstd is installed but not libzstd-dev.
Building DPCPP went fine so I wanted to compile Blender (no error what soever that libzstd-dev is missing).

So I went on trying to compile Blender which as I mentioned before uses --offload-compress and the compiled failed.
The error message included a complain that zstd cannot be found or isn't installed, but as I mentioned before in my case zstd is installed.
So as a user not knowing what is the actual root cause I automatically assumed the feature is broken in current DPCPP version, generating the impression of an unstable main branch.

Fortunately I knew @ph0b and could ping him. But as an external user I would have probably stop trying DPCPP and skipped trying out SYCL altogether and would stick to CUDA.
If we want people to adopt DPCPP we should try our best to avoid such negative user experiences.

[steffenlarsen]

Thank you, @sherholz-intel ! Getting a sense for the user's experiences (good or bad) helps us understand the issue and hopefully avoid too much tunnel vision. Please keep it coming! 😄

@uditagarwal97 - Could we make FORCE_ON the default of a new configure.py option? That way we can let developers easily turn it off if they don't want to resolve the dependency. We should also open an internal request to have zstd installed on relevant internal machines.

[aelovikov-intel]

Could we make FORCE_ON the default of a new configure.py option?

Maybe make it default to FORCE_ON even in CMake, but provide an option to switch to just ON with explicit buy-in (-DSYCL_ALLOW_NO_ZSTDLIB). Then downstream build (or any developer who knows what they're doing) can use that.

[uditagarwal97]

Hi @sherholz-intel ,

Thanks for sharing your experience. Based on your and @ph0b's feedback, I've created a pull request (#17990) to improve --offload-compress documentation and error message. I've also added a section in our getting started guide on how to build compiler with zstd support. I hope these changes will alleviate some of your pain points.

Could we make FORCE_ON the default of a new configure.py option?

Maybe make it default to FORCE_ON even in CMake, but provide an option to switch to just ON with explicit buy-in

I'd prefer a new configure.py option to force zstd linkage over changing to FORCE_ON in the CMakefile directly. Mainly because, once SYCL is available in the upstream LLVM, they will have our current behavior (zstd being optional). In #17990, I've added a new configure.py option to force zstd linkage. If we still want to flip the switch and make zstd a required dependency for DPC++, IMO, we should also discuss this change with the LLVM community as zstd is also used in other places (like in HIP toolchain). I could happily initiate this discussion with the LLVM community, if needed 😄 .