[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 ↗

(On Diff #214294)

Why do you need this compound statement?

clang/lib/Driver/Types.cpp

313–315 ↗

(On Diff #214294)

How about:

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

Similar below.

348–349 ↗

(On Diff #214294)

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 ↗

(On Diff #214294)

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

[plotfi]

clang/lib/Driver/Types.cpp

313–315 ↗

(On Diff #214294)

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

3220 ↗

(On Diff #214408)

@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

3326 ↗

(On Diff #214408)

PL.empty() instead

clang/lib/Driver/Types.cpp

11 ↗

(On Diff #214408)

Can you sort these includes?

309 ↗

(On Diff #214408)

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

[plotfi]

clang/lib/Driver/Types.cpp

11 ↗

(On Diff #214408)

sorted.

309 ↗

(On Diff #214408)

elided

[compnerd]

clang/include/clang/Driver/Types.h

107 ↗

(On Diff #214541)

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 ↗

(On Diff #214541)

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!

cfe/trunk/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

cfe/trunk/lib/Driver/Driver.cpp

void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
 
   // Builder to be used to build offloading actions.
   OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
 
   // Construct the actions to perform.
   HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
   ActionList LinkerInputs;
 
-  phases::ID FinalPhase;
   {
     Arg *FinalPhaseArg;
-    FinalPhase = getFinalPhase(Args, &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);
     }
ActionList LinkerInputs;
     }
   }
 
   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);
-    if (PL[0] > FinalPhase)
+    types::getCompilationPhases(*this, Args, InputType, PL);
+    if (PL.empty())
       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;

cfe/trunk/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/Types.h"
+#include "clang/Driver/Driver.h"
+#include "clang/Driver/DriverDiagnostic.h"
+#include "clang/Driver/Options.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringSwitch.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(Id, PhaseList);
+
+  // 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.
+  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))
+    llvm::copy_if(PhaseList, std::back_inserter(P),
+                  [](phases::ID Phase) { return Phase <= phases::Preprocess; });
+
+  // --precompile only runs up to precompilation.
+  // This is a clang extension and is not compatible with GCC.
+  else if (DAL.getLastArg(options::OPT__precompile))
+    llvm::copy_if(PhaseList, std::back_inserter(P),
+                  [](phases::ID Phase) { return Phase <= phases::Precompile; });
+
+  // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
+  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))
+    llvm::copy_if(PhaseList, std::back_inserter(P),
+                  [](phases::ID Phase) { return Phase <= phases::Compile; });
+
+  else if (DAL.getLastArg(options::OPT_S) ||
+           DAL.getLastArg(options::OPT_emit_llvm))
+    llvm::copy_if(PhaseList, std::back_inserter(P),
+                  [](phases::ID Phase) { return Phase <= phases::Backend; });
+
+  else if (DAL.getLastArg(options::OPT_c))
+    llvm::copy_if(PhaseList, std::back_inserter(P),
+                  [](phases::ID Phase) { return Phase <= phases::Assemble; });
+
+  // Generally means, do every phase until Link.
+  else
+    P = PhaseList;
+}
+
 ID types::lookupCXXTypeForCType(ID Id) {
   switch (Id) {
   default:
     return Id;
 
   case types::TY_C:
     return types::TY_CXX;
   case types::TY_PP_C: