[ThinLTO] Clear dllimport when setting dso_local

[• espindola]

Summary

This is PR36686.

If a user of a library is LTOed with that library we take the opportunity to set dso_local, but we don't clear dllimport, which creates an invalid IR.

[grimar]

Looks reasonable to me.
(I think Teresa / Peter are more appropriate people to give a final approvement).

[tejohnson]

LGTM, but what about the regular LTO case (i.e. we set this directly on the GV in LTO.cpp, instead of on the summary and then propagating in the backends here for ThinLTO)?

[• rafael]

Teresa Johnson via Phabricator via llvm-commits
<[e-mail]> writes:

tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

LGTM, but what about the regular LTO case (i.e. we set this directly on the GV in LTO.cpp, instead of on the summary and then propagating in the backends here for ThinLTO)?

The regular lib/Linker code already handles this.

Cheers,
Rafael

[• espindola]

r327408

lib/Transforms/Utils/FunctionImportUtils.cpp

 }
 
 void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
 
   // Check the summaries to see if the symbol gets resolved to a known local
   // definition.
   if (GV.hasName()) {
     ValueInfo VI = ImportIndex.getValueInfo(GV.getGUID());
-    if (VI && VI.isDSOLocal())
+    if (VI && VI.isDSOLocal()) {
       GV.setDSOLocal(true);
+      if (GV.hasDLLImportStorageClass())
+        GV.setDLLStorageClass(GlobalValue::DefaultStorageClass);
+    }
   }
 
   bool DoPromote = false;
   if (GV.hasLocalLinkage() &&
       ((DoPromote = shouldPromoteLocalToGlobal(&GV)) || isPerformingImport())) {
     // Once we change the name or linkage it is difficult to determine
     // again whether we should promote since shouldPromoteLocalToGlobal needs
     // to locate the summary (based on GUID from name and linkage). Therefore,

test/ThinLTO/X86/Inputs/dsolocal_dllimport.ll

+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.11.0"
+define dso_local dllexport i32 @foo() {
+  ret i32 42
+}

test/ThinLTO/X86/dsolocal_dllimport.ll

+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %S/Inputs/dsolocal_dllimport.ll -o %t2.bc
+; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t -r=%t1.bc,bar,px -r=%t1.bc,__imp_foo, -r=%t2.bc,foo -save-temps
+; RUN: llvm-dis < %t.1.3.import.bc | FileCheck %s
+
+; If a user (dllimport) is LTOed with a library, check that we replace dllimport with dso_local.
+
+; CHECK: declare dso_local void @foo()
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.11.0"
+define void @bar() {
+  call void @foo()
+  ret void
+}
+declare dllimport void @foo()