[Openmp-commits] [PATCH] D71988: [OpenMP][WIP] Make the kmp_depend_info type fit in 128 bits.

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added a comment.

In D71988#1799064 <https://reviews.llvm.org/D71988#1799064>, @jdoerfert wrote:

> > I would expect that some versioning is needed..
>
> Versioning is fine with me. Versioning the runtime would be even preferable.


When would you expect bumps of the version? As an example `libc++` and `libstdc++` have versions, but they're not frequently bumped (once in a decade or less?) because rebuilding all your applications can be very much work. That is why I think stable interfaces for runtime libraries are very convenient whenever possible.

>> If this really brings a benefit, we need new entry functions and update the compiler(s).
> 
> We go to great lengths to align to cache lines in some structs, e.g., `kmp_depnode`, but we are literally wasting space and alignment for others. Even if we do not adopt this, we should start to adopt some best practices.

There's a difference though: `kmp_depend_info_t` is passed by the compiler and evaluated once by the runtime library. If I recall correctly, all information is stored in internal structures afterwards (the mentioned `kmp_depnode` being one of them). Thus I wonder if this change brings a measurable improvement when reading the data once is not on the critical path. Breaking the ABI on the benefit of saving a few bytes on the stack seems a bit overkill just for the sake of doing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71988