[Openmp-commits] [PATCH] D126701: [Libomptarget] Do not use retaining attributes for the static library

[Joseph Huber via Phabricator via Openmp-commits]

jhuber6 added a comment.

In D126701#3547566 <https://reviews.llvm.org/D126701#3547566>, @JonChesterfield wrote:

> Is the difference that we link the .bc vs the .a at different points in the pipeline, or the behaviour of the mlink-builtin-bitcode? Which does internalise and aome attribute propagation, maybe other things.

We link the `.bc` early and internalize eagerly, the `.a` we link at the very end and treat it just like any other library. We could get rid of the internalization for the bitcode linking in Clang, but then the performance would be awful.

> On the face of it we should do exactly the same thing with the devicertl whether it is wrapped in an archive or not, so same point in pipeline and splice in the contents using the same hacks we have in place without it. Otherwise we can expect a long tail of basically spurious behaviour changes based on whether the archive convenience feature is in use or not.

The existing code is just a hacky workaround that's not necessary when using the static library, it shouldn't change any behavior otherwise. Right now we could get rid of the bitcode library and just use this static library, but that would tank the performance of non-LTO builds on NVPTX. I don't think it's too much of a problem to just keep this workaround where it's needed, but remove it where it's not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126701