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

Tue May 15 07:05:53 PDT 2018

gtbercea added a comment.

> 1. The just-built compiler doesn't exist by definition when CMake is invoked. This will prevent all kind of dynamic flag checking (Does `${LIBOMPTARGET_NVPTX_SELECTED_CUDA_COMPILER}` support `-fflag`?). This is how I propose to reliably implement a solution for https://reviews.llvm.org/D44992.

The compiler not existing can also happen if the user provides a path to a non-existing compiler. Remember, this is a default value which can always be overwritten by the user.
Flag checking will work in the same way as if the user had provided the path to a different compiler.

> 2. See the quote in my previous comment: This patch will result in rebuilding the entire library whenever there is a change in LLVM or Clang. While this doesn't seem to problematic at first, `clang` is usually the binary that finishes linking last because it's rather large. So the compilation of `libomptarget-nvptx-bc` can only start after almost all other steps of the build are done. I've found this really annoying in the past.

That's right, when Clang is available that's when the bclib is built. There is no problem if the bclib is built at the end of the build process actually. The BCLIB builds instantly so there's no perceptible overhead. Actually it's what the developer wants, if they make changes that affect the CUDA compilation then they want to see them reflected in the BCLIB.

> 3. There is a somewhat weaker problem when looking in the other direction: Let's say you have a fully setup LLVM build, but only want to build `libomptarget-nvptx-bc` (for whatever reason; let's just assume you are doing some changes in there that you want to test). This will result in the build system compiling the entire compiler before actually starting to build the library. I acknowledge that this will only happen after changes to files in LLVM or Clang, but if you synchronize to the latest changes once a day you can basically sit there and wait for your build to complete at the start of your working day.

If you only do changes to the files that are part of the bclib then the compiler will not be rebuilt. If the user updates the current compiler then he/she will have to wait anyway even if the bclib compiler path points to a different clang compiler.

An overall comment which I think applies to all your argument above: the choice of using the just-built compiler is a reasonable default which in an overwhelming number of cases is "what the user wants". This choice can always be overwritten using cmake flags. Those cmake flags are checked first and if a valid clang compiler is found then the building of the bclib will proceed there and then without depending on the just-built compiler. So for all your arguments above,  user can always just provide a different clang and it will work as you want.

> From what I read in the documentation the LLVM bitcode should be compatible with all versions down to LLVM/Clang 3.0. Is there a particular problem that you have in mind?

I don't think that works, when trying to inline newer LLVM code into older LLVM code I get an error saying that a different LLVM version is used or something along those lines. Since seeing something like that a few months ago I have always used the just-built compiler to make sure. This is a different matter though.

Repository:
  rOMP OpenMP

https://reviews.llvm.org/D46842