[Openmp-commits] [PATCH] D125698: [Libomptarget] Don't build the device runtime without a new Clang

Mon May 16 09:32:34 PDT 2022

jhuber6 added a comment.

In D125698#3516252 <https://reviews.llvm.org/D125698#3516252>, @tianshilei1992 wrote:

> In D125698#3516250 <https://reviews.llvm.org/D125698#3516250>, @jhuber6 wrote:
>
>> In D125698#3516229 <https://reviews.llvm.org/D125698#3516229>, @ye-luo wrote:
>>
>>> In D125698#3516202 <https://reviews.llvm.org/D125698#3516202>, @tianshilei1992 wrote:
>>>
>>>> LGTM.
>>>>
>>>> We are using CMake's command to build the device runtime now. There is no way to tell CMake to use another compiler rather than CMake's compilers to build a specific target. As a result, we either disable it, or we are doomed.
>>>> Well, accurately speaking, there seems to be a way to do that, see https://stackoverflow.com/questions/27168094/cmake-how-to-change-compiler-for-individual-target, but I don't think manipulating CMake's internal variables is a good practice.
>>>
>>> I didn't say to workaround CMake. I was saying it was working that clang builds DeviceRTL and gcc building all the rest and now it is broken. You can try it out yourself with the commit before D125315 <https://reviews.llvm.org/D125315>
>>
>> I'll just guard the new code then. Even though it shouldn't break if you're using the standard builds it will probably make it less noisy for people currently using the other methods.
>
> Eventually we will abandon the old method, no?

I'm not sure how long it will need to stick around, you get pretty poor performance linking in the library late without LTO on NVPTX since we can't do any inlining or constant propagation through the device runtime library. I'd like to get rid of it, but it'd definitely hit performance unless we just turned on LTO by default. I think I'll just not build the static library (Means you can't use LTO) right now if we don't have CMake configured properly, later we can figure out how to move and / or deprecate this stuff.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125698/new/

https://reviews.llvm.org/D125698