[PATCH] D41741: LowerTypeTests: Add limited support for aliases

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

Please add some negative tests, e.g. alias of function without jump table entry,



================
Comment at: lib/Transforms/IPO/LowerTypeTests.cpp:963
+    for (auto &U : F->uses()) {
+      if (isa<GlobalAlias>(U.getUser())) {
+        GlobalAlias *A = cast<GlobalAlias>(U.getUser());
----------------
`if (auto *A = dyn_cast<GlobalAlias>(U.getUser())) {`


================
Comment at: lib/Transforms/IPO/LowerTypeTests.cpp:966
+        StringRef AliasName = A->getName();
+        A->eraseFromParent();
+        Function::Create(F->getFunctionType(), GlobalValue::ExternalLinkage,
----------------
I think you want to RAUW the alias with the newly created function declaration, otherwise this will fail if the alias has any uses. See line 775 in this file for an example.


================
Comment at: lib/Transforms/IPO/LowerTypeTests.cpp:1705
+        StringRef Aliasee = cast<MDString>(AliasMD->getOperand(1))->getString();
+        if (!ExportedFunctions.count(AliasName) ||
+            !ExportedFunctions.count(Aliasee) ||
----------------
Is it possible for `ExportedFunctions.count(AliasName)` to be false in a case where we need to create an alias in the merged module? It seems possible, e.g. if the alias is referenced from a native object file or  a translation unit compiled without cfi-icall.


================
Comment at: lib/Transforms/IPO/LowerTypeTests.cpp:1711
+        ExportedFunctions.erase(AliasName);
+        M.getFunction(AliasName)->removeFromParent();
+
----------------
Similarly to my comment above, I think this will need to be a RAUW at the point where the alias is created in case the function declaration has any uses in the merged module.


Repository:
  rL LLVM

https://reviews.llvm.org/D41741