Sketch support for generating CC1 command line from CompilerInvocation

[dang]

Summary

This is an add-on to the RFC at http://lists.llvm.org/pipermail/cfe-dev/2020-May/065421.html

This introduces optional additional information in Option records that can
specify a mapping between an option and a field of a class (used here for
CompilerInvocation).
Also it adds a GenerateCC1CommandLine method to Compiler invocation that uses
the TableGen definitions to generate command line argument strings.

[dang]

Rebase on top of llvm/master

[dang]

Upload the right patch

[dexonsmith]

llvm/utils/TableGen/OptParserEmitter.cpp

163

This whitespace fixup LGTM, but you should commit it separately as an NFC commit. I suggest doing that now-ish; no need to wait for the RFC.

[Bigcheese]

clang/include/clang/Frontend/CompilerInvocation.h

156

Is this really a should not, or is it must not?

clang/lib/Frontend/CompilerInvocation.cpp

3683–3689

This should probably go in a separate function as it will grow. I would recommend something like ParseSimpleArgs and call it right before ParseAnalyzerArgs .

3686

How would this handle other option classes? I think it would be good to include a few different types of options in the first patch.

[Bigcheese]

clang/lib/Frontend/CompilerInvocation.cpp

3916

It's a little sad that we need to allocation every string just because of the -. We definitely need to be able to allocate strings for options with data, but it would be good if we could just have the strings with - prefixed in the option table if that's reasonable to do.

[dang]

Address some code review comments and sketch support for options that take a single value.

[dang]

clang/include/clang/Frontend/CompilerInvocation.h

156

You're right updated the wording to be more accurate

clang/lib/Frontend/CompilerInvocation.cpp

3686

I updated the patch to handle options that take a value. This doesn't show how to handle everything yet (it is missing how to handle options that take multiple values notably) although there are not that many of them (a quick skim of the code seems to indicate there are less than 100)

[Bigcheese]

clang/include/clang/Frontend/CompilerInvocation.h

248

Is there a reason for this to be a member of CompilerInvocation? The rest of the argument parsing functions are static file local functions.

clang/lib/Frontend/CompilerInvocation.cpp

3682

Put formatting fixups in a separate commit.

clang/unittests/Frontend/CompilerInvocationTest.cpp

41

This reminded me that some of the -cc1 arguments are positional such as -xc++. I think we'll need custom handling for INPUT arguments, although we don't need them (or want them) for the modules use case. Do you have any thoughts on how to handle them? I don't think answering that should block the initial patch though.

llvm/include/llvm/Option/OptParser.td

152

I'm not sure Required is a great name here. I initially read that as the option was required, not that it should always be emitted.

166–170

I noticed that this isn't being used for the enum case you added (-mrelocation-model). Do you intend to use it? You should either use it in this patch or remove it until it actually is used.

[dang]

Address some code review feedback

[dang]

clang/include/clang/Frontend/CompilerInvocation.h

248

All the keypaths have to be "relative" to CompilerInvocation so that different options can be processed uniformly i.e. we don't want to add extra specialization to the table-gen file that indicates if an option is for the analyzer or for codegen e.t.c. Some of the members of CompilerInvocation that store options are private, which means that if parseSimpleArgs which precludes it from being an free function with internal linkage. Unless you think all the CompilerInvocation members should be public, but that is a different can of worms.

clang/unittests/Frontend/CompilerInvocationTest.cpp

41

-x flags are no INPUT arguments as in it can appear anywhere but will apply to the inputs. The only part of CompilerInvocation that uses positional arguments is the input file(s) which is quite small (21 LOC) so I am happy to leave it as is, having a few line to add it at the end of the command line is not a big problem IMO.
The thing I am more worried about is that the processing for some options depends on other options having already been processed, I was planning to reorder them accordingly in the td file although I am not a fan of doing this, I think it would be better to addd the information using table-gen's DAG support although that would complicated the table-gen backend.

llvm/include/llvm/Option/OptParser.td

166–170

Yeah, I forgot to remove it, since the normalizer scheme subsumes the functionality.

[dang]

Updating the patch with the correct merge base

[dang]

This is the good diff

[dexonsmith]

I have a couple of drive-by suggestions, up to @dang and @Bigcheese whether to incorporate them.

clang/lib/Frontend/CompilerInvocation.cpp

137–142

I wonder if it's worth creating a .def file for the driver enums, something like:

#ifdef HANDLE_RELOCATION_MODEL
HANDLE_RELOCATION_MODEL("static", llvm::Reloc::Static)
HANDLE_RELOCATION_MODEL("pic", llvm::Reloc::PIC_)
HANDLE_RELOCATION_MODEL("ropi", llvm::Reloc::ROPI)
HANDLE_RELOCATION_MODEL("rwpi", llvm::Reloc::RWPI)
HANDLE_RELOCATION_MODEL("ropi-rwpio", llvm::Reloc::ROPI_RWPI)
HANDLE_RELOCATION_MODEL("dynamic-no-pic", llvm::Reloc::DynamicNoPIC)
#undef HANDLE_RELOCATION_MODEL
#endif // HANDLE_RELOCATION_MODEL

#ifdef HANDLE_DEBUG_INFO_KIND
HANDLE_DEBUG_INFO_KIND("line-tables-only", codegenoptions::DebugLineTablesOnly)
HANDLE_DEBUG_INFO_KIND("line-directives-only", codegenoptions::DebugDirectivesOnly)
HANDLE_DEBUG_INFO_KIND("limited", codegenoptions::LimitedDebugInfo)
HANDLE_DEBUG_INFO_KIND("standalone", codegenoptions::FullDebugInfo)
#undef HANDLE_DEBUG_INFO_KIND
#endif // HANDLE_DEBUG_INFO_KIND

// ...

Then you can use HANDLE_RELOCATION_MODEL in both normalize and denormalize, rather than duplicating the table.

Maybe we can go even further. Can you expand the Values array from the tablegen to include this info? Or rejigger the help text to leverage HANDLE_RELOCATION_MODEL (maybe pass in ValuesDefine<HANDLE_RELOCATION_MODEL>)? The current patch adds a value table; my first suggestion leaves us even; but maybe we can get rid of one.

3647

Seems like Options.inc could provide this as a default definition, not sure if that seems error-prone?

3665–3667

I prefer the style where the .inc file is responsible for the #undef calls. It avoids having to duplicate it everywhere (and the risk of forgetting it). WDYT?

[dang]

clang/lib/Frontend/CompilerInvocation.cpp

137–142

I think this suggestion, I can definitely do at least this to generate the necessary switch statements. I don't see why this should be a separate .def file, I can generate this from the tablegen with an extra field named ValuesAssociatedDefinitions or something to that effect. I'll do that now.

3647

Not sure how much of this do you mean? OPTION_WITH_MARSHALLING is a convenience thing that forces users to opt into getting the marshalling definitions. I think it might be better to provide default empty definitions for OPTION_WITH_MARSHALLING_FLAG and friends to achieve a similar effect without needing OPTION_WITH_MARSHALLING. I you mean the actual definitions here they rely on the ArgList & to be named Args which might make it error prone to include these as defaults.

3665–3667

I prefer it too, but the current tablegen backend doesn't generate them for other macros it defines, so I wanted to make it consistent... I could change the existing backend to work that way but I would then need to track all the usages of this across all the llvm-projects which I don't fancy doing. Let me know if you thing it is fine to break with the existing style for this one.

[dexonsmith]

clang/lib/Frontend/CompilerInvocation.cpp

3647

Right, I just mean you shouldn't have to define OPTION_WITH_MARSHALLING here.

3665–3667

It's probably pretty easy to clean up the existing ones, just grep for #undef of any of the defined macros and delete the lines. But if you don't have time I'm fine with you leaving it as-is (i.e. it seems good to be consistent with the rest of the backend).

[dang]

This address a bunch of existing feedback and makes the TableGen definitions of the additional information significantly nicer to write.

[dexonsmith]

clang/include/clang/Driver/CC1Options.td

215–218

Maybe you can avoid boilerplate with something like this:

ValuesAssociatedDefinitionsScope<"llvm::Reloc">,
ValuesAssociatedDefinitions<["Static", "PIC_", "RPOI", "RWPI", "ROPI_RWPI", "DynamicNoPIC"]>,
MarshallingInfoString<"CodeGenOpts.RelocationModel", "PIC_">,

Note that I shortened the second argument to MarshallingInfoString as well.

@Bigcheese, WDYT?

Also wondering if ValuesAssociatedDefinitions could be shorter. Maybe NormalizedValues (and then NormalizedValuesScope)?

clang/lib/Frontend/CompilerInvocation.cpp

125–127

Should we drop parameter names for ArgList, Diags, and DefaultTriple?

150–158

This looks like boilerplate that's going to be repeated a lot. I'm curious how many of the enum options will end up like this.

If more than a few, maybe we can add a .inc file that includes these function definitions? Triggered by adding:

AutoNormalizeEnum<"llvm::Reloc::Model", "RelocationModel">,

or something (instead of Normalizer<> and Denormalizer<>).

[dang]

This addresses the usability concern with having to specify fully scoped definitions for normalized values in the TableGen files.

[dang]

Implemented a draft of normalizer generation for simple enum based options

[dexonsmith]

clang/lib/Frontend/CompilerInvocation.cpp

3916

I want to highlight @Bigcheese's comment again so it doesn't get lost. I think a reasonable name would be SPELLING.

3928

The denormalizer should take the StringAllocator as an argument, and only allocate when necessary.

llvm/utils/TableGen/OptParserEmitter.cpp

294

Since this is meant to be included exactly once (as opposed to being an x-macro), perhaps this should be split out into a separate .inc.

318–321

It seems unnecessary to generate macros here; you can just generate the code directly...

But looking more closely, I wonder if we really need this generated code at all.

Instead, can we create a table like this in Options.inc?

struct EnumValue {
  const char *Name;
  unsigned Value;
};

struct EnumValueTable {
  const EnumValue *Table;
  unsigned Size;
};

static const EnumValue RelocationModelTable[] = {
  {"static", static_cast<unsigned>(llvm::Reloc::Static)},
  {"pic", static_cast<unsigned>(llvm::Reloc::_PIC)},
  // ...
};

static const EnumValue SomeOtherTable[] = {
  {"name", static_cast<unsigned>(clang::SomeOtherEnumValue)},
  // ...
};

static const EnumValueTable *EnumValueTables[] = {
  {&RelocationModelTable, sizeof(RelocationModelTable) / sizeof(EnumValue)},
  {&SomeOtherTable, sizeof(SomeOtherTable) / sizeof(EnumValue)},
};
static const unsigned EnumValueTablesSize =
    sizeof(EnumValueTables) / sizeof(EnumValueTable);

We can then have this code directly in Options.cpp:

static llvm::Optional<unsigned> extractSimpleEnum(
    llvm::opt::OptSpecifier Opt, int TableIndex,
    const ArgList &ArgList, DiagnosticsEngine &Diags) {
  assert(TableIndex >= 0);
  assert(TableIndex < EnumValueTablesSize);
  const EnumValueTable &Table = *EnumValueTables[TableIndex];

  auto Arg = Args.getLastArg(Opt);
  if (!Arg)
    return None;

  StringRef ArgValue = Arg->getValue();
  for (int I = 0, E = Table.Size; I != E; ++I)
    if (ArgValue == Table.Table[I].Name)
      return Table.Table[I].Value;

  Diags.Report(diag::err_drv_invalid_value)
      << Arg->getAsString(ArgList) << ArgValue;
  return None;
}

and change the normalizer contract to:

• return an Optional,
• take an llvm::opt::OptSpecifier,
• take an index into a table (set to -1 if there's no relevant table), and
• be responsible for the boilerplate call to getLastArg.

Simple enums would have NORMALIZER hardcoded to extractSimpleEnum. The handler code that calls it needs new arguments TABLE_INDEX and TYPE but also looks simpler this way:

if (auto MaybeValue = NORMALIZER(Opt##ID, TABLE_INDEX, Args, Diags)) \
  this->KEYPATH = static_cast<TYPE>(*MaybeValue);                    \
else                                                                 \
  this->KEYPATH = DEFAULT_VALUE;

We could similarly have a single serializeSimpleEnum function in Options.cpp for a hardcoded DENORMALIZER for simple enums:

static const char *serializeSimpleEnum(int TableIndex, unsigned Value) {
  assert(TableIndex >= 0);
  assert(TableIndex < EnumValueTablesSize);
  const EnumValueTable &Table = *EnumValueTables[TableIndex];
  for (int I = 0, E = Table.Size; I != E; ++I)
    if (Value == Table.Table[I].Value)
      return Table.Table[I].Name;

  llvm::report_fatal_error("good error message");
}

[dang]

Implement constant table based marshaling for simple enum based options.

[dang]

clang/lib/Frontend/CompilerInvocation.cpp

3916

Yes I haven't forgotten just have not gotten around to it yet. I just want to do some form of suffix merging with the string the the OptTable receives

llvm/utils/TableGen/OptParserEmitter.cpp

318–321

I originally wanted to generate the normalizers without the table to allow and arbitrary code fragment to be used as the normalized value (one that is potentially not a constant evaluated expression) and just regenerating the macros was the path of least resistance at the time. Having had a closer look at the existing cases of enum based options there doesn't seem to be a need for this functionality so I went ahead and did this. Although I haven't split the backends yet.

[dang]

Implement suffix merging to avoid allocations for option spelling when generating the command line. This update also removes unnecessary allocation for std::string based options and for enum based options.

[dang]

Fixed a couple of bugs.

[dang]

Address the clang-tidy issues.

[dang]

Allocate string when denormalizing an option backed by an std::string.

[Bigcheese]

clang/include/clang/Frontend/CompilerInvocation.h

192

inserting

clang/lib/Frontend/CompilerInvocation.cpp

160–161

llvm::report_fatal_error is only used for code paths we actually expect to potentially be hit. For programming errors we instead use assert or llvm_unreachable. See https://llvm.org/docs/ProgrammersManual.html#error-handling .

llvm/utils/TableGen/OptParserEmitter.cpp

239–249

Just to verify, this doesn't change the output for options that don't have marshalling info, right? Just want to make sure this doesn't change anything for other users of libOption.

274–275

What happens if there are none?

[dang]

llvm/utils/TableGen/OptParserEmitter.cpp

239–249

Yes of course all the previous behavior is in the WriteOptRecordFields lambda capture. This adds an entirely new macro table and leaves the existing one untouched.

274–275

There is never going to be none as there currently already is an enum-based option that uses this setup, but it would still be nice to have a guard for that. Getting on it...

[dang]

llvm/utils/TableGen/OptParserEmitter.cpp

274–275

Actually this does not need any changes, it will correctly report 0 if there are no value tables.

[dang]

Address code review feedback.

[dang]

Add a KeyPathPrefix field to factor out common key path prefixes, for example all CodeGenOpts.

[Bigcheese]

LGTM with KeyPathPrefix moved to the patch that actually uses it.

clang/include/clang/Driver/CC1Options.td

 
 let Flags = [CC1Option, CC1AsOption, NoDriverOption] in {
 
 def target_cpu : Separate<["-"], "target-cpu">,
   HelpText<"Target a specific cpu type">;
 def target_feature : Separate<["-"], "target-feature">,
   HelpText<"Target specific attributes">;
 def triple : Separate<["-"], "triple">,
-  HelpText<"Specify target triple (e.g. i686-apple-darwin9)">;
+  HelpText<"Specify target triple (e.g. i686-apple-darwin9)">,
+  MarshallingInfo<MarshallingStringAlwaysEmit<"TargetOpts->Triple", "llvm::sys::getDefaultTargetTriple()", "normalizeTriple", "">>;
 def target_abi : Separate<["-"], "target-abi">,
   HelpText<"Target a particular ABI type">;
 def target_sdk_version_EQ : Joined<["-"], "target-sdk-version=">,
   HelpText<"The version of target SDK used for compilation">;
 
 }
 
 def target_linker_version : Separate<["-"], "target-linker-version">,

 def massembler_fatal_warnings : Flag<["-"], "massembler-fatal-warnings">,
   HelpText<"Make assembler warnings fatal">;
 def mrelax_relocations : Flag<["--"], "mrelax-relocations">,
     HelpText<"Use relaxable elf relocations">;
 def msave_temp_labels : Flag<["-"], "msave-temp-labels">,
   HelpText<"Save temporary labels in the symbol table. "
            "Note this may change .s semantics and shouldn't generally be used "
            "on compiler-generated code.">;
 def mrelocation_model : Separate<["-"], "mrelocation-model">,
-  HelpText<"The relocation model to use">, Values<"static,pic,ropi,rwpi,ropi-rwpi,dynamic-no-pic">;
+  HelpText<"The relocation model to use">, Values<"static,pic,ropi,rwpi,ropi-rwpi,dynamic-no-pic">,
+  MarshallingInfo<MarshallingString<"CodeGenOpts.RelocationModel", "llvm::Reloc::PIC_", "normalizeRelocationModel", "denormalizeRelocationModel">>;
 def fno_math_builtin : Flag<["-"], "fno-math-builtin">,
   HelpText<"Disable implicit builtin knowledge of math functions">;
 }
 
 def disable_llvm_verifier : Flag<["-"], "disable-llvm-verifier">,
   HelpText<"Don't run the LLVM IR verifier pass">;
 def disable_llvm_passes : Flag<["-"], "disable-llvm-passes">,
   HelpText<"Use together with -emit-llvm to get pristine LLVM IR from the "
            "frontend by not running any LLVM passes at all">;

clang/include/clang/Frontend/CompilerInvocation.h

public:
   /// \returns true on success.
   ///
   /// \returns false if an error was encountered while parsing the arguments
   /// and attempts to recover and continue parsing the rest of the arguments.
   /// The recovery is best-effort and only guarantees that \p Res will end up in
   /// one of the vaild-to-access (albeit arbitrary) states.
   ///
   /// \param [out] Res - The resulting invocation.
   /// \param [in] CommandLineArgs - Array of argument strings, this must not
   /// contain "-cc1".
   static bool CreateFromArgs(CompilerInvocation &Res,
                              ArrayRef<const char *> CommandLineArgs,
                              DiagnosticsEngine &Diags);
 
   /// Get the directory where the compiler headers
   /// reside, relative to the compiler binary (found by the passed in
   /// arguments).
public:
   /// Retrieve a module hash string that is suitable for uniquely
   /// identifying the conditions under which the module was built.
   std::string getModuleHash() const;
 
   /// Generate a cc1-compatible command line arguments from this instance.
   ///
   /// \param [out] Args - The generated arguments. Note that the caller is
   /// responsible for insersting the path to the clang executable and "-cc1" if
   /// desired.
   /// \param StringAllocator - A function that given a Twine can allocate
   /// storage for a given command line argument and return a pointer to the
   /// newly allocated string. The returned pointer is what gets appended to
   /// Args.
   void
   generateCC1CommandLine(llvm::SmallVectorImpl<const char *> &Args,
                          llvm::function_ref<const char *(const llvm::Twine &)>
                              StringAllocator) const;
PreprocessorOutputOptions &getPreprocessorOutputOpts() {
     return PreprocessorOutputOpts;
   }
 
   const PreprocessorOutputOptions &getPreprocessorOutputOpts() const {
     return PreprocessorOutputOpts;
   }
 
   /// @}
+
+private:
+  /// Parse options for flags that expose marshalling information in their
+  /// table-gen definition
+  ///
+  /// \param Args - The argument list containing the arguments to parse
+  /// \param Diags - The DiagnosticsEngine associated with CreateFromArgs
+  /// \returns - True if parsing was successful, false otherwise
+  bool parseSimpleArgs(const llvm::opt::ArgList &Args,
+                       DiagnosticsEngine &Diags);
 };
 
 IntrusiveRefCntPtr<llvm::vfs::FileSystem>
 createVFSFromCompilerInvocation(const CompilerInvocation &CI,
                                 DiagnosticsEngine &Diags);
 
 IntrusiveRefCntPtr<llvm::vfs::FileSystem> createVFSFromCompilerInvocation(
     const CompilerInvocation &CI, DiagnosticsEngine &Diags,
     IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS);
 
 } // namespace clang
 
 #endif // LLVM_CLANG_FRONTEND_COMPILERINVOCATION_H

clang/lib/Frontend/CompilerInvocation.cpp

: LangOpts(new LangOptions(*X.getLangOpts())),
       TargetOpts(new TargetOptions(X.getTargetOpts())),
       DiagnosticOpts(new DiagnosticOptions(X.getDiagnosticOpts())),
       HeaderSearchOpts(new HeaderSearchOptions(X.getHeaderSearchOpts())),
       PreprocessorOpts(new PreprocessorOptions(X.getPreprocessorOpts())) {}
 
 CompilerInvocationBase::~CompilerInvocationBase() = default;
 
 //===----------------------------------------------------------------------===//
+// Normalizers
+//===----------------------------------------------------------------------===//
+
+static std::string normalizeTriple(const Arg *Arg, const ArgList &ArgList,
+                                   DiagnosticsEngine &Diags,
+                                   StringRef DefaultTriple) {
+  return llvm::Triple::normalize(Arg->getValue());
+}
+
+static llvm::Reloc::Model
+normalizeRelocationModel(const Arg *Arg, const ArgList &ArgList,
+                         DiagnosticsEngine &Diags,
+                         llvm::Reloc::Model DefaultValue) {
+  StringRef ArgValue = Arg->getValue();
+  auto RM = llvm::StringSwitch<llvm::Optional<llvm::Reloc::Model>>(ArgValue)
+                .Case("static", llvm::Reloc::Static)
+                .Case("pic", llvm::Reloc::PIC_)
+                .Case("ropi", llvm::Reloc::ROPI)
+                .Case("rwpi", llvm::Reloc::RWPI)
+                .Case("ropi-rwpi", llvm::Reloc::ROPI_RWPI)
+                .Case("dynamic-no-pic", llvm::Reloc::DynamicNoPIC)
+                .Default(None);
+
+  if (RM.hasValue())
+    return *RM;
+
+  Diags.Report(diag::err_drv_invalid_value)
+      << Arg->getAsString(ArgList) << ArgValue;
+  return DefaultValue;
+}
+
+static const char *denormalizeRelocationModel(llvm::Reloc::Model RM) {
+  switch (RM) {
+  case llvm::Reloc::Static:
+    return "static";
+  case llvm::Reloc::PIC_:
+    return "pic";
+  case llvm::Reloc::DynamicNoPIC:
+    return "dynamic-no-pic";
+  case llvm::Reloc::ROPI:
+    return "ropi";
+  case llvm::Reloc::RWPI:
+    return "rwpi";
+  case llvm::Reloc::ROPI_RWPI:
+    return "ropi-rwpi";
+  }
+}
+
+//===----------------------------------------------------------------------===//
 // Deserialization (from args)
 //===----------------------------------------------------------------------===//
 
 static unsigned getOptimizationLevel(ArgList &Args, InputKind IK,
                                      DiagnosticsEngine &Diags) {
   unsigned DefaultOpt = llvm::CodeGenOpt::None;
   if (IK.getLanguage() == Language::OpenCL && !Args.hasArg(OPT_cl_opt_disable))
     DefaultOpt = llvm::CodeGenOpt::Default;
static bool ParseMigratorArgs(MigratorOptions &Opts, ArgList &Args) {
   return true;
 }
 
 static void ParseCommentArgs(CommentOptions &Opts, ArgList &Args) {
   Opts.BlockCommandNames = Args.getAllArgValues(OPT_fcomment_block_commands);
   Opts.ParseAllComments = Args.hasArg(OPT_fparse_all_comments);
 }
 
-static llvm::Reloc::Model getRelocModel(ArgList &Args,
-                                        DiagnosticsEngine &Diags) {
-  if (Arg *A = Args.getLastArg(OPT_mrelocation_model)) {
-    StringRef Value = A->getValue();
-    auto RM = llvm::StringSwitch<llvm::Optional<llvm::Reloc::Model>>(Value)
-                  .Case("static", llvm::Reloc::Static)
-                  .Case("pic", llvm::Reloc::PIC_)
-                  .Case("ropi", llvm::Reloc::ROPI)
-                  .Case("rwpi", llvm::Reloc::RWPI)
-                  .Case("ropi-rwpi", llvm::Reloc::ROPI_RWPI)
-                  .Case("dynamic-no-pic", llvm::Reloc::DynamicNoPIC)
-                  .Default(None);
-    if (RM.hasValue())
-      return *RM;
-    Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Value;
-  }
-  return llvm::Reloc::PIC_;
-}
-
 /// Create a new Regex instance out of the string value in \p RpassArg.
 /// It returns a pointer to the newly generated Regex instance.
 static std::shared_ptr<llvm::Regex>
 GenerateOptimizationRemarkRegex(DiagnosticsEngine &Diags, ArgList &Args,
                                 Arg *RpassArg) {
   StringRef Val = RpassArg->getValue();
   std::string RegexError;
   std::shared_ptr<llvm::Regex> Pattern = std::make_shared<llvm::Regex>(Val);
static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   Opts.SaveTempLabels = Args.hasArg(OPT_msave_temp_labels);
   Opts.NoDwarfDirectoryAsm = Args.hasArg(OPT_fno_dwarf_directory_asm);
   Opts.SoftFloat = Args.hasArg(OPT_msoft_float);
   Opts.StrictEnums = Args.hasArg(OPT_fstrict_enums);
   Opts.StrictReturn = !Args.hasArg(OPT_fno_strict_return);
   Opts.StrictVTablePointers = Args.hasArg(OPT_fstrict_vtable_pointers);
   Opts.ForceEmitVTables = Args.hasArg(OPT_fforce_emit_vtables);
   Opts.UnwindTables = Args.hasArg(OPT_munwind_tables);
-  Opts.RelocationModel = getRelocModel(Args, Diags);
   Opts.ThreadModel =
       std::string(Args.getLastArgValue(OPT_mthread_model, "posix"));
   if (Opts.ThreadModel != "posix" && Opts.ThreadModel != "single")
     Diags.Report(diag::err_drv_invalid_value)
         << Args.getLastArg(OPT_mthread_model)->getAsString(Args)
         << Opts.ThreadModel;
   Opts.TrapFuncName = std::string(Args.getLastArgValue(OPT_ftrap_function_EQ));
   Opts.UseInitArray = !Args.hasArg(OPT_fno_use_init_array);
if (Arg *A = Args.getLastArg(options::OPT_target_sdk_version_EQ)) {
     if (Version.tryParse(A->getValue()))
       Diags.Report(diag::err_drv_invalid_value)
           << A->getAsString(Args) << A->getValue();
     else
       Opts.SDKVersion = Version;
   }
 }
 
+bool CompilerInvocation::parseSimpleArgs(const ArgList &Args,
+                                         DiagnosticsEngine &Diags) {
+#define OPTION_WITH_MARSHALLING
+#define OPTION_WITH_MARSHALLING_FLAG(PREFIX_TYPE, NAME, ID, KIND, GROUP,       \
+                                     ALIAS, ALIASARGS, FLAGS, PARAM, HELPTEXT, \
+                                     METAVAR, VALUES, ALWAYS_EMIT, KEYPATH,    \
+                                     IS_POSITIVE, DEFAULT_VALUE)               \
+  this->KEYPATH = Args.hasArg(OPT_##ID) && IS_POSITIVE;
+
+#define OPTION_WITH_MARSHALLING_STRING(                                        \
+    PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,        \
+    HELPTEXT, METAVAR, VALUES, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,            \
+    NORMALIZER, DENORMALIZER)                                                  \
+  {                                                                            \
+    auto Arg = Args.getLastArg(OPT_##ID);                                      \
+    this->KEYPATH =                                                            \
+        !Arg ? DEFAULT_VALUE : NORMALIZER(Arg, Args, Diags, DEFAULT_VALUE);    \
+  }
+
+#include "clang/Driver/Options.inc"
+#undef OPTION_WITH_MARSHALLING_STRING
+#undef OPTION_WITH_MARSHALLING_FLAG
+#undef OPTION_WITH_MARSHALLING
+  return true;
+}
+
 bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res,
                                         ArrayRef<const char *> CommandLineArgs,
                                         DiagnosticsEngine &Diags) {
   bool Success = true;
 
   // Parse the arguments.
   const OptTable &Opts = getDriverOptTable();
   const unsigned IncludedFlagsBitmask = options::CC1Option;
   unsigned MissingArgIndex, MissingArgCount;
   InputArgList Args = Opts.ParseArgs(CommandLineArgs, MissingArgIndex,
                                      MissingArgCount, IncludedFlagsBitmask);
-
-#define OPTION_WITH_MARSHALLING(PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS,     \
-                                ALIASARGS, FLAGS, PARAM, HELPTEXT, METAVAR,    \
-                                VALUES, KEYPATH, IS_POSITIVE, DEFAULT_VALUE)   \
-  if (Option::KIND##Class == Option::FlagClass)                                \
-    Res.KEYPATH = Args.hasArg(OPT_##ID) && IS_POSITIVE;
-#include "clang/Driver/Options.inc"
-#undef OPTION_WITH_MARSHALLING
-
   LangOptions &LangOpts = *Res.getLangOpts();
 
   // Check for missing argument error.
   if (MissingArgCount) {
     Diags.Report(diag::err_drv_missing_argument)
         << Args.getArgString(MissingArgIndex) << MissingArgCount;
     Success = false;
   }
 
   // Issue errors on unknown arguments.
   for (const auto *A : Args.filtered(OPT_UNKNOWN)) {
     auto ArgString = A->getAsString(Args);
     std::string Nearest;
     if (Opts.findNearest(ArgString, Nearest, IncludedFlagsBitmask) > 1)
       Diags.Report(diag::err_drv_unknown_argument) << ArgString;
     else
       Diags.Report(diag::err_drv_unknown_argument_with_suggestion)
           << ArgString << Nearest;
     Success = false;
   }
 
+  Success &= Res.parseSimpleArgs(Args, Diags);
   Success &= ParseAnalyzerArgs(*Res.getAnalyzerOpts(), Args, Diags);
   Success &= ParseMigratorArgs(Res.getMigratorOpts(), Args);
   ParseDependencyOutputArgs(Res.getDependencyOutputOpts(), Args);
   if (!Res.getDependencyOutputOpts().OutputFile.empty() &&
       Res.getDependencyOutputOpts().Targets.empty()) {
     Diags.Report(diag::err_fe_dependency_file_requires_MT);
     Success = false;
   }
#include "clang/Basic/LangOptions.def"
   return llvm::APInt(64, code).toString(36, /*Signed=*/false);
 }
 
 void CompilerInvocation::generateCC1CommandLine(
     SmallVectorImpl<const char *> &Args,
     llvm::function_ref<const char *(const Twine &)> StringAllocator) const {
 #define PREFIX(PREFIX_TYPE, BRACED_INIT)                                       \
   const char *PREFIX_TYPE[4] = BRACED_INIT;
-#define OPTION_WITH_MARSHALLING(PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS,     \
-                                ALIASARGS, FLAGS, PARAM, HELPTEXT, METAVAR,    \
-                                VALUES, KEYPATH, IS_POSITIVE, DEFAULT_VALUE)   \
-  if (Option::KIND##Class == Option::FlagClass &&                              \
-      IS_POSITIVE != DEFAULT_VALUE && this->KEYPATH != DEFAULT_VALUE)          \
+
+#define OPTION_WITH_MARSHALLING
+#define OPTION_WITH_MARSHALLING_FLAG(PREFIX_TYPE, NAME, ID, KIND, GROUP,       \
+                                     ALIAS, ALIASARGS, FLAGS, PARAM, HELPTEXT, \
+                                     METAVAR, VALUES, ALWAYS_EMIT, KEYPATH,    \
+                                     IS_POSITIVE, DEFAULT_VALUE)               \
+  if (FLAGS & options::CC1Option && IS_POSITIVE != DEFAULT_VALUE &&            \
+      (this->KEYPATH != DEFAULT_VALUE || ALWAYS_EMIT))                         \
     Args.push_back(StringAllocator(Twine(PREFIX_TYPE[0]) + NAME));
+
+#define OPTION_WITH_MARSHALLING_STRING(                                        \
+    PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,        \
+    HELPTEXT, METAVAR, VALUES, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,            \
+    NORMALIZER, DENORMALIZER)                                                  \
+  if ((FLAGS & options::CC1Option) &&                                          \
+      (this->KEYPATH != DEFAULT_VALUE || ALWAYS_EMIT)) {                       \
+    if (Option::KIND##Class == Option::SeparateClass) {                        \
+      Args.push_back(StringAllocator(Twine(PREFIX_TYPE[0]) + NAME));           \
+      Args.push_back(StringAllocator(DENORMALIZER(this->KEYPATH)));            \
+    }                                                                          \
+  }
+
 #include "clang/Driver/Options.inc"
+#undef OPTION_WITH_MARSHALLING_STRING
+#undef OPTION_WITH_MARSHALLING_FLAG
 #undef OPTION_WITH_MARSHALLING
 #undef PREFIX
 }
 
 namespace clang {
 
 IntrusiveRefCntPtr<llvm::vfs::FileSystem>
 createVFSFromCompilerInvocation(const CompilerInvocation &CI,

clang/unittests/Frontend/CompilerInvocationTest.cpp

 //===- unittests/Frontend/CompilerInvocationTest.cpp - CI tests //---------===//
 //
 // 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/Frontend/CompilerInvocation.h"
+#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "llvm/Support/Host.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 using namespace clang;
 
+using ::testing::Contains;
+using ::testing::Each;
+using ::testing::StrEq;
+using ::testing::StrNe;
+
 namespace {
 
-TEST(CompilerInvocation, CanGenerateCC1CommandLine) {
+class CC1CommandLineGenerationTest : public ::testing::Test {
+public:
+  IntrusiveRefCntPtr<DiagnosticsEngine> Diags;
+  SmallVector<const char *, 32> GeneratedArgs;
+  SmallVector<std::string, 32> GeneratedArgsStorage;
+
+  const char *operator()(const Twine &Arg) {
+    return GeneratedArgsStorage.emplace_back(Arg.str()).c_str();
+  }
+
+  CC1CommandLineGenerationTest()
+      : Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions())) {}
+};
+
+TEST_F(CC1CommandLineGenerationTest, CanGenerateCC1CommandLineFlag) {
   const char *Args[] = {"clang", "-xc++", "-fmodules-strict-context-hash", "-"};
 
-  IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
-      CompilerInstance::createDiagnostics(new DiagnosticOptions());
+  CompilerInvocation CInvok;
+  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+
+  CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fmodules-strict-context-hash")));
+}
+
+TEST_F(CC1CommandLineGenerationTest, CanGenerateCC1CommandLineSeparate) {
+  const char *TripleCStr = "i686-apple-darwin9";
+  const char *Args[] = {"clang", "-xc++", "-triple", TripleCStr, "-"};
 
   CompilerInvocation CInvok;
   CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
 
-  SmallVector<const char *, 32> GeneratedArgs;
-  SmallVector<std::string, 32> GeneratedArgsStorage;
-  auto StringAlloc = [&GeneratedArgsStorage](const Twine &Arg) {
-    return GeneratedArgsStorage.emplace_back(Arg.str()).c_str();
-  };
+  CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq(TripleCStr)));
+}
+
+TEST_F(CC1CommandLineGenerationTest,
+       CanGenerateCC1CommandLineSeparateRequired) {
+  const char *TripleCStr =
+      llvm::Triple::normalize(llvm::sys::getDefaultTargetTriple()).c_str();
+  const char *Args[] = {"clang", "-xc++", "-triple", TripleCStr, "-"};
+
+  CompilerInvocation CInvok;
+  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+
+  CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+
+  // Triple should always be emitted even if it is the default
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq(TripleCStr)));
+}
+
+TEST_F(CC1CommandLineGenerationTest, CanGenerateCC1CommandLineSeparateEnum) {
+  const char *RelocationModelCStr = "static";
+  const char *Args[] = {"clang", "-xc++", "-mrelocation-model",
+                        RelocationModelCStr, "-"};
+
+  CompilerInvocation CInvok;
+  CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+
+  CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+
+  // Non default relocation model
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq(RelocationModelCStr)));
+  GeneratedArgs.clear();
+
+  RelocationModelCStr = "pic";
+  Args[3] = RelocationModelCStr;
 
-  CInvok.generateCC1CommandLine(GeneratedArgs, StringAlloc);
+  CompilerInvocation CInvok1;
+  CompilerInvocation::CreateFromArgs(CInvok1, Args, *Diags);
 
-  ASSERT_STREQ(GeneratedArgs[0], "-fmodules-strict-context-hash");
+  CInvok1.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Each(StrNe(RelocationModelCStr)));
 }
 
 } // anonymous namespace

llvm/include/llvm/Option/OptParser.td

class OptionGroup<string name> {
   string EnumName = ?; // Uses the def name if undefined.
   string Name = name;
   string HelpText = ?;
   OptionGroup Group = ?;
   list<OptionFlag> Flags = [];
 }
 
 // Add support for generating marshalling code
-
-class OptionMarshallingInfo<code keypath> {
+class OptionMarshallingInfo<string kind, code keypath> {
+  string Kind = kind;
+  bit ShouldAlwaysEmit = 0;
   code KeyPath = keypath;
   // Used by the Flag option kind.
   bit IsPositive = ?;
   code DefaultValue = ?;
-  list<code> EnumValues = ?;
+  code Normalizer = ?;
+  code Denormalizer = ?;
 }
 
 // Define the option class.
 
 class Option<list<string> prefixes, string name, OptionKind kind> {
   string EnumName = ?; // Uses the def name if undefined.
   list<string> Prefixes = prefixes;
   string Name = name;

 class Group<OptionGroup group> { OptionGroup Group = group; }
 class HelpText<string text> { string HelpText = text; }
 class MetaVarName<string name> { string MetaVarName = name; }
 class Values<string value> { string Values = value; }
 class ValuesCode<code valuecode> { code ValuesCode = valuecode; }
 
 class MarshallingInfo<OptionMarshallingInfo info> { OptionMarshallingInfo MarshallingInfo = info; }
 class MarshallingFlag<code keypath, bit ispositive, code defaultvalue>
-  : OptionMarshallingInfo<keypath> {
+  : OptionMarshallingInfo<"flag", keypath> {
   bit IsPositive = ispositive;
   code DefaultValue = defaultvalue;
 }
-class MarshallingString<code keypath, code defaultvalue>
-  : OptionMarshallingInfo<keypath> {
-  code DefaultValue = defaultvalue;
+class MarshallingFlagAlwaysEmit<code keypath, bit ispositive, code defaultvalue>
+  : MarshallingFlag<keypath, ispositive, defaultvalue> {
+  let ShouldAlwaysEmit = 1;
 }
-class MarshallingEnum<code keypath, code defaultvalue, list<code> enumvalues>
-  : OptionMarshallingInfo<keypath> {
+class MarshallingString<code keypath, code defaultvalue, code normalizer, code denormalizer>
+  : OptionMarshallingInfo<"string", keypath> {
   code DefaultValue = defaultvalue;
-  list<code>EnumValues = enumvalues;
+  code Normalizer = normalizer;
+  code Denormalizer = denormalizer;
+}
+class MarshallingStringAlwaysEmit<code keypath, code defaultvalue, code normalizer, code denormalizer>
+  : MarshallingString<keypath, defaultvalue, normalizer, denormalizer> {
+  let ShouldAlwaysEmit = 1;
 }
-
 // Predefined options.
 
 // FIXME: Have generator validate that these appear in correct position (and
 // aren't duplicated).
 def INPUT : Option<[], "<input>", KIND_INPUT>;
 def UNKNOWN : Option<[], "<unknown>", KIND_UNKNOWN>;

llvm/utils/TableGen/OptParserEmitter.cpp

 //===- OptParserEmitter.cpp - Table Driven Command Line Parsing -----------===//
 //
 // 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 "OptEmitter.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include <cctype>
 #include <cstring>
 #include <map>
 
 using namespace llvm;
 
 static const std::string getOptionName(const Record &R) {
   // Use the record name unless EnumName is defined.
   if (isa<UnsetInit>(R.getValueInit("EnumName")))
     return std::string(R.getName());
 
   return std::string(R.getValueAsString("EnumName"));
 }
 
 static raw_ostream &write_cstring(raw_ostream &OS, llvm::StringRef Str) {
   OS << '"';
   OS.write_escaped(Str);
   OS << '"';
   return OS;
 }
 
-static void emitMarshallingInfo(raw_ostream &OS, const Record &R) {
-  OS << R.getValueAsString("KeyPath");
-  OS << ", ";
-  if (!isa<UnsetInit>(R.getValueInit("IsPositive")))
-    OS << R.getValueAsBit("IsPositive");
-  else
-    OS << "INVALID";
+static void emitMarshallingInfoFlag(raw_ostream &OS, const Record *R) {
+  OS << R->getValueAsBit("IsPositive");
+  OS << ",";
+  OS << R->getValueAsString("DefaultValue");
+}
+
+static void emitMarshallingInfoString(raw_ostream &OS, const Record *R) {
+  OS << R->getValueAsString("DefaultValue");
   OS << ", ";
-  if (!isa<UnsetInit>(R.getValueInit("DefaultValue")))
-    OS << R.getValueAsString("DefaultValue");
-  else
-    OS << "INVALID";
+  OS << R->getValueAsString("Normalizer");
+  OS << ", ";
+  OS << R->getValueAsString("Denormalizer");
 }
 
 /// OptParserEmitter - This tablegen backend takes an input .td file
 /// describing a list of options and emits a data structure for parsing and
 /// working with those options when given an input command line.
 namespace llvm {
 void EmitOptParser(RecordKeeper &Records, raw_ostream &OS) {
   // Get the option groups and options.
auto WriteOptRecordFields = [&](raw_ostream &OS, const Record &R) {
     // The option prefix;
     std::vector<StringRef> prf = R.getValueAsListOfStrings("Prefixes");
     OS << Prefixes[PrefixKeyT(prf.begin(), prf.end())] << ", ";
 
     // The option string.
     write_cstring(OS, R.getValueAsString("Name"));
 
     // The option identifier name.
     OS << ", " << getOptionName(R);
 
     // The option kind.
     OS << ", " << R.getValueAsDef("Kind")->getValueAsString("Name");
 
     // The containing option group (if any).
     OS << ", ";
     const ListInit *GroupFlags = nullptr;
     if (const DefInit *DI = dyn_cast<DefInit>(R.getValueInit("Group"))) {
auto WriteOptRecordFields = [&](raw_ostream &OS, const Record &R) {
     // The option Values. Used for shell autocompletion.
     OS << ", ";
     if (!isa<UnsetInit>(R.getValueInit("Values")))
       write_cstring(OS, R.getValueAsString("Values"));
     else
       OS << "nullptr";
   };
 
   for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
     const Record &R = *Opts[i];
 
     // Start a single option entry.
     OS << "OPTION(";
     WriteOptRecordFields(OS, R);
     OS << ")\n";
   }
   OS << "#endif // OPTION\n";
 
   OS << "#ifdef OPTION_WITH_MARSHALLING\n";
-  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
-    const Record &R = *Opts[i];
+  for (unsigned I = 0, E = Opts.size(); I != E; ++I) {
+    const Record &R = *Opts[I];
 
     if (!isa<UnsetInit>(R.getValueInit("MarshallingInfo"))) {
-      OS << "OPTION_WITH_MARSHALLING(";
+      Record *MarshallingInfoRecord =
+          cast<DefInit>(R.getValueInit("MarshallingInfo"))->getDef();
+      StringRef KindStr = MarshallingInfoRecord->getValueAsString("Kind");
+      auto KindInfoPair =
+          StringSwitch<std::pair<
+              const char *, llvm::function_ref<void(raw_ostream &, Record *)>>>(
+              KindStr)
+              .Case("flag", std::make_pair("OPTION_WITH_MARSHALLING_FLAG",
+                                           &emitMarshallingInfoFlag))
+              .Case("string", std::make_pair("OPTION_WITH_MARSHALLING_STRING",
+                                             &emitMarshallingInfoString))
+              .Default(std::make_pair("", nullptr));
+      OS << KindInfoPair.first << "(";
       WriteOptRecordFields(OS, R);
       OS << ", ";
-      emitMarshallingInfo(
-          OS, *cast<DefInit>(R.getValueInit("MarshallingInfo"))->getDef());
+      OS << MarshallingInfoRecord->getValueAsBit("ShouldAlwaysEmit");
+      OS << ", ";
+      OS << MarshallingInfoRecord->getValueAsString("KeyPath");
+      OS << ", ";
+      KindInfoPair.second(OS, MarshallingInfoRecord);
       OS << ")\n";
     }
   }
   OS << "#endif // OPTION_WITH_MARSHALLING\n";
 
   OS << "\n";
   OS << "#ifdef OPTTABLE_ARG_INIT\n";
   OS << "//////////\n";
   OS << "// Option Values\n\n";
   for (unsigned I = 0, E = Opts.size(); I != E; ++I) {
     const Record &R = *Opts[I];
     if (isa<UnsetInit>(R.getValueInit("ValuesCode")))
       continue;
     OS << "{\n";
     OS << "bool ValuesWereAdded;\n";
     OS << R.getValueAsString("ValuesCode");
     OS << "\n";
     for (StringRef Prefix : R.getValueAsListOfStrings("Prefixes")) {
       OS << "ValuesWereAdded = Opt.addValues(";
       std::string S(Prefix);
       S += R.getValueAsString("Name");
       write_cstring(OS, S);
       OS << ", Values);\n";
       OS << "(void)ValuesWereAdded;\n";
       OS << "assert(ValuesWereAdded && \"Couldn't add values to "
             "OptTable!\");\n";
     }
     OS << "}\n";
   }
   OS << "\n";
   OS << "#endif // OPTTABLE_ARG_INIT\n";
 }
 } // end namespace llvm