[LTO] Add test coverage for previous dllimport fix

[ormris]

Summary

Internal testing showed that the change made in commit 62fcfc5a needed more test coverage. Specifically, the imported function shouldn't be externally visible and the whole test needed to be run in regular LTO mode.

[tejohnson]

The original patch (D55627) added test/LTO/Resolution/X86/local-def-dllimport.ll, which tested ThinLTO. Does that test need to be changed? Is the main issue that we were only testing ThinLTO and not regular LTO?

[ormris]

No, I don't think we need to modify the original test. It's now used to test the changes in d2ef8c1f.

[ormris]

ping

[tejohnson]

lgtm

In D148950#4293068, @ormris wrote:

No, I don't think we need to modify the original test. It's now used to test the changes in d2ef8c1f.

Ah I see. Looks like that test was then actually testing the similar support in llvm/lib/Transforms/Utils/FunctionImportUtils.cpp (which is why it was affected by d2ef8c1f aka D74751), and not the support in LTO.cpp added in D55627. Which makes me wonder if we now have any tests covering the removal of the dllimport in FunctionImportUtils.cpp... After looking, that was added in f5220fb68f2baef2e6d2e0bf11c12caa9e22ef29 and there was a test added with it (llvm/test/ThinLTO/X86/dsolocal_dllimport.ll). Can you make sure that test is still testing the handling in FunctionImportUtils.cpp, and also add a comment to llvm/test/LTO/Resolution/X86/local-def-dllimport.ll as to what it is actually testing (which I guess is that we *don't* remove the dllimport if we didn't mark a def as dso_local during the function import handling?).

[ormris]

Thanks! I'll take a look at those other testing-related questions...

llvm/test/LTO/Resolution/X86/protected-def-dllimport.ll

+; RUN: llvm-as --disable-verify -o %t0.bc %s
+
+; RUN: llvm-lto2 run -r %t0.bc,__imp_f,l \
+; RUN:               -r %t0.bc,g,p \
+; RUN:               -r %t0.bc,main,x \
+; RUN:               -save-temps -o %t1 %t0.bc
+; RUN: llvm-dis %t1.0.0.preopt.bc -o - | FileCheck %s
+source_filename = "test.cpp"
+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"
+
[e-mail] = appending global [2 x ptr] [ ptr @main, ptr @f ], section "llvm.metadata"
+
+$g = comdat any
+@g = global i8 42, comdat, !type !0
+
+; CHECK: declare protected ptr @f()
+define protected dllimport i8* @f() {
+  ret i8* @g
+}
+
+define available_externally i32 @main() {
+  %1 = call i8* @f()
+  %2 = ptrtoint i8* %1 to i32
+  ret i32 %2
+}
+
+!llvm.module.flags = !{!1}
+
+!0 = !{i32 0, !"typeid"}
+!1 = !{i32 1, !"ThinLTO", i32 0}