[PATCH] D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries

[Peter Collingbourne via Phabricator via llvm-commits]

pcc accepted this revision.
pcc added a comment.

LGTM



================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:1009
     FDecl->setVisibility(Visibility);
+    Visibility = GlobalValue::HiddenVisibility;
 
----------------
dmikulin wrote:
> pcc wrote:
> > Is there a reason to set the visibility indirectly via this variable here?
> I need to set visibility late, after replaceCfiUses(). If I do it sooner, the symbol becomes dso_local, and it breaks the logic in replaceCfiUses().
Okay, please mention that in a comment.


================
Comment at: llvm/test/Transforms/LowerTypeTests/cfi-direct-call.ll:15
+  call void @internal_default_def()
+  call void @internal_hidden_def()
+  call void @local_b()
----------------
You might also want to test the case where a default visibility function is `dso_local`.


https://reviews.llvm.org/D46326