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

[Vlad Tsyrklevich via Phabricator via llvm-commits]

vlad.tsyrklevich reopened this revision.
vlad.tsyrklevich added a comment.
This revision is now accepted and ready to land.

Dmitry, I can run the tests for you locally; however, it's probably worth having @pcc take a look first.



================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:425
 
-  void replaceWeakDeclarationWithJumpTablePtr(Function *F, Constant *JT);
+  void replaceWeakDeclarationWithJumpTablePtr(Function *F, Constant *JT, bool IsDefinition);
   void moveInitializerToModuleConstructor(GlobalVariable *GV);
----------------
clang-format


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:436
+  /// Unlike replaceAllUsesWith this function skips blockaddr and direct call
+  /// uses.
+  void replaceCfiUses(Function *Old, Value *New, bool IsDefinition);
----------------
Update


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:1469
+      replaceCfiUses(F, FAlias, IsDefinition);
+      if (!F->hasLocalLinkage())
+        F->setVisibility(GlobalVariable::HiddenVisibility);
----------------
Why change the visibility to hidden only for !hasLocalLinkage ?


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:1655
+
+    // Must handle Constants specially, we cannot call replaceUsesOfWith on a
+    // constant because they are uniqued.
----------------
Did new changes cause this to be required?


================
Comment at: llvm/test/Transforms/LowerTypeTests/cfi-direct-call.ll:8
+declare void @external_nodecl()
+;declare void @internal_default_def()
+declare hidden void @internal_hidden_def()
----------------
remove?


https://reviews.llvm.org/D46326