[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

> Modulo optimizer bugs, __attribute__((used)) hasn't changed semantics.
> If your downstream project does not handle llvm.compiler.used, maybe handle it now :)

There seems to be some confusion here. The 'downstream project' is openmp, which has worked around this regression in D117211 <https://reviews.llvm.org/D117211> and D117231 <https://reviews.llvm.org/D117231>.

Before this patch, __attribute__((used)) mapped onto llvm.used, and the variables so annotated made it all the way to the compiled artefact. After this patch, it is mapped onto llvm.compiler.used, gets hit by an internalisation pass and ends up in the compiled output but missing from the symbol table. That is itself presumably a bug, as the linker should have completely discarded it, with much the same effect.

With this patch applied, what's the remaining use case for __attribute__((used))? It can no longer be used to keep something in the final executable, so it seems s/sed used/retain/g is the recommendation for programs that previously used that attribute.

It's possible that the check in internalize that skips over llvm.used and not llvm.compiler.used is itself a bug, and the intent is for llvm.used to be identical to llvm.compiler.used, but if that's the case we should delete the llvm.compiler.used array.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97446