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

[Mehdi AMINI via llvm-commits]

joker.eph marked 3 inline comments as done.

================
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 (...)*)
----------------
tejohnson wrote:
> maybe add ", since there is a single copy of each, and they are not exported and discardable" for clarity
The "since there is a single copy of each" part seems irrelevant: this is really about are they exported or not. 
And we only import alias when they point to a linkonce_odr function, so these are not exported and won't need to be resolved (I'm not sure if this limitation has any other source than being inherited from the old ModuleLinker).


================
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)
----------------
tejohnson wrote:
> 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.
Resolved in the sense of the function `ResolveODR`:

```
/// We'd like to drop these function if they are no longer referenced in the
/// current module. However there is a chance that another module is still
/// referencing them because of the import. We make sure we always emit at least
/// one copy.
```

So it is about the aliases being linkonce *and* being exported, that needs to be turned into weak(_odr).

Are we talking about the same "resolve"?

================
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 (...)*)
----------------
tejohnson wrote:
> 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."
I should write "The current implementation is limited (for no real reasons) to aliases to linkonce_odr", unless there is something I missed...



http://reviews.llvm.org/D19308