[Openmp-commits] [PATCH] D46842: [OpenMP][libomptarget] Make bitcode library building depend on clang and llvm-linker being available

Fri May 18 08:31:17 PDT 2018

Hahnfeld added a subscriber: hfinkel.
Hahnfeld added a comment.

In https://reviews.llvm.org/D46842#1103727, @gtbercea wrote:

> Your argument that an older version is better is actually wrong. Using an older version to build the BCLIB is always a BAD idea. The reason why you want to build the BCLIB is so that the runtime functions actually get inlined. If you use an older compiler the functions will not be inlined which defeats the purpose of having the BCLIB in the first place. I stumbled again upon this problem today after updating to the latest changes.

To be precise, I didn't say that "an older version is better". My statement was that it works and that older versions of the compiler generate bitcode files that are compatible with a larger number of (newer) compilers.

But you are right, inlining of old bitcode files broke last month with https://reviews.llvm.org/rC329829. Luckily, this can be fixed, see https://reviews.llvm.org/D47070.

> I do not understand why you are opposing this, so far I haven't heard a single strong argument against this.

"strong" is certainly subjective and may imply different things for you and me. If I wanted to put my point to the extreme, I'd say that there is none of your arguments left because I sat down and fixed all the problems you have been describing so far.

Coming back to my opinion: I don't like using the just-built compiler because that's not how CMake usually works when you build a project. The build system takes a single (version of a) compiler that can't change afterwards, determines its abilities through dynamic feature checking and compiles the source code. The last two step can be repeated as often as the project or its prerequisites change.

In https://reviews.llvm.org/D46842#1104142, @guansong wrote:

> I second this opinion. Besides, I think when the runtime is in-tree built, the compiler to compile the bc file HAS be the just-built clang compiler.

I don't see why that has to be the case. As I explained throughout the comments, you can build the bitcode library with an older compiler and still inline it with a newer version.

> After that said, I am not sure if we consider in-tree runtime built for the moment. It sounds we do not?

You can build the bclib in-tree, it just won't use the just-built compiler but `CMAKE_C_COMPILER` by default.

> I may missed some earlier discussions. To me, I am not sure how to exactly set the dependence to make that happen. Our old code seems did that and then get changed?

I'll happily link to the previous discussion once more: The "support" for using the just-built compiler was removed during review of https://reviews.llvm.org/D14254. CC'ing Hal who chimed in last time, maybe he can add his opinion and convince one of the sides (I'm also fine with being convinced myself if I do still miss an important point!)

Repository:
  rOMP OpenMP

https://reviews.llvm.org/D46842