llvm icon indicating copy to clipboard operation
llvm copied to clipboard

[SYCL][ESIMD] Fix a compilation of mix of unnamed ESIMD and non ESIMD kernels

Open fineg74 opened this issue 2 years ago • 1 comments

The purpose of this PR is to fix an issue that leads to runtime exception when unnamed ESIMD and non ESIMD kernels are present.

fineg74 avatar Aug 10 '22 18:08 fineg74

Complementary test PR: https://github.com/intel/llvm-test-suite/pull/1143

fineg74 avatar Aug 10 '22 18:08 fineg74

@fineg74, could you please describe what the actual problem is - why unnamed ESIMD kernels lead to this problem?

kbobrovs avatar Aug 15 '22 17:08 kbobrovs

@fineg74, could you please describe what the actual problem is - why unnamed ESIMD kernels lead to this problem?

The immediate cause of the issue is that ESIMD kernel is wrapped with RoundedRangeKernel and sycl_explicit_esimd attribute is not propagated to the wrapper class. As a result ModuleSplitter in sycl-post-link creates a spurious split for the wrapper and later creates a spurious non-esimd module and pulls called functions into it. As a result non-esimd module would contain esimd functions and later fails to link. It looks like that for named ESIMD kernels the attributes are properly propagated and therefore the issue doesn't occur. The fix proposed here fixes it by using a fact that ESIMD functions can only be called from ESIMD kernels so spurious split is eliminated and modules are properly created and linked. It is possible to implement it in a more formalized way by creating a pass that would propagate the attributes before performing module split but it would require significant code duplication between ModuleSplitter and the new pass.

fineg74 avatar Aug 15 '22 20:08 fineg74

We should try to limit ESIMD specifics penetration into general-purpose tools like post-link. My suggestion would be to find the wrapper function and add 'sycl_explicit_simd' metadata to it before doing any splits in the post-link. I believe the wrapper should have spir_kernel calling convention, which automatically makes it an entry point. The task of finding the wrapper(s) is then searching for all spir_kernels up the call chain. ESIMD splitting and lowering infra will then correctly interpret the 'sycl_explicit_simd' metadata and do what's needed.

If the wrapper does not have spir_kernel (and is a spir_function), then the user ESIMD kernel must be annotated with spir_kernel. In which case the search for the wrapper would be finding terminal function in the callgraph starting from the user kernel up the call chain. Callgraph traversal up the call chain is implemented in ESIMDUtils.cpp (traverseCallgraphUp). In this case (no spir_kernel in the wrapper), we should also force the wrapper to be an entry point by copying the "sycl_module_id" attribute from the user kernel or creating a dummy one.

kbobrovs avatar Aug 16 '22 21:08 kbobrovs

... I would suggest to implement finding the wrapper and adding the metadata as a pass defined within the llvm/lib/SYCLLowerIR directory and invoked in sycl-post-link.cpp somewhere around (and similarly to) the const bool InvokeSimdMet = runModulePass<SYCLLowerInvokeSimdPass>(*M);

kbobrovs avatar Aug 16 '22 21:08 kbobrovs