[NFC][clang] Adding argument based Phase list filtering to getComplicationPhases

[plotfi]

Summary

This patch removes usage of FinalPhase from anywhere outside of the scope where it is used to do argument handling.
It also adds argument based trimming of the Phase list pulled out of the Types.def table.

[aaron.ballman]

clang/lib/Driver/Driver.cpp

3220

Why do you need this compound statement?

clang/lib/Driver/Types.cpp

313–315

How about:

llvm::copy_if(PhaseList, std::back_inserter(P), [](phases::ID Phase) { return Phase <= phases.Preprocess; });

Similar below.

348–349

P = PhaseList; ? Or does P come in with data already in it, in which case, llvm::copy() is a better choice.

[plotfi]

clang/lib/Driver/Driver.cpp

3220

I think the diff I posted here was a little bit off. I will update.

[plotfi]

clang/lib/Driver/Types.cpp

313–315

That could work. I was using llvm::remove_if before and ran into some problematic behavior.

[plotfi]

using llvm::copy

[plotfi]

clang/lib/Driver/Driver.cpp

3219–3220

@aaron.ballman How's this look to you now? I have another NFC patch coming up that will move this entire block into its a separate function. I think finally this Drive::BuildActions code can have the code that handles and does things with various arguments separate from the actual job phase pipeline setup code.

[aaron.ballman]

clang/lib/Driver/Driver.cpp

3318

PL.empty() instead

clang/lib/Driver/Types.cpp

11

Can you sort these includes?

309

You can elide braces for all of the if statement bodies here.

[plotfi]

clang/lib/Driver/Types.cpp

11

sorted.

309

elided

[compnerd]

clang/include/clang/Driver/Types.h

107

This really makes things confusing, perhaps renaming getCompilationPhases to getCompletePhaseList or something might make it less confusing? Although, I suppose that you do have follow up patches to improve this.

[plotfi]

clang/include/clang/Driver/Types.h

107

I'm mainly going by the original name. But I can certainly do this in a followup patch. Actually I'd prefer to not do it here, and address this in a patch where I remove the other getFinalPhase method from the Driver class.

[plotfi]

@aaron.ballman How does this look to you?

[aaron.ballman]

LGTM!

clang/include/clang/Driver/Types.h

 //===--- Types.h - Input & Temporary Driver Types ---------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_DRIVER_TYPES_H
 #define LLVM_CLANG_DRIVER_TYPES_H
 
 #include "clang/Driver/Phases.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Option/ArgList.h"
 
 namespace llvm {
 class StringRef;
 }
 namespace clang {
 namespace driver {
+class Driver;
 namespace types {
   enum ID {
     TY_INVALID,
 #define TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, ...) TY_##ID,
 #include "clang/Driver/Types.def"
 #undef TYPE
     TY_LAST
   };
#undef TYPE
   /// specified type name.
   ID lookupTypeForTypeSpecifier(const char *Name);
 
   /// getCompilationPhases - Get the list of compilation phases ('Phases') to be
   /// done for type 'Id'.
   void getCompilationPhases(
     ID Id,
     llvm::SmallVectorImpl<phases::ID> &Phases);
+  void getCompilationPhases(const clang::driver::Driver &Driver,
+                            llvm::opt::DerivedArgList &DAL, ID Id,
+                            llvm::SmallVectorImpl<phases::ID> &Phases);
 
   /// lookupCXXTypeForCType - Lookup CXX input type that corresponds to given
   /// C type (used for clang++ emulation of g++ behaviour)
   ID lookupCXXTypeForCType(ID Id);
 
   /// Lookup header file input type that corresponds to given
   /// source file type (used for clang-cl emulation of \Yc).
   ID lookupHeaderTypeForSourceType(ID Id);
 
 } // end namespace types
 } // end namespace driver
 } // end namespace clang
 
 #endif

clang/lib/Driver/Driver.cpp

void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
                           const InputList &Inputs, ActionList &Actions) const {
   llvm::PrettyStackTraceString CrashInfo("Building compilation actions");
 
   if (!SuppressMissingInputWarning && Inputs.empty()) {
     Diag(clang::diag::err_drv_no_input_files);
     return;
   }
 
-  Arg *FinalPhaseArg;
-  phases::ID FinalPhase = getFinalPhase(Args, &FinalPhaseArg);
-
-  if (FinalPhase == phases::Link) {
-    if (Args.hasArg(options::OPT_emit_llvm))
-      Diag(clang::diag::err_drv_emit_llvm_link);
-    if (IsCLMode() && LTOMode != LTOK_None &&
-        !Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
-      Diag(clang::diag::err_drv_lto_without_lld);
-  }
-
   // Reject -Z* at the top level, these options should never have been exposed
   // by gcc.
   if (Arg *A = Args.getLastArg(options::OPT_Z_Joined))
     Diag(clang::diag::err_drv_use_of_Z_option) << A->getAsString(Args);
 
   // Diagnose misuse of /Fo.
   if (Arg *A = Args.getLastArg(options::OPT__SLASH_Fo)) {
     StringRef V = A->getValue();
if (YcArg && YuArg && strcmp(YcArg->getValue(), YuArg->getValue()) != 0) {
     Args.eraseArg(options::OPT__SLASH_Yu);
     YcArg = YuArg = nullptr;
   }
   if (YcArg && Inputs.size() > 1) {
     Diag(clang::diag::warn_drv_yc_multiple_inputs_clang_cl);
     Args.eraseArg(options::OPT__SLASH_Yc);
     YcArg = nullptr;
   }
-  if (FinalPhase == phases::Preprocess || Args.hasArg(options::OPT__SLASH_Y_)) {
+
+  // Builder to be used to build offloading actions.
+  OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
+
+  // Construct the actions to perform.
+  HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
+  ActionList LinkerInputs;
+
+  {
+    Arg *FinalPhaseArg;
+    phases::ID FinalPhase = getFinalPhase(Args, &FinalPhaseArg);
+
+    if (FinalPhase == phases::Link) {
+      if (Args.hasArg(options::OPT_emit_llvm))
+        Diag(clang::diag::err_drv_emit_llvm_link);
+      if (IsCLMode() && LTOMode != LTOK_None &&
+          !Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
+        Diag(clang::diag::err_drv_lto_without_lld);
+    }
+
+    if (FinalPhase == phases::Preprocess ||
+        Args.hasArg(options::OPT__SLASH_Y_)) {
       // If only preprocessing or /Y- is used, all pch handling is disabled.
       // Rather than check for it everywhere, just remove clang-cl pch-related
       // flags here.
       Args.eraseArg(options::OPT__SLASH_Fp);
       Args.eraseArg(options::OPT__SLASH_Yc);
       Args.eraseArg(options::OPT__SLASH_Yu);
       YcArg = YuArg = nullptr;
     }
 
-  // Builder to be used to build offloading actions.
-  OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
-
-  // Construct the actions to perform.
-  HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
-  ActionList LinkerInputs;
-
     unsigned LastPLSize = 0;
     for (auto &I : Inputs) {
       types::ID InputType = I.first;
       const Arg *InputArg = I.second;
 
       llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> PL;
       types::getCompilationPhases(InputType, PL);
       LastPLSize = PL.size();
 
       // If the first step comes after the final phase we are doing as part of
       // this compilation, warn the user about it.
       phases::ID InitialPhase = PL[0];
       if (InitialPhase > FinalPhase) {
         if (InputArg->isClaimed())
           continue;
 
         // Claim here to avoid the more general unused warning.
         InputArg->claim();
 
         // Suppress all unused style warnings with -Qunused-arguments
         if (Args.hasArg(options::OPT_Qunused_arguments))
           continue;
 
         // Special case when final phase determined by binary name, rather than
         // by a command-line argument with a corresponding Arg.
         if (CCCIsCPP())
           Diag(clang::diag::warn_drv_input_file_unused_by_cpp)
               << InputArg->getAsString(Args) << getPhaseName(InitialPhase);
         // Special case '-E' warning on a previously preprocessed file to make
         // more sense.
         else if (InitialPhase == phases::Compile &&
-               FinalPhase == phases::Preprocess &&
+                 (Args.getLastArg(options::OPT__SLASH_EP,
+                                  options::OPT__SLASH_P) ||
+                  Args.getLastArg(options::OPT_E) ||
+                  Args.getLastArg(options::OPT_M, options::OPT_MM)) &&
                  getPreprocessedType(InputType) == types::TY_INVALID)
           Diag(clang::diag::warn_drv_preprocessed_input_file_unused)
               << InputArg->getAsString(Args) << !!FinalPhaseArg
               << (FinalPhaseArg ? FinalPhaseArg->getOption().getName() : "");
         else
           Diag(clang::diag::warn_drv_input_file_unused)
               << InputArg->getAsString(Args) << getPhaseName(InitialPhase)
               << !!FinalPhaseArg
               << (FinalPhaseArg ? FinalPhaseArg->getOption().getName() : "");
         continue;
       }
 
       if (YcArg) {
         // Add a separate precompile phase for the compile phase.
         if (FinalPhase >= phases::Compile) {
           const types::ID HeaderType = lookupHeaderTypeForSourceType(InputType);
           llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> PCHPL;
           types::getCompilationPhases(HeaderType, PCHPL);
           // Build the pipeline for the pch file.
-        Action *ClangClPch =
-            C.MakeAction<InputAction>(*InputArg, HeaderType);
+          Action *ClangClPch = C.MakeAction<InputAction>(*InputArg, HeaderType);
           for (phases::ID Phase : PCHPL)
             ClangClPch = ConstructPhaseAction(C, Args, Phase, ClangClPch);
           assert(ClangClPch);
           Actions.push_back(ClangClPch);
           // The driver currently exits after the first failed command.  This
           // relies on that behavior, to make sure if the pch generation fails,
           // the main compilation won't run.
           // FIXME: If the main compilation fails, the PCH generation should
           // probably not be considered successful either.
         }
       }
+    }
+
+    // If we are linking, claim any options which are obviously only used for
+    // compilation.
+    // FIXME: Understand why the last Phase List length is used here.
+    if (FinalPhase == phases::Link && LastPLSize == 1) {
+      Args.ClaimAllArgs(options::OPT_CompileOnly_Group);
+      Args.ClaimAllArgs(options::OPT_cl_compile_Group);
+    }
+  }
+
+  for (auto &I : Inputs) {
+    types::ID InputType = I.first;
+    const Arg *InputArg = I.second;
+
+    llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> PL;
+    types::getCompilationPhases(*this, Args, InputType, PL);
+    if (!PL.size())
+      continue;
+
+    llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> FullPL;
+    types::getCompilationPhases(InputType, FullPL);
 
     // Build the pipeline for this file.
     Action *Current = C.MakeAction<InputAction>(*InputArg, InputType);
 
     // Use the current host action in any of the offloading actions, if
     // required.
     if (OffloadBuilder.addHostDependenceToDeviceActions(Current, InputArg))
       break;
 
     for (phases::ID Phase : PL) {
 
-      // We are done if this step is past what the user requested.
-      if (Phase > FinalPhase)
-        break;
-
       // Add any offload action the host action depends on.
       Current = OffloadBuilder.addDeviceDependencesToHostAction(
-          Current, InputArg, Phase, FinalPhase, PL);
+          Current, InputArg, Phase, PL.back(), FullPL);
       if (!Current)
         break;
 
       // Queue linker inputs.
       if (Phase == phases::Link) {
         assert(Phase == PL.back() && "linking must be final compilation step.");
         LinkerInputs.push_back(Current);
         Current = nullptr;
void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
 
   // Add a link action if necessary.
   if (!LinkerInputs.empty()) {
     Action *LA = C.MakeAction<LinkJobAction>(LinkerInputs, types::TY_Image);
     LA = OffloadBuilder.processHostLinkAction(LA);
     Actions.push_back(LA);
   }
 
-  // If we are linking, claim any options which are obviously only used for
-  // compilation.
-  // FIXME: Understand why the last Phase List length is used here.
-  if (FinalPhase == phases::Link && LastPLSize == 1) {
-    Args.ClaimAllArgs(options::OPT_CompileOnly_Group);
-    Args.ClaimAllArgs(options::OPT_cl_compile_Group);
-  }
-
   // If --print-supported-cpus, -mcpu=? or -mtune=? is specified, build a custom
   // Compile phase that prints out supported cpu models and quits.
   if (Arg *A = Args.getLastArg(options::OPT_print_supported_cpus)) {
     // Use the -mcpu=? flag as the dummy input to cc1.
     Actions.clear();
     Action *InputAc = C.MakeAction<InputAction>(*A, types::TY_C);
     Actions.push_back(
         C.MakeAction<PrecompileJobAction>(InputAc, types::TY_Nothing));

clang/lib/Driver/Types.cpp

 //===--- Types.cpp - Driver input & temporary type information ------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/Driver/Driver.h"
+#include "clang/Driver/Options.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Types.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Option/Arg.h"
 #include <cassert>
 #include <cstring>
 
 using namespace clang::driver;
 using namespace clang::driver::types;
 
 struct TypeInfo {
   const char *Name;

 // FIXME: The list is now in Types.def but for now this function will verify
 //        the old behavior and a subsequent change will delete most of the body.
 void types::getCompilationPhases(ID Id, llvm::SmallVectorImpl<phases::ID> &P) {
   P = getInfo(Id).Phases;
   assert(0 < P.size() && "Not enough phases in list");
   assert(P.size() <= phases::MaxNumberOfPhases && "Too many phases in list");
 }
 
+void types::getCompilationPhases(const clang::driver::Driver &Driver,
+                                 llvm::opt::DerivedArgList &DAL, ID Id,
+                                 llvm::SmallVectorImpl<phases::ID> &P) {
+  llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> PhaseList;
+  types::getCompilationPhases(InputType, PL);
+
+  if (Driver->CCCIsCPP() || DAL.getLastArg(options::OPT_E) ||
+      DAL.getLastArg(options::OPT__SLASH_EP) ||
+      DAL.getLastArg(options::OPT_M, options::OPT_MM) ||
+      DAL.getLastArg(options::OPT__SLASH_P)) {
+    // Filter to compiler mode. When the compiler is run as a preprocessor then
+    // compilation is not an option.
+    // -S runs the compiler in Assembly listing mode.
+    for (auto Phase : PhaseList)
+      if (Phase <= phases::Preprocess)
+        P.push_back(Phase);
+  } else if (DAL.getLastArg(options::OPT__precompile)) {
+    // --precompile only runs up to precompilation.
+    // This is a clang extension and is not compatible with GCC.
+    for (auto Phase : PhaseList)
+      if (Phase <= phases::Precompile)
+        P.push_back(Phase);
+  } else if (DAL.getLastArg(options::OPT_fsyntax_only) ||
+             DAL.getLastArg(options::OPT_print_supported_cpus) ||
+             DAL.getLastArg(options::OPT_module_file_info) ||
+             DAL.getLastArg(options::OPT_verify_pch) ||
+             DAL.getLastArg(options::OPT_rewrite_objc) ||
+             DAL.getLastArg(options::OPT_rewrite_legacy_objc) ||
+             DAL.getLastArg(options::OPT__migrate) ||
+             DAL.getLastArg(options::OPT_emit_iterface_stubs) ||
+             DAL.getLastArg(options::OPT__analyze,
+                            options::OPT__analyze_auto) ||
+             DAL.getLastArg(options::OPT_emit_ast)) {
+    // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
+    for (auto Phase : PhaseList)
+      if (Phase <= phases::Compile)
+        P.push_back(Phase);
+  } else if (DAL.getLastArg(options::OPT_S) ||
+             DAL.getLastArg(options::OPT_emit_llvm)) {
+    for (auto Phase : PhaseList)
+      if (Phase <= phases::Backend)
+        P.push_back(Phase);
+  } else if (DAL.getLastArg(options::OPT_c)) {
+    for (auto Phase : PhaseList)
+      if (Phase <= phases::Assemble)
+        P.push_back(Phase);
+  } else {
+    // Generally means, do every phase until Link.
+    for (auto Phase : PhaseList)
+      P.push_back(Phase);
+  }
+  llvm_unreachable("getCompilationPhases(): Reached end of function.")
+}
+
 ID types::lookupCXXTypeForCType(ID Id) {
   switch (Id) {
   default:
     return Id;
 
   case types::TY_C:
     return types::TY_CXX;
   case types::TY_PP_C: