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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

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.)

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.



================
Comment at: llvm/test/ThinLTO/X86/asm.ll:6
+
+; RUN: llvm-lto2 run %t/a.bc %t/b.bc -o %t/out -save-temps -r=%t/a.bc,ref,plx -r=%t/b.bc,ff_h264_cabac_tables,pl
+
----------------
tejohnson wrote:
> Should we be checking the output somewhere? I'm assuming there is a missing FileCheck that corresponds to the CHECK further below.
Forgot it. Sorry..


================
Comment at: llvm/test/ThinLTO/X86/asm.ll:11
+; NM-NOT: {{.}}
+; NM:     ---------------- T ref
+; NM-NOT: {{.}}
----------------
tejohnson wrote:
> What is the behavior here without this patch?
This is not changed. `ff_h264_cabac_tables` is not in the IR symbol table.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97755