[libclc] Stop installing CLC headers

[frasercrmck]

The libclc headers are an implementation detail and are not intended to be used by others as OpenCL headers. The only artifacts of libclc we want to publish are the LLVM bytecode libraries.

As the headers have been incidentally broken by recent changes, this commit takes the step to stop installing the headers at all. Downstreams can use clang's own OpenCL headers, and/or its -fdeclare-opencl-builtins flag.

Fixes #119967.

[frasercrmck]

CC @karolherbst.

[frasercrmck]

This doesn't look right either but I don't know enough about pkg-config files. I wouldn't expect this to work with a libexecdir full of LLVM bytecode files.

[karolherbst]

One thing to note is, that clang's internal opencl headers need a bit of workarounds to get them properly working with certain use cases.

Like what we need to do is to undefine __SPIR__ and __SPIRV__ explicitly, if our target doesn't support images then __IMAGE_SUPPORT__ needs to be undefined as well (though I think that's clang internal, not the headers).

The headers enable a few extensions automatically due to the spirv target. But I think that can't be fixed without breaking other users here.

Might be good to document some of it if it hasn't happen already. Though it might also be that mesa is one of the few projects caring about any of this, because we compile from OpenCL C down to SPIR-V, not any hardware target.

Anyway, something I wanted to mention before I forget.

[arsenm]

I'm surprised any of the extension macros are defined in the builtin header, and they probably should not be there. The target info in clang records the extensions from the target, and I'd expect the predefined macros to be populated from there

[karolherbst]

The base header enables a bunch of extensions when __SPIRV__ or __SPIR__ is found, and that define is set automatically when a SPIRV or SPIR tripple is used.

Maybe there is a better tripple to be used in this case? However we do want to compile to SPIR-V (with the native target in the future anyway), it's just it enables a bunch of things automatically.

I think it's fine to do it that way, it's just a bit of surprising behavior if you want to have tight control over what extensions are enabled or not.

[karolherbst]

Forgot to drop a link to the lines in question: https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/opencl-c-base.h#L16

[arsenm]

Maybe there is a better tripple to be used in this case? However we do want to compile to SPIR-V (with the native target in the future anyway), it's just it enables a bunch of things automatically.

I'm not sure I understand what the issue is. The defines here just declare that the extension is available, it does not enable them. You need to use the pragma to actually enable the extension (which the header does to enable required types later on in that file for declarations, but it cleans up after itself with #pragma OPENCL EXTENSION all : disable at the end

clang already does have -cl-ext for enabling / disabling extensions on the command line, I'm not sure if we have an equivalent for changing the target's reported set

[karolherbst]

Maybe there is a better tripple to be used in this case? However we do want to compile to SPIR-V (with the native target in the future anyway), it's just it enables a bunch of things automatically.

I'm not sure I understand what the issue is. The defines here just declare that the extension is available, it does not enable them. You need to use the pragma to actually enable the extension (which the header does to enable required types later on in that file for declarations, but it cleans up after itself with #pragma OPENCL EXTENSION all : disable at the end

clang already does have -cl-ext for enabling / disabling extensions on the command line, I'm not sure if we have an equivalent for changing the target's reported set

ah yeah, that's what I meant. Like if you are a normal CLI user and compile to SPIR-V I think it's reasonable to do it like that, just for users like mesa who actually implement an OpenCL runtime, we have to be very selective about what to advertise to conform to the OpenCL and OpenCL C spec as the official conformance test suite checks if API queries match what defines are enabled at compile time at runtime.

So far simply adding __SPIR__ and __SPIRV__ to the undefs solves this problem for us. But if there are others doing similar things, they might want to know about this, though most might just compile to a hardware target and never run into this issue in the first place.

[arsenm]

I suppose we could interpret the environment part of the triple to influence the target's reported set of extensions (plus consolidate all of these defines into the compiler). Do we even get consistent behavior when using -fdeclare-opencl-bulitins? I could believe we could just delete the header defines without any issue

[frasercrmck]

Would we want to make this part of the LLVM 20 release if we merge this PR in on time? We didn't announce its deprecation in advance, but the headers are currently unusable.

[frasercrmck]

ping. Are the discussions above blocking the PR?

If clang's internal headers are unsuitable must we fix those before landing this, or temporarily fix the installation of libclc's headers instead?

[arsenm]

I'm fine removing the install, but I don't know how mesa is using this these days. The clang builtin header issues seem like a separate problem

[frasercrmck]

Who might be best to ping about that, do you know?

[frasercrmck]

CC @nikic

[frasercrmck]

It looks that with https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33805 mesa is no longer using the libclc headers.

[nikic]

This should be okay from the mesa side now. https://github.com/search?q=%22include+clc%2Fclc.h%22&type=code has some other mentions of clc/clc.h includes, but from a cursory looks all of those look very old.

[frasercrmck]

/cherry-pick a2b0576

[llvmbot]

/pull-request #130017