[PATCH] D107934: [LowerTypeTests] Emit cfi_jt aliases regardless of function export

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 9 13:57:20 PDT 2021


pcc added inline comments.


================
Comment at: compiler-rt/test/cfi/cfi_jt_aliases.c:1
+// REQUIRES: lld-available, lto
+
----------------
This isn't an execution test so I don't think it should be together with the other execution tests. Also I think it will fail on non-ELF platforms because you're using `llvm-readelf` etc. It seems better to make this a test of the LTO pipeline (see e.g. llvm/test/ThinLTO/X86/cache-icall.ll).


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:1555
+      if (!IsJumpTableCanonical) {
+        GlobalAlias *JtAlias = GlobalAlias::create(
+            F->getValueType(), 0, GlobalValue::ExternalLinkage,
----------------
tejohnson wrote:
> Where and how does the new alias symbol get used in the LTO case? I only see it used when importing in the backend in the ThinLTO case.
I think the idea is that these symbols only exist for debugging purposes.

I think this could be cleaned up a bit though to ensure that the symbols aren't being exported unnecessarily. Can we set the linkage to internal for non-exported symbols and add to llvm.used to ensure that the symbols aren't dropped?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107934



More information about the llvm-commits mailing list