[lld-macho] Extend lto-internalize-unnamed-addr.ll

[int3]

Summary

• Test the case where a symbol is sometimes linkonce_odr and sometimes weak_odr
• Test the visibility of the symbols at the IR level, after the internalize stage of LTO is done. (Previously we only checked the visibility of symbols in the final output binary.)

[modimo]

lld/test/MachO/lto-internalize-unnamed-addr.ll

28

thinlto+dylib shares the same output as thinlto by itself. Can check for that as well in the bitcode output

49

I find it odd that local_unnamed is able to be internalized in fullLTO but not thinLTO. The code path is https://github.com/llvm/llvm-project/blob/9f616a467fc710f084b7e57812a2ed64c214c2c6/llvm/lib/LTO/LTO.cpp#L349 where any linkonce_odr gets upgraded to weak but if this is the only copy that exists it appears to fit within the rules coming from D20348. cc @tejohnson is there an opportunity here?

70

Add final output binary check for LTO-DAG and THINLTO-DAG as well?

[int3]

update

lld/test/MachO/lto-internalize-unnamed-addr.ll

49

I realize global_unnamed_sometimes_linkonce has the same issue too. I've added a FIXME for this

70

whoops, yeah. missed this

[tejohnson]

lld/test/MachO/lto-internalize-unnamed-addr.ll

49

That's true, if there is only one copy no need to make the prevailing one weak_odr. Is changing that sufficient to get the internalization in the ThinLTO case?

[int3]

lld/test/MachO/lto-internalize-unnamed-addr.ll

49

I tried https://gist.github.com/int3/b66de20462da71e8757e09bacfc361ca but that just seems to result in global_unnamed and local_unnamed_const no longer being marked as hidden...

[tejohnson]

lld/test/MachO/lto-internalize-unnamed-addr.ll

49

To get hidden back this would need to be adjusted accordingly to handle linkonce_odr:
http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/FunctionImport.cpp#980

[int3]

lld/test/MachO/lto-internalize-unnamed-addr.ll

49

ah gotcha. yeah I got the hiddens back after changing that, as well as removing this earlier condition: http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/FunctionImport.cpp#952

A bunch of check-llvm tests are now failing though. I would like to go through them at some point and figure out if they're legit, but I'd like to focus on other aspects of LLD for now. @modimo do you mind stamping this first?

[modimo]

lld/test/MachO/lto-internalize-unnamed-addr.ll

49

Sure.

lld/test/MachO/lto-internalize-unnamed-addr.ll

 ; REQUIRES: x86
 ; RUN: rm -rf %t; split-file %s %t
 ;; This test covers both FullLTO and ThinLTO code paths because we have observed
 ;; (unexpected) differences between the two.
 ; RUN: llvm-as %t/test.ll -o %t/test.o
 ; RUN: llvm-as %t/test2.ll -o %t/test2.o
 ; RUN: opt -module-summary %t/test.ll -o %t/test.thinlto.o
 ; RUN: opt -module-summary %t/test2.ll -o %t/test2.thinlto.o
 
-; RUN: %lld -lSystem %t/test.o %t/test2.o -o %t/test
+; RUN: %lld -lSystem %t/test.o %t/test2.o -o %t/test -save-temps
+; RUN: llvm-dis < %t/test.0.2.internalize.bc | FileCheck %s --check-prefix=LTO-BC
 ; RUN: llvm-nm -m %t/test | FileCheck %s --check-prefix=LTO
 
-; RUN: %lld -lSystem -dylib %t/test.o %t/test2.o -o %t/test.dylib
+; RUN: %lld -lSystem -dylib %t/test.o %t/test2.o -o %t/test.dylib -save-temps
+; RUN: llvm-dis < %t/test.dylib.0.2.internalize.bc | FileCheck %s --check-prefix=LTO-BC-DYLIB
 ; RUN: llvm-nm -m %t/test.dylib | FileCheck %s --check-prefix=LTO-DYLIB
 
-; RUN: %lld -lSystem %t/test.thinlto.o %t/test2.thinlto.o -o %t/test.thinlto
+; RUN: %lld -lSystem %t/test.thinlto.o %t/test2.thinlto.o -o %t/test.thinlto \
+; RUN:   -save-temps
+; RUN: llvm-dis < %t/test.thinlto.o.2.internalize.bc | FileCheck %s --check-prefix=THINLTO-BC
+; RUN: llvm-dis < %t/test2.thinlto.o.2.internalize.bc | FileCheck %s --check-prefix=THINLTO-BC-2
 ; RUN: llvm-nm -m %t/test.thinlto | FileCheck %s --check-prefix=THINLTO
 
-; RUN: %lld -lSystem -dylib %t/test.thinlto.o %t/test2.thinlto.o -o %t/test.thinlto.dylib
+; RUN: %lld -lSystem -dylib %t/test.thinlto.o %t/test2.thinlto.o -o \
+; RUN:   %t/test.thinlto.dylib -save-temps
+; RUN: llvm-dis < %t/test.thinlto.o.2.internalize.bc | FileCheck %s --check-prefix=THINLTO-BC
+; RUN: llvm-dis < %t/test2.thinlto.o.2.internalize.bc | FileCheck %s --check-prefix=THINLTO-BC-2
 ; RUN: llvm-nm -m %t/test.thinlto.dylib | FileCheck %s --check-prefix=THINLTO
 
+; LTO-BC-DAG: @global_unnamed = internal unnamed_addr global i8 42
+; LTO-BC-DAG: @global_unnamed_sometimes_linkonce = internal unnamed_addr global i8 42
+; LTO-BC-DAG: @local_unnamed_const = internal constant i8 42
+; LTO-BC-DAG: @local_unnamed = internal global i8 42
+; LTO-BC-DAG: @local_unnamed_always_const = internal constant i8 42
+; LTO-BC-DAG: @local_unnamed_sometimes_const = internal constant i8 42
+
+; LTO-BC-DYLIB-DAG: @global_unnamed = internal unnamed_addr global i8 42
+; LTO-BC-DYLIB-DAG: @local_unnamed_const = internal constant i8 42
+; LTO-BC-DYLIB-DAG: @local_unnamed = weak_odr global i8 42
+; LTO-BC-DYLIB-DAG: @global_unnamed_sometimes_linkonce = weak_odr unnamed_addr global i8 42
+; LTO-BC-DYLIB-DAG: @local_unnamed_always_const = internal constant i8 42
+; LTO-BC-DYLIB-DAG: @local_unnamed_sometimes_const = weak_odr constant i8 42
+
+; THINLTO-BC-DAG: @global_unnamed = weak_odr hidden unnamed_addr global i8 42
+; THINLTO-BC-DAG: @global_unnamed_sometimes_linkonce = weak_odr unnamed_addr global i8 42
+; THINLTO-BC-DAG: @local_unnamed_const = weak_odr hidden local_unnamed_addr constant i8 42
+; THINLTO-BC-DAG: @local_unnamed_always_const = weak_odr hidden local_unnamed_addr constant i8 42
+; THINLTO-BC-DAG: @local_unnamed_sometimes_const = weak_odr local_unnamed_addr constant i8 42
+; THINLTO-BC-DAG: @local_unnamed = weak_odr local_unnamed_addr global i8 42
+
+; THINLTO-BC-2-DAG: @global_unnamed_sometimes_linkonce = available_externally unnamed_addr global i8 42
+; THINLTO-BC-2-DAG: @local_unnamed_always_const = available_externally local_unnamed_addr constant i8 42
+; THINLTO-BC-2-DAG: @local_unnamed_sometimes_const = available_externally local_unnamed_addr global i8 42
+
 ; LTO-DAG: (__DATA,__data) non-external _global_unnamed
+; LTO-DAG: (__DATA,__data) non-external _global_unnamed_sometimes_linkonce
 ; LTO-DAG: (__DATA,__data) non-external _local_unnamed
 ;; LD64 marks this with (was a private external). IMO both LD64 and LLD should
 ;; mark all the other internalized symbols with (was a private external).
 ; LTO-DAG: (__TEXT,__const) non-external _local_unnamed_always_const
 ; LTO-DAG: (__TEXT,__const) non-external _local_unnamed_const
 ;; LD64 doesn't internalize this -- it emits it as a weak external -- which I
 ;; think is a missed optimization on its end.
 ; LTO-DAG: (__TEXT,__const) non-external _local_unnamed_sometimes_const
 
 ;; The output here is largely identical to LD64's, except that the non-external
 ;; symbols here are all marked as (was a private external) by LD64. LLD should
 ;; follow suit.
 ; LTO-DYLIB-DAG: (__DATA,__data) non-external _global_unnamed
+; LTO-DYLIB-DAG: (__DATA,__data) weak external _global_unnamed_sometimes_linkonce
 ; LTO-DYLIB-DAG: (__DATA,__data) weak external _local_unnamed
 ; LTO-DYLIB-DAG: (__TEXT,__const) non-external _local_unnamed_always_const
 ; LTO-DYLIB-DAG: (__TEXT,__const) non-external _local_unnamed_const
 ; LTO-DYLIB-DAG: (__TEXT,__const) weak external _local_unnamed_sometimes_const
 
 ; THINLTO-DAG: (__DATA,__data) non-external (was a private external) _global_unnamed
+;; FIXME: These next two symbols should probably be internalized, just like they
+;; are under fullLTO.
+; THINLTO-DAG: (__DATA,__data) weak external _global_unnamed_sometimes_linkonce
 ; THINLTO-DAG: (__DATA,__data) weak external _local_unnamed
 ; THINLTO-DAG: (__TEXT,__const) non-external (was a private external) _local_unnamed_always_const
 ; THINLTO-DAG: (__TEXT,__const) non-external (was a private external) _local_unnamed_const
 ;; LD64 actually fails to link when the following symbol is included in the test
 ;; input, instead producing this error:
 ;; reference to bitcode symbol '_local_unnamed_sometimes_const' which LTO has not compiled in '_used' from /tmp/lto.o for architecture x86_64
 ; THINLTO-DAG: (__TEXT,__const) weak external _local_unnamed_sometimes_const
 
 ;--- test.ll
 target triple = "x86_64-apple-darwin"
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
 @global_unnamed = linkonce_odr unnamed_addr global i8 42
+@global_unnamed_sometimes_linkonce = linkonce_odr unnamed_addr global i8 42
 @local_unnamed_const = linkonce_odr local_unnamed_addr constant i8 42
 @local_unnamed_always_const = linkonce_odr local_unnamed_addr constant i8 42
 @local_unnamed_sometimes_const = linkonce_odr local_unnamed_addr constant i8 42
 @local_unnamed = linkonce_odr local_unnamed_addr global i8 42
-@used = hidden constant [5 x i8*] [i8* @global_unnamed, i8* @local_unnamed,
+@used = hidden constant [6 x i8*] [i8* @global_unnamed,
+  i8* @global_unnamed_sometimes_linkonce, i8* @local_unnamed,
   i8* @local_unnamed_const, i8* @local_unnamed_always_const,
   i8* @local_unnamed_sometimes_const]
[e-mail] = appending global [1 x [5 x i8*]*] [[5 x i8*]* @used]
[e-mail] = appending global [1 x [6 x i8*]*] [[6 x i8*]* @used]
 
 define void @main() {
   ret void
 }
 
 ;--- test2.ll
 target triple = "x86_64-apple-darwin"
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
+@global_unnamed_sometimes_linkonce = weak_odr unnamed_addr global i8 42
 @local_unnamed_always_const = linkonce_odr local_unnamed_addr constant i8 42
 @local_unnamed_sometimes_const = linkonce_odr local_unnamed_addr global i8 42