llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Michael Kruse (Meinersbur) <details> <summary>Changes</summary> The original #<!-- -->218802 was reverted in #<!-- -->225416 because it introduced $O(n^2)$ behavior in the number of driver inputs. Fix by storing the final phase in the Compilation object instead of recomputing it every time when needed. Also fixes a potential inconsistency in `BuildOffloadingActions` which does not have the full list of inputs available to accurately determine the final action. The alternative to adding members to the Compilation object would have been to pass 2 new arguments through a lot of calls. IMHO the Compilation should also store the input list, as it is specific to a compilation invocation. It already includes two lists (raw and translated) of arguments from which the inputs are derived, i.e. the input lists are already specific to the Compilation object. `getFinalPhase` has already a TODO that wants it to be folded into the more comprehensive clang/Driver/Types.h mechanism. This is out of scope of this PR. --- Full diff: https://github.com/llvm/llvm-project/pull/225678.diff 7 Files Affected: - (modified) clang/include/clang/Driver/Compilation.h (+17) - (modified) clang/include/clang/Driver/Driver.h (+6-1) - (modified) clang/include/clang/Driver/Types.h (+4-3) - (modified) clang/lib/Driver/Driver.cpp (+54-14) - (modified) clang/lib/Driver/Types.cpp (+4-2) - (added) clang/test/Driver/Inputs/object0.o () - (added) clang/test/Driver/pch-inputs.h (+46) ``````````diff diff --git a/clang/include/clang/Driver/Compilation.h b/clang/include/clang/Driver/Compilation.h index 825806b6cfe33..f98fc66b6cb1b 100644 --- a/clang/include/clang/Driver/Compilation.h +++ b/clang/include/clang/Driver/Compilation.h @@ -13,6 +13,7 @@ #include "clang/Basic/OffloadArch.h" #include "clang/Driver/Action.h" #include "clang/Driver/Job.h" +#include "clang/Driver/Phases.h" #include "clang/Driver/Util.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" @@ -68,6 +69,13 @@ class Compilation { /// own argument translation. llvm::opt::DerivedArgList *TranslatedArgs; + /// Which compilation phase is supposed to be the last job. + phases::ID FinalPhase; + + /// Which compiler argument determined what the \p FinalPhase should be (used + /// for diagnostics). + llvm::opt::Arg *FinalPhaseArg = nullptr; + /// The list of actions we've created via MakeAction. This is not accessible /// to consumers; it's here just to manage ownership. std::vector<std::unique_ptr<Action>> AllActions; @@ -201,6 +209,15 @@ class Compilation { llvm::opt::DerivedArgList &getArgs() { return *TranslatedArgs; } + void setFinalPhase(phases::ID FinalPhase, llvm::opt::Arg *FinalPhaseArg) { + this->FinalPhase = FinalPhase; + this->FinalPhaseArg = FinalPhaseArg; + } + + phases::ID getFinalPhase() const { return FinalPhase; } + + llvm::opt::Arg *getFinalPhaseArg() const { return FinalPhaseArg; } + ActionList &getActions() { return Actions; } const ActionList &getActions() const { return Actions; } diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index 15b6fdcb8a574..fa4eef66e3dea 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -332,15 +332,20 @@ class Driver { LLVM_PREFERRED_TYPE(bool) unsigned ProbePrecompiled : 1; -public: // getFinalPhase - Determine which compilation mode we are in and record // which option we used to determine the final phase. // TODO: Much of what getFinalPhase returns are not actually true compiler // modes. Fold this functionality into Types::getCompilationPhases and // handleArguments. phases::ID getFinalPhase(const llvm::opt::DerivedArgList &DAL, + llvm::ArrayRef<InputTy>, llvm::opt::Arg **FinalPhaseArg = nullptr) const; + /// Set the final phase in \p C based on compiler arguments, driver state, and + /// the \p Inputs to be processed. + void updateFinalPhase(Compilation &C, llvm::ArrayRef<InputTy> Inputs) const; + +public: llvm::Expected<std::unique_ptr<llvm::MemoryBuffer>> executeProgram(llvm::ArrayRef<llvm::StringRef> Args) const; diff --git a/clang/include/clang/Driver/Types.h b/clang/include/clang/Driver/Types.h index 9dd89e1904a4f..e4f3536ed1c6e 100644 --- a/clang/include/clang/Driver/Types.h +++ b/clang/include/clang/Driver/Types.h @@ -115,9 +115,10 @@ namespace types { /// done for type 'Id' up until including LastPhase. llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> getCompilationPhases(ID Id, phases::ID LastPhase = phases::IfsMerge); - llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> - getCompilationPhases(const clang::driver::Driver &Driver, - llvm::opt::DerivedArgList &DAL, ID Id); + llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> getCompilationPhases( + const clang::driver::Driver &Driver, llvm::opt::DerivedArgList &DAL, + llvm::ArrayRef<std::pair<ID, const llvm::opt::Arg *>> Inputs, ID Id, + phases::ID FinalPhase); /// lookupCXXTypeForCType - Lookup CXX input type that corresponds to given /// C type (used for clang++ emulation of g++ behaviour) diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 7649941a68b1c..cba622553a3f3 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -352,8 +352,10 @@ InputArgList Driver::ParseArgStrings(ArrayRef<const char *> ArgStrings, // Determine which compilation mode we are in. We look for options which // affect the phase, starting with the earliest phases, and record which -// option we used to determine the final phase. +// option we used to determine the final phase. In absence of any explicit +// action command line option, derive the compilation mode from the inputs. phases::ID Driver::getFinalPhase(const DerivedArgList &DAL, + llvm::ArrayRef<InputTy> Inputs, Arg **FinalPhaseArg) const { Arg *PhaseArg = nullptr; phases::ID FinalPhase; @@ -401,9 +403,33 @@ phases::ID Driver::getFinalPhase(const DerivedArgList &DAL, } else if ((PhaseArg = DAL.getLastArg(options::OPT_emit_interface_stubs))) { FinalPhase = phases::IfsMerge; - // Otherwise do everything. - } else - FinalPhase = phases::Link; + // Otherwise autodetect from last phase triggered by input file. + } else { + FinalPhase = phases::Preprocess; + bool AnyPhase = false; + for (auto &I : Inputs) { + types::ID InputType = I.first; + const Arg *InputArg = I.second; + + // Linker options should not trigger more phases. + if (InputArg->getOption().hasFlag(options::LinkerInput)) + continue; + + // Relies on the compilation phases being ordered. + auto PL = types::getCompilationPhases(InputType); + if (PL.empty()) + continue; + + phases::ID LastPL = PL.back(); + if (LastPL > FinalPhase) + FinalPhase = LastPL; + AnyPhase = true; + } + + // Fall back to "do everything" when consistency check fails. + if (!AnyPhase || FinalPhase > phases::Link) + FinalPhase = phases::Link; + } if (FinalPhaseArg) *FinalPhaseArg = PhaseArg; @@ -411,6 +437,13 @@ phases::ID Driver::getFinalPhase(const DerivedArgList &DAL, return FinalPhase; } +void Driver::updateFinalPhase(Compilation &C, + llvm::ArrayRef<InputTy> Inputs) const { + Arg *FinalPhaseArg = nullptr; + phases::ID FinalPhase = getFinalPhase(C.getArgs(), Inputs, &FinalPhaseArg); + C.setFinalPhase(FinalPhase, FinalPhaseArg); +} + llvm::Expected<std::unique_ptr<llvm::MemoryBuffer>> Driver::executeProgram(llvm::ArrayRef<llvm::StringRef> Args) const { llvm::SmallString<64> OutputFile; @@ -1847,13 +1880,17 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) { // Construct the list of inputs. InputList Inputs; BuildInputs(C->getDefaultToolChain(), *TranslatedArgs, Inputs); + updateFinalPhase(*C, Inputs); + phases::ID FinalPhase = C->getFinalPhase(); + if (HasConfigFileTail && Inputs.size()) { - Arg *FinalPhaseArg; - if (getFinalPhase(*TranslatedArgs, &FinalPhaseArg) == phases::Link) { + if (FinalPhase == phases::Link) { DerivedArgList TranslatedLinkerIns(*CfgOptionsTail); for (Arg *A : *CfgOptionsTail) TranslatedLinkerIns.append(A); BuildInputs(C->getDefaultToolChain(), TranslatedLinkerIns, Inputs); + updateFinalPhase(*C, Inputs); + FinalPhase = C->getFinalPhase(); } } @@ -2121,6 +2158,7 @@ void Driver::generateCompilationDiagnostics( // Construct the list of inputs. InputList Inputs; BuildInputs(C.getDefaultToolChain(), C.getArgs(), Inputs); + updateFinalPhase(C, Inputs); ArgStringList IRInputs; for (InputList::iterator it = Inputs.begin(), ie = Inputs.end(); it != ie;) { @@ -3433,8 +3471,8 @@ void Driver::handleArguments(Compilation &C, DerivedArgList &Args, YcArg = nullptr; } - Arg *FinalPhaseArg; - phases::ID FinalPhase = getFinalPhase(Args, &FinalPhaseArg); + phases::ID FinalPhase = C.getFinalPhase(); + llvm::opt::Arg *FinalPhaseArg = C.getFinalPhaseArg(); if (FinalPhase == phases::Link) { if (Args.hasArgNoClaim(options::OPT_hipstdpar)) { @@ -3532,7 +3570,7 @@ void Driver::handleArguments(Compilation &C, DerivedArgList &Args, Diag(clang::diag::warn_drv_input_file_unused) << InputArg->getAsString(Args) << getPhaseName(InitialPhase) << !!FinalPhaseArg - << (FinalPhaseArg ? FinalPhaseArg->getOption().getName() : ""); + << (FinalPhaseArg ? FinalPhaseArg->getSpelling() : ""); continue; } @@ -3609,12 +3647,12 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args, Args.ClaimAllArgs(options::OPT_no_offload_new_driver); Args.ClaimAllArgs(options::OPT_offload_new_driver); + phases::ID FinalPhase = C.getFinalPhase(); bool HIPRDCDeviceOnlyFatBin = C.isOffloadingHostKind(Action::OFK_HIP) && offloadDeviceOnly() && Args.hasArg(options::OPT_hip_link) && Args.hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc, false) && - getFinalPhase(Args) == phases::Link && - !Args.hasArg(options::OPT_emit_llvm) && + FinalPhase == phases::Link && !Args.hasArg(options::OPT_emit_llvm) && Args.hasFlag(options::OPT_gpu_bundle_output, options::OPT_no_gpu_bundle_output, true); @@ -3627,7 +3665,8 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args, types::ID InputType = I.first; const Arg *InputArg = I.second; - auto PL = types::getCompilationPhases(*this, Args, InputType); + auto PL = + types::getCompilationPhases(*this, Args, Inputs, InputType, FinalPhase); if (PL.empty()) continue; @@ -4125,7 +4164,7 @@ Driver::BuildOffloadingActions(Compilation &C, llvm::opt::DerivedArgList &Args, // Don't build offloading actions if we do not have a compile action. If // preprocessing only ignore embedding. if (!(isa<CompileJobAction>(HostAction) || - getFinalPhase(Args) == phases::Preprocess)) + C.getFinalPhase() == phases::Preprocess)) return HostAction; bool UsesLLVMOffloading = Args.hasArg( @@ -4178,7 +4217,8 @@ Driver::BuildOffloadingActions(Compilation &C, llvm::opt::DerivedArgList &Args, .isOSDarwin()) HostAction->setCannotBeCollapsedWithNextDependentAction(); - auto PL = types::getCompilationPhases(*this, Args, InputType); + auto PL = types::getCompilationPhases(*this, Args, {Input}, InputType, + C.getFinalPhase()); for (phases::ID Phase : PL) { if (Phase == phases::Link) { diff --git a/clang/lib/Driver/Types.cpp b/clang/lib/Driver/Types.cpp index 4cca8acd515d7..e04d3c9a05ace 100644 --- a/clang/lib/Driver/Types.cpp +++ b/clang/lib/Driver/Types.cpp @@ -429,8 +429,10 @@ types::getCompilationPhases(ID Id, phases::ID LastPhase) { llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> types::getCompilationPhases(const clang::driver::Driver &Driver, - llvm::opt::DerivedArgList &DAL, ID Id) { - return types::getCompilationPhases(Id, Driver.getFinalPhase(DAL)); + llvm::opt::DerivedArgList &DAL, + llvm::ArrayRef<InputTy> Inputs, ID Id, + phases::ID FinalPhase) { + return types::getCompilationPhases(Id, FinalPhase); } ID types::lookupCXXTypeForCType(ID Id) { diff --git a/clang/test/Driver/Inputs/object0.o b/clang/test/Driver/Inputs/object0.o new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/clang/test/Driver/pch-inputs.h b/clang/test/Driver/pch-inputs.h new file mode 100644 index 0000000000000..a1108e7bba40d --- /dev/null +++ b/clang/test/Driver/pch-inputs.h @@ -0,0 +1,46 @@ +// RUN: rm -rf %t +// RUN: mkdir %t + +// Warn about linker options being ignored when not linking +// RUN: %clang %s -lfoo -o %t/tmp1.pch -### 2>&1 | FileCheck %s --check-prefix=UNUSED-L,SINGLEHEADER +// RUN: %clang %s -x c++-header -lfoo -o %t/tmp1.pch -### 2>&1 | FileCheck %s --check-prefix=UNUSED-L,SINGLEHEADER +// UNUSED-L: clang: warning: -lfoo: 'linker' input unused when '' is present [-Wunused-command-line-argument] + +// RUN: %clang %s -Wl,--whole-archive -o %t/tmp1.pch -### 2>&1 | FileCheck %s --check-prefix=UNUSED-WL,SINGLEHEADER +// UNUSED-WL: clang: warning: -Wl,--whole-archive: 'linker' input unused when '' is present [-Wunused-command-line-argument] + +// RUN: %clang %S/Inputs/header1.h %S/Inputs/header2.h -lfoo -### 2>&1 | FileCheck %s --check-prefix=UNUSED-L,MULTIHEADER + + +// Error with single -o when there are multiple output files +// RUN: not %clang %S/Inputs/header1.h %S/Inputs/header2.h -lfoo -o %t/tmp2.pch -### 2>&1 | FileCheck %s --check-prefix=UNUSED-L,MULTIOUTPUT +// MULTIOUTPUT: clang: error: cannot specify -o when generating multiple output files + +// An actual linker input file (object0.o) triggers an error, not a warning +// RUN: not %clang %s %S/Inputs/object0.o -o %t/tmp3.pch -### 2>&1 | FileCheck %s --check-prefix=MULTIOUTPUT + +// Other input types that do not link +// RUN: %clang -x cl-header %s -Xlinker -somelinkerflag -### 2>&1 | FileCheck %s --check-prefix=UNUSED-XLINKER +// RUN: %clang -x objective-c++-header %s -Xlinker -somelinkerflag -### 2>&1 | FileCheck %s --check-prefix=UNUSED-XLINKER +// RUN: %clang -x hlsl %s -Xlinker -somelinkerflag -### 2>&1 | FileCheck %s --check-prefix=UNUSED-XLINKER +// UNUSED-XLINKER: clang: warning: -Xlinker -somelinkerflag: 'linker' input unused when '' is present [-Wunused-command-line-argument] + + +// Normal case: Single header file input compiles to .pch even without --precompile +// RUN: %clang %s -o %t/tmp1.pch -### 2>&1 | FileCheck %s --check-prefix=SINGLEHEADER +// SINGLEHEADER: "-cc1" +// SINGLEHEADER: "-emit-pch" +// SINGLEHEADER: "-o" +// SINGLEHEADER: tmp1.pch" + + +// Multiple header files input compiles to one .pch each even without --precompile +// RUN: %clang %S/Inputs/header1.h %S/Inputs/header2.h -### 2>&1 | FileCheck %s --check-prefix=MULTIHEADER +// MULTIHEADER: "-cc1" +// MULTIHEADER: "-emit-pch" +// MULTIHEADER: "-o" +// MULTIHEADER: header1.h.pch" +// MULTIHEADER: "-cc1" +// MULTIHEADER: "-emit-pch" +// MULTIHEADER: "-o" +// MULTIHEADER: header2.h.pch" `````````` </details> https://github.com/llvm/llvm-project/pull/225678 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
