[PATCH] D35875: ThinLTO: Don't import aliases of any kind (even linkonce_odr)

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

> they had to be emitted linkonce_odr in all the destination modules (even if they weren't used by an alias) rather than as available_externally - causing extra object size.

Can you clarify this? It isn't clear to me.

> There's no reason to believe this particularly narrow alias importing was especially/meaningfully important, only that it was /possible/ to implement in this way.

Do you have benchmark results (SPEC and your internal ones)? Also statistics on the number of imports before/after?



================
Comment at: test/Linker/funcimport.ll:63
 ; An alias to an imported function is imported as alias if the function is not
 ; available_externally.
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=linkoncefunc:%t.bc -S | FileCheck %s --check-prefix=IMPORTGLOB5
----------------
Is this comment still up to date?


================
Comment at: test/ThinLTO/X86/alias_import.ll:5
 ; RUN: llvm-lto -thinlto-action=promote -thinlto-index %t.index.bc %t2.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=PROMOTE
-; RUN: llvm-lto -thinlto-action=promote -thinlto-index %t.index.bc %t2.bc -o - | llvm-lto -thinlto-action=internalize -thinlto-index %t.index.bc -thinlto-module-id=%t2.bc - -o - | llvm-dis -o - | FileCheck %s --check-prefix=PROMOTE-INTERNALIZE
 ; RUN: llvm-lto -thinlto-action=import -thinlto-index %t.index.bc %t1.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=IMPORT
----------------
It isn't clear to me that what is tested here is obsolete by your patch.


================
Comment at: test/Transforms/FunctionImport/funcimport.ll:43
 
 ; Aliases import the aliasee function
 declare void @linkoncealias(...) #1
----------------
Comment seems out-of-date?


https://reviews.llvm.org/D35875