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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:985
+  if (F->isDeclarationForLinker() && isDefinition) {
+    // Functions with default visibility may be overriden at run time,
+    // don't short curcuit them
----------------
Can you check for `dso_local` instead of default visibility here? I think that should be exactly the property that you want to check for.


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:1009
     FDecl->setVisibility(Visibility);
+    Visibility = GlobalValue::HiddenVisibility;
 
----------------
Is there a reason to set the visibility indirectly via this variable here?


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:1651
+    if (isDirectCall(U) &&
+        (Old->getVisibility() != GlobalVariable::DefaultVisibility ||
+         !IsDefinition))
----------------
Similarly I think you can check for `dso_local` here.


https://reviews.llvm.org/D46326