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

Fri May 18 01:25:37 PDT 2018

guansong added a comment.

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

> I am not at all convinced. I think there are absolutely no good arguments against allowing the just built compiler to build the BCLIB (as a default, this is simply a default which can be overwritten at any time by the user). The drawbacks you mention have no detrimental impact on the quality of life of the compiler developer. Developers improving other parts of the compiler who do not care about OpenMP and device offloading can just disable the building of the BCLIB because it is optional after all. The BCLIB is something that is required for performance not correctness so it can be disabled at any time. Even when BCLIB building is turned on there is no perceptible overhead to it, compared to the time it takes to compile a cpp file somewhere in the source code, the BCLIB builds instantly.
>
> 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.
>
> I do not understand why you are opposing this, so far I haven't heard a single strong argument against this.

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.

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

Repository:
  rOMP OpenMP

https://reviews.llvm.org/D46842