[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG29125ddf1323: Start adding support for generating CC1 
command lines from CompilerInvocation (authored by dang).

Changed prior to commit:
  https://reviews.llvm.org/D79796?vs=272416=273091#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -10,11 +10,13 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
@@ -33,6 +35,210 @@
   return OS;
 }
 
+static const std::string getOptionSpelling(const Record ,
+   size_t ) {
+  std::vector Prefixes = R.getValueAsListOfStrings("Prefixes");
+  StringRef Name = R.getValueAsString("Name");
+  if (Prefixes.empty()) {
+PrefixLength = 0;
+return Name.str();
+  }
+  PrefixLength = Prefixes[0].size();
+  return (Twine(Prefixes[0]) + Twine(Name)).str();
+}
+
+static const std::string getOptionSpelling(const Record ) {
+  size_t PrefixLength;
+  return getOptionSpelling(R, PrefixLength);
+}
+
+static void emitNameUsingSpelling(raw_ostream , const Record ) {
+  size_t PrefixLength;
+  OS << "&";
+  write_cstring(OS, StringRef(getOptionSpelling(R, PrefixLength)));
+  OS << "[" << PrefixLength << "]";
+}
+
+class MarshallingKindInfo {
+public:
+  const Record 
+  const char *MacroName;
+  bool ShouldAlwaysEmit;
+  StringRef KeyPath;
+  StringRef DefaultValue;
+  StringRef NormalizedValuesScope;
+
+  void emit(raw_ostream ) const {
+write_cstring(OS, StringRef(getOptionSpelling(R)));
+OS << ", ";
+OS << ShouldAlwaysEmit;
+OS << ", ";
+OS << KeyPath;
+OS << ", ";
+emitScopedNormalizedValue(OS, DefaultValue);
+OS << ", ";
+emitSpecific(OS);
+  }
+
+  virtual Optional emitValueTable(raw_ostream ) const {
+return None;
+  }
+
+  virtual ~MarshallingKindInfo() = default;
+
+  static std::unique_ptr create(const Record );
+
+protected:
+  void emitScopedNormalizedValue(raw_ostream ,
+ StringRef NormalizedValue) const {
+if (!NormalizedValuesScope.empty())
+  OS << NormalizedValuesScope << "::";
+OS << NormalizedValue;
+  }
+
+  virtual void emitSpecific(raw_ostream ) const = 0;
+  MarshallingKindInfo(const Record , const char *MacroName)
+  : R(R), MacroName(MacroName) {}
+};
+
+class MarshallingFlagInfo final : public MarshallingKindInfo {
+public:
+  bool IsPositive;
+
+  void emitSpecific(raw_ostream ) const override { OS << IsPositive; }
+
+  static std::unique_ptr create(const Record ) {
+std::unique_ptr Ret(new MarshallingFlagInfo(R));
+Ret->IsPositive = R.getValueAsBit("IsPositive");
+return Ret;
+  }
+
+private:
+  MarshallingFlagInfo(const Record )
+  : MarshallingKindInfo(R, "OPTION_WITH_MARSHALLING_FLAG") {}
+};
+
+class MarshallingStringInfo final : public MarshallingKindInfo {
+public:
+  StringRef NormalizerRetTy;
+  StringRef Normalizer;
+  StringRef Denormalizer;
+  int TableIndex = -1;
+  std::vector Values;
+  std::vector NormalizedValues;
+  std::string ValueTableName;
+
+  static constexpr const char *ValueTablePreamble = R"(
+struct SimpleEnumValue {
+  const char *Name;
+  unsigned Value;
+};
+
+struct SimpleEnumValueTable {
+  const SimpleEnumValue *Table;
+  unsigned Size;
+};
+)";
+
+  static constexpr const char *ValueTablesDecl =
+  "static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
+
+  void emitSpecific(raw_ostream ) const override {
+emitScopedNormalizedValue(OS, NormalizerRetTy);
+OS << ", ";
+OS << Normalizer;
+OS << ", ";
+OS << Denormalizer;
+OS << ", ";
+OS << TableIndex;
+  }
+
+  Optional emitValueTable(raw_ostream ) const override {
+if (TableIndex == -1)
+  return {};
+OS << "static const SimpleEnumValue " << ValueTableName << "[] = {\n";
+for (unsigned I = 0, E = Values.size(); I != E; ++I) {
+  OS << "{";
+  write_cstring(OS, Values[I]);
+  OS << ",";
+  OS << "static_cast(";
+  emitScopedNormalizedValue(OS, NormalizedValues[I]);
+  OS << ")},";
+}
+OS << "};\n";
+return StringRef(ValueTableName);
+  }
+
+  static std::unique_ptr create(const Record ) {
+

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 272416.
dang added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -10,11 +10,13 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
@@ -33,6 +35,212 @@
   return OS;
 }
 
+static const std::string getOptionSpelling(const Record ,
+   size_t ) {
+  std::vector Prefixes = R.getValueAsListOfStrings("Prefixes");
+  StringRef Name = R.getValueAsString("Name");
+  if (Prefixes.empty()) {
+PrefixLength = 0;
+return Name.str();
+  }
+  PrefixLength = Prefixes[0].size();
+  return (Twine(Prefixes[0]) + Twine(Name)).str();
+}
+
+static const std::string getOptionSpelling(const Record ) {
+  size_t PrefixLength;
+  return getOptionSpelling(R, PrefixLength);
+}
+
+static void emitNameUsingSpelling(raw_ostream , const Record ) {
+  size_t PrefixLength;
+  OS << "&";
+  write_cstring(OS, StringRef(getOptionSpelling(R, PrefixLength)));
+  OS << "[" << PrefixLength << "]";
+}
+
+class MarshallingKindInfo {
+public:
+  const Record 
+  const char *MacroName;
+  bool ShouldAlwaysEmit;
+  StringRef KeyPathPrefix;
+  StringRef KeyPath;
+  StringRef DefaultValue;
+  StringRef NormalizedValuesScope;
+
+  void emit(raw_ostream ) const {
+write_cstring(OS, StringRef(getOptionSpelling(R)));
+OS << ", ";
+OS << ShouldAlwaysEmit;
+OS << ", ";
+OS << KeyPathPrefix << KeyPath;
+OS << ", ";
+emitScopedNormalizedValue(OS, DefaultValue);
+OS << ", ";
+emitSpecific(OS);
+  }
+
+  virtual Optional emitValueTable(raw_ostream ) const {
+return None;
+  }
+
+  virtual ~MarshallingKindInfo() = default;
+
+  static std::unique_ptr create(const Record );
+
+protected:
+  void emitScopedNormalizedValue(raw_ostream ,
+ StringRef NormalizedValue) const {
+if (!NormalizedValuesScope.empty())
+  OS << NormalizedValuesScope << "::";
+OS << NormalizedValue;
+  }
+
+  virtual void emitSpecific(raw_ostream ) const = 0;
+  MarshallingKindInfo(const Record , const char *MacroName)
+  : R(R), MacroName(MacroName) {}
+};
+
+class MarshallingFlagInfo final : public MarshallingKindInfo {
+public:
+  bool IsPositive;
+
+  void emitSpecific(raw_ostream ) const override { OS << IsPositive; }
+
+  static std::unique_ptr create(const Record ) {
+std::unique_ptr Ret(new MarshallingFlagInfo(R));
+Ret->IsPositive = R.getValueAsBit("IsPositive");
+return Ret;
+  }
+
+private:
+  MarshallingFlagInfo(const Record )
+  : MarshallingKindInfo(R, "OPTION_WITH_MARSHALLING_FLAG") {}
+};
+
+class MarshallingStringInfo final : public MarshallingKindInfo {
+public:
+  StringRef NormalizerRetTy;
+  StringRef Normalizer;
+  StringRef Denormalizer;
+  int TableIndex = -1;
+  std::vector Values;
+  std::vector NormalizedValues;
+  std::string ValueTableName;
+
+  static constexpr const char *ValueTablePreamble = R"(
+struct SimpleEnumValue {
+  const char *Name;
+  unsigned Value;
+};
+
+struct SimpleEnumValueTable {
+  const SimpleEnumValue *Table;
+  unsigned Size;
+};
+)";
+
+  static constexpr const char *ValueTablesDecl =
+  "static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
+
+  void emitSpecific(raw_ostream ) const override {
+emitScopedNormalizedValue(OS, NormalizerRetTy);
+OS << ", ";
+OS << Normalizer;
+OS << ", ";
+OS << Denormalizer;
+OS << ", ";
+OS << TableIndex;
+  }
+
+  Optional emitValueTable(raw_ostream ) const override {
+if (TableIndex == -1)
+  return {};
+OS << "static const SimpleEnumValue " << ValueTableName << "[] = {\n";
+for (unsigned I = 0, E = Values.size(); I != E; ++I) {
+  OS << "{";
+  write_cstring(OS, Values[I]);
+  OS << ",";
+  OS << "static_cast(";
+  emitScopedNormalizedValue(OS, NormalizedValues[I]);
+  OS << ")},";
+}
+OS << "};\n";
+return StringRef(ValueTableName);
+  }
+
+  static std::unique_ptr create(const Record ) {
+assert(!isa(R.getValueInit("NormalizerRetTy")) &&
+   "String options 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 272337.
dang added a comment.

Address code review feedback.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -10,11 +10,13 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
@@ -33,6 +35,210 @@
   return OS;
 }
 
+static const std::string getOptionSpelling(const Record ,
+   size_t ) {
+  std::vector Prefixes = R.getValueAsListOfStrings("Prefixes");
+  StringRef Name = R.getValueAsString("Name");
+  if (Prefixes.empty()) {
+PrefixLength = 0;
+return Name.str();
+  }
+  PrefixLength = Prefixes[0].size();
+  return (Twine(Prefixes[0]) + Twine(Name)).str();
+}
+
+static const std::string getOptionSpelling(const Record ) {
+  size_t PrefixLength;
+  return getOptionSpelling(R, PrefixLength);
+}
+
+static void emitNameUsingSpelling(raw_ostream , const Record ) {
+  size_t PrefixLength;
+  OS << "&";
+  write_cstring(OS, StringRef(getOptionSpelling(R, PrefixLength)));
+  OS << "[" << PrefixLength << "]";
+}
+
+class MarshallingKindInfo {
+public:
+  const Record 
+  const char *MacroName;
+  bool ShouldAlwaysEmit;
+  StringRef KeyPath;
+  StringRef DefaultValue;
+  StringRef NormalizedValuesScope;
+
+  void emit(raw_ostream ) const {
+write_cstring(OS, StringRef(getOptionSpelling(R)));
+OS << ", ";
+OS << ShouldAlwaysEmit;
+OS << ", ";
+OS << KeyPath;
+OS << ", ";
+emitScopedNormalizedValue(OS, DefaultValue);
+OS << ", ";
+emitSpecific(OS);
+  }
+
+  virtual Optional emitValueTable(raw_ostream ) const {
+return None;
+  }
+
+  virtual ~MarshallingKindInfo() = default;
+
+  static std::unique_ptr create(const Record );
+
+protected:
+  void emitScopedNormalizedValue(raw_ostream ,
+ StringRef NormalizedValue) const {
+if (!NormalizedValuesScope.empty())
+  OS << NormalizedValuesScope << "::";
+OS << NormalizedValue;
+  }
+
+  virtual void emitSpecific(raw_ostream ) const = 0;
+  MarshallingKindInfo(const Record , const char *MacroName)
+  : R(R), MacroName(MacroName) {}
+};
+
+class MarshallingFlagInfo final : public MarshallingKindInfo {
+public:
+  bool IsPositive;
+
+  void emitSpecific(raw_ostream ) const override { OS << IsPositive; }
+
+  static std::unique_ptr create(const Record ) {
+std::unique_ptr Ret(new MarshallingFlagInfo(R));
+Ret->IsPositive = R.getValueAsBit("IsPositive");
+return Ret;
+  }
+
+private:
+  MarshallingFlagInfo(const Record )
+  : MarshallingKindInfo(R, "OPTION_WITH_MARSHALLING_FLAG") {}
+};
+
+class MarshallingStringInfo final : public MarshallingKindInfo {
+public:
+  StringRef NormalizerRetTy;
+  StringRef Normalizer;
+  StringRef Denormalizer;
+  int TableIndex = -1;
+  std::vector Values;
+  std::vector NormalizedValues;
+  std::string ValueTableName;
+
+  static constexpr const char *ValueTablePreamble = R"(
+struct SimpleEnumValue {
+  const char *Name;
+  unsigned Value;
+};
+
+struct SimpleEnumValueTable {
+  const SimpleEnumValue *Table;
+  unsigned Size;
+};
+)";
+
+  static constexpr const char *ValueTablesDecl =
+  "static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
+
+  void emitSpecific(raw_ostream ) const override {
+emitScopedNormalizedValue(OS, NormalizerRetTy);
+OS << ", ";
+OS << Normalizer;
+OS << ", ";
+OS << Denormalizer;
+OS << ", ";
+OS << TableIndex;
+  }
+
+  Optional emitValueTable(raw_ostream ) const override {
+if (TableIndex == -1)
+  return {};
+OS << "static const SimpleEnumValue " << ValueTableName << "[] = {\n";
+for (unsigned I = 0, E = Values.size(); I != E; ++I) {
+  OS << "{";
+  write_cstring(OS, Values[I]);
+  OS << ",";
+  OS << "static_cast(";
+  emitScopedNormalizedValue(OS, NormalizedValues[I]);
+  OS << ")},";
+}
+OS << "};\n";
+return StringRef(ValueTableName);
+  }
+
+  static std::unique_ptr create(const Record ) {
+assert(!isa(R.getValueInit("NormalizerRetTy")) &&
+   "String options must have a type");
+
+std::unique_ptr Ret(new MarshallingStringInfo(R));
+Ret->NormalizerRetTy = 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang marked 3 inline comments as done.
dang added inline comments.



Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:430-440
+  std::vector> OptsWithMarshalling;
+  for (unsigned I = 0, E = Opts.size(); I != E; ++I) {
+const Record  = *Opts[I];
 
+// Start a single option entry.
+OS << "OPTION(";
+WriteOptRecordFields(OS, R);

Bigcheese wrote:
> 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.
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.



Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:468-469
+  OS << "};\n";
+  OS << "static const unsigned SimpleEnumValueTablesSize = "
+"sizeof(SimpleEnumValueTables) / sizeof(SimpleEnumValueTable);\n";
+

Bigcheese wrote:
> What happens if there are none?
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...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang marked 2 inline comments as done.
dang added inline comments.



Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:468-469
+  OS << "};\n";
+  OS << "static const unsigned SimpleEnumValueTablesSize = "
+"sizeof(SimpleEnumValueTables) / sizeof(SimpleEnumValueTable);\n";
+

dang wrote:
> Bigcheese wrote:
> > What happens if there are none?
> 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...
Actually this does not need any changes, it will correctly report 0 if there 
are no value tables.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added inline comments.



Comment at: clang/include/clang/Frontend/CompilerInvocation.h:193
+  /// \param [out] Args - The generated arguments. Note that the caller is
+  /// responsible for insersting the path to the clang executable and "-cc1" if
+  /// desired.

inserting



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:160-161
+
+  llvm::report_fatal_error("The simple enum value was not correctly defined in 
"
+   "the tablegen option description");
+}

`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 .



Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:430-440
+  std::vector> OptsWithMarshalling;
+  for (unsigned I = 0, E = Opts.size(); I != E; ++I) {
+const Record  = *Opts[I];
 
+// Start a single option entry.
+OS << "OPTION(";
+WriteOptRecordFields(OS, R);

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.



Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:468-469
+  OS << "};\n";
+  OS << "static const unsigned SimpleEnumValueTablesSize = "
+"sizeof(SimpleEnumValueTables) / sizeof(SimpleEnumValueTable);\n";
+

What happens if there are none?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 272125.
dang added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -10,11 +10,13 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
@@ -33,6 +35,210 @@
   return OS;
 }
 
+static const std::string getOptionSpelling(const Record ,
+   size_t ) {
+  std::vector Prefixes = R.getValueAsListOfStrings("Prefixes");
+  StringRef Name = R.getValueAsString("Name");
+  if (Prefixes.empty()) {
+PrefixLength = 0;
+return Name.str();
+  }
+  PrefixLength = Prefixes[0].size();
+  return (Twine(Prefixes[0]) + Twine(Name)).str();
+}
+
+static const std::string getOptionSpelling(const Record ) {
+  size_t PrefixLength;
+  return getOptionSpelling(R, PrefixLength);
+}
+
+static void emitNameUsingSpelling(raw_ostream , const Record ) {
+  size_t PrefixLength;
+  OS << "&";
+  write_cstring(OS, StringRef(getOptionSpelling(R, PrefixLength)));
+  OS << "[" << PrefixLength << "]";
+}
+
+class MarshallingKindInfo {
+public:
+  const Record 
+  const char *MacroName;
+  bool ShouldAlwaysEmit;
+  StringRef KeyPath;
+  StringRef DefaultValue;
+  StringRef NormalizedValuesScope;
+
+  void emit(raw_ostream ) const {
+write_cstring(OS, StringRef(getOptionSpelling(R)));
+OS << ", ";
+OS << ShouldAlwaysEmit;
+OS << ", ";
+OS << KeyPath;
+OS << ", ";
+emitScopedNormalizedValue(OS, DefaultValue);
+OS << ", ";
+emitSpecific(OS);
+  }
+
+  virtual Optional emitValueTable(raw_ostream ) const {
+return None;
+  }
+
+  virtual ~MarshallingKindInfo() = default;
+
+  static std::unique_ptr create(const Record );
+
+protected:
+  void emitScopedNormalizedValue(raw_ostream ,
+ StringRef NormalizedValue) const {
+if (!NormalizedValuesScope.empty())
+  OS << NormalizedValuesScope << "::";
+OS << NormalizedValue;
+  }
+
+  virtual void emitSpecific(raw_ostream ) const = 0;
+  MarshallingKindInfo(const Record , const char *MacroName)
+  : R(R), MacroName(MacroName) {}
+};
+
+class MarshallingFlagInfo final : public MarshallingKindInfo {
+public:
+  bool IsPositive;
+
+  void emitSpecific(raw_ostream ) const override { OS << IsPositive; }
+
+  static std::unique_ptr create(const Record ) {
+std::unique_ptr Ret(new MarshallingFlagInfo(R));
+Ret->IsPositive = R.getValueAsBit("IsPositive");
+return Ret;
+  }
+
+private:
+  MarshallingFlagInfo(const Record )
+  : MarshallingKindInfo(R, "OPTION_WITH_MARSHALLING_FLAG") {}
+};
+
+class MarshallingStringInfo final : public MarshallingKindInfo {
+public:
+  StringRef NormalizerRetTy;
+  StringRef Normalizer;
+  StringRef Denormalizer;
+  int TableIndex = -1;
+  std::vector Values;
+  std::vector NormalizedValues;
+  std::string ValueTableName;
+
+  static constexpr const char *ValueTablePreamble = R"(
+struct SimpleEnumValue {
+  const char *Name;
+  unsigned Value;
+};
+
+struct SimpleEnumValueTable {
+  const SimpleEnumValue *Table;
+  unsigned Size;
+};
+)";
+
+  static constexpr const char *ValueTablesDecl =
+  "static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
+
+  void emitSpecific(raw_ostream ) const override {
+emitScopedNormalizedValue(OS, NormalizerRetTy);
+OS << ", ";
+OS << Normalizer;
+OS << ", ";
+OS << Denormalizer;
+OS << ", ";
+OS << TableIndex;
+  }
+
+  Optional emitValueTable(raw_ostream ) const override {
+if (TableIndex == -1)
+  return {};
+OS << "static const SimpleEnumValue " << ValueTableName << "[] = {\n";
+for (unsigned I = 0, E = Values.size(); I != E; ++I) {
+  OS << "{";
+  write_cstring(OS, Values[I]);
+  OS << ",";
+  OS << "static_cast(";
+  emitScopedNormalizedValue(OS, NormalizedValues[I]);
+  OS << ")},";
+}
+OS << "};\n";
+return StringRef(ValueTableName);
+  }
+
+  static std::unique_ptr create(const Record ) {
+assert(!isa(R.getValueInit("NormalizerRetTy")) &&
+   "String options must have a type");
+
+std::unique_ptr Ret(new 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 271942.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -10,11 +10,13 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
@@ -33,6 +35,210 @@
   return OS;
 }
 
+static const std::string getOptionSpelling(const Record ,
+   size_t ) {
+  std::vector Prefixes = R.getValueAsListOfStrings("Prefixes");
+  StringRef Name = R.getValueAsString("Name");
+  if (Prefixes.empty()) {
+PrefixLength = 0;
+return Name.str();
+  }
+  PrefixLength = Prefixes[0].size();
+  return (Twine(Prefixes[0]) + Twine(Name)).str();
+}
+
+static const std::string getOptionSpelling(const Record ) {
+  size_t PrefixLength;
+  return getOptionSpelling(R, PrefixLength);
+}
+
+static void emitNameUsingSpelling(raw_ostream , const Record ) {
+  size_t PrefixLength;
+  OS << "&";
+  write_cstring(OS, StringRef(getOptionSpelling(R, PrefixLength)));
+  OS << "[" << PrefixLength << "]";
+}
+
+class MarshallingKindInfo {
+public:
+  const Record 
+  const char *MacroName;
+  bool ShouldAlwaysEmit;
+  StringRef KeyPath;
+  StringRef DefaultValue;
+  StringRef NormalizedValuesScope;
+
+  void emit(raw_ostream ) const {
+write_cstring(OS, StringRef(getOptionSpelling(R)));
+OS << ", ";
+OS << ShouldAlwaysEmit;
+OS << ", ";
+OS << KeyPath;
+OS << ", ";
+emitScopedNormalizedValue(OS, DefaultValue);
+OS << ", ";
+emitSpecific(OS);
+  }
+
+  virtual Optional emitValueTable(raw_ostream ) const {
+return None;
+  }
+
+  virtual ~MarshallingKindInfo() = default;
+
+  static std::unique_ptr create(const Record );
+
+protected:
+  void emitScopedNormalizedValue(raw_ostream ,
+ StringRef NormalizedValue) const {
+if (!NormalizedValuesScope.empty())
+  OS << NormalizedValuesScope << "::";
+OS << NormalizedValue;
+  }
+
+  virtual void emitSpecific(raw_ostream ) const = 0;
+  MarshallingKindInfo(const Record , const char *MacroName)
+  : R(R), MacroName(MacroName) {}
+};
+
+class MarshallingFlagInfo final : public MarshallingKindInfo {
+public:
+  bool IsPositive;
+
+  void emitSpecific(raw_ostream ) const override { OS << IsPositive; }
+
+  static std::unique_ptr create(const Record ) {
+std::unique_ptr Ret(new MarshallingFlagInfo(R));
+Ret->IsPositive = R.getValueAsBit("IsPositive");
+return Ret;
+  }
+
+private:
+  MarshallingFlagInfo(const Record )
+  : MarshallingKindInfo(R, "OPTION_WITH_MARSHALLING_FLAG") {}
+};
+
+class MarshallingStringInfo final : public MarshallingKindInfo {
+public:
+  StringRef NormalizerRetTy;
+  StringRef Normalizer;
+  StringRef Denormalizer;
+  int TableIndex = -1;
+  std::vector Values;
+  std::vector NormalizedValues;
+  std::string ValueTableName;
+
+  static constexpr const char *ValueTablePreamble = R"(
+struct SimpleEnumValue {
+  const char *Name;
+  unsigned Value;
+};
+
+struct SimpleEnumValueTable {
+  const SimpleEnumValue *Table;
+  unsigned Size;
+};
+)";
+
+  static constexpr const char *ValueTablesDecl =
+  "static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
+
+  void emitSpecific(raw_ostream ) const override {
+emitScopedNormalizedValue(OS, NormalizerRetTy);
+OS << ", ";
+OS << Normalizer;
+OS << ", ";
+OS << Denormalizer;
+OS << ", ";
+OS << TableIndex;
+  }
+
+  Optional emitValueTable(raw_ostream ) const override {
+if (TableIndex == -1)
+  return {};
+OS << "static const SimpleEnumValue " << ValueTableName << "[] = {\n";
+for (unsigned I = 0, E = Values.size(); I != E; ++I) {
+  OS << "{";
+  write_cstring(OS, Values[I]);
+  OS << ",";
+  OS << "static_cast(";
+  emitScopedNormalizedValue(OS, NormalizedValues[I]);
+  OS << ")},";
+}
+OS << "};\n";
+return StringRef(ValueTableName);
+  }
+
+  static std::unique_ptr create(const Record ) {
+assert(!isa(R.getValueInit("NormalizerRetTy")) &&
+   "String options must have a type");
+
+std::unique_ptr Ret(new MarshallingStringInfo(R));
+Ret->NormalizerRetTy = R.getValueAsString("NormalizerRetTy");
+
+Ret->Normalizer = 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 271790.
dang added a comment.

Address the clang-tidy issues.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -10,11 +10,13 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
@@ -33,6 +35,210 @@
   return OS;
 }
 
+static const std::string getOptionSpelling(const Record ,
+   size_t ) {
+  std::vector Prefixes = R.getValueAsListOfStrings("Prefixes");
+  StringRef Name = R.getValueAsString("Name");
+  if (Prefixes.empty()) {
+PrefixLength = 0;
+return Name.str();
+  }
+  PrefixLength = Prefixes[0].size();
+  return (Twine(Prefixes[0]) + Twine(Name)).str();
+}
+
+static const std::string getOptionSpelling(const Record ) {
+  size_t PrefixLength;
+  return getOptionSpelling(R, PrefixLength);
+}
+
+static void emitNameUsingSpelling(raw_ostream , const Record ) {
+  size_t PrefixLength;
+  OS << "&";
+  write_cstring(OS, StringRef(getOptionSpelling(R, PrefixLength)));
+  OS << "[" << PrefixLength << "]";
+}
+
+class MarshallingKindInfo {
+public:
+  const Record 
+  const char *MacroName;
+  bool ShouldAlwaysEmit;
+  StringRef KeyPath;
+  StringRef DefaultValue;
+  StringRef NormalizedValuesScope;
+
+  void emit(raw_ostream ) const {
+write_cstring(OS, StringRef(getOptionSpelling(R)));
+OS << ", ";
+OS << ShouldAlwaysEmit;
+OS << ", ";
+OS << KeyPath;
+OS << ", ";
+emitScopedNormalizedValue(OS, DefaultValue);
+OS << ", ";
+emitSpecific(OS);
+  }
+
+  virtual Optional emitValueTable(raw_ostream ) const {
+return None;
+  }
+
+  virtual ~MarshallingKindInfo() = default;
+
+  static std::unique_ptr create(const Record );
+
+protected:
+  void emitScopedNormalizedValue(raw_ostream ,
+ StringRef NormalizedValue) const {
+if (!NormalizedValuesScope.empty())
+  OS << NormalizedValuesScope << "::";
+OS << NormalizedValue;
+  }
+
+  virtual void emitSpecific(raw_ostream ) const = 0;
+  MarshallingKindInfo(const Record , const char *MacroName)
+  : R(R), MacroName(MacroName) {}
+};
+
+class MarshallingFlagInfo final : public MarshallingKindInfo {
+public:
+  bool IsPositive;
+
+  void emitSpecific(raw_ostream ) const override { OS << IsPositive; }
+
+  static std::unique_ptr create(const Record ) {
+std::unique_ptr Ret(new MarshallingFlagInfo(R));
+Ret->IsPositive = R.getValueAsBit("IsPositive");
+return Ret;
+  }
+
+private:
+  MarshallingFlagInfo(const Record )
+  : MarshallingKindInfo(R, "OPTION_WITH_MARSHALLING_FLAG") {}
+};
+
+class MarshallingStringInfo final : public MarshallingKindInfo {
+public:
+  StringRef NormalizerRetTy;
+  StringRef Normalizer;
+  StringRef Denormalizer;
+  int TableIndex = -1;
+  std::vector Values;
+  std::vector NormalizedValues;
+  std::string ValueTableName;
+
+  static constexpr const char *ValueTablePreamble = R"(
+struct SimpleEnumValue {
+  const char *Name;
+  unsigned Value;
+};
+
+struct SimpleEnumValueTable {
+  const SimpleEnumValue *Table;
+  unsigned Size;
+};
+)";
+
+  static constexpr const char *ValueTablesDecl =
+  "static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
+
+  void emitSpecific(raw_ostream ) const override {
+emitScopedNormalizedValue(OS, NormalizerRetTy);
+OS << ", ";
+OS << Normalizer;
+OS << ", ";
+OS << Denormalizer;
+OS << ", ";
+OS << TableIndex;
+  }
+
+  Optional emitValueTable(raw_ostream ) const override {
+if (TableIndex == -1)
+  return {};
+OS << "static const SimpleEnumValue " << ValueTableName << "[] = {\n";
+for (unsigned I = 0, E = Values.size(); I != E; ++I) {
+  OS << "{";
+  write_cstring(OS, Values[I]);
+  OS << ",";
+  OS << "static_cast(";
+  emitScopedNormalizedValue(OS, NormalizedValues[I]);
+  OS << ")},";
+}
+OS << "};\n";
+return StringRef(ValueTableName);
+  }
+
+  static std::unique_ptr create(const Record ) {
+assert(!isa(R.getValueInit("NormalizerRetTy")) &&
+   "String options must have a type");
+
+std::unique_ptr Ret(new MarshallingStringInfo(R));
+Ret->NormalizerRetTy = 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 271392.
dang added a comment.

Fixed a couple of bugs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -10,11 +10,13 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
@@ -33,6 +35,210 @@
   return OS;
 }
 
+static const std::string getOptionSpelling(const Record ,
+   size_t ) {
+  std::vector Prefixes = R.getValueAsListOfStrings("Prefixes");
+  StringRef Name = R.getValueAsString("Name");
+  if (Prefixes.empty()) {
+PrefixLength = 0;
+return Name.str();
+  }
+  PrefixLength = Prefixes[0].size();
+  return (Twine(Prefixes[0]) + Twine(Name)).str();
+}
+
+static const std::string getOptionSpelling(const Record ) {
+  size_t PrefixLength;
+  return getOptionSpelling(R, PrefixLength);
+}
+
+static void emitNameUsingSpelling(raw_ostream , const Record ) {
+  size_t PrefixLength;
+  OS << "&";
+  write_cstring(OS, StringRef(getOptionSpelling(R, PrefixLength)));
+  OS << "[" << PrefixLength << "]";
+}
+
+class MarshallingKindInfo {
+public:
+  const Record 
+  const char *MacroName;
+  bool ShouldAlwaysEmit;
+  StringRef KeyPath;
+  StringRef DefaultValue;
+  StringRef NormalizedValuesScope;
+
+  void emit(raw_ostream ) const {
+write_cstring(OS, StringRef(getOptionSpelling(R)));
+OS << ", ";
+OS << ShouldAlwaysEmit;
+OS << ", ";
+OS << KeyPath;
+OS << ", ";
+emitScopedNormalizedValue(OS, DefaultValue);
+OS << ", ";
+emitSpecific(OS);
+  }
+
+  virtual Optional emitValueTable(raw_ostream ) const {
+return None;
+  }
+
+  virtual ~MarshallingKindInfo() = default;
+
+  static std::unique_ptr create(const Record );
+
+protected:
+  void emitScopedNormalizedValue(raw_ostream ,
+ StringRef NormalizedValue) const {
+if (!NormalizedValuesScope.empty())
+  OS << NormalizedValuesScope << "::";
+OS << NormalizedValue;
+  }
+
+  virtual void emitSpecific(raw_ostream ) const = 0;
+  MarshallingKindInfo(const Record , const char *MacroName)
+  : R(R), MacroName(MacroName) {}
+};
+
+class MarshallingFlagInfo final : public MarshallingKindInfo {
+public:
+  bool IsPositive;
+
+  void emitSpecific(raw_ostream ) const override { OS << IsPositive; }
+
+  static std::unique_ptr create(const Record ) {
+std::unique_ptr Ret(new MarshallingFlagInfo(R));
+Ret->IsPositive = R.getValueAsBit("IsPositive");
+return Ret;
+  }
+
+private:
+  MarshallingFlagInfo(const Record )
+  : MarshallingKindInfo(R, "OPTION_WITH_MARSHALLING_FLAG") {}
+};
+
+class MarshallingStringInfo final : public MarshallingKindInfo {
+public:
+  StringRef NormalizerRetTy;
+  StringRef Normalizer;
+  StringRef Denormalizer;
+  int TableIndex = -1;
+  std::vector Values;
+  std::vector NormalizedValues;
+  std::string ValueTableName;
+
+  static constexpr const char *ValueTablePreamble = R"(
+struct SimpleEnumValue {
+  const char *Name;
+  unsigned Value;
+};
+
+struct SimpleEnumValueTable {
+  const SimpleEnumValue *Table;
+  unsigned Size;
+};
+)";
+
+  static constexpr const char *ValueTablesDecl =
+  "static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
+
+  void emitSpecific(raw_ostream ) const override {
+emitScopedNormalizedValue(OS, NormalizerRetTy);
+OS << ", ";
+OS << Normalizer;
+OS << ", ";
+OS << Denormalizer;
+OS << ", ";
+OS << TableIndex;
+  }
+
+  Optional emitValueTable(raw_ostream ) const override {
+if (TableIndex == -1)
+  return {};
+OS << "static const SimpleEnumValue " << ValueTableName << "[] = {\n";
+for (unsigned I = 0, E = Values.size(); I != E; ++I) {
+  OS << "{";
+  write_cstring(OS, Values[I]);
+  OS << ",";
+  OS << "static_cast(";
+  emitScopedNormalizedValue(OS, NormalizedValues[I]);
+  OS << ")},";
+}
+OS << "};\n";
+return StringRef(ValueTableName);
+  }
+
+  static std::unique_ptr create(const Record ) {
+assert(!isa(R.getValueInit("NormalizerRetTy")) &&
+   "String options must have a type");
+
+std::unique_ptr Ret(new MarshallingStringInfo(R));
+Ret->NormalizerRetTy = 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 271006.
dang marked 5 inline comments as done and an inline comment as not done.
dang added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -10,11 +10,13 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
@@ -33,6 +35,208 @@
   return OS;
 }
 
+static const std::string getOptionSpelling(const Record ,
+   size_t ) {
+  std::vector Prefixes = R.getValueAsListOfStrings("Prefixes");
+  StringRef Name = R.getValueAsString("Name");
+  if (Prefixes.empty())
+return Name.str();
+  PrefixLength = Prefixes[0].size();
+  return (Twine(Prefixes[0]) + Twine(Name)).str();
+}
+
+static const std::string getOptionSpelling(const Record ) {
+  size_t PrefixLength;
+  return getOptionSpelling(R, PrefixLength);
+}
+
+static void emitNameUsingSpelling(raw_ostream , const Record ) {
+  size_t PrefixLength;
+  OS << "&";
+  write_cstring(OS, StringRef(getOptionSpelling(R, PrefixLength)));
+  OS << "[" << PrefixLength << "]";
+}
+
+class MarshallingKindInfo {
+public:
+  const Record 
+  const char *MacroName;
+  bool ShouldAlwaysEmit;
+  StringRef KeyPath;
+  StringRef DefaultValue;
+  StringRef NormalizedValuesScope;
+
+  void emit(raw_ostream ) const {
+write_cstring(OS, StringRef(getOptionSpelling(R)));
+OS << ", ";
+OS << ShouldAlwaysEmit;
+OS << ", ";
+OS << KeyPath;
+OS << ", ";
+emitScopedNormalizedValue(OS, DefaultValue);
+OS << ", ";
+emitSpecific(OS);
+  }
+
+  virtual Optional emitValueTable(raw_ostream ) const {
+return None;
+  }
+
+  virtual ~MarshallingKindInfo() = default;
+
+  static std::unique_ptr create(const Record );
+
+protected:
+  void emitScopedNormalizedValue(raw_ostream ,
+ StringRef NormalizedValue) const {
+if (!NormalizedValuesScope.empty())
+  OS << NormalizedValuesScope << "::";
+OS << NormalizedValue;
+  }
+
+  virtual void emitSpecific(raw_ostream ) const = 0;
+  MarshallingKindInfo(const Record , const char *MacroName)
+  : R(R), MacroName(MacroName) {}
+};
+
+class MarshallingFlagInfo final : public MarshallingKindInfo {
+public:
+  bool IsPositive;
+
+  void emitSpecific(raw_ostream ) const override { OS << IsPositive; }
+
+  static std::unique_ptr create(const Record ) {
+std::unique_ptr Ret(new MarshallingFlagInfo(R));
+Ret->IsPositive = R.getValueAsBit("IsPositive");
+return Ret;
+  }
+
+private:
+  MarshallingFlagInfo(const Record )
+  : MarshallingKindInfo(R, "OPTION_WITH_MARSHALLING_FLAG") {}
+};
+
+class MarshallingStringInfo final : public MarshallingKindInfo {
+public:
+  StringRef NormalizerRetTy;
+  StringRef Normalizer;
+  StringRef Denormalizer;
+  int TableIndex = -1;
+  std::vector Values;
+  std::vector NormalizedValues;
+  std::string ValueTableName;
+
+  static constexpr const char *ValueTablePreamble = R"(
+struct SimpleEnumValue {
+  const char *Name;
+  unsigned Value;
+};
+
+struct SimpleEnumValueTable {
+  const SimpleEnumValue *Table;
+  unsigned Size;
+};
+)";
+
+  static constexpr const char *ValueTablesDecl =
+  "static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
+
+  void emitSpecific(raw_ostream ) const override {
+emitScopedNormalizedValue(OS, NormalizerRetTy);
+OS << ", ";
+OS << Normalizer;
+OS << ", ";
+OS << Denormalizer;
+OS << ", ";
+OS << TableIndex;
+  }
+
+  Optional emitValueTable(raw_ostream ) const override {
+if (TableIndex == -1)
+  return {};
+OS << "static const SimpleEnumValue " << ValueTableName << "[] = {\n";
+for (unsigned I = 0, E = Values.size(); I != E; ++I) {
+  OS << "{";
+  write_cstring(OS, Values[I]);
+  OS << ",";
+  OS << "static_cast(";
+  emitScopedNormalizedValue(OS, NormalizedValues[I]);
+  OS << ")},";
+}
+OS << "};\n";
+return StringRef(ValueTableName);
+  }
+
+  static std::unique_ptr 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang marked 2 inline comments as done.
dang added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3845
+  IS_POSITIVE != DEFAULT_VALUE && this->KEYPATH != DEFAULT_VALUE)  
\
+Args.push_back(StringAllocator(Twine(PREFIX_TYPE[0]) + NAME));
+#include "clang/Driver/Options.inc"

dexonsmith wrote:
> Bigcheese wrote:
> > 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.
> I want to highlight @Bigcheese's comment again so it doesn't get lost. I 
> think a reasonable name would be `SPELLING`.
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



Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:351-354
+OS << "#define " << MacroName << "(V, D) .Case(V, D)\n";
+emitValueTable(OS, OptionName, R.getValueAsString("Values"),
+   NormalizedValuesScope, NormalizedValues);
+OS << "#undef " << MacroName << "\n";

dexonsmith wrote:
> 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(llvm::Reloc::Static)},
>   {"pic", static_cast(llvm::Reloc::_PIC)},
>   // ...
> };
> 
> static const EnumValue SomeOtherTable[] = {
>   {"name", static_cast(clang::SomeOtherEnumValue)},
>   // ...
> };
> 
> static const EnumValueTable *EnumValueTables[] = {
>   {, sizeof(RelocationModelTable) / sizeof(EnumValue)},
>   {, 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 extractSimpleEnum(
> llvm::opt::OptSpecifier Opt, int TableIndex,
> const ArgList , DiagnosticsEngine ) {
>   assert(TableIndex >= 0);
>   assert(TableIndex < EnumValueTablesSize);
>   const EnumValueTable  = *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(*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  = *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");
> }
> ```
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 270802.
dang marked an inline comment as done.
dang added a comment.

Implement constant table based marshaling for simple enum based options.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -10,11 +10,13 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
@@ -33,6 +35,182 @@
   return OS;
 }
 
+class MarshallingKindInfo {
+public:
+  const Record 
+  const char *MacroName;
+  bool ShouldAlwaysEmit;
+  StringRef KeyPath;
+  StringRef DefaultValue;
+  StringRef NormalizedValuesScope;
+
+  void emit(raw_ostream ) const {
+OS << ShouldAlwaysEmit;
+OS << ", ";
+OS << KeyPath;
+OS << ", ";
+emitScopedNormalizedValue(OS, DefaultValue);
+OS << ", ";
+emitSpecific(OS);
+  }
+
+  virtual Optional emitValueTable(raw_ostream ) const {
+return None;
+  }
+
+  virtual ~MarshallingKindInfo() = default;
+
+  static std::unique_ptr create(const Record );
+
+protected:
+  void emitScopedNormalizedValue(raw_ostream ,
+ StringRef NormalizedValue) const {
+if (!NormalizedValuesScope.empty())
+  OS << NormalizedValuesScope << "::";
+OS << NormalizedValue;
+  }
+
+  virtual void emitSpecific(raw_ostream ) const = 0;
+  MarshallingKindInfo(const Record , const char *MacroName)
+  : R(R), MacroName(MacroName) {}
+};
+
+class MarshallingFlagInfo final : public MarshallingKindInfo {
+public:
+  bool IsPositive;
+
+  void emitSpecific(raw_ostream ) const override { OS << IsPositive; }
+
+  static std::unique_ptr create(const Record ) {
+std::unique_ptr Ret(new MarshallingFlagInfo(R));
+Ret->IsPositive = R.getValueAsBit("IsPositive");
+return Ret;
+  }
+
+private:
+  MarshallingFlagInfo(const Record )
+  : MarshallingKindInfo(R, "OPTION_WITH_MARSHALLING_FLAG") {}
+};
+
+class MarshallingStringInfo final : public MarshallingKindInfo {
+public:
+  StringRef NormalizerRetTy;
+  StringRef Normalizer;
+  StringRef Denormalizer;
+  int TableIndex = -1;
+  std::vector Values;
+  std::vector NormalizedValues;
+  std::string ValueTableName;
+
+  static constexpr const char *ValueTablePreamble = R"(
+struct SimpleEnumValue {
+  const char *Name;
+  unsigned Value;
+};
+
+struct SimpleEnumValueTable {
+  const SimpleEnumValue *Table;
+  unsigned Size;
+};
+)";
+
+  static constexpr const char *ValueTablesDecl =
+  "static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
+
+  void emitSpecific(raw_ostream ) const override {
+emitScopedNormalizedValue(OS, NormalizerRetTy);
+OS << ", ";
+OS << Normalizer;
+OS << ", ";
+OS << Denormalizer;
+OS << ", ";
+OS << TableIndex;
+  }
+
+  Optional emitValueTable(raw_ostream ) const override {
+if (TableIndex == -1)
+  return {};
+OS << "static const SimpleEnumValue " << ValueTableName << "[] = {\n";
+for (unsigned I = 0, E = Values.size(); I != E; ++I) {
+  OS << "{\"" << Values[I] << "\",";
+  OS << "static_cast(";
+  emitScopedNormalizedValue(OS, NormalizedValues[I]);
+  OS << ")},";
+}
+OS << "};\n";
+return StringRef(ValueTableName);
+  }
+
+  static std::unique_ptr create(const Record ) {
+assert(!isa(R.getValueInit("NormalizerRetTy")) &&
+   "String options must have a type");
+
+std::unique_ptr Ret(new MarshallingStringInfo(R));
+Ret->NormalizerRetTy = R.getValueAsString("NormalizerRetTy");
+
+Ret->Normalizer = R.getValueAsString("Normalizer");
+Ret->Denormalizer = R.getValueAsString("Denormalizer");
+
+if (!isa(R.getValueInit("NormalizedValues"))) {
+  assert(!isa(R.getValueInit("Values")) &&
+ "Cannot provide normalized values for value-less options");
+  Ret->TableIndex = NextTableIndex++;
+  Ret->NormalizedValues = R.getValueAsListOfStrings("NormalizedValues");
+  Ret->Values.reserve(Ret->NormalizedValues.size());
+  Ret->ValueTableName = getOptionName(R) + "ValueTable";
+
+  StringRef ValuesStr = R.getValueAsString("Values");
+  for (;;) {
+size_t Idx = ValuesStr.find(',');
+if (Idx == StringRef::npos)
+  break;
+if (Idx > 0)
+

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3845
+  IS_POSITIVE != DEFAULT_VALUE && this->KEYPATH != DEFAULT_VALUE)  
\
+Args.push_back(StringAllocator(Twine(PREFIX_TYPE[0]) + NAME));
+#include "clang/Driver/Options.inc"

Bigcheese wrote:
> 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.
I want to highlight @Bigcheese's comment again so it doesn't get lost. I think 
a reasonable name would be `SPELLING`.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3903
+  Args.push_back(StringAllocator(Twine(PREFIX_TYPE[0]) + NAME));   
\
+  Args.push_back(StringAllocator(DENORMALIZER(this->KEYPATH)));
\
+}  
\

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



Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:327
+  OS << "\n";
+  OS << "#ifdef AUTO_NORMALIZING_OPTIONS";
+  OS << "\n\n";

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`.



Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:351-354
+OS << "#define " << MacroName << "(V, D) .Case(V, D)\n";
+emitValueTable(OS, OptionName, R.getValueAsString("Values"),
+   NormalizedValuesScope, NormalizedValues);
+OS << "#undef " << MacroName << "\n";

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(llvm::Reloc::Static)},
  {"pic", static_cast(llvm::Reloc::_PIC)},
  // ...
};

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

static const EnumValueTable *EnumValueTables[] = {
  {, sizeof(RelocationModelTable) / sizeof(EnumValue)},
  {, 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 extractSimpleEnum(
llvm::opt::OptSpecifier Opt, int TableIndex,
const ArgList , DiagnosticsEngine ) {
  assert(TableIndex >= 0);
  assert(TableIndex < EnumValueTablesSize);
  const EnumValueTable  = *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(*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  = *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");
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 270445.
dang edited the summary of this revision.
dang added a comment.

Implemented a draft of normalizer generation for simple enum based options


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -10,6 +10,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include 
@@ -33,6 +34,50 @@
   return OS;
 }
 
+static void emitMarshallingInfoFlag(raw_ostream , const Record ) {
+  OS << R.getValueAsBit("IsPositive");
+}
+
+static void emitMarshallingInfoString(raw_ostream , const Record ) {
+  OS << R.getValueAsString("Normalizer");
+  OS << ", ";
+  OS << R.getValueAsString("Denormalizer");
+}
+
+static void emitScopedNormalizedValue(raw_ostream ,
+  StringRef NormalizedValuesScope,
+  StringRef NormalizedValue) {
+  if (!NormalizedValuesScope.empty())
+OS << NormalizedValuesScope << "::";
+  OS << NormalizedValue;
+}
+
+static void emitValueTable(raw_ostream , StringRef OptionID,
+   StringRef Values, StringRef NormalizedValuesScope,
+   std::vector NormalizedValues) {
+  SmallVector SplitValues;
+  Values.split(SplitValues, ',');
+  assert(SplitValues.size() == NormalizedValues.size() &&
+ "The number of associated definitions doesn't match the number of "
+ "values");
+
+  SmallString<64> MacroName("HANDLE_");
+  MacroName += OptionID.upper();
+  MacroName += "_VALUES";
+  OS << "#ifdef " << MacroName << "\n";
+  for (unsigned I = 0, E = SplitValues.size(); I != E; ++I) {
+OS << MacroName << "(\"" << SplitValues[I] << "\",";
+emitScopedNormalizedValue(OS, NormalizedValuesScope, NormalizedValues[I]);
+OS << ")\n";
+  }
+  OS << "#endif\n";
+}
+
+struct MarshallingKindInfo {
+  const char *MacroName;
+  void (*Emit)(raw_ostream , const Record );
+};
+
 /// 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.
@@ -135,12 +180,8 @@
 
   OS << "//\n";
   OS << "// Options\n\n";
-  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
-const Record  = *Opts[i];
-
-// Start a single option entry.
-OS << "OPTION(";
 
+  auto WriteOptRecordFields = [&](raw_ostream , const Record ) {
 // The option prefix;
 std::vector prf = R.getValueAsListOfStrings("Prefixes");
 OS << Prefixes[PrefixKeyT(prf.begin(), prf.end())] << ", ";
@@ -223,11 +264,119 @@
   write_cstring(OS, R.getValueAsString("Values"));
 else
   OS << "nullptr";
+  };
 
+  std::vector OptsWithMarshalling;
+  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
+const Record  = *Opts[i];
+
+// Start a single option entry.
+OS << "OPTION(";
+WriteOptRecordFields(OS, R);
 OS << ")\n";
+if (!isa(R.getValueInit("MarshallingKind")))
+  OptsWithMarshalling.push_back();
   }
   OS << "#endif // OPTION\n";
 
+  std::vector AutoNormalizableOpts;
+  for (unsigned I = 0, E = OptsWithMarshalling.size(); I != E; ++I) {
+const Record  = *OptsWithMarshalling[I];
+assert(!isa(R.getValueInit("KeyPath")) &&
+   !isa(R.getValueInit("DefaultValue")) &&
+   "Must provide at least a key-path and a default value for emitting "
+   "marshalling information");
+StringRef KindStr = R.getValueAsString("MarshallingKind");
+auto KindInfo = StringSwitch(KindStr)
+.Case("flag", {"OPTION_WITH_MARSHALLING_FLAG",
+   })
+.Case("string", {"OPTION_WITH_MARSHALLING_STRING",
+ })
+.Default({"", nullptr});
+StringRef NormalizedValuesScope;
+if (!isa(R.getValueInit("NormalizedValuesScope")))
+  NormalizedValuesScope = R.getValueAsString("NormalizedValuesScope");
+
+OS << "#ifdef " << KindInfo.MacroName << "\n";
+OS << KindInfo.MacroName << "(";
+WriteOptRecordFields(OS, R);
+OS << ", ";
+OS << R.getValueAsBit("ShouldAlwaysEmit");
+OS << ", ";
+OS << R.getValueAsString("KeyPath");
+OS << 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 270117.
dang added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -9,7 +9,9 @@
 #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/Support/raw_ostream.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include 
@@ -33,6 +35,50 @@
   return OS;
 }
 
+static void emitMarshallingInfoFlag(raw_ostream , const Record ) {
+  OS << R.getValueAsBit("IsPositive");
+}
+
+static void emitMarshallingInfoString(raw_ostream , const Record ) {
+  OS << R.getValueAsString("Normalizer");
+  OS << ", ";
+  OS << R.getValueAsString("Denormalizer");
+}
+
+static void emitScopedNormalizedValue(raw_ostream ,
+  StringRef NormalizedValuesScope,
+  StringRef NormalizedValue) {
+  if (!NormalizedValuesScope.empty())
+OS << NormalizedValuesScope << "::";
+  OS << NormalizedValue;
+}
+
+static void emitValueTable(raw_ostream , StringRef OptionID,
+   StringRef Values, StringRef NormalizedValuesScope,
+   std::vector NormalizedValues) {
+  SmallVector SplitValues;
+  Values.split(SplitValues, ',');
+  assert(SplitValues.size() == NormalizedValues.size() &&
+ "The number of associated definitions doesn't match the number of "
+ "values");
+
+  SmallString<64> MacroName("HANDLE_");
+  MacroName += OptionID.upper();
+  MacroName += "_VALUES";
+  OS << "#ifdef " << MacroName << "\n";
+  for (unsigned I = 0, E = SplitValues.size(); I != E; ++I) {
+OS << MacroName << "(\"" << SplitValues[I] << "\",";
+emitScopedNormalizedValue(OS, NormalizedValuesScope, NormalizedValues[I]);
+OS << ")\n";
+  }
+  OS << "#endif\n";
+}
+
+struct MarshallingKindInfo {
+  const char *MacroName;
+  void (*Emit)(raw_ostream , const Record );
+};
+
 /// 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.
@@ -135,12 +181,8 @@
 
   OS << "//\n";
   OS << "// Options\n\n";
-  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
-const Record  = *Opts[i];
-
-// Start a single option entry.
-OS << "OPTION(";
 
+  auto WriteOptRecordFields = [&](raw_ostream , const Record ) {
 // The option prefix;
 std::vector prf = R.getValueAsListOfStrings("Prefixes");
 OS << Prefixes[PrefixKeyT(prf.begin(), prf.end())] << ", ";
@@ -223,11 +265,62 @@
   write_cstring(OS, R.getValueAsString("Values"));
 else
   OS << "nullptr";
+  };
 
+  std::vector OptsWithMarshalling;
+  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
+const Record  = *Opts[i];
+
+// Start a single option entry.
+OS << "OPTION(";
+WriteOptRecordFields(OS, R);
 OS << ")\n";
+if (!isa(R.getValueInit("MarshallingKind")))
+  OptsWithMarshalling.push_back();
   }
   OS << "#endif // OPTION\n";
 
+  for (unsigned I = 0, E = OptsWithMarshalling.size(); I != E; ++I) {
+const Record  = *OptsWithMarshalling[I];
+assert(!isa(R.getValueInit("KeyPath")) &&
+   !isa(R.getValueInit("DefaultValue")) &&
+   "Must provide at least a key-path and a default value for emitting "
+   "marshalling information");
+StringRef KindStr = R.getValueAsString("MarshallingKind");
+auto KindInfo = StringSwitch(KindStr)
+.Case("flag", {"OPTION_WITH_MARSHALLING_FLAG",
+   })
+.Case("string", {"OPTION_WITH_MARSHALLING_STRING",
+ })
+.Default({"", nullptr});
+StringRef NormalizedValuesScope;
+if (!isa(R.getValueInit("NormalizedValuesScope")))
+  NormalizedValuesScope = R.getValueAsString("NormalizedValuesScope");
+
+OS << "#ifdef " << KindInfo.MacroName << "\n";
+OS << KindInfo.MacroName << "(";
+WriteOptRecordFields(OS, R);
+OS << ", ";
+OS << R.getValueAsBit("ShouldAlwaysEmit");
+OS << ", ";
+OS << 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: clang/include/clang/Driver/CC1Options.td:216-221
 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">,
+  MarshallingInfoString<"CodeGenOpts.RelocationModel", "llvm::Reloc::PIC_">,
+  Normalizer<"normalizeRelocationModel">, 
Denormalizer<"denormalizeRelocationModel">,
+  ValuesAssociatedDefinitions<["llvm::Reloc::Static", "llvm::Reloc::PIC_", 
"llvm::Reloc::ROPI",
+  "llvm::Reloc::RWPI", "llvm::Reloc::ROPI_RWPI", "llvm::Reloc::DynamicNoPIC"]>;

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`)?



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:125-127
+static std::string normalizeTriple(const Arg *Arg, const ArgList ,
+   DiagnosticsEngine ,
+   StringRef DefaultTriple) {

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:150-158
+static const char *denormalizeRelocationModel(llvm::Reloc::Model RM) {
+  switch (RM) {
+#define HANDLE_MRELOCATION_MODEL_VALUES(V, D)  
\
+  case D:  
\
+return V;
+#include "clang/Driver/Options.inc"
+#undef HANDLE_MRELOCATION_MODEL_VALUES

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<>`).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 269926.
dang added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -9,6 +9,7 @@
 #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"
@@ -33,6 +34,40 @@
   return OS;
 }
 
+static void emitMarshallingInfoFlag(raw_ostream , const Record ) {
+  OS << R.getValueAsBit("IsPositive");
+}
+
+static void emitMarshallingInfoString(raw_ostream , const Record ) {
+  OS << R.getValueAsString("Normalizer");
+  OS << ", ";
+  OS << R.getValueAsString("Denormalizer");
+}
+
+static void emitValueTable(raw_ostream , StringRef OptionID,
+   StringRef Values,
+   std::vector AssociatedDefinitions) {
+  SmallVector SplitValues;
+  Values.split(SplitValues, ',');
+  assert(SplitValues.size() == AssociatedDefinitions.size() &&
+ "The number of associated definitions doesn't match the number of "
+ "values");
+
+  SmallString<64> MacroName("HANDLE_");
+  MacroName += OptionID.upper();
+  MacroName += "_VALUES";
+  OS << "#ifdef " << MacroName << "\n";
+  for (unsigned I = 0, E = SplitValues.size(); I != E; ++I)
+OS << MacroName << "(\"" << SplitValues[I] << "\","
+   << AssociatedDefinitions[I] << ")\n";
+  OS << "#endif\n";
+}
+
+struct MarshallingKindInfo {
+  const char *MacroName;
+  void (*Emit)(raw_ostream , const Record );
+};
+
 /// 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.
@@ -135,12 +170,8 @@
 
   OS << "//\n";
   OS << "// Options\n\n";
-  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
-const Record  = *Opts[i];
-
-// Start a single option entry.
-OS << "OPTION(";
 
+  auto WriteOptRecordFields = [&](raw_ostream , const Record ) {
 // The option prefix;
 std::vector prf = R.getValueAsListOfStrings("Prefixes");
 OS << Prefixes[PrefixKeyT(prf.begin(), prf.end())] << ", ";
@@ -223,11 +254,56 @@
   write_cstring(OS, R.getValueAsString("Values"));
 else
   OS << "nullptr";
+  };
 
+  std::vector OptsWithMarshalling;
+  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
+const Record  = *Opts[i];
+
+// Start a single option entry.
+OS << "OPTION(";
+WriteOptRecordFields(OS, R);
 OS << ")\n";
+if (!isa(R.getValueInit("MarshallingKind")))
+  OptsWithMarshalling.push_back();
   }
   OS << "#endif // OPTION\n";
 
+  for (unsigned I = 0, E = OptsWithMarshalling.size(); I != E; ++I) {
+const Record  = *OptsWithMarshalling[I];
+assert(!isa(R.getValueInit("KeyPath")) &&
+   !isa(R.getValueInit("DefaultValue")) &&
+   "Must provide at least a key-path and a default value for emitting "
+   "marshalling information");
+StringRef KindStr = R.getValueAsString("MarshallingKind");
+auto KindInfo = StringSwitch(KindStr)
+.Case("flag", {"OPTION_WITH_MARSHALLING_FLAG",
+   })
+.Case("string", {"OPTION_WITH_MARSHALLING_STRING",
+ })
+.Default({"", nullptr});
+OS << "#ifdef " << KindInfo.MacroName << "\n";
+OS << KindInfo.MacroName << "(";
+WriteOptRecordFields(OS, R);
+OS << ", ";
+OS << R.getValueAsBit("ShouldAlwaysEmit");
+OS << ", ";
+OS << R.getValueAsString("KeyPath");
+OS << ", ";
+OS << R.getValueAsString("DefaultValue");
+OS << ",";
+KindInfo.Emit(OS, R);
+OS << ")\n";
+OS << "#endif\n";
+
+if (!isa(R.getValueInit("ValuesAssociatedDefinitions"))) {
+  assert(!isa(R.getValueInit("Values")) &&
+ "Cannot provide associated definitions for value-less options");
+  emitValueTable(OS, getOptionName(R), R.getValueAsString("Values"),
+ R.getValueAsListOfStrings("ValuesAssociatedDefinitions"));
+}
+  }
+
   OS << "\n";
   OS << "#ifdef OPTTABLE_ARG_INIT\n";
   OS << 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3647
+ DiagnosticsEngine ) {
+#define OPTION_WITH_MARSHALLING
+#define OPTION_WITH_MARSHALLING_FLAG(PREFIX_TYPE, NAME, ID, KIND, GROUP,   
\

dang wrote:
> dexonsmith wrote:
> > Seems like `Options.inc` could provide this as a default definition, not 
> > sure if that seems error-prone?
> 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.
Right, I just mean you shouldn't have to define `OPTION_WITH_MARSHALLING` here.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3665-3667
+#undef OPTION_WITH_MARSHALLING_STRING
+#undef OPTION_WITH_MARSHALLING_FLAG
+#undef OPTION_WITH_MARSHALLING

dang wrote:
> dexonsmith wrote:
> > 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?
> 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.
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).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang marked 3 inline comments as done.
dang added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:137-142
+.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)

dexonsmith wrote:
> 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`)? The current patch adds a value 
> table; my first suggestion leaves us even; but maybe we can get rid of one.
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.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3647
+ DiagnosticsEngine ) {
+#define OPTION_WITH_MARSHALLING
+#define OPTION_WITH_MARSHALLING_FLAG(PREFIX_TYPE, NAME, ID, KIND, GROUP,   
\

dexonsmith wrote:
> Seems like `Options.inc` could provide this as a default definition, not sure 
> if that seems error-prone?
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.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3665-3667
+#undef OPTION_WITH_MARSHALLING_STRING
+#undef OPTION_WITH_MARSHALLING_FLAG
+#undef OPTION_WITH_MARSHALLING

dexonsmith wrote:
> 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?
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

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




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:137-142
+.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)

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`)? The current patch adds a value table; 
my first suggestion leaves us even; but maybe we can get rid of one.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3647
+ DiagnosticsEngine ) {
+#define OPTION_WITH_MARSHALLING
+#define OPTION_WITH_MARSHALLING_FLAG(PREFIX_TYPE, NAME, ID, KIND, GROUP,   
\

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3665-3667
+#undef OPTION_WITH_MARSHALLING_STRING
+#undef OPTION_WITH_MARSHALLING_FLAG
+#undef OPTION_WITH_MARSHALLING

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?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 268872.
dang added a comment.

This is the good diff


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -9,6 +9,7 @@
 #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"
@@ -33,6 +34,20 @@
   return OS;
 }
 
+static void emitMarshallingInfoFlag(raw_ostream , const Record *R) {
+  OS << R->getValueAsBit("IsPositive");
+  OS << ",";
+  OS << R->getValueAsString("DefaultValue");
+}
+
+static void emitMarshallingInfoString(raw_ostream , const Record *R) {
+  OS << R->getValueAsString("DefaultValue");
+  OS << ", ";
+  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.
@@ -135,12 +150,8 @@
 
   OS << "//\n";
   OS << "// Options\n\n";
-  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
-const Record  = *Opts[i];
-
-// Start a single option entry.
-OS << "OPTION(";
 
+  auto WriteOptRecordFields = [&](raw_ostream , const Record ) {
 // The option prefix;
 std::vector prf = R.getValueAsListOfStrings("Prefixes");
 OS << Prefixes[PrefixKeyT(prf.begin(), prf.end())] << ", ";
@@ -223,11 +234,48 @@
   write_cstring(OS, R.getValueAsString("Values"));
 else
   OS << "nullptr";
+  };
+
+  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
+const Record  = *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  = *Opts[I];
+
+if (!isa(R.getValueInit("MarshallingInfo"))) {
+  Record *MarshallingInfoRecord =
+  cast(R.getValueInit("MarshallingInfo"))->getDef();
+  StringRef KindStr = MarshallingInfoRecord->getValueAsString("Kind");
+  auto KindInfoPair =
+  StringSwitch>>(
+  KindStr)
+  .Case("flag", std::make_pair("OPTION_WITH_MARSHALLING_FLAG",
+   ))
+  .Case("string", std::make_pair("OPTION_WITH_MARSHALLING_STRING",
+ ))
+  .Default(std::make_pair("", nullptr));
+  OS << KindInfoPair.first << "(";
+  WriteOptRecordFields(OS, R);
+  OS << ", ";
+  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";
Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -80,6 +80,18 @@
   list Flags = [];
 }
 
+// Add support for generating marshalling code
+class OptionMarshallingInfo {
+  string Kind = kind;
+  bit ShouldAlwaysEmit = 0;
+  code KeyPath = keypath;
+  // Used by the Flag option kind.
+  bit IsPositive = ?;
+  code DefaultValue = ?;
+  code Normalizer = ?;
+  code Denormalizer = ?;
+}
+
 // Define the option class.
 
 class Option prefixes, string name, OptionKind kind> {
@@ -97,6 +109,7 @@
   OptionGroup Group = ?;
   Option Alias = ?;
   list AliasArgs = [];
+  OptionMarshallingInfo MarshallingInfo = ?;
 }
 
 // Helpers for defining options.
@@ -130,6 +143,26 @@
 class Values { string Values = value; }
 class ValuesCode { code ValuesCode = valuecode; }
 
+class MarshallingInfo { OptionMarshallingInfo MarshallingInfo = info; }
+class MarshallingFlag
+  : OptionMarshallingInfo<"flag", keypath> {
+  bit IsPositive = ispositive;
+  code DefaultValue = defaultvalue;
+}
+class MarshallingFlagAlwaysEmit
+  : MarshallingFlag {
+  let ShouldAlwaysEmit = 1;
+}
+class MarshallingString
+  : OptionMarshallingInfo<"string", keypath> {
+  code DefaultValue = 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 268869.
dang marked an inline comment as done.
dang added a comment.

Updating the patch with the correct merge base


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -9,6 +9,7 @@
 #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"
@@ -33,6 +34,20 @@
   return OS;
 }
 
+static void emitMarshallingInfoFlag(raw_ostream , const Record *R) {
+  OS << R->getValueAsBit("IsPositive");
+  OS << ",";
+  OS << R->getValueAsString("DefaultValue");
+}
+
+static void emitMarshallingInfoString(raw_ostream , const Record *R) {
+  OS << R->getValueAsString("DefaultValue");
+  OS << ", ";
+  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.
@@ -135,12 +150,8 @@
 
   OS << "//\n";
   OS << "// Options\n\n";
-  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
-const Record  = *Opts[i];
-
-// Start a single option entry.
-OS << "OPTION(";
 
+  auto WriteOptRecordFields = [&](raw_ostream , const Record ) {
 // The option prefix;
 std::vector prf = R.getValueAsListOfStrings("Prefixes");
 OS << Prefixes[PrefixKeyT(prf.begin(), prf.end())] << ", ";
@@ -223,11 +234,48 @@
   write_cstring(OS, R.getValueAsString("Values"));
 else
   OS << "nullptr";
+  };
+
+  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
+const Record  = *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  = *Opts[I];
+
+if (!isa(R.getValueInit("MarshallingInfo"))) {
+  Record *MarshallingInfoRecord =
+  cast(R.getValueInit("MarshallingInfo"))->getDef();
+  StringRef KindStr = MarshallingInfoRecord->getValueAsString("Kind");
+  auto KindInfoPair =
+  StringSwitch>>(
+  KindStr)
+  .Case("flag", std::make_pair("OPTION_WITH_MARSHALLING_FLAG",
+   ))
+  .Case("string", std::make_pair("OPTION_WITH_MARSHALLING_STRING",
+ ))
+  .Default(std::make_pair("", nullptr));
+  OS << KindInfoPair.first << "(";
+  WriteOptRecordFields(OS, R);
+  OS << ", ";
+  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";
Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -80,6 +80,18 @@
   list Flags = [];
 }
 
+// Add support for generating marshalling code
+class OptionMarshallingInfo {
+  string Kind = kind;
+  bit ShouldAlwaysEmit = 0;
+  code KeyPath = keypath;
+  // Used by the Flag option kind.
+  bit IsPositive = ?;
+  code DefaultValue = ?;
+  code Normalizer = ?;
+  code Denormalizer = ?;
+}
+
 // Define the option class.
 
 class Option prefixes, string name, OptionKind kind> {
@@ -97,6 +109,7 @@
   OptionGroup Group = ?;
   Option Alias = ?;
   list AliasArgs = [];
+  OptionMarshallingInfo MarshallingInfo = ?;
 }
 
 // Helpers for defining options.
@@ -130,6 +143,26 @@
 class Values { string Values = value; }
 class ValuesCode { code ValuesCode = valuecode; }
 
+class MarshallingInfo { OptionMarshallingInfo MarshallingInfo = info; }
+class MarshallingFlag
+  : OptionMarshallingInfo<"flag", keypath> {
+  bit IsPositive = ispositive;
+  code DefaultValue = defaultvalue;
+}
+class MarshallingFlagAlwaysEmit
+  : MarshallingFlag {
+  let ShouldAlwaysEmit = 1;
+}
+class MarshallingString
+  : 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang marked 4 inline comments as done.
dang added inline comments.



Comment at: clang/include/clang/Frontend/CompilerInvocation.h:248
+  /// \returns - True if parsing was successful, false otherwise
+  bool parseSimpleArgs(const llvm::opt::ArgList ,
+   DiagnosticsEngine );

Bigcheese wrote:
> Is there a reason for this to be a member of `CompilerInvocation`? The rest 
> of the argument parsing functions are static file local functions.
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.



Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:41
+TEST_F(CC1CommandLineGenerationTest, CanGenerateCC1CommandLineFlag) {
+  const char *Args[] = {"clang", "-xc++", "-fmodules-strict-context-hash", 
"-"};
+

Bigcheese wrote:
> 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.
`-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.



Comment at: llvm/include/llvm/Option/OptParser.td:167-171
+class MarshallingEnum enumvalues>
+  : OptionMarshallingInfo<"enum", keypath> {
+  code DefaultValue = defaultvalue;
+  listEnumValues = enumvalues;
+}

Bigcheese wrote:
> 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.
Yeah, I forgot to remove it, since the normalizer scheme subsumes the 
functionality.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 268746.
dang marked 3 inline comments as done.
dang added a comment.

Address some code review feedback


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -9,6 +9,7 @@
 #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"
@@ -33,18 +34,18 @@
   return OS;
 }
 
-static void emitMarshallingInfo(raw_ostream , const Record ) {
-  OS << R.getValueAsString("KeyPath");
+static void emitMarshallingInfoFlag(raw_ostream , const Record *R) {
+  OS << R->getValueAsBit("IsPositive");
+  OS << ",";
+  OS << R->getValueAsString("DefaultValue");
+}
+
+static void emitMarshallingInfoString(raw_ostream , const Record *R) {
+  OS << R->getValueAsString("DefaultValue");
   OS << ", ";
-  if (!isa(R.getValueInit("IsPositive")))
-OS << R.getValueAsBit("IsPositive");
-  else
-OS << "INVALID";
+  OS << R->getValueAsString("Normalizer");
   OS << ", ";
-  if (!isa(R.getValueInit("DefaultValue")))
-OS << R.getValueAsString("DefaultValue");
-  else
-OS << "INVALID";
+  OS << R->getValueAsString("Denormalizer");
 }
 
 /// OptParserEmitter - This tablegen backend takes an input .td file
@@ -246,15 +247,30 @@
   OS << "#endif // OPTION\n";
 
   OS << "#ifdef OPTION_WITH_MARSHALLING\n";
-  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
-const Record  = *Opts[i];
+  for (unsigned I = 0, E = Opts.size(); I != E; ++I) {
+const Record  = *Opts[I];
 
 if (!isa(R.getValueInit("MarshallingInfo"))) {
-  OS << "OPTION_WITH_MARSHALLING(";
+  Record *MarshallingInfoRecord =
+  cast(R.getValueInit("MarshallingInfo"))->getDef();
+  StringRef KindStr = MarshallingInfoRecord->getValueAsString("Kind");
+  auto KindInfoPair =
+  StringSwitch>>(
+  KindStr)
+  .Case("flag", std::make_pair("OPTION_WITH_MARSHALLING_FLAG",
+   ))
+  .Case("string", std::make_pair("OPTION_WITH_MARSHALLING_STRING",
+ ))
+  .Default(std::make_pair("", nullptr));
+  OS << KindInfoPair.first << "(";
   WriteOptRecordFields(OS, R);
   OS << ", ";
-  emitMarshallingInfo(
-  OS, *cast(R.getValueInit("MarshallingInfo"))->getDef());
+  OS << MarshallingInfoRecord->getValueAsBit("ShouldAlwaysEmit");
+  OS << ", ";
+  OS << MarshallingInfoRecord->getValueAsString("KeyPath");
+  OS << ", ";
+  KindInfoPair.second(OS, MarshallingInfoRecord);
   OS << ")\n";
 }
   }
Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -81,13 +81,15 @@
 }
 
 // Add support for generating marshalling code
-
-class OptionMarshallingInfo {
+class OptionMarshallingInfo {
+  string Kind = kind;
+  bit ShouldAlwaysEmit = 0;
   code KeyPath = keypath;
   // Used by the Flag option kind.
   bit IsPositive = ?;
   code DefaultValue = ?;
-  list EnumValues = ?;
+  code Normalizer = ?;
+  code Denormalizer = ?;
 }
 
 // Define the option class.
@@ -143,20 +145,24 @@
 
 class MarshallingInfo { OptionMarshallingInfo MarshallingInfo = info; }
 class MarshallingFlag
-  : OptionMarshallingInfo {
+  : OptionMarshallingInfo<"flag", keypath> {
   bit IsPositive = ispositive;
   code DefaultValue = defaultvalue;
 }
-class MarshallingString
-  : OptionMarshallingInfo {
-  code DefaultValue = defaultvalue;
+class MarshallingFlagAlwaysEmit
+  : MarshallingFlag {
+  let ShouldAlwaysEmit = 1;
 }
-class MarshallingEnum enumvalues>
-  : OptionMarshallingInfo {
+class MarshallingString
+  : OptionMarshallingInfo<"string", keypath> {
   code DefaultValue = defaultvalue;
-  listEnumValues = enumvalues;
+  code Normalizer = normalizer;
+  code Denormalizer = denormalizer;
+}
+class MarshallingStringAlwaysEmit
+  : MarshallingString {
+  let ShouldAlwaysEmit = 1;
 }
-
 // Predefined options.
 
 // FIXME: Have generator validate that these appear in correct position (and
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added inline comments.



Comment at: clang/include/clang/Frontend/CompilerInvocation.h:248
+  /// \returns - True if parsing was successful, false otherwise
+  bool parseSimpleArgs(const llvm::opt::ArgList ,
+   DiagnosticsEngine );

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3658
  MissingArgCount, IncludedFlagsBitmask);
+
   LangOptions  = *Res.getLangOpts();

Put formatting fixups in a separate commit.



Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:41
+TEST_F(CC1CommandLineGenerationTest, CanGenerateCC1CommandLineFlag) {
+  const char *Args[] = {"clang", "-xc++", "-fmodules-strict-context-hash", 
"-"};
+

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.



Comment at: llvm/include/llvm/Option/OptParser.td:153
+}
+class MarshallingFlagRequired
+  : MarshallingFlag {

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.



Comment at: llvm/include/llvm/Option/OptParser.td:167-171
+class MarshallingEnum enumvalues>
+  : OptionMarshallingInfo<"enum", keypath> {
+  code DefaultValue = defaultvalue;
+  listEnumValues = enumvalues;
+}

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang marked 5 inline comments as done.
dang added inline comments.



Comment at: clang/include/clang/Frontend/CompilerInvocation.h:156
   /// \param [out] Res - The resulting invocation.
+  /// \param [in] CommandLineArgs - Array of argument strings, this should not
+  /// contain "-cc1".

Bigcheese wrote:
> Is this really a should not, or is it must not?
You're right updated the wording to be more accurate



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3607
+VALUES, KEYPATH, IS_POSITIVE, DEFAULT_VALUE)   
\
+  if (Option::KIND##Class == Option::FlagClass)
\
+Res.KEYPATH = Args.hasArg(OPT_##ID) && IS_POSITIVE;

Bigcheese wrote:
> 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.
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)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 266195.
dang added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -9,6 +9,7 @@
 #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"
@@ -33,6 +34,22 @@
   return OS;
 }
 
+static void emitMarshallingInfoFlag(raw_ostream , const Record *R) {
+  OS << R->getValueAsBit("IsPositive");
+  OS << ",";
+  OS << R->getValueAsString("DefaultValue");
+}
+
+static void emitMarshallingInfoString(raw_ostream , const Record *R) {
+  OS << R->getValueAsString("DefaultValue");
+  OS << ", ";
+  OS << R->getValueAsString("Normalizer");
+  OS << ", ";
+  OS << R->getValueAsString("Denormalizer");
+}
+
+static void emitMarshallingInfoEnum(raw_ostream , const Record *R) {}
+
 /// 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.
@@ -135,12 +152,8 @@
 
   OS << "//\n";
   OS << "// Options\n\n";
-  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
-const Record  = *Opts[i];
-
-// Start a single option entry.
-OS << "OPTION(";
 
+  auto WriteOptRecordFields = [&](raw_ostream , const Record ) {
 // The option prefix;
 std::vector prf = R.getValueAsListOfStrings("Prefixes");
 OS << Prefixes[PrefixKeyT(prf.begin(), prf.end())] << ", ";
@@ -149,7 +162,7 @@
 write_cstring(OS, R.getValueAsString("Name"));
 
 // The option identifier name.
-OS  << ", "<< getOptionName(R);
+OS << ", " << getOptionName(R);
 
 // The option kind.
 OS << ", " << R.getValueAsDef("Kind")->getValueAsString("Name");
@@ -190,8 +203,7 @@
 int NumFlags = 0;
 const ListInit *LI = R.getValueAsListInit("Flags");
 for (Init *I : *LI)
-  OS << (NumFlags++ ? " | " : "")
- << cast(I)->getDef()->getName();
+  OS << (NumFlags++ ? " | " : "") << cast(I)->getDef()->getName();
 if (GroupFlags) {
   for (Init *I : *GroupFlags)
 OS << (NumFlags++ ? " | " : "")
@@ -224,11 +236,50 @@
   write_cstring(OS, R.getValueAsString("Values"));
 else
   OS << "nullptr";
+  };
 
+  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
+const Record  = *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  = *Opts[I];
+
+if (!isa(R.getValueInit("MarshallingInfo"))) {
+  Record *MarshallingInfoRecord =
+  cast(R.getValueInit("MarshallingInfo"))->getDef();
+  StringRef KindStr = MarshallingInfoRecord->getValueAsString("Kind");
+  auto KindInfoPair =
+  StringSwitch>>(
+  KindStr)
+  .Case("flag", std::make_pair("OPTION_WITH_MARSHALLING_FLAG",
+   ))
+  .Case("string", std::make_pair("OPTION_WITH_MARSHALLING_STRING",
+ ))
+  .Case("enum", std::make_pair("OPTION_WITH_MARSHALLING_ENUM",
+   ))
+  .Default(std::make_pair("", nullptr));
+  OS << KindInfoPair.first << "(";
+  WriteOptRecordFields(OS, R);
+  OS << ", ";
+  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";
Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -80,6 +80,19 @@
   list Flags = [];
 }
 
+// Add support for generating marshalling code
+class OptionMarshallingInfo {
+  string Kind = kind;
+  bit 

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added inline comments.



Comment at: clang/include/clang/Frontend/CompilerInvocation.h:156
   /// \param [out] Res - The resulting invocation.
+  /// \param [in] CommandLineArgs - Array of argument strings, this should not
+  /// contain "-cc1".

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3604-3610
+#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

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3607
+VALUES, KEYPATH, IS_POSITIVE, DEFAULT_VALUE)   
\
+  if (Option::KIND##Class == Option::FlagClass)
\
+Res.KEYPATH = Args.hasArg(OPT_##ID) && IS_POSITIVE;

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.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3845
+  IS_POSITIVE != DEFAULT_VALUE && this->KEYPATH != DEFAULT_VALUE)  
\
+Args.push_back(StringAllocator(Twine(PREFIX_TYPE[0]) + NAME));
+#include "clang/Driver/Options.inc"

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits