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

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added a comment.

In D125698#3516262 <https://reviews.llvm.org/D125698#3516262>, @ye-luo wrote:

> 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?
>
> The old method actually work best in the current state. You thought the runtime route is better, more robust and correct. That maybe true for release but not main. I encountered clang from main miscompiling libomptarget host code and resulted in de-referencing nullptr. So right now LLVM_ENABLE_PROJECTS works best because only the DeviceLib is built by just-built Clang. This routine also doesn't require building Clang as a separate step.

I didn't say runtime routine is like that. What I were saying is, using CMake's build command to build a target is better. `LLVM_ENABLE_PROJECTS` in fact has nothing to do with this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125698