scott.linder added a comment.

In D77923#1978261 <https://reviews.llvm.org/D77923#1978261>, @arsenm wrote:

> In D77923#1978172 <https://reviews.llvm.org/D77923#1978172>, @scott.linder 
> wrote:
>
> > https://www.khronos.org/registry/OpenCL/sdk/2.0/docs/man/xhtml/preprocessorDirectives.html
> >  describes `__OPENCL_VERSION__` as "an integer reflecting the version 
> > number of the OpenCL supported by the OpenCL device." as contrasted with 
> > `__OPENCL_C_VERSION__` which is described as "an integer reflecting the 
> > OpenCL C version specified by the -cl-std build option to clBuildProgram or 
> > clCompileProgram. If the -cl-std build option is not specified, the OpenCL 
> > C version supported by the compiler for this OpenCL device will be used."
> >
> > But I don't see where the correct use of these is defined? How should the 
> > user decide which to use? It does seem like `__OPENCL_VERSION__` and 
> > `__OPENCL_C_VERSION__` can differ, e.g. if you compile for a device 
> > supporting 2.0 with `-cl-std=1.2`, but why would `__OPENCL_VERSION__` ever 
> > be referenced then?
>
>
> I don't know why you would ever use this; but the standard says it should be 
> defined, so I guess we have to define it


Right, I just want to understand the actual semantics and it doesn't seem 
clear. I agree that I don't see a use for `__OPENCL_VERSION__` but we define it 
so I'm sure people are using it, so likely we need to be careful that this 
patch preserves the existing behavior of the runtime.

In D77923#1976497 <https://reviews.llvm.org/D77923#1976497>, @jvesely wrote:

> OPENCL_VERSION is something that should be really set by the opencl driver 
> rather than the compiler.
>  coarse-grained SVM can be done without FEATURE_FLAT_ADDRESS_SPACE, so those 
> gpus can get over 1.2, while the compiler can be used in an implementation 
> that doesn't go above 1.2 even for gfx700+


I don't know why this would imply we can't just have a static 
`__OPENCL_VERSION__` for each device in Clang? I don't see anything in the 
standard that says you are not allowed to have (going with your example) 
`__OPENCL_VERSION__=200` and `__OPENCL_C_VERSION=120` at the same time.

It seems like we should be able to just have `__OPENCL_VERSION__` defined in 
Clang, and let the runtime control setting `__OPENCL_C_VERSION__` via 
`-cl-std=`. If the current patch sets `__OPENCL_VERSION__` differently than the 
runtime does then it seems like we should make it match, but I'm not sure how 
best to confirm that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77923/new/

https://reviews.llvm.org/D77923



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to