ThinLTO: Add test cases for promote+internalize.

[pcc]

Summary

This tests the effect of both promotion and internalization on a module,
and helps show that D21883 is NFC wrt promotion+internalization.

[tejohnson]

test/ThinLTO/X86/alias_import.ll

91

Probably because alias to linkonce odr are the only aliases we will import. So they will be exported and therefore not internalizable. See selectCallee() in FunctionImport.cpp

tools/llvm-lto/llvm-lto.cpp

100

Rather than add a new type here, can the test just do two invocations of llvm-lto - one invoking promotion and then feeding the resulting bitcode into another invoking internalization?

[pcc]

test/ThinLTO/X86/alias_import.ll

91

Okay, I'll update the comment here

tools/llvm-lto/llvm-lto.cpp

100

That's what I tried first of all, but I got assertion failures like this:

llvm-lto: ../lib/Transforms/IPO/FunctionImport.cpp:536: auto llvm::thinLTOInternalizeModule(llvm::Module &, const GVSummaryMapTy &)::(anonymous class)::operator()(const llvm::GlobalValue &) const: Assertion `GS != DefinedGlobals.end()' failed.

I think this is caused by the internalization code looking up symbol names by GUID, which will be different as a result of the module identifier being different if the input bitcode is piped in. (I thought the whole point of the source_filename thing was to try to protect against that sort of problem, but evidently it doesn't help here -- even if I apply a diff like this:

diff --git a/test/ThinLTO/X86/Inputs/alias_import.ll b/test/ThinLTO/X86/Inputs/alias_import.ll
index 36e5ad1..5f1324b 100644
--- a/test/ThinLTO/X86/Inputs/alias_import.ll
+++ b/test/ThinLTO/X86/Inputs/alias_import.ll
@@ -1,6 +1,4 @@
-
-
-
+source_filename = "2.c"
 
 @globalfuncAlias = alias void (...), bitcast (void ()* @globalfunc to void (...)*)
 @globalfuncWeakAlias = weak alias void (...), bitcast (void ()* @globalfunc to void (...)*)
diff --git a/test/ThinLTO/X86/alias_import.ll b/test/ThinLTO/X86/alias_import.ll
index d4cc996..c7ffc12 100644
--- a/test/ThinLTO/X86/alias_import.ll
+++ b/test/ThinLTO/X86/alias_import.ll
@@ -2,7 +2,7 @@
 ; RUN: opt -module-summary %p/Inputs/alias_import.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc %t2.bc
 ; 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-internalize -thinlto-index %t.index.bc %t2.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=PROMOTE-INTERNALIZE
+; RUN: llvm-lto -thinlto-action=promote -thinlto-index %t.index.bc %t2.bc -o - | llvm-lto -thinlto-action=internalize -thinlto-index %t.index.bc -exported-symbol=dummy - -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
 
 ;
@@ -126,6 +126,8 @@
 ; PROMOTE-INTERNALIZE-DAG: define internal void @linkoncefunc()
 ; PROMOTE-INTERNALIZE-DAG: define internal void @weakfunc()
 
+source_filename = "1.c"
+
 define i32 @main() #0 {
 entry:
   call void @globalfuncAlias()

the combined summary still looks like this here:

<ENTRY [...] /> record string = '[...]/Output/alias_import.ll.tmp1.bc'
<ENTRY [...] /> record string = '[...]/Output/alias_import.ll.tmp2.bc

)

[tejohnson]

tools/llvm-lto/llvm-lto.cpp

100

The name in the combined summary is the path to the bitcode (used to locate a function during importing).

However, setting the source_filename in the LLVM assembly should cause the right GUID naming to occur, I'm not sure offhand why it isn't enabling consistent GUID computation. The code in thinLTOInternalizeModule does attempt to get the original name for something that has already been promoted. Can you look at the intermediate bitcode files and see if they contain a SOURCE_FILENAME record?

[pcc]

tools/llvm-lto/llvm-lto.cpp

100

The name in the combined summary is the path to the bitcode (used to locate a function during importing).

Ah, yes, of course.

However, setting the source_filename in the LLVM assembly should cause the right GUID naming to occur, I'm not sure offhand why it isn't enabling consistent GUID computation.

Okay, here's what's going on here.

• The client is using ModuleSummaryIndex::collectDefinedGVSummariesPerModule to group global value summaries by summary module path [1].
• We then use the module identifier as a key to look up the global value summary map entry [2].
• That lookup yields an empty map because the module identifier does not match any summary module path. As a result, none of the lookups in thinLTOInternalizeModule succeed.

I see two possible solutions here:

• do what I'm doing in this patch
• provide some way to override the module identifier in the loaded module in order to allow the lookup to succeed

I'm partial to the former because it's more in line with what I see as the end goal here (combining the internalize and promote phases). I don't think we should extend the module summary format (for example by allowing a source filename to be used as a key) for the same reason. Let me know what you think.

[1] http://llvm-cs.pcc.me.uk/lib/IR/ModuleSummaryIndex.cpp#88
[2] http://llvm-cs.pcc.me.uk/lib/LTO/ThinLTOCodeGenerator.cpp#663

[mehdi_amini]

tools/llvm-lto/llvm-lto.cpp

100

Could the option "-thinlto-module-id" to llvm-lto help here?

[pcc]

tools/llvm-lto/llvm-lto.cpp

100

Yes, that looks like exactly what I'm looking for. Let me see if that works.

[pcc]

• Instead of adding promote-internalize, pipe promote into internalize and use -thinlto-module-id
• Add promote+internalize test for a linkonce function

[pcc]

• Update comment

[mehdi_amini]

(Title is out-of-sync, just make sure it does not end-up in the final commit)

[tejohnson]

LGTM

test/ThinLTO/X86/alias_import.ll

 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/alias_import.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc %t2.bc
 ; 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-internalize -thinlto-index %t.index.bc %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
 
 ;
 ; Alias can't point to "available_externally", so we can only import an alias
 ; when we can import the aliasee with a linkage that won't be
 ; available_externally, i.e linkOnceODR. (FIXME this limitation could be lifted)
 ; PROMOTE-DAG: @globalfuncAlias = alias void (...), bitcast (void ()* @globalfunc to void (...)*)
 ; PROMOTE-DAG: @globalfuncWeakAlias = weak alias void (...), bitcast (void ()* @globalfunc to void (...)*)

 ; IMPORT-DAG: declare void @linkoncefuncWeakODRAlias()
 ; IMPORT-DAG: declare void @linkoncefuncLinkonceODRAlias()
 ; IMPORT-DAG: declare void @weakfuncAlias()
 ; IMPORT-DAG: declare void @weakfuncWeakAlias()
 ; IMPORT-DAG: declare void @weakfuncLinkonceAlias()
 ; IMPORT-DAG: declare void @weakfuncWeakODRAlias()
 ; IMPORT-DAG: declare void @weakfuncLinkonceODRAlias()
 
-
+; Promotion + internalization should internalize all of these (except for aliases of
+; linkonce_odr functions for some reason).
+; PROMOTE-INTERNALIZE-DAG: @globalfuncAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @globalfuncWeakAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @globalfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @globalfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @globalfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @internalfuncAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @internalfuncWeakAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @internalfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @internalfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @internalfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @internalfunc.llvm.0 to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncAlias = alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncWeakAlias = internal alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncLinkonceODRAlias = weak_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @weakODRfuncAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @weakODRfuncWeakAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @weakODRfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @weakODRfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @weakODRfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @linkoncefuncAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @linkoncefuncWeakAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @linkoncefuncLinkonceAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @linkoncefuncWeakODRAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @linkoncefuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @weakfuncAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @weakfuncWeakAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @weakfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @weakfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: @weakfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+; PROMOTE-INTERNALIZE-DAG: define internal void @globalfunc()
+; PROMOTE-INTERNALIZE-DAG: define internal void @internalfunc.llvm.0()
+; PROMOTE-INTERNALIZE-DAG: define internal void @linkonceODRfunc()
+; PROMOTE-INTERNALIZE-DAG: define internal void @weakODRfunc()
+; PROMOTE-INTERNALIZE-DAG: define internal void @linkoncefunc()
+; PROMOTE-INTERNALIZE-DAG: define internal void @weakfunc()
 
 define i32 @main() #0 {
 entry:
   call void @globalfuncAlias()
   call void @globalfuncWeakAlias()
   call void @globalfuncLinkonceAlias()
   call void @globalfuncWeakODRAlias()
   call void @globalfuncLinkonceODRAlias()

test/ThinLTO/X86/weak_resolution.ll

 ; Do setup work for all below tests: generate bitcode and combined index
 ; RUN: opt -module-summary %s -o %t.bc
 ; RUN: opt -module-summary %p/Inputs/weak_resolution.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t3.bc %t.bc %t2.bc
 
 ; Verify that prevailing weak for linker symbol is selected across modules,
 ; non-prevailing ODR are not kept when possible, but non-ODR non-prevailing
 ; are not affected.
 ; RUN: llvm-lto -thinlto-action=promote %t.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=MOD1
+; RUN: llvm-lto -thinlto-action=promote-internalize %t.bc -thinlto-index=%t3.bc -exported-symbol=linkoncefunc -o - | llvm-dis -o - | FileCheck %s --check-prefix=MOD1-INT
 ; RUN: llvm-lto -thinlto-action=promote %t2.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=MOD2
 ; When exported, we always preserve a linkonce
 ; RUN: llvm-lto -thinlto-action=promote %t.bc -thinlto-index=%t3.bc -o - --exported-symbol=linkonceodrfuncInSingleModule | llvm-dis -o - | FileCheck %s --check-prefix=EXPORTED
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
 
 ; Alias are resolved, but can't be turned into "available_externally"

 ; MOD1: define weak void @weakfunc()
 ; MOD2: define weak void @weakfunc()
 define weak void @weakfunc() #0 {
 entry:
   ret void
 }
 
 ; MOD1: define linkonce_odr void @linkonceodrfuncInSingleModule()
+; MOD1-INT: define internal void @linkonceodrfuncInSingleModule()
 ; EXPORTED: define weak_odr void @linkonceodrfuncInSingleModule()
 define linkonce_odr void @linkonceodrfuncInSingleModule() #0 {
 entry:
   ret void
 }

tools/llvm-lto/llvm-lto.cpp

 
 enum ThinLTOModes {
   THINLINK,
   THINDISTRIBUTE,
   THINEMITIMPORTS,
   THINPROMOTE,
   THINIMPORT,
   THININTERNALIZE,
+  THINPROMOTEINTERNALIZE,
   THINOPT,
   THINCODEGEN,
   THINALL
 };
 
 cl::opt<ThinLTOModes> ThinLTOMode(
     "thinlto-action", cl::desc("Perform a single ThinLTO stage:"),
     cl::values(
         clEnumValN(
             THINLINK, "thinlink",
             "ThinLink: produces the index by linking only the summaries."),
         clEnumValN(THINDISTRIBUTE, "distributedindexes",
                    "Produces individual indexes for distributed backends."),
         clEnumValN(THINEMITIMPORTS, "emitimports",
                    "Emit imports files for distributed backends."),
         clEnumValN(THINPROMOTE, "promote",
                    "Perform pre-import promotion (requires -thinlto-index)."),
         clEnumValN(THINIMPORT, "import", "Perform both promotion and "
                                          "cross-module importing (requires "
                                          "-thinlto-index)."),
         clEnumValN(THININTERNALIZE, "internalize",
                    "Perform internalization driven by -exported-symbol "
                    "(requires -thinlto-index)."),
+        clEnumValN(
+            THINPROMOTEINTERNALIZE, "promote-internalize",
+            "Perform promotion and internalization (requires -thinlto-index)."),
         clEnumValN(THINOPT, "optimize", "Perform ThinLTO optimizations."),
         clEnumValN(THINCODEGEN, "codegen", "CodeGen (expected to match llc)"),
         clEnumValN(THINALL, "run", "Perform ThinLTO end-to-end"),
         clEnumValEnd));
 
 static cl::opt<std::string>
     ThinLTOIndex("thinlto-index",
                  cl::desc("Provide the index produced by a ThinLink, required "
void run() {
     case THINEMITIMPORTS:
       return emitImports();
     case THINPROMOTE:
       return promote();
     case THINIMPORT:
       return import();
     case THININTERNALIZE:
       return internalize();
+    case THINPROMOTEINTERNALIZE:
+      return promoteInternalize();
     case THINOPT:
       return optimize();
     case THINCODEGEN:
       return codegen();
     case THINALL:
       return runAll();
     }
   }
for (auto &Filename : InputFilenames) {
       if (OutputName.empty()) {
         OutputName = Filename + ".thinlto.promoted.bc";
       }
       writeModuleToFile(*TheModule, OutputName);
     }
   }
 
   /// Load the combined index from disk, then load every file referenced by
+  /// the index and add them to the generator, finally perform promotion
+  /// and internalization on the files mentioned on the command line (these must
+  /// match the index content).
+  void promoteInternalize() {
+    if (InputFilenames.size() != 1 && !OutputFilename.empty())
+      report_fatal_error("Can't handle a single output filename and multiple "
+                         "input files, do not provide an output filename and "
+                         "the output files will be suffixed from the input "
+                         "ones.");
+
+    auto Index = loadCombinedIndex();
+    for (auto &Filename : InputFilenames) {
+      LLVMContext Ctx;
+      auto TheModule = loadModule(Filename, Ctx);
+
+      ThinGenerator.promote(*TheModule, *Index);
+      ThinGenerator.internalize(*TheModule, *Index);
+
+      std::string OutputName = OutputFilename;
+      if (OutputName.empty()) {
+        OutputName = Filename + ".thinlto.promoted.bc";
+      }
+      writeModuleToFile(*TheModule, OutputName);
+    }
+  }
+
+  /// Load the combined index from disk, then load every file referenced by
   /// the index and add them to the generator, then performs the promotion and
   /// cross module importing on the files mentioned on the command line
   /// (these must match the index content).
   void import() {
     if (InputFilenames.size() != 1 && !OutputFilename.empty())
       report_fatal_error("Can't handle a single output filename and multiple "
                          "input files, do not provide an output filename and "
                          "the output files will be suffixed from the input "