SPIR target enables all CL optional extensions

Hi,

I'm trying to bring back up the mesa CL 3.0 support at the moment, and
running into a problem I'm not sure how to solve.

There has been some work around optional CL 3.0 features at the
compiler level, but one of the things is the SPIR backend now always
enables all optional CL 3.0 features, but for current mesa I'd like to
expose CL3.0 with no optional features but still use the SPIR target.

For example now tests like ./basic/test_basic vector_swizzle

testing type char3

ends in a lot of these.

Build log for device "llvmpipe (LLVM 13.0.0, 256 bits)" is: ------------
input.cl:13:5: error: no matching function for call to 'vstore3'
/home/cldrv/lib/clang/13.0.0/include/opencl-c.h:11502:13: note:
candidate function not viable: cannot pass pointer to address space
'__global' as a pointer to address space '__private' in 3rd argument

This is because the generic address space defines are being enabled
for all SPIR targets, for our use case I'd rather control that from
the Mesa side or have some way to restrict these options, is something
like that available?

Dave.

Jesse set me straight on irc.
c->getTargetOpts().OpenCLExtensionsAsWritten.push_back("-__opencl_c_generic_address_space");

sprinkling a bunch of these around gets me going.

Dave.

Correct some of the feature macros that add features through the libraries are
now unconditionally defined in the internal header files. Although clang was
always setting all optional features to supported ones for generic targets like
SPIR however there was a frontend-only flag ‘-cl-ext’ that allowed to override
this.

We have started the discussion to restore this functionality for library-based
features and here are two short-term approaches that have been discussed so
far:

  1. Use ‘-D__undef_’ flag to skip defining the macros. I have
    created an RFC patch to demonstrate the idea earlier:
    https://reviews.llvm.org/D91531

  2. Extend ‘-cl-ext’ interface to handle such features.
    Anton has been looking at this in:
    https://reviews.llvm.org/D103241?id=348222#inline-97980

Overall, originally SPIR has been added to clang as a device-agnostic target in
a primary use case. But I feel in a long term we should consider improving the
design by adding an ability to specify a concrete device or maybe a class of
devices compiled for. Otherwise, I don’t think always enumerating the list of
features/extensions using ‘-cl-ext’/’-D’ with each clang invocation is extremely
usable. So this area definitely needs some improvements.

FYI this topic is being discussed in the scope of OpenCL Tooling
teleconferences within Khronos. The next session is on Thu next week.

Cheers,
Anastasia

Correct some of the feature macros that add features through the libraries are
now unconditionally defined in the internal header files. Although clang was
always setting all optional features to supported ones for generic targets like
SPIR however there was a frontend-only flag '-cl-ext' that allowed to override
this.

We have started the discussion to restore this functionality for library-based
features and here are two short-term approaches that have been discussed so
far:

1. Use '-D__undef_<feature name>' flag to skip defining the macros. I have
created an RFC patch to demonstrate the idea earlier:
https://reviews.llvm.org/D91531

2. Extend '-cl-ext' interface to handle such features.
Anton has been looking at this in:
https://reviews.llvm.org/D103241?id=348222#inline-97980

Overall, originally SPIR has been added to clang as a device-agnostic target in
a primary use case. But I feel in a long term we should consider improving the
design by adding an ability to specify a concrete device or maybe a class of
devices compiled for. Otherwise, I don't think always enumerating the list of
features/extensions using '-cl-ext'/'-D' with each clang invocation is extremely
usable. So this area definitely needs some improvements.

Yes for mesa/clover, we don't want SPIR to be device-agnostic, we want
to give clang the list of supported extensions and have
opencl-c.h work with that. At the moment even if I give it the
supported extensions I expect I'd fall over the __SPIR__ turns
on all exts in opencl-c.h.

Dave.

FYI this topic is being discussed in the scope of OpenCL Tooling
teleconferences within Khronos. The next session is on Thu next week.

I'll keep an eye on this, probably not timezone compatible with me.

Dave.

Was there any followup on this from the Tooling teleconf?

I'm back at having to remove the #if (__SPIR__) define everything as I
want to use
SPIRV internally in my compiler without all the extensions defined.

Dave.

Hi David,

We were slightly inclined towards option 2 for now which means if a library-only feature is passed to ‘-cl-ext’, the frontend will implicitly define a macro. We will probably want to add a warning for this or something but the details can be refined in the review.

However, as a temporary solution, we could also go for option 1 at it might be easier to implement.

If you would like to create a patch I or Anton can take a look and review it.

Cheers,
Anastasia