From 13609cbaa570a971c89e925587292e1e56433cb5 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Tue, 17 Apr 2018 16:39:25 +0000 Subject: [PATCH] [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds Summary: The clang driver option -save-temps was not passed to the LTO config, so when invoking the ThinLTO backends via clang during distributed builds there was no way to get LTO to save temp files. Getting this to work with ThinLTO distributed builds also required changing the driver to avoid a separate compile step to emit unoptimized bitcode when the input was already bitcode under -save-temps. Not only is this unnecessary in general, it is problematic for ThinLTO backends since the temporary bitcode file to the backend would not match the module path in the combined index, leading to incorrect ThinLTO backend index-based optimizations. Reviewers: pcc Subscribers: mehdi_amini, inglorion, eraman, cfe-commits Differential Revision: https://reviews.llvm.org/D45217 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@330194 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Driver/Options.td | 2 +- include/clang/Frontend/CodeGenOptions.h | 3 +++ lib/CodeGen/BackendUtil.cpp | 9 +++++++++ lib/Driver/Driver.cpp | 19 +++++++++++++++++-- lib/Driver/ToolChains/Clang.cpp | 3 +++ lib/Frontend/CompilerInvocation.cpp | 11 +++++++++-- test/CodeGen/thinlto_backend.ll | 12 ++++++++++-- test/Driver/thinlto_backend.c | 9 +++++++++ 8 files changed, 61 insertions(+), 7 deletions(-) diff --git a/include/clang/Driver/Options.td b/include/clang/Driver/Options.td index fa4777a144..5a3a7442ac 100644 --- a/include/clang/Driver/Options.td +++ b/include/clang/Driver/Options.td @@ -2275,7 +2275,7 @@ def fno_rtlib_add_rpath: Flag<["-"], "fno-rtlib-add-rpath">, Flags<[NoArgumentUn HelpText<"Do not add -rpath with architecture-specific resource directory to the linker flags">; def r : Flag<["-"], "r">, Flags<[LinkerInput,NoArgumentUnused]>, Group; -def save_temps_EQ : Joined<["-", "--"], "save-temps=">, Flags<[DriverOption]>, +def save_temps_EQ : Joined<["-", "--"], "save-temps=">, Flags<[CC1Option, DriverOption]>, HelpText<"Save intermediate compilation results.">; def save_temps : Flag<["-", "--"], "save-temps">, Flags<[DriverOption]>, Alias, AliasArgs<["cwd"]>, diff --git a/include/clang/Frontend/CodeGenOptions.h b/include/clang/Frontend/CodeGenOptions.h index 6ab64f6562..3307c50408 100644 --- a/include/clang/Frontend/CodeGenOptions.h +++ b/include/clang/Frontend/CodeGenOptions.h @@ -203,6 +203,9 @@ public: /// the summary and module symbol table (and not, e.g. any debug metadata). std::string ThinLinkBitcodeFile; + /// Prefix to use for -save-temps output. + std::string SaveTempsFilePrefix; + /// Name of file passed with -fcuda-include-gpubinary option to forward to /// CUDA runtime back-end for incorporating them into host-side object file. std::string CudaGpuBinaryFileName; diff --git a/lib/CodeGen/BackendUtil.cpp b/lib/CodeGen/BackendUtil.cpp index 67af3aeb94..cf0251b46e 100644 --- a/lib/CodeGen/BackendUtil.cpp +++ b/lib/CodeGen/BackendUtil.cpp @@ -1117,6 +1117,15 @@ static void runThinLTOBackend(ModuleSummaryIndex *CombinedIndex, Module *M, return llvm::make_unique(std::move(OS)); }; lto::Config Conf; + if (CGOpts.SaveTempsFilePrefix != "") { + if (Error E = Conf.addSaveTemps(CGOpts.SaveTempsFilePrefix + ".", + /* UseInputModulePath */ false)) { + handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) { + errs() << "Error setting up ThinLTO save-temps: " << EIB.message() + << '\n'; + }); + } + } Conf.CPU = TOpts.CPU; Conf.CodeModel = getCodeModel(CGOpts); Conf.MAttrs = TOpts.Features; diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp index e2c241143f..f83b319726 100644 --- a/lib/Driver/Driver.cpp +++ b/lib/Driver/Driver.cpp @@ -3379,19 +3379,34 @@ class ToolSelector final { const Tool *combineBackendCompile(ArrayRef ActionInfo, const ActionList *&Inputs, ActionList &CollapsedOffloadAction) { - if (ActionInfo.size() < 2 || !canCollapsePreprocessorAction()) + if (ActionInfo.size() < 2) return nullptr; auto *BJ = dyn_cast(ActionInfo[0].JA); auto *CJ = dyn_cast(ActionInfo[1].JA); if (!BJ || !CJ) return nullptr; + // Check if the initial input (to the compile job or its predessor if one + // exists) is LLVM bitcode. In that case, no preprocessor step is required + // and we can still collapse the compile and backend jobs when we have + // -save-temps. I.e. there is no need for a separate compile job just to + // emit unoptimized bitcode. + bool InputIsBitcode = true; + for (size_t i = 1; i < ActionInfo.size(); i++) + if (ActionInfo[i].JA->getType() != types::TY_LLVM_BC && + ActionInfo[i].JA->getType() != types::TY_LTO_BC) { + InputIsBitcode = false; + break; + } + if (!InputIsBitcode && !canCollapsePreprocessorAction()) + return nullptr; + // Get compiler tool. const Tool *T = TC.SelectTool(*CJ); if (!T) return nullptr; - if (T->canEmitIR() && (SaveTemps || EmbedBitcode)) + if (T->canEmitIR() && ((SaveTemps && !InputIsBitcode) || EmbedBitcode)) return nullptr; Inputs = &CJ->getInputs(); diff --git a/lib/Driver/ToolChains/Clang.cpp b/lib/Driver/ToolChains/Clang.cpp index f6c0061387..49a0c4cdc1 100644 --- a/lib/Driver/ToolChains/Clang.cpp +++ b/lib/Driver/ToolChains/Clang.cpp @@ -3267,6 +3267,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, Args.AddLastArg(CmdArgs, options::OPT_fthinlto_index_EQ); } + if (const Arg *A = Args.getLastArg(options::OPT_save_temps_EQ)) + Args.AddLastArg(CmdArgs, options::OPT_save_temps_EQ); + // Embed-bitcode option. if (C.getDriver().embedBitcodeInObject() && !C.getDriver().isUsingLTO() && (isa(JA) || isa(JA))) { diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index 147657f62f..3ff5fcd14a 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -507,7 +507,8 @@ static void setPGOUseInstrumentor(CodeGenOptions &Opts, static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK, DiagnosticsEngine &Diags, - const TargetOptions &TargetOpts) { + const TargetOptions &TargetOpts, + const FrontendOptions &FrontendOpts) { bool Success = true; llvm::Triple Triple = llvm::Triple(TargetOpts.Triple); @@ -758,6 +759,12 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK, << A->getAsString(Args) << "-x ir"; Opts.ThinLTOIndexFile = Args.getLastArgValue(OPT_fthinlto_index_EQ); } + if (Arg *A = Args.getLastArg(OPT_save_temps_EQ)) + Opts.SaveTempsFilePrefix = + llvm::StringSwitch(A->getValue()) + .Case("obj", FrontendOpts.OutputFile) + .Default(llvm::sys::path::filename(FrontendOpts.OutputFile).str()); + Opts.ThinLinkBitcodeFile = Args.getLastArgValue(OPT_fthin_link_bitcode_EQ); Opts.MSVolatile = Args.hasArg(OPT_fms_volatile); @@ -2927,7 +2934,7 @@ bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res, LangOpts.IsHeaderFile); ParseTargetArgs(Res.getTargetOpts(), Args, Diags); Success &= ParseCodeGenArgs(Res.getCodeGenOpts(), Args, DashX, Diags, - Res.getTargetOpts()); + Res.getTargetOpts(), Res.getFrontendOpts()); ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args, Res.getFileSystemOpts().WorkingDir); if (DashX.getFormat() == InputKind::Precompiled || diff --git a/test/CodeGen/thinlto_backend.ll b/test/CodeGen/thinlto_backend.ll index 40530ec536..5b1964ac7d 100644 --- a/test/CodeGen/thinlto_backend.ll +++ b/test/CodeGen/thinlto_backend.ll @@ -26,8 +26,16 @@ ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t4.o -x ir %t5.o -c -fthinlto-index=%t.thinlto.bc ; RUN: llvm-nm %t4.o | count 0 -; Ensure f2 was imported -; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc +; Ensure f2 was imported. Check for all 3 flavors of -save-temps[=cwd|obj]. +; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj +; RUN: llvm-dis %t1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s +; RUN: mkdir -p %T/dir1 +; RUN: (cd %T/dir1 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd) +; RUN: llvm-dis %T/dir1/*1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s +; RUN: mkdir -p %T/dir2 +; RUN: (cd %T/dir2 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps) +; RUN: llvm-dis %T/dir2/*1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s +; CHECK-IMPORT: define available_externally void @f2() ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s ; CHECK-OBJ: T f1 ; CHECK-OBJ-NOT: U f2 diff --git a/test/Driver/thinlto_backend.c b/test/Driver/thinlto_backend.c index 6def5821b6..b2b45f5708 100644 --- a/test/Driver/thinlto_backend.c +++ b/test/Driver/thinlto_backend.c @@ -5,6 +5,15 @@ // RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### 2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION // CHECK-THINLTOBE-ACTION: -fthinlto-index= +// -save-temps should be passed to cc1 +// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD +// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD +// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-OBJ +// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc +// CHECK-SAVE-TEMPS-CWD: -save-temps=cwd +// CHECK-SAVE-TEMPS-OBJ: -save-temps=obj +// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc + // Ensure clang driver gives the expected error for incorrect input type // RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 | FileCheck %s -check-prefix=CHECK-WARNING // CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only allowed with '-x ir' -- 2.40.0