Make libdevice work with -ffast-math

[hdelan]

See discussion here #9768

Libdevice doesn't do anything for -ffast-math. We should enable branching so that __spirv_ocl_cos or __spirv_ocl_native_cos are chosen based on the value set for -ffast-math. This may require a compiler pass since we link libdevice as bc bundles.

SPIR-V is currently getting -ffast-math behaviour for cosf since it relies on use of compiler intrinsics, but these don't have full coverage so some tests fail. When this is implemented this test should be re-enabled for SPIR-V https://github.com/intel/llvm/pull/9768/files#diff-bcdb4487c2827e573175267bdceb404b166460192e119ee45199d32d5fc13288R5

Tag @steffenlarsen

[bader]

This may require a compiler pass since we link libdevice as bc bundles.

You can also:

1. Have two bc bundles and choose one based on the -ffast-math.
2. Choose the right SPIR-V built in DPC++ header with __FAST_MATH__ macro. This seems to be simplest version to implement and maintain.

[hdelan]

1. This sounds like it would be in line with the current libdevice design.
2. This would eliminate the need for libdevice, is this preferable?

[bader]

2. his would eliminate the need for libdevice, is this preferable?

I see a lot of benefits from this approach vs using LLVM bitcode, but I don't think we can eliminate the need for libdevice completely. See https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/supported/C-CXX-StandardLibrary.rst#fallback-implementation.

[hdelan]

This would at very least make libdevice unnecessary for AOT targets like NVPTX and AMDGPU. I think the header approach is also preferable and it would allow us to simplify the driver action for these targets

[hdelan]

Hi @bader I have thought about this a bit and looked at the current behaviour of the CUDA toolchain. This is what the CUDA TC does:

Include a __clang_cuda_runtime_wrapper.h This needs to be the first thing included in compilation since it needs to predeclare some cstdlib funcs as static, and with other attributes that cannot be added after a first declaration (if first declaration is in a stdlib header that clang does not own). This header will then include things like cmath, cstdlib etc stubs which all seem to #include_next the stdlib header, as well as defining some other stuff. Also note that without the static keyword the compilation will compile every function in the headers, whereas a static func will only be compiled if it is used. So it is critical to make the first declaration of such stdlib funcs.

In order to get this to work in SYCL we would at minimum need to:

Make our own versions of cstdlib headers which must get included before any cstdlib headers. We would need to make sure that our wrapper headers were tightly integrated with the stdlib headers, which may tricky since there will be some variation in these headers that we cannot control, based on the different macros that are set for different systems, OSs etc.

The CUDA version seems to work pretty well, and I think that this can be achieved but it will take a lot of testing. I think adding a include to the beginning of all SYCL compilation is a lot cleaner than the current libdevice bundle-unbundle design. Although it is worth noting that the current libdevice design works OK for most cases. Let me know your thoughts

[hdelan]

Another caveat - I am not certain if the header only approach as CUDA uses is compatible with a single pass compilation. Maybe you have a better idea than I do about this

[bader]

I think adding a include to the beginning of all SYCL compilation is a lot cleaner than the current libdevice bundle-unbundle design. Although it is worth noting that the current libdevice design works OK for most cases. Let me know your thoughts

+1.

Today, SYCL version of libdevice is linked late in the compilation pipeline after first round of optimizations, which potentially impacts the performance of the application and compilation speed. When we optimize a single translation unit, the compiler sees an external function call to libdevice functions. If we link the libdevice implementation before the optimizations the compiler will see the whole body of libdevice functions can be inlined and optimized early. Most libdevice functions are small wrappers around compiler intrinsics or consists of a small # of instructions. Inlining them can unblock more optimizations, which can't handle external function calls.

We can achieve that with compiler header solution CUDA programming model uses or linking libdevice to each translation unit using -mlink-bitcode-file compiler frontend flag. Linking bitcode file has following pros:

• No need to bundle/unbundle code (vs what we use today)
• Provides more optimizations opportunities (vs what we use today)
• Skip parsing of cmath wrappers for each translation unit (vs approach with includes)

Unfortunately, we can't use __FFAST_MATH__ macro to switch between fast and precise libdevice versions using libdevice bitcode. The simple solution is to have two versions of libdevice and pass corresponding version based on -ffast-math option value.

Another caveat - I am not certain if the header only approach as CUDA uses is compatible with a single pass compilation. Maybe you have a better idea than I do about this

Using compiler headers is not different from using any DPC++ or user headers. The only difference is that they are implicitly added by the compiler ahead of other headers. The solutions for DPC++ headers should work for these wrapper headers too.

[hdelan]

OK so I think a straightforward solution for AOT targets would be to compile libdevice into single bc libraries per triple which is then linked with -mlink-builtin-bitcode at cc1 phase at the same time as say libclc. This would simplify the driver action considerably. We can modify libdevice so that fast-llvm-libdevice-nvptx64-nvidia-cuda.bc exists as well as llvm-libdevice-nvptx64-nvidia-cuda.bc and the correct one is chosen per TU based on whether -ffast-math is chosen at compile time for that TU.

I think this approach gives us the best of both worlds: we are able to leverage the existing infrastructure while simplifying driver action.

[bader]

Yes, I think it's worth trying this way.

Here is some more food for thought.

I should mention that although this approach gives us "the best of both worlds", we need to duplicate the whole libdevice to be able to have fast implementations for only a part the library.
To mitigate this concern, we could combine it with your original proposal from the description:

This may require a compiler pass since we link libdevice as bc bundles.

I think CUDA's libdevice solves this issue with NNVMReflect pass to handle multi-versioning (https://www.llvm.org/docs/NVPTXUsage.html#tutorial-linking-with-libdevice). We can consider something similar (e.g. we have passes handling specialization constants).
At the same time, I've seen some messages in LLVM community that approach is NNVMReflect is wrong and they wish to drop it. We can probably start with the solution described in this message and address code duplication concerns separately.

[bader]

FYI: #10250 is related to things we discuss here.

[hdelan]

We need to duplicate the whole libdevice to be able to have fast implementations for only a part the library.

I think we could just have a separate fast lib which would provide the definitions only for symbols that have a native version cos -> __spirv_ocl_native_cos. We could link both at cc1 time with -mlink-builtin-bitcode. We would need to make sure that the fast symbols get chosen over the normal symbols so the order would have to be -mlink-builtin-bitcode reduced-fast-libdevice.bc -mlink-builtin-bitcode normal-libdevice.bc. It should not matter that there are symbols found in both bc libs, llvm-link will just take the first version it finds.