[PATCH] D14623: [ThinLTO] Comdat importing fixes and related cleanup

[Rafael Espíndola via llvm-commits]

On 30 November 2015 at 11:39, Teresa Johnson <tejohnson at google.com> wrote:
> tejohnson added inline comments.
>
> ================
> Comment at: lib/Linker/LinkModules.cpp:558
> @@ +557,3 @@
> +  void stripDeclsFromComdats();
> +  void convertInvalidAliasesToDeclarations();
> +
> ----------------
> rafael wrote:
>> We cannot convert aliases to declarations.
> Why? Note that after the conversion it isn't an alias, just a declaration to an externally defined symbol. This is needed to import anything that references an alias (especially now with your change in r254170).
>
> I.e. if we import function foo() and it calls f() which is actually an alias to function f2(). If we don't import f2 (or with r254170 if f2() is not linkonce), then f() cannot be an alias in the destination (importing) module. So we convert the imported f() alias into a declaration. The linker will resolve this to the f() alias in the importing module eventually. It shouldn't be any different than having a module reference an alias defined elsewhere originally.

Why do we create the alias in the first place?

If we don't want the alias to be imported, we should have not tried to clone it.
If we want to import the alias but not the aliasee, we have to convert
the aliasee to internal linkage. That is something that is currently
done only in the gold plugin. The existing llvm-link just gives the
wrong result.

All the logic behind doImportAsDefinition is something I hope to move
out of the IR moving logic with the IRMover.

Cheers,
Rafael