OpenCL-Docs
OpenCL-Docs copied to clipboard
KhronosGroup
[OpenCL C 3.0] Clarify use of extension pragma (#82).
This change adds clarification for the compiler directive that enables and disables the extensions.
Summary:
- Add high-level description of OpenCL specific pragma directives into the core spec.
- Clarify the use of extension pragma in the extension spec.
- Clarify the need of extension pragma directive in cl_khr_fp64 and cl_khr_fp16.
Coming from an end-user perspective trying to do the minimal amount of thinking that results in portable code, if there is an accompanying pragma to any extension I'm using, I'll put it there, whether it's mandated or not. That is the defensive behavior. If some 1.2, 2.x extensions mandated it, but 3.0 extensions allow omission, I'll still add the pragma cause then I don't have to go back and check when the pragma was introduced.
If the spec will allow omission of new extensions going forward, due to mighty useful non-language altering 1.x extensions such as printf still having an associated pragma, people wanting to write portable code will likely still default to the no-brainer policy of stamping out the pragma anyway.
@MathiasMagnus If we accept the pragma optionally by default it will cover your use case isn't it? But also allows more flexibility for implementations or developers that prefer to be more specific. As a matter of fact unknown pragmas are always just ignored by the compiler so feel free to add them as you feel.
FYI the suggestion is not to make the extensions behave differently in various versions of OpenCL C. The idea is to fix the behaviour for each of them across all OpenCL C versions. I guess the main benefit is to simplify implementations of future extensions but we might be able to do something better for some existing ones too.
As a matter of fact clang has extensions that don't even have kernel language changes. Not clear why? I don't think we can scale if we add every single extensions into clang. We have quite a number of them.
I found an odd case where enabling the int64 atomic extensions is required to use the 64-bit C11 atomic functions while investigating a separate issue: https://godbolt.org/z/Kb3Pxf
My main reason for pointing this out is to say that specifying extensions pragmas to be optional after the fact is dangerous and runs the risk of breaking shipping implementations in unpredictable ways. I'd feel much more comfortable requiring a pragma by default, at least for shipping extensions, and then explicitly stating when a pragma is optional for newer extensions.
To continue @MathiasMagnus 's comment though, I wonder if there is a better way: What if a newer compiler supported a mechanism to enable all (or at least most?) extensions in one shot, without listing them individually?
One way to do this is to support:
#pragma OPENCL EXTENSION all : enable
This is explicitly disallowed today, though disabling "all" extensions is allowed.
Since enabling "all" extensions is also debatably dangerous, we could instead define a subset of extensions - the ones that don't impact compiler internals - and support enabling them in one shot. I haven't thought of the right noun for this, but the basic idea is something like:
#pragma OPENCL EXTENSION basic_subset : enable
The key is to add a new feature to simplify enabling extensions that could apply uniformly to new extensions and old extensions, vs. specifying rules for each extension. I think this would preserve compatibility with shipping kernels and implementations, while providing a simpler option for newer kernels targeting newer compilers.
What do you think?
I found an odd case where enabling the int64 atomic extensions is required to use the 64-bit C11 atomic functions while investigating a separate issue: https://godbolt.org/z/Kb3Pxf
This is just one example out of many. I don't know what adding extra types have to do with the pragma directives. We could easily make them available if extensions are supported. And their presence can be checked using extension macros. It is very sad that with time we have added so many extensions with pragmas that do nothing but complicate developer's life and add extra code in the implementation. :(
My main reason for pointing this out is to say that specifying extensions pragmas to be optional after the fact is dangerous and runs the risk of breaking shipping implementations in unpredictable ways. I'd feel much more comfortable requiring a pragma by default, at least for shipping extensions, and then explicitly stating when a pragma is optional for newer extensions.
Yes, this should be more conservative i.e. in case implementations don't provide the pragma it would just be ignored at the price of an extra warning. However, I think we should encourage to specify the use of pragma explicitly and especially for the future extensions. Alternatively, we can omit the default behavior and just rely on each extension to explicitly specify the pragma use. If nothing is specified about the pragma it will just comply to old behavior. I would still go through all the published non-vendor extensions and attempt to define the behavior explicitly.
Since enabling "all" extensions is also debatably dangerous, we could instead define a subset of extensions - the ones that don't impact compiler internals - and support enabling them in one shot.
I presume this has been discussed already?
The key is to add a new feature to simplify enabling extensions that could apply uniformly to new extensions and old extensions, vs. specifying rules for each extension. I think this would preserve compatibility with shipping kernels and implementations, while providing a simpler option for newer kernels targeting newer compilers. What do you think?
Interesting, I am keen to explore this idea further. Do we want to specify this list or leave it defined by implementation to allow the inclusion of vendor extensions too? We could also think of a combined approach. I guess this would be an additional utility though? I would still attempt to specify the behavior per extension too.
I have looked at this topic for a bit longer and I think we should require to describe how the pragma changes the compiler state otherwise it is a black magic for the developers and tooling so it doesn't really help to just state whether or not the pragma is needed.
If we look at other pragmas i.e. FP_CONTRACT, it is explicitly explained what it actually does. For the extensions however, I can only find some information for cl_khr_fp16. I believe that the lack of information has led to misunderstanding and misinterpretations that has resulted in inadequate implementation that I would like to correct as much as possible.
I feel that many extensions have been implemented incorrectly wrt pragma. For example, we not only add pragma for nearly every extension we also guard the use of identifiers by the pragma i.e. code using types or functions from the extensions would only be compiled if pragma enable was added. However, I don't see how it is doing anything but bringing extra complexity and inconvenience. Perhaps, the confusion was due to some early extensions for example cl_khr_fp64 was used to alter the state of parser that would map double from invalid reserved identifier into a valid one or it would change the floating-point literal into a double type constant instead of float. Although the use of double type could still be accepted without the need of pragma.
Currently, there are plenty of extensions that add new types or functions before parsing the source code from extensions supported by the target. However, this is done during the compiler setup, before parsing. When pragma is actually parsed nothing changes for such extensions because the types have already been added previously. Generally, C/C++ doesn't allow to disable the identifiers. So we wouldn't have a way to load and unload them dynamically w/o significant changes in the architecture of the compiler. The only thing we do with pragma for those extensions is to accept/reject the code that uses the identifiers i.e. types or functions when pragma enabled/disable.
Earlier example illustrates what I am describing here:
I found an odd case where enabling the int64 atomic extensions is required to use the 64-bit C11 atomic functions while investigating a separate issue: https://godbolt.org/z/Kb3Pxf
Another possible scenario is if we had the same identifiers in multiple extensions then we could use pragma to resolve which extensions identifier to use from, so it would be some sort of namespace. However namespaces are not available in C99 and therefore we can't add multiple identifiers with the same name.
I don't see a need for pragma in those use cases. And if nobody else can highlight where this could be useful the right action for upstream implementation would be to just remove the need for pragmas and also the extensions that don't need the compiler support from clang. Note that this will largely not affect backward compatibility of existing code, but an extra warning for unknown pragmas might appear in some source code. There has never been clear description of how and where the pragma is used so perhaps a warning won't be surprising behavior after all.
Should there also be some clarification in here about the interaction between pragma directives and preprocessor macros? For example, AIUI:
#pragma OPENCL EXTENSION cl_khr_fp64 : disable
#ifdef cl_khr_fp64
// Code here is actually compiled
#endif
I.e., I think the intent is that preprocessor macros communicate to the OpenCL-C kernel what extensions are available, and kernels use pragmas to indicate which extensions they want to use. What might not be immediately obvious to a developer is that the preprocessor runs before pragmas are evaluated, so a pragma ... disable doesn't undef the extension macro.
I have made some investigation and I realized that the majority of extensions will have a section with the pragma in the same exact format regardless of whether they even add any change to OpenCL C. However, neither individual extensions nor extensions spec or language spec explain whether and how the pragma is to be used. Moreover the extensions spec and language spec doesn't contain the pragmas for any extension, so only individual extension specs do. That means that neither upstream implementation in Clang nor existing kernels using pragmas can be conformant. My guess is that the pragma in the specification is a result of copy-paste. These are the only extensions that somehow refer to the potential use of pragmas: cl_khr_fp16, cl_khr_int64_base_atomics, cl_khr_int64_extended_atomics. Those are the wording in the spec.
Unless the cl_khr_fp16 extension is supported and has been enabled. The half scalar and vector types can only be used if the cl_khr_fp16 extension is supported andhas been enabled. The double scalar and vector types can only be used if double precision is supported. The atomic_long and atomic_ulong types are supported if the cl_khr_int64_base_atomics andcl_khr_int64_extended_atomics extensions are supported and have been enabled. The atomic_double type is only supported if double precision is supported and the cl_khr_int64_base_atomics and cl_khr_int64_extended_atomics extensions are supported andhave been enabled. If the device address space is 64-bits, the data types atomic_intptr_t, atomic_uintptr_t,atomic_size_t and atomic_ptrdiff_t are supported if the cl_khr_int64_base_atomics andcl_khr_int64_extended_atomics extensions are supported and have been enabled.
The problem I have with those is that I don't understand what exactly should happen when the pragma is enabled and disabled. After several discussions with the developers I realised that there are multiple interpretations, including the one implemented in clang that in my opinion doesn't help but only makes things complicated. In addition the upstream implementation is inconsistent i.e. (1) pragma is required for some but not the others and (2) the same extension can be implemented differently in two default headers.
To resolve this I suggest the following steps:
- Clarify general use of extension pragma to avoid future mistakes and confusions: https://github.com/KhronosGroup/OpenCL-Docs/pull/355
- Identify in what extensions the pragma is needed and add clarification to each of those i.e. what happens when pragma is disabled/enabled.
- Add clarifications to the existing extensions that don't need pragma but list it in the extension spec that the pragma is only accepted for backward compatibility reasons but doesn't change any functionality. I.e. new kernels won't need to add it but the existing kernels will continue to compile without the warnings about the unknown pragmas.
- Modify upstream implementation according to the clarified spec.
Should there also be some clarification in here about the interaction between pragma directives and preprocessor macros? For example, AIUI:
#pragma OPENCL EXTENSION cl_khr_fp64 : disable #ifdef cl_khr_fp64 // Code here is actually compiled #endifI.e., I think the intent is that preprocessor macros communicate to the OpenCL-C kernel what extensions are available, and kernels use pragmas to indicate which extensions they want to use. What might not be immediately obvious to a developer is that the preprocessor runs before pragmas are evaluated, so a
pragma ... disabledoesn't undef the extension macro.
Ok, right now the clarification says that each pragma should explain what it changes in the standard behavior. So it is implying there is no change by default i.e. it doesn't affect any macros. Although I see some value in being explicit with respect to some common cases. I will try to refine the wording to explain this explicitly. Thanks for the input!
Thanks, these latest changes look very good and we could have avoided a lot of confusion if we adopted this policy from the beginning.
I've left a few specific comments on the changes themselves and I have a few general questions:
1. Should `cl_khr_fp16` also require an extension pragma, specifically to enable half-precision literals such as `1.0h`? See, for example: https://godbolt.org/z/brMWTn
We could do. For now I only wanted to specify the pragma where we know that it is needed. For the half literal, I don't really understand what benefit does the pragma bring. If cl_khr_fp16 is supported 1.0h will always be parsed as a half literal. The only difference that with pragma enabled it is parsed without an error but with pragma disabled it is parsed with an error. I find this extra step of enabling not only useless but also complicated. If the type is supported it should just be possible to use it without any pragma. If it is not supported then the error is given regardless of whether pragma is enabled or not. This is how extensions normally work.
2. How confident are we that no other implementations require other extension pragmas, either rightly or wrongly? I'm still worried that these changes will cause shipping implementations to start rejecting kernels that are valid according to the spec after these changes, but that were debatably invalid beforehand because they are missing extension pragmas. * See previous comment here: [#355 (comment)](https://github.com/KhronosGroup/OpenCL-Docs/pull/355#discussion_r457707521) * Also see previous comment here: [#355 (comment)](https://github.com/KhronosGroup/OpenCL-Docs/pull/355#issuecomment-662260546)For (2), do we know which extensions currently require an extension pragma in Clang? Is it tractable to update the descriptions of these extensions to say something to the effect of: These extensions shouldn't need an extension pragma, and newer implementations may not require an extension pragma, but some implementations do require an extension pragma so it should be included for maximum portability?
Ok, right now my intention is not to fix any default behavior because either way we fix it we can't have any prove that no vendor is using the pragma differently. So my intention was to make the unspecified pragma an implementation-defined so to say. This is exactly what it is right now. However, perhaps my current wording doesn't reflect this well as it only states that the implementations are allowed to ignore pragmas:
Implementations are not required to accept extension pragmas if their behavior is not specified explicitly.
Do you think we should add something like:
Implementations are allowed to require the pragma even if its behavior if not specified.?
Regarding the individual extensions in Clang if you check this RFC
https://lists.llvm.org/pipermail/cfe-dev/2020-September/066911.html
only extensions in list I seem to require the pragma right now. But I think even in that list many don't need the pragma, it has just been implemented incorrectly. In my refactoring proposal, I would like to remove the dependency on the pragma in most of the extensions but keep the pragma for backward compatibility only so that will be parsed by ignored (https://reviews.llvm.org/D91534). For now the only extension where I find the pragma useful is cl_khr_fp64.
I don't want to feature-creep this PR, but I think the observations in this comment are good and we should fix them, perhaps by refactoring the intro section in the extensions spec: https://github.com/KhronosGroup/OpenCL-Docs/pull/355#issuecomment-721245003
I think the intent is that preprocessor macros communicate to the OpenCL-C kernel what extensions are available, and kernels use pragmas to indicate which extensions they want to use. What might not be immediately obvious to a developer is that the preprocessor runs before pragmas are evaluated, so a pragma ... disable doesn't undef the extension macro.
@etomzak would you mind creating an issue to track this so it doesn't slip through the cracks? Thanks!
I don't want to feature-creep this PR, but I think the observations in this comment are good and we should fix them, perhaps by refactoring the intro section in the extensions spec: #355 (comment)
I think the intent is that preprocessor macros communicate to the OpenCL-C kernel what extensions are available, and kernels use pragmas to indicate which extensions they want to use. What might not be immediately obvious to a developer is that the preprocessor runs before pragmas are evaluated, so a pragma ... disable doesn't undef the extension macro.
@etomzak would you mind creating an issue to track this so it doesn't slip through the cracks? Thanks!
Btw I have covered this in my change to the extension spec:
There is no default behavior for the extension pragma directive that would apply to extensions universally e.g. it does not define or undefine an associated predefined extension macro; it does not add or remove function or type declarations.
Or do you think we need to add something more?
Btw I have covered this in my change to the extension spec:
Or do you think we need to add something more?
I think we're a lot closer but I think we could describe the intent behind the extension defines, probably by slightly refactoring the description in section 1.2 of the extension spec. It would make more sense to me at least to describe the extension #defines first, since they're always present, and then to describe the extension #pragmas next, for example.
I think we're a lot closer but I think we could describe the intent behind the extension defines, probably by slightly refactoring the description in section 1.2 of the extension spec. It would make more sense to me at least to describe the extension #defines first, since they're always present, and then to describe the extension #pragmas next, for example.
I've opened #514 to track this.
Hello, could you please take another look at my comments here https://github.com/KhronosGroup/OpenCL-Docs/pull/355#pullrequestreview-541524654 and ensure they're addressed?
I'm especially concerned about:
Extension pragma directives are optional and they are only required for extensions that change the compiler behavior i.e. the source code is interpreted differently when the pragma directive is used. Each extension should specify whether it adds the directive or not.
My previous comment describes three extensions that currently require a pragma to function correctly by all Clang-based compilers, but this PR does not update any of these extensions to say that a pragma is required.
Even with some recent cleanup there are a lot of extension pragmas that Clang currently accepts (link). How many of these extensions do we need to update to say that a pragma is required?
Hello, could you please take another look at my comments here #355 (review) and ensure they're addressed?
I'm especially concerned about:
Extension pragma directives are optional and they are only required for extensions that change the compiler behavior i.e. the source code is interpreted differently when the pragma directive is used. Each extension should specify whether it adds the directive or not.
My previous comment describes three extensions that currently require a pragma to function correctly by all Clang-based compilers, but this PR does not update any of these extensions to say that a pragma is required.
Even with some recent cleanup there are a lot of extension pragmas that Clang currently accepts (link). How many of these extensions do we need to update to say that a pragma is required?
I believe this is covered in my earlier reply https://github.com/KhronosGroup/OpenCL-Docs/pull/355#issuecomment-736621940
I was surprised to see a pragma for fp64 but not fp16 in the current iteration of this PR. I've read the extensive discussion above, and I'm not sure I have anything truly new to add. The rationale for keeping pragma's to a minimum seems to come down to:
Note that I don't think guarding the use of types or functions that are available if an extension is supported by a pragma is useful and I have therefore excluded those (see my earlier comment).
From a language design perspective I see the case for this. However, this change retroactively applies this definition to all extensions, implying a potential difference between current and future implementations. The version of the Intel GPU OpenCL driver that my laptop happens to have installed errors if trying to use cl_khr_fp16 functionality without the pragma enable. This PR says that pragma is optional, but at least one shipped implementation treats it as mandatory.
I'm not sure what the complete set of extensions are that falls into this category, but this thread seems to at least document fp16 and 64-bit atomics as being in this situation.
I was surprised to see a pragma for fp64 but not fp16 in the current iteration of this PR. I've read the extensive discussion above, and I'm not sure I have anything truly new to add. The rationale for keeping pragma's to a minimum seems to come down to:
Note that I don't think guarding the use of types or functions that are available if an extension is supported by a pragma is useful and I have therefore excluded those (see my earlier comment).
From a language design perspective I see the case for this. However, this change retroactively applies this definition to all extensions, implying a potential difference between current and future implementations.
Just to point out the intension for this PR is to document the current situation i.e. use of pragma is implementation-defined for most of extensions because the spec has never documented their use. I appreciate perhaps I've failed to deliver this message. Therefore I was thinking whether adding the following statement can help.
Ok, right now my intention is not to fix any default behavior because either way we fix it we can't have any prove that no vendor is using the pragma differently. So my intention was to make the unspecified pragma an implementation-defined so to say. This is exactly what it is right now. However, perhaps my current wording doesn't reflect this well as it only states that the implementations are allowed to ignore pragmas:
Implementations are not required to accept extension pragmas if their behavior is not specified explicitly.
Do you think we should add something like:
Implementations are allowed to require the pragma even if its behavior if not specified.?
I am trying to understand what is blocking this PR or general progress on #82 at the moment? What would be helpful if someone provides any feedback of what is to be changed and how? I will do my best to make it happen.
Just FYI perhaps I haven't made it clear - there are significant improvements in tooling that are blocked on this that the community can benefit including the implementation of OpenCL 3.0.
I am trying to understand what is blocking this PR or general progress on #82 at the moment? What would be helpful if someone provides any feedback of what is to be changed and how? I will do my best to make it happen.
Hi @AnastasiaStulova , my biggest concern is that kernels written against a version of the spec including these changes may fail against conformant implementations that were developed against a version of the spec without these changes. Specifically, our Clang-based implementation currently requires a pragma to enable an extension in at least three cases that are not described in this PR (and there may be more):
- this example for OpenCL C 2.0 64-bit atomics.
-
this example for
cl_khr_fp16 -
this example for
cl_intel_device_side_avc_motion_estimation
I very much agree that the current policies for extension pragmas are unclear, confusing, and/or complicated for tooling. The changes in this PR are a huge improvement, we just need to be sure we don't break shipping implementations while we're at it.
I very much agree that the current policies for extension pragmas are unclear, confusing, and/or complicated for tooling. The changes in this PR are a huge improvement, we just need to be sure we don't break shipping implementations while we're at it.
I am with you on that and this is why I have completely changed this PR to document that the behavior is implementation-defined. Do you agree with this? It just what it is now already, we can't fix the behavior based on what clang does, because we don't know what other implementations are doing or have been doing.
If the wording on the PR doesn't reflect the intention I am happy to change it and I have made the following suggestion.
Ok, right now my intention is not to fix any default behavior because either way we fix it we can't have any prove that no vendor is using the pragma differently. So my intention was to make the unspecified pragma an implementation-defined so to say. This is exactly what it is right now. However, perhaps my current wording doesn't reflect this well as it only states that the implementations are allowed to ignore pragmas:
Implementations are not required to accept extension pragmas if their behavior is not specified explicitly.Do you think we should add something like:Implementations are allowed to require the pragma even if its behavior if not specified.
Does it make sense?
If it makes things simpler I suggest to drop fp64 from this PR because it seems to confuse people. It was added originally with the initial wording and I kept it mainly because I know that the pragma is useful. But I am ok to leave it out and follow up on individual extensions in the following PRs.
Since this PR came up again in today's teleconference: If we want to get this over the finish line (and it still looks like most of the changes are really good, and exactly what we need), I think we need to add similar text that this PR adds for cl_khr_fp64 to the list of extensions described here:
- https://github.com/KhronosGroup/OpenCL-Docs/issues/82#issuecomment-635467096
- (also here: https://github.com/KhronosGroup/OpenCL-Docs/issues/82#issuecomment-785496025)
Rightly or wrongly, this is the set of extensions that we know will require an extension pragma on shipping implementations.
If we want to differentiate between the set of extensions that can cause a diagnostic with an unrecognized pragma vs. the set of extensions that cannot (described here: https://github.com/KhronosGroup/OpenCL-Docs/issues/82#issuecomment-786411529), then we'll need to update a few more extensions to say this.
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
Anastasia Stulova seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.
