[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added a comment.

I'm not sure we have a consensus on api stability. Usually llvm allows mixing libraries and compilers from different sources, e.g. the various libunwind or compiler-rt vs libgcc.  Libomptarget in general appears to be considered fixed and has external users (intel, maybe gcc).

The device runtime would be ill served by this default. This is the only openmp device runtime library which works with llvm. It's statically linked, usually as bitcode when performance is important. The code used to handle target offloading for nvptx is spread across the compiler and the runtime, probably not optimally.

I'm not familiar with the gcc-nvptx-openmp implementation. Reading through https://gcc.gnu.org/wiki/Offloading strongly suggests a totally independent implementation to this one. I don't think gcc can be using this runtime library for nvptx. It definitely doesn't for amdgcn. Proprietary compilers might be using this code, but we can have no duty of care to toolchains that use this code without telling us they're doing so.

Therefore the only backwards/forwards compatibility we can strive for is between different versions of clang and the device runtime. That seems potentially useful - one could use a release clang binary while working on the deviceRTL or vice versa. It's an expensive developer convenience though.

We would pay with things like the API rot fixed above. Introducing a faster lowering for an openmp construct would mean a redundant path through clang and some version checking to guess which runtime library we're targeting, which is not presently versioned. Likewise moving code between compiler and runtime becomes much more expensive to implement. Getting it wrong is inevitable given our test coverage.

I think the project is much better served by assuming that the runtime library used by clang is the one from the same hash in the monorepo. That leaves us free to fix debt and improve performance, at the price of needing to build clang from (near to) trunk while developing the rtl.

Perhaps we can embrace API stability later on, once we have things like versioning and a solid optimisation pipeline in place, especially if gcc wants to use the deviceRTL for nvptx. Now is too early.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83268