[PATCH] D97755: [IRSymTab] Set FB_used on llvm.compiler.used symbols

Wed Mar 3 13:08:24 PST 2021

tejohnson added a comment.

In D97755#2601054 <https://reviews.llvm.org/D97755#2601054>, @MaskRay wrote:

> This issue is like a pretty corner case thing. We do not parse inline asm instructions, so we don't know its IR symbol table.
> This combined with the fact that (we previously used `llvm.used`) made it not a problem.
>
> In D97755#2600903 <https://reviews.llvm.org/D97755#2600903>, @tejohnson wrote:
>
>> Are we missing a test that goes all the way from __attribute__((used)) -> IR -> LTO? It seems such a test would have helped catch the issue caused by the change of that attribute.
>
> To me this proposal feels like a bit closer to cross-project testing: https://lists.llvm.org/pipermail/llvm-dev/2021-January/148121.html
> Such tests are useful but I haven't figured out how to do it for LTO.
> (I can see some `clang/test/CodeGen/thinlto*` tests but I am unsure whether a `attribute((used))`->llvm-lto test should be placed there.)

There's already a bunch of tests in that directory that use llvm-lto or llvm-lto2 in some fashion. I think it would be good to add this test, since it's really the only way to ensure that the attribute is having the intended effect. It's often added specifically for the LTO case to avoid symbol deletion.

> The majority tests are layered, e.g. touching LLVM IR transformation should generally not affect clang tests; touching LLVM backend should not affect LLVM IR transformation tests.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97755