[NFC][clang] Refactor getCompilationPhases()+Types.def step 2.

[plotfi]

Summary

• Removing a few of the entries in the Flags for the Types.def table.
• Removing redundant parts of getCompilationPhases().

[compnerd]

This looks good to me generally. I don't fully understand the reason for u being kept, is that something you intend to clean up in a subsequent patch?

[plotfi]

In D65176#1598431, @compnerd wrote:

This looks good to me generally. I don't fully understand the reason for u being kept, is that something you intend to clean up in a subsequent patch?

I’d like to remove it but I don’t yet understand its purpose. Might deal with it in another patch along with ‘A’

[aaron.ballman]

clang/include/clang/Driver/Types.def

39–45 ↗

(On Diff #211390)

Why should we document the removed flags, since users cannot write them anyway?

[compnerd]

clang/include/clang/Driver/Types.def

39–45 ↗

(On Diff #211390)

Actually, that makes sense to me. The reasoning for it (and the key thing to note about the documentation) is that it helps downstream forks as it indicates how to migrate. Now, if you believe that this bit of functionality is unlikely to be used, thats a different story. But, I don't think that it hurts to have the documentation in the commit message instead.

[aaron.ballman]

LGTM!

clang/include/clang/Driver/Types.def

39–45 ↗

(On Diff #211390)

Okay, that makes sense to me. Thank you for the explanation!

[plotfi]

Removing 'A'

[plotfi]

clang/include/clang/Driver/Types.def

39–45 ↗

(On Diff #211390)

Yeah I just wanted a heads up for anyone downstream or anyone trying to update this table. Unfortunately I dont really understand the concept behind "user specified" types here. I assume it means things like clang -xc++ or something like that. I will move this comment into the commit message. Thanks!

cfe/trunk/include/clang/Driver/Types.def

 
 // The third value is that id of the type for preprocessed inputs of
 // this type, or INVALID if this type is not preprocessed.
 
 // The fourth value is the suffix to use when creating temporary files
 // of this type, or null if unspecified.
 
 // The fifth value is a string containing option flags. Valid values:
-//  a - The type should only be assembled.
-//  p - The type should only be precompiled.
 //  u - The type can be user specified (with -x).
-//  m - Precompiling this type produces a module file.
-//  A - The type's temporary suffix should be appended when generating
-//      outputs of this type.
 
 // The sixth value is a variadic list of phases for each type. Eventually the
 // options flag string will be replaced with this variadic list.
+// Most of the options in Flags have been removed in favor of subsuming their
+// meaning from the phases list.
 
 // C family source language (with and without preprocessing).
 TYPE("cpp-output",               PP_C,         INVALID,         "i",     "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("c",                        C,            PP_C,            "c",     "u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("cl",                       CL,           PP_C,            "cl",    "u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("cuda-cpp-output",          PP_CUDA,      INVALID,         "cui",   "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("cuda",                     CUDA,         PP_CUDA,         "cu",    "u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("cuda",                     CUDA_DEVICE,  PP_CUDA,         "cu",    "" , phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("hip-cpp-output",           PP_HIP,       INVALID,         "cui",   "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("hip",                      HIP,          PP_HIP,          "cu",    "u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("hip",                      HIP_DEVICE,   PP_HIP,          "cu",    "" , phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("objective-c-cpp-output",   PP_ObjC,      INVALID,         "mi",    "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("objc-cpp-output",          PP_ObjC_Alias, INVALID,        "mi",    "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("objective-c",              ObjC,         PP_ObjC,         "m",     "u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("c++-cpp-output",           PP_CXX,       INVALID,         "ii",    "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("c++",                      CXX,          PP_CXX,          "cpp",   "u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("objective-c++-cpp-output", PP_ObjCXX,    INVALID,         "mii",   "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("objc++-cpp-output",        PP_ObjCXX_Alias, INVALID,      "mii",   "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("objective-c++",            ObjCXX,       PP_ObjCXX,       "mm",    "u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("renderscript",             RenderScript, PP_C,            "rs",    "u", phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 
 // C family input files to precompile.
-TYPE("c-header-cpp-output",      PP_CHeader,   INVALID,         "i",     "p",  phases::Precompile)
-TYPE("c-header",                 CHeader,      PP_CHeader,      "h",     "pu", phases::Preprocess, phases::Precompile)
-TYPE("cl-header",                CLHeader,     PP_CHeader,      "h",     "pu", phases::Preprocess, phases::Precompile)
-TYPE("objective-c-header-cpp-output", PP_ObjCHeader, INVALID,   "mi",    "p",  phases::Precompile)
-TYPE("objective-c-header",       ObjCHeader,   PP_ObjCHeader,   "h",     "pu", phases::Preprocess, phases::Precompile)
-TYPE("c++-header-cpp-output",    PP_CXXHeader, INVALID,         "ii",    "p",  phases::Precompile)
-TYPE("c++-header",               CXXHeader,    PP_CXXHeader,    "hh",    "pu", phases::Preprocess, phases::Precompile)
-TYPE("objective-c++-header-cpp-output", PP_ObjCXXHeader, INVALID, "mii", "p",  phases::Precompile)
-TYPE("objective-c++-header",     ObjCXXHeader, PP_ObjCXXHeader, "h",     "pu", phases::Preprocess, phases::Precompile)
-TYPE("c++-module",               CXXModule,    PP_CXXModule,    "cppm",  "mu", phases::Preprocess, phases::Precompile, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("c++-module-cpp-output",    PP_CXXModule, INVALID,         "iim",   "m",  phases::Precompile, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
+TYPE("c-header-cpp-output",      PP_CHeader,   INVALID,         "i",     "",   phases::Precompile)
+TYPE("c-header",                 CHeader,      PP_CHeader,      "h",     "u",  phases::Preprocess, phases::Precompile)
+TYPE("cl-header",                CLHeader,     PP_CHeader,      "h",     "u",  phases::Preprocess, phases::Precompile)
+TYPE("objective-c-header-cpp-output", PP_ObjCHeader, INVALID,   "mi",    "",   phases::Precompile)
+TYPE("objective-c-header",       ObjCHeader,   PP_ObjCHeader,   "h",     "u",  phases::Preprocess, phases::Precompile)
+TYPE("c++-header-cpp-output",    PP_CXXHeader, INVALID,         "ii",    "",   phases::Precompile)
+TYPE("c++-header",               CXXHeader,    PP_CXXHeader,    "hh",    "u",  phases::Preprocess, phases::Precompile)
+TYPE("objective-c++-header-cpp-output", PP_ObjCXXHeader, INVALID, "mii", "",   phases::Precompile)
+TYPE("objective-c++-header",     ObjCXXHeader, PP_ObjCXXHeader, "h",     "u",  phases::Preprocess, phases::Precompile)
+TYPE("c++-module",               CXXModule,    PP_CXXModule,    "cppm",  "u",  phases::Preprocess, phases::Precompile, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
+TYPE("c++-module-cpp-output",    PP_CXXModule, INVALID,         "iim",   "",   phases::Precompile, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 
 // Other languages.
 TYPE("ada",                      Ada,          INVALID,         nullptr, "u",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("assembler",                PP_Asm,       INVALID,         "s",     "au", phases::Assemble, phases::Link)
-TYPE("assembler-with-cpp",       Asm,          PP_Asm,          "S",     "au", phases::Preprocess, phases::Assemble, phases::Link)
+TYPE("assembler",                PP_Asm,       INVALID,         "s",     "u",  phases::Assemble, phases::Link)
+TYPE("assembler-with-cpp",       Asm,          PP_Asm,          "S",     "u",  phases::Preprocess, phases::Assemble, phases::Link)
 TYPE("f95",                      PP_Fortran,   INVALID,         nullptr, "u",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("f95-cpp-input",            Fortran,      PP_Fortran,      nullptr, "u",  phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("java",                     Java,         INVALID,         nullptr, "u",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 
 // LLVM IR/LTO types. We define separate types for IR and LTO because LTO
 // outputs should use the standard suffixes.
 TYPE("ir",                       LLVM_IR,      INVALID,         "ll",    "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("ir",                       LLVM_BC,      INVALID,         "bc",    "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("lto-ir",                   LTO_IR,       INVALID,         "s",     "",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("lto-bc",                   LTO_BC,       INVALID,         "o",     "",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 
 // Misc.
 TYPE("ast",                      AST,          INVALID,         "ast",   "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("ifs",                      IFS,          INVALID,         "ifs",   "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("pcm",                      ModuleFile,   INVALID,         "pcm",   "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("plist",                    Plist,        INVALID,         "plist", "",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("rewritten-objc",           RewrittenObjC,INVALID,         "cpp",   "",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("rewritten-legacy-objc",    RewrittenLegacyObjC,INVALID,   "cpp",   "",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("remap",                    Remap,        INVALID,         "remap", "",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("precompiled-header",       PCH,          INVALID,         "gch",   "A", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
+TYPE("precompiled-header",       PCH,          INVALID,         "gch",   "",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("object",                   Object,       INVALID,         "o",     "",  phases::Link)
 TYPE("treelang",                 Treelang,     INVALID,         nullptr, "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("image",                    Image,        INVALID,         "out",   "",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("dSYM",                     dSYM,         INVALID,         "dSYM",  "A", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
+TYPE("dSYM",                     dSYM,         INVALID,         "dSYM",  "",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("dependencies",             Dependencies, INVALID,         "d",     "",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("cuda-fatbin",              CUDA_FATBIN,  INVALID,         "fatbin","A", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("hip-fatbin",               HIP_FATBIN,   INVALID,         "hipfb", "A", phases::Compile, phases::Backend, phases::Assemble, phases::Link)
+TYPE("cuda-fatbin",              CUDA_FATBIN,  INVALID,         "fatbin","",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
+TYPE("hip-fatbin",               HIP_FATBIN,   INVALID,         "hipfb", "",  phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("none",                     Nothing,      INVALID,         nullptr, "u", phases::Compile, phases::Backend, phases::Assemble, phases::Link)

cfe/trunk/lib/Driver/Types.cpp

static const TypeInfo &getInfo(unsigned id) {
   return TypeInfos[id - 1];
 }
 
 const char *types::getTypeName(ID Id) {
   return getInfo(Id).Name;
 }
 
 types::ID types::getPreprocessedType(ID Id) {
-  return getInfo(Id).PreprocessedType;
+  ID PPT = getInfo(Id).PreprocessedType;
+  assert((llvm::is_contained(getInfo(Id).Phases, phases::Preprocess) !=
+          (PPT == TY_INVALID)) &&
+         "Unexpected Preprocess Type.");
+  return PPT;
+}
+
+static bool isPrepeocessedModuleType(ID Id) {
+  return Id == TY_CXXModule || Id == TY_PP_CXXModule;
 }
 
 types::ID types::getPrecompiledType(ID Id) {
-  if (strchr(getInfo(Id).Flags, 'm'))
+  if (isPrepeocessedModuleType(Id))
     return TY_ModuleFile;
   if (onlyPrecompileType(Id))
     return TY_PCH;
   return TY_INVALID;
 }
 
 const char *types::getTypeTempSuffix(ID Id, bool CLMode) {
   if (CLMode) {
     switch (Id) {
     case TY_Object:
     case TY_LTO_BC:
       return "obj";
     case TY_Image:
       return "exe";
     case TY_PP_Asm:
       return "asm";
     default:
       break;
     }
   }
   return getInfo(Id).TempSuffix;
 }
 
 bool types::onlyAssembleType(ID Id) {
-  return strchr(getInfo(Id).Flags, 'a');
+  return llvm::is_contained(getInfo(Id).Phases, phases::Assemble) &&
+         !llvm::is_contained(getInfo(Id).Phases, phases::Compile) &&
+         !llvm::is_contained(getInfo(Id).Phases, phases::Backend);
 }
 
 bool types::onlyPrecompileType(ID Id) {
-  return strchr(getInfo(Id).Flags, 'p');
+  return llvm::is_contained(getInfo(Id).Phases, phases::Precompile) &&
+         !isPrepeocessedModuleType(Id);
 }
 
 bool types::canTypeBeUserSpecified(ID Id) {
   return strchr(getInfo(Id).Flags, 'u');
 }
 
 bool types::appendSuffixForType(ID Id) {
-  return strchr(getInfo(Id).Flags, 'A');
+  return Id == TY_PCH || Id == TY_dSYM || Id == TY_CUDA_FATBIN ||
+         Id == TY_HIP_FATBIN;
 }
 
 bool types::canLipoType(ID Id) {
   return (Id == TY_Nothing ||
           Id == TY_Image ||
           Id == TY_Object ||
           Id == TY_LTO_BC);
 }
types::ID types::lookupTypeForTypeSpecifier(const char *Name) {
 
   return TY_INVALID;
 }
 
 // FIXME: Why don't we just put this list in the defs file, eh.
 // 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) {
-  if (Id != TY_Object) {
-    if (getPreprocessedType(Id) != TY_INVALID) {
-      P.push_back(phases::Preprocess);
-    }
-
-    if (getPrecompiledType(Id) != TY_INVALID) {
-      P.push_back(phases::Precompile);
-    }
-
-    if (!onlyPrecompileType(Id)) {
-      if (!onlyAssembleType(Id)) {
-        P.push_back(phases::Compile);
-        P.push_back(phases::Backend);
-      }
-      P.push_back(phases::Assemble);
-    }
-  }
-
-  if (!onlyPrecompileType(Id)) {
-    P.push_back(phases::Link);
-  }
-
-  // Check that the static Phase list matches.
-  // TODO: These will be deleted.
-  const llvm::SmallVectorImpl<phases::ID> &Phases = getInfo(Id).Phases;
-  assert(Phases.size() == P.size() &&
-         std::equal(Phases.begin(), Phases.end(), P.begin()) &&
-         "Invalid phase or size");
-
-  // TODO: This function is still being used to assert that the phase list in
-  //       Types.def is correct. Everything above this comment will be removed
-  //       in a subsequent NFC commit.
-  P = Phases;
+  P = getInfo(Id).Phases;
   assert(0 < P.size() && "Not enough phases in list");
   assert(P.size() <= phases::MaxNumberOfPhases && "Too many phases in list");
 }
 
 ID types::lookupCXXTypeForCType(ID Id) {
   switch (Id) {
   default:
     return Id;