[ESIMD] Don't add inline hints when compiling for -O0 #3422
Conversation
| // functions to be inlined into the kernel itself. To overcome this | ||
| // limitation, mark every function called from ESIMD kernel with | ||
| // 'alwaysinline' attribute. | ||
| if ((F.getCallingConv() != CallingConv::SPIR_KERNEL) && | ||
| if (!OptLevelO0 && (F.getCallingConv() != CallingConv::SPIR_KERNEL) && |
There was a problem hiding this comment.
I guess this is not the first time I questioning overall design here: why can't we do the same transformation + launch alwaysinline pass within GPU vector backend?
It seems a bit strange to me that in target-independent tool like sycl-post-link we have quite a lot of workarounds specifically for GPU vector compute backend.
There was a problem hiding this comment.
I believe that is the limitation of the GPU backend. @kbobrovs , can you please comment?
There was a problem hiding this comment.
I understand that this is a limitation, I just don't understand why device-specific backend can't apply device-specific lowering to its input so it is canonicalized before performing actual work on it?
Again, I understand that technically our interface is just a SPIR-V and we haven't documented any particular optimization level which is expected by the backend, but I would expect that such things as inlining (for example) should be simple enough to be implemented in device-specific backend if it is strictly needed in there. I would be complaining less if this were a requirement for more than one backend, i.e. it would be more generic.
Clarification: I'm not trying to block this particular PR, I'm just not so happy with the approach of putting things which are unique to a particular backend into this more or less generic tool. Just trying to see if there is something we can do about that
There was a problem hiding this comment.
@AlexeySachkov, yes you are right. The ideal is to have ESIMD BE do all necessary preparation, including recognition of some/all of the ESIMD/SPIRV intrinsics. This has been brought up few times before. But look at this from a different perspective: we have an external BE, which is not part of the LLVM Code Gen infrastructure, and we still want to use it given the interface it provides. Luckily, the BE is not quite external, so the plan is to gradually move ESIMD-dependent parts into the BE mid/long-term, and the extra LLVM IR passes should be the first candidate.
tagging @kvladimi
Strictly speaking, these particular changes are not sycl-post-link tools changes (which serves just as an LTO driver here) and are isolated in the LowerESIMD pass. But the point you raised is valid anyway. BTW, we plan to move LowerESIMD back to BackendUtils.cpp, as having it in LTO broke few things.
|
Per discussion with @DenisBakhvalov, this patch should be reworked a bit.
|
|
Closed in favor of #3439 |
No description provided.