[ThinLTO] Disable importing and other cross-module optis at -O0

[tejohnson]

Summary

There is no point to importing at -O0, since we won't inline. We should
also disable other cross-module optimizations.

(Plan to backport this fix to the 3.9 branch to fix PR30774)

[tejohnson]

Ping

[tejohnson]

Forgot to add llvm-commits before, just added. There haven't been any comments yet, so only thing missing on mailing list is the original description:

There is no point to importing at -O0, since we won't inline. We should
also disable other cross-module optimizations.

(Plan to backport this fix to the 3.9 branch to fix PR30774)

[pcc]

LGTM

Regarding 3.9 I don't think it's worth backporting this as a workaround for some other bug, you'd need a separate patch anyway since 3.9 branched before the new LTO API landed.

[mehdi_amini]

The list does not have the patch either

[tejohnson]

In D25918#583785, @pcc wrote:

LGTM

Regarding 3.9 I don't think it's worth backporting this as a workaround for some other bug, you'd need a separate patch anyway since 3.9 branched before the new LTO API landed.

That's true it won't be a straight backport to 3.9, due to the different implementation. But I think it is the most straightforward way to address the other bug, which doesn't have a simple fix.

lib/LTO/LTO.cpp

Error LTO::runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache,
   for (auto &Mod : ThinLTO.ModuleMap)
     if (!ModuleToDefinedGVSummaries.count(Mod.first))
       ModuleToDefinedGVSummaries.try_emplace(Mod.first);
 
   StringMap<FunctionImporter::ImportMapTy> ImportLists(
       ThinLTO.ModuleMap.size());
   StringMap<FunctionImporter::ExportSetTy> ExportLists(
       ThinLTO.ModuleMap.size());
+  StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> ResolvedODR;
+
+  if (Conf.OptLevel > 0) {
     ComputeCrossModuleImport(ThinLTO.CombinedIndex, ModuleToDefinedGVSummaries,
                              ImportLists, ExportLists);
 
     std::set<GlobalValue::GUID> ExportedGUIDs;
     for (auto &Res : GlobalResolutions) {
       if (!Res.second.IRName.empty() &&
           Res.second.Partition == GlobalResolution::External)
         ExportedGUIDs.insert(GlobalValue::getGUID(Res.second.IRName));
     }
 
-  auto isPrevailing = [&](GlobalValue::GUID GUID, const GlobalValueSummary *S) {
+    auto isPrevailing = [&](GlobalValue::GUID GUID,
+                            const GlobalValueSummary *S) {
       return ThinLTO.PrevailingModuleForGUID[GUID] == S->modulePath();
     };
     auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
       const auto &ExportList = ExportLists.find(ModuleIdentifier);
       return (ExportList != ExportLists.end() &&
               ExportList->second.count(GUID)) ||
              ExportedGUIDs.count(GUID);
     };
     thinLTOInternalizeAndPromoteInIndex(ThinLTO.CombinedIndex, isExported);
 
-  StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> ResolvedODR;
     auto recordNewLinkage = [&](StringRef ModuleIdentifier,
                                 GlobalValue::GUID GUID,
                                 GlobalValue::LinkageTypes NewLinkage) {
       ResolvedODR[ModuleIdentifier][GUID] = NewLinkage;
     };
 
     thinLTOResolveWeakForLinkerInIndex(ThinLTO.CombinedIndex, isPrevailing,
                                        recordNewLinkage);
+  }
 
   std::unique_ptr<ThinBackendProc> BackendProc =
       ThinLTO.Backend(Conf, ThinLTO.CombinedIndex, ModuleToDefinedGVSummaries,
                       AddStream, Cache);
 
   // Partition numbers for ThinLTO jobs start at 1 (see comments for
   // GlobalResolution in LTO.h). Task numbers, however, start at
   // ParallelCodeGenParallelismLevel if an LTO module is present, as tasks 0

test/ThinLTO/X86/Inputs/funcimport2.ll

+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+
+define i32 @main() #0 {
+entry:
+  call void (...) @foo()
+  ret i32 0
+}
+
+declare void @foo(...) #1

test/ThinLTO/X86/funcimport2.ll

+; Do setup work for all below tests: generate bitcode and combined index
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/funcimport2.ll -o %t2.bc
+
+; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN:     -r=%t1.bc,_foo,plx \
+; RUN:     -r=%t2.bc,_main,plx \
+; RUN:     -r=%t2.bc,_foo,l
+; RUN: llvm-dis %t.o.1.3.import.bc -o - | FileCheck %s
+; CHECK: define available_externally void @foo()
+
+; We shouldn't do any importing at -O0
+; rm -f %t.o.1.3.import.bc
+; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN:     -O0 \
+; RUN:     -r=%t1.bc,_foo,plx \
+; RUN:     -r=%t2.bc,_main,plx \
+; RUN:     -r=%t2.bc,_foo,l
+; RUN: llvm-dis %t.o.1.3.import.bc -o - | FileCheck %s --check-prefix=CHECKO0
+; CHECKO0: declare void @foo(...)
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+define void @foo() #0 {
+entry:
+  ret void
+}

test/tools/gold/X86/Inputs/thinlto_funcimport.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @main() #0 {
+entry:
+  call void (...) @foo()
+  ret i32 0
+}
+
+declare void @foo(...) #1

test/tools/gold/X86/thinlto_funcimport.ll

+; Do setup work for all below tests: generate bitcode and combined index
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/thinlto_funcimport.ll -o %t2.bc
+
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:    --plugin-opt=save-temps \
+; RUN:    --plugin-opt=thinlto \
+; RUN:    -shared %t1.bc %t2.bc -o %t
+; RUN: llvm-dis %t2.bc.3.import.bc -o - | FileCheck %s
+; CHECK: define available_externally void @foo()
+
+; We shouldn't do any importing at -O0
+; rm -f %t2.bc.3.import.bc
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:    --plugin-opt=save-temps \
+; RUN:    --plugin-opt=thinlto \
+; RUN:    --plugin-opt=O0 \
+; RUN:    -shared %t1.bc %t2.bc -o %t
+; RUN: llvm-dis %t2.bc.3.import.bc -o - | FileCheck %s --check-prefix=CHECKO0
+; CHECKO0: declare void @foo(...)
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @foo() #0 {
+entry:
+  ret void
+}

tools/llvm-lto2/llvm-lto2.cpp

 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/Threading.h"
 
 using namespace llvm;
 using namespace lto;
 using namespace object;
 
+static cl::opt<char>
+    OptLevel("O", cl::desc("Optimization level. [-O0, -O1, -O2, or -O3] "
+                           "(default = '-O2')"),
+             cl::Prefix, cl::ZeroOrMore, cl::init('2'));
+
 static cl::list<std::string> InputFilenames(cl::Positional, cl::OneOrMore,
                                             cl::desc("<input bitcode files>"));
 
 static cl::opt<std::string> OutputFilename("o", cl::Required,
                                            cl::desc("Output filename"),
                                            cl::value_desc("filename"));
 
 static cl::opt<std::string> CacheDir("cache-dir", cl::desc("Cache Directory"),
int main(int argc, char **argv) {
   if (SaveTemps)
     check(Conf.addSaveTemps(OutputFilename + "."),
           "Config::addSaveTemps failed");
 
   // Run a custom pipeline, if asked for.
   Conf.OptPipeline = OptPipeline;
   Conf.AAPipeline = AAPipeline;
 
+  Conf.OptLevel = OptLevel - '0';
+
   ThinBackend Backend;
   if (ThinLTODistributedIndexes)
     Backend = createWriteIndexesThinBackend("", "", true, "");
   else
     Backend = createInProcessThinBackend(Threads);
   LTO Lto(std::move(Conf), std::move(Backend));
 
   bool HasErrors = false;