[PATCH] D135427: [LTO] Make local linkage GlobalValue in non-prevailing COMDAT available_externally

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In D135427#3869003 <https://reviews.llvm.org/D135427#3869003>, @MaskRay wrote:

> I put this in the description
>
>> To fix this, update the regular LTO change D34803 <https://reviews.llvm.org/D34803> to track local linkage GlobalValues, and port the code to ThinLTO (GlobalAliases are not handled.)

Is the part about GlobalAliases not handled stale now with the most recent update?



================
Comment at: llvm/lib/IR/Verifier.cpp:849
+            GA.hasAvailableExternallyLinkage(),
+        "Alias should have available_externally, private, internal, linkonce, "
+        "weak, linkonce_odr, "
----------------
Would probably need to update https://llvm.org/docs/LangRef.html#aliases too.
Should we be checking that if alias is available_externally, then its aliasee is as well?


================
Comment at: llvm/lib/Transforms/IPO/FunctionImport.cpp:1160
+    Changed = false;
+    // If an alias references a GlobalValue in a non-prevailing comdat, change
+    // it to available_externally. For simplicity we don't handle ConstantExpr
----------------
Would an alternative be converting the alias to a declaration? Slight pessimization in optimization ability but the verifier and other documentation doesn't need to be change.

But what if this was the prevailing copy of the alias? Could that happen?


================
Comment at: llvm/lib/Transforms/IPO/FunctionImport.cpp:1162
+    // it to available_externally. For simplicity we don't handle ConstantExpr
+    // aliasee, which is unlikely used in a COMDAT.
+    for (auto &GA : TheModule.aliases()) {
----------------
Should that be asserted?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135427/new/

https://reviews.llvm.org/D135427