Revert "[ThinLTO][test] Add visibility related tests"

[bgraur]

Summary

Both newly added tests fail in Release.

This reverts commit 52aa4e210744361a5ed6dc50fef78ed91706e508.

[MaskRay]

How did it break tests? It tested well on Linux/macOS/Windows.

[MaskRay]

I am pretty sure this is a misfire.

[MaskRay]

This was due to a -DLLVM_ENABLE_ASSERTIONS=off vs -DLLVM_ENABLE_ASSERTIONS=on difference. llvm-lto2 needs -enable-import-metadata

[bgraur]

Ideally, this roll-forward should also have included this fix.
The test failure was 100% reproducible.

[MaskRay]

In D93957#2475753, @bgraur wrote:

Ideally, this roll-forward should also have included this fix.
The test failure was 100% reproducible.

I have checked some Linux/macOS/Windows built bots and even tested on a ppc64le machine. The description of this commit isn't clear about what happened.

[bgraur]

The descritption is: "Both newly added tests fail in Release."

That means:
1/ Build llvm in Release configuration.
2/ Execute the tests and they will fail.

Checking any build bots won't show any failure because the problem is a test failure, not a build breakage.

[MaskRay]

Most build bots are configured with -DLLVM_ENABLE_ASSERTIONS=ON. Anyhow I figured out the root cause and make the behavior not dependent on -DLLVM_ENABLE_ASSERTIONS= to prevent future issues like this.

[lebedev.ri]

In D93957#2475760, @bgraur wrote:

The descritption is: "Both newly added tests fail in Release."

That means:
1/ Build llvm in Release configuration.
2/ Execute the tests and they will fail.

Checking any build bots won't show any failure because the problem is a test failure, not a build breakage.

Are you suggesting that buildbots don't actually run tests?

[bgraur]

Not sure how the build bots are configured.

What is sure is that if you run the tests using the llvm tools these tests will always fail in Release.

So, the conclusion is one of the following:
1/ the bots didn't run your tests.
2/ the bots have a different run configuration which hides the failures.

llvm/test/ThinLTO/X86/visibility-elf.ll

-; RUN: split-file %s %t
-; RUN: opt -module-summary %t/a.ll -o %ta.bc
-; RUN: opt -module-summary %t/b.ll -o %tb.bc
-
-;; Test visibility propagation. The prevailing definitions are all from %tb.bc.
-; RUN: llvm-lto2 run -save-temps -o %t1.bc %ta.bc %tb.bc \
-; RUN:   -r=%ta.bc,var1,l -r=%ta.bc,var2,l \
-; RUN:   -r=%ta.bc,hidden_def_weak_def,l -r=%ta.bc,protected_def_weak_def,l -r=%ta.bc,protected_def_weak_hidden_def,l \
-; RUN:   -r=%ta.bc,protected_def_hidden_ref,l -r=%ta.bc,not_imported,l -r=%ta.bc,hidden_def_ref,l \
-; RUN:   -r=%ta.bc,hidden_def_weak_ref,l \
-; RUN:   -r=%ta.bc,ext, -r=%ta.bc,main,plx \
-; RUN:   -r=%tb.bc,var1,plx -r=%tb.bc,var2,plx \
-; RUN:   -r=%tb.bc,hidden_def_weak_def,pl -r=%tb.bc,protected_def_weak_def,pl -r=%tb.bc,protected_def_weak_hidden_def,pl \
-; RUN:   -r=%tb.bc,protected_def_hidden_ref,pl -r=%tb.bc,not_imported,pl -r=%tb.bc,hidden_def_ref,pl \
-; RUN:   -r=%tb.bc,hidden_def_weak_ref,pl
-; RUN: llvm-dis < %t1.bc.1.3.import.bc | FileCheck %s
-
-;; %tb.bc does not import anything, so we just check 1.promote.
-; RUN: llvm-dis < %t1.bc.2.1.promote.bc | FileCheck %s --check-prefix=CHECK2
-
-;--- a.ll
-target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-; CHECK:      @var1 = external global i32, align 4
-; CHECK-NEXT: @var2 = available_externally hidden global i32 1, align 4
-
-@var1 = weak global i32 1, align 4
-@var2 = extern_weak global i32
-
-declare void @ext(void ()*)
-
-; CHECK: declare i32 @hidden_def_weak_def()
-; CHECK: declare void @protected_def_weak_def()
-; CHECK: declare hidden void @protected_def_weak_hidden_def()
-;; Currently the visibility is not propagated onto an unimported function,
-;; because we don't have summaries for declarations.
-; CHECK: declare extern_weak void @not_imported()
-; CHECK: define available_externally hidden void @hidden_def_ref() !thinlto_src_module !0
-; CHECK: define available_externally hidden void @hidden_def_weak_ref() !thinlto_src_module !0
-;; This can be hidden, but we cannot communicate the declaration's visibility
-;; to other modules because declarations don't have summaries, and the IRLinker
-;; overrides it when importing the protected def.
-; CHECK: define available_externally protected void @protected_def_hidden_ref() !thinlto_src_module !0
-
-; CHECK2: define hidden i32 @hidden_def_weak_def()
-; CHECK2: define protected void @protected_def_weak_def()
-; CHECK2: define protected void @protected_def_weak_hidden_def()
-; CHECK2: define hidden void @hidden_def_ref()
-; CHECK2: define hidden void @hidden_def_weak_ref()
-; CHECK2: define protected void @protected_def_hidden_ref()
-; CHECK2: define hidden void @not_imported()
-
-define weak i32 @hidden_def_weak_def() {
-entry:
-  %0 = load i32, i32* @var2
-  ret i32 %0
-}
-
-define weak void @protected_def_weak_def() {
-entry:
-  ret void
-}
-
-define weak hidden void @protected_def_weak_hidden_def() {
-entry:
-  ret void
-}
-
-declare extern_weak void @not_imported()
-
-declare void @hidden_def_ref()
-declare extern_weak void @hidden_def_weak_ref()
-declare hidden void @protected_def_hidden_ref()
-
-define i32 @main() {
-entry:
-  call void @ext(void ()* bitcast (i32 ()* @hidden_def_weak_def to void ()*))
-  call void @ext(void ()* @protected_def_weak_def)
-  call void @ext(void ()* @protected_def_weak_hidden_def)
-  call void @ext(void ()* @hidden_def_ref)
-  call void @ext(void ()* @hidden_def_weak_ref)
-  call void @ext(void ()* @protected_def_hidden_ref)
-  call void @ext(void ()* @not_imported)
-
-  ;; Calls ensure the functions are imported.
-  call i32 @hidden_def_weak_def()
-  call void @protected_def_weak_def()
-  call void @protected_def_weak_hidden_def()
-  call void @hidden_def_ref()
-  call void @hidden_def_weak_ref()
-  call void @protected_def_hidden_ref()
-  ret i32 0
-}
-
-;--- b.ll
-target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-@var1 = hidden global i32 1, align 4
-@var2 = hidden global i32 1, align 4
-
-define hidden i32 @hidden_def_weak_def() {
-entry:
-  %0 = load i32, i32* @var1
-  ret i32 %0
-}
-
-define protected void @protected_def_weak_def() {
-entry:
-  ret void
-}
-
-define protected void @protected_def_weak_hidden_def() {
-entry:
-  ret void
-}
-
-define hidden void @hidden_def_ref() {
-entry:
-  ret void
-}
-
-define hidden void @hidden_def_weak_ref() {
-entry:
-  ret void
-}
-
-define protected void @protected_def_hidden_ref() {
-entry:
-  ret void
-}
-
-define hidden void @not_imported() {
-entry:
-  ret void
-}

llvm/test/ThinLTO/X86/visibility-macho.ll

-; RUN: split-file %s %t
-; RUN: opt -module-summary %t/a.ll -o %ta.bc
-; RUN: opt -module-summary %t/b.ll -o %tb.bc
-
-;; Test visibility propagation. The prevailing definitions are all from %tb.bc.
-; RUN: llvm-lto2 run -save-temps -o %t1.bc %ta.bc %tb.bc \
-; RUN:   -r=%ta.bc,_var1,l -r=%ta.bc,_var2,l \
-; RUN:   -r=%ta.bc,_hidden_def_weak_def,l -r=%ta.bc,_not_imported,l -r=%ta.bc,_hidden_def_ref,l \
-; RUN:   -r=%ta.bc,_hidden_def_weak_ref,l \
-; RUN:   -r=%ta.bc,_ext, -r=%ta.bc,_main,plx \
-; RUN:   -r=%tb.bc,_var1,plx -r=%tb.bc,_var2,plx \
-; RUN:   -r=%tb.bc,_hidden_def_weak_def,pl -r=%tb.bc,_not_imported,pl -r=%tb.bc,_hidden_def_ref,pl \
-; RUN:   -r=%tb.bc,_hidden_def_weak_ref,pl
-; RUN: llvm-dis < %t1.bc.1.3.import.bc | FileCheck %s
-; RUN: llvm-dis < %t1.bc.2.1.promote.bc | FileCheck %s --check-prefix=CHECK2
-
-;--- a.ll
-target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-apple-macosx10.15.0"
-
-; CHECK:      @var1 = external global i32, align 4
-; CHECK-NEXT: @var2 = available_externally hidden global i32 1, align 4
-
-@var1 = weak global i32 1, align 4
-@var2 = extern_weak global i32
-
-declare void @ext(void ()*)
-
-; CHECK: declare i32 @hidden_def_weak_def()
-;; Currently the visibility is not propagated for an unimported function.
-; CHECK: declare extern_weak dso_local void @not_imported()
-; CHECK: define available_externally hidden void @hidden_def_ref() !thinlto_src_module !0
-; CHECK: define available_externally hidden void @hidden_def_weak_ref() !thinlto_src_module !0
-
-; CHECK2: define hidden i32 @hidden_def_weak_def()
-; CHECK2: define hidden void @hidden_def_ref()
-; CHECK2: define hidden void @hidden_def_weak_ref()
-; CHECK2: define hidden void @not_imported()
-
-define weak i32 @hidden_def_weak_def() {
-entry:
-  %0 = load i32, i32* @var2
-  ret i32 %0
-}
-
-declare extern_weak void @not_imported()
-
-declare void @hidden_def_ref()
-declare extern_weak void @hidden_def_weak_ref()
-
-define i32 @main() {
-entry:
-  call void @ext(void ()* bitcast (i32 ()* @hidden_def_weak_def to void ()*))
-  call void @ext(void ()* @hidden_def_ref)
-  call void @ext(void ()* @hidden_def_weak_ref)
-  call void @ext(void ()* @not_imported)
-
-  ;; Calls ensure the functions are imported.
-  call i32 @hidden_def_weak_def()
-  call void @hidden_def_ref()
-  call void @hidden_def_weak_ref()
-  ret i32 0
-}
-
-;--- b.ll
-target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-apple-macosx10.15.0"
-
-@var1 = hidden global i32 1, align 4
-@var2 = hidden global i32 1, align 4
-
-define hidden i32 @hidden_def_weak_def() {
-entry:
-  %0 = load i32, i32* @var1
-  ret i32 %0
-}
-
-define hidden void @hidden_def_ref() {
-entry:
-  ret void
-}
-
-define hidden void @hidden_def_weak_ref() {
-entry:
-  ret void
-}
-
-define hidden void @not_imported() {
-entry:
-  ret void
-}