sycl icon indicating copy to clipboard operation
sycl copied to clipboard

[SYCL] Refactor SPIR implementation

Open agozillon opened this issue 5 years ago • 0 comments

This is related to: https://github.com/intel/llvm/issues/44

The current way we support SPIR builtins is as follows:

We attempt to translate any function in the cl::_spirv (after the Reflower pass this namespace looks like: spirv_ocl) namespace to its SPIR builtin mangling. This occurs in our LLVM InSPIRation pass and works by removing the namespace mangling from the builtin and altering its Z value (Z representing the number of characters in a functions mangled name excluding arguments). There is currently no check to make sure they exist or do not exist as SPIR functions at compile time, if it doesn't exist you'll get a runtime ABI exception (can't find X function) as it won't have linked properly with xocc's SPIR builtin library.

Currently id builtins (get_global_id) are defined in spirv_vars.hpp alongside the SPIRV equivalents and based on a define the compiler creates based on our -fsycl-xocc-device the SPIR builtins are swapped with their SPIRV equivalents. For the math functions we hook into the exact same math builtins as SPIRV inside builtins.hpp without any requirement to optionally swap builtins out.

Short Term Goal:

Move SPIR specific builtins from a cl::__spirv namespace to an equivalent cl::__spir namespace and separate files/folders where necessary. This will lead to something a little cleaner and easier to understand and also divorce the SPIR builtins from transformation by the Reflower making it perhaps a little more stable. As currently any changes to the Reflower's translation of the cl::__spirv namespace is felt by the InSPIRation pass.

It may be more worthwhile to ignore this short term goal and wait for the long term goal based on our priorities.

Long Term Goal:

The idea would be to transform what we currently do, to the newer builtins offloading method where the ToolChain dictates what library functions are used in place of certain placeholder builtins (as explained by Alexey briefly in the linked issue). I believe Alexey is currently looking into this, but once this has been merged into the main implementation we should consider refactoring our implementation to align with this. This may take a reasonable amount of reworking.

agozillon avatar Apr 15 '19 22:04 agozillon