[PATCH] D47652: [LowerTypeTests] Limit when icall jumptable entries are emitted

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.
Herald added a subscriber: dexonsmith.


================
Comment at: lib/Transforms/IPO/LowerTypeTests.cpp:1715
+              if (GVS->isLive())
+                External |= GVS->linkage() == GlobalValue::ExternalLinkage;
+
----------------
Wouldn't this apply not just to ExternalLinkage but to other non-local linkages?


================
Comment at: lib/Transforms/IPO/LowerTypeTests.cpp:1779
     bool IsExported = false;
-    if (isa<Function>(GO) && ExportedFunctions.count(GO.getName())) {
-      IsDefinition |= ExportedFunctions[GO.getName()].Linkage == CFL_Definition;
-      IsExported = true;
+    if (isa<Function>(GO)) {
+      if (ExportedFunctions.count(GO.getName())) {
----------------
Use dyn_cast instead of isa to avoid needing the cast below.


================
Comment at: lib/Transforms/IPO/LowerTypeTests.cpp:1788
+      } else if (!cast<Function>(GO).hasAddressTaken()) {
+        if (!CrossDsoCfi || !IsDefinition || !GO.hasExternalLinkage())
+          continue;
----------------
Same comment about ExternalLinkage.


================
Comment at: test/Transforms/LowerTypeTests/function-ext.ll:10
 ; WASM32: declare !type !{{[0-9]+}} void @foo()
 declare !type !0 void @foo()
 
----------------
With this change I don't think this test is testing what it is intended to test any more. Can you insert an artificial reference to foo?


Repository:
  rL LLVM

https://reviews.llvm.org/D47652