18:37pmoreau: karolherbst: Re “clover: Ensure OpenCL C version matches requirements”: what do you think, should I just drop the patch for now given that we do not expose anything higher than OpenCL C 1.1 at the moment, and then come up with a patch taking care of the specific requirements that OpenCL C 3.0 has?
18:37pmoreau: Also, do you have the opportunity to test the vec3 patch on Fermi+/see if you get some regressions?
18:57karolherbst: pmoreau: uhm.. why is CL 3.0 the issue here?
18:57karolherbst: the CL C version is quite unrelated
18:57karolherbst: if we support CL 1.2 we could just expose it, but I assume we don't
18:57karolherbst: *CL c
18:58pmoreau: It’s more with having something that picks the advertised version as appropriate rather than hardcoding one, similar to what I ended up doing for the OpenCL version.
18:59karolherbst: yeah.. I mean, doesn't matter. If we fullfill all the reqs for CLC we are good to go. _But_ as our implementation isn't strictly compliant maybe we shouldn't? dunno
19:00karolherbst: we are not even 1.0 compliant so maybe it doesn't matter
19:00pmoreau: And the issue with CL 3.0 is whether CL C 1.2 gets exposed or a higher version, but for the higher version we need to check whether all optional features are supported.
19:00karolherbst: ohh, we don't
19:00karolherbst: CL C 2.0 is massive
19:01karolherbst: we already fail the image bits
19:01pmoreau: I know we don’t :-D It’s more with having checks in place so that if we ever do it will be automatically exposed.
19:01karolherbst: yeah and I think it's fine as long as we actually check everything
19:01karolherbst: but we should also cap it
19:02pmoreau: Cap it to what?
19:02karolherbst: like adding all the automatic stuff, but still don't let it go beyond 1.2 :D
19:02karolherbst: or 1.1
19:02pmoreau: Ah okay
19:02karolherbst: and add a TODO like "verify we actually implement all the CL C features first"
19:03pmoreau: Well
19:03karolherbst: so we can get the automatic stuff in, but don't have to worry about advertising too much
19:03airlied:has a big patch to opencl-c.h to add all the optional features and cl3.0
19:03airlied: it's stuck in review for ever
19:03karolherbst: airlied: ahh, which MR?
19:03pmoreau: CL_DEVICE_OPENCL_C_VERSION is about the highest version supported by the compiler, not what the driver might support AFAIU.
19:04karolherbst: pmoreau: well...
19:04karolherbst: it should be?
19:04airlied: https://reviews.llvm.org/D92004
19:04karolherbst: there is a platform thing most likley as well
19:04karolherbst: airlied: ahh.. llvm
19:05karolherbst: Anastasia answered recently, so I guess if her steps are followed things will progress :p
19:05pmoreau: Well, you have the OpenCL version supported by the platform, the OpenCL version supported by the device, and the OpenCL C version supported by the compiler for a device.
19:06karolherbst: pmoreau: well sure, but if the compiler can't produce a compatible binary for that device?
19:06pmoreau: And there is a macro you can use in your kernel to get the OpenCL version supported by the device.
19:06karolherbst: ohh sure
19:06karolherbst: but that's like checking for compiler features still
19:06karolherbst: or what the compiler enabled
19:06karolherbst: you can pass in a CL version into the compilation pipeline
19:07karolherbst: so you can e.g. force CL 1.0 even though your device supports 2.0
19:07pmoreau: Right, so the compiler might support an OpenCL C version that might not be compatible with the device but that doesn’t mean you have to use it, and you could decide to use an older version to match what the device supports.
19:08karolherbst: you can't use it
19:09karolherbst: even if the compiler supports 2.0 for a device not supporting it, it won't enable it
19:09karolherbst: and fail compilation
19:10pmoreau: One use case I could think of is, let’s imagine OpenCL C 1.1 added some cool new stuff that we support even on Tesla, but we can’t enable OpenCL 1.1 because of the increased requirement on local memory, even if we do support everything else.
19:10karolherbst: not sure we have to fail compilation if a higher version is selected though, but if there are no fallbacks for features the compilation has to fail
19:10karolherbst: yeah...
19:10karolherbst: we could allow the user to compile it, but with a chance of failing
19:10karolherbst: that's probably good enough
19:10karolherbst: so we advertise 1.0, but if the user just tries anyway, it might succeed
19:11karolherbst: the question is just if our compiler can deal with actual constraints
19:11karolherbst: _but_
19:11karolherbst: local size isn't really so much of a compiler problem
19:11karolherbst: because those buffers sizes might not be known at compile time
19:12pmoreau: No but that’s the reason we can’t expose OpenCL 1.1, not OpenCL C 1.1.
19:12karolherbst: why does CL C 1.1 has this req?
19:12pmoreau: It doesn’t!
19:12karolherbst: so we can expose it?
19:12pmoreau: I’m talking about OpenCL 1.1
19:12karolherbst: yeah, and?
19:12pmoreau: We can’t expose OpenCL 1.1, but we can expose OpenCL C 1.1
19:12karolherbst: so we advertise CL 1.0 and CL C 1.1
19:12pmoreau: Right
19:13karolherbst: and with CL 3.0 we can even default to it
19:13karolherbst: :D
19:13karolherbst: in the end clover will advertise CL 3.0 anyway
19:13karolherbst: just devices will be stuck with 1.1
19:14pmoreau: Yes
19:15pmoreau: I’ll just go ahead and drop that patch from the series.
19:15pmoreau: What about the vec3 one, do you want to validate it on Fermi+?
19:16karolherbst: not really. You can verify with llvmpipe if you want to check
19:18pmoreau: llvmpipe works with clover?
19:19karolherbst: yeah
19:19karolherbst: LP_CL=1
19:19karolherbst: I think we even CI on it already :D
19:19karolherbst: airlied was adding some tests with piglit
19:24pmoreau: I might need to build it first, I might have disabled it.
19:28karolherbst: could be
19:55pmoreau: karolherbst: Do I still have your Rb on “clover: Do not advertise OpenCL x.y when unsupported” after the changes I made to match Francisco’s requests?
19:55karolherbst: yeah
19:56pmoreau: Thanks!