[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

Fri Jan 13 03:31:43 PST 2017

pekka.jaaskelainen accepted this revision.
pekka.jaaskelainen added inline comments.
This revision is now accepted and ready to land.

================
Comment at: docs/UsersManual.rst:2130
+
+- x86 is used by some implementations that are x86 compatible
+  (e.g. `POCL <http://portablecl.org/>`_) and currently remains for backwards
----------------
Anastasia wrote:
> pekka.jaaskelainen wrote:
> > Anastasia wrote:
> > > pekka.jaaskelainen wrote:
> > > > This is a bit confusing paragraph, probably due to my confusing explanations of the problems with pocl. Currently pocl tries not to use FASM for preserving logical AS IDs to LLVM IR due to the bunch of problems it constantly produces with seemingly little benefits for common CPUs. My patch related to this considered only the argument info switch. Now pocl only derives the logical iDS from kernel arguments (and all other IDs within the body of the IR function are lost for flat machines).  In my patch, the argument info's address space IDs were made constant and identical to SPIR's as previously they were the target's (which meant losing the AS IDs altogether for flat AS machines).
> > > > 
> > > > You seem to document the arg-info md switch later, so this paragraph might be removed or converted to my pocl blurb which mentions the need for further processing for CPUs.
> > > Yes, it's perhaps a bit confusing indeed. I find the usage of the x86 backend for OpenCL is generally a bit confusing. Also there are a lot of program paths even in Clang that don't play well for OpenCL purposes or are not handled properly and are therefore a source of bugs.
> > > 
> > > I was just wondering whether it would be possible to switch to using SPIR as a generic target at some point in the future and "deprecate" the x86 target for OpenCL . Do you think this might work for POCL? At the same time we have upcoming SPIR-V support as a new feature that might reshape things.
> > > 
> > > Regarding the address space I just know that it's common to use fake address space map with the x86 backend for some out of tree implementations to add missing memory segments to this target. That's why I added this text here.
> > > 
> > > 
> > We have considered using SPIR internally, but it is likely to bring the same problems as with the FASM as we need to convert to the target bitcode to ensure all target specific IR level optimizations (especially vectorization) is applied properly. Why do you think using the real target when generating from OpenCL C kernels is problematic? 
> Perhaps, it would be nice to understand better how you use x86 backend in your toolchain. What I have seen in some projects that some invalid IR was produced or Clang hit an asset in the place which has to be done differently in some CodeGen paths just for OpenCL. So the typical fix for this to specialize on the OpenCL is not that easy to get into upstream because the target hasn't been created for what it's being used for and people like to see more modular code in general anyways.
> 
> The target is ideally not just a processor architecture but a combination of hardware and runtime software layers (i.e. OpenCL runtime in our case). Btw what triple do you use exactly while compiling OpenCL for x86?
We use just the regular target triplet without any opencl specific parts.This is because of at least two reasons a) we link in builtins as target-specific bitcodes b) we want to ensure all ordinary LLVM IR optimizations (some of which are target specific) are applied to the fully linked kernel bitcode to get all the performance improvements. Apart from the address spaces, I'm not sure which OpenCL-specific code paths you might mean in the code gen. I'd turn it the other way: if there's no need such special casing, it's better to do like with other frontends and have the usual hardware target instead of SPIR (which then needs to be converted to the target IR just for optimizations and bitcode linking).

================
Comment at: docs/UsersManual.rst:2133
+
+- x86 is used by some implementations that are x86 compatible and currently
+  remains for backwards compatibility (with older implementations prior to
----------------
Anastasia wrote:
> pekka.jaaskelainen wrote:
> > ARM (and others) are treated similarly in pocl: OpenCL C is just yet another frontend language in this picture, like C/C++. I would not "deprecate" that path as of yet as forcing to another intermediate representation (which SPIR basically is, albeit derived from LLVM IR) always loses some information on the way.
> I have removed the deprecation bit from the documentation now. Do you want me to change anything else here?
> 
> I think the idea of SPIR target was not to force the developers to yet another IR but rather to create a common generic target that is specifically tailored to OpenCL needs (not just as a language but RT as well). Also the intention of SPIR was to add missing semantics of OpenCL to LLVM IR, so it's supposed not to lose the information ideally even though I am not completely sure the goal was achieved. But since SPIR-V is gaining more popularity I am not sure how much SPIR will continue to be relevant in the community.
> 
> One more problem with x86 target is that there are currently not many contributors in the community that are willing to support it for OpenCL. That's why I am a bit reluctant to put it in the document as fully supported target. Do you have any relevant fixes in your internal branches btw? 
No additional fixes, the upstream Clang/LLVM works just fine. The main source of grief was the fake address space map to transfer the logical address space data, which I decided to get off of for now. 

https://reviews.llvm.org/D28080