[PATCH] D19308: ThinLTO: Resolve linkonce_odr aliases just like functions

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 20 14:27:00 PDT 2016


tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

Just a few suggestions on the test case comments.


================
Comment at: test/ThinLTO/X86/alias_resolution.ll:8
@@ +7,3 @@
+
+; Do not resolve all these aliases
+; PROMOTE-DAG: @globalfuncAlias = alias void (...), bitcast (void ()* @globalfunc to void (...)*)
----------------
maybe add ", since there is a single copy of each, and they are not exported and discardable" for clarity

================
Comment at: test/ThinLTO/X86/alias_resolution.ll:35
@@ +34,3 @@
+
+; Only alias to LinkonceODR function are resolved
+; (This is a current limitation of the implementation that could be lifted)
----------------
Shouldn't this be:
"Only linkonce_odr alias is resolved in the case when there is a single copy"
I guess this depends on what it means to be "resolved". I took "resolved" to mean we decided which copy to select, which isn't the case for most aliases.

================
Comment at: test/ThinLTO/X86/alias_resolution.ll:50
@@ +49,3 @@
+
+; Only import aliases that have been resolved, i.e. alias to a linkonce_odr.
+; IMPORT-NOT:  @linkonceODRfuncWeakAlias = weak alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
----------------
The word "resolved" here seems contradictory to how it is being used in the rest of the patch. Maybe change this to something like:
" Only import aliases that will not become available_externally, i.e. alias to a linkonce_odr."


http://reviews.llvm.org/D19308





More information about the llvm-commits mailing list